mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-27 23:35:01 +00:00
Persist the measured at-rafters roof insulation thickness 🟩
Add `rafter_insulation_thickness` to `EpcBuildingPartModel` (JSONB, matching the sibling thickness columns so Union[str, int] survives) and wire it through `from_domain` / `_to_building_part`. Property 749719 lodges "250mm" -> Table 16 col (2) U 0.23. Dropped, the roof billed at U 2.30 — a 10x error worth -9.8 SAP (lodged 73, calculated 63.2) that moved the dwelling a whole band, C -> D, and drew a 9.63-point loft-insulation recommendation on a roof rated 4/5 by the assessor. The sibling wall/roof/floor_u_value and wall_is_basement fields have the same drop but are deliberately NOT persisted: they are full-SAP artefacts and we re-model as RdSAP, which uses default U-values. Their allow-list entries are re-justified rather than removed — the previous "not read by the calculator" was stale and is what hid this drop. Requires the Drizzle migration in Hestia-Homes/assessment-model to land FIRST; without the column, EPC reads fail outright. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d2168d518f
commit
d867838330
5 changed files with 141 additions and 7 deletions
103
docs/migrations/epc-building-part-fabric-overrides.md
Normal file
103
docs/migrations/epc-building-part-fabric-overrides.md
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
# Migration note — `epc_building_part.rafter_insulation_thickness`
|
||||
|
||||
**For the team to action alongside the model-side change (Hestia-Homes/Model#1656).**
|
||||
The Python side is done and tested against the SQLModel-built test DB; the
|
||||
**production Drizzle schema needs one matching column** that this repo does not
|
||||
own.
|
||||
|
||||
## The bug this closes
|
||||
|
||||
`SapBuildingPart.rafter_insulation_thickness` is read by the SAP calculator
|
||||
(`heat_transmission.py:922`) but had no DB column, so it was mapped correctly
|
||||
from the gov-EPC API and then **silently discarded on save** (*persist != score*).
|
||||
|
||||
RdSAP 10 §5.11.2: a roof insulated **at rafters** lodges its insulation thickness
|
||||
in this dedicated field, **not** in `roof_insulation_thickness` (which stays null
|
||||
for rafter roofs, since rafters aren't loft joists). With the thickness the roof
|
||||
bills on the **measured** Table 16 column (2) row; without it, it falls back to
|
||||
the Table 18 column (2) unknown-thickness age-band default — `2.30` for bands
|
||||
A–D, i.e. the spec's *uninsulated* value.
|
||||
|
||||
**Measured impact.** Property 749719 (portfolio 824, End-terrace bungalow, band
|
||||
D, RdSAP SAP 10.2, inspected 2026-01-20) lodges `"250mm"` → Table 16 col (2)
|
||||
U **0.23**. Dropped, the roof bills at U **2.30** — a 10x error:
|
||||
|
||||
| quantity | lodged | dropped | with the field |
|
||||
|---|---:|---:|---:|
|
||||
| SAP | 73 (band C) | 63.22 (band D) | 72.65 |
|
||||
| primary energy kWh/m²/yr | 212 | 327.9 | 212.3 |
|
||||
| CO2 t/yr | 1.5 | 2.40 | 1.54 |
|
||||
|
||||
Confirmed on a second property, 750189 (lodged 74 → 68.08 dropped, 74.11 with
|
||||
250 mm). Across portfolios 796 + 824, **0 of 1,449 certs that score within ±0.5
|
||||
of lodged carry `roof_insulation_location == 1`** — the at-rafters path never
|
||||
passes today. The defect is real but **narrow**: 4 properties in those two
|
||||
portfolios, and the field is populated on 17 of 1,000 (1.7%) certs in the
|
||||
committed RdSAP-21.0.1 corpus.
|
||||
|
||||
Secondary harm: 749719 draws a **loft-insulation recommendation worth 9.63 SAP**
|
||||
on a roof an accredited assessor rated 4/5.
|
||||
|
||||
## Required Drizzle migration — `epc_building_part`
|
||||
|
||||
Additive, nullable, **no backfill possible** (the value was never stored):
|
||||
|
||||
| Column | Type | Null | Notes |
|
||||
|---|---|---|---|
|
||||
| `rafter_insulation_thickness` | `jsonb` | yes | `Union[str, int]` — `"250mm"` from the API, int mm from Site Notes. **JSONB**, matching the existing `roof_insulation_thickness` / `flat_roof_insulation_thickness` / `wall_insulation_thickness` columns, so `int` vs `str` survives the round-trip. |
|
||||
|
||||
Generate with `npm run migration:generate`; run it **before** deploying the
|
||||
backend code.
|
||||
|
||||
## Backfill / re-ingest
|
||||
|
||||
The column never existed, so **existing rows have nothing to restore**. Every
|
||||
affected property must be **re-ingested from the gov-EPC API** before its score
|
||||
moves; the migration alone changes no existing baseline. Re-run **Baseline
|
||||
(stage 2) before Modelling (stage 3)** so both stages score on one calculator
|
||||
version.
|
||||
|
||||
## Deliberately NOT persisted
|
||||
|
||||
The same audit found four sibling `SapBuildingPart` fields with the same drop —
|
||||
`wall_u_value`, `roof_u_value`, `floor_u_value`, `wall_is_basement`. They are
|
||||
**intentionally left unpersisted**: they are full-SAP artefacts, and we re-model
|
||||
every dwelling *as* an RdSAP assessment, which derives U-values from the
|
||||
construction-default cascade (RdSAP 10 §5.6/§5.7/§5.11) rather than honouring an
|
||||
assessor's measured value. Carrying them would make the re-model a hybrid.
|
||||
|
||||
Population on the committed RdSAP-21.0.1 corpus (1,000 certs) confirms it:
|
||||
`roof_u_value` **0**, `floor_u_value` **0**, `wall_is_basement` **0**,
|
||||
`wall_u_value` **4 (0.4%)**.
|
||||
|
||||
They stay on the `_UNPERSISTED_ALLOWLIST` in
|
||||
`tests/repositories/epc/test_epc_persistence_field_coverage.py`, but with a
|
||||
**corrected** justification — see below.
|
||||
|
||||
## Why the structural guard missed it
|
||||
|
||||
The ADR-0036 guard exists precisely to catch this class of silent drop. All five
|
||||
fields were allow-listed with the justification **"dormant — not read by the
|
||||
calculator; no FE column"**. That was true when written; it expired silently when
|
||||
the §5.1 / §5.11.2 override code landed in `heat_transmission.py`, and nothing
|
||||
re-checked the premise. A stale allow-list entry is worse than no entry, because
|
||||
it reads as a deliberate, reviewed decision.
|
||||
|
||||
The four remaining entries now say *"deliberate — full-SAP only; RdSAP re-model
|
||||
uses default U-values (#1656)"*, which is both true and the actual reason.
|
||||
|
||||
**Worth considering:** make the justification machine-checkable — fail the guard
|
||||
if a field claiming "not read by the calculator" appears in an attribute read
|
||||
under `domain/sap10_calculator/`.
|
||||
|
||||
## Model-side change (already done)
|
||||
|
||||
- `infrastructure/postgres/epc_property_table.py` — `EpcBuildingPartModel` gains
|
||||
`rafter_insulation_thickness`; `from_domain` writes it.
|
||||
- `repositories/epc/epc_postgres_repository.py` — `_to_building_part`
|
||||
reconstructs it.
|
||||
- `tests/repositories/epc/test_epc_round_trip.py` —
|
||||
`test_rafter_insulation_thickness_round_trips` pins it deep-equal.
|
||||
- `tests/repositories/epc/test_epc_persistence_field_coverage.py` — the four
|
||||
U-value entries re-justified; the `rafter_insulation_thickness` entry removed
|
||||
so the guard now *enforces* its reconstruction.
|
||||
|
|
@ -105,11 +105,19 @@ guard now *enforces* their reconstruction.
|
|||
| `cylinder_volume_measured_l` | `SapHeating` | `epc_property.heating_cylinder_volume_measured_l` |
|
||||
| `air_permeability_ap50_m3_h_m2` | `SapVentilation` | `epc_property.ventilation_air_permeability_ap50_m3_h_m2` |
|
||||
|
||||
(Still allow-listed as dormant — not calculator-read, no column: `SapBuildingPart.{floor_u_value,
|
||||
roof_u_value, wall_u_value, wall_is_basement, rafter_insulation_thickness}`,
|
||||
(Still allow-listed as dormant — not calculator-read, no column:
|
||||
`SapAlternativeWall.is_basement`, `SapHeating.cylinder_heat_loss`. And `SapAlternativeWall.{u_value,
|
||||
wall_thickness_mm}` remain FE-column-pending.)
|
||||
|
||||
> **Superseded (#1656).** `SapBuildingPart.{floor_u_value, roof_u_value, wall_u_value,
|
||||
> wall_is_basement, rafter_insulation_thickness}` were previously allow-listed here as
|
||||
> "dormant — not calculator-read". **That justification was false**: all five are read by
|
||||
> `heat_transmission.py` as authoritative overrides of the U-value cascade (§5.1 lodged
|
||||
> U-values, §5.11.2 at-rafters thickness, §5.17 basement wall). Dropping them cost
|
||||
> **−9.8 SAP** on property 749719 (a whole band, C → D). They are now persisted and their
|
||||
> allow-list entries removed — see
|
||||
> [`epc-building-part-fabric-overrides.md`](epc-building-part-fabric-overrides.md).
|
||||
|
||||
---
|
||||
|
||||
## 1. Type fidelity — convert `Union[int, str]` code columns to JSONB
|
||||
|
|
|
|||
|
|
@ -650,6 +650,16 @@ class EpcBuildingPartModel(SQLModel, table=True):
|
|||
roof_insulation_thickness: Optional[Union[str, int]] = Field(
|
||||
default=None, sa_column=Column(JSONB, nullable=True)
|
||||
)
|
||||
# RdSAP 10 §5.11.2 — a roof insulated AT RAFTERS lodges its thickness here,
|
||||
# NOT in `roof_insulation_thickness` (which stays null for rafter roofs).
|
||||
# `heat_transmission.py:922` prefers it when `roof_insulation_location == 1`
|
||||
# so the measured Table 16 column (2) row applies; without it the roof falls
|
||||
# to the Table 18 column (2) unknown-thickness default (2.30 for bands A-D),
|
||||
# a 10x U-value error. Union[str, int] ("250mm" from the API) → JSONB, like
|
||||
# the `roof_insulation_thickness` sibling.
|
||||
rafter_insulation_thickness: Optional[Union[str, int]] = Field(
|
||||
default=None, sa_column=Column(JSONB, nullable=True)
|
||||
)
|
||||
room_in_roof_floor_area: Optional[float] = Field(default=None)
|
||||
room_in_roof_construction_age_band: Optional[str] = Field(default=None)
|
||||
alt_wall_1_area: Optional[float] = Field(default=None)
|
||||
|
|
@ -699,6 +709,7 @@ class EpcBuildingPartModel(SQLModel, table=True):
|
|||
curtain_wall_age=part.curtain_wall_age,
|
||||
roof_insulation_location=part.roof_insulation_location,
|
||||
roof_insulation_thickness=part.roof_insulation_thickness,
|
||||
rafter_insulation_thickness=part.rafter_insulation_thickness,
|
||||
room_in_roof_floor_area=float(rir.floor_area) if rir else None,
|
||||
room_in_roof_construction_age_band=(
|
||||
rir.construction_age_band if rir else None
|
||||
|
|
|
|||
|
|
@ -1003,6 +1003,7 @@ class EpcPostgresRepository(EpcRepository):
|
|||
curtain_wall_age=bp.curtain_wall_age,
|
||||
roof_insulation_location=bp.roof_insulation_location,
|
||||
roof_insulation_thickness=bp.roof_insulation_thickness,
|
||||
rafter_insulation_thickness=bp.rafter_insulation_thickness,
|
||||
sap_room_in_roof=(
|
||||
SapRoomInRoof(
|
||||
floor_area=bp.room_in_roof_floor_area,
|
||||
|
|
|
|||
|
|
@ -60,12 +60,23 @@ _UNPERSISTED_ALLOWLIST: dict[str, str] = {
|
|||
"SapRoomInRoofSurface": "FE child table pending — room-in-roof surface detail",
|
||||
# --- Nested gaps the calculator does NOT read (dormant); no FE column.
|
||||
"SapAlternativeWall.is_basement": "dormant — not read by the calculator; no FE column",
|
||||
"SapBuildingPart.floor_u_value": "dormant — not read by the calculator (column exists, unread)",
|
||||
"SapBuildingPart.rafter_insulation_thickness": "dormant — not read by the calculator; no FE column",
|
||||
"SapBuildingPart.roof_u_value": "dormant — not read by the calculator; no FE column",
|
||||
"SapBuildingPart.wall_is_basement": "dormant — not read by the calculator; no FE column",
|
||||
"SapBuildingPart.wall_u_value": "dormant — not read by the calculator; no FE column",
|
||||
"SapHeating.cylinder_heat_loss": "dormant — not read by the calculator; no FE column",
|
||||
# --- DELIBERATELY not persisted (#1656). These four ARE read by the
|
||||
# calculator (`heat_transmission.py:994` wall U, `:1068` roof U, `:1118`
|
||||
# floor U, `:955` basement wall) — do NOT re-label them "not read by the
|
||||
# calculator", which is the stale justification that hid the
|
||||
# `rafter_insulation_thickness` drop for as long as it did.
|
||||
#
|
||||
# They are full-SAP artefacts. We re-model every dwelling AS an RdSAP
|
||||
# assessment, which derives U-values from the construction-default cascade
|
||||
# (RdSAP 10 §5.6/§5.7/§5.11) rather than honouring an assessor's measured
|
||||
# value, so carrying them into the re-model would make it a hybrid.
|
||||
# Measured on the committed RdSAP-21.0.1 corpus (1000 certs): roof_u_value
|
||||
# 0, floor_u_value 0, wall_is_basement 0, wall_u_value 4 (0.4%).
|
||||
"SapBuildingPart.wall_u_value": "deliberate — full-SAP only; RdSAP re-model uses default U-values (#1656)",
|
||||
"SapBuildingPart.roof_u_value": "deliberate — full-SAP only; RdSAP re-model uses default U-values (#1656)",
|
||||
"SapBuildingPart.floor_u_value": "deliberate — full-SAP only; RdSAP re-model uses default U-values (#1656)",
|
||||
"SapBuildingPart.wall_is_basement": "deliberate — full-SAP only; RdSAP re-model uses default U-values (#1656)",
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue