From 41ded4740ed6b8ca72ca79286c8dcb7524595f31 Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Wed, 8 Jul 2026 13:05:43 +0000 Subject: [PATCH] Cascade raw API glazing_type on lodged sap_windows, not pass through raw datatypes/epc/domain/mapper.py's from_rdsap_schema_{17_0,17_1,18_0,19_0, 20_0_0,21_0_0} all stored a lodged sap_windows[].glazing_type verbatim instead of routing it through _api_cascade_glazing_type (which _api_sap_window, used by 21_0_1, already does correctly). The raw API code is the RdSAP-21 glazing enum (e.g. code 1 = "DG pre-2002"), not the SAP 10.2 Table 6b cascade enum the calculator's U/g-value tables are keyed on (cascade code 1 = single glazed) -- so any lodged window whose raw code happened to collide with a different cascade meaning was silently mis-rated. Found while validating PR #1503 (property 753950 / uprn 100021969385) against Elmhurst: our engine's windows_w_per_k was 82.27 vs Elmhurst's 51.44 -- the dwelling's larger window (91% of glazed area) lodges raw glazing_type=1 and was being modelled as single glazed instead of double, the dominant contributor to a SAP 42 vs Elmhurst 44 gap. Fixing this narrows windows_w_per_k to 50.26 (Elmhurst 51.44, ~2% off) and moves the engine's score to 43. Extracted the 5 duplicated reduced-schema-window blocks (17.0/17.1/18.0/ 19.0/20.0.0, which lodge a minimal window shape with no glazing_gap/ frame_factor/transmission fields) into a shared _reduced_field_api_sap_ window helper that also now populates window_transmission_details/ frame_factor via the SAP10 lookup instead of leaving them None. 21.0.0's richer inline block gets the same one-line cascade fix. +6 regression tests (one per affected schema seam + 21.0.0). Full domain/ schema/prediction/comparable-properties/sap10_calculator/modelling_e2e suites pass (2746 passed, only the 2 pre-existing unrelated failures); component-accuracy gate 26/26 unaffected. --- datatypes/epc/domain/mapper.py | 144 +++++++----------- .../domain/tests/test_from_rdsap_schema.py | 56 +++++++ 2 files changed, 108 insertions(+), 92 deletions(-) diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index 5a04e5165..76ef848af 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -728,22 +728,7 @@ class EpcPropertyDataMapper: # rich certs keep their lodged per-window geometry (used directly: # window_width = area, height = 1.0). sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_17_0_sap_windows(schema) ), @@ -1301,23 +1286,7 @@ class EpcPropertyDataMapper: cylinder_insulation_thickness_mm=schema.sap_heating.cylinder_insulation_thickness, ), sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - # ADR-0028: 14 rich certs lodge real per-window area. - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_17_1_sap_windows(schema) ), @@ -1508,26 +1477,7 @@ class EpcPropertyDataMapper: # glazed_area band + TFA (added in a later slice). The 10 rich # certs keep their lodged per-window geometry. sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - # ADR-0028: the 10 rich certs lodge a real per-window - # window_area (Measurement) -- use it directly as geometry - # (width = area, height = 1.0) rather than the placeholder - # windowless 0x0. - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_18_0_sap_windows(schema) ), @@ -1754,22 +1704,7 @@ class EpcPropertyDataMapper: # rich certs keep their lodged per-window geometry (used directly: # window_width = area, height = 1.0). sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_19_0_sap_windows(schema) ), @@ -2020,28 +1955,7 @@ class EpcPropertyDataMapper: # ADR-0027: 993/1000 omit sap_windows → synthesise from glazed_area # band + TFA. The 7 rich certs keep their lodged per-window geometry. sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - # ADR-0027: the 7 rich certs lodge a real per-window - # window_area (Measurement) — use it directly as - # geometry (width = area, height = 1.0, matching the - # synthesis convention) rather than the placeholder - # windowless 0x0 that modelled these data-richest certs - # as having no glazing at all. - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_20_0_0_sap_windows(schema) ), @@ -2289,7 +2203,11 @@ class EpcPropertyDataMapper: orientation=w.orientation, window_type=w.window_type, frame_factor=w.frame_factor, - glazing_type=w.glazing_type, + # See `_reduced_field_api_sap_window`: the raw API glazing + # code is the RdSAP-21 glazing enum, not the SAP 10.2 + # cascade enum the calculator's U/g-value tables are keyed + # on — must be canonicalised, not passed through raw. + glazing_type=_api_cascade_glazing_type(w.glazing_type), window_width=_measurement_value(w.window_width), window_height=_measurement_value(w.window_height), draught_proofed=w.draught_proofed == "true", @@ -5217,6 +5135,48 @@ def _api_sap_roof_window(w: Any) -> SapRoofWindow: ) +def _reduced_field_api_sap_window(w: Any) -> SapWindow: + """Build a `SapWindow` from one reduced-field API schema `sap_windows` + entry (RdSAP 17.0/17.1/18.0/19.0/20.0.0's minimal shape: orientation, + window_area, window_type, glazing_type, window_location — no glazing_gap, + frame_factor or window_transmission_details lodged). + + Mirrors `_api_sap_window`'s glazing-type cascade: the raw API + `glazing_type` must be canonicalised via `_api_cascade_glazing_type` + before storage — these codes are the RdSAP-21 glazing enum (e.g. code 1 + = "DG pre-2002"), NOT the SAP 10.2 Table 6b cascade enum the calculator's + `_G_LIGHT_BY_GLAZING_CODE`/U-value lookups are keyed on (cascade code 1 = + single glazed). Previously stored raw, this silently modelled any window + lodging API code 1 as single-glazed instead of double — cert + 0141-2860-6891-9124-5625 (uprn 100021969385, property 753950, RdSAP-16.2) + over-stated windows_w_per_k 82.27 vs Elmhurst's 51.44 (a 31 W/K gap, the + dominant single contributor to a SAP 42 vs Elmhurst 44 mismatch).""" + transmission = _api_glazing_transmission(w.glazing_type, 0) + return SapWindow( + frame_material=None, + glazing_gap=0, + orientation=w.orientation, + window_type=w.window_type, + glazing_type=_api_cascade_glazing_type(w.glazing_type), + window_width=_measurement_value(w.window_area), + window_height=1.0, + draught_proofed=False, + window_location=w.window_location, + window_wall_type=0, + permanent_shutters_present=False, + frame_factor=transmission[2] if transmission is not None else None, + window_transmission_details=( + WindowTransmissionDetails( + u_value=transmission[0], + data_source="SAP10 lookup (glazing_type, glazing_gap)", + solar_transmittance=transmission[1], + ) + if transmission is not None + else None + ), + ) + + def _api_sap_window(w: Any) -> SapWindow: """Build a `SapWindow` from one API schema sap_windows entry, routing the glazing-type + glazing-gap pair through the spec diff --git a/datatypes/epc/domain/tests/test_from_rdsap_schema.py b/datatypes/epc/domain/tests/test_from_rdsap_schema.py index f6ce46bfb..e6e8ed4ba 100644 --- a/datatypes/epc/domain/tests/test_from_rdsap_schema.py +++ b/datatypes/epc/domain/tests/test_from_rdsap_schema.py @@ -385,6 +385,25 @@ class TestFromRdSapSchema21_0_0: # photovoltaic_supply is None when the measured shape is present assert result.sap_energy_source.photovoltaic_supply is None + def test_lodged_window_glazing_type_is_cascaded_not_raw(self) -> None: + # Arrange — the fixture's own glazing_type (14) happens to be a + # coincident code (cascade(14) == 14), which would hide this bug. + # Override to code 1 ("DG pre-2002" in the RdSAP-21 glazing enum), + # which must canonicalise to cascade code 2 — passed through raw it + # would misread as cascade code 1 (single glazed). See the sibling + # `_reduced_field_api_sap_window` fix for the reduced-schema seams; + # this is the from_rdsap_schema_21_0_0 site of the same bug class. + data = load("21_0_0.json") + data["sap_windows"][0]["glazing_type"] = 1 + + schema = from_dict(RdSapSchema21_0_0, data) + + # Act + result = EpcPropertyDataMapper.from_rdsap_schema_21_0_0(schema) + + # Assert + assert result.sap_windows[0].glazing_type == 2 + def test_photovoltaic_array_orientation_nd_nulls_only_that_field(self) -> None: # Arrange — a 3-array dwelling where the middle array lodges the RdSAP # 'ND' ("Not Defined") sentinel for orientation. Regression for the @@ -2969,6 +2988,43 @@ def test_nd_glazing_without_single_signal_keeps_double_modal_default( assert all(w.glazing_type == 2 for w in result.sap_windows) +@pytest.mark.parametrize("schema_cls, mapper, fixture", _REDUCED_FIELD_SEAMS) +def test_lodged_sap_windows_glazing_type_is_cascaded_not_raw( + schema_cls: Any, mapper: Any, fixture: str +) -> None: + # Arrange — a real lodged `sap_windows[]` entry (the rich-cert path, not + # synthesis) whose raw API `glazing_type` code (1 = "DG pre-2002" in the + # RdSAP-21 glazing enum) differs from the SAP 10.2 cascade enum the + # calculator's U/g-value tables are keyed on (cascade code 1 = single + # glazed). Previously stored raw and unmodified — cert + # 0141-2860-6891-9124-5625 (uprn 100021969385, property 753950) lodges + # exactly this shape and was silently modelled as single glazed on its + # dominant window (91% of glazed area), overstating windows_w_per_k + # 82.27 vs Elmhurst's 51.44 W/K. + data = load(fixture) + data["sap_windows"] = [ + { + "orientation": 5, + "window_area": {"value": 18.66, "quantity": "square metres"}, + "window_type": 1, + "glazing_type": 1, + "window_location": 0, + } + ] + + # Act + result = mapper(from_dict(schema_cls, data)) + + # Assert — glazing_type is the CASCADED value (2), not the raw API code + # (1) that would misread as single glazed; transmission details/U-value + # are populated from the SAP10 lookup rather than left None. + assert result.sap_windows + window = result.sap_windows[0] + assert window.glazing_type == 2 + assert window.window_transmission_details is not None + assert window.window_transmission_details.u_value == pytest.approx(2.8) + + class TestReducedFieldNdGlazingResolver: """Unit-level pins for the shared ND resolver (schema-independent)."""