Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions property_core/enrichment.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ async def _fetch_postcode(postcode: str) -> None:
# attaching a neighbouring property's EPC.
cert_cache: Dict[str, Any] = {}
ambiguous = 0
by_method: Dict[str, int] = {}

for postcode, indices in postcode_groups.items():
page = postcode_pages.get(postcode)
Expand Down Expand Up @@ -146,6 +147,7 @@ async def _fetch_postcode(postcode: str) -> None:
# confidence tier cannot silently be reported as certainty.
match_score = selected.confidence
comp.epc_match_method = selected.method
by_method[selected.method] = by_method.get(selected.method, 0) + 1
floor_sqm = match.floor_area
price = comp.price

Expand All @@ -169,6 +171,20 @@ async def _fetch_postcode(postcode: str) -> None:
comp.price_per_sqm = None
comp.price_per_sqft = None

# `ambiguous` was counted and never read, so how often selection refused --
# and therefore what any change to it is worth -- could not be measured from
# anything this function left behind. Logged with the method breakdown so a
# before/after is readable without instrumenting a release.
if ambiguous or by_method:
_log.info(
"EPC enrichment: %d matched (%s), %d refused as ambiguous, "
"%d postcode(s)",
sum(by_method.values()),
", ".join(f"{m}={n}" for m, n in sorted(by_method.items())) or "none",
ambiguous,
len(postcode_groups),
)

return comps


Expand Down
95 changes: 91 additions & 4 deletions property_core/epc/selection.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
"""Safe candidate selection.

v1.14 contract — deliberately only three ways to select a certificate:
v1.14 contract — deliberately only four ways to select a certificate:

1. exact UPRN match (exactly one candidate carries it),
2. exact normalized full-address equality, or
2. exact normalized full-address equality,
3. the same, after canonicalizing a LEADING "Flat <n>" / "Apartment <n>"
designator to one token (see _canon),
designator to one token (see _canon), or
4. several candidates proven to be ONE property -- an agreed non-empty UPRN
and agreed canonical address -- in which case the newest certificate by
registration date wins (see _one_property_latest),

and otherwise EPCAmbiguousMatchError.

Rule 4 is not a fifth relaxation of the kind catalogued below. Those all
invented a winner from partial evidence about *which property*. This one adds no
property evidence at all: it applies only once identity is already established
by rules 1-3's standard, and then chooses among that single property's own
certificate history, which is a thing properties genuinely have -- re-certified
on every sale and let.

There is no structured street/building/unit acceptance path. One existed and was
repaired four times, each round finding a new way for partial evidence to look
sufficient:
Expand Down Expand Up @@ -53,6 +63,7 @@

import re
from dataclasses import dataclass
from datetime import date
from typing import Iterable, Optional

from property_core.epc.errors import EPCAmbiguousMatchError
Expand Down Expand Up @@ -89,6 +100,64 @@ def _canon(s: Optional[str]) -> str:
return _DESIGNATOR_RE.sub(r"flat \1", _norm(s), count=1)


def _registration_date(row: EPCSearchRow) -> Optional[date]:
"""The row's registration date, or None if it cannot be ordered.

Requires a canonical ISO round trip. Comparing the strings directly would
be safe only for `YYYY-MM-DD`, and the repo has the scar for it: an
unvalidated date once sorted after a real one and was read as "beyond
coverage" (see `ppd_source.validate_date_range`). Anything unparseable
yields None, which refuses rather than orders arbitrarily.
"""
text = (row.registration_date or "").strip()
if not text:
return None
try:
parsed = date.fromisoformat(text)
except (TypeError, ValueError):
return None
return parsed if parsed.isoformat() == text else None


def _one_property_latest(rows: list[EPCSearchRow]) -> Optional[EPCSearchRow]:
"""The newest certificate, when every row is provably the SAME property.

Properties are re-certified on sale and on let, so a property with several
certificates is the normal case. Before this, every one of them was
unreachable by address: the collision rule saw two rows and refused, and no
amount of correct address text could get past it.

Deliberately narrower than "same UPRN wins". All three must hold:

* **every** row carries a non-empty UPRN and they are all equal. UPRN is
optional upstream and often absent, so absence proves nothing and two
blanks are not agreement.
* the rows agree on canonical address text. A shared UPRN with *different*
addresses is contradictory upstream data, not one property, and picking
between them would be the precise failure this module exists to prevent.
* the dates order strictly. A tie has no "most recent", and resolving one
by row order is a defect already named in the module docstring.

Any of those failing returns None, and the caller refuses as before.
"""
uprns = {r.uprn for r in rows}
if len(uprns) != 1:
return None
only = next(iter(uprns))
if not only:
return None
if len({_canon(r.address) for r in rows}) != 1:
return None

dated = [(_registration_date(r), r) for r in rows]
if any(d is None for d, _ in dated):
return None
dated.sort(key=lambda pair: pair[0])
if dated[-1][0] == dated[-2][0]:
return None
return dated[-1][1]


@dataclass(frozen=True)
class SelectionResult:
"""The selected row and the identity evidence that selected it.
Expand All @@ -97,10 +166,17 @@ class SelectionResult:
retained so callers that record a score keep a stable field, not because
there is a spectrum. ``method`` distinguishes literal equality from
designator-canonicalized equality so a consumer can treat them differently.

