Spec the room-in-roof persistence gap for the FE migration 📋

`SapRoomInRoof.detailed_surfaces` is read by the SAP calculator
(heat_transmission.py:590, :690-696, :1311-1470) but has no Postgres column, so
the per-surface geometry is silently discarded on save. Without it the cascade
abandons the RdSAP 10 §3.10 "Detailed" route — each surface at its own Table 17 /
Table 4 U-value — and bills the whole notional roof shell at the Table 18
column (4) age-band default, typically 2.30 W/m2K for pre-1976 stock.

Measured on the committed 1000-cert RdSAP-21.0.1 corpus: 108 certs (10.8%) lodge
RIR geometry across 118 building parts; 107 of 108 score differently when it is
dropped; 15 change EPC band; worst case -15.21 SAP (uprn 10012119141, top-floor
maisonette, band E -> F). Larger in both population and magnitude than the
rafter_insulation_thickness gap that opened #1656.

Doc only — the Drizzle tables are owned by Hestia-Homes/assessment-model and the
model-side wiring follows once they exist.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

docs/migrations/epc-room-in-roof-geometry.md

@@ -0,0 +1,269 @@
+# Migration note — `epc_room_in_roof` + `epc_room_in_roof_surface`
+
+**For the team to action alongside the model-side change.**
+The Python domain + calculator are already correct; the **production Drizzle
+schema needs two new child tables** that this repo does not own. Until they
+exist, every room-in-roof dwelling is re-modelled on a corrupted fabric picture.
+
+This is the same defect class as
+[`epc-building-part-fabric-overrides.md`](../../../../workspaces/model/docs/migrations/epc-building-part-fabric-overrides.md)
+(#1656, `rafter_insulation_thickness`) but **6x the population and 1.5x the
+worst-case magnitude**. It is the §2.2 sketch in
+[`epc-property-round-trip-fidelity.md`](../../../../workspaces/model/docs/migrations/epc-property-round-trip-fidelity.md),
+promoted from **P1** to **P0** on the evidence below.
+
+---
+
+## The bug this closes
+
+`SapBuildingPart.sap_room_in_roof` (`SapRoomInRoof`) is flattened on save to
+exactly **two** columns on `epc_building_part`
+(`infrastructure/postgres/epc_property_table.py:663-664`, written at
+`:713-716`, reconstructed at `repositories/epc/epc_postgres_repository.py:979-988`):
+
+| kept | dropped |
+|---|---|
+| `floor_area` → `room_in_roof_floor_area` (`double precision`) | `detailed_surfaces: List[SapRoomInRoofSurface]` — **the entire §3.10 / §3.9.2 per-surface geometry** |
+| `construction_age_band` → `room_in_roof_construction_age_band` (`text`) | `common_wall_length_m`, `common_wall_height_m`, `gable_1_length_m`, `gable_1_height_m`, `gable_2_length_m`, `gable_2_height_m` |
+
+`detailed_surfaces` is read by the SAP calculator in three places —
+`heat_transmission.py:590` (suppresses the Simplified `A_RR` shell),
+`heat_transmission.py:690-696` (rooflight-absorption discriminator) and
+`heat_transmission.py:1311-1470` (the whole Detailed-RR per-surface billing
+loop) — and mapped from the gov-EPC API by
+`datatypes/epc/domain/mapper.py:5507-5556` (`_api_build_room_in_roof`). It is
+then **silently discarded on save** (*persist != score*).
+
+### Why dropping it moves SAP so far
+
+With `detailed_surfaces`, the calculator bills each surface at its own U-value
+and routes it to the right worksheet line — slopes/flat ceilings/stud walls to
+roof (line 30) at **Table 17** U-values (RdSAP 10 Specification 10-06-2025,
+**p.44**), gables to party at **U = 0.25** or to walls at the main-wall U per
+**Table 4** (**p.22**), connected walls at **U = 0** — and derives the §3.10.1
+residual roof area (**p.24**).
+
+Without it, `_part_geometry` falls to the §3.9.1 Simplified Type 1 branch
+(`heat_transmission.py:590-591`) and bills the **whole shell**
+`A_RR = 12.5 × √(A_RR_floor / 1.5)` (spec §3.9.1(d), **p.22**) at the single
+**Table 18 column (4) "Room-in-roof, all elements"** age-band default
+(`domain/sap10_ml/rdsap_uvalues.py:1250`, spec **p.45**) — typically
+**2.30 W/m²K** for pre-1976 bands. Every party gable that should have billed at
+0.25, every connected gable that should have billed at 0, and every measured
+insulation thickness is lost, and none of them deduct from the residual.
+
+So the loss is not a small perturbation of one element — it **switches the
+dwelling from the Detailed cascade to the Simplified all-elements default**.
+
+---
+
+## Measured impact
+
+**Corpus:** `backend/epc_api/json_samples/RdSAP-Schema-21.0.1/corpus.jsonl`,
+1,000 certs, 1,000 mapped, 0 skips. No production DB touched.
+
+**Method** (script: `rir_impact.py` / `rir_v2` in this scratchpad, reproducible):
+
+1. `EpcPropertyDataMapper.from_api_response(cert)` → `EpcPropertyData`.
+2. `Sap10Calculator().calculate(epc).sap_score_continuous` → `sap_with`.
+3. Emulate the round-trip loss: for every building part carrying a
+   `SapRoomInRoof`, replace it with
+   `SapRoomInRoof(floor_area=..., construction_age_band=...)` — i.e. **exactly**
+   what `_to_building_part` can reconstruct today.
+4. Re-score → `sap_without`. `delta = sap_without − sap_with`.
+
+### Population
+
+| measure | count | of 1,000 |
+|---|---:|---:|
+| certs lodging a room-in-roof | **108** | 10.8% |
+| **building parts** lodging a RIR | **118** | — |
+| certs whose RIR carries `detailed_surfaces` | **108** | 10.8% (100% of RIR certs) |
+| total surfaces that would be dropped | **350** | avg 3.2 per cert |
+| certs where the drop moves SAP at all | **107 / 108** | 99.1% |
+| certs where the drop moves the **integer** SAP | **72 / 108** | 66.7% |
+| certs where the drop moves the **EPC band** | **15 / 108** | 13.9% |
+| mean \|Δ SAP\| over the 107 movers | **1.96** | |
+| median \|Δ SAP\| | **1.50** | |
+| direction | 78 worse, 29 better | |
+
+Surface-kind distribution across the 350 dropped surfaces:
+`gable_wall` 140, `gable_wall_external` 45, `common_wall` 44, `slope` 31,
+`gable_wall_sheltered` 27, `stud_wall` 23, `connected_wall` 22,
+`flat_ceiling` 18.
+
+Per-surface field population (of 350): `kind` 350, `area_m2` 350,
+`insulation_thickness_mm` 33, `insulation_type` 0, `u_value` 0.
+
+### Worst 15 certs
+
+| uprn | dwelling type | lodged SAP | with RIR geom | without | Δ | band |
+|---|---|---:|---:|---:|---:|---|
+| 10012119141 | Top-floor maisonette | 45 | 45.89 | 30.68 | **−15.21** | E → F |
+| 100090399615 | Semi-detached house | 67 | 67.75 | 58.83 | −8.92 | D → E |
+| 100040268694 | Mid-terrace house | 58 | 57.97 | 49.16 | −8.81 | D → E |
+| 100032223878 | End-terrace bungalow | 11 | 17.67 | 9.04 | −8.62 | G → G |
+| 22077224 | Semi-detached bungalow | 37 | 33.90 | 27.14 | −6.76 | F → F |
+| 100040259104 | Detached house | 68 | 68.82 | 62.88 | −5.94 | C → D |
+| 90077194 | Mid-terrace house | 65 | 63.69 | 58.69 | −5.00 | D → D |
+| 100061646773 | Top-floor maisonette | 68 | 63.46 | 58.52 | −4.94 | D → D |
+| 100031270375 | Mid-terrace house | 69 | 70.93 | 66.29 | −4.64 | C → D |
+| 10009812472 | Detached house | 40 | 39.40 | 35.14 | −4.26 | E → F |
+| 100040550095 | Mid-terrace house | 40 | 29.87 | 25.67 | −4.20 | F → F |
+| 100031601615 | Mid-terrace house | 73 | 73.01 | 69.12 | −3.89 | C → C |
+| 100050549118 | Semi-detached bungalow | 69 | 68.70 | 64.85 | −3.85 | C → D |
+| 100050964834 | Semi-detached house | 60 | 62.47 | 58.66 | −3.81 | D → D |
+| 100011565830 | Semi-detached house | 72 | 72.64 | 68.84 | −3.80 | C → C |
+
+Note the **`with RIR geom` column tracks the lodged SAP closely** — across these
+15, median |lodged − with| = **0.89**, median |lodged − without| = **5.12**. The
+geometry is not a modelling nicety, it is what makes the engine agree with the
+accredited assessor.
+
+The 29 "better" cases are real too, and just as wrong: a dwelling whose gables
+are exposed (`gable_wall_external`, billed at the main-wall U, often well above
+the Table 18 col (4) default) can score **higher** once its Detailed surfaces are
+dropped. Worst positive drift is **+3.31 SAP** (uprn 72351021). Both directions corrupt the baseline against which every recommendation
+is priced.
+
+---
+
+## Premise correction — the scalar geometry fields are dormant
+
+The §2.2 sketch, and the framing of this ticket, treats the six scalar
+Type-2 fields (`common_wall_*`, `gable_{1,2}_{length,height}_m`) as co-equal
+with `detailed_surfaces`. **They are not.** Corpus measurement:
+
+| field | populated (of 118 RIR building parts) |
+|---|---:|
+| `floor_area` | 118 |
+| `construction_age_band` | 118 |
+| `detailed_surfaces` | **118** |
+| `gable_1_length_m` | 80 |
+| `gable_2_length_m` | 80 |
+| `common_wall_length_m` | **0** |
+| `common_wall_height_m` | **0** |
+| `gable_1_height_m` | **0** |
+| `gable_2_height_m` | **0** |
+
+`common_wall_length_m`, `common_wall_height_m`, `gable_1_height_m` and
+`gable_2_height_m` are **never written by any mapper** — a repo-wide grep for
+assignments finds only `mapper.py:5534-5535` (the two `gable_*_length_m`). And
+the two lengths that *are* set are themselves inert: `_part_geometry`'s gable
+formula (`heat_transmission.py:605-619`) `continue`s unless a matching **height**
+is present, which on the API path it never is. The mapper comment at
+`mapper.py:5536-5546` says so explicitly ("The scalar `gable_*_length_m` fields
+alone can't trigger this ... billing the WHOLE shell as roof").
+
+**Verified by control experiment:** stripping only the six scalars while keeping
+`detailed_surfaces` produces **Δ = 0.000 on all 108 certs**. The entire measured
+impact is attributable to `detailed_surfaces`.
+
+They should still be persisted — the Elmhurst/Site-Notes ingestion path and the
+§3.9.2 Type-2 branch can populate them, and leaving a calculator-read field
+without a column is exactly how #1656 happened — but they are **P2 completeness**,
+not P0. Ship them in the same migration (they are six nullable floats on a table
+we are creating anyway) and note the priority split honestly rather than
+inflating the case.
+
+**Second correction:** "118 of 1,000 corpus **certs**" is a building-part count.
+The cert-level figure is **108 of 1,000 (10.8%)** across **118 building parts**.
+And "101 of 108 change SAP" is **107 of 108** at continuous resolution, **72 of
+108** at integer resolution. The headline −15.21 worst case is confirmed exactly.
+
+---
+
+## Required Drizzle migration
+
+Additive, two new tables, **no backfill possible** (the values were never
+stored). Column-type conventions mirror the existing `epc_building_part` /
+`epc_floor_dimension` / `epc_photovoltaic_array` tables.
+
+### `epc_room_in_roof` — 0..1 per `epc_building_part`
+
+| Column | Type | Null | Notes |
+|---|---|---|---|
+| `id` | `serial` PK | no | |
+| `epc_building_part_id` | `integer` FK → `epc_building_part.id`, **unique** | no | `SapBuildingPart.sap_room_in_roof` is `Optional[SapRoomInRoof]`, not a list — one row max. Unique constraint enforces it. `ON DELETE CASCADE`. |
+| `floor_area` | `double precision` | no | `Union[int, float]`, always populated. **Not** `jsonb` — unlike the code fields, int-vs-float is not semantically load-bearing here (the calculator does `float(rir.floor_area)` at `heat_transmission.py:581`). This matches the existing `room_in_roof_floor_area` column, which the migration **supersedes**. |
+| `construction_age_band` | `text` | no | Always populated. Keys Table 17 / Table 18 lookups. |
+| `common_wall_length_m` | `double precision` | **yes** | §3.9.2. Null ⇒ no Type-2 common wall; `heat_transmission.py:596` requires **both** length and height non-null before billing `L × (0.25 + H)`. Nullability is the discriminator — a `0.0` default would be wrong but harmless; a `NOT NULL DEFAULT 0` is a silent behaviour change. |
+| `common_wall_height_m` | `double precision` | **yes** | As above. Also feeds the gable quadratic correction at `heat_transmission.py:604,612-618`. |
+| `gable_1_length_m` | `double precision` | **yes** | §3.9.1/§3.9.2. `heat_transmission.py:609` skips the gable unless length **and** height are non-null. |
+| `gable_1_height_m` | `double precision` | **yes** | As above. |
+| `gable_2_length_m` | `double precision` | **yes** | As above. |
+| `gable_2_height_m` | `double precision` | **yes** | As above. |
+
+### `epc_room_in_roof_surface` — 0..n per `epc_room_in_roof`
+
+| Column | Type | Null | Notes |
+|---|---|---|---|
+| `id` | `serial` PK | no | |
+| `epc_room_in_roof_id` | `integer` FK → `epc_room_in_roof.id` | no | `ON DELETE CASCADE`. |
+| `surface_index` | `integer` | no | **Ordering column — required.** `detailed_surfaces` is a `List`; without a stable order the round-trip deep-equality test cannot pass and the reload order is whatever Postgres returns. Same convention as `epc_photovoltaic_array.array_index` (`epc_property_table.py:844`). Unique together with `epc_room_in_roof_id`. |
+| `kind` | `text` | no | One of `slope`, `flat_ceiling`, `stud_wall`, `gable_wall`, `gable_wall_external`, `gable_wall_sheltered`, `common_wall`, `connected_wall`. **Do not** make this a Postgres enum — the domain types it as `str` and the set has grown twice already (`gable_wall_sheltered`, `connected_wall`). A CHECK constraint would turn a future mapper addition into a save-time 500. |
+| `area_m2` | `double precision` | no | **Can be negative** — a §3.9.2 step (b) absent-gable adjustment produces a signed area that deducts from the A_RR residual (`heat_transmission.py:1362-1369`, `:1377-1389`). **No CHECK ≥ 0.** |
+| `insulation_thickness_mm` | `integer` | **yes** | Null ⇒ fall back to Table 18 col (4) age-band default (`rdsap_uvalues.py:1176-1177`); non-null ⇒ Table 17 measured row. **The null-vs-zero distinction is the U-value branch** (spec p.44: the `none` row is 2.3, but 12 mm is 1.5). A `DEFAULT 0` would silently bill every measured roof at the uninsulated value. |
+| `insulation_type` | `text` | **yes** | `mineral_wool` / `eps` / `pur` / `pir`. Selects Table 17 sub-column (a) mineral-wool/EPS vs (b) PUR/PIR (`rdsap_uvalues.py:1155-1161`). Null ⇒ column (a). 0/350 populated in the current API corpus but set by the Elmhurst/Site-Notes path. |
+| `u_value` | `double precision` | **yes** | Assessor-lodged U override (W/m²K). Read at `heat_transmission.py:1385` (`gable_wall_external`), `:1400` (`gable_wall_sheltered`), `:1416` (`common_wall`). Null ⇒ fall through to the Table 4 cascade (main-wall U / sheltered formula). **Null is meaningful — it is not "unknown, assume 0".** |
+
+### Supersession of the flattened columns
+
+`epc_building_part.room_in_roof_floor_area` and
+`room_in_roof_construction_age_band` become redundant. **Do not drop them in
+this migration.** Land the new tables additively, deploy the backend, verify the
+round-trip, then drop the two old columns in a follow-up. Dropping them in the
+same migration makes the deploy non-reversible.
+
+Generate with **`npm run migration:generate`** — do **not** hand-write the
+snapshot metadata. Run the migration **before** deploying the backend code.
+
+---
+
+## Backfill / re-ingest
+
+The columns never existed, so **existing rows have nothing to restore**. The
+migration alone changes no baseline. Every affected property must be
+**re-ingested from the gov-EPC API** (the raw JSON is still in `epc_cache`, so
+the source is recoverable) before its score moves.
+
+Expected blast radius: **~11% of the estate** — any dwelling with a room in
+roof. Re-run **Baseline (stage 2) before Modelling (stage 3)** so both stages
+score on one calculator version, otherwise a portfolio will show a fabricated
+~2-point improvement that is really just the baseline being corrected.
+
+---
+
+## Model-side changes required (not yet done)
+
+- `infrastructure/postgres/epc_property_table.py` — new
+  `EpcRoomInRoofModel` and `EpcRoomInRoofSurfaceModel` SQLModels following the
+  `EpcPhotovoltaicArrayModel` shape (`:833-863`), each with a `from_domain`
+  classmethod. Remove the `rir` flattening from
+  `EpcBuildingPartModel.from_domain` (`:684`, `:713-716`) once the old columns
+  are dropped.
+- `repositories/epc/epc_postgres_repository.py` — save the RIR + surface rows in
+  the building-part save loop; reconstruct in `_to_building_part`, replacing the
+  two-field rebuild at `:979-988`. Surfaces must be selected
+  `ORDER BY surface_index`.
+- `tests/repositories/epc/test_epc_round_trip.py` — a
+  `test_room_in_roof_detailed_surfaces_round_trip` asserting deep-equality,
+  including a **negative `area_m2`** surface and a **null vs 0**
+  `insulation_thickness_mm` pair.
+- `tests/repositories/epc/test_epc_persistence_field_coverage.py` — remove the
+  room-in-roof `_UNPERSISTED_ALLOWLIST` entry so the ADR-0036 structural guard
+  *enforces* reconstruction.
+- `docs/migrations/epc-property-round-trip-fidelity.md` — §2.2 marked done;
+  the "Still open" list at `:61-66` updated.
+
+## Why the structural guard missed it
+
+The ADR-0036 guard recurses into nested dataclasses (that is how the PV-array
+and floor-flag drops were caught). `SapRoomInRoof` is on the
+`_UNPERSISTED_ALLOWLIST` with a *correct* justification — "no FE column" — which
+is precisely the problem: an accurate allow-list entry with no attached
+**severity** reads as "known and accepted" rather than "known and costing 15 SAP
+points". The #1656 note already proposes making the justification
+machine-checkable. This case argues for a second field: an allow-list entry for a
+**calculator-read** attribute should be required to carry a measured corpus
+impact, so "we know" and "we've decided it's fine" stop looking identical.