mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-06-30 13:10:47 +00:00
docs: ADR-0039 (override-aware rebaseline) + ADR-0040 (nested persistence) 🟪
Record the two load-bearing decisions and correct the CONTEXT.md 'Calculated SAP10 Performance' entry: the lodged figure is kept at >=10.2 only for a pristine lodged cert; overrides/prediction adopt the calculator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c776341e03
commit
79b86a38c1
3 changed files with 57 additions and 1 deletions
|
|
@ -128,7 +128,7 @@ The SAP / EPC Band / carbon emissions / Primary Energy Intensity the modelling p
|
|||
_Avoid_: modelled performance, rebaselined performance (only correct when rebaselining ran), scored values
|
||||
|
||||
**Calculated SAP10 Performance**:
|
||||
The SAP score, EPC Band, CO2 emissions, Primary Energy Intensity, space heating kWh, and hot water kWh produced by **SAP10 Calculation** from a Property's EpcPropertyData. It is **not** a separately-persisted third value-set beside Lodged and Effective: in every baselining scenario the calculator's output *is* the **Effective Performance** (real lodged SAP10 EPC with no overrides ⇒ Calculated = Lodged = Effective; overrides or an estimated / pre-SAP10 EPC ⇒ Calculated = Effective, there being no lodged SAP10 figure to compare against). The calculator is therefore the mechanism that produces Effective Performance, having superseded the old ML-API rebaseliner. The calculator is **load-bearing**: for `sap_version < 10.2` (lodged under a superseded methodology) its output *is* the Effective Performance; for `≥ 10.2` the API's lodged figures are kept and the calculator runs **alongside, logging any divergence** (SAP > 0.5, PEUI/CO2 beyond tolerance) as a validation signal (see [[sap-spec-version]]). It is load-bearing for **Bill Derivation regardless of version** (the EPC lodges no per-end-use kWh), so a calculator strict-raise **aborts the batch** and the un-mapped cert is fixed immediately. ADR-0009 introduced the term, amended by ADR-0010, realized by ADR-0013 (whose shadow stepping-stone is superseded) and ADR-0014.
|
||||
The SAP score, EPC Band, CO2 emissions, Primary Energy Intensity, space heating kWh, and hot water kWh produced by **SAP10 Calculation** from a Property's EpcPropertyData. It is **not** a separately-persisted third value-set beside Lodged and Effective: in every baselining scenario the calculator's output *is* the **Effective Performance** (real lodged SAP10 EPC with no overrides ⇒ Calculated = Lodged = Effective; overrides or an estimated / pre-SAP10 EPC ⇒ Calculated = Effective, there being no lodged SAP10 figure to compare against). The calculator is therefore the mechanism that produces Effective Performance, having superseded the old ML-API rebaseliner. The calculator is **load-bearing**: for `sap_version < 10.2` (lodged under a superseded methodology) its output *is* the Effective Performance; for `≥ 10.2` the API's lodged figures are kept **only when the picture is a pristine lodged cert** — when Landlord Overrides or EPC Prediction changed the physical state (Rebaselining trigger (b)/(c)), the calculator's output *is* the Effective Performance even at ≥ 10.2 (ADR-0039), because the accredited lodged figure no longer describes the dwelling. Divergence (SAP > 0.5, PEUI/CO2 beyond tolerance) is logged **only in that pristine-lodged case** — the one case where lodged-vs-calc is a meaningful validation signal (see [[sap-spec-version]]); when the calculator's output is adopted there is nothing to validate against. It is load-bearing for **Bill Derivation regardless of version** (the EPC lodges no per-end-use kWh), so a calculator strict-raise **aborts the batch** and the un-mapped cert is fixed immediately. ADR-0009 introduced the term, amended by ADR-0010, realized by ADR-0013 (whose shadow stepping-stone is superseded) and ADR-0014.
|
||||
_Avoid_: calculator output, computed performance, worksheet performance, SAP10 output, calculated value-set (it is not a stored third set)
|
||||
|
||||
**SAP10 Calculation**:
|
||||
|
|
|
|||
27
docs/adr/0039-override-aware-rebaselining.md
Normal file
27
docs/adr/0039-override-aware-rebaselining.md
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
# Effective baseline rebaselines off the calculator when overrides or prediction moved the picture
|
||||
|
||||
## Context
|
||||
|
||||
**Rebaselining** (CONTEXT.md) is meant to fire on three triggers: (a) the cert was lodged under a superseded methodology (`sap_version < 10.2`), (b) **Landlord Overrides / Site Notes changed the physical state**, or (c) the picture was **estimated (EPC Prediction) or remapped**. On any trigger the calculator's scoring of the Effective EPC *is* the Effective Performance; otherwise Effective equals the lodged accredited figure.
|
||||
|
||||
The code only implemented trigger (a). `CalculatorRebaseliner` branched solely on `sap_version`: for a cert at `sap_version >= 10.2` it **kept the lodged figure** and ran the calculator only to log divergence — regardless of whether Landlord Overrides had changed the dwelling, or the picture was a synthesised prediction. So a property's stored **Effective baseline echoed the lodged accredited headline** even when the authoritative picture (EPC + overrides) described a different dwelling.
|
||||
|
||||
This surfaced as a portfolio-796 hit-list (e.g. property 725634): a lodged ASHP+solar cert scored B/81, but the landlord override reassigned it to electric storage heaters on an uninsulated cavity. The stored Effective stayed **B/81** (lodged echo) while the modelling plan modelled the override-applied picture at **C/D** — an incoherent "already B, post-works C/D" plan. The same applied to predicted properties, whose synthetic headline was trusted as if accredited.
|
||||
|
||||
The "trust lodged at ≥10.2" rule (ADR-0037 / the *Calculated SAP10 Performance* glossary entry) exists for a real reason: the deterministic calculator under-reads genuine accredited certs (e.g. 74.6 vs an accredited 81 on the un-overridden 725634 ASHP cert), so for a **pristine** lodged cert the accredited figure is the better baseline. The bug was applying that rule unconditionally — including to certs the landlord had explicitly corrected.
|
||||
|
||||
## Decision
|
||||
|
||||
Thread a **`physical_state_changed`** signal (`Property.physical_state_changed` — true on Site Notes, Landlord Overrides, or EPC Prediction) from the `PropertyBaselineOrchestrator` into the Rebaseliner. The calculator output becomes Effective Performance whenever **any** trigger fired — `pre_sap10` (a) **or** `physical_state_changed` (b/c) — tagged `pre_sap10` / `physical_state_changed` / `both`. Only a **pristine lodged SAP ≥ 10.2 cert with no overrides** keeps its accredited figure (`reason = "none"`), and that is now the *only* case the calculator runs purely to validate (and the only case the accredited-vs-calc divergence warning is meaningful, so divergence is logged only there).
|
||||
|
||||
This realises trigger (b)/(c) as the domain model always claimed — **restoring documented behaviour, not new policy**. "EPC + overrides is authoritative" (decided with Khalim): a landlord's correction is trusted over the lodged cert, and a mistaken override is a *data* problem fixed at source (with the client), not papered over by the baseline.
|
||||
|
||||
`physical_state_changed` is deliberately coarse — **any** resolved override trips it, not only a "material" one. A no-op override (asserting what the cert already says) would re-introduce the calculator's known under-read into the baseline; we accept that because landlords rarely enter no-op overrides, and "the assembled picture is authoritative" is the simpler, more defensible rule than a materiality threshold.
|
||||
|
||||
## Consequences
|
||||
|
||||
- Re-running the baseline flips every overridden / predicted property's Effective from its lodged/synthetic headline to the calculator's scoring of EPC + overrides. **Many legitimately change band** (725634 → D, 739060 → D, 721985 → C, …). The displayed baseline and the modelling plan now derive from the *same* picture, so they agree — the incoherent "B baseline, C/D plan" is gone.
|
||||
- FE/product should expect a **visible baseline shift** on overridden/predicted properties — this is correct Rebaselining. Pristine lodged ≥10.2 certs are unchanged.
|
||||
- A wrong override now *visibly* downgrades a property (725634 was really an ASHP cert). That is intended: the override is authoritative for modelling, and bad override data is a separate, surfaced concern — an override-plausibility scanner (anomalies like "another dwelling above", community-heated + gas-boiler) is its own ticket.
|
||||
- The baseline is only correct on a **faithful** Effective EPC: this decision is paired with [ADR-0040](0040-nested-field-epc-persistence-fidelity.md) (the DB EPC must round-trip without dropping PV arrays / floor flags), and the orchestrator must read the same faithful picture the plan modelled.
|
||||
- Numbers finalise only after deploy + re-model (the handler re-fetches the live cert and re-saves it faithfully, then the baseline recomputes). The `rebaseline_reason` enum (`none` / `pre_sap10` / `physical_state_changed` / `both`) already existed in the FE schema; this is a backend-only change.
|
||||
29
docs/adr/0040-nested-field-epc-persistence-fidelity.md
Normal file
29
docs/adr/0040-nested-field-epc-persistence-fidelity.md
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
# EPC persistence must round-trip nested scoring fields, guarded recursively
|
||||
|
||||
## Context
|
||||
|
||||
The modelling pipeline scores the **DB-stored** `EpcPropertyData` (ADR-0003: the DB is the read model). ADR-0036 added a round-trip-fidelity guarantee — an `EpcPropertyData` saved to the `epc_property` tables, reloaded and mapped back must reconstruct the original — backed by a deep-equality round-trip test and a structural coverage guard.
|
||||
|
||||
Both had a blind spot: they only covered **top-level** `EpcPropertyData` fields. The round-trip test only catches a dropped field when a *fixture populates it*, and the chosen fixtures (RdSAP-21.0.0/21.0.1) happened not to exercise certain fields; the structural guard inspected only `dataclasses.fields(EpcPropertyData)`, never recursing into nested objects. So fields on nested sub-objects were dropped **silently**, corrupting the scored baseline:
|
||||
|
||||
- `sap_energy_source.photovoltaic_arrays` — no table existed; a dwelling's solar PV was dropped on save, **−12 SAP points** on a real ASHP+solar property (725634: `calc(live)` 74.6 → `calc(reloaded)` 62.1).
|
||||
- `SapFloorDimension.is_exposed_floor` / `is_above_partially_heated_space` — no columns; a `True` flag reloaded as the `False` default, **flipping the floor's heat-loss path** (+8 SAP points on 721534).
|
||||
- Recursing the guard then surfaced **seven more calculator-read nested fields** with no column (community-heating fuel + CHP fraction, alt-wall `is_sheltered`, wall insulation thermal conductivity, `pv_diverter_present`, measured cylinder volume, AP50 air permeability) — the same silent-drop class.
|
||||
|
||||
A lossy read model is corrosive: the handler models off the faithful **in-memory** live cert (so the *plan* is right) but the baseline re-reads the lossy DB projection, so the two disagree for reasons unrelated to any override — compounding the ADR-0039 baseline confusion.
|
||||
|
||||
## Decision
|
||||
|
||||
Make the `epc_property` projection **faithful for every scoring field**, and make the guard able to *see* nested gaps:
|
||||
|
||||
1. **New storage** (FE/Drizzle migrations, applied additively): `epc_photovoltaic_array` child table (one row per array, ordered by `array_index`); `epc_floor_dimension.{is_exposed_floor, is_above_partially_heated_space}`; and columns for the seven calculator-read nested fields. Python SQLModel + `save`/`_compose` mappers wire each, with deep-equality round-trip tests.
|
||||
2. **Recursive structural guard**: the ADR-0036 coverage guard now walks **every domain dataclass reachable from `EpcPropertyData`** (through Optionals/lists) and asserts each field is either reconstructed by a `_compose`/`_to_*` mapper or on an allow-list keyed by `Class.field`. Adding a field at any depth now forces a conscious persist-or-justify decision.
|
||||
|
||||
Fields the calculator does **not** read stay allow-listed as dormant (e.g. `SapBuildingPart.{floor_u_value, roof_u_value, wall_u_value}`); a few structural gaps (`SapRoofWindow`, room-in-roof detail, alt-wall `u_value`/`wall_thickness_mm`) remain FE-column-pending and are explicitly allow-listed, not silent. The top-level `extract_fans_count` (a mirror of the persisted `sap_ventilation` copy the calculator actually reads) is reconstructed from that column so the object round-trips complete with no exception.
|
||||
|
||||
## Consequences
|
||||
|
||||
- The DB EPC round-trips **exactly for every scoring field** — verified against the live schema: 725634 and 721534 now reload with SAP delta `+0.00` (was −12.5 / +8.2). Residual non-equality is limited to allow-listed dormant fields and a sub-millimetre `real`-column float artifact, none scoring-relevant.
|
||||
- This is a **prerequisite for [ADR-0039](0039-override-aware-rebaselining.md)**: storing `calc(EPC + overrides)` as the Effective baseline is only correct if the stored EPC is faithful. Until a property is re-modelled (handler re-fetches + re-saves), its existing DB row stays lossy, so the live baseline finalises only after deploy + re-model.
|
||||
- A class of silent persistence drops can no longer hide: the recursive guard fails CI on any unjustified nested gap.
|
||||
- New EPC scoring fields now carry a persistence cost (column + mapper + round-trip assertion) by construction — the intended friction.
|
||||
Loading…
Add table
Reference in a new issue