From 8c2c19495c76d55ddc5639424a749f58d7276066 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 15:53:04 +0000 Subject: [PATCH] =?UTF-8?q?Flag=20a=20modelling=20baseline=20that=20disagr?= =?UTF-8?q?ees=20with=20the=20persisted=20effective=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- orchestration/modelling_orchestrator.py | 60 ++++++++++++++++++- tests/orchestration/fakes.py | 13 +++- .../test_modelling_baseline_coherence.py | 2 + 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/orchestration/modelling_orchestrator.py b/orchestration/modelling_orchestrator.py index 4c9447306..fdadd594f 100644 --- a/orchestration/modelling_orchestrator.py +++ b/orchestration/modelling_orchestrator.py @@ -57,6 +57,12 @@ from repositories.unit_of_work import UnitOfWork logger = logging.getLogger(__name__) +# How far Modelling's continuous re-score may sit from the persisted Effective +# SAP before it counts as a divergence. The persisted figure is the *rounded* +# integer, so up to half a point is rounding alone; beyond a full point the two +# stages are scoring different pictures (#1655). +_MAX_BASELINE_DIVERGENCE: Final[float] = 1.0 + # Best-practice install sequence for the role-3 attribution cascade (ADR-0016): # walls → roof → ventilation → floor, per the legacy `Recommendations` class. # Ventilation sits after the fabric that triggers it so its (negative) marginal @@ -130,6 +136,7 @@ class ModellingOrchestrator: uow.solar, prop.identity.uprn ) has_recommendations = False + checked_baseline = False for scenario in scenarios: plan = self._plan_for( scorer, @@ -142,6 +149,17 @@ class ModellingOrchestrator: solar_potential=solar_potential, considered_measures=considered_measures, ) + # The baseline is the same picture for every Scenario, so + # check it once per Property — and off the Plan's own + # baseline Score, which is already computed, rather than + # paying for another whole-dwelling re-score per Property. + if not checked_baseline: + _check_baseline_coherence( + property_id, + plan.baseline.sap_continuous, + uow.property_baseline.get_for_property(property_id), + ) + checked_baseline = True uow.plan.save( plan, property_id=property_id, @@ -500,7 +518,47 @@ def _check_baseline_coherence( modelled_baseline_sap: float, persisted: Optional[PropertyBaselinePerformance], ) -> None: - raise NotImplementedError + """Assert that Modelling and the Baseline stage agree on what the Property + scores now, and say so loudly when they do not (ADR-0002, #1655). + + Modelling re-scores the Effective EPC rather than reading the persisted + Effective Performance, so the two can drift apart. The app treats the + persisted figure as the Property's current state and reads a Plan's `post_*` + against it, so a drift is not cosmetic: it surfaces to the user as a gain + nobody modelled (property 749719 reported "+9.65 SAP" on a plan whose own + CO2, bill and consumption savings were all exactly 0.0). + + We flag rather than anchor deliberately. Overwriting the Plan's SAP with the + persisted integer would make it disagree with its own CO2 / primary energy / + bill, which all come off the same SapResult — trading one incoherence for + another, and hiding the drift instead of surfacing it. A divergence here is + a real defect upstream (see #1656) and should be investigated, not papered + over. + + A missing row is a data gap, not a modelling fault: warn and carry on, so it + degrades one Property rather than aborting the batch (ADR-0012 reserves the + abort for a load-bearing calculator raise).""" + if persisted is None: + logger.warning( + "no persisted Baseline Performance for property_id=%s; modelling " + "against its own re-score (modelled_baseline_sap=%.2f)", + property_id, + modelled_baseline_sap, + ) + return + effective_sap: int = persisted.effective.sap_score + if abs(modelled_baseline_sap - effective_sap) <= _MAX_BASELINE_DIVERGENCE: + return + logger.error( + "modelling baseline diverges from persisted Effective Performance for " + "property_id=%s: modelled=%.2f effective=%s (%s). Plan post_* figures " + "are read against the effective baseline, so this surfaces as an " + "unmodelled gain — see #1655/#1656.", + property_id, + modelled_baseline_sap, + effective_sap, + persisted.rebaseline_reason, + ) def _target_sap(scenario: Scenario) -> Optional[float]: diff --git a/tests/orchestration/fakes.py b/tests/orchestration/fakes.py index 9820e4ebf..0271ae2a9 100644 --- a/tests/orchestration/fakes.py +++ b/tests/orchestration/fakes.py @@ -178,17 +178,24 @@ class FakeSpatialRepo(SpatialRepository): class FakePropertyBaselineRepo(PropertyBaselineRepository): - def __init__(self) -> None: + def __init__( + self, by_property: Optional[dict[int, PropertyBaselinePerformance]] = None + ) -> None: self.saved: list[tuple[PropertyBaselinePerformance, int]] = [] + self._by_property = by_property or {} def save(self, baseline: PropertyBaselinePerformance, property_id: int) -> int: self.saved.append((baseline, property_id)) + self._by_property[property_id] = baseline return len(self.saved) def get_for_property( self, property_id: int - ) -> Optional[PropertyBaselinePerformance]: # pragma: no cover - raise NotImplementedError + ) -> Optional[PropertyBaselinePerformance]: + """None for a Property the Baseline stage never established — the real + repo's behaviour, and what the Modelling coherence check treats as a + data gap rather than a fault.""" + return self._by_property.get(property_id) class FakeScenarioRepository(ScenarioRepository): diff --git a/tests/orchestration/test_modelling_baseline_coherence.py b/tests/orchestration/test_modelling_baseline_coherence.py index 8be5d7575..c12b4d6a9 100644 --- a/tests/orchestration/test_modelling_baseline_coherence.py +++ b/tests/orchestration/test_modelling_baseline_coherence.py @@ -43,6 +43,8 @@ def _persisted(effective_sap: int) -> PropertyBaselinePerformance: lodged=None, effective=effective, rebaseline_reason="physical_state_changed", + space_heating_kwh=0.0, + water_heating_kwh=0.0, )