review: store epc_building_part.wall_insulation_thickness as JSONB

PR feedback (dancafc): the SQLModel column was Optional[str], but the
domain `SapBuildingPart.wall_insulation_thickness` is Optional[Union[str,
int]] — `_api_resolve_wall_insulation_thickness` returns an int mm when the
API lodges `wall_insulation_thickness == "measured"` (SAP 10.2 §5.7 /
Table 8). The plain str column round-trips that int back as the string
"100", corrupting the Table 8 insulated-wall U-value lookup.

This column was missed in the round-trip-fidelity §1 JSONB sweep
(#1129) — its `Union[str, int]` sibling `roof_insulation_thickness` was
converted, but `wall_insulation_thickness` was not, and no 21.0.0/21.0.1
fixture lodges "measured" so the gap stayed latent. Convert to JSONB
(matching `roof_insulation_thickness` / `flat_roof_insulation_thickness`),
align the column type to Optional[Union[str, int]] (also removes a pyright
type-mismatch), record it in the migration doc §1, and add a round-trip
guard test asserting an int survives as an int (fails as '100' == 100 on
the old str column).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Khalim Conn-Kowlessar 2026-06-05 10:07:24 +00:00
parent c882cb2c95
commit 77f90e144e
3 changed files with 47 additions and 4 deletions

View file

@ -32,8 +32,9 @@ and **still require the matching DB migration** wherever the physical tables liv
`heating_secondary_heating_type`, `heating_shower_outlet_type`, `energy_pv_connection`;
`epc_main_heating_detail`: `main_fuel_type`, `heat_emitter_type`, `emitter_temperature`,
`main_heating_control`; `epc_building_part`: `wall_construction`, `wall_insulation_type`,
`party_wall_construction`, `flat_roof_insulation_thickness`, `roof_insulation_location`,
`roof_insulation_thickness`; `epc_window`: `glazing_gap`, `orientation`, `window_type`,
`party_wall_construction`, `wall_insulation_thickness`, `flat_roof_insulation_thickness`,
`roof_insulation_location`, `roof_insulation_thickness`; `epc_window`: `glazing_gap`,
`orientation`, `window_type`,
`glazing_type`, `window_location`, `window_wall_type`, `draught_proofed`,
`permanent_shutters_present`, `transmission_data_source`).
- **New scalar columns**`epc_property`: `heating_number_baths`, `heating_number_baths_wwhrs`,
@ -68,7 +69,7 @@ in the forward mapper so the Python type round-trips exactly (JSON scalars prese
|---|---|
| `epc_property` | `heating_cylinder_size`, `heating_immersion_heating_type`, `heating_cylinder_insulation_type`, `heating_secondary_heating_type`, `heating_shower_outlet_type`, `energy_pv_connection` |
| `epc_main_heating_detail` | `main_fuel_type`, `heat_emitter_type`, `emitter_temperature`, `main_heating_control` |
| `epc_building_part` | `wall_construction`, `wall_insulation_type`, `party_wall_construction`, `flat_roof_insulation_thickness`, `roof_insulation_location`, `roof_insulation_thickness` |
| `epc_building_part` | `wall_construction`, `wall_insulation_type`, `party_wall_construction`, `wall_insulation_thickness`, `flat_roof_insulation_thickness`, `roof_insulation_location`, `roof_insulation_thickness` |
| `epc_window` | `glazing_gap`, `orientation`, `window_type`, `glazing_type`, `window_location`, `window_wall_type`, `draught_proofed`, `permanent_shutters_present` |
(`energy_meter_type` and `energy_wind_turbines_terrain_type` are `str` in the domain — leave as

View file

@ -549,7 +549,15 @@ class EpcBuildingPartModel(SQLModel, table=True):
building_part_number: Optional[int] = Field(default=None)
wall_dry_lined: Optional[bool] = Field(default=None)
wall_thickness_mm: Optional[int] = Field(default=None)
wall_insulation_thickness: Optional[str] = Field(default=None)
# Union[str, int] — int mm when the API lodges
# `wall_insulation_thickness == "measured"` (resolved by
# `_api_resolve_wall_insulation_thickness`), else the lodged string
# ("NI", a numeric string, ...). JSONB to preserve int vs str on
# round-trip, exactly like the sibling `roof_insulation_thickness` /
# `flat_roof_insulation_thickness`.
wall_insulation_thickness: Optional[Union[str, int]] = Field(
default=None, sa_column=Column(JSONB, nullable=True)
)
floor_heat_loss: Optional[int] = Field(default=None)
floor_insulation_thickness: Optional[str] = Field(default=None)
flat_roof_insulation_thickness: Optional[Union[str, int]] = Field(

View file

@ -48,3 +48,37 @@ def test_epc_property_data_round_trips(schema_dir: str, db_engine: Engine) -> No
# Assert
assert reloaded == original
def test_building_part_wall_insulation_thickness_preserves_int(
db_engine: Engine,
) -> None:
# SAP 10.2 §5.7/Table 8: when the API lodges
# `wall_insulation_thickness == "measured"`, the mapper resolves the
# value to an int mm. The `epc_building_part.wall_insulation_thickness`
# column must therefore preserve int vs str on round-trip (JSONB), like
# its `roof_insulation_thickness` sibling — a plain str column would
# round-trip the int 100 back as "100" and corrupt the Table 8 lookup.
from dataclasses import replace
# Arrange — take a green fixture and force the measured-int case.
original = _load_epc("RdSAP-Schema-21.0.0")
assert original.sap_building_parts, "fixture must have a building part"
bp0 = replace(original.sap_building_parts[0], wall_insulation_thickness=100)
original = replace(
original,
sap_building_parts=[bp0, *original.sap_building_parts[1:]],
)
# 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 int survives as an int, not the string "100".
assert reloaded is not None
value = reloaded.sap_building_parts[0].wall_insulation_thickness
assert value == 100
assert isinstance(value, int)