From c9e0180c12e7682f004a202050d3a609b48f40d6 Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Wed, 8 Jul 2026 14:25:34 +0000 Subject: [PATCH] Widen EPC-prediction cohort search one step when the normal radius is too sparse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0034's nearby-postcode broadening degrades to a genuine NoSameTypeComparablesError whenever the default 1000m/30-postcode reach has no same-type comparable nearby (e.g. property_id=752685, portfolio 824 — the only Maisonette within reach). Adds one configurable extra widening step (EpcComparablePropertiesRepository.candidates_near now accepts widen_nearby_postcodes), tried only when the normal-radius walk falls short of `minimum` matches. modelling_e2e's handler wires this to a 3000m/60-postcode PostcodesIoClient as the single wider step. Co-Authored-By: Claude Sonnet 5 --- applications/modelling_e2e/handler.py | 8 +- .../epc_comparable_properties_repository.py | 50 ++++++++++-- ...st_epc_comparable_properties_repository.py | 77 +++++++++++++++++++ 3 files changed, 127 insertions(+), 8 deletions(-) diff --git a/applications/modelling_e2e/handler.py b/applications/modelling_e2e/handler.py index 51c3e8149..269411b63 100644 --- a/applications/modelling_e2e/handler.py +++ b/applications/modelling_e2e/handler.py @@ -471,7 +471,13 @@ def handler( overrides_reader = InMemoryPropertyOverridesReader(overrides_by_pid) prediction_attrs_reader = OverrideBackedPredictionAttributesReader(overrides_reader) comparables_repo = EpcComparablePropertiesRepository( - epc_client, geospatial, nearby_postcodes=PostcodesIoClient() + epc_client, + geospatial, + nearby_postcodes=PostcodesIoClient(), + # One step wider than the default 1000m/30 reach, tried only when that + # normal radius doesn't surface enough same-type comparables (ADR-0034 + # amendment) — a single extra step outward, not an unbounded expansion. + widen_nearby_postcodes=PostcodesIoClient(radius_m=3000, limit=60), ) predictor = EpcPrediction() diff --git a/repositories/comparable_properties/epc_comparable_properties_repository.py b/repositories/comparable_properties/epc_comparable_properties_repository.py index 20c51dfda..68328f395 100644 --- a/repositories/comparable_properties/epc_comparable_properties_repository.py +++ b/repositories/comparable_properties/epc_comparable_properties_repository.py @@ -75,10 +75,17 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository): epc_client: CohortEpcClient, geospatial: CohortGeospatial, nearby_postcodes: Optional[NearbyPostcodes] = None, + widen_nearby_postcodes: Optional[NearbyPostcodes] = None, ) -> None: self._epc_client = epc_client self._geospatial = geospatial self._nearby_postcodes = nearby_postcodes + # A second, wider-reaching `NearbyPostcodes` source, consulted only when + # the normal-radius walk fails to reach `minimum` same-type matches — one + # extra step outward before genuinely giving up (ADR-0034 amendment). A + # single configured step today; the same pattern chains to further steps + # later without changing this class's contract. + self._widen_nearby_postcodes = widen_nearby_postcodes # Cohort certs skipped because they are not yet mappable. Accumulates # across every postcode the instance serves; the caller reads it after # the run to report the mapper gaps (see modelling_e2e handler). @@ -117,12 +124,41 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository): remaining, further-away postcodes are not fetched, so a dense area resolves in one or two searches instead of the whole radius. Without a configured ``NearbyPostcodes`` source this degrades to the seed postcode - alone.""" - postcodes = ( - self._nearby_postcodes.nearby(postcode, coordinates) - if self._nearby_postcodes is not None - else [postcode] - ) + alone. + + If the normal-radius walk still falls short of ``minimum`` matches and a + ``widen_nearby_postcodes`` source is configured, the walk is retried + once against that wider source before giving up — a single extra step + outward, not an unbounded expansion.""" + postcodes = self._nearby(self._nearby_postcodes, postcode, coordinates) + candidates, matches = self._walk(postcodes, enough, minimum) + + if ( + enough is not None + and matches < minimum + and self._widen_nearby_postcodes is not None + ): + wider_postcodes = self._nearby( + self._widen_nearby_postcodes, postcode, coordinates + ) + candidates, _ = self._walk(wider_postcodes, enough, minimum) + + return candidates + + def _nearby( + self, + source: Optional[NearbyPostcodes], + postcode: str, + coordinates: Optional[Coordinates], + ) -> list[str]: + return source.nearby(postcode, coordinates) if source is not None else [postcode] + + def _walk( + self, + postcodes: list[str], + enough: Optional[Callable[[ComparableProperty], bool]], + minimum: int, + ) -> tuple[list[ComparableProperty], int]: candidates: list[ComparableProperty] = [] seen_certs: set[str] = set() matches = 0 @@ -136,7 +172,7 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository): matches += 1 if enough is not None and matches >= minimum: break - return candidates + return candidates, matches # Mapper-shape errors: the cert's lodged data does not fit the schema/mapper. # `ValueError` — missing required field / unmapped code (`UnmappedApiCode`); diff --git a/tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py b/tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py index 1a0b918ea..5b99bbd41 100644 --- a/tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py +++ b/tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py @@ -245,6 +245,83 @@ def test_candidates_near_without_a_source_uses_only_the_seed() -> None: assert [c.certificate_number for c in candidates] == ["CERT-1"] +def test_candidates_near_widens_once_when_the_normal_reach_is_insufficient() -> None: + # Arrange — the normal-radius nearby set (P0, P1) has no MATCH cert; the + # wider-radius set (P0, P1, P2, P3) reaches P3, which does. + client = _MultiPostcodeEpcClient( + { + "P0": [_result("OTHER-1", uprn=1)], + "P1": [_result("OTHER-2", uprn=2)], + "P2": [_result("OTHER-3", uprn=3)], + "P3": [_result("MATCH-1", uprn=4)], + } + ) + normal = _FakeNearbyPostcodes(["P0", "P1"]) + wider = _FakeNearbyPostcodes(["P0", "P1", "P2", "P3"]) + repo = EpcComparablePropertiesRepository( + client, + _FakeGeospatial({}), + nearby_postcodes=normal, + widen_nearby_postcodes=wider, + ) + + # Act + candidates = repo.candidates_near( + "P0", None, enough=lambda c: c.certificate_number.startswith("MATCH"), minimum=1 + ) + + # Assert — both the normal and the widened source were consulted (in that + # order), and the wider walk's matches are returned. + assert normal.calls == [("P0", None)] + assert wider.calls == [("P0", None)] + certs = {c.certificate_number for c in candidates} + assert "MATCH-1" in certs + + +def test_candidates_near_does_not_widen_when_the_normal_reach_is_enough() -> None: + # Arrange — the normal radius already yields the minimum match, so the + # wider (more expensive) source must never be consulted. + client = _MultiPostcodeEpcClient({"P0": [_result("MATCH-1", uprn=1)]}) + normal = _FakeNearbyPostcodes(["P0"]) + wider = _FakeNearbyPostcodes(["P0", "P1", "P2"]) + repo = EpcComparablePropertiesRepository( + client, + _FakeGeospatial({}), + nearby_postcodes=normal, + widen_nearby_postcodes=wider, + ) + + # Act + candidates = repo.candidates_near( + "P0", None, enough=lambda c: c.certificate_number.startswith("MATCH"), minimum=1 + ) + + # Assert + assert wider.calls == [] + assert [c.certificate_number for c in candidates] == ["MATCH-1"] + + +def test_candidates_near_without_enough_predicate_never_widens() -> None: + # Arrange — no `enough` predicate means the caller isn't gating on a match + # count (e.g. the unconditional aggregate path), so widening is meaningless + # and must not fire even with a configured wider source. + client = _MultiPostcodeEpcClient({"P0": [_result("CERT-1", uprn=1)]}) + normal = _FakeNearbyPostcodes(["P0"]) + wider = _FakeNearbyPostcodes(["P0", "P1"]) + repo = EpcComparablePropertiesRepository( + client, + _FakeGeospatial({}), + nearby_postcodes=normal, + widen_nearby_postcodes=wider, + ) + + # Act + repo.candidates_near("P0", None) + + # Assert + assert wider.calls == [] + + # --------------------------------------------------------------------------- # Unmappable cohort certs are skipped + recorded, not allowed to sink the cohort # ---------------------------------------------------------------------------