From 7a970ea393347a4ec62aba21772147d8b6157ae6 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 24 Jun 2026 08:34:30 +0000 Subject: [PATCH 1/4] fix(repositories): fold landlord overrides when re-hydrating a Property via the UoW MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PostgresUnitOfWork built its PropertyPostgresRepository without an overrides reader, so a Property re-hydrated through the unit silently dropped its Landlord Overrides (ADR-0032). The Baseline orchestrator runs through the UoW, so it scored the bare lodged EPC while the Plan modelled the override-folded Effective EPC — the two diverged (e.g. baseline effective 71/C vs plan baseline 62/D), producing "already at band C yet recommends reaching C". Wire PropertyOverridesPostgresReader into the unit's property repo (uow- independent committed reference data, read via the same session factory) so every re-hydration folds overrides, matching the live modelling path. Co-Authored-By: Claude Opus 4.8 (1M context) --- repositories/postgres_unit_of_work.py | 11 ++++- tests/repositories/test_unit_of_work.py | 66 +++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/repositories/postgres_unit_of_work.py b/repositories/postgres_unit_of_work.py index a828d8c6..8a66146c 100644 --- a/repositories/postgres_unit_of_work.py +++ b/repositories/postgres_unit_of_work.py @@ -14,6 +14,9 @@ from repositories.plan.plan_postgres_repository import PlanPostgresRepository from repositories.product.composite_product_repository import ( catalogue_with_off_catalogue_overrides, ) +from repositories.property.property_overrides_postgres_reader import ( + PropertyOverridesPostgresReader, +) from repositories.property.property_postgres_repository import ( PropertyPostgresRepository, ) @@ -41,8 +44,14 @@ class PostgresUnitOfWork(UnitOfWork): self._session = self._session_factory() epc_repo = EpcPostgresRepository(self._session) spatial_repo = SpatialPostgresRepository(self._session) + # Fold Landlord Overrides onto the Effective EPC on every re-hydration + # (ADR-0032), so what the Baseline orchestrator scores off ``uow.property`` + # matches what the Plan was modelled from. The reader is uow-independent — + # ``property_overrides`` is committed reference data — so it opens its own + # short read session per call via the same session factory. + overrides_reader = PropertyOverridesPostgresReader(self._session_factory) self.property = PropertyPostgresRepository( - self._session, epc_repo, spatial_repo + self._session, epc_repo, spatial_repo, overrides_reader ) self.epc = epc_repo self.solar = SolarPostgresRepository(self._session) diff --git a/tests/repositories/test_unit_of_work.py b/tests/repositories/test_unit_of_work.py index 5008d4c6..3cc13d6c 100644 --- a/tests/repositories/test_unit_of_work.py +++ b/tests/repositories/test_unit_of_work.py @@ -1,25 +1,45 @@ from __future__ import annotations +import json from collections.abc import Callable +from pathlib import Path +from typing import Any import pytest from sqlalchemy import Engine from sqlmodel import Session from datatypes.epc.domain.epc import Epc +from datatypes.epc.domain.epc_property_data import ( + BuildingPartIdentifier, + EpcPropertyData, +) +from datatypes.epc.domain.mapper import EpcPropertyDataMapper from domain.property_baseline.property_baseline_performance import PropertyBaselinePerformance from domain.property_baseline.performance import Performance +from infrastructure.postgres.property_override_table import PropertyOverrideRow +from infrastructure.postgres.property_table import PropertyRow +from repositories.epc.epc_postgres_repository import EpcPostgresRepository from repositories.plan.plan_repository import PlanRepository from repositories.postgres_unit_of_work import PostgresUnitOfWork from repositories.product.product_repository import ProductRepository from repositories.scenario.scenario_repository import ScenarioRepository from repositories.spatial.spatial_repository import SpatialRepository +_JSON_SAMPLES = Path(__file__).resolve().parents[2] / "backend/epc_api/json_samples" + def _session_factory(db_engine: Engine) -> Callable[[], Session]: return lambda: Session(db_engine) +def _epc() -> EpcPropertyData: + raw: dict[str, Any] = json.loads( + (_JSON_SAMPLES / "RdSAP-Schema-21.0.0" / "epc.json").read_text() + ) + return EpcPropertyDataMapper.from_api_response(raw) + + def _baseline() -> PropertyBaselinePerformance: perf = Performance( sap_score=72, epc_band=Epc.C, co2_emissions=1.8, primary_energy_intensity=180 @@ -86,6 +106,52 @@ def test_unit_exposes_the_spatial_cache_repo_bound_to_its_session( assert isinstance(uow.spatial, SpatialRepository) +def test_unit_hydrates_a_property_with_its_landlord_overrides_folded( + db_engine: Engine, +) -> None: + """A Property re-hydrated through the unit folds its Landlord Overrides onto + the Effective EPC (ADR-0032) — the same overlay the live modelling path + applies. Without this the Baseline Performance the orchestrator scores off + ``uow.property`` diverges from the Plan, which does apply the overrides. + """ + # Arrange — a lodged EPC (cavity main wall, 4) plus a solid-brick / internal + # wall override, persisted through the unit. + with PostgresUnitOfWork(_session_factory(db_engine)) as uow: + row = PropertyRow(portfolio_id=1, postcode="A0 0AA", address="1 St", uprn=1) + uow._session.add(row) # pyright: ignore[reportPrivateUsage] + uow._session.flush() # pyright: ignore[reportPrivateUsage] + property_id = row.id + assert property_id is not None + EpcPostgresRepository(uow._session).save( # pyright: ignore[reportPrivateUsage] + _epc(), property_id=property_id + ) + uow._session.add( # pyright: ignore[reportPrivateUsage] + PropertyOverrideRow( + property_id=property_id, + portfolio_id=1, + building_part=0, + override_component="wall_type", + override_value="Solid brick, with internal insulation", + original_spreadsheet_description="solid brick, insulated", + ) + ) + uow.commit() + + # Act — re-hydrate through a fresh unit. + with PostgresUnitOfWork(_session_factory(db_engine)) as uow: + prop = uow.property.get(property_id) + + main = next( + part + for part in prop.effective_epc.sap_building_parts + if part.identifier is BuildingPartIdentifier.MAIN + ) + + # Assert — the override is folded: cavity (4) → solid brick (3) / internal (3). + assert main.wall_construction == 3 + assert main.wall_insulation_type == 3 + + def test_leaving_the_block_without_commit_persists_nothing(db_engine: Engine) -> None: # Arrange new_unit = lambda: PostgresUnitOfWork(_session_factory(db_engine)) From 0c70280dea0bd2d2dec0a21fe6ed85605a885bd9 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 24 Jun 2026 09:07:24 +0000 Subject: [PATCH 2/4] guard(modelling_e2e): quarantine predicted Properties the calculator mis-scores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TEMPORARY guard (remove once the SAP calculator's oil-heating under-score is fixed): a predicted oil-boiler picture scores SAP 13/G against its own synthesised recorded SAP of 50/E, so the optimiser overshoots goal C all the way to band A and publishes nonsense. A predicted EpcPropertyData carries its recorded SAP (energy_rating_current). When the calculator baseline diverges from it by more than ~one band (20 SAP points), withhold the Plan: raise inside the per-property loop so the existing failure isolation drops just that property into `failures` and fails the subtask, while every other property still models and persists. Lodged Properties are untouched — they have a real recorded cert and the Rebaseliner already owns this check. Verified end-to-end against property 713406 (UPRN 100061849247): baseline 13.2 vs recorded 50 -> quarantined, no Plan written. Co-Authored-By: Claude Opus 4.8 (1M context) --- applications/modelling_e2e/handler.py | 45 +++++++++++++++++++ .../modelling_e2e/test_handler.py | 29 ++++++++++++ 2 files changed, 74 insertions(+) diff --git a/applications/modelling_e2e/handler.py b/applications/modelling_e2e/handler.py index c5aa189b..f9c55696 100644 --- a/applications/modelling_e2e/handler.py +++ b/applications/modelling_e2e/handler.py @@ -212,6 +212,35 @@ def _predict_epc( return predicted +# --- TEMPORARY GUARD: remove once the SAP calculator's oil-heating under-score +# is fixed (predicted oil-boiler picture scores SAP 13/G vs a recorded 50/E). --- +# A predicted EpcPropertyData carries its own recorded SAP (energy_rating_current, +# synthesised from the cohort). When the calculator's baseline score contradicts +# that by more than ~one EPC band the picture is being mis-scored, so any Plan +# built on it overshoots (e.g. goal C lands at band A). Quarantine the property — +# skip its Plan — rather than ship nonsense. Lodged properties are unaffected: +# they have a real recorded cert and the Rebaseliner already owns this check. +_PREDICTED_BASELINE_DIVERGENCE_GUARD = 20.0 # SAP points (~one EPC band) + + +class ImplausiblePredictedBaseline(Exception): + """A predicted Property's calculator baseline contradicts its recorded SAP by + more than a band — the calculator is mis-scoring the synthesised picture, so + the Plan is untrustworthy and is withheld (caught per-property as a failure).""" + + +def _predicted_baseline_is_implausible( + baseline_sap: float, recorded_sap: Optional[int] +) -> bool: + """True when a predicted Property's calculator baseline diverges from the + picture's own recorded SAP by more than the guard band. A missing recorded + SAP (no reference) is never implausible — the guard only fires on a concrete + contradiction.""" + if recorded_sap is None: + return False + return abs(baseline_sap - recorded_sap) > _PREDICTED_BASELINE_DIVERGENCE_GUARD + + @task_handler(task_source="modelling_e2e", source=Source.PROPERTY) def handler(body: dict[str, Any], context: Any) -> Optional[dict[str, Any]]: trigger = ModellingE2ETriggerBody.model_validate(body) @@ -389,6 +418,22 @@ def handler(body: dict[str, Any], context: Any) -> Optional[dict[str, Any]]: f"measures={len(plan.measures)}" ) + # Quarantine a predicted Property whose calculator baseline + # contradicts its synthesised recorded SAP (TEMPORARY guard — + # see _predicted_baseline_is_implausible). Raising drops this one + # property into `failures` and skips its Plan/Baseline; the rest + # of the batch is unaffected. + if predicted_epc is not None and _predicted_baseline_is_implausible( + plan.baseline.sap_continuous, effective_epc.energy_rating_current + ): + raise ImplausiblePredictedBaseline( + f"property={property_id}: predicted baseline SAP " + f"{plan.baseline.sap_continuous:.1f} diverges from the " + f"picture's recorded SAP {effective_epc.energy_rating_current} " + f"by > {_PREDICTED_BASELINE_DIVERGENCE_GUARD:.0f} points — " + f"likely a calculator mis-score; withholding the plan" + ) + if dry_run: measure_types = ( ", ".join(m.measure_type for m in plan.measures) or "none" diff --git a/tests/applications/modelling_e2e/test_handler.py b/tests/applications/modelling_e2e/test_handler.py index baa20103..18939029 100644 --- a/tests/applications/modelling_e2e/test_handler.py +++ b/tests/applications/modelling_e2e/test_handler.py @@ -68,6 +68,9 @@ def _plan_mock() -> MagicMock: plan = MagicMock() plan.measures = [] plan.cost_of_works = 0.0 + # A plausible baseline so the predicted-baseline guard stays silent (it + # compares this against the picture's recorded SAP). + plan.baseline.sap_continuous = 50.0 return plan @@ -330,6 +333,7 @@ def test_prediction_path_saves_predicted_epc_plan_and_baseline( mock_part = MagicMock() mock_part.identifier = BuildingPartIdentifier.MAIN mock_predicted_epc.sap_building_parts = [mock_part] + mock_predicted_epc.energy_rating_current = 50 # matches plan baseline -> guard silent mock_comparables = MagicMock() mock_comparables.members = [MagicMock()] # non-empty cohort @@ -534,6 +538,7 @@ def test_empty_own_postcode_broadens_to_nearby_and_predicts() -> None: mock_part = MagicMock() mock_part.identifier = BuildingPartIdentifier.MAIN mock_predicted_epc.sap_building_parts = [mock_part] + mock_predicted_epc.energy_rating_current = 50 # matches plan baseline -> guard silent # First select_comparables (own postcode) is empty → broaden; the second # (nearby cohort) finds comparables. @@ -757,6 +762,7 @@ def test_cohort_cache_prevents_duplicate_candidates_for_calls() -> None: mock_part = MagicMock() mock_part.identifier = BuildingPartIdentifier.MAIN mock_predicted_epc.sap_building_parts = [mock_part] + mock_predicted_epc.energy_rating_current = 50 # matches plan baseline -> guard silent mock_comparables = MagicMock() mock_comparables.members = [MagicMock()] @@ -919,3 +925,26 @@ def test_dry_run_skips_all_db_writes() -> None: # Assert — UoW never entered MockUoW.return_value.__enter__.assert_not_called() + + +def test_predicted_baseline_within_band_is_plausible() -> None: + # A predicted picture whose calculator baseline tracks its recorded SAP + # (here 50 vs 47) is trusted — the guard does not fire. + from applications.modelling_e2e.handler import _predicted_baseline_is_implausible + + assert _predicted_baseline_is_implausible(47.0, 50) is False + + +def test_predicted_baseline_beyond_band_is_implausible() -> None: + # The 713406 case: calculator scores the oil-boiler picture at 13 while the + # synthesised cert records 50 — a >20-point contradiction the guard rejects. + from applications.modelling_e2e.handler import _predicted_baseline_is_implausible + + assert _predicted_baseline_is_implausible(13.2, 50) is True + + +def test_predicted_baseline_without_a_recorded_sap_is_not_judged() -> None: + # No recorded SAP means no reference to contradict, so the guard stays silent. + from applications.modelling_e2e.handler import _predicted_baseline_is_implausible + + assert _predicted_baseline_is_implausible(13.2, None) is False From 80985865a2ec5eae3e93c8d28e5c5653a7d80d07 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 24 Jun 2026 09:33:54 +0000 Subject: [PATCH 3/4] Revert "guard(modelling_e2e): quarantine predicted Properties the calculator mis-scores" This reverts commit 0c70280dea0bd2d2dec0a21fe6ed85605a885bd9. --- applications/modelling_e2e/handler.py | 45 ------------------- .../modelling_e2e/test_handler.py | 29 ------------ 2 files changed, 74 deletions(-) diff --git a/applications/modelling_e2e/handler.py b/applications/modelling_e2e/handler.py index f9c55696..c5aa189b 100644 --- a/applications/modelling_e2e/handler.py +++ b/applications/modelling_e2e/handler.py @@ -212,35 +212,6 @@ def _predict_epc( return predicted -# --- TEMPORARY GUARD: remove once the SAP calculator's oil-heating under-score -# is fixed (predicted oil-boiler picture scores SAP 13/G vs a recorded 50/E). --- -# A predicted EpcPropertyData carries its own recorded SAP (energy_rating_current, -# synthesised from the cohort). When the calculator's baseline score contradicts -# that by more than ~one EPC band the picture is being mis-scored, so any Plan -# built on it overshoots (e.g. goal C lands at band A). Quarantine the property — -# skip its Plan — rather than ship nonsense. Lodged properties are unaffected: -# they have a real recorded cert and the Rebaseliner already owns this check. -_PREDICTED_BASELINE_DIVERGENCE_GUARD = 20.0 # SAP points (~one EPC band) - - -class ImplausiblePredictedBaseline(Exception): - """A predicted Property's calculator baseline contradicts its recorded SAP by - more than a band — the calculator is mis-scoring the synthesised picture, so - the Plan is untrustworthy and is withheld (caught per-property as a failure).""" - - -def _predicted_baseline_is_implausible( - baseline_sap: float, recorded_sap: Optional[int] -) -> bool: - """True when a predicted Property's calculator baseline diverges from the - picture's own recorded SAP by more than the guard band. A missing recorded - SAP (no reference) is never implausible — the guard only fires on a concrete - contradiction.""" - if recorded_sap is None: - return False - return abs(baseline_sap - recorded_sap) > _PREDICTED_BASELINE_DIVERGENCE_GUARD - - @task_handler(task_source="modelling_e2e", source=Source.PROPERTY) def handler(body: dict[str, Any], context: Any) -> Optional[dict[str, Any]]: trigger = ModellingE2ETriggerBody.model_validate(body) @@ -418,22 +389,6 @@ def handler(body: dict[str, Any], context: Any) -> Optional[dict[str, Any]]: f"measures={len(plan.measures)}" ) - # Quarantine a predicted Property whose calculator baseline - # contradicts its synthesised recorded SAP (TEMPORARY guard — - # see _predicted_baseline_is_implausible). Raising drops this one - # property into `failures` and skips its Plan/Baseline; the rest - # of the batch is unaffected. - if predicted_epc is not None and _predicted_baseline_is_implausible( - plan.baseline.sap_continuous, effective_epc.energy_rating_current - ): - raise ImplausiblePredictedBaseline( - f"property={property_id}: predicted baseline SAP " - f"{plan.baseline.sap_continuous:.1f} diverges from the " - f"picture's recorded SAP {effective_epc.energy_rating_current} " - f"by > {_PREDICTED_BASELINE_DIVERGENCE_GUARD:.0f} points — " - f"likely a calculator mis-score; withholding the plan" - ) - if dry_run: measure_types = ( ", ".join(m.measure_type for m in plan.measures) or "none" diff --git a/tests/applications/modelling_e2e/test_handler.py b/tests/applications/modelling_e2e/test_handler.py index 18939029..baa20103 100644 --- a/tests/applications/modelling_e2e/test_handler.py +++ b/tests/applications/modelling_e2e/test_handler.py @@ -68,9 +68,6 @@ def _plan_mock() -> MagicMock: plan = MagicMock() plan.measures = [] plan.cost_of_works = 0.0 - # A plausible baseline so the predicted-baseline guard stays silent (it - # compares this against the picture's recorded SAP). - plan.baseline.sap_continuous = 50.0 return plan @@ -333,7 +330,6 @@ def test_prediction_path_saves_predicted_epc_plan_and_baseline( mock_part = MagicMock() mock_part.identifier = BuildingPartIdentifier.MAIN mock_predicted_epc.sap_building_parts = [mock_part] - mock_predicted_epc.energy_rating_current = 50 # matches plan baseline -> guard silent mock_comparables = MagicMock() mock_comparables.members = [MagicMock()] # non-empty cohort @@ -538,7 +534,6 @@ def test_empty_own_postcode_broadens_to_nearby_and_predicts() -> None: mock_part = MagicMock() mock_part.identifier = BuildingPartIdentifier.MAIN mock_predicted_epc.sap_building_parts = [mock_part] - mock_predicted_epc.energy_rating_current = 50 # matches plan baseline -> guard silent # First select_comparables (own postcode) is empty → broaden; the second # (nearby cohort) finds comparables. @@ -762,7 +757,6 @@ def test_cohort_cache_prevents_duplicate_candidates_for_calls() -> None: mock_part = MagicMock() mock_part.identifier = BuildingPartIdentifier.MAIN mock_predicted_epc.sap_building_parts = [mock_part] - mock_predicted_epc.energy_rating_current = 50 # matches plan baseline -> guard silent mock_comparables = MagicMock() mock_comparables.members = [MagicMock()] @@ -925,26 +919,3 @@ def test_dry_run_skips_all_db_writes() -> None: # Assert — UoW never entered MockUoW.return_value.__enter__.assert_not_called() - - -def test_predicted_baseline_within_band_is_plausible() -> None: - # A predicted picture whose calculator baseline tracks its recorded SAP - # (here 50 vs 47) is trusted — the guard does not fire. - from applications.modelling_e2e.handler import _predicted_baseline_is_implausible - - assert _predicted_baseline_is_implausible(47.0, 50) is False - - -def test_predicted_baseline_beyond_band_is_implausible() -> None: - # The 713406 case: calculator scores the oil-boiler picture at 13 while the - # synthesised cert records 50 — a >20-point contradiction the guard rejects. - from applications.modelling_e2e.handler import _predicted_baseline_is_implausible - - assert _predicted_baseline_is_implausible(13.2, 50) is True - - -def test_predicted_baseline_without_a_recorded_sap_is_not_judged() -> None: - # No recorded SAP means no reference to contradict, so the guard stays silent. - from applications.modelling_e2e.handler import _predicted_baseline_is_implausible - - assert _predicted_baseline_is_implausible(13.2, None) is False From db3bf00602e32a0dd7b8fbe196d81b16073c04c9 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 24 Jun 2026 09:47:14 +0000 Subject: [PATCH 4/4] =?UTF-8?q?fix(synthesis):=20Coherent=20Heating=20Syst?= =?UTF-8?q?em=20=E2=80=94=20synthesisers=20own=20the=20whole=20boundary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dwelling's heating is one conceptual system, but its fields are scattered across EpcPropertyData (a gov-API schema mirror): the cluster on sap_heating, the electricity tariff on sap_energy_source.meter_type, hot-water flags loose at top level. Three places synthesise a heating system — Measure Options, Landlord Overrides, EPC Prediction's donor — and each hand-copied a different ad-hoc subset. The override and donor both dropped meter_type, so an electric-storage system landed on the template's single-rate meter and billed overnight heat at the peak rate: property 713406 scored SAP 13 (G) vs ~50 (E), inflating the HHRSH measure to +45.8 and overshooting the plan to band A. Establish a single Coherent Heating System boundary (CONTEXT.md) that every synthesiser must cover, with a source-appropriate fill policy (ADR-0035): - Override overlay *completes* the partial system the landlord named. Companion fields are now DERIVED from the SAP code, not hand-attached per archetype: the off-peak meter from the calculator's single off-peak classification (new OFF_PEAK_IMPLYING_HEATING_CODES = SAP §12 Rules 1-2), and an unobserved storage charge control defaults to the conservative manual control (Table 4e 2401). So adding a heating archetype is just adding its code — companions can't be forgotten. A contract test guards it (every off-peak code drags a Dual meter). - Prediction's heating donor now *carries* the donor's meter_type alongside its sap_heating cluster — the donor is already coherent. Coherence is a synthesis-time obligation only; the calculator still scores a real lodged cert exactly as lodged. Verified on 713406: baseline 13 -> 47.8 (E), matching its recorded rating; the phantom HHRSH recommendation is gone and the plan no longer overshoots to A. Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 4 + .../0035-coherent-heating-system-synthesis.md | 64 ++++++++++++++ .../main_heating_system_overlay.py | 58 +++++++++++-- domain/epc_prediction/epc_prediction.py | 9 +- domain/sap10_calculator/tables/table_12a.py | 14 +++ .../epc/test_main_heating_system_overlay.py | 86 +++++++++++++++++++ .../epc_prediction/test_epc_prediction.py | 16 ++++ .../domain/epc_prediction/test_validation.py | 1 + 8 files changed, 246 insertions(+), 6 deletions(-) create mode 100644 docs/adr/0035-coherent-heating-system-synthesis.md diff --git a/CONTEXT.md b/CONTEXT.md index ff71c2b2..04116f87 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -292,6 +292,10 @@ _Avoid_: "windows" as a Measure (name **double glazing** / **secondary glazing** The rule fixing the single lighting Measure the **Lighting** Recommendation offers. We convert **all non-LED bulbs** (incandescent + CFL + low-energy-unknown) to **LED** — all the way to LED, not the legacy "fill to low energy", because SAP rates LED efficacy above CFL (ADR-0023). One Measure, no planning gate (lighting isn't planning-restricted). Offered only when the dwelling lodges at least one non-LED bulb; a dwelling already all-LED, or one that lodged **no** bulb counts (nothing to size against), gets no Recommendation. Unlike the fabric measures it is a **whole-dwelling** Measure — its **Simulation Overlay** writes the four top-level bulb counts directly (`led = total`, others 0), the first overlay surface that isn't a building part / window / system sub-object. Priced at a flat **average price per bulb** × the count of non-LED bulbs replaced. A free Optimiser candidate (it *improves* SAP), contrast the forced ventilation **Measure Dependency**. _Avoid_: "low energy lighting" as the upgrade target (we go to **LED**); treating it as a forced dependency (it is a free candidate); pricing by floor area (it's per-bulb count × average) +**Coherent Heating System**: +The set of fields that together describe a dwelling's heating and must move **as a unit** whenever a heating system is *synthesised* — never a subset. It spans the main heating cluster (fuel, system, emitter, **controls**), the **hot water** it implies, the **off-peak meter** an electric-storage or CPSU system runs on, and the hot-water cylinder / solar flags. The fields are physically scattered across the `EpcPropertyData` (a gov-API schema mirror — the cluster, a loose meter on the energy source, loose top-level flags), but the *concept* is one system. Three places synthesise a heating system and each must cover the whole set with its own **fill policy**: a **Measure Option** sets a designed end-state (HHRSH / ASHP), a **Landlord Override** *completes* the partial system the landlord named with conservative assumptions for the fields they couldn't tell us (the off-peak meter is *derived* from the SAP code; an unobserved storage charge control defaults to the conservative manual one), and **EPC Prediction**'s heating donor *carries* a real neighbour's whole coherent set. Setting only a subset mis-scores: an electric-storage system left on a single-rate meter bills its overnight heat at the peak rate and collapses the SAP. **Coherence is owned by the synthesiser, never by the calculator** — a real lodged cert is scored exactly as lodged (ADR-0035). +_Avoid_: heating overlay (that is one *mechanism*, not the field set); meter as a separate concern (it is part of the system); normalising a lodged EPC (coherence is a synthesis-time obligation only) + **Heating Eligibility**: The rule fixing which **Measure Options** the single **Heating & Hot Water** Recommendation offers (ADR-0024, expanded). The competing Options are **mutually-exclusive** (the Optimiser picks at most one) and fall in two families: **whole-system replacements** — `high_heat_retention_storage_heaters`, `air_source_heat_pump` — which change main heating + **controls + fuel + meter + the implied hot water** at once (never a separate HW measure; the legacy heating-vs-HW split double-counted); and, for a dwelling keeping a serviceable wet boiler, **partial upgrades** — `gas_boiler_upgrade` (a like-for-like condensing **gas** boiler: gas→gas, or non-gas→gas only where mains gas is present; combi or regular-plus-cylinder, shaped by the dwelling) and the **system tune-up** (keep the boiler; install better **controls** + fix the **cylinder**), the tune-up offered at two competing control levels: `system_tune_up` (standard, SAP code 2106) and `system_tune_up_zoned` (time-and-temperature zone control, 2110 — more SAP uplift, more cost). Each Option is a **fixed, real, contractor-installable end-state** (ASHP via a fixed PCDB heat-pump index; HHR storage via `sap_main_heating_code=409`; the gas boiler via Table 4b code 102/104; controls via 2106/2110), not a derived ideal; **Product** stays cost-only, but a partial/bundle cost is **composed per dwelling** from the components the overlay installs (ADR-0025/0027), not a flat scalar. Eligibility encodes **only physical/planning installability** — the **Optimiser owns the economics**, so it must not re-gate on cost proxies: **ASHP** → houses/bungalows that are not **listed**/**heritage** and not already a heat pump (flats excluded — individual siting needs a survey; a **conservation area** still gets the offer, unlike glazing); **HHR storage** → off-gas or currently-electric dwellings, not community-heated or already HHR; **boiler upgrade / tune-up** → an existing (non-electric) wet boiler, the gas end-state gated on a mains-gas connection, a partial control upgrade offered only when it genuinely improves the existing control (never a downgrade or no-op). Floor area, fabric, fuel, and built form are **not** gates (the legacy ASHP built-form / 120 m² rule is dropped — no authoritative basis). A free Optimiser candidate, not a forced **Measure Dependency**. _Avoid_: separate "heating" and "hot water" recommendations (HW folds into each Option); gating ASHP on floor area / built form / fabric (eligibility is physical/planning only — the Optimiser decides cost-effectiveness); treating the whole-system replacements and the partial boiler/tune-up upgrades as **separate** Recommendations (they are mutually-exclusive Options within the one heating rec — separate recs would let the Optimiser co-select and double-charge); a standalone hot-water-only or controls-only Recommendation (controls + cylinder fold into the boiler/tune-up Option) diff --git a/docs/adr/0035-coherent-heating-system-synthesis.md b/docs/adr/0035-coherent-heating-system-synthesis.md new file mode 100644 index 00000000..030a6001 --- /dev/null +++ b/docs/adr/0035-coherent-heating-system-synthesis.md @@ -0,0 +1,64 @@ +# Coherent Heating System is owned by the synthesiser, not the calculator + +## Status + +accepted + +## Context + +A dwelling's heating is one conceptual **system**, but its fields are physically +scattered across `EpcPropertyData` because that type mirrors the gov-API / RdSAP +schema: the main cluster lives on `sap_heating`, the electricity tariff on +`sap_energy_source.meter_type`, and hot-water flags (`has_hot_water_cylinder`, +`solar_water_heating`) on loose top-level fields. + +Three places **synthesise** a heating system — a Measure Option (HHRSH/ASHP), a +Landlord Override, and EPC Prediction's heating donor — and each was hand-copying +a *different ad-hoc subset* of those fields. The override and the donor both +omitted `meter_type`. Property 713406 (a predicted property whose landlord +override set "Electric storage heaters") landed on the oil structural template's +single-rate meter, so its overnight storage heat billed at the peak electricity +rate: the calculator scored it **SAP 13 (G)** instead of **~50 (E)**, which then +inflated the HHRSH measure to **+45.8 SAP** and pushed the plan to band A. + +## Decision + +Treat **"a heating system"** as one named boundary (a **Coherent Heating +System** — see CONTEXT.md). Whoever *synthesises* a heating system owns covering +the whole boundary, with a fill policy suited to its source: + +- **Measure** → set a designed end-state (already did). +- **Landlord Override** → *complete* the partial system the landlord named. + Companion fields are **derived from the SAP code**, not hand-attached per + archetype: the off-peak meter from the calculator's single off-peak + classification (`OFF_PEAK_IMPLYING_HEATING_CODES`, SAP §12 Rules 1-2), and — + where a field has no logical pairing — a **conservative** default (storage + charge control → manual, SAP Table 4e 2401, the lowest-SAP storage control). + So adding a heating archetype is just adding its code; coherent companions + fall out and cannot be forgotten. +- **EPC Prediction donor** → *carry* a real neighbour's whole coherent set + (including its `meter_type`), since the donor is already internally coherent. + +**Coherence is a synthesis-time obligation, never a calculator normalisation.** +A real lodged cert is scored exactly as lodged — the calculator must not "fix" a +genuinely single-rate storage dwelling (its existing Unknown-meter inference is +a separate, spec-faithful net for certs that lodged *Unknown*, and does not fire +on an explicit meter). A contract test guards the override path: every off-peak +code the archetype map can emit must drag a `Dual` meter. + +## Considered Options + +- **Reify a `HeatingSystem` value object** all three produce/consume — rejected: + overlaps `HeatingOverlay` and is a large, risky restructure for the value. +- **A global coherence pass over the assembled effective EPC** — rejected: it + would corrupt real lodged certs (force storage→dual on a genuine single-rate + lodgement). Coherence must be scoped to synthesis points. +- **Set `meter_type="Unknown"` and lean on the calculator's storage inference** + — rejected as the primary mechanism: an explicit `Dual` is self-contained, + matches the measure overlay, carries display value, and does not depend on a + repo *extension* that could later be tightened toward spec. + +This amends [ADR-0029](0029-epc-prediction-from-comparable-properties.md) (the +heating donor now carries the meter) and extends +[ADR-0032](0032-landlord-override-epc-overlay.md) (the override overlay derives +coherent companions). diff --git a/domain/epc/property_overlays/main_heating_system_overlay.py b/domain/epc/property_overlays/main_heating_system_overlay.py index 1f06eabc..aeedeed2 100644 --- a/domain/epc/property_overlays/main_heating_system_overlay.py +++ b/domain/epc/property_overlays/main_heating_system_overlay.py @@ -7,6 +7,16 @@ archetype to a representative code and emits a whole-dwelling `HeatingOverlay` targeting `main_heating_details[0]` (`building_part` is ignored). It composes field-wise with the main_fuel / water_heating overlays. +**Coherent Heating System / drag-along (ADR-0035):** a landlord tells us the +*system*, not the dependent fields a coherent heating system carries — its +electricity tariff (meter) and, for storage heaters, its charge control. Rather +than hand-attach those per archetype (easy to forget when a new system is +added), they are **derived from the SAP code**: the off-peak meter from the +calculator's single off-peak classification (`OFF_PEAK_IMPLYING_HEATING_CODES`, +SAP §12), and the conservative manual charge control for storage heaters. So +adding a heating archetype is just adding its code — coherent companions fall +out. Synthesis owns coherence; the calculator never normalises a lodged cert. + The SEDBUK A-G efficiency band the Hyde "Heating" column carries is NOT honoured yet (no efficiency slot on the overlay/MainHeatingDetail) -- archetypes map to their modern/condensing Table 4b code, so an old low-rated boiler is currently @@ -20,11 +30,30 @@ from __future__ import annotations from typing import Optional from domain.modelling.simulation import EpcSimulation, HeatingOverlay +from domain.sap10_calculator.tables.table_12a import ( + OFF_PEAK_IMPLYING_HEATING_CODES, +) -# Canonical system archetype → representative `sap_main_heating_code` (SAP Table -# 4b boiler rows / Table 4a). Codes map to the modern/condensing variant (A-G -# efficiency deferred): 102 regular condensing, 104 condensing combi, 120 CPSU, -# 404 fan storage heaters, 191 direct-acting electric boiler. +# Off-peak (Economy 7) meter. Electric storage / CPSU systems charge overnight at +# the low rate and cannot run economically on a single-rate meter; "Dual" lets +# the §12 dispatch resolve the specific tariff (storage 7-hour, CPSU 10-hour). +_OFF_PEAK_METER = "Dual" + +# SAP Table 4e Group 4 storage charge-control code. Manual charge control is the +# *conservative* assumption when the landlord didn't tell us the control: its +# +0.7 C mean-internal-temperature adjustment is the largest of the storage +# controls (automatic / Celect +0.4, HHR 0), so it never over-credits an +# unobserved control. Scoped to storage *heaters* (Table 4a 401-409) — the only +# systems that take a charge control. +_MANUAL_CHARGE_CONTROL = 2401 +_STORAGE_HEATER_CODES = frozenset(range(401, 410)) + +# Canonical system archetype → representative SAP `sap_main_heating_code`. Codes +# map to the modern/condensing variant (A-G efficiency deferred): 102 regular +# condensing, 104 condensing combi, 120 CPSU, 401-404 storage heaters, 191 +# direct-acting electric. Companion fields (meter / charge control) are NOT +# listed here — they are derived from the code below, so a new archetype is just +# a code. _MAIN_HEATING_CODES: dict[str, int] = { "Gas boiler, combi": 104, "Gas boiler, regular": 102, @@ -37,10 +66,29 @@ _MAIN_HEATING_CODES: dict[str, int] = { } +def _meter_for(code: int) -> Optional[str]: + """The coherent off-peak meter a heating code implies, or None when the + system is single-rate. Keyed off the calculator's §12 off-peak set so the + "which systems are off-peak" knowledge has one home.""" + return _OFF_PEAK_METER if code in OFF_PEAK_IMPLYING_HEATING_CODES else None + + +def _charge_control_for(code: int) -> Optional[int]: + """The conservative storage charge control to assume when unobserved, or + None for systems that don't take one.""" + return _MANUAL_CHARGE_CONTROL if code in _STORAGE_HEATER_CODES else None + + def main_heating_overlay_for( main_heating_value: str, building_part: int ) -> Optional[EpcSimulation]: code = _MAIN_HEATING_CODES.get(main_heating_value) if code is None: return None - return EpcSimulation(heating=HeatingOverlay(sap_main_heating_code=code)) + return EpcSimulation( + heating=HeatingOverlay( + sap_main_heating_code=code, + meter_type=_meter_for(code), + main_heating_control=_charge_control_for(code), + ) + ) diff --git a/domain/epc_prediction/epc_prediction.py b/domain/epc_prediction/epc_prediction.py index bcd4f1cf..59de4201 100644 --- a/domain/epc_prediction/epc_prediction.py +++ b/domain/epc_prediction/epc_prediction.py @@ -86,13 +86,20 @@ class EpcPrediction: (a recent cert reflects the current system). This makes the predicted heating both representative and internally coherent, rather than whatever the size-representative template happened to carry. No donor (no neighbour - lodges a main heating system) leaves the template's heating in place.""" + lodges a main heating system) leaves the template's heating in place. + + The coherent heating system spans more than `sap_heating` (ADR-0035): its + electricity tariff (`sap_energy_source.meter_type`) and hot-water flags + live on loose top-level fields. Carry the donor's whole set, not a subset + — otherwise a donated storage system lands on the template's single-rate + meter and the SAP score collapses (off-peak heat billed at the peak rate).""" donor = _heating_donor(comparables.members) if donor is None: return predicted.sap_heating = copy.deepcopy(donor.epc.sap_heating) predicted.has_hot_water_cylinder = donor.epc.has_hot_water_cylinder predicted.solar_water_heating = donor.epc.solar_water_heating + predicted.sap_energy_source.meter_type = donor.epc.sap_energy_source.meter_type @staticmethod def _apply_glazing_mode( diff --git a/domain/sap10_calculator/tables/table_12a.py b/domain/sap10_calculator/tables/table_12a.py index ab5394f1..7261200e 100644 --- a/domain/sap10_calculator/tables/table_12a.py +++ b/domain/sap10_calculator/tables/table_12a.py @@ -262,6 +262,20 @@ _RULE_3_TEN_HOUR_CODES: Final[frozenset[int]] = frozenset( + [691, 692, 693, 694, 699] # electric room heaters (Table 4a) ) +# The heating codes whose *presence* implies an off-peak (dual) meter: electric +# CPSU (Rule 1) and storage-based electric (Rule 2). These charge overnight and +# cannot run economically on a single rate, so the §12 dispatch already infers +# off-peak for them when the meter is Unknown (see `tariff_dispatch`). Exposed so +# *synthesis* (Landlord-Override / EPC-Prediction) can pair a coherent off-peak +# meter with such a system from the SAP code alone — the single source of "which +# systems are off-peak". Rule 3 (direct-acting electric, heat pumps, room +# heaters) is deliberately NOT here: those run on demand and live on single-rate +# meters too. A "Dual" meter on any of these lets the §12 dispatch resolve the +# specific tariff (CPSU → 10-hour, storage → 7-hour). +OFF_PEAK_IMPLYING_HEATING_CODES: Final[frozenset[int]] = ( + _RULE_1_CPSU_CODES | _RULE_2_STORAGE_CODES +) + def _meter_is_unknown(meter_type: object) -> bool: """True when the meter is the RdSAP "Unknown" sentinel (code 3 / the diff --git a/tests/domain/epc/test_main_heating_system_overlay.py b/tests/domain/epc/test_main_heating_system_overlay.py index 8ccde2b9..27672ef2 100644 --- a/tests/domain/epc/test_main_heating_system_overlay.py +++ b/tests/domain/epc/test_main_heating_system_overlay.py @@ -11,8 +11,12 @@ import pytest from domain.epc.property_overrides.main_heating_system_type import MainHeatingSystemType from domain.epc.property_overlays.main_fuel_overlay import fuel_overlay_for from domain.epc.property_overlays.main_heating_system_overlay import ( + _MAIN_HEATING_CODES, main_heating_overlay_for, ) +from domain.sap10_calculator.tables.table_12a import ( + OFF_PEAK_IMPLYING_HEATING_CODES, +) from domain.epc.property_overlays.water_heating_overlay import ( water_heating_overlay_for, ) @@ -73,6 +77,88 @@ def test_storage_heater_subtypes_decode_to_their_codes( assert simulation.heating.sap_main_heating_code == code +@pytest.mark.parametrize( + "main_heating_value", + [ + "Electric storage heaters, old", + "Electric storage heaters, slimline", + "Electric storage heaters, convector", + "Electric storage heaters, fan", + ], +) +def test_storage_heaters_carry_an_off_peak_meter(main_heating_value: str) -> None: + # Storage heaters run on an Economy 7 (off-peak) tariff by design; setting + # only the heating code while leaving a single-rate meter bills every heating + # kWh at the peak rate and collapses the score (an override-set storage + # dwelling left on an oil donor's single meter scored SAP 13 vs ~50 on + # Economy 7). The overlay carries the off-peak meter, like the HHRSH measure. + simulation = main_heating_overlay_for(main_heating_value, 0) + assert simulation is not None + assert simulation.heating is not None + assert simulation.heating.meter_type == "Dual" + + +@pytest.mark.parametrize( + "main_heating_value", ["Gas boiler, combi", "Direct-acting electric"] +) +def test_non_storage_heating_leaves_the_meter_untouched( + main_heating_value: str, +) -> None: + # Only storage heaters imply an off-peak tariff; gas and direct-acting + # electric (single-rate) keep whatever meter the dwelling already has. + simulation = main_heating_overlay_for(main_heating_value, 0) + assert simulation is not None + assert simulation.heating is not None + assert simulation.heating.meter_type is None + + +@pytest.mark.parametrize( + "main_heating_value", + [ + "Electric storage heaters, old", + "Electric storage heaters, slimline", + "Electric storage heaters, convector", + "Electric storage heaters, fan", + ], +) +def test_storage_heaters_drag_along_conservative_manual_charge_control( + main_heating_value: str, +) -> None: + # The landlord names the system, not its charge control. Manual charge + # control (Table 4e code 2401, +0.7 C MIT adjustment) is the lowest-SAP + # storage control, so it's the safe assumption that never over-credits. + simulation = main_heating_overlay_for(main_heating_value, 0) + assert simulation is not None + assert simulation.heating is not None + assert simulation.heating.main_heating_control == 2401 + + +@pytest.mark.parametrize( + "main_heating_value", ["Gas boiler, combi", "Direct-acting electric"] +) +def test_non_storage_heating_does_not_drag_along_a_charge_control( + main_heating_value: str, +) -> None: + # Charge control is a storage-heater concept; other systems keep their own. + simulation = main_heating_overlay_for(main_heating_value, 0) + assert simulation is not None + assert simulation.heating is not None + assert simulation.heating.main_heating_control is None + + +def test_off_peak_archetypes_drag_a_dual_meter_others_do_not() -> None: + # Contract (the drag-along guard): the off-peak meter is derived from the SAP + # code via the calculator's single off-peak classification, so any heating + # archetype in the map whose code implies off-peak MUST synthesise a Dual + # meter — adding an off-peak system can never silently leave it single-rate — + # and a single-rate system must never gain one. + for value, code in _MAIN_HEATING_CODES.items(): + simulation = main_heating_overlay_for(value, 0) + assert simulation is not None and simulation.heating is not None + expected = "Dual" if code in OFF_PEAK_IMPLYING_HEATING_CODES else None + assert simulation.heating.meter_type == expected, value + + @pytest.mark.parametrize( "main_heating_value", ["Unknown", "", "Air source heat pump", "Community heating"], diff --git a/tests/domain/epc_prediction/test_epc_prediction.py b/tests/domain/epc_prediction/test_epc_prediction.py index 1f1ef42f..00a6ce57 100644 --- a/tests/domain/epc_prediction/test_epc_prediction.py +++ b/tests/domain/epc_prediction/test_epc_prediction.py @@ -12,6 +12,7 @@ from datatypes.epc.domain.epc_property_data import ( EpcPropertyData, MainHeatingDetail, SapBuildingPart, + SapEnergySource, SapFloorDimension, SapHeating, SapWindow, @@ -47,6 +48,7 @@ def _epc( water_heating_code: Optional[int] = 1, has_hot_water_cylinder: bool = True, solar_water_heating: bool = False, + meter_type: str = "2", ) -> EpcPropertyData: epc: EpcPropertyData = object.__new__(EpcPropertyData) epc.property_type = "2" @@ -84,6 +86,9 @@ def _epc( epc.sap_heating = heating epc.has_hot_water_cylinder = has_hot_water_cylinder epc.solar_water_heating = solar_water_heating + energy: SapEnergySource = object.__new__(SapEnergySource) + energy.meter_type = meter_type + epc.sap_energy_source = energy return epc @@ -543,3 +548,14 @@ def test_applies_a_known_wall_override_over_the_mode() -> None: # Assert — the known override overrides the cohort mode. assert predicted.sap_building_parts[0].wall_construction == 2 + + +def test_heating_donor_carries_the_donors_off_peak_meter() -> None: + # The coherent heating system spans the meter (ADR-0035): the donor's + # off-peak meter must travel with its heating cluster, replacing the + # template's single-rate meter — otherwise a donated storage system bills at + # the peak rate and the score collapses. + predicted = _epc(meter_type="2") # the structural template's single meter + donor = _epc(meter_type="Dual", main_fuel_type=29) # the cohort's heating + EpcPrediction._apply_heating_donor(predicted, _cohort(donor)) + assert predicted.sap_energy_source.meter_type == "Dual" diff --git a/tests/domain/epc_prediction/test_validation.py b/tests/domain/epc_prediction/test_validation.py index 1e20b1b1..e335eb8b 100644 --- a/tests/domain/epc_prediction/test_validation.py +++ b/tests/domain/epc_prediction/test_validation.py @@ -65,6 +65,7 @@ def _comparable( energy: SapEnergySource = object.__new__(SapEnergySource) energy.photovoltaic_supply = None energy.photovoltaic_arrays = None + energy.meter_type = "2" epc.sap_energy_source = energy return ComparableProperty( epc=epc,