From f3a164c371da2fe2281712a0fc1c34630829cf54 Mon Sep 17 00:00:00 2001
From: Khalim Conn-Kowlessar <kconnkowlessar@gmail.com>
Date: Wed, 24 Jun 2026 13:56:05 +0000
Subject: [PATCH] =?UTF-8?q?Guard=20EpcPropertyData=20round-trip=20field=20?=
 =?UTF-8?q?coverage=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) <noreply@anthropic.com>
---
 .../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."
+    )