From 3282cef04ede67f5cbbcadb75d65d608536a2239 Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Wed, 1 Jul 2026 10:41:52 +0000 Subject: [PATCH] Fix RdSAP-17.0 mapper dropping lodged secondary heating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `from_rdsap_schema_17_0` path hardcoded `sap_heating.secondary_heating_type` and `secondary_fuel_type` to None, unlike every other API path (19.0/20.0/…), which read them from `schema.sap_heating`. The gov EPC API *does* populate a secondary heating SAP code for a genuine fixed secondary (e.g. 691 "electric room heaters", 694), so the hardcode silently discarded it. Because `recommend_secondary_heating_removal` gates purely on `sap_heating.secondary_heating_type`, any 17.0 cert carrying a fixed secondary would never get its removal recommendation. Surfaced while auditing portfolio 814's recommendations; 814 has 17.0 certs but none with a fixed secondary, so the defect was latent there — this restores parity with the other paths so it cannot bite. Mirrors the 19.0 path exactly (secondary_fuel_type via `_api_secondary_fuel_type`). Adds a regression guard that injects a fixed secondary into a real 17.0 cert and asserts the code survives the mapping. Co-Authored-By: Claude Opus 4.8 (1M context) --- datatypes/epc/domain/mapper.py | 13 +++- .../test_mapper_secondary_heating_17_0.py | 61 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 tests/datatypes/epc/domain/test_mapper_secondary_heating_17_0.py diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index e89062610..e17b43033 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -643,8 +643,17 @@ class EpcPropertyDataMapper: immersion_heating_type=schema.sap_heating.immersion_heating_type, cylinder_insulation_type=schema.sap_heating.cylinder_insulation_type, cylinder_thermostat=None, - secondary_fuel_type=None, - secondary_heating_type=None, + # Preserve the lodged secondary heating from the 17.0 sap_heating + # block, matching every other API path (19.0/20.0/…). Hardcoding + # these to None silently dropped a lodged FIXED secondary (SAP + # codes like 691/694 that the gov API does populate), which + # suppressed the secondary_heating_removal recommendation for any + # 17.0 cert carrying one. + secondary_fuel_type=_api_secondary_fuel_type( + schema.sap_heating.secondary_fuel_type, + schema.sap_heating.secondary_heating_type, + ), + secondary_heating_type=schema.sap_heating.secondary_heating_type, cylinder_insulation_thickness_mm=None, ), # ADR-0028: 990/1000 omit sap_windows -> synthesised from the diff --git a/tests/datatypes/epc/domain/test_mapper_secondary_heating_17_0.py b/tests/datatypes/epc/domain/test_mapper_secondary_heating_17_0.py new file mode 100644 index 000000000..0d94df908 --- /dev/null +++ b/tests/datatypes/epc/domain/test_mapper_secondary_heating_17_0.py @@ -0,0 +1,61 @@ +"""Regression: the RdSAP-Schema-17.0 mapper path must preserve a lodged secondary +heating system, like every other API path. + +`from_rdsap_schema_17_0` previously hardcoded `sap_heating.secondary_heating_type` +(and `secondary_fuel_type`) to None, unlike the 19.0/20.0/… paths which read +`schema.sap_heating.secondary_heating_type`. The gov EPC API *does* populate that +field for a genuine FIXED secondary (SAP codes such as 691 "electric room +heaters" / 694), so the hardcode silently dropped it — and because +`recommend_secondary_heating_removal` gates purely on +`sap_heating.secondary_heating_type`, a 17.0 cert with a fixed secondary would +never get its removal recommendation. Portfolio 814 happened not to contain such +a cert, so the bug was latent; this guard makes sure it stays fixed. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +from datatypes.epc.domain.mapper import EpcPropertyDataMapper + +# A real RdSAP-Schema-17.0 cert whose lodged secondary is absent (maps to None); +# we inject a fixed secondary to prove the 17.0 path now carries it through. +_FIXTURE = Path("tests/fixtures/epc_prediction/PE71NT/cert-26619d8e7f8e.json") + +# SAP 10.2 secondary-heating code for fixed electric room heaters, exactly as the +# gov API lodges it (seen live on portfolio-814 certs), + its off-peak fuel code. +_FIXED_SECONDARY_TYPE = 691 +_SECONDARY_FUEL = 29 + + +def _load_17_0_cert() -> dict[str, Any]: + data: dict[str, Any] = json.loads(_FIXTURE.read_text()) + assert data["schema_type"] == "RdSAP-Schema-17.0" + return data + + +def test_17_0_path_preserves_lodged_fixed_secondary() -> None: + # Arrange — a 17.0 cert carrying a lodged fixed secondary in sap_heating. + data = _load_17_0_cert() + data["sap_heating"]["secondary_heating_type"] = _FIXED_SECONDARY_TYPE + data["sap_heating"]["secondary_fuel_type"] = _SECONDARY_FUEL + + # Act + epc = EpcPropertyDataMapper.from_api_response(data) + + # Assert — the 17.0 mapper carries the code through (it used to be dropped to + # None), so secondary_heating_removal can fire on this dwelling. + assert epc.sap_heating.secondary_heating_type == _FIXED_SECONDARY_TYPE + + +def test_17_0_path_leaves_absent_secondary_as_none() -> None: + # Arrange — the unmodified cert has no lodged secondary. + data = _load_17_0_cert() + + # Act + epc = EpcPropertyDataMapper.from_api_response(data) + + # Assert — no secondary lodged still maps to None (no phantom removal). + assert epc.sap_heating.secondary_heating_type is None