From 56add97bd9de4f461bdeacce03c084c2e3e17763 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 24 Jun 2026 13:56:03 +0000 Subject: [PATCH 1/4] =?UTF-8?q?Map=20non-separated=20conservatory=20on=20t?= =?UTF-8?q?he=20gov-API=20RdSAP-19.0=20path=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror the merged 21.0.1 fix (d501535c): declare the four glazed conservatory fields on the 19.0 SapBuildingPart so from_dict stops dropping them, exclude the glazed BP from the fabric loop, and carry it as EpcPropertyData.sap_conservatory (§6.1). Fixes cert 718138 (conservatory_type=4) mis-scoring as a fabric part and failing to persist on the NOT-NULL construction_age_band. Co-Authored-By: Claude Opus 4.8 (1M context) --- datatypes/epc/domain/mapper.py | 6 ++ .../domain/tests/test_from_rdsap_schema.py | 73 +++++++++++++++++++ datatypes/epc/schema/rdsap_schema_19_0.py | 10 +++ 3 files changed, 89 insertions(+) diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index 10ec411d..ed91551d 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -1568,8 +1568,14 @@ class EpcPropertyDataMapper: else None ), ) + # RdSAP 10 §6.1 — exclude the glazed conservatory BP from the + # fabric loop; it is carried as `sap_conservatory` below and + # billed by the §6.1 cascade (window/rooflight/floor), not as + # a dwelling building part. Mirrors the 21.0.1 path. for bp in schema.sap_building_parts + if getattr(bp, "glazed_perimeter", None) is None ], + sap_conservatory=_api_sap_conservatory(schema.sap_building_parts), ) @staticmethod diff --git a/datatypes/epc/domain/tests/test_from_rdsap_schema.py b/datatypes/epc/domain/tests/test_from_rdsap_schema.py index a2463eae..070b100c 100644 --- a/datatypes/epc/domain/tests/test_from_rdsap_schema.py +++ b/datatypes/epc/domain/tests/test_from_rdsap_schema.py @@ -2433,3 +2433,76 @@ class TestNonSeparatedConservatoryApiMirror: # Assert — §6.2: disregarded; no conservatory geometry. assert epc.sap_conservatory is None assert conservatory_geometry(epc) is None + + +class TestNonSeparatedConservatoryApiMirror19_0: + """RdSAP 10 §6.1 — the same glazed-BP conservatory the 21.0.1 mapper + handles is also lodged on RdSAP-Schema-19.0 (repro cert 718138). The four + glazed fields were undeclared on the 19.0 `SapBuildingPart`, so `from_dict` + dropped them: the conservatory mapped to a fabric-less building part that + mis-scored and could not persist (NOT-NULL `construction_age_band`). The + 19.0 mapper now mirrors the 21.0.1 split. + + Unlike 21.0.1, the 19.0 mapper keeps the **lodged** `total_floor_area` + scalar (a real 19.0 cert already lodges the non-separated conservatory in + it — 718138: dwelling 140.23 + conservatory 9.71 = 149.94 → lodged 150), so + appending a conservatory does NOT change `total_floor_area_m2`. The §6.1 + floor-area fold reaches the score through the calculator's dimensions, not + through the scalar.""" + + def test_from_api_response_splits_out_conservatory_building_part( + self, + ) -> None: + # Arrange — the 19.0 dwelling plus a non-separated double-glazed + # conservatory glazed BP. + from datatypes.epc.domain.epc_property_data import SapConservatory + from domain.sap10_calculator.worksheet.conservatory import ( + conservatory_geometry, + ) + + dwelling_part_count = len( + EpcPropertyDataMapper.from_api_response(load("19_0.json")).sap_building_parts + ) + + cert = load("19_0.json") + cert["conservatory_type"] = 4 + cert["sap_building_parts"].append( + { + "floor_area": 12.0, + "room_height": 1, + "double_glazed": "Y", + "glazed_perimeter": 9.0, + } + ) + + # Act + epc = EpcPropertyDataMapper.from_api_response(cert) + + # Assert — conservatory split out; the glazed BP is NOT a fabric part. + assert epc.sap_conservatory == SapConservatory( + floor_area_m2=12.0, + glazed_perimeter_m=9.0, + double_glazed=True, + thermally_separated=False, + room_height_storeys=1.0, + ) + assert len(epc.sap_building_parts) == dwelling_part_count + # §6.1 fold is active (surfaces derived by the shared cascade). + assert conservatory_geometry(epc) is not None + + def test_separated_conservatory_lodges_no_glazed_building_part(self) -> None: + # Arrange — a separated conservatory (type 2/3) lodges NO glazed BP; + # the dwelling is unchanged. + from domain.sap10_calculator.worksheet.conservatory import ( + conservatory_geometry, + ) + + cert = load("19_0.json") + cert["conservatory_type"] = 2 + + # Act + epc = EpcPropertyDataMapper.from_api_response(cert) + + # Assert — §6.2: disregarded; no conservatory geometry. + assert epc.sap_conservatory is None + assert conservatory_geometry(epc) is None diff --git a/datatypes/epc/schema/rdsap_schema_19_0.py b/datatypes/epc/schema/rdsap_schema_19_0.py index b91dff35..2ba0f3cb 100644 --- a/datatypes/epc/schema/rdsap_schema_19_0.py +++ b/datatypes/epc/schema/rdsap_schema_19_0.py @@ -122,6 +122,16 @@ class SapBuildingPart: wall_insulation_thickness: Optional[str] = None floor_insulation_thickness: Optional[str] = None flat_roof_insulation_thickness: Optional[Union[str, int]] = None + # RdSAP 10 §6.1 — a NON-SEPARATED conservatory (`conservatory_type == 4`) is + # lodged by the gov API as a glazed "building part" carrying ONLY these four + # fields (no fabric, no floor dimensions). Previously undeclared → dropped by + # `from_dict`, so the conservatory was silently lost on the 19.0 path. The + # mapper splits this BP out into `EpcPropertyData.sap_conservatory`. Mirrors + # the 21.0.1 schema declaration. + floor_area: Optional[Union[Measurement, int, float]] = None + room_height: Optional[Union[Measurement, int, float]] = None + double_glazed: Optional[str] = None + glazed_perimeter: Optional[Union[Measurement, int, float]] = None @dataclass From 03a0d9c1ca128b7347ab4855c741c693ea0c295e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 24 Jun 2026 13:56:05 +0000 Subject: [PATCH 2/4] =?UTF-8?q?Round-trip=20the=20non-separated=20conserva?= =?UTF-8?q?tory=20through=20persistence=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persist SapConservatory as five nullable conservatory_* columns on epc_property (1:1 with the dwelling) and rebuild it in _compose, so the §6.1 fold survives save -> reload -> score. Without this the scored (re-hydrated) EPC silently dropped the conservatory (persist != score) — a latent gap shared with the 21.0.1 path. Adds a deep-equality round-trip test. ADR-0036. Co-Authored-By: Claude Opus 4.8 (1M context) --- infrastructure/postgres/epc_property_table.py | 33 +++++++++++++++++ repositories/epc/epc_postgres_repository.py | 28 +++++++++++++++ tests/repositories/epc/test_epc_round_trip.py | 36 +++++++++++++++++++ 3 files changed, 97 insertions(+) diff --git a/infrastructure/postgres/epc_property_table.py b/infrastructure/postgres/epc_property_table.py index c41d797d..0c207e0d 100644 --- a/infrastructure/postgres/epc_property_table.py +++ b/infrastructure/postgres/epc_property_table.py @@ -72,6 +72,16 @@ class EpcPropertyModel(SQLModel, table=True): has_conservatory: Optional[bool] = Field(default=None) has_heated_separate_conservatory: Optional[bool] = Field(default=None) conservatory_type: Optional[int] = Field(default=None) + # RdSAP 10 §6.1 — geometry of a NON-SEPARATED conservatory folded into the + # dwelling (`EpcPropertyData.sap_conservatory`). 1:1 with the dwelling, so + # flat nullable columns rather than a child table; all None when there is no + # non-separated conservatory. Must round-trip — scoring reads the reloaded + # picture, and a dropped conservatory silently disregards it (ADR-0036). + conservatory_floor_area_m2: Optional[float] = Field(default=None) + conservatory_glazed_perimeter_m: Optional[float] = Field(default=None) + conservatory_double_glazed: Optional[bool] = Field(default=None) + conservatory_thermally_separated: Optional[bool] = Field(default=None) + conservatory_room_height_storeys: Optional[float] = Field(default=None) # Counts door_count: int @@ -247,6 +257,29 @@ class EpcPropertyModel(SQLModel, table=True): has_conservatory=data.has_conservatory, has_heated_separate_conservatory=data.has_heated_separate_conservatory, conservatory_type=data.conservatory_type, + conservatory_floor_area_m2=( + data.sap_conservatory.floor_area_m2 + if data.sap_conservatory + else None + ), + conservatory_glazed_perimeter_m=( + data.sap_conservatory.glazed_perimeter_m + if data.sap_conservatory + else None + ), + conservatory_double_glazed=( + data.sap_conservatory.double_glazed if data.sap_conservatory else None + ), + conservatory_thermally_separated=( + data.sap_conservatory.thermally_separated + if data.sap_conservatory + else None + ), + conservatory_room_height_storeys=( + data.sap_conservatory.room_height_storeys + if data.sap_conservatory + else None + ), door_count=data.door_count, wet_rooms_count=data.wet_rooms_count, extensions_count=data.extensions_count, diff --git a/repositories/epc/epc_postgres_repository.py b/repositories/epc/epc_postgres_repository.py index faa86323..d44938e6 100644 --- a/repositories/epc/epc_postgres_repository.py +++ b/repositories/epc/epc_postgres_repository.py @@ -21,6 +21,7 @@ from datatypes.epc.domain.epc_property_data import ( RenewableHeatIncentive, SapAlternativeWall, SapBuildingPart, + SapConservatory, SapEnergySource, SapFlatDetails, SapFloorDimension, @@ -507,6 +508,7 @@ class EpcPostgresRepository(EpcRepository): conservatory_type=p.conservatory_type, has_conservatory=p.has_conservatory, has_heated_separate_conservatory=p.has_heated_separate_conservatory, + sap_conservatory=self._to_conservatory(p), blocked_chimneys_count=p.blocked_chimneys_count, energy_rating_average=p.energy_rating_average, current_energy_efficiency_band=( @@ -853,6 +855,32 @@ class EpcPostgresRepository(EpcRepository): ), ) + @private + def _to_conservatory(self, p: EpcPropertyModel) -> Optional[SapConservatory]: + # RdSAP 10 §6.1 — rebuild the non-separated conservatory the mapper + # split out of the fabric parts. Presence is signalled by the floor + # area (the §6.1 fold always sets all five geometry columns together); + # all None when there is no non-separated conservatory (ADR-0036). + if p.conservatory_floor_area_m2 is None: + return None + return SapConservatory( + floor_area_m2=p.conservatory_floor_area_m2, + glazed_perimeter_m=_require( + p.conservatory_glazed_perimeter_m, "conservatory_glazed_perimeter_m" + ), + double_glazed=_require( + p.conservatory_double_glazed, "conservatory_double_glazed" + ), + thermally_separated=_require( + p.conservatory_thermally_separated, + "conservatory_thermally_separated", + ), + room_height_storeys=_require( + p.conservatory_room_height_storeys, + "conservatory_room_height_storeys", + ), + ) + @private def _to_ventilation(self, p: EpcPropertyModel) -> Optional[SapVentilation]: if not p.ventilation_present: diff --git a/tests/repositories/epc/test_epc_round_trip.py b/tests/repositories/epc/test_epc_round_trip.py index 8233bda3..f32e40b1 100644 --- a/tests/repositories/epc/test_epc_round_trip.py +++ b/tests/repositories/epc/test_epc_round_trip.py @@ -50,6 +50,42 @@ def test_epc_property_data_round_trips(schema_dir: str, db_engine: Engine) -> No assert reloaded == original +def test_non_separated_conservatory_round_trips(db_engine: Engine) -> None: + # RdSAP 10 §6.1 — a non-separated conservatory (conservatory_type=4) maps to + # `EpcPropertyData.sap_conservatory` (the glazed BP is split out of the + # fabric parts). Persistence must round-trip it: scoring reads the reloaded + # picture, so a dropped `sap_conservatory` silently disregards the + # conservatory (persist != score). We inject the conservatory onto the + # known-clean 21.0.1 sample so the ONLY thing that can break deep-equality + # is `sap_conservatory` itself. + + # Arrange — known-clean base + a non-separated double-glazed conservatory. + raw: dict[str, Any] = json.loads( + (_JSON_SAMPLES / "RdSAP-Schema-21.0.1" / "epc.json").read_text() + ) + raw["conservatory_type"] = 4 + raw["sap_building_parts"].append( + { + "floor_area": 12.0, + "room_height": 1, + "double_glazed": "Y", + "glazed_perimeter": 9.0, + } + ) + original = EpcPropertyDataMapper.from_api_response(raw) + assert original.sap_conservatory is not None, "mapper must split the conservatory" + + # Act + with Session(db_engine) as session: + epc_property_id = EpcPostgresRepository(session).save(original) + session.commit() + with Session(db_engine) as session: + reloaded = EpcPostgresRepository(session).get(epc_property_id) + + # Assert — the conservatory survives the round-trip, deep-equal. + assert reloaded == original + + def test_building_part_wall_insulation_thickness_preserves_int( db_engine: Engine, ) -> None: From f3a164c371da2fe2281712a0fc1c34630829cf54 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 24 Jun 2026 13:56:05 +0000 Subject: [PATCH 3/4] =?UTF-8?q?Guard=20EpcPropertyData=20round-trip=20fiel?= =?UTF-8?q?d=20coverage=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fail if any EpcPropertyData field is neither reconstructed by _compose nor on a documented allow-list, turning latent persistence gaps into explicit decisions (would have caught the conservatory and roof-window drops). ADR-0036. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test_epc_persistence_field_coverage.py | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 tests/repositories/epc/test_epc_persistence_field_coverage.py diff --git a/tests/repositories/epc/test_epc_persistence_field_coverage.py b/tests/repositories/epc/test_epc_persistence_field_coverage.py new file mode 100644 index 00000000..8f59c336 --- /dev/null +++ b/tests/repositories/epc/test_epc_persistence_field_coverage.py @@ -0,0 +1,92 @@ +"""Structural guard: every `EpcPropertyData` field must round-trip. + +The deep-equality round-trip test (`test_epc_round_trip.py`) only catches a +dropped field when a *fixture populates it* — the existing gaps (conservatory, +roof windows, solar-HW collector) were latent precisely because no fixture +exercised them (see `docs/migrations/epc-property-round-trip-fidelity.md`). + +This guard closes that blind spot structurally: it asserts that every field on +`EpcPropertyData` is either reconstructed by `EpcPostgresRepository._compose` +(the DB → domain mapper) or listed — with a reason — in the allow-list below. +Adding a new domain field therefore forces a conscious persist-or-justify +decision; persisting a previously-gapped field means deleting its allow-list +entry. ADR-0036. +""" + +from __future__ import annotations + +import ast +import dataclasses +import inspect +import textwrap + +from datatypes.epc.domain.epc_property_data import EpcPropertyData +from repositories.epc.epc_postgres_repository import EpcPostgresRepository + +# Fields deliberately NOT reconstructed by `_compose`, each with its reason. +_UNPERSISTED_ALLOWLIST: dict[str, str] = { + # Redundant top-level field: the calculator reads + # `sap_ventilation.extract_fans_count` (which round-trips via + # `_to_ventilation`), never this top-level duplicate. + "extract_fans_count": "redundant; scoring uses sap_ventilation.extract_fans_count", + # Not read by the calculator (dormant); no DB column yet. + "air_tightness": "dormant — not read by the calculator; no FE column", + "lzc_energy_sources": "dormant — not read by the calculator; no FE column", + # Scoring-relevant round-trip gaps awaiting FE columns / child table + # (tracked follow-ups; see docs/migrations/epc-property-round-trip-fidelity.md). + "sap_roof_windows": "FE child table pending — tracked round-trip gap", + "solar_hw_collector_orientation": "FE column pending — tracked round-trip gap", + "solar_hw_collector_pitch_deg": "FE column pending — tracked round-trip gap", + "solar_hw_overshading": "FE column pending — tracked round-trip gap", +} + + +def _compose_reconstructed_fields() -> set[str]: + """The keyword names `_compose` passes to the top-level `EpcPropertyData(...)` + constructor — i.e. the fields it reconstructs from the DB row.""" + source = textwrap.dedent(inspect.getsource(EpcPostgresRepository._compose)) + tree = ast.parse(source) + for node in ast.walk(tree): + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "EpcPropertyData" + ): + return {kw.arg for kw in node.keywords if kw.arg is not None} + raise AssertionError("no EpcPropertyData(...) construction found in _compose") + + +def test_every_epc_property_data_field_is_persisted_or_allowlisted() -> None: + # Arrange + all_fields = {f.name for f in dataclasses.fields(EpcPropertyData)} + reconstructed = _compose_reconstructed_fields() + + # Act — fields neither reconstructed nor explicitly excused. + silently_dropped = all_fields - reconstructed - set(_UNPERSISTED_ALLOWLIST) + + # Assert — nothing falls through the round-trip unnoticed. + assert silently_dropped == set(), ( + "EpcPropertyData field(s) are dropped on DB round-trip without a " + f"documented reason: {sorted(silently_dropped)}. Either reconstruct them " + "in EpcPostgresRepository._compose, or add them to " + "_UNPERSISTED_ALLOWLIST with a justification." + ) + + +def test_allowlist_has_no_stale_entries() -> None: + # Arrange + all_fields = {f.name for f in dataclasses.fields(EpcPropertyData)} + reconstructed = _compose_reconstructed_fields() + + # Act — allow-list entries that are now persisted, or no longer fields. + redundant = { + name + for name in _UNPERSISTED_ALLOWLIST + if name in reconstructed or name not in all_fields + } + + # Assert — the allow-list stays honest as gaps are closed. + assert redundant == set(), ( + f"stale _UNPERSISTED_ALLOWLIST entries (now persisted or removed): " + f"{sorted(redundant)} — delete them." + ) From 1a059c75bb7899781ad63eb501773e09fa35ba48 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 24 Jun 2026 13:56:05 +0000 Subject: [PATCH 4/4] =?UTF-8?q?Document=20non-separated=20conservatory=20h?= =?UTF-8?q?andling=20(terms,=20ADR,=20migration)=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CONTEXT.md gains Non-Separated / Separated Conservatory glossary terms; ADR-0036 records the mapper-owned §6.1 split + 1:1 persistence + round-trip contract; the migration note lists the five new epc_property columns for the FE schema repo. Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 8 ++ ...mapper-split-and-round-trip-persistence.md | 78 +++++++++++++++++++ .../epc-property-round-trip-fidelity.md | 21 ++++- 3 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 docs/adr/0036-non-separated-conservatory-mapper-split-and-round-trip-persistence.md diff --git a/CONTEXT.md b/CONTEXT.md index 04116f87..f92c5b53 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -39,6 +39,14 @@ _Avoid_: schema version, EPC format An EPC issued for a residential dwelling, as opposed to a commercial one. _Avoid_: residential EPC, home EPC +**Non-Separated Conservatory**: +A conservatory joined to the dwelling without an external-quality thermal separation (open archway or only an internal-quality door). RdSAP treats it as **part of the dwelling** (§6.1): the assessor measures its glazed perimeter, floor area, glazing type and storey height, and its floor area, glazed walls (as a window), glazed roof (as a rooflight) and ground-loss all fold into the dwelling's score. On the gov register this is `conservatory_type = 4`, lodged as a glazed "building part" carrying only those four measurements — never fabric or floor dimensions. It is the only conservatory kind that affects the score, so it must be **carried as a distinct part of the dwelling picture**, not treated as a generic fabric building part (which mis-scores it and lacks the construction fields the register requires). +_Avoid_: "floor-only building part" / "degenerate part" (it is a fully-measured glazed conservatory — it only *looks* empty when a schema drops its four fields); "heated conservatory" (heating is the question asked of a **Separated Conservatory**, not this one) + +**Separated Conservatory**: +A conservatory partitioned from the dwelling by an external-quality door (substantial, draught-sealed). RdSAP **disregards** it entirely (§6.2) — no measurements are taken and it contributes nothing to the score; the register records only whether it has fixed heaters (`conservatory_type = 2` unheated / `3` heated) and lodges no glazed building part. `conservatory_type = 1` means no conservatory at all. +_Avoid_: conflating with a **Non-Separated Conservatory** (the separated one is scored as if absent); "thermally separated" as a synonym for "heated" (separation is about the door, heating is a further question asked only once separated) + ### Properties and addresses **Property**: diff --git a/docs/adr/0036-non-separated-conservatory-mapper-split-and-round-trip-persistence.md b/docs/adr/0036-non-separated-conservatory-mapper-split-and-round-trip-persistence.md new file mode 100644 index 00000000..ac5c0147 --- /dev/null +++ b/docs/adr/0036-non-separated-conservatory-mapper-split-and-round-trip-persistence.md @@ -0,0 +1,78 @@ +--- +Status: accepted +--- + +# Non-separated conservatory: mapper-owned §6.1 split, persisted 1:1, with round-trip fidelity as the contract + +Decided in a `/grill-with-docs` session (2026-06-24) while fixing the gov-API +conservatory path (repro cert 718138, `RdSAP-Schema-19.0`). Extends +[ADR-0015](0015-mappers-own-cert-normalization.md) (mappers own cert +normalization) and echoes [ADR-0035](0035-coherent-heating-system-synthesis.md) +(coherence is a synthesis-time obligation, never a calculator normalisation). + +## Context + +The gov register lodges a **Non-Separated Conservatory** (`conservatory_type = 4`) +as a glazed "building part" carrying only `{floor_area, room_height, +double_glazed, glazed_perimeter}` — no construction age band, no wall/roof, no +floor dimensions. Treated as a generic fabric building part it is **mis-scored** +(its floor heat-loss is counted as a normal part) and **cannot persist** (the +`epc_building_part` columns `construction_age_band` / `wall_construction` are +NOT-NULL). The correct RdSAP treatment folds it into the dwelling under §6.1; a +**Separated Conservatory** (`type 2/3`) is disregarded under §6.2 and lodges no +glazed part. See CONTEXT.md for both terms. + +Two placements were available for the §6.1 "split" (detect the glazed part, lift +it out of the fabric parts, carry it as the dwelling's conservatory): + +- in the **mapper**, producing a normalized `EpcPropertyData` with + `sap_conservatory` set and the glazed part excluded from `sap_building_parts` + (the existing 21.0.1 choice, commit `d501535c`); +- at the **calculator / persistence-reload boundary**, re-deriving the split + each time the picture is scored. + +The production flow is *map → save → reload-from-DB → score*. The mapper split +alone is invisible to the scored picture, because the reload (`_compose`) did not +reconstruct `sap_conservatory` and the glazed part was excluded from the +persisted `sap_building_parts` — so the conservatory was silently dropped on the +path that actually scores. This is the same latent gap the 21.0.1 fix carried. + +## Decision + +1. **The §6.1 split lives in the mapper.** It is cert-shape knowledge, not + physics (ADR-0015). `EpcPropertyData` — `sap_conservatory` populated, glazed + part excluded from `sap_building_parts` — is the canonical normalized shape. + We do **not** re-derive the split in the calculator or at reload; a global + coherence pass over the assembled picture is rejected for the same reason + ADR-0035 rejected one for heating. +2. **Persist the conservatory 1:1 on `epc_property`.** `SapConservatory` is a + single optional value per dwelling, so it is five nullable columns on + `epc_property` (alongside the existing scalar conservatory flags), not a child + table. Save writes them; `_compose` rebuilds `SapConservatory` from them. The + columns are FE/Drizzle-owned — documented in + `docs/migrations/epc-property-round-trip-fidelity.md`, applied there, never by + this repo. +3. **Round-trip fidelity is the contract that makes the split correct.** Because + the split lives only in the mapper, the *only* thing guaranteeing the scored + (reloaded) picture matches the mapped one is that `EpcPropertyData` survives + `save → get` unchanged. We treat that as an enforced invariant: the + deep-equality round-trip test (Model#1129) gains a non-separated-conservatory + case, and a structural field-coverage guard fails if any `EpcPropertyData` + field is neither reconstructed by `_compose` nor on a documented allow-list — + turning "latent until a fixture exercises it" into an explicit decision. + +## Consequences + +- A future reader sees a conservatory — a "building part" in the source — + persisted as flat columns on `epc_property` and absent from the dwelling's + building parts. That is deliberate: it is a §6.1 dwelling attribute, not a + fabric part. +- The persistence + guard are schema-version-agnostic, so they close the same + latent gap for the already-merged 21.0.1 path, not only 19.0. +- The structural guard documents `sap_roof_windows` and the `solar_hw_collector_*` + fields as known round-trip gaps (their FE columns are tracked follow-ups), so + they are visible rather than silently dropped. +- The per-schema mapper piece (declare the four glazed fields + filter + fold) + still has to be ported to the remaining gov mappers (17.0/17.1/18.0/20.0.0/ + 21.0.0) one validated cert at a time; this ADR covers the shared shape, not + that rollout. diff --git a/docs/migrations/epc-property-round-trip-fidelity.md b/docs/migrations/epc-property-round-trip-fidelity.md index 53590a2f..a23d0f62 100644 --- a/docs/migrations/epc-property-round-trip-fidelity.md +++ b/docs/migrations/epc-property-round-trip-fidelity.md @@ -48,11 +48,21 @@ and **still require the matching DB migration** wherever the physical tables liv - **§2.1 `epc_renewable_heat_incentive` table** (#1137) — now created on the SQLModel and wired into save/get; the round-trip test asserts **full deep-equality** (no exclusion). DB migration still required. +- **§3.1 conservatory geometry** (ADR-0036) — the five `conservatory_*` columns are now on the + SQLModel and wired into save/`_compose`; the round-trip test gains a non-separated-conservatory + case (deep-equality). DB migration still required. This closes the §6.1 conservatory round-trip + for **every** gov schema, not just the 19.0 cert that surfaced it. + +**Structural guard (ADR-0036):** `tests/repositories/epc/test_epc_persistence_field_coverage.py` +now fails if any `EpcPropertyData` field is neither reconstructed by `_compose` nor on a documented +allow-list — so the gaps below are no longer "latent until a fixture exercises them", they are +explicit and tracked. **Still open (follow-up issues):** the remaining §2 structural tables (room-in-roof detail, PV -arrays, roof windows) + §3 nested-wall fields (`SapAlternativeWall.u_value`/`wall_thickness_mm`) + -`SapFloorDimension` exposed-floor flags — none populated in the 21.0.0/21.0.1 fixtures, so latent -until a richer fixture exercises them. +arrays, **roof windows** — `sap_roof_windows`) + §3 nested-wall fields +(`SapAlternativeWall.u_value`/`wall_thickness_mm`) + `SapFloorDimension` exposed-floor flags + the +solar-HW collector columns (`solar_hw_collector_orientation` / `_pitch_deg` / `_overshading`, §3.1). +Each is on the structural-guard allow-list with a reason until its columns land. --- @@ -124,6 +134,11 @@ the Type-2 geometry and the Detailed-measurement surfaces. Replace with a child ### 3.1 `epc_property` additions | Column | Type | Source | Pri | |---|---|---|---| +| `conservatory_floor_area_m2` | float (double precision), null | `sap_conservatory.floor_area_m2` | **P0** | +| `conservatory_glazed_perimeter_m` | float (double precision), null | `sap_conservatory.glazed_perimeter_m` | **P0** | +| `conservatory_double_glazed` | bool, null | `sap_conservatory.double_glazed` | **P0** | +| `conservatory_thermally_separated` | bool, null | `sap_conservatory.thermally_separated` | **P0** | +| `conservatory_room_height_storeys` | float (double precision), null | `sap_conservatory.room_height_storeys` | **P0** | | `addendum_stone_walls` | bool, null | `addendum.stone_walls` | P2 | | `addendum_system_build` | bool, null | `addendum.system_build` | P2 | | `addendum_numbers` | JSONB, null | `addendum.addendum_numbers` (`List[int]`) | P2 |