mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-27 23:35:01 +00:00
Merge pull request #1595 from Hestia-Homes/fix/1589-refetch-epc-is-ignored
Apply the ADR-0001 Recency Tie-Break when refetch_epc=True (#1589)
This commit is contained in:
commit
217dcecfa9
5 changed files with 286 additions and 19 deletions
|
|
@ -387,6 +387,21 @@ def _dedupe_skipped(
|
|||
return unique
|
||||
|
||||
|
||||
def _newer_lodged(
|
||||
stored: Optional[EpcPropertyData], fetched: Optional[EpcPropertyData]
|
||||
) -> tuple[Optional[EpcPropertyData], bool]:
|
||||
"""ADR-0001 Recency Tie-Break over the two lodged sources. Returns
|
||||
(chosen, chosen_is_fetched) — the bool gates the EPC save, so a stored EPC
|
||||
that wins is never re-persisted."""
|
||||
if fetched is None:
|
||||
return stored, False
|
||||
if stored is None:
|
||||
return fetched, True
|
||||
if fetched.inspection_date > stored.inspection_date:
|
||||
return fetched, True
|
||||
return stored, False
|
||||
|
||||
|
||||
def _predict_epc(
|
||||
*,
|
||||
property_id: int,
|
||||
|
|
@ -592,11 +607,11 @@ def handler(
|
|||
)
|
||||
epc_repo = EpcPostgresRepository(read_session)
|
||||
# Lodged EPCs are read by UPRN (keyed by UPRN below); predicted EPCs stay
|
||||
# keyed on property_id.
|
||||
stored_lodged_epcs: dict[int, EpcPropertyData] = (
|
||||
epc_repo.get_for_properties(list(set(uprns.values())))
|
||||
if not refetch_epc
|
||||
else {}
|
||||
# keyed on property_id. Read whatever refetch_epc says — the flag decides
|
||||
# whether the gov register is consulted for a *candidate*, never whether a
|
||||
# stored assessment can win the ADR-0001 Recency Tie-Break (#1589).
|
||||
stored_lodged_epcs: dict[int, EpcPropertyData] = epc_repo.get_for_properties(
|
||||
list(set(uprns.values()))
|
||||
)
|
||||
stored_predicted_epcs: dict[int, EpcPropertyData] = (
|
||||
epc_repo.get_predicted_for_properties(property_ids)
|
||||
|
|
@ -628,19 +643,24 @@ def handler(
|
|||
)
|
||||
|
||||
stored_lodged = stored_lodged_epcs.get(uprn)
|
||||
# Stays None when neither source has an assessment; the prediction
|
||||
# path handles this property.
|
||||
epc: Optional[EpcPropertyData] = None
|
||||
lodged_epc_is_new = False
|
||||
if refetch_epc:
|
||||
epc: Optional[EpcPropertyData] = epc_client.get_by_uprn(uprn)
|
||||
lodged_epc_is_new = epc is not None
|
||||
epc, lodged_epc_is_new = _newer_lodged(
|
||||
stored_lodged, epc_client.get_by_uprn(uprn)
|
||||
)
|
||||
if epc is not None and not lodged_epc_is_new:
|
||||
logger.info(
|
||||
f"property={pid} stored lodged EPC is newer than the "
|
||||
f"fetched cert — keeping the stored assessment"
|
||||
)
|
||||
elif stored_lodged is not None:
|
||||
logger.info(
|
||||
f"property={pid} using stored lodged EPC (refetch_epc=False)"
|
||||
)
|
||||
epc = stored_lodged
|
||||
else:
|
||||
epc = (
|
||||
None # no stored lodged EPC; prediction path handles this property
|
||||
)
|
||||
resolved_overrides = overrides_reader.overrides_for(pid)
|
||||
flag_fuel_mismatch(resolved_overrides)
|
||||
overrides = overlays_from(resolved_overrides)
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ from __future__ import annotations
|
|||
# ---------------------------------------------------------------------------
|
||||
# CONFIG
|
||||
# ---------------------------------------------------------------------------
|
||||
PORTFOLIO_ID: int = 796
|
||||
PORTFOLIO_ID: int = 838
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
import sys
|
||||
|
|
@ -27,7 +27,9 @@ engine = build_engine()
|
|||
|
||||
with engine.connect() as conn:
|
||||
rows = conn.execute(
|
||||
text("SELECT id, postcode FROM property WHERE portfolio_id = :pid ORDER BY postcode, id"),
|
||||
text(
|
||||
"SELECT id, postcode FROM property WHERE portfolio_id = :pid ORDER BY postcode, id"
|
||||
),
|
||||
{"pid": PORTFOLIO_ID},
|
||||
).fetchall()
|
||||
|
||||
|
|
|
|||
68
scripts/properties_by_postcode_838.txt
Normal file
68
scripts/properties_by_postcode_838.txt
Normal file
|
|
@ -0,0 +1,68 @@
|
|||
'M11 1WN': [754931]
|
||||
'M11 2LF': [754927]
|
||||
'M11 3LS': [754878]
|
||||
'M11 3NP': [754935]
|
||||
'M11 3NT': [754839]
|
||||
'M11 3QP': [754934]
|
||||
'M11 3RQ': [754926]
|
||||
'M11 3SU': [754860]
|
||||
'M11 4BU': [754750]
|
||||
'M11 4DF': [754912]
|
||||
'M11 4HH': [754831]
|
||||
'M11 4NF': [754915]
|
||||
'M11 4PD': [754874]
|
||||
'M11 4PP': [754824]
|
||||
'M11 4PT': [754830]
|
||||
'M11 4PW': [754916]
|
||||
'M11 4PX': [754816]
|
||||
'M11 4QS': [754922]
|
||||
'M11 4RS': [754904]
|
||||
'M11 4SD': [754930]
|
||||
'M11 4WE': [754917]
|
||||
'M11 4WF': [754921]
|
||||
'M11 4WP': [754759]
|
||||
'M11 4WT': [754925]
|
||||
'M11 1WH': [754897, 754898]
|
||||
'M11 3BN': [754858, 754859]
|
||||
'M11 3LD': [754932, 754933]
|
||||
'M11 3LN': [754928, 754929]
|
||||
'M11 3RH': [754832, 754833]
|
||||
'M11 4BT': [754913, 754914]
|
||||
'M11 4DN': [754908, 754909]
|
||||
'M11 4GA': [754804, 754805]
|
||||
'M11 4NG': [754757, 754758]
|
||||
'M11 4PH': [754828, 754829]
|
||||
'M11 4PZ': [754910, 754911]
|
||||
'M11 4RJ': [754923, 754924]
|
||||
'M11 4RT': [754766, 754767]
|
||||
'M11 3NJ': [754875, 754876, 754877]
|
||||
'M11 4FB': [754905, 754906, 754907]
|
||||
'M11 4NH': [754785, 754786, 754787]
|
||||
'M11 4NZ': [754731, 754732, 754733]
|
||||
'M11 4PL': [754817, 754818, 754819]
|
||||
'M11 4QD': [754825, 754826, 754827]
|
||||
'M11 4WL': [754918, 754919, 754920]
|
||||
'M11 1NG': [754879, 754880, 754881, 754882]
|
||||
'M11 3RF': [754844, 754845, 754846, 754847]
|
||||
'M11 3RN': [754840, 754841, 754842, 754843]
|
||||
'M11 4NJ': [754888, 754889, 754890, 754891]
|
||||
'M11 4NQ': [754788, 754789, 754790, 754791]
|
||||
'M11 4PQ': [754820, 754821, 754822, 754823]
|
||||
'M11 4SA': [754776, 754777, 754778, 754779]
|
||||
'M11 4WJ': [754792, 754793, 754794, 754795]
|
||||
'M11 1WL': [754899, 754900, 754901, 754902, 754903]
|
||||
'M11 3AG': [754834, 754835, 754836, 754837, 754838]
|
||||
'M11 3LH': [754869, 754870, 754871, 754872, 754873]
|
||||
'M11 4NL': [754892, 754893, 754894, 754895, 754896]
|
||||
'M11 4PG': [754780, 754781, 754782, 754783, 754784]
|
||||
'M11 4QB': [754883, 754884, 754885, 754886, 754887]
|
||||
'M11 4PJ': [754760, 754761, 754762, 754763, 754764, 754765]
|
||||
'M11 4PN': [754751, 754752, 754753, 754754, 754755, 754756]
|
||||
'M11 3HL': [754861, 754862, 754863, 754864, 754865, 754866, 754867, 754868]
|
||||
'M11 4DH': [754768, 754769, 754770, 754771, 754772, 754773, 754774, 754775]
|
||||
'M11 4TL': [754796, 754797, 754798, 754799, 754800, 754801, 754802, 754803]
|
||||
'M11 1NW': [754848, 754849, 754850, 754851, 754852, 754853, 754854, 754855, 754856, 754857]
|
||||
'M11 4BZ': [754806, 754807, 754808, 754809, 754810, 754811, 754812, 754813, 754814, 754815]
|
||||
'M11 4WU': [754734, 754735, 754736, 754737, 754738, 754739, 754740, 754741, 754742, 754743, 754744, 754745, 754746, 754747, 754748, 754749]
|
||||
|
||||
Total postcodes: 66, total properties: 205
|
||||
|
|
@ -23,8 +23,8 @@ from utilities.logger import setup_logger
|
|||
# ---------------------------------------------------------------------------
|
||||
# CONFIG — edit these before running
|
||||
# ---------------------------------------------------------------------------
|
||||
PORTFOLIO_ID: int = 796
|
||||
SCENARIO_ID: int = 1268
|
||||
PORTFOLIO_ID: int = 838
|
||||
SCENARIO_ID: int = 1296
|
||||
SQS_QUEUE_NAME: str = "modelling_e2e-queue-dev"
|
||||
|
||||
# Max number of properties to process this run (cost cap).
|
||||
|
|
|
|||
|
|
@ -7,7 +7,11 @@ is needed. One test per distinct behaviour path.
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import dataclasses
|
||||
import json
|
||||
from contextlib import ExitStack
|
||||
from datetime import date
|
||||
from pathlib import Path
|
||||
from typing import Any, Iterator, Optional
|
||||
from unittest.mock import MagicMock, call, patch
|
||||
from uuid import UUID, uuid4
|
||||
|
|
@ -18,6 +22,8 @@ from pydantic import ValidationError
|
|||
from applications.modelling_e2e.modelling_e2e_trigger_body import (
|
||||
ModellingE2ETriggerBody,
|
||||
)
|
||||
from datatypes.epc.domain.epc_property_data import EpcPropertyData
|
||||
from datatypes.epc.domain.mapper import EpcPropertyDataMapper
|
||||
from domain.tasks.subtasks import SubTask
|
||||
from repositories.epc.epc_postgres_repository import EpcSaveRequest
|
||||
|
||||
|
|
@ -1403,9 +1409,177 @@ def test_cohort_cache_prevents_duplicate_candidates_for_calls() -> None:
|
|||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# refetch_epc flag
|
||||
# Recency Tie-Break between the two lodged sources (ADR-0001)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_JSON_SAMPLES = Path(__file__).resolve().parents[3] / "backend/epc_api/json_samples"
|
||||
|
||||
|
||||
def _lodged_epc(inspection: str) -> EpcPropertyData:
|
||||
"""A real lodged EPC surveyed on `inspection` — the tie-break reads only
|
||||
inspection_date, but a real cert keeps the test honest about the type."""
|
||||
raw: dict[str, Any] = json.loads(
|
||||
(_JSON_SAMPLES / "RdSAP-Schema-21.0.0" / "epc.json").read_text()
|
||||
)
|
||||
epc = EpcPropertyDataMapper.from_api_response(raw)
|
||||
return dataclasses.replace(epc, inspection_date=date.fromisoformat(inspection))
|
||||
|
||||
|
||||
def test_stored_survey_newer_than_gov_cert_wins_the_tie_break() -> None:
|
||||
"""The bug in #1589: a newer stored survey (e.g. PasHub site notes) must beat
|
||||
an older gov-register cert, and must not be flagged for re-persisting."""
|
||||
# Arrange
|
||||
from applications.modelling_e2e.handler import _newer_lodged
|
||||
|
||||
stored = _lodged_epc("2026-05-01")
|
||||
fetched = _lodged_epc("2023-12-01")
|
||||
|
||||
# Act
|
||||
chosen, chosen_is_fetched = _newer_lodged(stored, fetched)
|
||||
|
||||
# Assert
|
||||
assert chosen is stored
|
||||
assert chosen_is_fetched is False
|
||||
|
||||
|
||||
def test_gov_cert_newer_than_stored_survey_wins_and_is_flagged_for_saving() -> None:
|
||||
"""A genuinely fresher gov cert still wins, and is flagged new so it persists."""
|
||||
# Arrange
|
||||
from applications.modelling_e2e.handler import _newer_lodged
|
||||
|
||||
stored = _lodged_epc("2023-12-01")
|
||||
fetched = _lodged_epc("2026-05-01")
|
||||
|
||||
# Act
|
||||
chosen, chosen_is_fetched = _newer_lodged(stored, fetched)
|
||||
|
||||
# Assert
|
||||
assert chosen is fetched
|
||||
assert chosen_is_fetched is True
|
||||
|
||||
|
||||
def test_gov_cert_is_used_when_nothing_is_stored() -> None:
|
||||
"""Only the gov cert exists: it is the assessment, and it must be persisted."""
|
||||
# Arrange
|
||||
from applications.modelling_e2e.handler import _newer_lodged
|
||||
|
||||
fetched = _lodged_epc("2023-12-01")
|
||||
|
||||
# Act
|
||||
chosen, chosen_is_fetched = _newer_lodged(None, fetched)
|
||||
|
||||
# Assert
|
||||
assert chosen is fetched
|
||||
assert chosen_is_fetched is True
|
||||
|
||||
|
||||
def test_stored_survey_survives_a_gov_register_with_no_cert() -> None:
|
||||
"""The second failure shape in #1589: recently-surveyed homes have no public
|
||||
cert, and a None from the register must not wipe out the stored survey."""
|
||||
# Arrange
|
||||
from applications.modelling_e2e.handler import _newer_lodged
|
||||
|
||||
stored = _lodged_epc("2026-05-01")
|
||||
|
||||
# Act
|
||||
chosen, chosen_is_fetched = _newer_lodged(stored, None)
|
||||
|
||||
# Assert
|
||||
assert chosen is stored
|
||||
assert chosen_is_fetched is False
|
||||
|
||||
|
||||
def test_no_lodged_epc_from_either_source_defers_to_prediction() -> None:
|
||||
"""Neither source has an assessment: the caller gets None and predicts."""
|
||||
# Arrange
|
||||
from applications.modelling_e2e.handler import _newer_lodged
|
||||
|
||||
# Act
|
||||
chosen, chosen_is_fetched = _newer_lodged(None, None)
|
||||
|
||||
# Assert
|
||||
assert chosen is None
|
||||
assert chosen_is_fetched is False
|
||||
|
||||
|
||||
def test_same_day_assessments_prefer_the_stored_survey() -> None:
|
||||
"""Exact inspection_date tie: ADR-0001's "if we surveyed it, trust the
|
||||
survey" breaks it for the stored side."""
|
||||
# Arrange
|
||||
from applications.modelling_e2e.handler import _newer_lodged
|
||||
|
||||
stored = _lodged_epc("2026-05-01")
|
||||
fetched = _lodged_epc("2026-05-01")
|
||||
|
||||
# Act
|
||||
chosen, chosen_is_fetched = _newer_lodged(stored, fetched)
|
||||
|
||||
# Assert
|
||||
assert chosen is stored
|
||||
assert chosen_is_fetched is False
|
||||
|
||||
|
||||
def test_refetch_epc_true_models_the_newer_stored_survey_without_resaving_it() -> None:
|
||||
"""#1589: refetch_epc=True must not let an older gov cert override a newer
|
||||
stored survey. The stored survey is modelled, and is not re-persisted."""
|
||||
# Arrange
|
||||
mock_engine = _engine_mock([PROPERTY_ID], [UPRN], [POSTCODE])
|
||||
stored_epc = _lodged_epc("2026-05-01") # PasHub site-notes survey
|
||||
gov_epc = _lodged_epc("2023-12-01") # older public cert
|
||||
mock_plan = _plan_mock()
|
||||
mock_uow = MagicMock()
|
||||
|
||||
with ExitStack() as stack:
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.os.environ", _ENV))
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._get_engine", return_value=mock_engine)
|
||||
)
|
||||
mock_epc_client = stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.EpcClientService")
|
||||
).return_value
|
||||
mock_epc_client.get_by_uprn.return_value = gov_epc
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.GeospatialS3Repository"))
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.GoogleSolarApiClient"))
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._spatial_for", return_value=None)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._solar_insights_for", return_value=None)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.overlays_from", return_value=[])
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.PropertyOverridesPostgresReader")
|
||||
).return_value.overrides_for_many.return_value = {}
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.ScenarioPostgresRepository")
|
||||
).return_value.get_many.return_value = [MagicMock()]
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.catalogue_snapshot_with_off_catalogue_overrides")
|
||||
)
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.Session"))
|
||||
mock_run_modelling = stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.run_modelling", return_value=mock_plan)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.EpcPostgresRepository")
|
||||
).return_value.get_for_properties.return_value = {UPRN: stored_epc}
|
||||
MockUoW = stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.PostgresUnitOfWork")
|
||||
)
|
||||
MockUoW.return_value.__enter__.return_value = mock_uow
|
||||
MockUoW.return_value.__exit__.return_value = False
|
||||
|
||||
# Act
|
||||
_call_handler({**_BODY, "refetch_epc": True})
|
||||
|
||||
# Assert — the survey was modelled, not the older gov cert...
|
||||
modelled_epc = mock_run_modelling.call_args.args[0]
|
||||
assert modelled_epc.inspection_date == date(2026, 5, 1)
|
||||
# ...and it came from the DB unchanged, so it is not re-saved
|
||||
mock_uow.epc.save_batch.assert_not_called()
|
||||
|
||||
|
||||
def test_refetch_epc_false_with_stored_epc_skips_api_call() -> None:
|
||||
"""refetch_epc=False + stored lodged EPC present: EpcClientService.get_by_uprn
|
||||
|
|
@ -1759,10 +1933,13 @@ def test_repredict_epc_false_without_stored_predicted_epc_falls_back_to_live_pre
|
|||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.run_modelling", return_value=mock_plan)
|
||||
)
|
||||
# No stored predicted EPC
|
||||
stack.enter_context(
|
||||
# Nothing stored: no predicted EPC, and no lodged EPC to win the
|
||||
# tie-break against the register's None
|
||||
mock_epc_repo = stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.EpcPostgresRepository")
|
||||
).return_value.get_predicted_for_properties.return_value = {}
|
||||
).return_value
|
||||
mock_epc_repo.get_predicted_for_properties.return_value = {}
|
||||
mock_epc_repo.get_for_properties.return_value = {}
|
||||
MockUoW = stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.PostgresUnitOfWork")
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue