mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-27 23:35:01 +00:00
Persist the explicit basement-wall determination 🟩
RdSAP 10 §5.17 / Table 23. `wall_is_basement` is not a lodged U-value — it is a disambiguation flag selecting WHICH RdSAP default applies, so unlike wall/roof/floor_u_value it must survive the round-trip. 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 it. Dropping the field inverted the answer — a system-built wall came back as a basement wall, dragging the whole ground floor onto the Table 23 basement-floor U-value via `has_basement`. Declared on no RdSAP schema, so it never arrives via the gov API; the earlier corpus scan showing 0 occurrences measured the wrong population. The Elmhurst site-notes path sets it and those EPCs are persisted (scripts/wchg_elmhurst_ingest.py). Nullable is load-bearing: None = "not stated, use the heuristic", False = "explicitly system-built, do not". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d867838330
commit
42ed6f8e46
5 changed files with 101 additions and 11 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)",
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue