From 6d2982f826d694ae32f0090837f71ffc5a5c55dd Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 22 Jul 2026 15:51:31 +0000 Subject: [PATCH 1/8] =?UTF-8?q?Persist=20cylinder=5Fheat=5Floss=20so=20it?= =?UTF-8?q?=20survives=20the=20DB=20round-trip=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SapHeating.cylinder_heat_loss is read by the calculator (cert_to_inputs §4 cylinder loss) but had no column, so it was silently dropped on save: a dwelling reloaded from Postgres lost its lodged cylinder loss and fell back to the Table 2b age-band default — worst case -15.53 SAP over the corpus round-trip (#1665). Add heating_cylinder_heat_loss (nullable double precision) to EpcPropertyModel, write it in from_domain and reconstruct it in the SapHeating compose path, mirroring heating_cylinder_volume_measured_l. Remove the now-false _UNPERSISTED_ALLOWLIST entry ("dormant — not read by the calculator") so the ADR structural guard enforces it. Verified: worst cylinder cert 10091630692 round-trip -15.53 -> 0.0000. Companion assessment-model migration (heating_cylinder_heat_loss column) must land + be applied per-environment before the backend deploys (deploy gate). Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/postgres/epc_property_table.py | 4 ++++ repositories/epc/epc_postgres_repository.py | 1 + tests/repositories/epc/test_epc_persistence_field_coverage.py | 1 - 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/infrastructure/postgres/epc_property_table.py b/infrastructure/postgres/epc_property_table.py index d2c2de654..512621d6c 100644 --- a/infrastructure/postgres/epc_property_table.py +++ b/infrastructure/postgres/epc_property_table.py @@ -168,6 +168,9 @@ class EpcPropertyModel(SQLModel, table=True): ) heating_cylinder_insulation_thickness_mm: Optional[int] = Field(default=None) heating_cylinder_volume_measured_l: Optional[int] = Field(default=None) + # #1665: read by cert_to_inputs (§4 cylinder loss); dropping it billed the + # cylinder at the Table 2b age-band default (worst case -15.53 SAP). + heating_cylinder_heat_loss: Optional[float] = Field(default=None) heating_wwhrs_index_number_1: Optional[int] = Field(default=None) heating_wwhrs_index_number_2: Optional[int] = Field(default=None) heating_shower_outlet_type: Optional[Union[int, str]] = Field( @@ -359,6 +362,7 @@ class EpcPropertyModel(SQLModel, table=True): heating_secondary_heating_type=h.secondary_heating_type, heating_cylinder_insulation_thickness_mm=h.cylinder_insulation_thickness_mm, heating_cylinder_volume_measured_l=h.cylinder_volume_measured_l, + heating_cylinder_heat_loss=h.cylinder_heat_loss, heating_wwhrs_index_number_1=h.instantaneous_wwhrs.wwhrs_index_number1, heating_wwhrs_index_number_2=h.instantaneous_wwhrs.wwhrs_index_number2, heating_shower_outlet_type=shower.shower_outlet_type if shower else None, diff --git a/repositories/epc/epc_postgres_repository.py b/repositories/epc/epc_postgres_repository.py index caa038e4e..81620b069 100644 --- a/repositories/epc/epc_postgres_repository.py +++ b/repositories/epc/epc_postgres_repository.py @@ -909,6 +909,7 @@ class EpcPostgresRepository(EpcRepository): secondary_heating_type=p.heating_secondary_heating_type, cylinder_insulation_thickness_mm=p.heating_cylinder_insulation_thickness_mm, cylinder_volume_measured_l=p.heating_cylinder_volume_measured_l, + cylinder_heat_loss=p.heating_cylinder_heat_loss, number_baths=p.heating_number_baths, number_baths_wwhrs=p.heating_number_baths_wwhrs, electric_shower_count=p.heating_electric_shower_count, diff --git a/tests/repositories/epc/test_epc_persistence_field_coverage.py b/tests/repositories/epc/test_epc_persistence_field_coverage.py index a3f12bb12..af416983a 100644 --- a/tests/repositories/epc/test_epc_persistence_field_coverage.py +++ b/tests/repositories/epc/test_epc_persistence_field_coverage.py @@ -60,7 +60,6 @@ _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", - "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 From 9d2ac59ed2a977b547b5cc7adb06413e0710805a Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 22 Jul 2026 16:12:18 +0000 Subject: [PATCH 2/8] =?UTF-8?q?Store=20uprn=20as=20BIGINT=20so=20real=2012?= =?UTF-8?q?-digit=20UPRNs=20persist=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EpcPropertyModel.uprn was `Optional[int]`, which SQLModel maps to INTEGER, so a real 11/12-digit UPRN (e.g. 10000452538) overflowed int32 and hard-failed the insert with NumericValueOutOfRange — a failure on EVERY real-cert save, masked because the round-trip fixtures use small fake UPRNs. The FE `property.uprn` is already bigint; pin the column to BigInteger so ours matches (backend-only, no migration). Verified: uprn 10000452538 saves + reloads. Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/postgres/epc_property_table.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/infrastructure/postgres/epc_property_table.py b/infrastructure/postgres/epc_property_table.py index 512621d6c..f77128799 100644 --- a/infrastructure/postgres/epc_property_table.py +++ b/infrastructure/postgres/epc_property_table.py @@ -1,7 +1,7 @@ from __future__ import annotations from typing import ClassVar, Optional, Union -from sqlalchemy import Column +from sqlalchemy import BigInteger, Column from sqlalchemy import Enum as SAEnum from sqlalchemy.dialects.postgresql import JSONB from sqlmodel import SQLModel, Field @@ -36,7 +36,12 @@ class EpcPropertyModel(SQLModel, table=True): source: str = Field(default="lodged") # Identity / admin - uprn: Optional[int] = Field(default=None) + # BIGINT: real 12-digit UPRNs overflow int32 (the FE `property.uprn` is + # already bigint). SQLModel maps `int` -> INTEGER by default, so pin the + # column type or every real-UPRN insert hard-fails NumericValueOutOfRange. + uprn: Optional[int] = Field( + default=None, sa_column=Column(BigInteger, nullable=True) + ) uprn_source: Optional[str] = Field(default=None) report_reference: Optional[str] = Field(default=None) report_type: Optional[str] = Field(default=None) From 0890b59b56c84608d3f994e843993ffe5cd265bc Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 23 Jul 2026 08:31:43 +0000 Subject: [PATCH 3/8] =?UTF-8?q?Persist=20alternative-wall=20u=5Fvalue=20/?= =?UTF-8?q?=20thickness=20/=20is=5Fbasement=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SapAlternativeWall.u_value (heat_transmission.py:1616), .wall_thickness_mm (:1645) and .is_basement (:1618) were read by the calculator but had no column, so they dropped on save. Latent today (0 gov-API certs lodge them — live only on the Elmhurst/site-notes path), but is_basement is the nastiest shape: it does not null out, it FLIPS meaning (True -> None -> is_basement_wall reads False), silently switching off the RdSAP §5.17 / Table 23 basement-wall U path. Add alt_wall_{1,2}_{u_value,thickness_mm,is_basement} to EpcBuildingPartModel, write in from_domain, reconstruct in _to_alt_wall, and drop the three _UNPERSISTED_ALLOWLIST entries so the structural guard enforces them. is_basement is nullable (None "not stated" != False "not a basement"; a NOT NULL DEFAULT false re-enables the code-6 heuristic — same reasoning as #1661's wall_is_basement). Companion assessment-model migration: six nullable columns on epc_building_part (deploy gate). Round-trip + field-coverage guard green. Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/postgres/epc_property_table.py | 15 +++++++++++++++ repositories/epc/epc_postgres_repository.py | 10 ++++++++++ .../epc/test_epc_persistence_field_coverage.py | 4 ---- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/infrastructure/postgres/epc_property_table.py b/infrastructure/postgres/epc_property_table.py index f77128799..c5f20269f 100644 --- a/infrastructure/postgres/epc_property_table.py +++ b/infrastructure/postgres/epc_property_table.py @@ -686,6 +686,12 @@ class EpcBuildingPartModel(SQLModel, table=True): alt_wall_1_thickness_measured: Optional[str] = Field(default=None) alt_wall_1_insulation_thickness: Optional[str] = Field(default=None) alt_wall_1_is_sheltered: Optional[bool] = Field(default=None) + # #1665: calculator-read alt-wall fields (heat_transmission.py) with no column. + alt_wall_1_u_value: Optional[float] = Field(default=None) + alt_wall_1_thickness_mm: Optional[int] = Field(default=None) + # Nullable is load-bearing: None ("not stated") != False ("not a basement"). + # A NOT NULL DEFAULT false re-enables the code-6 basement heuristic (cf #1661). + alt_wall_1_is_basement: Optional[bool] = Field(default=None) alt_wall_2_area: Optional[float] = Field(default=None) alt_wall_2_dry_lined: Optional[str] = Field(default=None) alt_wall_2_construction: Optional[int] = Field(default=None) @@ -693,6 +699,9 @@ class EpcBuildingPartModel(SQLModel, table=True): alt_wall_2_thickness_measured: Optional[str] = Field(default=None) alt_wall_2_insulation_thickness: Optional[str] = Field(default=None) alt_wall_2_is_sheltered: Optional[bool] = Field(default=None) + alt_wall_2_u_value: Optional[float] = Field(default=None) + alt_wall_2_thickness_mm: Optional[int] = Field(default=None) + alt_wall_2_is_basement: Optional[bool] = Field(default=None) @classmethod def from_domain( @@ -741,6 +750,9 @@ class EpcBuildingPartModel(SQLModel, table=True): aw1.wall_insulation_thickness if aw1 else None ), alt_wall_1_is_sheltered=aw1.is_sheltered if aw1 else None, + alt_wall_1_u_value=aw1.u_value if aw1 else None, + alt_wall_1_thickness_mm=aw1.wall_thickness_mm if aw1 else None, + alt_wall_1_is_basement=aw1.is_basement if aw1 else None, alt_wall_2_area=aw2.wall_area if aw2 else None, alt_wall_2_dry_lined=aw2.wall_dry_lined if aw2 else None, alt_wall_2_construction=aw2.wall_construction if aw2 else None, @@ -750,6 +762,9 @@ class EpcBuildingPartModel(SQLModel, table=True): aw2.wall_insulation_thickness if aw2 else None ), alt_wall_2_is_sheltered=aw2.is_sheltered if aw2 else None, + alt_wall_2_u_value=aw2.u_value if aw2 else None, + alt_wall_2_thickness_mm=aw2.wall_thickness_mm if aw2 else None, + alt_wall_2_is_basement=aw2.is_basement if aw2 else None, ) diff --git a/repositories/epc/epc_postgres_repository.py b/repositories/epc/epc_postgres_repository.py index 81620b069..c633cc542 100644 --- a/repositories/epc/epc_postgres_repository.py +++ b/repositories/epc/epc_postgres_repository.py @@ -1044,6 +1044,13 @@ class EpcPostgresRepository(EpcRepository): else bp.alt_wall_2_insulation_thickness ) sheltered = bp.alt_wall_1_is_sheltered if n == 1 else bp.alt_wall_2_is_sheltered + u_value = bp.alt_wall_1_u_value if n == 1 else bp.alt_wall_2_u_value + thickness_mm = ( + bp.alt_wall_1_thickness_mm if n == 1 else bp.alt_wall_2_thickness_mm + ) + is_basement = ( + bp.alt_wall_1_is_basement if n == 1 else bp.alt_wall_2_is_basement + ) return SapAlternativeWall( wall_area=area, wall_dry_lined=_require(dry_lined, f"alt_wall_{n}_dry_lined"), @@ -1058,6 +1065,9 @@ class EpcPostgresRepository(EpcRepository): # Nullable column (added later than the alt wall itself); a row from # before the column existed reads None → the domain default False. is_sheltered=sheltered if sheltered is not None else False, + u_value=u_value, + wall_thickness_mm=thickness_mm, + is_basement=is_basement, ) @private diff --git a/tests/repositories/epc/test_epc_persistence_field_coverage.py b/tests/repositories/epc/test_epc_persistence_field_coverage.py index af416983a..631d69707 100644 --- a/tests/repositories/epc/test_epc_persistence_field_coverage.py +++ b/tests/repositories/epc/test_epc_persistence_field_coverage.py @@ -45,8 +45,6 @@ _UNPERSISTED_ALLOWLIST: dict[str, str] = { "EpcPropertyData.solar_hw_collector_orientation": "FE column pending — tracked round-trip gap", "EpcPropertyData.solar_hw_collector_pitch_deg": "FE column pending — tracked round-trip gap", "EpcPropertyData.solar_hw_overshading": "FE column pending — tracked round-trip gap", - "SapAlternativeWall.u_value": "FE column pending — tracked round-trip gap", - "SapAlternativeWall.wall_thickness_mm": "FE column pending — tracked round-trip gap", # Room-in-roof detail: only floor_area + age_band reconstruct from the # inlined columns; the geometry / surface detail awaits an FE child table # (tracked round-trip gap, docs/migrations §2). @@ -58,8 +56,6 @@ _UNPERSISTED_ALLOWLIST: dict[str, str] = { "SapRoomInRoof.gable_2_height_m": "FE child table pending — room-in-roof detail", "SapRoomInRoof.gable_2_length_m": "FE child table pending — room-in-roof detail", "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", # --- 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 From 7c626dda8e20719ff4cabf1fe767f62f3637627a Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 23 Jul 2026 09:00:59 +0000 Subject: [PATCH 4/8] =?UTF-8?q?Persist=20roof=20windows=20via=20the=20epc?= =?UTF-8?q?=5Froof=5Fwindow=20child=20table=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EpcPropertyData.sap_roof_windows (List[SapRoofWindow]) had no table, so 55 corpus certs lost their rooflights on the DB round-trip and were re-scored without them (§3 (27a) heat transmission + §6 solar gain). Add EpcRoofWindowModel (mirrors EpcPhotovoltaicArrayModel) with a roof_window_index order column and the resolved SapRoofWindow fields; wire save (batch insert), delete (both delete paths), both read paths (get + get_many) and _compose reconstruction; drop the two _UNPERSISTED_ALLOWLIST entries. u_value_raw / g_perpendicular / frame_factor are stored resolved, not re-derived: the Table 24 lookup is keyed on glazing_type AND glazing_gap, and glazing_gap is not retained on SapRoofWindow, so they can't be reconstructed from the persisted fields. window_location is JSONB (int on API / str on site-notes). Verified: a roof-window cert round-trips sap_roof_windows to deep equality. Companion FE migration: assessment-model#443 (epc_roof_window). Deploy gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/postgres/epc_property_table.py | 44 +++++++++++++++ repositories/epc/epc_postgres_repository.py | 53 +++++++++++++++++++ .../test_epc_persistence_field_coverage.py | 2 - 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/infrastructure/postgres/epc_property_table.py b/infrastructure/postgres/epc_property_table.py index c5f20269f..e16b8624c 100644 --- a/infrastructure/postgres/epc_property_table.py +++ b/infrastructure/postgres/epc_property_table.py @@ -15,6 +15,7 @@ from datatypes.epc.domain.epc_property_data import ( SapBuildingPart, SapFloorDimension, SapFlatDetails, + SapRoofWindow, SapWindow, ) @@ -896,6 +897,49 @@ class EpcPhotovoltaicArrayModel(SQLModel, table=True): ) +class EpcRoofWindowModel(SQLModel, table=True): + __tablename__: ClassVar[str] = ( + "epc_roof_window" # pyright: ignore[reportIncompatibleVariableOverride] + ) + + id: Optional[int] = Field(default=None, primary_key=True) + epc_property_id: int = Field(foreign_key="epc_property.id", nullable=False) + + # List position on `sap_roof_windows`, persisted so the order is recoverable + # for round-trip equality (#1665). Fields mirror SapRoofWindow: area/U feed + # §3 (27a) heat transmission; orientation/pitch/g/frame_factor feed §6 solar. + roof_window_index: int + area_m2: float + u_value_raw: float + orientation: int + pitch_deg: float + g_perpendicular: float + frame_factor: float + glazing_type: int + # SAP building-part index — int on the API path, str on site-notes; JSONB + # preserves the int-vs-str distinction (mirrors SapWindow.window_location). + window_location: Union[int, str] = Field( + sa_column=Column(JSONB, nullable=False) + ) + + @classmethod + def from_domain( + cls, rw: SapRoofWindow, roof_window_index: int, epc_property_id: int + ) -> EpcRoofWindowModel: + return cls( + epc_property_id=epc_property_id, + roof_window_index=roof_window_index, + area_m2=rw.area_m2, + u_value_raw=rw.u_value_raw, + orientation=rw.orientation, + pitch_deg=rw.pitch_deg, + g_perpendicular=rw.g_perpendicular, + frame_factor=rw.frame_factor, + glazing_type=rw.glazing_type, + window_location=rw.window_location, + ) + + class EpcEnergyElementModel(SQLModel, table=True): __tablename__: ClassVar[str] = ( "epc_energy_element" # pyright: ignore[reportIncompatibleVariableOverride] diff --git a/repositories/epc/epc_postgres_repository.py b/repositories/epc/epc_postgres_repository.py index c633cc542..dbfc45404 100644 --- a/repositories/epc/epc_postgres_repository.py +++ b/repositories/epc/epc_postgres_repository.py @@ -29,6 +29,7 @@ from datatypes.epc.domain.epc_property_data import ( SapFlatDetails, SapFloorDimension, SapHeating, + SapRoofWindow, SapRoomInRoof, SapVentilation, SapWindow, @@ -48,6 +49,7 @@ from infrastructure.postgres.epc_property_table import ( EpcPropertyEnergyPerformanceModel, EpcPropertyModel, EpcRenewableHeatIncentiveModel, + EpcRoofWindowModel, EpcWindowModel, ) from repositories.epc.epc_repository import ( @@ -195,6 +197,7 @@ class EpcPostgresRepository(EpcRepository): parts_ordered: list[tuple[Any, int]] = [] # (SapBuildingPart, epc_property_id) window_rows: list[dict[str, Any]] = [] pv_rows: list[dict[str, Any]] = [] + roof_window_rows: list[dict[str, Any]] = [] element_rows: list[dict[str, Any]] = [] flat_rows: list[dict[str, Any]] = [] rhi_rows: list[dict[str, Any]] = [] @@ -231,6 +234,13 @@ class EpcPostgresRepository(EpcRepository): frozenset({"id"}), ) ) + for idx, roof_window in enumerate(d.sap_roof_windows or []): + roof_window_rows.append( + _col_values( + EpcRoofWindowModel.from_domain(roof_window, idx, epc_pid), + frozenset({"id"}), + ) + ) for etype, els in ( ("roof", d.roofs), ("wall", d.walls), @@ -284,6 +294,8 @@ class EpcPostgresRepository(EpcRepository): self._session.execute(_sa_insert(EpcWindowModel), window_rows) # type: ignore[deprecated] if pv_rows: self._session.execute(_sa_insert(EpcPhotovoltaicArrayModel), pv_rows) # type: ignore[deprecated] + if roof_window_rows: + self._session.execute(_sa_insert(EpcRoofWindowModel), roof_window_rows) # type: ignore[deprecated] if element_rows: self._session.execute(_sa_insert(EpcEnergyElementModel), element_rows) # type: ignore[deprecated] if flat_rows: @@ -359,6 +371,7 @@ class EpcPostgresRepository(EpcRepository): EpcBuildingPartModel, EpcWindowModel, EpcPhotovoltaicArrayModel, + EpcRoofWindowModel, EpcFlatDetailsModel, EpcRenewableHeatIncentiveModel, ): @@ -406,6 +419,7 @@ class EpcPostgresRepository(EpcRepository): EpcBuildingPartModel, EpcWindowModel, EpcPhotovoltaicArrayModel, + EpcRoofWindowModel, EpcFlatDetailsModel, EpcRenewableHeatIncentiveModel, ): @@ -553,6 +567,13 @@ class EpcPostgresRepository(EpcRepository): .order_by(EpcPhotovoltaicArrayModel.array_index) # type: ignore[arg-type] ).all() ) + roof_windows_by = _group_by_epc( + self._session.exec( + select(EpcRoofWindowModel) + .where(col(EpcRoofWindowModel.epc_property_id).in_(epc_ids)) + .order_by(EpcRoofWindowModel.roof_window_index) # type: ignore[arg-type] + ).all() + ) part_ids = [ bp.id for parts in parts_by.values() for bp in parts if bp.id is not None ] @@ -570,6 +591,7 @@ class EpcPostgresRepository(EpcRepository): floor_dims_by_part=floor_dims_by_part, window_rows=windows_by.get(epc_id, []), pv_array_rows=pv_arrays_by.get(epc_id, []), + roof_window_rows=roof_windows_by.get(epc_id, []), flat_row=flat_by.get(epc_id), rhi_row=rhi_by.get(epc_id), ) @@ -632,6 +654,7 @@ class EpcPostgresRepository(EpcRepository): ).first() window_rows = self._windows(epc_property_id) pv_array_rows = self._pv_arrays(epc_property_id) + roof_window_rows = self._roof_windows(epc_property_id) floor_dims_by_part = self._floor_dims_by_part( [bp.id for bp in part_rows if bp.id is not None] ) @@ -644,6 +667,7 @@ class EpcPostgresRepository(EpcRepository): floor_dims_by_part=floor_dims_by_part, window_rows=window_rows, pv_array_rows=pv_array_rows, + roof_window_rows=roof_window_rows, flat_row=flat_row, rhi_row=rhi_row, ) @@ -659,6 +683,7 @@ class EpcPostgresRepository(EpcRepository): floor_dims_by_part: dict[int, list[EpcFloorDimensionModel]], window_rows: list[EpcWindowModel], pv_array_rows: list[EpcPhotovoltaicArrayModel], + roof_window_rows: list[EpcRoofWindowModel], flat_row: Optional[EpcFlatDetailsModel], rhi_row: Optional[EpcRenewableHeatIncentiveModel], ) -> EpcPropertyData: @@ -688,6 +713,11 @@ class EpcPostgresRepository(EpcRepository): door_count=p.door_count, sap_heating=self._to_sap_heating(p, heating_rows), sap_windows=[self._to_window(w) for w in window_rows], + sap_roof_windows=( + [self._to_roof_window(r) for r in roof_window_rows] + if roof_window_rows + else None + ), sap_energy_source=self._to_energy_source(p, pv_array_rows), sap_building_parts=[ self._to_building_part( @@ -869,6 +899,29 @@ class EpcPostgresRepository(EpcRepository): ).all() ) + @private + def _roof_windows(self, epc_property_id: int) -> list[EpcRoofWindowModel]: + return list( + self._session.exec( + select(EpcRoofWindowModel) + .where(EpcRoofWindowModel.epc_property_id == epc_property_id) + .order_by(EpcRoofWindowModel.roof_window_index) # type: ignore[arg-type] + ).all() + ) + + @private + def _to_roof_window(self, r: EpcRoofWindowModel) -> SapRoofWindow: + return SapRoofWindow( + area_m2=r.area_m2, + u_value_raw=r.u_value_raw, + orientation=r.orientation, + pitch_deg=r.pitch_deg, + g_perpendicular=r.g_perpendicular, + frame_factor=r.frame_factor, + glazing_type=r.glazing_type, + window_location=r.window_location, + ) + @private def _to_energy_element(self, e: EpcEnergyElementModel) -> EnergyElement: return EnergyElement( diff --git a/tests/repositories/epc/test_epc_persistence_field_coverage.py b/tests/repositories/epc/test_epc_persistence_field_coverage.py index 631d69707..2c32c3f4c 100644 --- a/tests/repositories/epc/test_epc_persistence_field_coverage.py +++ b/tests/repositories/epc/test_epc_persistence_field_coverage.py @@ -40,8 +40,6 @@ _UNPERSISTED_ALLOWLIST: dict[str, str] = { "EpcPropertyData.lzc_energy_sources": "dormant — not read by the calculator; no FE column", # Scoring-relevant round-trip gaps awaiting FE columns / child tables # (tracked follow-ups; see docs/migrations/epc-property-round-trip-fidelity.md). - "EpcPropertyData.sap_roof_windows": "FE child table pending — tracked round-trip gap", - "SapRoofWindow": "FE child table pending — tracked round-trip gap (sap_roof_windows)", "EpcPropertyData.solar_hw_collector_orientation": "FE column pending — tracked round-trip gap", "EpcPropertyData.solar_hw_collector_pitch_deg": "FE column pending — tracked round-trip gap", "EpcPropertyData.solar_hw_overshading": "FE column pending — tracked round-trip gap", From 846a3238b3e0615a2fb035557020af7d5cf5c081 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 23 Jul 2026 09:17:10 +0000 Subject: [PATCH 5/8] =?UTF-8?q?Persist=20room-in-roof=20geometry=20via=20c?= =?UTF-8?q?hild=20tables=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SapRoomInRoof.detailed_surfaces (+ the gable/common-wall scalars) had no columns, so 107 corpus certs lost their whole §3.9/§3.10 room-in-roof geometry on the DB round-trip and fell back to the Simplified all-elements age-band default — worst case -15.21 SAP (#1665 / #1664). Add EpcRoomInRoofModel (0..1 per building part, unique FK) + EpcRoomInRoofSurface Model (0..n, surface_index order). Save nests via RETURNING (building part -> RIR -> surfaces); delete clears bottom-up (surfaces -> room_in_roof -> parts) since the FE FKs are ON DELETE no action; both read paths group + reconstruct in a new _to_room_in_roof / _to_rir_surface, replacing the two-field flat rebuild in _to_building_part. The flat room_in_roof_* columns on epc_building_part are superseded and dropped in a follow-up (#1664). Nullable is preserved on insulation_thickness_mm / u_value (null vs 0 is the Table 17-vs-18 branch); area is unconstrained (a §3.9.2 absent-gable adjustment is signed). Drop the 8 _UNPERSISTED_ALLOWLIST entries. Verified: worst RIR cert 10012119141 round-trip -15.21 -> 0.0000 with detailed_surfaces at deep equality. Companion FE migration: assessment-model#443 (epc_room_in_roof + epc_room_in_roof_surface). Deploy gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/postgres/epc_property_table.py | 83 ++++++++ repositories/epc/epc_postgres_repository.py | 184 ++++++++++++++++-- .../test_epc_persistence_field_coverage.py | 11 -- 3 files changed, 253 insertions(+), 25 deletions(-) diff --git a/infrastructure/postgres/epc_property_table.py b/infrastructure/postgres/epc_property_table.py index e16b8624c..c08e6c8c5 100644 --- a/infrastructure/postgres/epc_property_table.py +++ b/infrastructure/postgres/epc_property_table.py @@ -16,6 +16,8 @@ from datatypes.epc.domain.epc_property_data import ( SapFloorDimension, SapFlatDetails, SapRoofWindow, + SapRoomInRoof, + SapRoomInRoofSurface, SapWindow, ) @@ -940,6 +942,87 @@ class EpcRoofWindowModel(SQLModel, table=True): ) +class EpcRoomInRoofModel(SQLModel, table=True): + __tablename__: ClassVar[str] = ( + "epc_room_in_roof" # pyright: ignore[reportIncompatibleVariableOverride] + ) + + id: Optional[int] = Field(default=None, primary_key=True) + # 0..1 per building part (SapBuildingPart.sap_room_in_roof is Optional, not a + # list) — enforced UNIQUE by the FE migration. #1665 + #1664. + epc_building_part_id: int = Field( + foreign_key="epc_building_part.id", nullable=False + ) + # floor_area is Union[int, float] in the domain; int-vs-float is not + # semantically load-bearing (the calculator does float(rir.floor_area)). + floor_area: float + construction_age_band: str + # §3.9.2 Simplified Type 2 gable/common-wall scalars (dormant on the API + # path — 0 corpus — but populated by the Elmhurst/site-notes path). + common_wall_length_m: Optional[float] = Field(default=None) + common_wall_height_m: Optional[float] = Field(default=None) + gable_1_length_m: Optional[float] = Field(default=None) + gable_1_height_m: Optional[float] = Field(default=None) + gable_2_length_m: Optional[float] = Field(default=None) + gable_2_height_m: Optional[float] = Field(default=None) + + @classmethod + def from_domain( + cls, rir: SapRoomInRoof, epc_building_part_id: int + ) -> EpcRoomInRoofModel: + return cls( + epc_building_part_id=epc_building_part_id, + floor_area=float(rir.floor_area), + construction_age_band=rir.construction_age_band, + common_wall_length_m=rir.common_wall_length_m, + common_wall_height_m=rir.common_wall_height_m, + gable_1_length_m=rir.gable_1_length_m, + gable_1_height_m=rir.gable_1_height_m, + gable_2_length_m=rir.gable_2_length_m, + gable_2_height_m=rir.gable_2_height_m, + ) + + +class EpcRoomInRoofSurfaceModel(SQLModel, table=True): + __tablename__: ClassVar[str] = ( + "epc_room_in_roof_surface" # pyright: ignore[reportIncompatibleVariableOverride] + ) + + id: Optional[int] = Field(default=None, primary_key=True) + epc_room_in_roof_id: int = Field( + foreign_key="epc_room_in_roof.id", nullable=False + ) + # List position on `detailed_surfaces`, persisted for round-trip order. + surface_index: int + kind: str + # Can be NEGATIVE: a §3.9.2 absent-gable adjustment produces a signed area + # that deducts from the A_RR residual — no CHECK >= 0. + area_m2: float + # Null vs 0 is the U-value branch (Table 17 measured row vs Table 18 col (4) + # age-band default) — keep nullable, never DEFAULT 0. + insulation_thickness_mm: Optional[int] = Field(default=None) + insulation_type: Optional[str] = Field(default=None) + # Assessor U override; null falls through to the Table 4 cascade. Null != 0. + u_value: Optional[float] = Field(default=None) + + @classmethod + def from_domain( + cls, + surface: SapRoomInRoofSurface, + surface_index: int, + epc_room_in_roof_id: int, + ) -> EpcRoomInRoofSurfaceModel: + return cls( + epc_room_in_roof_id=epc_room_in_roof_id, + surface_index=surface_index, + kind=surface.kind, + area_m2=surface.area_m2, + insulation_thickness_mm=surface.insulation_thickness_mm, + insulation_type=surface.insulation_type, + u_value=surface.u_value, + ) + + class EpcEnergyElementModel(SQLModel, table=True): __tablename__: ClassVar[str] = ( "epc_energy_element" # pyright: ignore[reportIncompatibleVariableOverride] diff --git a/repositories/epc/epc_postgres_repository.py b/repositories/epc/epc_postgres_repository.py index dbfc45404..e4d16577f 100644 --- a/repositories/epc/epc_postgres_repository.py +++ b/repositories/epc/epc_postgres_repository.py @@ -31,6 +31,7 @@ from datatypes.epc.domain.epc_property_data import ( SapHeating, SapRoofWindow, SapRoomInRoof, + SapRoomInRoofSurface, SapVentilation, SapWindow, ShowerOutlet, @@ -50,6 +51,8 @@ from infrastructure.postgres.epc_property_table import ( EpcPropertyModel, EpcRenewableHeatIncentiveModel, EpcRoofWindowModel, + EpcRoomInRoofModel, + EpcRoomInRoofSurfaceModel, EpcWindowModel, ) from repositories.epc.epc_repository import ( @@ -330,6 +333,36 @@ class EpcPostgresRepository(EpcRepository): if floor_rows: self._session.execute(_sa_insert(EpcFloorDimensionModel), floor_rows) # type: ignore[deprecated] + # Room-in-roof: 0..1 per building part, each with 0..n surfaces. + # Insert the RIR rows with RETURNING, then zip to resolve the + # surface FKs (same positional-zip contract as the parts above). + rir_pairs = [ + (part.sap_room_in_roof, bp_row[0]) + for (part, _), bp_row in zip(parts_ordered, returned_bps) + if part.sap_room_in_roof is not None + ] + if rir_pairs: + rir_rows = [ + _col_values( + EpcRoomInRoofModel.from_domain(rir, bp_id), frozenset({"id"}) + ) + for rir, bp_id in rir_pairs + ] + returned_rirs = self._session.execute( # type: ignore[deprecated] + _sa_insert(EpcRoomInRoofModel).returning(EpcRoomInRoofModel.__table__.c["id"]), # type: ignore[attr-defined] + rir_rows, + ).all() + surface_rows = [ + _col_values( + EpcRoomInRoofSurfaceModel.from_domain(surface, s_idx, rir_row[0]), + frozenset({"id"}), + ) + for (rir, _), rir_row in zip(rir_pairs, returned_rirs) + for s_idx, surface in enumerate(rir.detailed_surfaces or []) + ] + if surface_rows: + self._session.execute(_sa_insert(EpcRoomInRoofSurfaceModel), surface_rows) # type: ignore[deprecated] + return epc_property_ids def _delete_for_properties( @@ -359,6 +392,29 @@ class EpcPostgresRepository(EpcRepository): if i is not None ] if part_ids: + # Room-in-roof surfaces FK the RIR row, which FKs the building part; + # with ON DELETE no action we must clear the graph bottom-up + # (surfaces -> room_in_roof) before the building parts below. + rir_ids = [ + i + for i in self._session.exec( + select(EpcRoomInRoofModel.id).where( + col(EpcRoomInRoofModel.epc_building_part_id).in_(part_ids) + ) + ).all() + if i is not None + ] + if rir_ids: + self._session.exec( # type: ignore[call-overload] + delete(EpcRoomInRoofSurfaceModel).where( + col(EpcRoomInRoofSurfaceModel.epc_room_in_roof_id).in_(rir_ids) + ) + ) + self._session.exec( # type: ignore[call-overload] + delete(EpcRoomInRoofModel).where( + col(EpcRoomInRoofModel.epc_building_part_id).in_(part_ids) + ) + ) self._session.exec( # type: ignore[call-overload] delete(EpcFloorDimensionModel).where( col(EpcFloorDimensionModel.epc_building_part_id).in_(part_ids) @@ -407,6 +463,29 @@ class EpcPostgresRepository(EpcRepository): if i is not None ] if part_ids: + # Room-in-roof surfaces FK the RIR row, which FKs the building part; + # with ON DELETE no action we must clear the graph bottom-up + # (surfaces -> room_in_roof) before the building parts below. + rir_ids = [ + i + for i in self._session.exec( + select(EpcRoomInRoofModel.id).where( + col(EpcRoomInRoofModel.epc_building_part_id).in_(part_ids) + ) + ).all() + if i is not None + ] + if rir_ids: + self._session.exec( # type: ignore[call-overload] + delete(EpcRoomInRoofSurfaceModel).where( + col(EpcRoomInRoofSurfaceModel.epc_room_in_roof_id).in_(rir_ids) + ) + ) + self._session.exec( # type: ignore[call-overload] + delete(EpcRoomInRoofModel).where( + col(EpcRoomInRoofModel.epc_building_part_id).in_(part_ids) + ) + ) self._session.exec( # type: ignore[call-overload] delete(EpcFloorDimensionModel).where( col(EpcFloorDimensionModel.epc_building_part_id).in_(part_ids) @@ -578,6 +657,10 @@ class EpcPostgresRepository(EpcRepository): bp.id for parts in parts_by.values() for bp in parts if bp.id is not None ] floor_dims_by_part = self._floor_dims_by_part(part_ids) + rir_by_part = self._rir_by_part(part_ids) + surfaces_by_rir = self._rir_surfaces_by_rir( + [r.id for r in rir_by_part.values() if r.id is not None] + ) result: dict[int, EpcPropertyData] = {} for key, parent in parents_by_key.items(): @@ -589,6 +672,8 @@ class EpcPostgresRepository(EpcRepository): heating_rows=heating_by.get(epc_id, []), part_rows=parts_by.get(epc_id, []), floor_dims_by_part=floor_dims_by_part, + rir_by_part=rir_by_part, + surfaces_by_rir=surfaces_by_rir, window_rows=windows_by.get(epc_id, []), pv_array_rows=pv_arrays_by.get(epc_id, []), roof_window_rows=roof_windows_by.get(epc_id, []), @@ -612,6 +697,33 @@ class EpcPostgresRepository(EpcRepository): grouped.setdefault(row.epc_building_part_id, []).append(row) return grouped + @private + def _rir_by_part(self, part_ids: list[int]) -> dict[int, EpcRoomInRoofModel]: + if not part_ids: + return {} + rows = self._session.exec( + select(EpcRoomInRoofModel).where( + col(EpcRoomInRoofModel.epc_building_part_id).in_(part_ids) + ) + ).all() + return {row.epc_building_part_id: row for row in rows} + + @private + def _rir_surfaces_by_rir( + self, rir_ids: list[int] + ) -> dict[int, list[EpcRoomInRoofSurfaceModel]]: + if not rir_ids: + return {} + rows = self._session.exec( + select(EpcRoomInRoofSurfaceModel) + .where(col(EpcRoomInRoofSurfaceModel.epc_room_in_roof_id).in_(rir_ids)) + .order_by(EpcRoomInRoofSurfaceModel.surface_index) # type: ignore[arg-type] + ).all() + grouped: dict[int, list[EpcRoomInRoofSurfaceModel]] = {} + for row in rows: + grouped.setdefault(row.epc_room_in_roof_id, []).append(row) + return grouped + def get(self, epc_property_id: int) -> EpcPropertyData: p = self._session.get(EpcPropertyModel, epc_property_id) if p is None: @@ -655,8 +767,11 @@ class EpcPostgresRepository(EpcRepository): window_rows = self._windows(epc_property_id) pv_array_rows = self._pv_arrays(epc_property_id) roof_window_rows = self._roof_windows(epc_property_id) - floor_dims_by_part = self._floor_dims_by_part( - [bp.id for bp in part_rows if bp.id is not None] + part_ids = [bp.id for bp in part_rows if bp.id is not None] + floor_dims_by_part = self._floor_dims_by_part(part_ids) + rir_by_part = self._rir_by_part(part_ids) + surfaces_by_rir = self._rir_surfaces_by_rir( + [r.id for r in rir_by_part.values() if r.id is not None] ) return self._compose( p=p, @@ -665,6 +780,8 @@ class EpcPostgresRepository(EpcRepository): heating_rows=heating_rows, part_rows=part_rows, floor_dims_by_part=floor_dims_by_part, + rir_by_part=rir_by_part, + surfaces_by_rir=surfaces_by_rir, window_rows=window_rows, pv_array_rows=pv_array_rows, roof_window_rows=roof_window_rows, @@ -681,6 +798,8 @@ class EpcPostgresRepository(EpcRepository): heating_rows: list[EpcMainHeatingDetailModel], part_rows: list[EpcBuildingPartModel], floor_dims_by_part: dict[int, list[EpcFloorDimensionModel]], + rir_by_part: dict[int, EpcRoomInRoofModel], + surfaces_by_rir: dict[int, list[EpcRoomInRoofSurfaceModel]], window_rows: list[EpcWindowModel], pv_array_rows: list[EpcPhotovoltaicArrayModel], roof_window_rows: list[EpcRoofWindowModel], @@ -721,7 +840,10 @@ class EpcPostgresRepository(EpcRepository): sap_energy_source=self._to_energy_source(p, pv_array_rows), sap_building_parts=[ self._to_building_part( - bp, floor_dims_by_part.get(bp.id, []) if bp.id is not None else [] + bp, + floor_dims_by_part.get(bp.id, []) if bp.id is not None else [], + rir_by_part.get(bp.id) if bp.id is not None else None, + surfaces_by_rir, ) for bp in part_rows ], @@ -1028,7 +1150,11 @@ class EpcPostgresRepository(EpcRepository): @private def _to_building_part( - self, bp: EpcBuildingPartModel, floor_rows: list[EpcFloorDimensionModel] + self, + bp: EpcBuildingPartModel, + floor_rows: list[EpcFloorDimensionModel], + rir_model: Optional[EpcRoomInRoofModel], + surfaces_by_rir: dict[int, list[EpcRoomInRoofSurfaceModel]], ) -> SapBuildingPart: return SapBuildingPart( identifier=BuildingPartIdentifier(bp.identifier), @@ -1059,19 +1185,49 @@ class EpcPostgresRepository(EpcRepository): 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, - construction_age_band=_require( - bp.room_in_roof_construction_age_band, - "room_in_roof_construction_age_band", - ), - ) - if bp.room_in_roof_floor_area is not None - else None + # Reconstruct from the epc_room_in_roof child table (full geometry + # incl. detailed_surfaces); the flat room_in_roof_* columns on + # epc_building_part are superseded and dropped in a follow-up (#1664). + sap_room_in_roof=self._to_room_in_roof(rir_model, surfaces_by_rir), + ) + + @private + def _to_room_in_roof( + self, + rir_model: Optional[EpcRoomInRoofModel], + surfaces_by_rir: dict[int, list[EpcRoomInRoofSurfaceModel]], + ) -> Optional[SapRoomInRoof]: + if rir_model is None: + return None + surfaces = ( + surfaces_by_rir.get(rir_model.id, []) if rir_model.id is not None else [] + ) + return SapRoomInRoof( + floor_area=rir_model.floor_area, + construction_age_band=rir_model.construction_age_band, + common_wall_length_m=rir_model.common_wall_length_m, + common_wall_height_m=rir_model.common_wall_height_m, + gable_1_length_m=rir_model.gable_1_length_m, + gable_1_height_m=rir_model.gable_1_height_m, + gable_2_length_m=rir_model.gable_2_length_m, + gable_2_height_m=rir_model.gable_2_height_m, + detailed_surfaces=( + [self._to_rir_surface(s) for s in surfaces] if surfaces else None ), ) + @private + def _to_rir_surface( + self, s: EpcRoomInRoofSurfaceModel + ) -> SapRoomInRoofSurface: + return SapRoomInRoofSurface( + kind=s.kind, + area_m2=s.area_m2, + insulation_thickness_mm=s.insulation_thickness_mm, + insulation_type=s.insulation_type, + u_value=s.u_value, + ) + @private def _to_alt_wall( self, bp: EpcBuildingPartModel, n: int diff --git a/tests/repositories/epc/test_epc_persistence_field_coverage.py b/tests/repositories/epc/test_epc_persistence_field_coverage.py index 2c32c3f4c..440179dd6 100644 --- a/tests/repositories/epc/test_epc_persistence_field_coverage.py +++ b/tests/repositories/epc/test_epc_persistence_field_coverage.py @@ -43,17 +43,6 @@ _UNPERSISTED_ALLOWLIST: dict[str, str] = { "EpcPropertyData.solar_hw_collector_orientation": "FE column pending — tracked round-trip gap", "EpcPropertyData.solar_hw_collector_pitch_deg": "FE column pending — tracked round-trip gap", "EpcPropertyData.solar_hw_overshading": "FE column pending — tracked round-trip gap", - # Room-in-roof detail: only floor_area + age_band reconstruct from the - # inlined columns; the geometry / surface detail awaits an FE child table - # (tracked round-trip gap, docs/migrations §2). - "SapRoomInRoof.common_wall_height_m": "FE child table pending — room-in-roof detail", - "SapRoomInRoof.common_wall_length_m": "FE child table pending — room-in-roof detail", - "SapRoomInRoof.detailed_surfaces": "FE child table pending — room-in-roof detail", - "SapRoomInRoof.gable_1_height_m": "FE child table pending — room-in-roof detail", - "SapRoomInRoof.gable_1_length_m": "FE child table pending — room-in-roof detail", - "SapRoomInRoof.gable_2_height_m": "FE child table pending — room-in-roof detail", - "SapRoomInRoof.gable_2_length_m": "FE child table pending — room-in-roof detail", - "SapRoomInRoofSurface": "FE child table pending — room-in-roof surface detail", # --- 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 From 50493f07c6cfb23fc22828298d8de1ea06739e9e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 23 Jul 2026 09:23:26 +0000 Subject: [PATCH 6/8] =?UTF-8?q?Persist=20the=20lodged=20wall=5Fu=5Fvalue?= =?UTF-8?q?=20override=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SapBuildingPart.wall_u_value (the gov-EPC lodged wall U, authoritative over the derived default) had no column and dropped on save, so the reloaded dwelling fell back to the age-band default U — -2.32 SAP on the 2 affected RdSAP corpus certs. Surfaced by the new DB-round-trip gate. The prior _UNPERSISTED_ALLOWLIST justification ("deliberate — full-SAP only; RdSAP re-model uses default U-values") was stale: the cascade DOES honour the lodged U on the API path. Add wall_u_value to EpcBuildingPartModel, write in from_domain, reconstruct in _to_building_part, drop the allowlist entry. Verified: certs 39036600 / 38137278 round-trip 72.43 / 69.11 exactly. Needs an added FE column (epc_building_part.wall_u_value, double precision NULL) on assessment-model#443. Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/postgres/epc_property_table.py | 6 ++++++ repositories/epc/epc_postgres_repository.py | 1 + .../repositories/epc/test_epc_persistence_field_coverage.py | 1 - 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/infrastructure/postgres/epc_property_table.py b/infrastructure/postgres/epc_property_table.py index c08e6c8c5..2cbe8c932 100644 --- a/infrastructure/postgres/epc_property_table.py +++ b/infrastructure/postgres/epc_property_table.py @@ -680,6 +680,11 @@ class EpcBuildingPartModel(SQLModel, table=True): # 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) + # #1665: lodged wall U (gov-EPC `wall_u_value`), authoritative over the + # derived default when present; dropped it fell back to the age-band default + # (-2.32 SAP). The prior allowlist "RdSAP uses defaults" was stale — the + # cascade does honour the lodged U on the API path. + wall_u_value: Optional[float] = 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) @@ -740,6 +745,7 @@ class EpcBuildingPartModel(SQLModel, table=True): roof_insulation_thickness=part.roof_insulation_thickness, rafter_insulation_thickness=part.rafter_insulation_thickness, wall_is_basement=part.wall_is_basement, + wall_u_value=part.wall_u_value, 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 e4d16577f..b53fe81a1 100644 --- a/repositories/epc/epc_postgres_repository.py +++ b/repositories/epc/epc_postgres_repository.py @@ -1185,6 +1185,7 @@ class EpcPostgresRepository(EpcRepository): roof_insulation_thickness=bp.roof_insulation_thickness, rafter_insulation_thickness=bp.rafter_insulation_thickness, wall_is_basement=bp.wall_is_basement, + wall_u_value=bp.wall_u_value, # Reconstruct from the epc_room_in_roof child table (full geometry # incl. detailed_surfaces); the flat room_in_roof_* columns on # epc_building_part are superseded and dropped in a follow-up (#1664). diff --git a/tests/repositories/epc/test_epc_persistence_field_coverage.py b/tests/repositories/epc/test_epc_persistence_field_coverage.py index 440179dd6..a43ba20e7 100644 --- a/tests/repositories/epc/test_epc_persistence_field_coverage.py +++ b/tests/repositories/epc/test_epc_persistence_field_coverage.py @@ -59,7 +59,6 @@ _UNPERSISTED_ALLOWLIST: dict[str, str] = { # `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)", } From eba2142cdffa1f33e4ab501badda7365ee8cc18e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 23 Jul 2026 09:23:26 +0000 Subject: [PATCH 7/8] =?UTF-8?q?Add=20the=20DB-round-trip=20corpus=20gate?= =?UTF-8?q?=20that=20closes=20the=20blind=20spot=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The accuracy corpus scores straight off the mapped fixture and never saves, so it is structurally blind to any field the calculator reads but the schema drops (#1665). This gate maps -> saves to the ephemeral test DB -> reads back -> re-scores every corpus cert and asserts the reloaded SAP equals the in-memory SAP to 1e-9. It would have caught cylinder_heat_loss / room-in-roof / roof-windows / wall_u_value on day one; a new drop now fails here immediately. Save+read+rollback per cert keeps the graph off disk (bounded, ~18s over 1000 certs). One documented, precise skip: the pre-#1666 decimal main_heating_fraction cohort, whose INTEGER column truncates the raw decimal on save (0.8 -> 1) — fixed upstream by #1666 (mapper normalisation, on #1672). Remove the skip once #1672 is in main so the gate asserts a hard zero. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../epc/test_epc_roundtrip_corpus_gate.py | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 tests/repositories/epc/test_epc_roundtrip_corpus_gate.py diff --git a/tests/repositories/epc/test_epc_roundtrip_corpus_gate.py b/tests/repositories/epc/test_epc_roundtrip_corpus_gate.py new file mode 100644 index 000000000..700e94cf4 --- /dev/null +++ b/tests/repositories/epc/test_epc_roundtrip_corpus_gate.py @@ -0,0 +1,97 @@ +"""Durable DB-round-trip gate for the persistence blind spot (#1665). + +Production scores a dwelling AFTER a Postgres save+reload; the accuracy corpus +(`test_sap_accuracy_corpus.py`) scores straight off the mapped fixture and never +saves, so it is structurally blind to any field the calculator reads but the +schema drops. This gate closes that hole: every corpus cert is mapped, saved to +the ephemeral test DB, read back, and re-scored — and the reloaded SAP must equal +the in-memory SAP to 1e-9. Any field the calculator reads but the schema drops +makes the two diverge, so a new persistence blind spot fails here immediately +instead of being found by hand months later (it would have caught +cylinder_heat_loss / room-in-roof / roof-windows / wall_u_value on day one). + +Known, documented exception — the `main_heating_fraction` DECIMAL cohort. The +gov-EPC API lodges a two-main split as either a decimal fraction (`[0.8, 0.2]`) +or a percent (`[80, 20]`); the `epc_main_heating_detail.main_heating_fraction` +column is INTEGER, so a decimal pair truncates on save (`0.8 -> 1`). The real fix +is #1666 (normalise to percent AT THE MAPPER, before save) which lands on the +SAP-accuracy branch (#1672). Until #1672 is in `main`, those certs cannot +round-trip; they are skipped here via a precise predicate (a non-integer lodged +fraction). Once #1672 merges and this branch rebases, delete `_is_pre_1666_*` +and the skip so the gate asserts a hard zero. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +from sqlalchemy import Engine +from sqlmodel import Session + +from datatypes.epc.domain.epc_property_data import EpcPropertyData +from datatypes.epc.domain.mapper import EpcPropertyDataMapper +from domain.sap10_calculator.calculator import Sap10Calculator +from repositories.epc.epc_postgres_repository import EpcPostgresRepository + +_CORPUS = ( + Path(__file__).resolve().parents[3] + / "backend/epc_api/json_samples/RdSAP-Schema-21.0.1/corpus.jsonl" +) +_TOLERANCE = 1e-9 + + +def _is_pre_1666_decimal_fraction(epc: EpcPropertyData) -> bool: + """True when a lodged main_heating_fraction is non-integer, so the INTEGER + column truncates it on save. #1666 (mapper normalisation) removes this; + remove this predicate + skip once #1672 is in main.""" + for main in epc.sap_heating.main_heating_details: + frac = main.main_heating_fraction + if frac is not None and float(frac) != int(frac): + return True + return False + + +def test_round_trip_preserves_sap_over_the_corpus(db_engine: Engine) -> None: + # Arrange + calc = Sap10Calculator() + lines = _CORPUS.read_text().splitlines() + + # Act — map -> save -> reload -> re-score every cert, collecting any drift. + diverged: list[str] = [] + skipped_pre_1666 = 0 + for line in lines: + raw: dict[str, Any] = json.loads(line) + epc = EpcPropertyDataMapper.from_api_response(raw) + if epc is None: + continue + if _is_pre_1666_decimal_fraction(epc): + skipped_pre_1666 += 1 + continue + try: + direct = calc.calculate(epc).sap_score_continuous + except Exception: + continue + # Save -> flush (assigns FKs, writes the graph within the txn) -> read + # back in the SAME session -> roll back, so the round-trip exercises the + # real save/reconstruct without accumulating 1000 graphs on disk. + with Session(db_engine) as session: + repo = EpcPostgresRepository(session) + epc_property_id = repo.save(epc) + session.flush() + reloaded = repo.get(epc_property_id) + session.rollback() + reloaded_sap = calc.calculate(reloaded).sap_score_continuous + if abs(reloaded_sap - direct) > _TOLERANCE: + diverged.append( + f"uprn {raw.get('uprn')}: in-memory {direct:.6f} != reloaded " + f"{reloaded_sap:.6f} (Δ {reloaded_sap - direct:+.4f})" + ) + + # Assert — no field the calculator reads is dropped on the DB round-trip. + assert not diverged, ( + f"{len(diverged)} cert(s) lost calculator-read fields on the DB round-trip " + f"({skipped_pre_1666} pre-#1666 decimal-fraction certs skipped):\n" + + "\n".join(diverged[:20]) + ) From 11ea2c20776fce865a6f661a3b330d49ee5a6d02 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 23 Jul 2026 09:30:03 +0000 Subject: [PATCH 8/8] =?UTF-8?q?Assert=20a=20hard=20zero=20on=20the=20round?= =?UTF-8?q?-trip=20gate=20now=20#1666=20has=20landed=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1672 (which includes #1666's main_heating_fraction normalisation at the mapper) is merged into main. The decimal-fraction cohort now maps to integer percent before save, so the INTEGER column no longer truncates and all 1000 corpus certs round-trip their SAP to 1e-9. Remove the temporary _is_pre_1666_decimal_fraction skip so the gate asserts a hard zero over the whole corpus. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../epc/test_epc_roundtrip_corpus_gate.py | 29 ++----------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/tests/repositories/epc/test_epc_roundtrip_corpus_gate.py b/tests/repositories/epc/test_epc_roundtrip_corpus_gate.py index 700e94cf4..bbf8d2bd3 100644 --- a/tests/repositories/epc/test_epc_roundtrip_corpus_gate.py +++ b/tests/repositories/epc/test_epc_roundtrip_corpus_gate.py @@ -10,15 +10,7 @@ makes the two diverge, so a new persistence blind spot fails here immediately instead of being found by hand months later (it would have caught cylinder_heat_loss / room-in-roof / roof-windows / wall_u_value on day one). -Known, documented exception — the `main_heating_fraction` DECIMAL cohort. The -gov-EPC API lodges a two-main split as either a decimal fraction (`[0.8, 0.2]`) -or a percent (`[80, 20]`); the `epc_main_heating_detail.main_heating_fraction` -column is INTEGER, so a decimal pair truncates on save (`0.8 -> 1`). The real fix -is #1666 (normalise to percent AT THE MAPPER, before save) which lands on the -SAP-accuracy branch (#1672). Until #1672 is in `main`, those certs cannot -round-trip; they are skipped here via a precise predicate (a non-integer lodged -fraction). Once #1672 merges and this branch rebases, delete `_is_pre_1666_*` -and the skip so the gate asserts a hard zero. +Save+read+rollback per cert keeps the graph off disk (bounded, ~18s / 1000 certs). """ from __future__ import annotations @@ -30,7 +22,6 @@ from typing import Any from sqlalchemy import Engine from sqlmodel import Session -from datatypes.epc.domain.epc_property_data import EpcPropertyData from datatypes.epc.domain.mapper import EpcPropertyDataMapper from domain.sap10_calculator.calculator import Sap10Calculator from repositories.epc.epc_postgres_repository import EpcPostgresRepository @@ -42,17 +33,6 @@ _CORPUS = ( _TOLERANCE = 1e-9 -def _is_pre_1666_decimal_fraction(epc: EpcPropertyData) -> bool: - """True when a lodged main_heating_fraction is non-integer, so the INTEGER - column truncates it on save. #1666 (mapper normalisation) removes this; - remove this predicate + skip once #1672 is in main.""" - for main in epc.sap_heating.main_heating_details: - frac = main.main_heating_fraction - if frac is not None and float(frac) != int(frac): - return True - return False - - def test_round_trip_preserves_sap_over_the_corpus(db_engine: Engine) -> None: # Arrange calc = Sap10Calculator() @@ -60,15 +40,11 @@ def test_round_trip_preserves_sap_over_the_corpus(db_engine: Engine) -> None: # Act — map -> save -> reload -> re-score every cert, collecting any drift. diverged: list[str] = [] - skipped_pre_1666 = 0 for line in lines: raw: dict[str, Any] = json.loads(line) epc = EpcPropertyDataMapper.from_api_response(raw) if epc is None: continue - if _is_pre_1666_decimal_fraction(epc): - skipped_pre_1666 += 1 - continue try: direct = calc.calculate(epc).sap_score_continuous except Exception: @@ -91,7 +67,6 @@ def test_round_trip_preserves_sap_over_the_corpus(db_engine: Engine) -> None: # Assert — no field the calculator reads is dropped on the DB round-trip. assert not diverged, ( - f"{len(diverged)} cert(s) lost calculator-read fields on the DB round-trip " - f"({skipped_pre_1666} pre-#1666 decimal-fraction certs skipped):\n" + f"{len(diverged)} cert(s) lost calculator-read fields on the DB round-trip:\n" + "\n".join(diverged[:20]) )