diff --git a/docs/migrations/epc-building-part-fabric-overrides.md b/docs/migrations/epc-building-part-fabric-overrides.md index a29a4d9f8..805265f90 100644 --- a/docs/migrations/epc-building-part-fabric-overrides.md +++ b/docs/migrations/epc-building-part-fabric-overrides.md @@ -1,4 +1,4 @@ -# Migration note — `epc_building_part.rafter_insulation_thickness` +# Migration note — `epc_building_part` RdSAP construction inputs **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 @@ -45,6 +45,7 @@ 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. | +| `wall_is_basement` | `boolean` | **yes — load-bearing** | Must be nullable, **not** `NOT NULL DEFAULT false`. `null` = "not stated, fall back to the gov-API code-6 heuristic"; `false` = "explicitly system-built, do NOT apply the heuristic". Collapsing `false` to `null` inverts the result. | Generate with `npm run migration:generate`; run it **before** deploying the backend code. @@ -57,18 +58,45 @@ 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. +## The second column — `wall_is_basement` + +RdSAP 10 §5.17 / Table 23. It is **not** a lodged U-value: it is a +disambiguation flag selecting *which* RdSAP default applies, so it belongs with +`rafter_insulation_thickness`, not with the U-values below. + +RdSAP code 6 is canonically **system-built**, and the gov-API path infers +"basement" from a code-6 heuristic. The Elmhurst site-notes mapper sets +`wall_is_basement=False` precisely to defeat that heuristic. Dropping the field +does not merely lose data — it **inverts** the answer: + +``` +wall_construction = 6 (system-built) + Elmhurst sets wall_is_basement=False -> main_wall_is_basement = False + after DB round-trip (dropped -> None) -> main_wall_is_basement = True <- flipped +``` + +A system-built wall then bills as a basement wall, and via `has_basement` drags +the whole ground floor onto the Table 23 basement-floor U-value. + +The field is declared on **no** RdSAP schema, so it never arrives via the gov +API — but the Elmhurst site-notes path sets it, and site-notes EPCs **are** +persisted (`scripts/wchg_elmhurst_ingest.py`). A corpus scan of gov-API certs +shows 0 occurrences and is therefore the wrong population to measure it on. + +Rare but real: `main_wall_is_basement` is documented at 54 of 67k parts in the +2026 sweep. + ## 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 +The same audit found three sibling `SapBuildingPart` fields with the same drop — +`wall_u_value`, `roof_u_value`, `floor_u_value`. 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%)**. +`roof_u_value` **0**, `floor_u_value` **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 @@ -97,7 +125,9 @@ under `domain/sap10_calculator/`. - `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. + `test_rafter_insulation_thickness_round_trips` pins it deep-equal; + `test_wall_is_basement_round_trips` pins the code-6 inversion case. +- `tests/repositories/epc/test_epc_persistence_field_coverage.py` — the three + U-value entries re-justified; the `rafter_insulation_thickness` and + `wall_is_basement` entries removed so the guard now *enforces* their + reconstruction. diff --git a/infrastructure/postgres/epc_property_table.py b/infrastructure/postgres/epc_property_table.py index d00ed184c..d2c2de654 100644 --- a/infrastructure/postgres/epc_property_table.py +++ b/infrastructure/postgres/epc_property_table.py @@ -660,6 +660,14 @@ class EpcBuildingPartModel(SQLModel, table=True): rafter_insulation_thickness: Optional[Union[str, int]] = Field( default=None, sa_column=Column(JSONB, nullable=True) ) + # RdSAP 10 §5.17 / Table 23 — selects the basement-wall U-value column for + # the part's primary wall (`heat_transmission.py:955` via + # `SapBuildingPart.main_wall_is_basement`). NOT a lodged U-value: it picks + # WHICH RdSAP default applies, so unlike wall/roof/floor_u_value it must be + # persisted. Nullable is load-bearing — `None` means "not stated, fall back + # to the gov-API code-6 heuristic", `False` means "explicitly system-built, + # do NOT apply the heuristic". Collapsing False to None inverts the answer. + wall_is_basement: Optional[bool] = Field(default=None) 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) @@ -710,6 +718,7 @@ class EpcBuildingPartModel(SQLModel, table=True): roof_insulation_location=part.roof_insulation_location, roof_insulation_thickness=part.roof_insulation_thickness, rafter_insulation_thickness=part.rafter_insulation_thickness, + wall_is_basement=part.wall_is_basement, 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 diff --git a/repositories/epc/epc_postgres_repository.py b/repositories/epc/epc_postgres_repository.py index 8afe946a7..caa038e4e 100644 --- a/repositories/epc/epc_postgres_repository.py +++ b/repositories/epc/epc_postgres_repository.py @@ -1004,6 +1004,7 @@ class EpcPostgresRepository(EpcRepository): roof_insulation_location=bp.roof_insulation_location, roof_insulation_thickness=bp.roof_insulation_thickness, rafter_insulation_thickness=bp.rafter_insulation_thickness, + wall_is_basement=bp.wall_is_basement, sap_room_in_roof=( SapRoomInRoof( floor_area=bp.room_in_roof_floor_area, diff --git a/tests/repositories/epc/test_epc_persistence_field_coverage.py b/tests/repositories/epc/test_epc_persistence_field_coverage.py index a44ee4fdf..a3f12bb12 100644 --- a/tests/repositories/epc/test_epc_persistence_field_coverage.py +++ b/tests/repositories/epc/test_epc_persistence_field_coverage.py @@ -72,11 +72,14 @@ _UNPERSISTED_ALLOWLIST: dict[str, str] = { # (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%). + # 0, floor_u_value 0, wall_u_value 4 (0.4%). + # + # `wall_is_basement` is deliberately NOT in this list — it is a + # disambiguation flag selecting which RdSAP default applies (§5.17/Table 23), + # not a measured U-value, and it IS persisted. "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)", } diff --git a/tests/repositories/epc/test_epc_round_trip.py b/tests/repositories/epc/test_epc_round_trip.py index d18a2bc0d..0cacee2aa 100644 --- a/tests/repositories/epc/test_epc_round_trip.py +++ b/tests/repositories/epc/test_epc_round_trip.py @@ -282,3 +282,50 @@ def test_rafter_insulation_thickness_round_trips(db_engine: Engine) -> None: # Assert — the measured rafter thickness survives, deep-equal. assert reloaded == original + + +def test_wall_is_basement_round_trips(db_engine: Engine) -> None: + # RdSAP 10 §5.17 / Table 23 — `wall_is_basement` decides whether a part's + # primary wall bills on the basement-wall column (and, via `has_basement`, + # drags the whole ground floor onto the Table 23 basement-floor U-value). + # + # It is NOT a lodged U-value: it is a disambiguation flag that selects WHICH + # RdSAP default applies, so unlike wall/roof/floor_u_value it must survive + # the round-trip. The gov API never lodges it (declared on no RdSAP schema); + # the Elmhurst site-notes mapper sets it, and site-notes EPCs ARE persisted + # (scripts/wchg_elmhurst_ingest.py). + # + # Dropping it is worse than lossy, it INVERTS: RdSAP code 6 is canonically + # SYSTEM-BUILT, and the API path infers "basement" from a code-6 heuristic. + # Elmhurst sets `wall_is_basement=False` precisely to defeat that heuristic. + # Round-tripping False -> None silently re-enables it, so a system-built wall + # comes back as a basement wall. + from dataclasses import replace + + from datatypes.epc.domain.epc_property_data import ( + BASEMENT_WALL_CONSTRUCTION_CODE, + ) + + # Arrange — a system-built (code 6) main wall explicitly flagged NOT basement. + original = _load_epc("RdSAP-Schema-21.0.1") + assert original.sap_building_parts, "fixture must have a building part" + bp0 = replace( + original.sap_building_parts[0], + wall_construction=BASEMENT_WALL_CONSTRUCTION_CODE, + wall_is_basement=False, + ) + original = replace( + original, sap_building_parts=[bp0, *original.sap_building_parts[1:]] + ) + assert original.sap_building_parts[0].main_wall_is_basement is False + + # Act + with Session(db_engine) as session: + epc_property_id = EpcPostgresRepository(session).save(original) + session.commit() + with Session(db_engine) as session: + reloaded = EpcPostgresRepository(session).get(epc_property_id) + + # Assert — the wall is still system-built, not silently re-read as basement. + assert reloaded is not None + assert reloaded.sap_building_parts[0].main_wall_is_basement is False