From 31edd01999b930530ad802c68205784ed8ec19ec Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 15:50:39 +0000 Subject: [PATCH] =?UTF-8?q?Flag=20a=20modelling=20baseline=20that=20disagr?= =?UTF-8?q?ees=20with=20the=20persisted=20effective=20=F0=9F=9F=A5?= 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 | 14 +++ .../test_modelling_baseline_coherence.py | 96 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 tests/orchestration/test_modelling_baseline_coherence.py diff --git a/orchestration/modelling_orchestrator.py b/orchestration/modelling_orchestrator.py index ed324e975..4c9447306 100644 --- a/orchestration/modelling_orchestrator.py +++ b/orchestration/modelling_orchestrator.py @@ -1,6 +1,7 @@ from __future__ import annotations from collections.abc import Callable +import logging from typing import Final, Optional from datatypes.epc.domain.epc import Epc @@ -27,6 +28,9 @@ from domain.modelling.plan import Plan, PlanMeasure from domain.modelling.recommendation import MeasureOption, Recommendation from domain.modelling.generators.roof_recommendation import recommend_roof_insulation from domain.modelling.portfolio_goal import PortfolioGoal +from domain.property_baseline.property_baseline_performance import ( + PropertyBaselinePerformance, +) from domain.modelling.scenario import Scenario from domain.modelling.scoring.scoring import ( MeasureImpact, @@ -51,6 +55,8 @@ from repositories.product.product_repository import ProductRepository from repositories.solar.solar_repository import SolarRepository from repositories.unit_of_work import UnitOfWork +logger = logging.getLogger(__name__) + # 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 @@ -489,6 +495,14 @@ def _objective_for( return build_objective(bill_derivation) +def _check_baseline_coherence( + property_id: int, + modelled_baseline_sap: float, + persisted: Optional[PropertyBaselinePerformance], +) -> None: + raise NotImplementedError + + def _target_sap(scenario: Scenario) -> Optional[float]: """The SAP rating the Optimiser repairs toward — the floor of the goal band for an INCREASING_EPC goal, else None (no SAP target). diff --git a/tests/orchestration/test_modelling_baseline_coherence.py b/tests/orchestration/test_modelling_baseline_coherence.py new file mode 100644 index 000000000..8be5d7575 --- /dev/null +++ b/tests/orchestration/test_modelling_baseline_coherence.py @@ -0,0 +1,96 @@ +"""Modelling and the Baseline stage must agree on what the Property scores now. + +Per ADR-0002 the persisted Effective Performance is what the app reports as the +Property's current state, and a Plan's `post_*` figures are read against it. The +Modelling stage re-scores the Effective EPC itself rather than reading that row, +so the two can drift apart silently — and when they do, the app subtracts one +baseline from the other and reports a gain nobody modelled (#1652 / #1655: +property 749719 showed "+9.65 SAP" on a plan whose own CO2, bill and consumption +savings were all exactly 0.0). + +Modelling keeps its own re-score — anchoring the SAP to the persisted integer +would leave the Plan's SAP disagreeing with its own CO2 / primary energy / bill, +which all come off the same SapResult. Instead the divergence is made *loud*, so +a future drift surfaces as an error instead of a phantom gain in a board pack. +""" + +from __future__ import annotations + +import logging +from typing import Optional + +import pytest + +from datatypes.epc.domain.epc import Epc +from domain.property_baseline.performance import Performance +from domain.property_baseline.property_baseline_performance import ( + PropertyBaselinePerformance, +) +from orchestration.modelling_orchestrator import ( + _check_baseline_coherence, # pyright: ignore[reportPrivateUsage] +) + + +def _persisted(effective_sap: int) -> PropertyBaselinePerformance: + """A Baseline Performance whose Effective half scores `effective_sap`.""" + effective = Performance( + sap_score=effective_sap, + epc_band=Epc.from_sap_score(effective_sap), + co2_emissions=2.0, + primary_energy_intensity=200, + ) + return PropertyBaselinePerformance( + lodged=None, + effective=effective, + rebaseline_reason="physical_state_changed", + ) + + +def test_divergence_beyond_rounding_is_logged_as_an_error( + caplog: pytest.LogCaptureFixture, +) -> None: + # Arrange — the 749719 case: the Baseline stage persisted an Effective SAP of + # 63, but Modelling scored the same Property at 72.65. + persisted: Optional[PropertyBaselinePerformance] = _persisted(63) + + # Act + with caplog.at_level(logging.ERROR): + _check_baseline_coherence(749719, 72.65, persisted) + + # Assert — loud, and it names the Property and both figures. + assert "749719" in caplog.text + assert "72.65" in caplog.text + assert "63" in caplog.text + assert any(r.levelno == logging.ERROR for r in caplog.records) + + +def test_a_baseline_agreeing_within_rounding_is_silent( + caplog: pytest.LogCaptureFixture, +) -> None: + # Arrange — the persisted Effective SAP is the *rounded* score, so a + # continuous 63.22 against a stored 63 is agreement, not divergence. + persisted: Optional[PropertyBaselinePerformance] = _persisted(63) + + # Act + with caplog.at_level(logging.WARNING): + _check_baseline_coherence(749719, 63.22, persisted) + + # Assert + assert caplog.records == [] + + +def test_a_property_with_no_persisted_baseline_warns_rather_than_failing( + 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). + + # Act + with caplog.at_level(logging.WARNING): + _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)