Merge pull request #1657 from Hestia-Homes/fix/optimiser-band-floor-rounding

Judge band membership on the published rating; assert modelling/baseline coherence
This commit is contained in:
KhalimCK 2026-07-21 18:03:25 +01:00 committed by GitHub
commit 1dd9a9a472
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 756 additions and 32 deletions

View file

@ -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 AG representing a property's current or potential energy efficiency rating.
_Avoid_: energy rating, EPC grade, EPC score
A single letter AG 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**:

View file

@ -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

View file

@ -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.

View file

@ -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 610 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.

View file

@ -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:

View file

@ -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(

View file

@ -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}

View file

@ -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:

View file

@ -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:

View file

@ -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(

View file

@ -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

View file

@ -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:

View file

@ -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]:

View file

@ -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):

View file

@ -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

View file

@ -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

View file

@ -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