From f55982a4eefab7fd2c323080d446cfb354dd367d Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:00:35 +0000 Subject: [PATCH 01/20] =?UTF-8?q?Publish=20a=20half-point=20SAP=20rating?= =?UTF-8?q?=20rounded=20up=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) --- .../sap10_calculator/worksheet/test_rating.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/domain/sap10_calculator/worksheet/test_rating.py b/tests/domain/sap10_calculator/worksheet/test_rating.py index 8eccda886..3867abe9f 100644 --- a/tests/domain/sap10_calculator/worksheet/test_rating.py +++ b/tests/domain/sap10_calculator/worksheet/test_rating.py @@ -109,6 +109,29 @@ def test_sap_rating_integer_rounds_to_nearest_and_clamps_to_minimum_one() -> Non assert catastrophic == 1 +def test_sap_rating_integer_rounds_a_half_point_up_not_to_even() -> None: + # Arrange — §13 "rounded to the nearest integer" is half-UP under SAP, the + # convention the mapper already follows (`_round_half_up_2dp`). Python's + # built-in `round` is half-to-EVEN, so it drops an exact x.5 to x whenever x + # is even. That bites hardest at the band floors, where one point is a whole + # band: a continuous 68.5 must publish as SAP 69 (band C), not 68 (band D). + # The linear branch (9) inverts exactly, so these ECFs land on a true .5: + # ECF = (100 − 68.5) / 13.95 → SAP 68.5 (band C floor) + # ECF = (100 − 54.5) / 13.95 → SAP 54.5 (band D floor) + ecf_at_band_c_floor = (100.0 - 68.5) / 13.95 + ecf_at_band_d_floor = (100.0 - 54.5) / 13.95 + assert sap_rating(ecf=ecf_at_band_c_floor) == 68.5 + assert sap_rating(ecf=ecf_at_band_d_floor) == 54.5 + + # Act + at_band_c_floor = sap_rating_integer(ecf=ecf_at_band_c_floor) + at_band_d_floor = sap_rating_integer(ecf=ecf_at_band_d_floor) + + # Assert — the half point rounds up into the band above. + assert at_band_c_floor == 69 + assert at_band_d_floor == 55 + + def test_environmental_impact_rating_linear_branch_below_threshold() -> None: # Arrange — Equations (10)/(12): CF = CO2/(TFA+45); when CF < 28.3, # EI = 100 − 1.34 × CF. For a 100 m² home emitting 1500 kg CO2/yr: From 149d957248a85cf20cac2e8c2dd576f354a5e646 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:02:44 +0000 Subject: [PATCH 02/20] =?UTF-8?q?Publish=20a=20half-point=20SAP=20rating?= =?UTF-8?q?=20rounded=20up=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) --- domain/sap10_calculator/worksheet/rating.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/domain/sap10_calculator/worksheet/rating.py b/domain/sap10_calculator/worksheet/rating.py index ca132479a..6ed2e73fd 100644 --- a/domain/sap10_calculator/worksheet/rating.py +++ b/domain/sap10_calculator/worksheet/rating.py @@ -20,6 +20,7 @@ Table 12 (page 191). from __future__ import annotations +from decimal import ROUND_HALF_UP, Decimal from math import log10 from typing import Final @@ -59,8 +60,14 @@ def sap_rating_integer(*, ecf: float) -> int: """SAP 10.2 §13: round the continuous SAP rating to the nearest integer and clamp to a minimum of 1 ("if the result of the calculation is less than 1 the rating should be quoted as 1"). The integer value is the - one published on the EPC.""" - return max(1, round(sap_rating(ecf=ecf))) + one published on the EPC. + + "Nearest" is half-UP, as everywhere else the spec rounds (see the mapper's + `_round_half_up_2dp`). Python's built-in `round` is half-to-EVEN and would + drop an exact x.5 to x for even x — at a band floor that publishes a whole + band low (a continuous 68.5 as SAP 68 / band D rather than 69 / band C).""" + rating: Decimal = Decimal(sap_rating(ecf=ecf)) + return max(1, int(rating.quantize(Decimal("1"), rounding=ROUND_HALF_UP))) def environmental_impact_rating( From 1e4526f33f2e72787141cd36e6e9de2365618431 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:03:25 +0000 Subject: [PATCH 03/20] =?UTF-8?q?Locate=20a=20band's=20floor=20on=20the=20?= =?UTF-8?q?continuous=20SAP=20scale=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) --- datatypes/epc/domain/epc.py | 3 +++ tests/datatypes/epc/domain/test_epc.py | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/datatypes/epc/domain/epc.py b/datatypes/epc/domain/epc.py index ae4fd8241..6c65cf0a9 100644 --- a/datatypes/epc/domain/epc.py +++ b/datatypes/epc/domain/epc.py @@ -46,3 +46,6 @@ class Epc(Enum): Epc.G: 1, } return bounds[self] + + def sap_lower_bound_continuous(self) -> float: + raise NotImplementedError diff --git a/tests/datatypes/epc/domain/test_epc.py b/tests/datatypes/epc/domain/test_epc.py index 474c5e890..062a4d78b 100644 --- a/tests/datatypes/epc/domain/test_epc.py +++ b/tests/datatypes/epc/domain/test_epc.py @@ -4,6 +4,8 @@ target).""" from __future__ import annotations +from decimal import ROUND_HALF_UP, Decimal + import pytest from datatypes.epc.domain.epc import Epc @@ -24,3 +26,27 @@ def test_sap_lower_bound_returns_the_band_floor() -> None: def test_band_floor_round_trips_through_from_sap_score(band: Epc) -> None: # Act / Assert — a band's floor scores back to that band. assert Epc.from_sap_score(band.sap_lower_bound()) is band + + +@pytest.mark.parametrize("band", list(Epc)) +def test_continuous_band_floor_is_where_the_published_rating_enters_the_band( + band: Epc, +) -> None: + # Arrange — the Optimiser judges "reached band X" on the calculator's + # *continuous* SAP, but a dwelling is in a band by its *published* rating + # (the half-up-rounded integer). The continuous floor is the point those two + # agree: the lowest continuous score whose published rating is in the band. + # Asserted against the real publishing path (round half up → from_sap_score) + # rather than an arithmetic literal, so the two cannot drift apart. + def published_band(continuous: float) -> Epc: + rounded = int(Decimal(continuous).quantize(Decimal("1"), rounding=ROUND_HALF_UP)) + return Epc.from_sap_score(rounded) + + # Act + continuous_floor: float = band.sap_lower_bound_continuous() + + # Assert — at the floor the dwelling publishes into the band; a hair below + # it publishes into a worse one. (Band G has no worse band to fall into.) + assert published_band(continuous_floor) is band + if band is not Epc.G: + assert published_band(continuous_floor - 0.01) is not band From 31ad9448d21a0419215a3cfa0607aa86cde1c143 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:03:54 +0000 Subject: [PATCH 04/20] =?UTF-8?q?Locate=20a=20band's=20floor=20on=20the=20?= =?UTF-8?q?continuous=20SAP=20scale=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) --- datatypes/epc/domain/epc.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/datatypes/epc/domain/epc.py b/datatypes/epc/domain/epc.py index 6c65cf0a9..67b69dbaa 100644 --- a/datatypes/epc/domain/epc.py +++ b/datatypes/epc/domain/epc.py @@ -48,4 +48,13 @@ class Epc(Enum): return bounds[self] def sap_lower_bound_continuous(self) -> float: - raise NotImplementedError + """The band floor on the **continuous** SAP scale — the lowest un-rounded + rating whose *published* rating falls in this band (C → 68.5). + + A dwelling is in a band by its published rating, which is the continuous + score rounded half up, so the published rating enters the band half a + point below the integer floor. Anything judging "has this dwelling + reached band X?" against a continuous score must compare with this, not + with `sap_lower_bound`: a dwelling at a continuous 68.6 publishes as SAP + 69 and *is* band C, but reads as 0.4 short of an integer floor of 69.""" + return self.sap_lower_bound() - 0.5 From 8548b8304b17122215eaf1260e67008c00648831 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:04:48 +0000 Subject: [PATCH 05/20] =?UTF-8?q?Sell=20no=20works=20to=20a=20dwelling=20a?= =?UTF-8?q?lready=20at=20the=20goal=20band=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) --- .../test_modelling_goal_band_floor.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/orchestration/test_modelling_goal_band_floor.py diff --git a/tests/orchestration/test_modelling_goal_band_floor.py b/tests/orchestration/test_modelling_goal_band_floor.py new file mode 100644 index 000000000..3238fa477 --- /dev/null +++ b/tests/orchestration/test_modelling_goal_band_floor.py @@ -0,0 +1,42 @@ +"""The Optimiser judges "already at the goal band" on the *published* rating. + +A dwelling is in a band by the rating published on its EPC — the continuous SAP +rounded half up. The Optimiser works in continuous SAP, so a goal of "reach band +X" has to be met at the point the published rating enters band X (the integer +floor less half a point), not at the integer floor itself. Comparing a +continuous score against the integer floor makes a dwelling that already +publishes in the band read as a fraction short, and the Optimiser buys the +cheapest measure to close a gap that does not exist (#1652 / #1654 — 630 homes +in portfolio 796 published at SAP 69 / band C, each sold ~1 marginal SAP point +of works toward a band they had already reached). + +End-to-end through ``run_modelling`` (no database) with the real calculator, +against Elmhurst certs that sit in the boundary window. +""" + +from __future__ import annotations + +from datatypes.epc.domain.epc_property_data import EpcPropertyData +from domain.modelling.plan import Plan +from harness.console import run_modelling +from tests.domain.modelling._elmhurst_recommendation import ( + parse_recommendation_summary, +) + + +def _dwelling(fixture: str) -> EpcPropertyData: + return parse_recommendation_summary(fixture) + + +def test_dwelling_published_at_the_goal_band_is_sold_no_works() -> None: + # Arrange — cert 001431 with a flat roof scores a continuous 54.575, which + # publishes as SAP 55: exactly the band D floor, so the dwelling already IS + # band D. Against an integer floor of 55 it reads as 0.425 short. + already_band_d = _dwelling("flat_roof_001431_before.pdf") + + # Act — ask for the band it is already in. + plan: Plan = run_modelling(already_band_d, goal_band="D", print_table=False) + + # Assert — nothing to buy: the goal is already met. + assert plan.measures == () + assert plan.cost_of_works == 0.0 From bc2f6c8a8215db31e914ca93b250e0232db2c077 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:05:55 +0000 Subject: [PATCH 06/20] =?UTF-8?q?Sell=20no=20works=20to=20a=20dwelling=20a?= =?UTF-8?q?lready=20at=20the=20goal=20band=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 | 11 +++++++++-- .../test_modelling_goal_band_floor.py | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/orchestration/modelling_orchestrator.py b/orchestration/modelling_orchestrator.py index 69e88feac..ed324e975 100644 --- a/orchestration/modelling_orchestrator.py +++ b/orchestration/modelling_orchestrator.py @@ -491,10 +491,17 @@ def _objective_for( 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).""" + band for an INCREASING_EPC goal, else None (no SAP target). + + On the **continuous** scale, because that is the currency every figure the + Optimiser compares it against is in. The dwelling is in the goal band once + its *published* rating is, which happens half a point below the integer + floor — target the integer and a dwelling publishing at the floor reads as a + fraction short, and the Optimiser buys works toward a band it is already in + (#1654).""" if scenario.goal != PortfolioGoal.INCREASING_EPC.value: return None - return float(Epc(scenario.goal_value).sap_lower_bound()) + return Epc(scenario.goal_value).sap_lower_bound_continuous() def _best_practice_key(option: MeasureOption) -> int: diff --git a/tests/orchestration/test_modelling_goal_band_floor.py b/tests/orchestration/test_modelling_goal_band_floor.py index 3238fa477..63210345b 100644 --- a/tests/orchestration/test_modelling_goal_band_floor.py +++ b/tests/orchestration/test_modelling_goal_band_floor.py @@ -40,3 +40,18 @@ def test_dwelling_published_at_the_goal_band_is_sold_no_works() -> None: # Assert — nothing to buy: the goal is already met. assert plan.measures == () assert plan.cost_of_works == 0.0 + + +def test_dwelling_below_the_goal_band_is_still_taken_to_it() -> None: + # Arrange — the same cert with an uninsulated loft scores a continuous + # 37.349, published SAP 37: band F, genuinely short of band D. Lowering the + # target by half a point must not let the package stop short of the band. + below_band_d = _dwelling("loft_001431_before.pdf") + + # Act + plan: Plan = run_modelling(below_band_d, goal_band="D", print_table=False) + + # Assert — real works, and they carry the dwelling into band D. + assert plan.measures != () + assert plan.cost_of_works > 0.0 + assert plan.post_retrofit.sap_continuous >= 54.5 From 08abd3131b50c0cc573e2ecb951ea7abce1019d2 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:07:11 +0000 Subject: [PATCH 07/20] =?UTF-8?q?Band=20a=20continuous=20SAP=20score=20on?= =?UTF-8?q?=20its=20published=20rating=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) --- datatypes/epc/domain/epc.py | 4 ++++ tests/datatypes/epc/domain/test_epc.py | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/datatypes/epc/domain/epc.py b/datatypes/epc/domain/epc.py index 67b69dbaa..18d72b310 100644 --- a/datatypes/epc/domain/epc.py +++ b/datatypes/epc/domain/epc.py @@ -32,6 +32,10 @@ class Epc(Enum): return cls.F return cls.G + @classmethod + def from_sap_continuous(cls, score: float) -> "Epc": + raise NotImplementedError + def sap_lower_bound(self) -> int: """The minimum SAP rating in this band — the inverse of `from_sap_score` (A → 92, B → 81, C → 69, D → 55, E → 39, F → 21, diff --git a/tests/datatypes/epc/domain/test_epc.py b/tests/datatypes/epc/domain/test_epc.py index 062a4d78b..8731c5af3 100644 --- a/tests/datatypes/epc/domain/test_epc.py +++ b/tests/datatypes/epc/domain/test_epc.py @@ -28,6 +28,20 @@ def test_band_floor_round_trips_through_from_sap_score(band: Epc) -> None: assert Epc.from_sap_score(band.sap_lower_bound()) is band +def test_a_continuous_score_bands_on_its_published_rating() -> None: + # Arrange — callers holding a continuous SAP want the band the dwelling + # would be *published* in, which means rounding half up. Rounding half to + # even (Python's `round`) drops an exact x.5 at a band floor into the band + # below: a continuous 68.5 is published SAP 69 and so band C, not band D. + + # Act / Assert — the half point carries the score into the band above. + assert Epc.from_sap_continuous(68.5) is Epc.C + assert Epc.from_sap_continuous(54.5) is Epc.D + # And either side of a floor still bands the obvious way. + assert Epc.from_sap_continuous(68.49) is Epc.D + assert Epc.from_sap_continuous(69.4) is Epc.C + + @pytest.mark.parametrize("band", list(Epc)) def test_continuous_band_floor_is_where_the_published_rating_enters_the_band( band: Epc, From 07a71ad50408ecab42dabd3449bd6d37d65713b2 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:07:55 +0000 Subject: [PATCH 08/20] =?UTF-8?q?Band=20a=20continuous=20SAP=20score=20on?= =?UTF-8?q?=20its=20published=20rating=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) --- datatypes/epc/domain/epc.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/datatypes/epc/domain/epc.py b/datatypes/epc/domain/epc.py index 18d72b310..e1b6d95aa 100644 --- a/datatypes/epc/domain/epc.py +++ b/datatypes/epc/domain/epc.py @@ -1,3 +1,4 @@ +from decimal import ROUND_HALF_UP, Decimal from enum import Enum @@ -34,7 +35,14 @@ class Epc(Enum): @classmethod def from_sap_continuous(cls, score: float) -> "Epc": - raise NotImplementedError + """The band an un-rounded SAP rating is *published* in. + + A dwelling is banded on the integer published on its EPC, which rounds + half up (SAP 10.2 §13) — so this is `from_sap_score` over that rounding + rather than over Python's half-to-even `round`, which would drop an + exact x.5 at a band floor into the band below.""" + published: int = int(Decimal(score).quantize(Decimal("1"), rounding=ROUND_HALF_UP)) + return cls.from_sap_score(published) def sap_lower_bound(self) -> int: """The minimum SAP rating in this band — the inverse of From 27060f4d8147e30978549443621e5b89fbe17286 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:11:06 +0000 Subject: [PATCH 09/20] =?UTF-8?q?Band=20every=20continuous=20SAP=20score?= =?UTF-8?q?=20through=20the=20published-rating=20rule=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- domain/modelling/plan.py | 8 ++++---- harness/plan_table.py | 2 +- scripts/audit/anomalies.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/domain/modelling/plan.py b/domain/modelling/plan.py index 9e8349e9d..bd14da3af 100644 --- a/domain/modelling/plan.py +++ b/domain/modelling/plan.py @@ -91,13 +91,13 @@ class Plan: @property def post_epc_rating(self) -> Epc: - """The post-retrofit EPC band, from the rounded SAP rating.""" - return Epc.from_sap_score(round(self.post_retrofit.sap_continuous)) + """The post-retrofit EPC band, as it would be published.""" + return Epc.from_sap_continuous(self.post_retrofit.sap_continuous) @property def baseline_epc_rating(self) -> Epc: - """The baseline EPC band, from the rounded baseline SAP rating.""" - return Epc.from_sap_score(round(self.baseline.sap_continuous)) + """The baseline EPC band, as it would be published.""" + return Epc.from_sap_continuous(self.baseline.sap_continuous) @property def valuation(self) -> ValuationUplift: diff --git a/harness/plan_table.py b/harness/plan_table.py index 7c6e96f1a..ddb5d2513 100644 --- a/harness/plan_table.py +++ b/harness/plan_table.py @@ -17,7 +17,7 @@ _KG_PER_TONNE = 1000.0 def _band(sap_continuous: float) -> str: - return Epc.from_sap_score(round(sap_continuous)).value + return Epc.from_sap_continuous(sap_continuous).value def _signed_gbp(value: Optional[float]) -> str: diff --git a/scripts/audit/anomalies.py b/scripts/audit/anomalies.py index 0ee0386d4..7d8f1b11a 100644 --- a/scripts/audit/anomalies.py +++ b/scripts/audit/anomalies.py @@ -52,7 +52,7 @@ def _band_rank(band: Optional[str]) -> Optional[int]: def _band_of(score: Optional[float]) -> Optional[str]: if score is None: return None - return Epc.from_sap_score(round(score)).value + return Epc.from_sap_continuous(score).value def _int_or_none(value: object) -> Optional[int]: From 0df40278e518a703fc8e6e04a7b31d49bd1c9dce Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:12:33 +0000 Subject: [PATCH 10/20] =?UTF-8?q?Rank=20costed=20works=20on=20an=20already?= =?UTF-8?q?-compliant=20dwelling=20HIGH=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) --- tests/scripts/test_audit_anomalies.py | 56 +++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/scripts/test_audit_anomalies.py b/tests/scripts/test_audit_anomalies.py index dd8771500..07ba6d2bc 100644 --- a/tests/scripts/test_audit_anomalies.py +++ b/tests/scripts/test_audit_anomalies.py @@ -1,9 +1,13 @@ +import dataclasses from typing import Optional import pytest from scripts.audit.anomalies import ( + _REGISTRY, PropertyAudit, + Severity, + _already_meets_goal_with_works, _effective_lodged_divergence, _implausible_lodged_score, _plan_stops_short_of_goal, @@ -155,3 +159,55 @@ class TestPlanStopsShortOfGoal: # Assert assert result is None + + +class TestAlreadyMeetsGoalWithWorks: + """Costed works on a dwelling already in the goal band (#1652 / #1654). + + Ranked HIGH: this is the signature of the Optimiser judging the goal against + something other than the published band, and it bills real money — 630 homes + in portfolio 796 published at band C were each sold ~1 SAP point of works + toward a band they had already reached. It must not sit in the + lower-severity bucket that gets waved through as stale-plan noise. + """ + + def test_fires_when_a_dwelling_already_at_goal_is_sold_works(self) -> None: + # Arrange — published band C, scenario goal C, yet £678 of works. + audit = _make_audit(scenario_goal_band="C", cost_of_works=678.0) + audit = dataclasses.replace(audit, effective_band="C") + + # Act + result = _already_meets_goal_with_works(audit) + + # Assert + assert result is not None + assert "678" in result + + def test_silent_when_the_dwelling_is_below_the_goal_band(self) -> None: + # Arrange — published band D against a goal of C: works are warranted. + audit = _make_audit(scenario_goal_band="C", cost_of_works=678.0) + audit = dataclasses.replace(audit, effective_band="D") + + # Act + result = _already_meets_goal_with_works(audit) + + # Assert + assert result is None + + def test_silent_when_an_already_compliant_dwelling_is_sold_nothing(self) -> None: + # Arrange — already band C and a £0 plan: the correct outcome. + audit = _make_audit(scenario_goal_band="C", cost_of_works=0.0) + audit = dataclasses.replace(audit, effective_band="C") + + # Act + result = _already_meets_goal_with_works(audit) + + # Assert + assert result is None + + def test_is_registered_at_high_severity(self) -> None: + # Act — the severity the runner reports it under. + registered = {name: severity for name, severity, _ in _REGISTRY} + + # Assert + assert registered["already-meets-goal-with-works"] is Severity.HIGH From 68ffb0c47cc74c6fc929c6db0d7f56d863212ab2 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:12:58 +0000 Subject: [PATCH 11/20] =?UTF-8?q?Rank=20costed=20works=20on=20an=20already?= =?UTF-8?q?-compliant=20dwelling=20HIGH=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) --- scripts/audit/anomalies.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/scripts/audit/anomalies.py b/scripts/audit/anomalies.py index 7d8f1b11a..3f05993e1 100644 --- a/scripts/audit/anomalies.py +++ b/scripts/audit/anomalies.py @@ -163,10 +163,22 @@ def _plan_score_below_baseline(a: PropertyAudit) -> Optional[str]: return f"post SAP {a.post_sap:.1f} below effective baseline {a.effective_sap:.1f} (Δ{a.post_sap - a.effective_sap:.1f})" -@check("already-meets-goal-with-works", Severity.MEDIUM) +@check("already-meets-goal-with-works", Severity.HIGH) def _already_meets_goal_with_works(a: PropertyAudit) -> Optional[str]: """The property already meets/exceeds the scenario's goal band, yet the plan - spends money on measures — nothing should be recommended.""" + spends money on measures — nothing should be recommended. + + Provenance: #1652 / #1654. Ranked HIGH because it bills real money and is the + signature of the Optimiser judging the goal against something other than the + published band. It ran at MEDIUM and was waved through as stale-plan noise + while 630 homes in portfolio 796 published at band C were each sold ~1 SAP + point of works toward a band they had already reached (£282k), and 172 more + in 824. The cause there was a continuous baseline compared against the + *integer* band floor, so a dwelling publishing at the floor read as a + fraction short; the goal is now judged on the continuous floor + (`Epc.sap_lower_bound_continuous`). Any fresh firing means a baseline the + Optimiser used has diverged from the published one again — re-debug it, + do not re-bucket it.""" goal, base = _band_rank(a.scenario_goal_band), _band_rank(a.effective_band) if goal is None or base is None or base > goal: return None From 6f83418c4c940d63889c4a7cfcf14a3286e18af3 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 10:18:09 +0000 Subject: [PATCH 12/20] =?UTF-8?q?Type-clean=20the=20anomaly=20check=20test?= =?UTF-8?q?s=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/scripts/test_audit_anomalies.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/scripts/test_audit_anomalies.py b/tests/scripts/test_audit_anomalies.py index 07ba6d2bc..86d7889b1 100644 --- a/tests/scripts/test_audit_anomalies.py +++ b/tests/scripts/test_audit_anomalies.py @@ -1,16 +1,18 @@ import dataclasses from typing import Optional -import pytest - +# The checks are registry-driven: production reaches them through `_REGISTRY`, +# never by name, so the underscore marks "not a direct call site" rather than +# "untestable". Naming them here is the only way to exercise one check in +# isolation, hence the private-usage waivers. from scripts.audit.anomalies import ( - _REGISTRY, + _REGISTRY, # pyright: ignore[reportPrivateUsage] PropertyAudit, Severity, - _already_meets_goal_with_works, - _effective_lodged_divergence, - _implausible_lodged_score, - _plan_stops_short_of_goal, + _already_meets_goal_with_works, # pyright: ignore[reportPrivateUsage] + _effective_lodged_divergence, # pyright: ignore[reportPrivateUsage] + _implausible_lodged_score, # pyright: ignore[reportPrivateUsage] + _plan_stops_short_of_goal, # pyright: ignore[reportPrivateUsage] ) From 31edd01999b930530ad802c68205784ed8ec19ec Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 15:50:39 +0000 Subject: [PATCH 13/20] =?UTF-8?q?Flag=20a=20modelling=20baseline=20that=20?= =?UTF-8?q?disagrees=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) From 8c2c19495c76d55ddc5639424a749f58d7276066 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 15:53:04 +0000 Subject: [PATCH 14/20] =?UTF-8?q?Flag=20a=20modelling=20baseline=20that=20?= =?UTF-8?q?disagrees=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, ) From 08d2b874d5ab341bbce3bc4cc8fdc9aa5fad55d1 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 15:53:59 +0000 Subject: [PATCH 15/20] =?UTF-8?q?Flag=20works=20bought=20for=20no=20real?= =?UTF-8?q?=20SAP=20gain=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) --- scripts/audit/anomalies.py | 5 ++ tests/scripts/test_audit_anomalies.py | 68 +++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/scripts/audit/anomalies.py b/scripts/audit/anomalies.py index 3f05993e1..022f6fe27 100644 --- a/scripts/audit/anomalies.py +++ b/scripts/audit/anomalies.py @@ -187,6 +187,11 @@ def _already_meets_goal_with_works(a: PropertyAudit) -> Optional[str]: return f"already {a.effective_band} >= goal {a.scenario_goal_band} but cost_of_works £{a.cost_of_works:.0f}" +@check("costed-plan-without-real-gain", Severity.HIGH) +def _costed_plan_without_real_gain(a: PropertyAudit) -> Optional[str]: + raise NotImplementedError + + @check("plan-stops-short-of-goal", Severity.HIGH) def _plan_stops_short_of_goal(a: PropertyAudit) -> Optional[str]: """The default plan ends BELOW the scenario's goal band on a scenario that is diff --git a/tests/scripts/test_audit_anomalies.py b/tests/scripts/test_audit_anomalies.py index 86d7889b1..e34bf250d 100644 --- a/tests/scripts/test_audit_anomalies.py +++ b/tests/scripts/test_audit_anomalies.py @@ -10,6 +10,7 @@ from scripts.audit.anomalies import ( PropertyAudit, Severity, _already_meets_goal_with_works, # pyright: ignore[reportPrivateUsage] + _costed_plan_without_real_gain, # pyright: ignore[reportPrivateUsage] _effective_lodged_divergence, # pyright: ignore[reportPrivateUsage] _implausible_lodged_score, # pyright: ignore[reportPrivateUsage] _plan_stops_short_of_goal, # pyright: ignore[reportPrivateUsage] @@ -213,3 +214,70 @@ class TestAlreadyMeetsGoalWithWorks: # Assert assert registered["already-meets-goal-with-works"] is Severity.HIGH + + +class TestCostedPlanWithoutRealGain: + """Money spent for no meaningful SAP movement (#1652 / #1655). + + The coverage hole the #1652 diagnosis found: `plan-score-below-baseline` + only fires on a *drop* beyond 0.5 and `zero-works-post-differs` only on £0 + plans, so a costed plan landing flat against the effective baseline was + caught by neither — including the 112 homes whose modelled post-retrofit SAP + sat ~0.2 *below* their effective baseline. + """ + + def test_fires_when_works_are_bought_for_no_real_gain(self) -> None: + # Arrange — £6,240 of works moving the dwelling 0.2 SAP. + audit = _make_audit( + effective_sap=63.0, post_sap=63.2, cost_of_works=6240.0 + ) + + # Act + result = _costed_plan_without_real_gain(audit) + + # Assert + assert result is not None + assert "6240" in result or "6,240" in result + + def test_fires_when_the_post_score_lands_below_the_baseline(self) -> None: + # Arrange — the likely-downgrade case: paid works, post *below* baseline. + audit = _make_audit( + effective_sap=63.0, post_sap=62.8, cost_of_works=1029.0 + ) + + # Act + result = _costed_plan_without_real_gain(audit) + + # Assert + assert result is not None + + def test_silent_when_the_works_deliver_a_real_uplift(self) -> None: + # Arrange — £6,240 buying 9 SAP points: what a retrofit should look like. + audit = _make_audit( + effective_sap=63.0, post_sap=72.0, cost_of_works=6240.0 + ) + + # Act + result = _costed_plan_without_real_gain(audit) + + # Assert + assert result is None + + def test_silent_on_a_zero_cost_plan(self) -> None: + # Arrange — no money spent, so there is nothing to justify; a £0 plan + # that moves nothing is the correct outcome for an already-compliant + # dwelling, and is covered by its own check. + audit = _make_audit(effective_sap=63.0, post_sap=63.0, cost_of_works=0.0) + + # Act + result = _costed_plan_without_real_gain(audit) + + # Assert + assert result is None + + def test_is_registered_at_high_severity(self) -> None: + # Act + registered = {name: severity for name, severity, _ in _REGISTRY} + + # Assert + assert registered["costed-plan-without-real-gain"] is Severity.HIGH From 31be36cca5a4d12e79993a0ce22e10f8e59b0a38 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 15:57:25 +0000 Subject: [PATCH 16/20] =?UTF-8?q?Flag=20works=20bought=20for=20no=20real?= =?UTF-8?q?=20SAP=20gain=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) --- scripts/audit/anomalies.py | 46 +++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/scripts/audit/anomalies.py b/scripts/audit/anomalies.py index 022f6fe27..8c341cd3d 100644 --- a/scripts/audit/anomalies.py +++ b/scripts/audit/anomalies.py @@ -26,6 +26,11 @@ be justified against the real distribution, not guessed. Read-only: this script never writes to the DB. """ +# Every check is reached through `_REGISTRY` via the `@check` decorator, never by +# name, so pyright sees each one as an unused function. That is the design, not a +# defect — suppress it for the module rather than tagging a dozen definitions. +# pyright: reportUnusedFunction=false + from __future__ import annotations import argparse @@ -42,6 +47,11 @@ from scripts.e2e_common import build_engine, load_env # A..G, A best — index is the rank (lower = better) for band comparisons. _BANDS = "ABCDEFG" +# The smallest SAP movement that counts as a real improvement. The effective +# baseline is a rounded integer, so anything at or under half a point is within +# the rounding the published band already absorbs — invisible on a certificate. +_MIN_REAL_SAP_GAIN = 0.5 + def _band_rank(band: Optional[str]) -> Optional[int]: if band is None or band not in _BANDS: @@ -189,7 +199,34 @@ def _already_meets_goal_with_works(a: PropertyAudit) -> Optional[str]: @check("costed-plan-without-real-gain", Severity.HIGH) def _costed_plan_without_real_gain(a: PropertyAudit) -> Optional[str]: - raise NotImplementedError + """Money was spent but the modelled post-retrofit SAP barely moves off the + effective baseline — works that buy no real improvement. + + Provenance: #1652 / #1655. This closes a coverage hole rather than adding a + new opinion: `plan-score-below-baseline` only fires on a *drop* beyond 0.5, + and `zero-works-post-differs` only on £0 plans, so a **costed** plan landing + flat against the baseline was caught by neither. It is what the 112 + "no real improvement" homes in portfolio 796 looked like — priced plans whose + post-retrofit SAP sat ~0.2 *below* the effective baseline — and what the + band-boundary defect (#1654) produced at scale: 630 homes each sold ~1 SAP + point of works toward a band they had already reached. + + The 0.5 threshold is the same "within rounding is not a real move" line the + published banding uses: the effective baseline is a rounded integer, so a + sub-half-point shift is not a change anyone can see on a certificate. Spend + that buys less than that is not a retrofit.""" + if a.effective_sap is None or a.post_sap is None: + return None + cost = a.cost_of_works or 0.0 + if cost <= 0.0: + return None + gain = a.post_sap - a.effective_sap + if gain > _MIN_REAL_SAP_GAIN: + return None + return ( + f"£{cost:.0f} of works for {gain:+.1f} SAP " + f"(effective {a.effective_sap:.1f} → post {a.post_sap:.1f})" + ) @check("plan-stops-short-of-goal", Severity.HIGH) @@ -555,11 +592,14 @@ def _load( rollups = { m["property_id"]: (m["solar_sap"], m["solar_bill"], m["n_measures"]) for m in ( - row._mapping for row in conn.execute(_ROLLUP_QUERY, params) + # `Row._mapping` is public API in SQLAlchemy 2.0; only the + # stub marks it protected. + row._mapping # pyright: ignore[reportPrivateUsage] + for row in conn.execute(_ROLLUP_QUERY, params) ) } for r in conn.execute(_QUERY, params): - m = r._mapping + m = r._mapping # pyright: ignore[reportPrivateUsage] solar_sap, solar_bill, n_measures = rollups.get(m["id"], (None, None, 0)) out.append( PropertyAudit( From 735a3a1c8e236d2ed9deba728b2e5c09ac109f9b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 16:04:49 +0000 Subject: [PATCH 17/20] =?UTF-8?q?Record=20the=20banding=20and=20baseline-c?= =?UTF-8?q?oherence=20decisions=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 8 +- ...rship-is-judged-on-the-published-rating.md | 70 +++++++++++++++++ ...aseline-coherence-rather-than-anchoring.md | 76 +++++++++++++++++++ 3 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 docs/adr/0064-band-membership-is-judged-on-the-published-rating.md create mode 100644 docs/adr/0065-modelling-asserts-baseline-coherence-rather-than-anchoring.md diff --git a/CONTEXT.md b/CONTEXT.md index 047087e97..c6b900858 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -28,8 +28,8 @@ The date an EPC was lodged with the government register; used to identify the mo _Avoid_: assessment date, submission date **EPC Band**: -A single letter A–G representing a property's current or potential energy efficiency rating. -_Avoid_: energy rating, EPC grade, EPC score +A single letter A–G representing a property's current or potential energy efficiency rating. A dwelling is in a band by its **published rating** — the continuous SAP rounded **half up** to the integer printed on the certificate (SAP 10.2 §13; Python's built-in `round` is half-to-*even* and must not be used for this). Anything holding a continuous score therefore judges band membership via `Epc.from_sap_continuous`, and any "has it reached band X?" test compares against `Epc.sap_lower_bound_continuous()` — the integer floor **less half a point** (C → 68.5), which is where the published rating enters the band. Comparing a continuous score against the raw integer floor makes a dwelling that already publishes in the band read as a fraction short (ADR-0064). +_Avoid_: energy rating, EPC grade, EPC score; comparing a continuous SAP against `sap_lower_bound()` **Schema Type**: The versioned RdSAP or SAP schema that describes the structure of an EPC's raw data (e.g. `RdSAP-Schema-21.0.1`). @@ -156,7 +156,7 @@ The SAP / EPC Band / carbon emissions / Primary Energy Intensity recorded on the _Avoid_: original performance, raw EPC values, recorded baseline **Effective Performance**: -The SAP / EPC Band / carbon emissions / Primary Energy Intensity the modelling pipeline actually scored against — equal to Lodged Performance when no Rebaselining trigger fires, replaced by **SAP10 Calculation** output (the deterministic `Sap10Calculator`, which superseded the old ML-API rebaseliner; an ML residual head over the calculator is future — ADR-0009/0013) when triggered. The half of Baseline Performance that says "what we modelled". +The SAP / EPC Band / carbon emissions / Primary Energy Intensity the modelling pipeline actually scored against — equal to Lodged Performance when no Rebaselining trigger fires, replaced by **SAP10 Calculation** output (the deterministic `Sap10Calculator`, which superseded the old ML-API rebaseliner; an ML residual head over the calculator is future — ADR-0009/0013) when triggered. The half of Baseline Performance that says "what we modelled". **Modelling does not read it**: it re-scores the Effective EPC itself (it needs a continuous score and a `SapResult` to bill from, and ADR-0011 forbids an in-memory hand-off between stages), then **asserts** its re-score agrees with this persisted figure and logs an error when it does not. The two are deliberately *not* reconciled — a divergence means the stages scored different pictures, and anchoring the Plan's SAP to this integer would leave it disagreeing with its own carbon, energy and bill figures (ADR-0065). Because a Plan's `post_*` are read against this figure, an unchecked divergence surfaces to the user as a gain nobody modelled. _Avoid_: modelled performance, rebaselined performance (only correct when rebaselining ran), scored values **Calculated SAP10 Performance**: @@ -309,7 +309,7 @@ A "selecting A requires B" edge between **Recommendations**, for couplings that _Avoid_: best-practice measure (legacy term), forced measure **Optimised Package**: -The subset of a Property's Recommendations selected by the Optimiser Service for installation. For an **Increasing EPC** goal the objective is **least-cost-to-target**: the cheapest package that reaches the goal band — so it **stops at the target and does not overshoot** into a higher band, leaving surplus budget unspent. When the target is **unreachable within budget**, it falls back to the **maximum improvement the budget buys** (best effort, below target). With **no budget** it is simply the cheapest package that reaches the target. Reaching the target is judged on the **true whole-package re-score** (ADR-0016), not on summed per-measure scores. (Other goals — Energy Savings, Reducing CO₂ — don't yet set a target and currently maximise improvement within budget; future work.) +The subset of a Property's Recommendations selected by the Optimiser Service for installation. For an **Increasing EPC** goal the objective is **least-cost-to-target**: the cheapest package that reaches the goal band — so it **stops at the target and does not overshoot** into a higher band, leaving surplus budget unspent. When the target is **unreachable within budget**, it falls back to the **maximum improvement the budget buys** (best effort, below target). With **no budget** it is simply the cheapest package that reaches the target. Reaching the target is judged on the **true whole-package re-score** (ADR-0016), not on summed per-measure scores — and on the **published** band (see **EPC Band**): the target is the goal band's floor on the continuous scale, so a dwelling whose published baseline already meets the goal is **already at target** and gets an empty, £0 package rather than a marginal top-up (ADR-0064). (Other goals — Energy Savings, Reducing CO₂ — don't yet set a target and currently maximise improvement within budget; future work.) _Avoid_: selected measures, default measures, optimal solution, recommended bundle **Measure Type**: diff --git a/docs/adr/0064-band-membership-is-judged-on-the-published-rating.md b/docs/adr/0064-band-membership-is-judged-on-the-published-rating.md new file mode 100644 index 000000000..0615c76c7 --- /dev/null +++ b/docs/adr/0064-band-membership-is-judged-on-the-published-rating.md @@ -0,0 +1,70 @@ +--- +status: accepted (extends ADR-0016; constrains ADR-0062's Increasing EPC path) +--- + +# Band membership is judged on the published rating, not on a raw continuous score + +A dwelling is in an EPC band by the **integer rating printed on its +certificate** — the continuous SAP rounded to the nearest whole point. The +Optimiser, however, works in continuous SAP, and judged "has this dwelling +reached band X?" by comparing that continuous score against the band's +**integer** floor (`Epc.sap_lower_bound()`, band C → `69.0`). + +Those two rules disagree in the half-point window below every band floor. A +dwelling at a continuous 68.6 publishes as SAP 69 and **is** band C, but read as +0.4 short of an integer floor of 69 — so the Optimiser bought the cheapest +measure available to close a gap that did not exist. In portfolio 796 / scenario +1268 ("Reach EPC C") this sold **630 homes already published at band C** about +one SAP point of works each, £282k in total; portfolio 824 / scenario 1278 added +172 more at £56k. The measures were real and were installed against real +budgets; the gap they closed was an artefact of comparing two different scales. + +A second, smaller disagreement sat underneath it: `sap_rating_integer` published +the rating using Python's built-in `round`, which is half-to-**even**, so an +exact `x.5` fell to `x` for even `x` — at a band floor, publishing a whole band +low. SAP rounds half **up**, the convention the mapper already followed +(`_round_half_up_2dp`). + +Found and fixed while working #1652 / #1654 with Khalim, 2026-07-21. + +## Decision + +**One rule decides band membership everywhere: round the continuous SAP half up, +then band the integer. Anything comparing a continuous score to a band judges it +on that basis.** + +- **`sap_rating_integer` rounds half up** (`Decimal` + `ROUND_HALF_UP`), matching + SAP 10.2 §13 and the mapper's existing convention. A continuous 68.5 publishes + as SAP 69. +- **`Epc.sap_lower_bound_continuous()`** is the band floor expressed on the + continuous scale — the integer floor less half a point (C → 68.5), i.e. the + lowest un-rounded score whose *published* rating is in the band. +- **`_target_sap` returns the continuous floor.** The Optimiser's target must be + in the currency of everything it is compared against. This single change fixes + all four comparison sites (`target_gain`, the two reached-target checks, and + `_repair_to_target`'s loop) without touching optimiser control flow. +- **`Epc.from_sap_continuous()`** bands a continuous score as it would be + published, replacing the repeated `from_sap_score(round(x))` idiom at every + call site (`Plan.post_epc_rating`, `Plan.baseline_epc_rating`, the plan table + harness, the anomaly auditor). +- Scoped to the **SAP-goal path only**: goal-aligned CO2 / bill objectives pass + `target_sap=None` (ADR-0062) and are untouched. + +## Consequences + +- A dwelling whose **published** baseline already meets the goal band gets a + **£0, measure-free** plan. Dwellings genuinely below the band still get plans + that reach it — the target moved by half a point, not the ambition. +- Re-modelling 796/1268 should drop ~630 plans and £282,763 of `cost_of_works` + (642 measures), and 824/1278 ~172 plans and £56,226 (176 measures). Both are + unbudgeted, so no budget interaction confounds the change. The remaining spend + is an upper bound: a lower target may also let genuinely-below dwellings stop + one measure earlier. +- Half-up publishing can move a rating by one point only when the continuous + score is *exactly* `x.5`. No fixture in the accuracy corpus is, and the full + SAP suite (2,072 tests, including the Elmhurst worksheet pins) passed + unchanged — but the corpus should be re-checked if the calculator's arithmetic + ever shifts onto exact halves. +- `already-meets-goal-with-works` moves MEDIUM → HIGH in the anomaly auditor. + It was firing correctly all along and was bucketed as expected stale-plan + noise, which is how this survived: a check nobody believes is not a check. diff --git a/docs/adr/0065-modelling-asserts-baseline-coherence-rather-than-anchoring.md b/docs/adr/0065-modelling-asserts-baseline-coherence-rather-than-anchoring.md new file mode 100644 index 000000000..21f3f25a6 --- /dev/null +++ b/docs/adr/0065-modelling-asserts-baseline-coherence-rather-than-anchoring.md @@ -0,0 +1,76 @@ +--- +status: accepted (extends ADR-0002, ADR-0004; composes with ADR-0011) +--- + +# Modelling asserts baseline coherence rather than anchoring to the persisted Effective + +The Baseline stage persists **Effective Performance** — what the app reports as a +Property's current state, and what a Plan's `post_*` figures are read against +(ADR-0002 / ADR-0004). The Modelling stage does not read that row: it re-scores +the Effective EPC itself, because it needs a continuous score and a `SapResult` +to price bills from (ADR-0011 forbids an in-memory hand-off between stages). + +Both stages hydrate the same aggregate through the same repo and call the same +calculator, so they normally agree — measured across portfolios 796 and 824, +mean divergence is **0.008 SAP** over 25,838 measure-free plans. But nothing +*enforces* that, and when they drift the failure is silent and user-visible: the +app subtracts one stage's baseline from the other's post score and reports a gain +nobody modelled. Property 749719 displayed **"+9.65 SAP"** on a plan whose own +CO2, bill and consumption savings were all exactly `0.0` — the plan was empty and +correct; the +9.65 was `post_sap` (72.65, modelling's baseline) minus +`effective_sap` (63, the Baseline stage's). Two baselines, subtracted. + +#1655 originally proposed **anchoring**: read the persisted Effective and use its +`effective_sap_score` as the Optimiser's baseline. We rejected that. + +Decided with Khalim, 2026-07-21, while working #1652 / #1655. + +## Decision + +**Modelling keeps its own re-score, and checks it against the persisted Effective +Performance. A divergence beyond rounding is logged as an error; it is never +silently reconciled.** + +- **`_check_baseline_coherence(property_id, modelled_baseline_sap, persisted)`** + runs once per Property, off the Plan's already-computed `baseline` Score — the + baseline picture is scenario-independent, and re-scoring a dwelling per + Property purely to check it would not pay for itself across a 31k batch. +- **Tolerance is 1.0 SAP.** The persisted figure is the *rounded* integer, so up + to half a point is rounding alone (ADR-0064); firing at 0.5 would be noise. +- **A missing baseline row warns and continues**, modelling against the + re-score. A data gap should degrade one Property, not abort a portfolio; + ADR-0012 reserves the abort for a load-bearing calculator raise. + +### Why not anchor + +- **It would break a different coherence to fix this one.** A `Score` carries + CO2 and primary energy beside SAP, and Bills are derived from the same + `SapResult`. Overwriting only the SAP with the persisted integer leaves a Plan + whose SAP disagrees with its own carbon, energy and bill figures. +- **It hides the defect instead of surfacing it.** A divergence means the two + stages scored different pictures — a real fault upstream. Anchoring makes the + numbers *agree* without making them *right*, and removes the evidence. +- **The figure being anchored to may itself be wrong.** #1656 documents SAP 10.2 + certs where the persisted Effective sits 6–10 points below the accredited + lodged rating. Hard-wiring Modelling to that number would propagate a known + error into every Plan's `post_*`. + +## Consequences + +- Plan `post_*` figures are unchanged. This adds a signal, not an arithmetic + change — no re-modelled figure moves because of this ADR. +- A future drift surfaces as an error naming the Property and both figures, + instead of as a phantom gain in a board pack. +- The guarantee is **detection, not prevention**: a divergent Property still + gets a Plan, and the app will still show the mismatched gain until the + underlying cause is fixed. Making it fatal is a live option if the error + proves rare enough in practice. +- Paired with a HIGH `costed-plan-without-real-gain` anomaly check, closing the + auditor's coverage hole: `plan-score-below-baseline` fired only on a *drop* + beyond 0.5 and `zero-works-post-differs` only on £0 plans, so a **costed** plan + landing flat against its baseline — the 112 "no real improvement" homes in 796 + — was caught by neither. +- The divergence this was built for was **not reproducible at the time of + writing**: the properties that showed it now agree with the persisted baseline, + most likely resolved by intervening calculator fixes. This is a guard against + recurrence, not a fix for live breakage. From 29157811e0a4b5811e1b1aea42a69b749bbe305f Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 16:38:49 +0000 Subject: [PATCH 18/20] =?UTF-8?q?Renumber=20the=20baseline-coherence=20ADR?= =?UTF-8?q?=20to=200066=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0065 is claimed by Scenario Export on the open #1546; this branch is the later of the two, so it moves. Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 2 +- ...delling-asserts-baseline-coherence-rather-than-anchoring.md} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename docs/adr/{0065-modelling-asserts-baseline-coherence-rather-than-anchoring.md => 0066-modelling-asserts-baseline-coherence-rather-than-anchoring.md} (100%) diff --git a/CONTEXT.md b/CONTEXT.md index c6b900858..259096bfd 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -156,7 +156,7 @@ The SAP / EPC Band / carbon emissions / Primary Energy Intensity recorded on the _Avoid_: original performance, raw EPC values, recorded baseline **Effective Performance**: -The SAP / EPC Band / carbon emissions / Primary Energy Intensity the modelling pipeline actually scored against — equal to Lodged Performance when no Rebaselining trigger fires, replaced by **SAP10 Calculation** output (the deterministic `Sap10Calculator`, which superseded the old ML-API rebaseliner; an ML residual head over the calculator is future — ADR-0009/0013) when triggered. The half of Baseline Performance that says "what we modelled". **Modelling does not read it**: it re-scores the Effective EPC itself (it needs a continuous score and a `SapResult` to bill from, and ADR-0011 forbids an in-memory hand-off between stages), then **asserts** its re-score agrees with this persisted figure and logs an error when it does not. The two are deliberately *not* reconciled — a divergence means the stages scored different pictures, and anchoring the Plan's SAP to this integer would leave it disagreeing with its own carbon, energy and bill figures (ADR-0065). Because a Plan's `post_*` are read against this figure, an unchecked divergence surfaces to the user as a gain nobody modelled. +The SAP / EPC Band / carbon emissions / Primary Energy Intensity the modelling pipeline actually scored against — equal to Lodged Performance when no Rebaselining trigger fires, replaced by **SAP10 Calculation** output (the deterministic `Sap10Calculator`, which superseded the old ML-API rebaseliner; an ML residual head over the calculator is future — ADR-0009/0013) when triggered. The half of Baseline Performance that says "what we modelled". **Modelling does not read it**: it re-scores the Effective EPC itself (it needs a continuous score and a `SapResult` to bill from, and ADR-0011 forbids an in-memory hand-off between stages), then **asserts** its re-score agrees with this persisted figure and logs an error when it does not. The two are deliberately *not* reconciled — a divergence means the stages scored different pictures, and anchoring the Plan's SAP to this integer would leave it disagreeing with its own carbon, energy and bill figures (ADR-0066). Because a Plan's `post_*` are read against this figure, an unchecked divergence surfaces to the user as a gain nobody modelled. _Avoid_: modelled performance, rebaselined performance (only correct when rebaselining ran), scored values **Calculated SAP10 Performance**: diff --git a/docs/adr/0065-modelling-asserts-baseline-coherence-rather-than-anchoring.md b/docs/adr/0066-modelling-asserts-baseline-coherence-rather-than-anchoring.md similarity index 100% rename from docs/adr/0065-modelling-asserts-baseline-coherence-rather-than-anchoring.md rename to docs/adr/0066-modelling-asserts-baseline-coherence-rather-than-anchoring.md From 223ef7c1960fa541add59392e3012c4bc28b88e1 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 16:46:25 +0000 Subject: [PATCH 19/20] =?UTF-8?q?Report=20missing=20baselines=20once=20per?= =?UTF-8?q?=20batch,=20and=20seed=20the=20harness's=20own=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The modelling_e2e handler models one property per run_modelling call, so a per-property "no persisted baseline" line would be one per property across a whole re-model. The harness has no Baseline stage by design and cannot drift from one, so it now establishes its own the way the Baseline orchestrator would; the Postgres-backed lane is where the assertion has something to catch. Co-Authored-By: Claude Opus 4.8 (1M context) --- harness/console.py | 24 ++++++++++ orchestration/modelling_orchestrator.py | 44 +++++++++++++------ .../test_modelling_baseline_coherence.py | 27 +++++++++--- 3 files changed, 74 insertions(+), 21 deletions(-) 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 From a80d6cad051fe0b35f49bae3d4dedf1d49ebc1b9 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 21 Jul 2026 16:57:22 +0000 Subject: [PATCH 20/20] =?UTF-8?q?Re-pin=20the=20trigger-attributes=20fixtu?= =?UTF-8?q?re=20to=20one=20that=20still=20buys=20lighting=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0390's floor+ASHP package reaches post 68.772, which publishes as 69 = band C, so under ADR-0064 it now meets the goal and stops before low-energy lighting — the same over-purchase the ADR removes, one level up at the package. 0380 fires the same three measures with identical trigger attributes, so every assertion stands unchanged, and it lands at post 74.4, clear of a band floor. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/harness/test_report.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/harness/test_report.py b/tests/harness/test_report.py index 84330584e..a89c79943 100644 --- a/tests/harness/test_report.py +++ b/tests/harness/test_report.py @@ -30,13 +30,20 @@ _GOLDEN = ( _WITHIN_TOLERANCE = "0036-6325-1100-0063-1226" _DIVERGENT = "0240-0200-5706-2365-8010" -# 0390 fires three measures — an uninsulated solid floor, low-energy lighting, +# 0380 fires three measures — an uninsulated solid floor, low-energy lighting, # and the ASHP bundle — so every fired measure's trigger attributes are -# exercised together. (0330, the previous fixture, now reaches band C on the -# correctly-sized ASHP alone: ADR-0049 sizes the pump to the dwelling's design -# heat loss, so the undersized-pump-era companion solid_floor_insulation is no -# longer needed there.) -_THREE_MEASURES = "0390-2254-6420-2126-5561" +# exercised together. +# +# Fixture history, both times because the *package* changed rather than the +# trigger logic under test: +# 0330 -> 0390: ADR-0049 sizes the pump to the dwelling's design heat loss, so +# the undersized-pump-era companion solid_floor_insulation stopped firing. +# 0390 -> 0380: ADR-0064 judges the goal band on the *published* rating, so +# 0390's floor+ASHP package (post 68.772, published 69 = band C) now meets +# the target and no longer buys low-energy lighting to clear an integer 69. +# 0380 reaches band C at post 74.4 — clear of a band floor, so it does not ride +# on the rounding rule it is standing in for. +_THREE_MEASURES = "0380-2530-6150-2326-4161" def _triggers_by_measure(report: PropertyReport) -> dict[str, MeasureTrigger]: