From bd84a2f19025334dc60ea76ed12424f958a6ab88 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 24 Jul 2026 12:43:19 +0000 Subject: [PATCH] =?UTF-8?q?Close=20the=20#1639=20residual=20PasHub=20LRHA?= =?UTF-8?q?=20WAVE=203=20mapper=20blocks=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clears the three non-table-add blocks tracked in #1639 plus a latent age-band bug they surfaced. LRHA WAVE 3 cohort: 77 → 103 of 104 computing. Block A (exact cylinder volume): a measured cylinder lodges its litres on a separate "Exact cylinder volume:" line; parse it in the extractor, carry it on WaterHeating.cylinder_volume_measured_l, route the "Exact cylinder volume" band to SAP cylinder-size code 6, and thread the litres into SapHeating (the calculator reads them only when the code is 6). Verified end-to-end on the real 166 L fixture. Block B (Controls line-wrap): PasHub wraps a long "Controls:" value across two PDF lines and _get_in read only the first, truncating the label so the mapper missed its Table 4e code. Re-join the continuation when the value ends on a conjunction/comma (grammatically incomplete) so "…room thermostat and TRVs" matches the existing code 2314. Scoped to Controls: — the shared _get_in is unchanged; a wider truncation audit is tracked separately. Age-band normalisation: the heat_network_age_band '2007 - 2011' block was one facet of a broader bug — PasHub lodges the construction age band in mixed templates ("K: …" prefixed, tight "1991-1995", space-padded "2007 - 2011") and only the letter form was normalised, so bare year ranges were SILENTLY MIS-RATED by the age-band-keyed U-value/DLF cascades. Normalise every template to its RdSAP letter in _extract_age_band: unblocks the heat-network dwellings AND corrects ~30 already-computing fixtures (within-0.5 42.9% → 49.5%, MAE 3.05 → 1.02). Sibling Guinness cohort unchanged (82.9% / 0.36). Block C (gable 'None') was already resolved on main — no change needed. Fixture 461178278096 lodges no main heating at all (MissingMainFuelType); tolerated as a known gap in the harness and ticketed (#1680) for the modelling decision. LRHA ratchets re-baselined to the corrected cohort (0.49 / 1.05). Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/documents_parser/extractor.py | 28 +++++++- .../documents_parser/tests/test_extractor.py | 66 +++++++++++++++++++ .../test_pashub_sap_accuracy_lrha_wave3.py | 37 +++++++++-- datatypes/epc/domain/mapper.py | 49 +++++++++++++- .../epc/domain/tests/test_from_site_notes.py | 50 +++++++++++++- .../epc/surveys/pashub_rdsap_site_notes.py | 3 + 6 files changed, 224 insertions(+), 9 deletions(-) diff --git a/backend/documents_parser/extractor.py b/backend/documents_parser/extractor.py index c9f26e254..dae4ea64e 100644 --- a/backend/documents_parser/extractor.py +++ b/backend/documents_parser/extractor.py @@ -605,6 +605,26 @@ class PasHubRdSapSiteNotesExtractor: ), ) + def _controls_value(self, data: List[str]) -> str: + """The main-heating "Controls:" value, re-joining a line-wrapped + continuation. PasHub wraps a long controls string across two PDF lines + (e.g. "...room thermostat and" / "TRVs"); `_get_in` reads only the first + token, truncating the label so the mapper misses its Table 4e control + code (#1639 block B — the community-heating charging controls). A value + ending in a conjunction or comma is grammatically incomplete, so the + next token is its continuation; a value ending on a noun is complete and + left untouched. Scoped to "Controls:" — the shared `_get_in` single-token + read is unchanged (a wider truncation audit is tracked separately).""" + try: + idx = data.index("Controls:") + except ValueError: + return "" + value = data[idx + 1].strip() if idx + 1 < len(data) else "" + continuation = data[idx + 2].strip() if idx + 2 < len(data) else "" + if continuation and value.endswith((" and", " or", ",")): + value = f"{value} {continuation}" + return value + def _parse_main_heating(self, data: List[str]) -> MainHeating: return MainHeating( selection_method=self._get_in( @@ -635,7 +655,7 @@ class PasHubRdSapSiteNotesExtractor: status=self._get_in(data, "Status") or "", central_heating_pump_age=self._get_in(data, "Central heating pump age:") or "", - controls=self._get_in(data, "Controls:") or "", + controls=self._controls_value(data), flue_gas_heat_recovery_system=self._bool_in( data, "Does the boiler have a Flue Gas Heat Recover", offset=2 ), @@ -660,10 +680,16 @@ class PasHubRdSapSiteNotesExtractor: def _parse_water_heating(self, data: List[str]) -> WaterHeating: thickness_raw = self._get_in(data, "Insulation Thickness (mm):") or self._get_in(data, "Thickness:") thickness_mm = int(thickness_raw.split()[0]) if thickness_raw else None + # A measured cylinder lodges its litres on a separate "Exact cylinder + # volume:" line ("166 litres" → 166), distinct from the "Cylinder Size:" + # band value. Same token-split pattern as the thickness lines above. + volume_raw = self._get_in(data, "Exact cylinder volume:") + cylinder_volume_measured_l = int(volume_raw.split()[0]) if volume_raw else None return WaterHeating( type=self._get_in(data, "Water Heating Type:") or "", system=self._get_in(data, "Water Heating System:") or "", cylinder_size=self._get_in(data, "Cylinder Size:") or "", + cylinder_volume_measured_l=cylinder_volume_measured_l, cylinder_measured_heat_loss=self._get_in( data, "Cylinder Measured Heat Loss:" ), diff --git a/backend/documents_parser/tests/test_extractor.py b/backend/documents_parser/tests/test_extractor.py index a855d8016..7e2d79530 100644 --- a/backend/documents_parser/tests/test_extractor.py +++ b/backend/documents_parser/tests/test_extractor.py @@ -537,6 +537,72 @@ class TestWaterHeatingCylinderThickness: assert hhw.water_heating.cylinder_size == "Normal (90-130 litres)" +class TestExactCylinderVolume: + """When the surveyor measures the cylinder, PasHub lodges the litres on a + SEPARATE "Exact cylinder volume:" line (the "Cylinder Size:" value is the + band label "Exact cylinder volume"). Token order confirmed against the real + extractor on fixture 461178278096: "Cylinder Size:" -> "Exact cylinder + volume", then "Exact cylinder volume:" -> "166 litres". The numeric value + is what the calculator reads for SAP cylinder-size code 6 (RdSAP 10 §10.5).""" + + @staticmethod + def _parse(section: list[str]) -> WaterHeating: + return PasHubRdSapSiteNotesExtractor([])._parse_water_heating(section) + + def test_exact_cylinder_volume_parsed(self) -> None: + wh = self._parse( + [ + "Water Heating System:", + "Electric immersion", + "Cylinder Size:", + "Exact cylinder volume", + "Exact cylinder volume:", + "166 litres", + ] + ) + assert wh.cylinder_size == "Exact cylinder volume" + assert wh.cylinder_volume_measured_l == 166 + + def test_exact_cylinder_volume_absent_on_banded_cylinder(self) -> None: + wh = self._parse(["Cylinder Size:", "Normal (90-130 litres)"]) + assert wh.cylinder_volume_measured_l is None + + +class TestControlsLineWrap: + """PasHub wraps a long main-heating "Controls:" value across two PDF lines + (e.g. "...room thermostat and" / "TRVs"); `_get_in` reads only the first, + truncating the label so the mapper misses its heat-network control code + (#1639 block B). A value ending in a conjunction/comma is grammatically + incomplete → the next token is its continuation, so re-join it. A complete + value (ends on a noun) is left alone.""" + + @staticmethod + def _controls(section: list[str]) -> str: + return PasHubRdSapSiteNotesExtractor([])._parse_main_heating(section).controls + + def test_wrapped_community_controls_rejoined(self) -> None: + section = [ + "System type:", + "Community heating system", + "Controls:", + "Charging system linked to use of community heating, room thermostat and", + "TRVs", + "Secondary Heating System", + ] + assert self._controls(section) == ( + "Charging system linked to use of community heating, " + "room thermostat and TRVs" + ) + + def test_complete_controls_not_joined(self) -> None: + section = [ + "Controls:", + "Programmer, room thermostat and TRVs", + "Does the boiler have a Flue Gas Heat Recover", + ] + assert self._controls(section) == "Programmer, room thermostat and TRVs" + + class TestImmersionType: @pytest.fixture def hhw(self) -> HeatingAndHotWater: diff --git a/backend/documents_parser/tests/test_pashub_sap_accuracy_lrha_wave3.py b/backend/documents_parser/tests/test_pashub_sap_accuracy_lrha_wave3.py index d734b17d7..628b46555 100644 --- a/backend/documents_parser/tests/test_pashub_sap_accuracy_lrha_wave3.py +++ b/backend/documents_parser/tests/test_pashub_sap_accuracy_lrha_wave3.py @@ -26,10 +26,20 @@ import pytest from backend.documents_parser.parser import parse_site_notes_pdf from datatypes.epc.domain.mapper import UnmappedPasHubLabel from domain.sap10_calculator.calculator import Sap10Calculator -from domain.sap10_calculator.exceptions import UnmappedSapCode +from domain.sap10_calculator.exceptions import MissingMainFuelType, UnmappedSapCode -# Same known in-progress main-heating-control mapper gap as the Guinness module. -_KNOWN_GAPS: tuple[type[Exception], ...] = (UnmappedSapCode, UnmappedPasHubLabel) +# Known, separately-tracked gaps that block a fixture rather than mis-rate it. +# `MissingMainFuelType`: one LRHA WAVE 3 fixture (461178278096) lodges NO main +# heating system at all — only an electric-immersion cylinder — so there is no +# fuel to rate. Modelling a "no main heating" dwelling is a policy decision +# tracked in its own follow-up; until then it fails loud at the boundary rather +# than inventing a heating system (ADR-0015). Surfaced once #1639 block A +# unmasked it (the fixture previously blocked on the cylinder label). +_KNOWN_GAPS: tuple[type[Exception], ...] = ( + UnmappedSapCode, + UnmappedPasHubLabel, + MissingMainFuelType, +) _FIXTURES_DIR = Path(__file__).parent / "fixtures" / "pashub_accuracy_lrha_wave3" _MANIFEST_PATH = _FIXTURES_DIR / "manifest.json" @@ -61,11 +71,28 @@ _MANIFEST_PATH = _FIXTURES_DIR / "manifest.json" # controls label ×6 (extractor fix, tracked separately), heat_network_age_band # '2007 - 2011' ×4 (calculator cascade dict, surfaced behind the biomass # fuel), room-in-roof gable 'None' ×1. +# 2026-07-24 (#1639): closed the residual non-table-add blocks. Block A (exact +# cylinder volume — extractor parse of "Exact cylinder volume:" + dataclass +# field + SAP cylinder-size code 6) and block B (re-join the line-wrapped +# "Controls:" continuation so "…room thermostat and TRVs" matches the existing +# code 2314) unblocked their fixtures. The heat_network_age_band '2007 - 2011' +# block turned out to be one facet of a broader latent bug: PasHub lodges the +# construction age band in mixed templates (letter-prefixed "K: …", tight +# "1991-1995", space-padded "2007 - 2011"), and only the letter form was being +# normalised — the bare year ranges flowed through un-normalised and were +# SILENTLY MIS-RATED by the age-band-keyed U-value/DLF cascades. Normalising +# every template to its RdSAP letter in `_extract_age_band` both unblocked the +# heat-network dwellings AND corrected ~30 already-computing fixtures → +# **103 computed / 1 blocked** (within-0.5 42.9% → 49.5%, MAE 3.05 → 1.02 — a +# real accuracy correction, not tuning). The 1 residual block is fixture +# 461178278096, which lodges no main heating at all (`MissingMainFuelType`, +# tolerated above pending a modelling decision). Floor/ceiling re-baselined to +# the corrected cohort. # Treat the constants as a regression tripwire, NOT a cohort accuracy figure; # re-baseline (coverage growth is not loosening) as further mapper fixes unblock # fixtures. -_MIN_WITHIN_HALF: float = 0.41 -_MAX_SAP_MAE: float = 3.0 +_MIN_WITHIN_HALF: float = 0.49 +_MAX_SAP_MAE: float = 1.05 _KNOWN_GAP_REASON = ( "pashub `from_site_notes` mapper does not int-code a main-heating " diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index a487f10de..3f8cfb6f9 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -6340,9 +6340,43 @@ def _is_floor_above_partially_heated_space(location: Optional[str]) -> bool: return "above partially heated" in location.lower() +# RdSAP 10 Table S1 England & Wales construction-age bands → letter code. Keyed +# by the year range with spaces around the dash collapsed and lower-cased, so +# both PasHub templates ("2007 - 2011" space-padded, "1991-1995" tight) resolve. +_AGE_RANGE_TO_BAND_LETTER: Dict[str, str] = { + "before 1900": "A", + "1900-1929": "B", + "1930-1949": "C", + "1950-1966": "D", + "1967-1975": "E", + "1976-1982": "F", + "1983-1990": "G", + "1991-1995": "H", + "1996-2002": "I", + "2003-2006": "J", + "2007-2011": "K", + "2012-2021": "L", + "2012 onwards": "L", + "2022 onwards": "M", + "2023 onwards": "M", +} + + def _extract_age_band(age_range: str) -> str: - """Return the letter code from a site-notes age range, e.g. 'I: 1996 - 2002' → 'I'.""" - return age_range.split(":")[0].strip() + """Return the RdSAP letter code from a site-notes age range. + + Two PasHub templates occur: the letter-prefixed "K: 2007 - 2011" (take the + letter) and a bare year range — either space-padded "2007 - 2011" or tight + "1991-1995" — which is normalised to its letter via the RdSAP band table. + The calculator keys its age-band cascades (heat-network DLF, U-values) on + the letter, so a stray year range strict-raises deep in the calculator + (#1639 — heat_network_age_band '2007 - 2011'). An unrecognised value is + returned unchanged (preserving the prior fall-through).""" + head = age_range.split(":")[0].strip() + if len(head) == 1 and head.isalpha(): + return head.upper() + key = re.sub(r"\s*-\s*", "-", head).lower() + return _AGE_RANGE_TO_BAND_LETTER.get(key, head) def _map_floor_dimensions( @@ -6904,6 +6938,14 @@ def _map_sap_heating( if cylinder_present else None ), + # Measured litres for the "Exact cylinder volume" band (code 6); the + # calculator reads it only when `cylinder_size == 6`. Mirrors the + # gov-API path (`cylinder_volume_measured_l=...cylinder_size_measured`). + cylinder_volume_measured_l=( + heating.water_heating.cylinder_volume_measured_l + if cylinder_present + else None + ), cylinder_insulation_type=_pashub_cylinder_insulation_type_code( heating.water_heating.insulation_type, cylinder_present ), @@ -7020,6 +7062,9 @@ _PASHUB_CYLINDER_SIZE_TO_SAP10: Dict[str, int] = { "Normal (90-130 litres)": 2, "Medium (131-170 litres)": 3, # Table 28 Medium → fixed 160 L "Large (>170 litres)": 4, + # Surveyor measured the cylinder: code 6 (Exact) reads the litres carried + # on `WaterHeating.cylinder_volume_measured_l`, not a Table 28 band volume. + "Exact cylinder volume": 6, } diff --git a/datatypes/epc/domain/tests/test_from_site_notes.py b/datatypes/epc/domain/tests/test_from_site_notes.py index 07b93ff5f..90698e53f 100644 --- a/datatypes/epc/domain/tests/test_from_site_notes.py +++ b/datatypes/epc/domain/tests/test_from_site_notes.py @@ -20,7 +20,11 @@ from datatypes.epc.domain.epc_property_data import ( ShowerOutlet, ShowerOutlets, ) -from datatypes.epc.domain.mapper import EpcPropertyDataMapper, UnmappedPasHubLabel +from datatypes.epc.domain.mapper import ( + EpcPropertyDataMapper, + UnmappedPasHubLabel, + _extract_age_band, +) from datatypes.epc.schema.tests.helpers import from_dict from datatypes.epc.surveys.pashub_rdsap_site_notes import PasHubRdSapSiteNotes @@ -838,6 +842,31 @@ class TestSystemBuildWallIsNotBasement: assert part.main_wall_is_basement is False +class TestAgeBandNormalisation: + """`_extract_age_band` must yield an RdSAP letter for every PasHub age-range + template. The letter-prefixed form ("K: 2007 - 2011") takes its letter; a + bare year range ("2007 - 2011" or the no-space "1991-1995") is normalised to + its letter via the RdSAP band table. The calculator keys age-band cascades + (heat-network DLF, U-values) on the letter, so a stray year range strict- + raises deep in the calculator (#1639 — heat_network_age_band '2007 - 2011').""" + + @pytest.mark.parametrize( + "age_range, expected_letter", + [ + ("K: 2007 - 2011", "K"), # letter-prefixed form + ("2007 - 2011", "K"), # bare, space-padded dash (the blocking form) + ("1991-1995", "H"), # bare, no-space dash + ("1996-2002", "I"), + ("before 1900", "A"), + ("H", "H"), # already a letter + ], + ) + def test_normalises_to_letter( + self, age_range: str, expected_letter: str + ) -> None: + assert _extract_age_band(age_range) == expected_letter + + class TestCylinderSizeCoding: """The surveyed §Water-Heating "Cylinder Size" label must resolve to the SAP10 cylinder-size enum (`_cylinder_volume_l_from_code` int-or-nones a raw @@ -860,6 +889,7 @@ class TestCylinderSizeCoding: ("Medium (131-170 litres)", 3), # Table 28 Medium → fixed 160 L ("Large (>170 litres)", 4), ("No Access", 2), # fed from the (gas) main → Table 28 "otherwise" + ("Exact cylinder volume", 6), # Table 28 code 6 → measured litres ], ) def test_label_maps_to_sap_code(self, label: str, expected_code: int) -> None: @@ -872,6 +902,24 @@ class TestCylinderSizeCoding: # Assert assert result.sap_heating.cylinder_size == expected_code + def test_exact_volume_threads_measured_litres(self) -> None: + # Arrange — a measured cylinder: band "Exact cylinder volume" (→ code 6) + # plus the surveyed litres. `_cylinder_volume_l_from_code` reads the + # measured value only when the code is 6 (RdSAP 10 §10.5). + data = self._with_cylinder("Exact cylinder volume") + data["heating_and_hot_water"]["water_heating"][ + "cylinder_volume_measured_l" + ] = 166 + + # Act + result = EpcPropertyDataMapper.from_site_notes( + from_dict(PasHubRdSapSiteNotes, data) + ) + + # Assert + assert result.sap_heating.cylinder_size == 6 + assert result.sap_heating.cylinder_volume_measured_l == 166 + def test_no_cylinder_stays_absent(self) -> None: # Arrange — a combi dwelling: no cylinder lodged. survey = from_dict( diff --git a/datatypes/epc/surveys/pashub_rdsap_site_notes.py b/datatypes/epc/surveys/pashub_rdsap_site_notes.py index 988ef01ad..4a30a238d 100644 --- a/datatypes/epc/surveys/pashub_rdsap_site_notes.py +++ b/datatypes/epc/surveys/pashub_rdsap_site_notes.py @@ -255,6 +255,9 @@ class WaterHeating: type: str system: str cylinder_size: str + # Litres from the surveyor's "Exact cylinder volume:" line — read by the + # calculator only when `cylinder_size` codes to 6 (Exact, RdSAP 10 §10.5). + cylinder_volume_measured_l: Optional[int] = None cylinder_measured_heat_loss: Optional[str] = None insulation_type: Optional[str] = None insulation_thickness_mm: Optional[int] = None