From b7fbbcca96c1ca5c65131ff1e9aaa1a49e2c2e4e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 28 May 2026 20:34:15 +0000 Subject: [PATCH] Slice S0380.51: strict-raise UnmappedApiCode on API integer enums MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the Elmhurst `UnmappedElmhurstLabel` coverage gate on the GOV.UK API path. The same failure mode (silently routing an unknown enum to a default / None hides cascade gaps until a downstream SAP- delta investigation surfaces them) was hitting the API mapper: existing helpers like `_api_floor_construction_str` returned None on unrecognised codes per the comment "Only the values observed across the 10 golden fixtures (1, 2) are mapped; unrecognised codes fall through to None." Adds `UnmappedApiCode(ValueError)` at the API mapper boundary and threads it through five strict helpers: - `_api_party_wall_construction_int` (RdSAP10 Table 15) - `_api_floor_construction_str` (Slice 88 floor signal) - `_api_floor_type_str` (RdSAP10 §5 rule (12)) - `_api_roof_construction_str` (Slice 89 cos(30°) factor) - `_api_sheltered_sides` (SAP10.2 §S5) Each helper distinguishes: - "lodging absent" → return None (unchanged behaviour) - "lodging present and mapped" → translate (unchanged behaviour) - "lodging present but unrecognised" → raise UnmappedApiCode (NEW) Two coverage gaps surfaced immediately at strict-run, both fixed in the same slice with the worksheet-backed lodged-floor descriptions: 1. `floor_heat_loss=2` — cert 7536 Main lodges this (floors[] description "To unheated space, insulated"); also lodged on cert 2031 / etc. Added mapping → "To unheated space". 2. `floor_heat_loss=3` — cert 7536 Ext2 lodges this with the same floors[] description as Main code 2 — same cascade signal. 3. `floor_heat_loss=6` — cert 9501 + cert 9390 (top-floor flats) lodge this with floors[] description "(another dwelling below)". The cascade routes party-floor handling via property_type=Flat + cert.floors[] description independently of this string, so the explicit None entry preserves the cascade match (cert 9501 stays at exact 1e-4 SAP vs worksheet 68.5252) while distinguishing "decided no string" from "unknown". Six new tests document the contract: - Five unit tests inject an out-of-range integer (99) into a real cohort cert JSON and assert UnmappedApiCode raises with the right `field` and `value`. - One coverage forcing function (`test_all_golden_fixtures_extract _via_api_without_unmapped_code_raise`) loops every JSON under `fixtures/golden/` through `from_api_response` and asserts no raise — future fixtures with unmapped enums fail this test until a dict entry is added. 763 → 769 pass + 0 fail (5 unit + 1 cohort-coverage test added). Pyright net-zero (32 → 32 baseline preserved). The pattern is ready to extend to other silently-falling-through helpers — e.g., `_api_glazing_transmission` (codes 4-12, 15+ noted in the existing comment as "not yet mapped — incremental coverage as new fixtures surface them"), `_api_cascade_glazing_type` (pass- through is intentional, so probably leave alone). Each addition is its own slice. --- .../tests/test_summary_pdf_mapper_chain.py | 121 +++++++++++++++++ datatypes/epc/domain/mapper.py | 126 +++++++++++++++--- 2 files changed, 231 insertions(+), 16 deletions(-) diff --git a/backend/documents_parser/tests/test_summary_pdf_mapper_chain.py b/backend/documents_parser/tests/test_summary_pdf_mapper_chain.py index 8258d26a..4c56fce0 100644 --- a/backend/documents_parser/tests/test_summary_pdf_mapper_chain.py +++ b/backend/documents_parser/tests/test_summary_pdf_mapper_chain.py @@ -40,6 +40,7 @@ import pytest from backend.documents_parser.elmhurst_extractor import ElmhurstSiteNotesExtractor from datatypes.epc.domain.mapper import ( EpcPropertyDataMapper, + UnmappedApiCode, UnmappedElmhurstLabel, ) from domain.sap10_calculator.calculator import calculate_sap_from_inputs @@ -1089,6 +1090,126 @@ def test_summary_mapper_raises_on_unmapped_glazing_type_label() -> None: assert excinfo.value.value == "Quintuple glazed with helium" +# ---------------------------------------------------------------------- +# API mapper strict-raise — mirror the Elmhurst UnmappedElmhurstLabel +# coverage gate on the GOV.UK API path. The same failure mode (silently +# routing an unknown enum to a default int / None hides cascade gaps +# until a SAP-delta investigation surfaces them) applies to API +# integer codes. Each strict helper is unit-tested for its raise +# behaviour; a cohort-coverage forcing function asserts every golden +# fixture extracts cleanly via `from_api_response`. +# ---------------------------------------------------------------------- + +_GOLDEN_FIXTURES_DIR = ( + Path(__file__).parents[3] + / "domain/sap10_calculator/rdsap/tests/fixtures/golden" +) + + +def _patch_api_doc_and_extract( + cert: str, mutator: "callable[[dict], None]" +) -> None: + """Load a golden cert JSON, apply a mutation, and run + `from_api_response`. Used by the strict-raise unit tests to inject + an unmapped integer code into a known-good document.""" + doc = json.loads((_GOLDEN_FIXTURES_DIR / f"{cert}.json").read_text()) + mutator(doc) + EpcPropertyDataMapper.from_api_response(doc) + + +def test_api_mapper_raises_on_unmapped_floor_construction_code() -> None: + # Arrange — start from a real cohort cert and inject an unmapped + # `floor_construction` integer (currently the dict covers 1 and 2). + # The mapper must raise `UnmappedApiCode` rather than silently + # dropping the floor_construction signal — losing it routes the + # cascade to the wrong solid-vs-suspended branch (see Slice + # S0380.27's floor_construction_type fix that closed cert 8135's + # PE -4.96 → -0.07). + def mutate(doc: "dict") -> None: + doc["sap_building_parts"][0]["sap_floor_dimensions"][0]["floor_construction"] = 99 + + # Act / Assert + with pytest.raises(UnmappedApiCode) as excinfo: + _patch_api_doc_and_extract("0380-2471-3250-2596-8761", mutate) + assert excinfo.value.field == "floor_construction" + assert excinfo.value.value == 99 + + +def test_api_mapper_raises_on_unmapped_roof_construction_code() -> None: + # Arrange — inject an unmapped roof_construction integer. The + # cascade's `_api_roof_construction_str` powers the cos(30°) + # inclined-surface factor and the flat-roof Table 18 column-3 + # dispatch — a silently-None value here under-counts roof loss + # for sloping ceilings or routes flat roofs to the wrong column. + def mutate(doc: "dict") -> None: + doc["sap_building_parts"][0]["roof_construction"] = 99 + + # Act / Assert + with pytest.raises(UnmappedApiCode) as excinfo: + _patch_api_doc_and_extract("0380-2471-3250-2596-8761", mutate) + assert excinfo.value.field == "roof_construction" + assert excinfo.value.value == 99 + + +def test_api_mapper_raises_on_unmapped_party_wall_construction_code() -> None: + # Arrange — inject an unmapped party_wall_construction. The cohort + # currently covers RdSAP10 Table 15 codes 0..5; out-of-range integers + # must raise so the next fixture forces an explicit dict entry. + def mutate(doc: "dict") -> None: + doc["sap_building_parts"][0]["party_wall_construction"] = 99 + + # Act / Assert + with pytest.raises(UnmappedApiCode) as excinfo: + _patch_api_doc_and_extract("0380-2471-3250-2596-8761", mutate) + assert excinfo.value.field == "party_wall_construction" + assert excinfo.value.value == 99 + + +def test_api_mapper_raises_on_unmapped_floor_heat_loss_code() -> None: + # Arrange — codes 4/5/8+ aren't in the dict; injecting one must + # raise. Codes 1/2/3/6/7 are mapped explicitly (some to None) so + # the strict gate distinguishes "decided no string" from "unknown". + def mutate(doc: "dict") -> None: + doc["sap_building_parts"][0]["floor_heat_loss"] = 99 + + # Act / Assert + with pytest.raises(UnmappedApiCode) as excinfo: + _patch_api_doc_and_extract("0380-2471-3250-2596-8761", mutate) + assert excinfo.value.field == "floor_heat_loss" + assert excinfo.value.value == 99 + + +def test_api_mapper_raises_on_unmapped_built_form_code() -> None: + # Arrange — codes 1..6 cover detached / semi-detached / terraces; + # an out-of-range integer must raise rather than silently routing + # through the cascade's `_DEFAULT_SHELTERED_SIDES = 2`. + def mutate(doc: "dict") -> None: + doc["built_form"] = 99 + + # Act / Assert + with pytest.raises(UnmappedApiCode) as excinfo: + _patch_api_doc_and_extract("0380-2471-3250-2596-8761", mutate) + assert excinfo.value.field == "built_form" + assert excinfo.value.value == 99 + + +def test_all_golden_fixtures_extract_via_api_without_unmapped_code_raise() -> None: + # Arrange — coverage forcing function on the API path: every JSON + # fixture in `fixtures/golden/` must round-trip through + # `from_api_response` without triggering an `UnmappedApiCode` raise + # from any strict helper. New cohort fixtures added in subsequent + # slices fall under the same gate; future API enum variants + # surface here at extraction time instead of as a downstream SAP + # delta. + fixtures = sorted(_GOLDEN_FIXTURES_DIR.glob("*.json")) + assert fixtures, f"no golden fixtures under {_GOLDEN_FIXTURES_DIR}" + + # Act / Assert — strict run for each fixture + for fixture in fixtures: + doc = json.loads(fixture.read_text()) + EpcPropertyDataMapper.from_api_response(doc) + + def test_summary_7800_two_electric_showers_count_as_two_not_one() -> None: # Arrange — cert 7800-1501-0922-7127-3563's Summary §16 lodges TWO # instantaneous electric showers ("Shower 01" + "Shower 11", both diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index 3b5539e2..348d27b9 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -2178,6 +2178,31 @@ def _elmhurst_party_wall_construction_int(coded: str) -> Optional[int]: return _ELMHURST_PARTY_WALL_CODE_TO_SAP10.get(_leading_code(coded)) +class UnmappedApiCode(ValueError): + """A GOV.UK API integer enum that the mapper does not yet know how + to translate to the SAP10 cascade-domain value. + + Raised by the strict API code helpers (floor_construction, + roof_construction, party_wall_construction, …) to surface mapper- + coverage gaps at the extraction boundary instead of silently + returning None and letting the cascade default to a wrong-but-not- + obviously-wrong value downstream. Mirrors `UnmappedElmhurstLabel` + on the Summary path. + + Distinguish "lodging absent" (helper returns None — correct) from + "lodging present but unrecognised" (raise — fixture variant the + mapper doesn't yet cover, needs a dict entry added). + """ + + def __init__(self, field: str, value: object) -> None: + super().__init__( + f"unmapped API {field} code: {value!r}; " + f"add an entry to the corresponding mapper lookup dict" + ) + self.field = field + self.value = value + + # GOV.UK API party_wall_construction enum → SAP10 wall_construction # integer (the domain `u_party_wall` consumes). The API uses a different # enum from the regular wall_construction field — RdSAP 10 Table 15 @@ -2216,10 +2241,18 @@ def _api_party_wall_construction_int(value: Union[int, str, None]) -> Optional[i """Translate the GOV.UK API `party_wall_construction` integer code (or 'NA' string) to the SAP10 wall_construction integer the cascade consumes. See `_API_PARTY_WALL_CONSTRUCTION_TO_SAP10` for the - enum semantics (RdSAP 10 Table 15).""" + enum semantics (RdSAP 10 Table 15). + + Strict-coverage: a lodged integer that isn't in the dict raises + `UnmappedApiCode` so the fixture forces a dict entry rather than + silently falling back to the cascade's U=0.25 house default. + `None` / 'NA' / digit-string variants stay as no-lodging (return + None — the cascade applies its own default).""" if value is None or isinstance(value, str): return None - return _API_PARTY_WALL_CONSTRUCTION_TO_SAP10.get(value) + if value not in _API_PARTY_WALL_CONSTRUCTION_TO_SAP10: + raise UnmappedApiCode("party_wall_construction", value) + return _API_PARTY_WALL_CONSTRUCTION_TO_SAP10[value] # GOV.UK API `floor_construction` integer → human-readable string the @@ -2254,8 +2287,17 @@ _API_ROOF_CONSTRUCTION_TO_STR: Dict[int, str] = { def _api_floor_construction_str(value: Optional[int]) -> Optional[str]: """Translate the API integer floor_construction code to the human-readable string the cascade reads via Slice 88's - `effective_floor_description` in `heat_transmission.py`.""" - return _API_FLOOR_CONSTRUCTION_TO_STR.get(value) if value is not None else None + `effective_floor_description` in `heat_transmission.py`. + + Strict-coverage: a lodged integer outside the mapped set raises + `UnmappedApiCode` so the next fixture forces a dict entry rather + than silently dropping back to the cascade's solid-vs-suspended + branching default.""" + if value is None: + return None + if value not in _API_FLOOR_CONSTRUCTION_TO_STR: + raise UnmappedApiCode("floor_construction", value) + return _API_FLOOR_CONSTRUCTION_TO_STR[value] # GOV.UK API `floor_heat_loss` integer → `floor_type` string the @@ -2266,8 +2308,35 @@ def _api_floor_construction_str(value: Optional[int]) -> Optional[str]: # floor" as the floor_type — otherwise the spec rule short-circuits # to False even when the cert is genuinely G+T (e.g. cert 001479 # Main, floor_heat_loss=7). -_API_FLOOR_HEAT_LOSS_TO_FLOOR_TYPE: Dict[int, str] = { - 1: "To external air", # exposed (cantilevered / over passageway) +# +# Code coverage across the cohort fixtures: +# 1 = "To external air" — exposed floor (cantilever / passageway) +# 2 = "To unheated space" — over garage / unheated basement / +# crawlspace; cert 7536 Main lodges this +# 3 = "To unheated space" — variant lodged by cert 7536 Ext2 with +# the same top-level floors[] description +# as code 2; route to the same cascade +# signal until a fixture forces them apart +# 6 = "(another dwelling below)" — top-floor flat over a party floor; +# cert 9501 lodges this. The cascade's +# floor-as-party-floor dispatch already +# handles this via `property_type=Flat` + +# cert.floors[].description, so the +# floor_type string from this helper is +# not consumed for the (12) spec rule +# in that path — explicit None preserves +# the cert 9501 cascade match without +# silently letting unknown codes through. +# 7 = "Ground floor" — typical ground-floor heat loss +# +# Codes 4/5/8+ are not yet observed in any fixture; the strict-raise +# path catches them at the extraction boundary so the next cert forces +# an explicit mapping decision. +_API_FLOOR_HEAT_LOSS_TO_FLOOR_TYPE: Dict[int, Optional[str]] = { + 1: "To external air", + 2: "To unheated space", + 3: "To unheated space", + 6: None, 7: "Ground floor", } @@ -2275,19 +2344,37 @@ _API_FLOOR_HEAT_LOSS_TO_FLOOR_TYPE: Dict[int, str] = { def _api_floor_type_str(floor_heat_loss: Optional[int]) -> Optional[str]: """Translate the API integer floor_heat_loss code to the human-readable floor_type the RdSAP 10 §5 (12) spec rule consumes - (see `_has_suspended_timber_floor_per_spec`).""" - return ( - _API_FLOOR_HEAT_LOSS_TO_FLOOR_TYPE.get(floor_heat_loss) - if floor_heat_loss is not None else None - ) + (see `_has_suspended_timber_floor_per_spec`). + + Strict-coverage: a lodged integer outside the mapped set raises + `UnmappedApiCode`. The mapped set includes explicit `None` entries + for codes whose floor-type-string isn't consumed by the (12) rule + on the cascade path that handles them (e.g. code 6 = "another + dwelling below" routes through the party-floor cascade independent + of this string). Explicit None entries DECIDE the no-string + outcome rather than letting unknown integers fall through.""" + if floor_heat_loss is None: + return None + if floor_heat_loss not in _API_FLOOR_HEAT_LOSS_TO_FLOOR_TYPE: + raise UnmappedApiCode("floor_heat_loss", floor_heat_loss) + return _API_FLOOR_HEAT_LOSS_TO_FLOOR_TYPE[floor_heat_loss] def _api_roof_construction_str(value: Optional[int]) -> Optional[str]: """Translate the API integer roof_construction code to the human-readable string the cascade reads via Slice 89's `roof_construction_type`-based cos(30°) factor in - `heat_transmission.py`.""" - return _API_ROOF_CONSTRUCTION_TO_STR.get(value) if value is not None else None + `heat_transmission.py`. + + Strict-coverage: a lodged integer outside the mapped set raises + `UnmappedApiCode` so the next fixture surfaces a missing dict + entry rather than silently dropping the inclined-surface cos(30°) + factor (or, worse, the flat-roof Table 18 column-3 dispatch).""" + if value is None: + return None + if value not in _API_ROOF_CONSTRUCTION_TO_STR: + raise UnmappedApiCode("roof_construction", value) + return _API_ROOF_CONSTRUCTION_TO_STR[value] # API `floor_heat_loss` integer that signals an exposed floor (lowest @@ -2317,13 +2404,20 @@ _API_BUILT_FORM_TO_SHELTERED_SIDES: Dict[int, int] = { def _api_sheltered_sides(built_form: object) -> Optional[int]: """Translate the API `built_form` integer code to the SAP10.2 §S5 - sheltered-sides count. Returns None when the form isn't recognised - so the cascade applies its own default (currently 2).""" + sheltered-sides count. + + Strict-coverage: a lodged integer outside the mapped set raises + `UnmappedApiCode` rather than silently routing through the + cascade's `_DEFAULT_SHELTERED_SIDES = 2` (which over-states + sheltering for detached / under-states for enclosed). Non-int / + None lodging stays as no-lodging.""" if isinstance(built_form, str) and built_form.isdigit(): built_form = int(built_form) if not isinstance(built_form, int): return None - return _API_BUILT_FORM_TO_SHELTERED_SIDES.get(built_form) + if built_form not in _API_BUILT_FORM_TO_SHELTERED_SIDES: + raise UnmappedApiCode("built_form", built_form) + return _API_BUILT_FORM_TO_SHELTERED_SIDES[built_form] # GOV.UK API `glazing_type` integer → (u_value W/m²K, g_perpendicular,