From 34d97a75f4821703a62988daf64714411726b8f0 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 26 Jun 2026 09:53:19 +0000 Subject: [PATCH 1/3] =?UTF-8?q?Construct=20sap=5Fventilation=20in=20from?= =?UTF-8?q?=5Frdsap=5Fschema=5F19=5F0=20/=2021=5F0=5F0=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 19.0 + 21.0.0 pass no sap_ventilation= at all → EpcPropertyData default None, dropping the entire §2 block (sheltered_sides + mechanical_ventilation_kind). Extend the MVHR mapper test to both and add a sheltered_sides regression, mirroring 21.0.1 (remaining ADR-0037 ventilation follow-up). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../domain/tests/test_from_rdsap_schema.py | 38 ++++++++++++++++--- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/datatypes/epc/domain/tests/test_from_rdsap_schema.py b/datatypes/epc/domain/tests/test_from_rdsap_schema.py index a817414c..07fc7ddb 100644 --- a/datatypes/epc/domain/tests/test_from_rdsap_schema.py +++ b/datatypes/epc/domain/tests/test_from_rdsap_schema.py @@ -2710,22 +2710,48 @@ def test_rdsap_mappers_carry_main_heating_controls_display( (RdSapSchema17_1, EpcPropertyDataMapper.from_rdsap_schema_17_1, "17_1.json"), (RdSapSchema18_0, EpcPropertyDataMapper.from_rdsap_schema_18_0, "18_0.json"), (RdSapSchema20_0_0, EpcPropertyDataMapper.from_rdsap_schema_20_0_0, "20_0_0.json"), + (RdSapSchema19_0, EpcPropertyDataMapper.from_rdsap_schema_19_0, "19_0.json"), + (RdSapSchema21_0_0, EpcPropertyDataMapper.from_rdsap_schema_21_0_0, "21_0_0.json"), ], ) def test_rdsap_mappers_map_mechanical_ventilation_kind( schema_cls: Any, mapper: Any, fixture: str ) -> None: # Calc-facing: an MVHR cert (mechanical_ventilation=4) must reach the §2 - # ventilation cascade as MVHR, not be silently treated as natural. These - # three build a bare SapVentilation(sheltered_sides=…) and dropped the - # top-level mechanical_ventilation; 17.0 + 21.0.1 + full-SAP already map it - # (via _sap_17_1_ventilation / their own wiring). 19.0 + 21.0.0 set no - # sap_ventilation at all — a bigger, separate gap (see followups). Natural + # ventilation cascade as MVHR, not be silently treated as natural. 17.1 / + # 18.0 / 20.0.0 built a bare SapVentilation(sheltered_sides=…) and dropped + # the top-level mechanical_ventilation (fixed in #1333); 19.0 + 21.0.0 set + # no sap_ventilation at all, dropping the whole block (fixed here — the + # remaining ADR-0037 ventilation follow-up). 17.0 + 21.0.1 + full-SAP + # already map it (via _sap_17_1_ventilation / their own wiring). Natural # certs (code 0/5 → None) are unchanged, so the fix only moves genuine - # MEV/MVHR certs (ADR-0037 follow-up). + # MEV/MVHR certs. data = load(fixture) data["mechanical_ventilation"] = 4 result = mapper(from_dict(schema_cls, data)) assert result.sap_ventilation.mechanical_ventilation_kind == "MVHR" + + +@pytest.mark.parametrize( + "schema_cls, mapper, fixture", + [ + (RdSapSchema19_0, EpcPropertyDataMapper.from_rdsap_schema_19_0, "19_0.json"), + (RdSapSchema21_0_0, EpcPropertyDataMapper.from_rdsap_schema_21_0_0, "21_0_0.json"), + ], +) +def test_rdsap_19_0_21_0_0_construct_sap_ventilation_block( + schema_cls: Any, mapper: Any, fixture: str +) -> None: + # 19.0 + 21.0.0 previously passed no `sap_ventilation=` at all → the + # EpcPropertyData default empty SapVentilation, dropping the entire §2 + # block (sheltered_sides + kind). Mirroring 21.0.1, they now construct it. + # `sheltered_sides` derives from built_form per §S5 (both fixtures lodge + # built_form=2 Semi-Detached → 1 sheltered side); a missing/empty block + # would leave it None and over-count shelter via the cascade default of 2. + schema = from_dict(schema_cls, load(fixture)) + + result = mapper(schema) + + assert result.sap_ventilation.sheltered_sides == 1 From a63276387ef7653119620bf1533fa426e9e57471 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 26 Jun 2026 09:55:08 +0000 Subject: [PATCH 2/3] =?UTF-8?q?Construct=20sap=5Fventilation=20in=20from?= =?UTF-8?q?=5Frdsap=5Fschema=5F19=5F0=20/=2021=5F0=5F0=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both passed no sap_ventilation= → EpcPropertyData default None, dropping the whole §2 block so MEV/MVHR certs scored as natural. Mirror 21.0.1's block for the only fields each schema lodges: mechanical_ventilation_kind (§2 MV-kind dispatch) + sheltered_sides (§S5, from built_form). Neither schema lodges the fan/flue/vent counts (RdSAP Table-5 age defaults), so those stay unset. Co-Authored-By: Claude Opus 4.8 (1M context) --- datatypes/epc/domain/mapper.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index 8f55db80..efacc71b 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -1698,6 +1698,19 @@ class EpcPropertyDataMapper: if getattr(bp, "glazed_perimeter", None) is None ], sap_conservatory=_api_sap_conservatory(schema.sap_building_parts), + # ADR-0037 follow-up: 19.0 previously passed no `sap_ventilation=`, + # dropping the whole §2 block. The schema only lodges the top-level + # `mechanical_ventilation` enum + `built_form` (no fan/flue/vent + # counts — RdSAP uses Table-5 age defaults), so mirror just those + # two fields from 21.0.1. An MEV/MVHR cert that defaulted to NATURAL + # under-stated its ventilation heat loss; `sheltered_sides` (§S5) + # avoids the cascade's shelter default of 2. + sap_ventilation=SapVentilation( + sheltered_sides=_api_sheltered_sides(schema.built_form), + mechanical_ventilation_kind=_api_mechanical_ventilation_kind( + schema.mechanical_ventilation + ), + ), ) @staticmethod @@ -2274,6 +2287,19 @@ class EpcPropertyDataMapper: else None ), ), + # ADR-0037 follow-up: 21.0.0 previously passed no `sap_ventilation=`, + # dropping the whole §2 block. Unlike 21.0.1 it does not lodge the + # fan/flue/vent counts, so mirror only the two fields it does carry: + # the top-level `mechanical_ventilation` enum (→ §2 MV-kind dispatch; + # an MEV/MVHR cert otherwise defaulted to NATURAL and under-stated + # ventilation heat loss) and `sheltered_sides` from built_form (§S5, + # vs the cascade's shelter default of 2). + sap_ventilation=SapVentilation( + sheltered_sides=_api_sheltered_sides(schema.built_form), + mechanical_ventilation_kind=_api_mechanical_ventilation_kind( + schema.mechanical_ventilation + ), + ), ) @staticmethod From cc1c763946132b4cb5c14c1820aa6ffca770362c Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 26 Jun 2026 10:00:48 +0000 Subject: [PATCH 3/3] =?UTF-8?q?docs:=20mark=2019.0/21.0.0=20ventilation=20?= =?UTF-8?q?fixed=20(feat/rdsap-19-21-ventilation)=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/baseline-downgrade-followups.md | 32 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/docs/baseline-downgrade-followups.md b/docs/baseline-downgrade-followups.md index 2157eb7f..0b7464c1 100644 --- a/docs/baseline-downgrade-followups.md +++ b/docs/baseline-downgrade-followups.md @@ -12,12 +12,15 @@ Each is **separate from** the full-SAP fix (`fix/baseline-downgrades`). `mechanical_ventilation_kind`. ✅ - `21_0_1` — rich inline `SapVentilation(...)` incl. the kind. ✅ - **`17_1` / `18_0` / `20_0_0`** — built `SapVentilation(sheltered_sides=…)` and - **dropped** the kind. **Fixed in this PR** (mirror `_api_mechanical_ventilation_kind`). -- **`19_0` / `21_0_0`** — set **no `sap_ventilation` at all** → the dataclass - default empty object. They drop the **entire** ventilation block (sheltered - sides + kind + everything), not just the kind. **Still open** — a bigger, - separate consistency fix (give them a proper `sap_ventilation` construction, - mirroring 21.0.1), not a one-liner. + **dropped** the kind. **Fixed in #1333** (mirror `_api_mechanical_ventilation_kind`). +- **`19_0` / `21_0_0`** — set **no `sap_ventilation` at all** → the + `EpcPropertyData` default (`None`). They dropped the **entire** ventilation + block (sheltered sides + kind), not just the kind. **Fixed in + `feat/rdsap-19-21-ventilation`**: both now construct `SapVentilation(...)` + mirroring 21.0.1, but only for the fields each schema lodges — the top-level + `mechanical_ventilation` enum (→ kind) and `sheltered_sides` (from + `built_form`). Neither lodges the fan/flue/vent counts, so those stay unset + (RdSAP Table-5 age defaults — see counts note below). Either way, an MEV/MVHR cert (`mechanical_ventilation ≠ 0`) is treated as **natural** by the affected mappers — wrong §2 ventilation cascade (and heat @@ -34,14 +37,15 @@ The granular **counts** (fans/flues/vents) are *not* a bug: older RdSAP open-dat certs don't lodge them, and the calc correctly uses RdSAP Table-5 age defaults. `percent_draughtproofed` is mapped (top-level) and read by the calc. -**Remaining fix (19.0 / 21.0.0):** give them a proper `sap_ventilation` -construction mirroring 21.0.1. **Calc-facing → validate** with the RdSAP-21.0.1 -corpus (must hold 73.3% / MAE 0.774) plus an **Elmhurst-anchored MEV/MVHR -`RealCertExpectation`** (the corpus is natural-vent-dominated, so the kind change -isn't exercised by it). Quantify blast radius: count older-RdSAP certs with -`mechanical_ventilation ≠ 0`. The 17.1/18.0/20.0.0 fix in this PR is guarded by a -mapper-level MVHR test + the corpus/mapper-corpus staying green, with the Elmhurst -MEV/MVHR anchor as the SAP-accuracy fast-follow. +**19.0 / 21.0.0 fix (done, `feat/rdsap-19-21-ventilation`):** both build +`SapVentilation(sheltered_sides=…, mechanical_ventilation_kind=…)` mirroring +21.0.1. Guarded by the extended mapper-level MVHR test + a `sheltered_sides` +regression, with the corpus (held 73.3% / MAE 0.774 — it's 21.0.1-only so the +change isn't exercised by it) and the 6002-case mapper-corpus staying green. +**Outstanding SAP-accuracy fast-follow:** an **Elmhurst-anchored MEV/MVHR +`RealCertExpectation`** (the corpus is natural-vent-dominated, so the kind +change isn't exercised by it). **Blast radius (still to quantify):** count +portfolio-796 certs on schema 19.0 / 21.0.0 with `mechanical_ventilation ≠ 0`. ## 2. FE "Main Fuel: Unknown" is FE-side, not a Model mapper gap