diff --git a/CONTEXT.md b/CONTEXT.md index 047087e97..259096bfd 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-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**: @@ -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/datatypes/epc/domain/epc.py b/datatypes/epc/domain/epc.py index ae4fd8241..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 @@ -32,6 +33,17 @@ class Epc(Enum): return cls.F return cls.G + @classmethod + def from_sap_continuous(cls, score: float) -> "Epc": + """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_sap_score` (A → 92, B → 81, C → 69, D → 55, E → 39, F → 21, @@ -46,3 +58,15 @@ class Epc(Enum): Epc.G: 1, } return bounds[self] + + def sap_lower_bound_continuous(self) -> float: + """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 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/0066-modelling-asserts-baseline-coherence-rather-than-anchoring.md b/docs/adr/0066-modelling-asserts-baseline-coherence-rather-than-anchoring.md new file mode 100644 index 000000000..21f3f25a6 --- /dev/null +++ b/docs/adr/0066-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. 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/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( 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/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/orchestration/modelling_orchestrator.py b/orchestration/modelling_orchestrator.py index 69e88feac..d4248f3f1 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,14 @@ 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__) + +# 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 @@ -111,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) @@ -124,6 +141,7 @@ class ModellingOrchestrator: uow.solar, prop.identity.uprn ) has_recommendations = False + checked_baseline = False for scenario in scenarios: plan = self._plan_for( scorer, @@ -136,6 +154,18 @@ 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: + 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, property_id=property_id, @@ -150,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( @@ -489,12 +527,69 @@ def _objective_for( return build_objective(bill_derivation) +def _check_baseline_coherence( + property_id: int, + modelled_baseline_sap: float, + persisted: Optional[PropertyBaselinePerformance], +) -> 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). + + 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, 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: + return True + effective_sap: int = persisted.effective.sap_score + if abs(modelled_baseline_sap - effective_sap) <= _MAX_BASELINE_DIVERGENCE: + return False + 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, + ) + return False + + 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/scripts/audit/anomalies.py b/scripts/audit/anomalies.py index 0ee0386d4..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: @@ -52,7 +62,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]: @@ -163,10 +173,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 @@ -175,6 +197,38 @@ 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]: + """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) 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 @@ -538,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( diff --git a/tests/datatypes/epc/domain/test_epc.py b/tests/datatypes/epc/domain/test_epc.py index 474c5e890..8731c5af3 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,41 @@ 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 + + +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, +) -> 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 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: 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]: 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 new file mode 100644 index 000000000..752b1c14f --- /dev/null +++ b/tests/orchestration/test_modelling_baseline_coherence.py @@ -0,0 +1,111 @@ +"""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", + space_heating_kwh=0.0, + water_heating_kwh=0.0, + ) + + +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_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) — 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): + missing = _check_baseline_coherence(749719, 63.22, None) + + # 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 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..63210345b --- /dev/null +++ b/tests/orchestration/test_modelling_goal_band_floor.py @@ -0,0 +1,57 @@ +"""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 + + +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 diff --git a/tests/scripts/test_audit_anomalies.py b/tests/scripts/test_audit_anomalies.py index dd8771500..e34bf250d 100644 --- a/tests/scripts/test_audit_anomalies.py +++ b/tests/scripts/test_audit_anomalies.py @@ -1,12 +1,19 @@ +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, # pyright: ignore[reportPrivateUsage] PropertyAudit, - _effective_lodged_divergence, - _implausible_lodged_score, - _plan_stops_short_of_goal, + 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] ) @@ -155,3 +162,122 @@ 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 + + +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