``uprn_latest_certificate`` is still identity, hence still 100: the property
is pinned by an agreed UPRN *and* agreed address text. What it additionally
discloses is that the property had more than one certificate and the newest
was taken -- a choice among one property's own history, never among
properties.
"""

row: EPCSearchRow
method: str # "uprn" | "exact_address" | "address_designator_normalized"
# | "uprn_latest_certificate"
confidence: int # always 100


Expand All @@ -126,8 +202,13 @@ def select_candidate(
if len(hits) == 1:
return SelectionResult(hits[0], "uprn", 100)
if len(hits) > 1:
latest = _one_property_latest(hits)
if latest is not None:
return SelectionResult(latest, "uprn_latest_certificate", 100)
raise EPCAmbiguousMatchError(
f"{len(hits)} certificates share UPRN {uprn}; cannot select one", hits)
f"{len(hits)} certificates share UPRN {uprn} but disagree on "
f"address or registration date, so they cannot be shown to be "
f"one property's certificate history; cannot select one", hits)
# A supplied UPRN that matches nothing is evidence of a MISS, not an
# invitation to fall back to weaker address text.
raise EPCAmbiguousMatchError(
Expand Down Expand Up @@ -179,6 +260,12 @@ def select_candidate(
return SelectionResult(row, method, 100)

if len(canon) > 1:
# Same property, certified more than once: identity is proven by UPRN,
# so this is a choice among one property's certificates, not among
# properties. The current certificate is the newest one.
latest = _one_property_latest(canon)
if latest is not None:
return SelectionResult(latest, "uprn_latest_certificate", 100)
if all(_norm(r.address) == target for r in canon):
raise EPCAmbiguousMatchError(
f"{len(canon)} certificates share the address text {address!r}; "
Expand Down
186 changes: 186 additions & 0 deletions tests/test_epc_uprn_recertification.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
"""One property certified twice is not two properties.

Reproduced live against the real EPC API on 2026-09-04:

EPCClient().search_by_postcode('NG11 9HD', address='27 Havenwood Rise')
-> EPCAmbiguousMatchError: 2 certificates share the address text
'27 Havenwood Rise'; cannot select one

Both candidates carry UPRN 100031555077. It is one house, certified twice, whose
address text differs only by a comma. Properties are re-certified on sale and on
let, so this is the normal case rather than an edge one, and every affected
property was unreachable by address — including through `enrich_comps_with_epc`,
where it silently left comps un-enriched.

The module this fixes has been repaired four times, each round finding a new way
for partial evidence to look sufficient. So the rule here is deliberately
narrower than "same UPRN wins":

same UPRN AND same canonical address AND a strict latest date

A shared UPRN with *different* address text is contradictory upstream data, not
one property, and still refuses. Equal dates still refuse, because "most recent"
does not exist. A missing date still refuses. Every existing guard therefore
passes unmodified — which is the check that this fix is not a fifth gap.
"""

from __future__ import annotations

import pytest

from property_core.epc.errors import EPCAmbiguousMatchError
from property_core.epc.selection import select_candidate
from property_core.epc.source_models import EPCSearchRow

UPRN = "100031555077"


def _row(cert: str, address: str, *, uprn: str | None = None,
registered: str | None = "2023-01-01") -> EPCSearchRow:
return EPCSearchRow.from_source({
"certificateNumber": cert, "addressLine1": address, "addressLine2": None,
"uprn": uprn, "postcode": "NG11 9HD", "currentEnergyEfficiencyBand": "D",
"registrationDate": registered, "schemaType": "RdSAP-Schema-20.0.0",
})


# --- the reproduced defect, both entry points -------------------------------


def test_the_live_repro_selects_the_most_recent_certificate():
"""The exact shape observed at NG11 9HD, comma and all."""
older = _row("6234-2126-73", "27 Havenwood Rise", uprn=UPRN, registered="2011-06-02")
newer = _row("8694-7123-28", "27, Havenwood Rise", uprn=UPRN, registered="2021-09-14")

result = select_candidate([older, newer], address="27 Havenwood Rise")

assert result.row.certificate_number == "8694-7123-28"
assert result.method == "uprn_latest_certificate"


def test_the_caller_supplied_uprn_path_is_fixed_too():
"""`select_candidate(uprn=...)` refused this identically before.

