diff --git a/harness/console.py b/harness/console.py index 2f33a3f70..a87197b38 100644 --- a/harness/console.py +++ b/harness/console.py @@ -29,6 +29,10 @@ from domain.modelling.recommendation import Recommendation from domain.modelling.scenario import Scenario from domain.modelling.solar_potential import SolarPotential from domain.property.property import Property, PropertyIdentity +from domain.property_baseline.performance import Performance +from domain.property_baseline.property_baseline_performance import ( + PropertyBaselinePerformance, +) from domain.property_baseline.rebaseliner import StubRebaseliner from domain.sap10_calculator.calculator import Sap10Calculator from harness.plan_table import format_plan_table @@ -49,6 +53,7 @@ from repositories.product.product_repository import ProductRepository from tests.orchestration.fakes import ( FakeEpcRepo, FakePlanRepository, + FakePropertyBaselineRepo, FakePropertyRepo, FakeScenarioRepository, FakeSolarRepo, @@ -230,7 +235,26 @@ def run_modelling( ) }, ) + # The Baseline stage this harness has no database for. Modelling asserts its + # re-score agrees with the persisted Effective Performance (ADR-0066), so + # establish it here the way `PropertyBaselineOrchestrator` would — from the + # calculator on this same EPC. Drift is structurally impossible in this lane + # (one process, one code version, one picture), so the assertion is a no-op; + # seeding it keeps the check honest instead of reporting "no baseline" once + # per Property across a whole re-model batch. ~1% of a run_modelling call. + baseline_repo = FakePropertyBaselineRepo() + baseline_repo.save( + PropertyBaselinePerformance( + lodged=None, + effective=Performance.from_sap_result(Sap10Calculator().calculate(epc)), + rebaseline_reason="physical_state_changed", + space_heating_kwh=0.0, + water_heating_kwh=0.0, + ), + _PROPERTY_ID, + ) unit = FakeUnitOfWork( + property_baseline=baseline_repo, property=property_repo, solar=FakeSolarRepo( by_uprn={_UPRN: solar_insights} diff --git a/orchestration/modelling_orchestrator.py b/orchestration/modelling_orchestrator.py index fdadd594f..d4248f3f1 100644 --- a/orchestration/modelling_orchestrator.py +++ b/orchestration/modelling_orchestrator.py @@ -123,6 +123,11 @@ class ModellingOrchestrator: # Resolve Fuel Rates once and reuse the BillDerivation across the batch, # so every baseline/post bill is priced at the same snapshot (ADR-0014). bill_derivation = BillDerivation(self._fuel_rates.get_current()) + # Properties the Baseline stage never established. Collected and reported + # once at the end of the batch — a whole batch can legitimately have none + # (the database-free harness), and one line each would bury the real + # divergence errors under a duplicate per Property. + without_baseline: list[int] = [] with self._unit_of_work() as uow: properties = uow.property.get_many(property_ids) scenarios: list[Scenario] = uow.scenario.get_many(scenario_ids) @@ -154,11 +159,12 @@ class ModellingOrchestrator: # baseline Score, which is already computed, rather than # paying for another whole-dwelling re-score per Property. if not checked_baseline: - _check_baseline_coherence( + if _check_baseline_coherence( property_id, plan.baseline.sap_continuous, uow.property_baseline.get_for_property(property_id), - ) + ): + without_baseline.append(property_id) checked_baseline = True uow.plan.save( plan, @@ -174,6 +180,14 @@ class ModellingOrchestrator: uow.property.mark_modelled( property_id, has_recommendations=has_recommendations ) + if without_baseline: + logger.warning( + "%s of %s properties had no persisted Baseline Performance " + "and were modelled against their own re-score; first ids: %s", + len(without_baseline), + len(property_ids), + without_baseline[:10], + ) uow.commit() def _plan_for( @@ -517,7 +531,7 @@ def _check_baseline_coherence( property_id: int, modelled_baseline_sap: float, persisted: Optional[PropertyBaselinePerformance], -) -> None: +) -> bool: """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). @@ -535,20 +549,21 @@ def _check_baseline_coherence( 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).""" + A missing row is a data gap, not a modelling fault, so it is **returned** + (``True``) for the caller to aggregate rather than logged here: it degrades + one Property instead of aborting the batch (ADR-0012 reserves the abort for a + load-bearing calculator raise), and an entire batch can legitimately have no + Baseline stage at all — the database-free harness the ``modelling_e2e`` + handler runs on has none, so a per-Property line would bury the real errors + under one duplicate per Property. + + A divergence stays per-Property: it is rare, and the Property is the thing + you need to go and look at.""" 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 + return True effective_sap: int = persisted.effective.sap_score if abs(modelled_baseline_sap - effective_sap) <= _MAX_BASELINE_DIVERGENCE: - return + return False logger.error( "modelling baseline diverges from persisted Effective Performance for " "property_id=%s: modelled=%.2f effective=%s (%s). Plan post_* figures " @@ -559,6 +574,7 @@ def _check_baseline_coherence( effective_sap, persisted.rebaseline_reason, ) + return False def _target_sap(scenario: Scenario) -> Optional[float]: diff --git a/tests/orchestration/test_modelling_baseline_coherence.py b/tests/orchestration/test_modelling_baseline_coherence.py index c12b4d6a9..752b1c14f 100644 --- a/tests/orchestration/test_modelling_baseline_coherence.py +++ b/tests/orchestration/test_modelling_baseline_coherence.py @@ -81,18 +81,31 @@ def test_a_baseline_agreeing_within_rounding_is_silent( assert caplog.records == [] -def test_a_property_with_no_persisted_baseline_warns_rather_than_failing( +def test_a_property_with_no_persisted_baseline_is_reported_not_logged( caplog: pytest.LogCaptureFixture, ) -> None: # Arrange — the Property never went through the Baseline stage. A data gap # should degrade one Property, not abort the batch (ADR-0012 reserves the - # abort for a load-bearing calculator raise). + # abort for a load-bearing calculator raise) — but it must not log per + # Property either: whole batches legitimately have no Baseline stage (the + # database-free harness the modelling_e2e handler runs on), and one line per + # Property buries the real errors in tens of thousands of duplicates. # Act with caplog.at_level(logging.WARNING): - _check_baseline_coherence(749719, 63.22, None) + missing = _check_baseline_coherence(749719, 63.22, None) - # Assert — visible, but not an error and not a raise. - assert "749719" in caplog.text - assert any(r.levelno == logging.WARNING for r in caplog.records) - assert not any(r.levelno >= logging.ERROR for r in caplog.records) + # Assert — reported back to the caller to aggregate, silent in itself. + assert missing is True + assert caplog.records == [] + + +def test_a_property_with_a_baseline_is_not_reported_as_missing() -> None: + # Arrange + persisted: Optional[PropertyBaselinePerformance] = _persisted(63) + + # Act + missing = _check_baseline_coherence(749719, 63.22, persisted) + + # Assert + assert missing is False