Merge pull request #1377 from Hestia-Homes/fix/1376-roof-party-ceiling-guard

Guard party-ceiling roof overrides so they never score as external roofs (#1376)

applications/landlord_description_overrides/handler.py

@@ -14,6 +14,12 @@ from domain.epc.property_overrides.main_fuel_type import MainFuelType
 from domain.epc.property_overrides.main_heating_system_type import MainHeatingSystemType
 from domain.epc.property_overrides.property_type import PropertyType
 from domain.epc.property_overrides.roof_type import RoofType
+from domain.epc.property_overrides.roof_party_ceiling_guard import (
+    roof_party_ceiling_guard,
+)
+from domain.data_transformation.guarded_column_classifier import (
+    GuardedColumnClassifier,
+)
 from domain.epc.property_overrides.water_heating_type import WaterHeatingType
 from domain.epc.property_overrides.wall_type import WallType
 from domain.epc.property_overrides.wall_type_construction_dates import (
@@ -115,8 +121,13 @@ def _build_columns(
         "roof_type": lambda src: ClassifiableColumn(
             name="roof_type",
             source_column=src,
-            classifier=ChatGptColumnClassifier(
-                chat_gpt, RoofType, RoofType.UNKNOWN
+            # A party ceiling ("another/same dwelling or premises above") has ~0
+            # heat loss and must never be classified as an external roof; the
+            # deterministic guard resolves those markers and the LLM handles the
+            # rest (#1376).
+            classifier=GuardedColumnClassifier(
+                guard=roof_party_ceiling_guard,
+                fallback=ChatGptColumnClassifier(chat_gpt, RoofType, RoofType.UNKNOWN),
             ),
             repo=LandlordOverridesRepository[RoofType](
                 session, LandlordRoofTypeOverrideRow

docs/adr/0041-flat-roofs-scored-by-insulation-thickness.md

@@ -0,0 +1,30 @@
+# Flat roofs are scored by insulation thickness (RdSAP Table 16 col 1), not the age-band default
+
+## Context
+
+A **Landlord Override** naming a flat roof with a known insulation depth (`flat: 50mm` / `100mm` / `150mm`) is classified to the canonical `RoofType` value `"Flat, insulated (assumed)"`, which the roof **Simulation Overlay** resolves to `roof_construction_type='Flat'` with **no thickness**. The calculator then takes the U-value from the flat age-band default (`_FLAT_ROOF_BY_AGE`, RdSAP Table 18 column (3)). The stated depth is **discarded** — ~102 `property_overrides` rows.
+
+The overlay's own comment asserted *"No flat RoofType value carries an explicit mm depth"*. That is a **taxonomy artefact, not a truth about the data**: the `RoofType` enum has flat members only for insulation *state* (`FLAT_INSULATED / _ASSUMED / LIMITED / NO_INSULATION`) and **no flat-thickness members** — whereas pitched has the full `PITCHED_LOFT_12MM … 400MM` ladder. So a flat depth has nowhere to land and is lost at **classification**, before the overlay ever runs.
+
+Reviewing the **RdSAP 10 Specification (10-06-2025)** during grilling (issue #1361 override audit / #1376) settled the domain fact: **Table 16** ("Roof U-values when loft insulation thickness is known"), **column (1)**, is headed *"Insulation at joists at ceiling level **and flat roof**"*. A flat roof **is** scored by thickness — 50 mm → 0.68, 100 mm → 0.40, 150 mm → 0.30. Table 18 (the age-band default) is explicitly only the fallback *"used when thickness of insulation cannot be determined"*. The earlier assumption that flat roofs are age-band-only was wrong.
+
+The calculator already implements this correctly: `u_roof` routes a flat roof **with** a thickness through `_ROOF_BY_THICKNESS` (which *is* Table 16 col (1)). So a flat roof given its depth scores right today — the only defect is that the depth never reaches the calculator, because the taxonomy can't carry it.
+
+This is the opposite of the wall / flat "as built (assumed)" cases (ADR-0033), where **no** real datum exists and the age band is the correct lever. Here a **real measurement** exists and RdSAP scores it.
+
+## Decision
+
+Make a flat roof's known insulation thickness first-class, so it reaches Table 16 col (1):
+
+- Add **flat-thickness members** to the `RoofType` taxonomy — `FLAT_12MM … FLAT_400_PLUS_MM` (values like `"Flat, 150 mm insulation"` — **not** "loft"; a flat roof has no loft). Minimum set for today's data: 50, 100, 150; the full ladder mirrors the pitched members for symmetry.
+- The roof **Simulation Overlay** emits `roof_insulation_thickness` for a flat roof with a known depth (reusing the existing mm regex), alongside `roof_construction_type='Flat'`, so `u_roof` reaches Table 16 col (1) instead of the age-band default.
+- **Reclassify** the existing `flat: Nmm` rows off `"Flat, insulated (assumed)"` onto the new members.
+- Correct the misleading overlay comment.
+
+The flat/pitched distinction is retained even though Table 16 col (1) gives the same U at a given thickness for both — it stays load-bearing for non-U concerns (measure eligibility, shape), so flat depths are **not** collapsed onto the pitched members.
+
+## Consequences
+
+- ~102 flat-roof properties re-score from the age-band default to their true thickness-based U-value — **both directions** (a newer-band flat roof with 150 mm improves; an old-band flat with 50 mm may worsen relative to its default).
+- **Cross-repo (FE-owned pgEnum, Dan).** The `RoofType` `value` column is a Drizzle-owned Postgres enum (see `main-heating-system-pgenum-is-fe-owned`; PR #1361 Class A/B). The new members must be added to the pgEnum by the FE owner before the classifier-cache `value` writes; the `property_overrides` (TEXT) reclassify is immediate. So this is an **enum-dependent slice**, grouped with the other new-archetype slices of #1376 — not the no-enum roof/glazing resolver slice.
+- The reclassify follows the established one-time-script shape (dry-run default, `--apply` in a transaction, idempotent), and surfaces any members the live enum does not yet carry as deferred, exactly as Class A did.

docs/adr/0042-glazing-override-reconciles-against-cert-composition.md

@@ -0,0 +1,31 @@
+# Landlord glazing override reconciles against the cert's per-window composition, not a whole-dwelling type
+
+## Context
+
+A **Landlord Override** for glazing is a whole-dwelling categorical (`GlazingType` — Single / Double pre- and post-2002 / Triple / Unknown). The glazing **Simulation Overlay** resolves it to one SAP10 `glazing_type` code, and `_fold_glazing` overwrites **every** window's `glazing_type` (and clears its lodged U-/g-value) — flattening the cert to a single type.
+
+The landlord descriptions are frequently **aggregate splits** ("40% double, 60% single"), and the LLM classifier collapses *any-double-present* to `"Double glazing"`. So ~319 `property_overrides` rows describing a **single-dominant** mix are scored as **fully double-glazed** — a material over-credit to the window U-values, hence to SAP.
+
+The Effective EPC already carries **per-window glazing**: `sap_windows`, each with its own `glazing_type` **and** `window_width`×`window_height` (so area is derivable), plus the dwelling-level `multiple_glazed_proportion`. This is **more granular** than any whole-dwelling summary, and a whole-dwelling percentage **cannot be faithfully assigned to specific windows** — "which windows are the single ones" is unknowable from an aggregate. `_fold_glazing` already holds the EPC at apply time, so the per-window data is available where the override is applied.
+
+Fixing this by "dominant type wins" would still **clobber real per-window variation** (a 60/40 dwelling flattened to one type). The faithful move is to treat the cert's per-window data as **authoritative** and the landlord aggregate as a **check** on it — the descriptions are, after all, aggregations of the same per-window split the cert records.
+
+## Decision
+
+The landlord glazing override **reconciles against the cert's per-window composition** instead of imposing a whole-dwelling type.
+
+- **Classifier emits a proportion, not just a dominant type.** The asserted glazing **proportion** (e.g. multiple-glazed %) is extracted by the **LLM** — which is there precisely because landlord inputs vary; we do **not** hard-parse one string format.
+- **Compute the cert's actual composition** from `sap_windows`, **area-weighted** by window (bigger windows dominate the dwelling U-window).
+- **Reconcile in `_fold_glazing`** (it holds the EPC), three outcomes:
+  - **Uniform assertion (~100% one type)** → apply the blanket type. Unambiguous, and a real correction (e.g. a full reglaze). Unchanged from today for clean cases.
+  - **Mixed, proportion within tolerance of the cert** → **no-op**. The cert already reflects it, per-window and more precisely — leave the better data alone.
+  - **Mixed, proportion materially different from the cert** → **no-op + flag**. The landlord genuinely disagrees, but per-window assignment is unknowable, so **do not fabricate a split** — surface it for review.
+- **Tolerance** is a tunable band on the area-weighted multiple-glazed %, pinned against the real distribution, not guessed.
+
+## Consequences
+
+- The ~319 over-credited rows stop being flattened to double; a mix that matches the cert is left on its per-window data, and a clean uniform reglaze still applies.
+- **No new enum.** A no-op reconciliation leaves the cert glazing untouched (as `Unknown` already does). But it needs a **classifier-output change** (emit a proportion) **plus** the reconcile in the **apply seam** (`_fold_glazing`) — so it is its **own slice**, distinct from the no-enum roof party-ceiling fix that ships first.
+- Genuine landlord-vs-cert glazing disagreements are **surfaced** rather than silently trusted (old behaviour: flatten to double) or silently overwritten.
+- **Alternatives rejected.** *Dominant type wins* — still clobbers per-window variation. *Carry a full landlord composition to re-derive per-window types* — per-window assignment from an aggregate is unknowable; we decline to fabricate it. *Deterministic regex of "X% double, Y% single"* — brittle against varied input, which is the LLM's job.
+- Pairs with the per-window fidelity the calculator already relies on: the reconciliation is only correct on a faithful Effective EPC whose `sap_windows` round-trip (cf. ADR-0040).

domain/data_transformation/guarded_column_classifier.py

@@ -0,0 +1,42 @@
+from __future__ import annotations
+
+from enum import Enum
+from typing import Callable, Optional, TypeVar
+
+from domain.data_transformation.column_classifier import ColumnClassifier
+
+E = TypeVar("E", bound=Enum)
+
+
+class GuardedColumnClassifier(ColumnClassifier[E]):
+    """A ``ColumnClassifier`` that resolves the descriptions a deterministic guard
+    is certain about, and delegates the rest to a fallback classifier.
+
+    The ``guard`` maps a raw description to a category member when it recognises it
+    deterministically (e.g. a party-ceiling roof marker — #1376), else ``None``.
+    Guard hits never reach the fallback, so an unreliable classifier (the LLM)
+    cannot override a description the guard is sure of — and the LLM is not billed
+    for it. Every description still appears in the result (guarded or fallen-back).
+    """
+
+    def __init__(
+        self,
+        guard: Callable[[str], Optional[E]],
+        fallback: ColumnClassifier[E],
+    ) -> None:
+        self._guard = guard
+        self._fallback = fallback
+
+    def classify(self, descriptions: set[str]) -> dict[str, E]:
+        guarded: dict[str, E] = {}
+        misses: set[str] = set()
+        for description in descriptions:
+            member = self._guard(description)
+            if member is not None:
+                guarded[description] = member
+            else:
+                misses.add(description)
+        # Only the misses reach the fallback — a fully-guarded batch never calls it.
+        if misses:
+            guarded.update(self._fallback.classify(misses))
+        return guarded

domain/epc/property_overrides/roof_party_ceiling_guard.py

@@ -0,0 +1,43 @@
+from __future__ import annotations
+
+import re
+from typing import Optional
+
+from domain.epc.property_overrides.roof_type import RoofType
+
+# The roof-type token (before any ``: <insulation>`` suffix), normalised to lower
+# alphanumerics, → its party-ceiling RoofType member. Normalising strips spacing
+# and case so both ``"another dwelling above"`` and ``"anotherdwellingabove"``
+# match the same marker.
+_PARTY_CEILING_MARKERS: dict[str, RoofType] = {
+    "anotherdwellingabove": RoofType.ADJACENT_ANOTHER_DWELLING_ABOVE,
+    "samedwellingabove": RoofType.ADJACENT_SAME_DWELLING_ABOVE,
+    "otherpremisesabove": RoofType.ADJACENT_OTHER_PREMISES_ABOVE,
+    # Both the "(another premises above)" adjacency and the redundant "Another
+    # Premises Above" taxonomy member normalise here; map to the parenthesised
+    # family (both resolve to no overlay, so there is no scoring difference).
+    "anotherpremisesabove": RoofType.ADJACENT_ANOTHER_PREMISES_ABOVE,
+}
+
+
+def _normalise_roof_token(description: str) -> str:
+    token = description.split(":", 1)[0]
+    return re.sub(r"[^a-z0-9]", "", token.lower())
+
+
+def roof_party_ceiling_guard(description: str) -> Optional[RoofType]:
+    """Deterministically resolve a party-ceiling roof description to its RoofType.
+
+    A building part whose top boundary is another (or the same) dwelling / premises
+    above has ~0 heat loss (RdSAP 10 Table 18: "There is no heat loss through the
+    roof of a building part that has the same dwelling or another dwelling above"),
+    so it must resolve to a party-ceiling `RoofType` member — which the roof
+    Simulation Overlay leaves as no overlay, keeping the lodged EPC — and never to
+    an external `Pitched` / `Flat` roof.
+
+    Recognises the party-ceiling markers regardless of a trailing insulation token
+    (``: 100mm`` / ``: unknown``) or spacing/case, and returns ``None`` for anything
+    that is not a party-ceiling marker so the LLM classifier still handles it
+    (#1376).
+    """
+    return _PARTY_CEILING_MARKERS.get(_normalise_roof_token(description))

scripts/reclassify_party_ceiling_roofs.py

@@ -0,0 +1,132 @@
+"""One-time re-classification of party-ceiling roof overrides mis-read as roofs.
+
+#1376: a landlord roof description that is a **party-ceiling** marker ("another /
+same dwelling or premises above") was occasionally classified by the LLM to an
+external ``Pitched, N mm loft insulation`` value when it carried a trailing depth —
+inventing roof heat loss where a party ceiling has ~0 (RdSAP 10 Table 18: "There
+is no heat loss through the roof of a building part that has the same dwelling or
+another dwelling above"). ~106 ``property_overrides`` rows (party-ceiling markers
+on any non-party-ceiling value), inconsistent with the ~13k of the same family
+already resolving to the party-ceiling member.
+
+The live classifier now applies ``roof_party_ceiling_guard`` deterministically
+(so new intakes are correct); this fixes the rows written before it. The **same
+guard** decides the correction here, so the backfill and the live path cannot
+drift.
+
+Updates the TEXT ``property_overrides.override_value`` (what the modelling reads —
+the actual fix) and the ``landlord_roof_type_overrides.value`` classifier cache.
+The party-ceiling members already exist in the roof pgEnum (13k rows store them),
+so no FE migration is needed.
+
+DRY-RUN BY DEFAULT: prints the row count it would change and writes nothing. Pass
+``--apply`` to execute inside a transaction. Idempotent — only rows whose stored
+value differs from the guard's member are touched, so a second run is a no-op.
+"""
+
+from __future__ import annotations
+
+import argparse
+from collections.abc import Iterable
+
+from sqlalchemy import Connection, text
+
+from domain.epc.property_overrides.roof_party_ceiling_guard import (
+    roof_party_ceiling_guard,
+)
+from scripts.e2e_common import build_engine, load_env
+
+
+def party_ceiling_corrections(
+    stored: Iterable[tuple[str, str]],
+) -> dict[str, str]:
+    """``(description, stored override_value)`` → the corrected value, for the
+    descriptions the party-ceiling guard resolves whose stored value is not already
+    the guard's member. Descriptions the guard leaves to the LLM (``None``) and
+    rows already on the right member are omitted — so the result is exactly the set
+    to fix, and re-running against corrected data yields an empty map (idempotent).
+    """
+    corrections: dict[str, str] = {}
+    for description, value in stored:
+        member = roof_party_ceiling_guard(description)
+        if member is not None and value != member.value:
+            corrections[description] = member.value
+    return corrections
+
+
+_DISTINCT = text(
+    """
+    SELECT DISTINCT lower(original_spreadsheet_description) AS description,
+                    override_value AS value
+    FROM property_overrides
+    WHERE override_component = 'roof_type'
+    """
+)
+_OVERRIDES_UPDATE = text(
+    """
+    UPDATE property_overrides
+       SET override_value = :new_value
+     WHERE override_component = 'roof_type'
+       AND lower(original_spreadsheet_description) = :description
+       AND override_value <> :new_value
+    """
+)
+_CACHE_UPDATE = text(
+    """
+    UPDATE landlord_roof_type_overrides
+       SET value = :new_value, updated_at = now()
+     WHERE lower(description) = :description
+       AND value::text <> :new_value
+    """
+)
+_OVERRIDES_COUNT = text(
+    """
+    SELECT count(*) FROM property_overrides
+     WHERE override_component = 'roof_type'
+       AND lower(original_spreadsheet_description) = :description
+       AND override_value <> :new_value
+    """
+)
+
+
+def reclassify(conn: Connection, *, apply: bool) -> int:
+    """NULL-free re-map: set every mis-classified party-ceiling roof override to the
+    guard's member. Returns the number of ``property_overrides`` rows found (that
+    ``--apply`` would / did correct)."""
+    stored = [(r.description, r.value) for r in conn.execute(_DISTINCT)]
+    total = 0
+    for description, new_value in party_ceiling_corrections(stored).items():
+        params = {"description": description, "new_value": new_value}
+        total += conn.execute(_OVERRIDES_COUNT, params).scalar() or 0
+        if apply:
+            conn.execute(_OVERRIDES_UPDATE, params)
+            conn.execute(_CACHE_UPDATE, params)
+    return total
+
+
+def main() -> None:
+    load_env()
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument(
+        "--apply",
+        action="store_true",
+        help="execute the updates (default: dry-run, writes nothing)",
+    )
+    args = parser.parse_args()
+
+    engine = build_engine()
+    with engine.begin() as conn:
+        conn.execute(text("SET statement_timeout = 120000"))
+        total = reclassify(conn, apply=args.apply)
+
+    verb = "re-classified" if args.apply else "would re-classify"
+    print(
+        f"{verb} {total} party-ceiling roof override row(s) from an external "
+        "roof value to the party-ceiling member."
+    )
+    if not args.apply:
+        print("\nDRY-RUN — nothing written. Re-run with --apply to execute.")
+
+
+if __name__ == "__main__":
+    main()

tests/domain/data_transformation/test_guarded_column_classifier.py

@@ -0,0 +1,45 @@
+from __future__ import annotations
+
+from enum import Enum
+from typing import Optional
+
+from domain.data_transformation.column_classifier import ColumnClassifier
+from domain.data_transformation.guarded_column_classifier import (
+    GuardedColumnClassifier,
+)
+
+
+class _Category(Enum):
+    GUARDED = "guarded"
+    FALLBACK = "fallback"
+    UNKNOWN = "unknown"
+
+
+class _RecordingFallback(ColumnClassifier[_Category]):
+    """Stands in for the LLM: records what it was asked and maps everything it
+    sees to FALLBACK, so the test can see which descriptions reached it."""
+
+    def __init__(self) -> None:
+        self.asked: set[str] = set()
+
+    def classify(self, descriptions: set[str]) -> dict[str, _Category]:
+        self.asked = set(descriptions)
+        return {d: _Category.FALLBACK for d in descriptions}
+
+
+def _guard(description: str) -> Optional[_Category]:
+    return _Category.GUARDED if description == "marker" else None
+
+
+def test_guard_hits_win_and_misses_fall_through_to_the_fallback() -> None:
+    # Arrange
+    fallback = _RecordingFallback()
+    classifier = GuardedColumnClassifier(guard=_guard, fallback=fallback)
+
+    # Act
+    result = classifier.classify({"marker", "other"})
+
+    # Assert — the guarded description takes the guard's member and never reaches
+    # the fallback; the unrecognised one is resolved by the fallback.
+    assert result == {"marker": _Category.GUARDED, "other": _Category.FALLBACK}
+    assert fallback.asked == {"other"}

tests/domain/epc/test_roof_party_ceiling_guard.py

@@ -0,0 +1,62 @@
+from __future__ import annotations
+
+import pytest
+
+from domain.epc.property_overrides.roof_type import RoofType
+from domain.epc.property_overrides.roof_party_ceiling_guard import (
+    roof_party_ceiling_guard,
+)
+
+
+def test_party_ceiling_marker_with_a_trailing_depth_resolves_to_the_party_ceiling_member() -> None:
+    # Arrange — the bug: a party ceiling ("another dwelling above") carrying a
+    # trailing loft depth the LLM misreads as an external pitched roof.
+    description = "another dwelling above: 100mm"
+
+    # Act
+    result = roof_party_ceiling_guard(description)
+
+    # Assert — it is a party ceiling (~0 heat loss), not a Pitched roof.
+    assert result is RoofType.ADJACENT_ANOTHER_DWELLING_ABOVE
+
+
+@pytest.mark.parametrize(
+    ("description", "expected"),
+    [
+        ("same dwelling above: unknown", RoofType.ADJACENT_SAME_DWELLING_ABOVE),
+        ("other premises above", RoofType.ADJACENT_OTHER_PREMISES_ABOVE),
+        ("another premises above: 150mm", RoofType.ADJACENT_ANOTHER_PREMISES_ABOVE),
+        # the unspaced source form and the "Another Premises Above" member value
+        # both normalise to the same marker.
+        ("Another Premises Above", RoofType.ADJACENT_ANOTHER_PREMISES_ABOVE),
+        ("samedwellingabove: 300mm", RoofType.ADJACENT_SAME_DWELLING_ABOVE),
+    ],
+)
+def test_every_party_ceiling_marker_variant_resolves_to_its_member(
+    description: str, expected: RoofType
+) -> None:
+    # Act
+    result = roof_party_ceiling_guard(description)
+
+    # Assert
+    assert result is expected
+
+
+@pytest.mark.parametrize(
+    "description",
+    [
+        "pitched, normal loft access: 100mm",
+        "flat: 150mm",
+        "pitched with sloping ceiling: unknown",
+        "roof room(s), insulated",
+    ],
+)
+def test_a_genuine_roof_description_is_left_to_the_llm(description: str) -> None:
+    # A non-party-ceiling roof must return None so the LLM classifier still
+    # resolves it — the guard only claims the markers it is certain about.
+
+    # Act
+    result = roof_party_ceiling_guard(description)
+
+    # Assert
+    assert result is None

tests/scripts/test_reclassify_party_ceiling_roofs.py

@@ -0,0 +1,33 @@
+"""The party-ceiling roof reclassify corrects only the mis-read rows, to the
+guard's member, and is idempotent (#1376)."""
+
+from __future__ import annotations
+
+from scripts.reclassify_party_ceiling_roofs import party_ceiling_corrections
+
+
+def test_only_misclassified_party_ceiling_rows_are_corrected() -> None:
+    # Arrange — a party ceiling mis-read as an external pitched roof (the bug),
+    # one already on the correct member, and a genuine pitched roof.
+    stored = [
+        ("another dwelling above: 100mm", "Pitched, 100 mm loft insulation"),
+        ("another dwelling above: unknown", "(another dwelling above)"),
+        ("pitched, normal loft access: 150mm", "Pitched, 150 mm loft insulation"),
+    ]
+
+    # Act
+    corrections = party_ceiling_corrections(stored)
+
+    # Assert — only the mis-read party ceiling is corrected, to its member value;
+    # the already-correct row and the genuine pitched roof are left alone.
+    assert corrections == {
+        "another dwelling above: 100mm": "(another dwelling above)"
+    }
+
+
+def test_already_corrected_data_yields_no_further_corrections() -> None:
+    # Arrange — a re-run over data the first pass already fixed.
+    stored = [("another dwelling above: 100mm", "(another dwelling above)")]
+
+    # Act / Assert — idempotent: nothing left to change.
+    assert party_ceiling_corrections(stored) == {}