A fix touching only the address path would leave `epc_lookup(uprn=...)`
broken for exactly the same properties.
"""
older = _row("A", "27 Havenwood Rise", uprn=UPRN, registered="2011-06-02")
newer = _row("B", "27 Havenwood Rise", uprn=UPRN, registered="2021-09-14")

assert select_candidate([older, newer], uprn=UPRN).row.certificate_number == "B"


def test_order_of_the_candidate_rows_does_not_decide():
"""Upstream row order resolving a tie is a defect this module already names."""
older = _row("A", "27 Havenwood Rise", uprn=UPRN, registered="2011-06-02")
newer = _row("B", "27 Havenwood Rise", uprn=UPRN, registered="2021-09-14")

for rows in ([older, newer], [newer, older]):
assert select_candidate(rows, address="27 Havenwood Rise").row.certificate_number == "B"


def test_three_certificates_for_one_property_select_the_newest():
rows = [
_row("A", "27 Havenwood Rise", uprn=UPRN, registered="2011-06-02"),
_row("C", "27, Havenwood Rise", uprn=UPRN, registered="2024-02-29"),
_row("B", "27 Havenwood Rise", uprn=UPRN, registered="2021-09-14"),
]
assert select_candidate(rows, address="27 Havenwood Rise").row.certificate_number == "C"


def test_the_designator_canonicalized_collision_is_also_resolved():
"""`Flat 2` / `Apartment 2` collapse to one address; same UPRN makes it one flat."""
rows = [
_row("A", "Flat 2, 24 Alexandra Road", uprn=UPRN, registered="2012-01-01"),
_row("B", "Apartment 2, 24 Alexandra Road", uprn=UPRN, registered="2022-01-01"),
]
assert select_candidate(rows, address="Flat 2, 24 Alexandra Road").row.certificate_number == "B"


# --- every reason to keep refusing ------------------------------------------


def test_a_shared_uprn_with_different_addresses_still_refuses():
"""Contradictory upstream data, not one property.

Selecting here would be the exact failure this module exists to prevent:
attaching another property's certificate, and with it a wrong floor area and
every price-per-sqft derived from it.
"""
rows = [
_row("A", "12 Elm Road", uprn=UPRN, registered="2011-01-01"),
_row("B", "14 Elm Road", uprn=UPRN, registered="2022-01-01"),
]
with pytest.raises(EPCAmbiguousMatchError):
select_candidate(rows, uprn=UPRN)


def test_equal_registration_dates_still_refuse():
"""There is no "most recent" between two certificates lodged the same day."""
rows = [
_row("A", "27 Havenwood Rise", uprn=UPRN, registered="2021-09-14"),
_row("B", "27 Havenwood Rise", uprn=UPRN, registered="2021-09-14"),
]
with pytest.raises(EPCAmbiguousMatchError):
select_candidate(rows, address="27 Havenwood Rise")


@pytest.mark.parametrize("bad", [None, "", "not-a-date", "2021-13-01", "14/09/2021"])
def test_an_unusable_date_on_any_candidate_still_refuses(bad):
"""No ordering exists, so there is nothing to be latest."""
rows = [
_row("A", "27 Havenwood Rise", uprn=UPRN, registered="2011-06-02"),
_row("B", "27 Havenwood Rise", uprn=UPRN, registered=bad),
]
with pytest.raises(EPCAmbiguousMatchError):
select_candidate(rows, address="27 Havenwood Rise")


def test_a_missing_uprn_on_any_candidate_still_refuses():
"""UPRN is optional upstream and often absent; absence proves nothing."""
rows = [
_row("A", "27 Havenwood Rise", uprn=UPRN, registered="2011-06-02"),
_row("B", "27 Havenwood Rise", uprn=None, registered="2021-09-14"),
]
with pytest.raises(EPCAmbiguousMatchError):
select_candidate(rows, address="27 Havenwood Rise")


def test_two_blank_uprns_do_not_count_as_agreement():
rows = [
_row("A", "27 Havenwood Rise", uprn=None, registered="2011-06-02"),
_row("B", "27 Havenwood Rise", uprn=None, registered="2021-09-14"),
]
with pytest.raises(EPCAmbiguousMatchError):
select_candidate(rows, address="27 Havenwood Rise")


def test_different_uprns_on_a_colliding_address_still_refuse():
"""Two genuinely different properties whose address text happens to collide."""
rows = [
_row("A", "27 Havenwood Rise", uprn="100000000001", registered="2011-06-02"),
_row("B", "27 Havenwood Rise", uprn="100000000002", registered="2021-09-14"),
]
with pytest.raises(EPCAmbiguousMatchError):
select_candidate(rows, address="27 Havenwood Rise")


# --- the single-candidate paths are untouched -------------------------------


def test_a_single_exact_match_is_still_exact_address():
row = _row("A", "27 Havenwood Rise", uprn=UPRN)
result = select_candidate([row], address="27 Havenwood Rise")
assert result.method == "exact_address"
assert result.confidence == 100


def test_a_single_uprn_hit_is_still_uprn():
row = _row("A", "27 Havenwood Rise", uprn=UPRN)
assert select_candidate([row], uprn=UPRN).method == "uprn"


def test_a_uprn_that_matches_nothing_still_does_not_fall_back():
row = _row("A", "27 Havenwood Rise", uprn=UPRN)
with pytest.raises(EPCAmbiguousMatchError, match="refusing to fall back"):
select_candidate([row], uprn="999999999999")
Loading