Merge pull request #1305 from Hestia-Homes/feature/e2e-runs

Feature/e2e runs

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**:

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

datatypes/epc/domain/tests/test_from_rdsap_schema.py

@@ -2460,3 +2460,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

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

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.

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`) +
+arrays, **roof windows** — `sap_roof_windows`) + §3 nested-wall fields
-`SapFloorDimension` exposed-floor flags — none populated in the 21.0.0/21.0.1 fixtures, so latent
+(`SapAlternativeWall.u_value`/`wall_thickness_mm`) + `SapFloorDimension` exposed-floor flags + the
-until a richer fixture exercises them.
+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 |

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,

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:

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."
+    )

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: