Merge pull request #1402 from Hestia-Homes/fix/glazing-dominant-single-guard

Resolve dominant-single glazing deterministically in the mix guard

domain/epc/property_overrides/glazing_mix_guard.py

@@ -5,12 +5,35 @@ from typing import Optional
 
 from domain.epc.property_overrides.glazing_type import GlazingType
 
-# "N% <base glazing type>" — the era (2002 / pre-2002) is irrelevant to whether it
-# is a mix, so only the base type is captured.
-_PERCENT_TYPE = re.compile(r"(\d+)%\s*(single|double|triple|secondary)")
+# "N% <base glazing type> [era]" — the base type decides whether it is a mix; the
+# era ("2002 or later" / "pre-2002") is captured too, so a dominant double/triple
+# can be resolved deterministically when its era is stated (see the guard below).
+# An absent or non-canonical era ("unknown age", "(SAP 9.94)") leaves the era group
+# empty, so that dominant split stays the LLM's job.
+_PERCENT_TYPE = re.compile(
+    r"(\d+)%\s*(single|double|triple|secondary)(?:\s*glazing)?"
+    r"(?:[\s,]*(pre-?2002|2002 or later))?"
+)
 # One base type at or above this share is a uniform assertion, not a mix — applied
 # as that type (a near-uniform reglaze), not deferred.
 _UNIFORM_THRESHOLD_PERCENT = 90
+# A dominant base type + stated era → the fully-determined canonical member. Single
+# glazing is era-free, so it needs no era. Double/triple with no (or non-canonical)
+# era is absent here and falls through to the LLM.
+_DOMINANT_MEMBER: dict[tuple[str, Optional[str]], GlazingType] = {
+    ("single", None): GlazingType.SINGLE,
+    ("double", "2002 or later"): GlazingType.DOUBLE_POST_2002,
+    ("double", "pre-2002"): GlazingType.DOUBLE_PRE_2002,
+    ("triple", "2002 or later"): GlazingType.TRIPLE_POST_2002,
+    ("triple", "pre-2002"): GlazingType.TRIPLE_PRE_2002,
+}
+
+
+def _canonical_era(era: str) -> Optional[str]:
+    """Normalise a matched era phrase to the key used in ``_DOMINANT_MEMBER``."""
+    if not era:
+        return None
+    return "2002 or later" if "2002 or later" in era else "pre-2002"
 
 
 def glazing_mix_guard(description: str) -> Optional[GlazingType]:
@@ -23,18 +46,36 @@ def glazing_mix_guard(description: str) -> Optional[GlazingType]:
     must resolve to ``GlazingType.MIXED`` (no overlay → the cert is kept), never to
     a single dominant type that would flatten every window (ADR-0042).
 
-    Recognises the structured ``"N% <type>, M% <type>"`` split and returns
-    ``MIXED`` when it is a genuine mix — two or more glazing types present and none
-    dominant (≥ the uniform threshold). Returns ``None`` for a uniform / near-
-    uniform assertion or an unparseable description, so the LLM classifier still
-    resolves it (a uniform reglaze is applied; varied phrasings are the LLM's job).
+    Recognises the structured ``"N% <type>, M% <type>"`` split and returns:
+
+    * ``MIXED`` for a genuine mix — two or more glazing types present and none
+      dominant (≥ the uniform threshold);
+    * the fully-determined member for a dominant near-uniform split whose type
+      carries no era ambiguity — single glazing (era-free), or a dominant
+      double/triple whose era is stated ("pre-2002" / "2002 or later"); the LLM
+      would otherwise flatten such a split onto its *minority* type;
+    * ``None`` otherwise — an unparseable description, or a dominant double/triple
+      with an unstated/non-canonical era ("unknown age") that still carries genuine
+      era ambiguity — so the LLM classifier resolves it.
     """
     matches = _PERCENT_TYPE.findall(description.lower())
-    base_types = {base_type for _, base_type in matches}
+    base_types = {base_type for _, base_type, _ in matches}
     if len(base_types) < 2:
         # One (or no) glazing type named — a uniform assertion, not a mix.
         return None
-    if max(int(percent) for percent, _ in matches) >= _UNIFORM_THRESHOLD_PERCENT:
-        # One type dominates — treat as that (near-)uniform type, not a mix.
-        return None
+    dominant_percent, dominant_type, dominant_era = max(
+        ((int(percent), base_type, era) for percent, base_type, era in matches),
+        key=lambda triple: triple[0],
+    )
+    if dominant_percent >= _UNIFORM_THRESHOLD_PERCENT:
+        # One type dominates — a near-uniform assertion, not a mix. A split is fully
+        # determined (and safe to apply deterministically) when the dominant type
+        # carries no era ambiguity: single glazing is era-free, and a dominant
+        # double/triple whose era is stated ("pre-2002" / "2002 or later") is
+        # equally pinned. In those cases claim the member ourselves — the LLM
+        # otherwise flattens the split onto the *minority* type. A dominant
+        # double/triple with an unstated or non-canonical era ("unknown age") still
+        # carries genuine era ambiguity, so it stays the LLM's job (falls through to
+        # ``None``).
+        return _DOMINANT_MEMBER.get((dominant_type, _canonical_era(dominant_era)))
     return GlazingType.MIXED

scripts/lisasrequest/reclassify_dominant_glazing.py

@@ -0,0 +1,125 @@
+"""Backfill glazing overrides where a dominant (>= 90%) split was flattened onto
+its *minority* type.
+
+Sibling to ``scripts/reclassify_mixed_glazing.py``. That script rescued genuine
+mixes (no type >= 90%) to ``MIXED``; it deliberately left near-uniform rows to the
+LLM classifier. But for a dominant split the LLM sometimes latched onto the
+minority type — a dominant-*single* dwelling ("4% Double glazing 2002 or later,
+96% Single Glazing") written as "Double glazing…", or the symmetric dominant-double
+case written as "Single glazing". ``glazing_mix_guard`` now resolves any
+era-unambiguous dominant split deterministically — SINGLE (era-free), or a
+double/triple whose era is stated — so this backfill fixes the rows written before
+that guard change.
+
+Uses the SAME guard as the live path, so the backfill and the classifier cannot
+drift. SCOPED TO ONE PORTFOLIO (``--portfolio``, default 796 = Hyde) and only
+touches ``property_overrides.override_value`` (TEXT — what the modelling reads);
+the global ``landlord_glazing_overrides`` classifier cache is left alone.
+
+DRY-RUN BY DEFAULT: prints what it would change and writes nothing. Pass
+``--apply`` to execute inside a transaction; it also writes an audit CSV of every
+row changed (property_id, uprn, old value, new value) so the change is reversible.
+Idempotent — only rows whose stored value differs from the guard's target member
+are touched.
+
+    python -m scripts.lisasrequest.reclassify_dominant_glazing            # dry run
+    python -m scripts.lisasrequest.reclassify_dominant_glazing --apply    # write
+"""
+
+from __future__ import annotations
+
+import argparse
+import csv
+import sys
+from collections import Counter
+from pathlib import Path
+
+from sqlalchemy import text
+
+_REPO_ROOT = Path(__file__).resolve().parents[2]
+sys.path.insert(0, str(_REPO_ROOT))
+
+from domain.epc.property_overrides.glazing_mix_guard import glazing_mix_guard  # noqa: E402
+from scripts.e2e_common import ENV_PATH, build_engine, load_env  # noqa: E402
+
+_SELECT = text(
+    """
+    SELECT po.property_id, pr.uprn,
+           po.original_spreadsheet_description AS description,
+           po.override_value AS value
+    FROM property_overrides po
+    JOIN property pr ON pr.id = po.property_id
+    WHERE po.portfolio_id = :portfolio
+      AND po.override_component = 'glazing'
+    """
+)
+_UPDATE = text(
+    """
+    UPDATE property_overrides
+       SET override_value = :new_value
+     WHERE portfolio_id = :portfolio
+       AND override_component = 'glazing'
+       AND property_id = :property_id
+       AND override_value <> :new_value
+    """
+)
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--portfolio", type=int, default=796)
+    parser.add_argument(
+        "--apply",
+        action="store_true",
+        help="execute the updates (default: dry-run, writes nothing)",
+    )
+    args = parser.parse_args()
+
+    load_env(ENV_PATH)
+    engine = build_engine()
+
+    audit: list[tuple[int, object, str, str, str]] = []  # pid, uprn, descr, old, new
+    tally: Counter[str] = Counter()
+    with engine.begin() as conn:
+        conn.execute(text("SET statement_timeout = 120000"))
+        for property_id, uprn, description, value in conn.execute(
+            _SELECT, {"portfolio": args.portfolio}
+        ):
+            member = glazing_mix_guard(description or "")
+            if member is None or value == member.value:
+                continue
+            tally[f"{value!r} -> {member.value!r}"] += 1
+            audit.append((property_id, uprn, description, value, member.value))
+            if args.apply:
+                conn.execute(
+                    _UPDATE,
+                    {
+                        "portfolio": args.portfolio,
+                        "property_id": property_id,
+                        "new_value": member.value,
+                    },
+                )
+        if not args.apply:
+            conn.rollback()
+
+    verb = "re-classified" if args.apply else "would re-classify"
+    print(f"portfolio {args.portfolio}: {verb} {len(audit)} glazing override row(s)")
+    for change, n in tally.most_common():
+        print(f"  {n:5d}  {change}")
+
+    if args.apply and audit:
+        out = _REPO_ROOT / "scripts" / "lisasrequest" / (
+            f"reclassify_dominant_glazing_{args.portfolio}_audit.csv"
+        )
+        with out.open("w", newline="") as fh:
+            w = csv.writer(fh)
+            w.writerow(["property_id", "uprn", "description", "old_value", "new_value"])
+            w.writerows(audit)
+        print(f"\naudit trail: {out}")
+    if not args.apply:
+        print("\nDRY-RUN — nothing written. Re-run with --apply to execute.")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

tests/domain/epc/test_glazing_mix_guard.py

@@ -18,19 +18,65 @@ def test_a_genuine_percentage_mix_classifies_as_mixed() -> None:
     assert result is GlazingType.MIXED
 
 
+@pytest.mark.parametrize(
+    "description",
+    [
+        "4% double glazing 2002 or later, 96% single glazing",
+        "10% double glazing 2002 or later, 90% single glazing",  # exactly at threshold
+        "5% double glazing pre-2002, 95% single glazing",
+    ],
+)
+def test_dominant_single_glazing_resolves_to_single(description: str) -> None:
+    # The bug: the LLM flattened a >= 90%-single dwelling onto its *minority* double
+    # type. Single glazing is era-free, so a dominant-single split is fully
+    # determined — the guard must claim it (SINGLE) rather than defer to the LLM.
+
+    # Act
+    result = glazing_mix_guard(description)
+
+    # Assert
+    assert result is GlazingType.SINGLE
+
+
+@pytest.mark.parametrize(
+    ("description", "expected"),
+    [
+        # >= 90% double with a *stated* era is era-pinned — the LLM otherwise
+        # flattens it onto the 4% minority single (the symmetric bug to the
+        # dominant-single case above).
+        ("96% double glazing 2002 or later, 4% single glazing", GlazingType.DOUBLE_POST_2002),
+        ("95% double glazing pre-2002, 5% single glazing", GlazingType.DOUBLE_PRE_2002),
+        ("90% double glazing 2002 or later, 10% single glazing", GlazingType.DOUBLE_POST_2002),
+        # Triple is era-bearing too — resolve when the era is stated.
+        ("92% triple glazing pre-2002, 8% single glazing", GlazingType.TRIPLE_PRE_2002),
+        ("91% triple glazing 2002 or later, 9% single glazing", GlazingType.TRIPLE_POST_2002),
+    ],
+)
+def test_dominant_era_stated_double_or_triple_resolves_deterministically(
+    description: str, expected: GlazingType
+) -> None:
+    # Act
+    result = glazing_mix_guard(description)
+
+    # Assert — a dominant double/triple whose era is stated is fully determined, so
+    # the guard claims it rather than trusting the LLM.
+    assert result is expected
+
+
 @pytest.mark.parametrize(
     "description",
     [
         "100% double glazing 2002 or later",  # uniform — one type
-        "96% double glazing 2002 or later, 4% single glazing",  # >= 90% -> uniform
+        "90% double glazing unknown age, 10% single glazing",  # dominant but era unstated
         "80% double glazing 2002 or later, 20% double glazing pre-2002",  # same base type
         "some double glazing and a bit of single",  # unparseable — no percentages
         "",
     ],
 )
-def test_uniform_or_unparseable_glazing_defers_to_the_llm(description: str) -> None:
-    # The guard only claims a genuine percentage mix; a uniform assertion (applied
-    # as that type) or a varied phrasing is left for the LLM classifier.
+def test_uniform_or_ambiguous_glazing_defers_to_the_llm(description: str) -> None:
+    # The guard only claims a genuine mix or a fully-determined dominant split. A
+    # uniform assertion, a dominant split whose era is unstated ("unknown age" — the
+    # era is genuinely ambiguous), or a varied phrasing is left for the LLM.
 
     # Act
     result = glazing_mix_guard(description)