From 678aa7affd2d0b80ed4b83c6fc725884d47f1ece Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Sat, 6 Jun 2026 18:27:41 +0000 Subject: [PATCH] fix(cascade): main-roof U ignores Room-in-Roof "no insulation" leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main pitched/flat roof U-value was derived from the JOINED text of every roofs[] entry. A room-in-roof carries its own §3.9/§3.10 shell area + U-value cascade (Table 17 / Table 18 col 4), so a multi-roof cert lodged "Pitched, insulated (assumed) | Roof room(s), no insulation (assumed)" leaked the RR's "no insulation" marker into the main roof's u_roof → U=2.30 applied to the WHOLE main roof, ~3x over-stating its heat loss. This is the 4700-family regular-roof-U leak. `_joined_main_roof_descriptions` drops "Roof room(s)" entries before the main-roof u_roof, falling back to the unfiltered join only for pure-RR dwellings (every entry an RR) to preserve their prior behaviour. The RR shell U is unaffected (computed separately) — golden 6035 stays green. RR-leak cluster (18 certs, RR "no insulation" + a non-RR primary roof): mean |err| 6.14 → 4.85, within-1.0 0 → 8, within-0.5 0 → 3. Eval headline 44.8% → 44.9%, mean |err| 1.851 → 1.824, mean signed -0.152 → -0.081. Two certs overshoot (other residuals the leak was masking); the spec rule is applied uniformly. Co-Authored-By: Claude Opus 4.8 --- .../worksheet/heat_transmission.py | 28 ++++++++++++- .../worksheet/test_heat_transmission.py | 39 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/domain/sap10_calculator/worksheet/heat_transmission.py b/domain/sap10_calculator/worksheet/heat_transmission.py index 5729fd9a..f90593ec 100644 --- a/domain/sap10_calculator/worksheet/heat_transmission.py +++ b/domain/sap10_calculator/worksheet/heat_transmission.py @@ -344,6 +344,32 @@ def _joined_descriptions(elements: list[Any]) -> Optional[str]: return " | ".join(parts) +def _joined_main_roof_descriptions(roofs: list[Any]) -> Optional[str]: + """Join roof descriptions for the MAIN (non-RR) roof U-value, dropping + "Roof room(s)" entries. + + A room-in-roof carries its own §3.9/§3.10 shell area + U-value cascade + (Table 17 / Table 18 col 4), so a "Roof room(s), no insulation + (assumed)" lodgement must NOT leak into the main pitched/flat roof's + `u_roof`. Without this filter a multi-roof cert like "Pitched, + insulated (assumed) | Roof room(s), no insulation (assumed)" applies + the RR's "no insulation" 2.30 to the WHOLE main roof, ~3x over-stating + its heat loss (the 4700-family regular-roof-U leak). + + Falls back to the unfiltered join when every roof entry is a Room-in- + Roof (pure-RR dwelling) so that case keeps its prior behaviour.""" + if not roofs: + return None + parts = [ + d + for e in roofs + if (d := getattr(e, "description", "")) and "roof room" not in d.lower() + ] + if not parts: + return _joined_descriptions(roofs) + return " | ".join(parts) + + def _part_geometry(part: SapBuildingPart) -> dict[str, float]: if not part.sap_floor_dimensions: # A part with no floor dimensions has no derivable RR shell or @@ -559,7 +585,7 @@ def heat_transmission_from_cert( return HeatTransmission(0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0) country = Country.from_code(epc.country_code) - roof_description = _joined_descriptions(epc.roofs) + roof_description = _joined_main_roof_descriptions(epc.roofs) wall_description = _joined_descriptions(epc.walls) floor_description = _joined_descriptions(epc.floors) diff --git a/tests/domain/sap10_calculator/worksheet/test_heat_transmission.py b/tests/domain/sap10_calculator/worksheet/test_heat_transmission.py index bb55b94a..71dd4197 100644 --- a/tests/domain/sap10_calculator/worksheet/test_heat_transmission.py +++ b/tests/domain/sap10_calculator/worksheet/test_heat_transmission.py @@ -36,12 +36,51 @@ from domain.sap10_calculator.worksheet.heat_transmission import ( heat_transmission_from_cert, ) from domain.sap10_calculator.worksheet.heat_transmission import ( + _joined_main_roof_descriptions, # pyright: ignore[reportPrivateUsage] _part_geometry, # pyright: ignore[reportPrivateUsage] _round_half_up, # pyright: ignore[reportPrivateUsage] _window_bp_index, # pyright: ignore[reportPrivateUsage] ) +class _Desc: + """Minimal stand-in for a roof element carrying a `description`.""" + + def __init__(self, description: str) -> None: + self.description = description + + +def test_joined_main_roof_descriptions_drops_room_in_roof_entries() -> None: + # Arrange — a multi-roof cert: main pitched roof (insulated) plus a + # Room-in-Roof lodged uninsulated. The RR has its own shell U cascade, + # so the main-roof U-value description must NOT inherit the RR's + # "no insulation" marker (which would force the whole main roof to + # U=2.30). Cert 8536-0624-4600-0934-1292. + roofs = [ + _Desc("Pitched, insulated (assumed)"), + _Desc("Roof room(s), no insulation (assumed)"), + ] + + # Act + result = _joined_main_roof_descriptions(roofs) + + # Assert — only the non-RR primary roof remains. + assert result == "Pitched, insulated (assumed)" + + +def test_joined_main_roof_descriptions_keeps_pure_rr_fallback() -> None: + # Arrange — a pure room-in-roof dwelling (every roof entry is an RR): + # filtering would leave nothing, so preserve prior behaviour by + # falling back to the unfiltered join. + roofs = [_Desc("Roof room(s), no insulation (assumed)")] + + # Act + result = _joined_main_roof_descriptions(roofs) + + # Assert + assert result == "Roof room(s), no insulation (assumed)" + + def test_part_geometry_floorless_part_honours_full_key_contract() -> None: # Arrange — a building part lodged with NO sap_floor_dimensions (e.g. # a party-wall-only or RR-only extension; observed on 5 certs in a