mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-12 13:29:04 +00:00
Merge pull request #1384 from Hestia-Homes/fix/1376-glazing-proportion-reconcile
Defer aggregate-mix glazing overrides to the cert's per-window glazing (#1376)
This commit is contained in:
commit
9db8b60a54
8 changed files with 307 additions and 4 deletions
|
|
@ -10,6 +10,7 @@ from applications.landlord_description_overrides.landlord_description_overrides_
|
|||
from domain.epc.property_overrides.built_form_type import BuiltFormType
|
||||
from domain.epc.property_overrides.construction_age_band import ConstructionAgeBand
|
||||
from domain.epc.property_overrides.glazing_type import GlazingType
|
||||
from domain.epc.property_overrides.glazing_mix_guard import glazing_mix_guard
|
||||
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
|
||||
|
|
@ -146,8 +147,15 @@ def _build_columns(
|
|||
"glazing": lambda src: ClassifiableColumn(
|
||||
name="glazing",
|
||||
source_column=src,
|
||||
classifier=ChatGptColumnClassifier(
|
||||
chat_gpt, GlazingType, GlazingType.UNKNOWN
|
||||
# An aggregate glazing mix ("40% double, 60% single") can't be applied
|
||||
# per-window, so the deterministic guard resolves the structured split
|
||||
# to MIXED (no overlay → keep the cert's per-window glazing) and the LLM
|
||||
# handles uniform / varied phrasings (#1376, ADR-0042).
|
||||
classifier=GuardedColumnClassifier(
|
||||
guard=glazing_mix_guard,
|
||||
fallback=ChatGptColumnClassifier(
|
||||
chat_gpt, GlazingType, GlazingType.UNKNOWN
|
||||
),
|
||||
),
|
||||
repo=LandlordOverridesRepository[GlazingType](
|
||||
session, LandlordGlazingOverrideRow
|
||||
|
|
|
|||
|
|
@ -29,3 +29,14 @@ The landlord glazing override **reconciles against the cert's per-window composi
|
|||
- 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).
|
||||
|
||||
## Amendment (#1376 implementation): the reconcile collapses to a `MIXED` sentinel
|
||||
|
||||
Implementing this surfaced that the full "carry a proportion → compare to the cert → flag when far" design buys less than its cost. The cert **already** carries the best-possible per-window mapping (`sap_windows`, each with its own `glazing_type`); a landlord aggregate ("40% double, 60% single") says *how much* is double but **not which windows**, so applying it would overwrite real per-window data with an arbitrary guess. Per-window assignment from an aggregate is therefore not just unknowable but strictly worse than the cert — so a mixed override **always defers**, regardless of the exact proportion. The proportion comparison only ever fed the "flag when far" branch, and carrying a proportion needs a new schema field (an FE/Drizzle dependency).
|
||||
|
||||
**Realized decision.** Add a `GlazingType.MIXED` sentinel. The LLM classifies a genuine aggregate-mix to `MIXED` (the LLM handles the varied phrasings — no regex); `MIXED` is absent from the overlay's `_GLAZING_CODES`, so it resolves to **no overlay** and `_fold_glazing` never runs — the cert's per-window glazing is preserved. A **uniform** assertion (one type ≳ 90%) still resolves to that type and is applied to every window (unambiguous — no guess). Existing mixed-as-double rows are reclassified to `MIXED` (a deterministic parse of the `%X / %Y` format is acceptable for fixing *known* rows). The "uniform vs mixed" threshold (~90% one type) is tunable, pinned against the real distribution.
|
||||
|
||||
**Consequences of the amendment.**
|
||||
- The defer behaviour needs **no apply-seam logic and no proportion field** — it falls out of "no overlay". The slice is: add the `MIXED` member, give the classifier the option, reclassify the ~319 rows.
|
||||
- `MIXED` is a **new FE-owned pgEnum value** (one member) — added by the FE owner (Dan); `property_overrides` (TEXT) reclassify runs immediately, the cache pgEnum write defers until the migration (the Class-A/B pattern).
|
||||
- The **"flag when landlord disagrees with the cert" is dropped for now** — since a mix always defers, the flag was informational only. `MIXED` is the hook if a future slice wants to surface those disagreements.
|
||||
|
|
|
|||
40
domain/epc/property_overrides/glazing_mix_guard.py
Normal file
40
domain/epc/property_overrides/glazing_mix_guard.py
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
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)")
|
||||
# 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
|
||||
|
||||
|
||||
def glazing_mix_guard(description: str) -> Optional[GlazingType]:
|
||||
"""Deterministically resolve an aggregate-mix glazing description to ``MIXED``.
|
||||
|
||||
A landlord glazing description is often a whole-dwelling proportion split
|
||||
("40% double glazing 2002 or later, 60% single glazing"). Such a mix cannot be
|
||||
faithfully applied per-window — it says *how much* is each type, not *which*
|
||||
windows — and the cert already carries the true per-window glazing, so a mix
|
||||
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).
|
||||
"""
|
||||
matches = _PERCENT_TYPE.findall(description.lower())
|
||||
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
|
||||
return GlazingType.MIXED
|
||||
|
|
@ -21,4 +21,11 @@ class GlazingType(Enum):
|
|||
DOUBLE_PRE_2002 = "Double glazing, pre-2002"
|
||||
TRIPLE_PRE_2002 = "Triple glazing, pre-2002"
|
||||
TRIPLE_POST_2002 = "Triple glazing, 2002 or later"
|
||||
# An aggregate mix of glazing types across the dwelling ("40% double, 60%
|
||||
# single"). Deliberately absent from the overlay's `_GLAZING_CODES`, so it
|
||||
# resolves to no overlay and the cert's per-window `sap_windows` are kept
|
||||
# rather than flattened to one type — a whole-dwelling proportion cannot say
|
||||
# which windows are which (ADR-0042 amendment, #1376). The FE-owned pgEnum
|
||||
# gains this member via a Drizzle migration.
|
||||
MIXED = "Mixed glazing"
|
||||
UNKNOWN = "Unknown"
|
||||
|
|
|
|||
147
scripts/reclassify_mixed_glazing.py
Normal file
147
scripts/reclassify_mixed_glazing.py
Normal file
|
|
@ -0,0 +1,147 @@
|
|||
"""One-time re-classification of aggregate-mix glazing overrides flattened to Double.
|
||||
|
||||
#1376 / ADR-0042: a landlord glazing description that is an aggregate mix
|
||||
("40% double glazing 2002 or later, 60% single glazing") was classified to a single
|
||||
``"Double glazing…"`` value whenever any double was present, and the overlay then
|
||||
overwrote **every** window to double — over-crediting a predominantly-single dwelling
|
||||
and clobbering the cert's per-window glazing. A whole-dwelling proportion cannot say
|
||||
*which* windows are which, so a mix must resolve to ``GlazingType.MIXED`` (no overlay
|
||||
→ the cert's per-window ``sap_windows`` are kept).
|
||||
|
||||
The live classifier now applies ``glazing_mix_guard`` deterministically (so new
|
||||
intakes of the structured split 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 immediate fix). The ``landlord_glazing_overrides.value`` classifier cache is a
|
||||
``glazing`` **pgEnum**; ``MIXED`` is a new FE-owned value, so cache writes are
|
||||
**deferred** until the Drizzle migration adds it (the Class-A/B pattern).
|
||||
|
||||
DRY-RUN BY DEFAULT: prints the counts it would change and writes nothing. Pass
|
||||
``--apply`` to execute inside a transaction. Idempotent — only rows whose stored
|
||||
value differs from the target member are touched.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
from collections.abc import Iterable
|
||||
|
||||
from sqlalchemy import Connection, text
|
||||
|
||||
from domain.epc.property_overrides.glazing_mix_guard import glazing_mix_guard
|
||||
from scripts.e2e_common import build_engine, load_env
|
||||
|
||||
|
||||
def mixed_glazing_corrections(
|
||||
stored: Iterable[tuple[str, str]],
|
||||
) -> dict[str, str]:
|
||||
"""``(description, stored override_value)`` → the ``MIXED`` value, for the
|
||||
descriptions the glazing mix guard resolves to a mix whose stored value is not
|
||||
already ``MIXED``. Uniform / unparseable descriptions and rows already on
|
||||
``MIXED`` are omitted, so re-running against corrected data is a no-op."""
|
||||
corrections: dict[str, str] = {}
|
||||
for description, value in stored:
|
||||
member = glazing_mix_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 = 'glazing'
|
||||
"""
|
||||
)
|
||||
_OVERRIDES_UPDATE = text(
|
||||
"""
|
||||
UPDATE property_overrides
|
||||
SET override_value = :new_value
|
||||
WHERE override_component = 'glazing'
|
||||
AND lower(original_spreadsheet_description) = :description
|
||||
AND override_value <> :new_value
|
||||
"""
|
||||
)
|
||||
_OVERRIDES_COUNT = text(
|
||||
"""
|
||||
SELECT count(*) FROM property_overrides
|
||||
WHERE override_component = 'glazing'
|
||||
AND lower(original_spreadsheet_description) = :description
|
||||
AND override_value <> :new_value
|
||||
"""
|
||||
)
|
||||
_CACHE_UPDATE = text(
|
||||
"""
|
||||
UPDATE landlord_glazing_overrides
|
||||
SET value = :new_value, updated_at = now()
|
||||
WHERE lower(description) = :description
|
||||
AND value::text <> :new_value
|
||||
"""
|
||||
)
|
||||
_ENUM_VALUES = text(
|
||||
"SELECT e.enumlabel FROM pg_enum e JOIN pg_type t ON t.oid = e.enumtypid "
|
||||
"WHERE t.typname = 'glazing'"
|
||||
)
|
||||
|
||||
|
||||
def reclassify(conn: Connection, *, apply: bool) -> tuple[int, set[str]]:
|
||||
"""Re-map aggregate-mix glazing overrides onto MIXED. Returns the number of
|
||||
``property_overrides`` rows found and the set of target values the live
|
||||
``glazing`` pgEnum does not yet carry (cache-deferred until the FE migration)."""
|
||||
stored = [(r.description, r.value) for r in conn.execute(_DISTINCT)]
|
||||
enum_values = {r[0] for r in conn.execute(_ENUM_VALUES)}
|
||||
total = 0
|
||||
deferred: set[str] = set()
|
||||
for description, new_value in mixed_glazing_corrections(stored).items():
|
||||
params = {"description": description, "new_value": new_value}
|
||||
total += conn.execute(_OVERRIDES_COUNT, params).scalar() or 0
|
||||
in_enum = new_value in enum_values
|
||||
if not in_enum:
|
||||
deferred.add(new_value)
|
||||
if apply:
|
||||
conn.execute(_OVERRIDES_UPDATE, params)
|
||||
if in_enum:
|
||||
conn.execute(_CACHE_UPDATE, params)
|
||||
return total, deferred
|
||||
|
||||
|
||||
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, deferred = reclassify(conn, apply=args.apply)
|
||||
|
||||
verb = "re-classified" if args.apply else "would re-classify"
|
||||
print(
|
||||
f"{verb} {total} aggregate-mix glazing override row(s) from a flattened "
|
||||
"Double onto MIXED (property_overrides / TEXT); the cert's per-window "
|
||||
"glazing is kept."
|
||||
)
|
||||
if deferred:
|
||||
print(
|
||||
f"\n{len(deferred)} target value(s) NOT yet in the glazing pgEnum — "
|
||||
"their classifier-cache rows are deferred until the FE-repo enum "
|
||||
"migration adds these members (property_overrides was still updated, "
|
||||
"which is what the modelling reads):"
|
||||
)
|
||||
for value in sorted(deferred):
|
||||
print(f" {value!r}")
|
||||
if not args.apply:
|
||||
print("\nDRY-RUN — nothing written. Re-run with --apply to execute.")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
39
tests/domain/epc/test_glazing_mix_guard.py
Normal file
39
tests/domain/epc/test_glazing_mix_guard.py
Normal file
|
|
@ -0,0 +1,39 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from domain.epc.property_overrides.glazing_type import GlazingType
|
||||
from domain.epc.property_overrides.glazing_mix_guard import glazing_mix_guard
|
||||
|
||||
|
||||
def test_a_genuine_percentage_mix_classifies_as_mixed() -> None:
|
||||
# Arrange — the bug: an aggregate mix the LLM would flatten to "Double".
|
||||
description = "40% double glazing 2002 or later, 60% single glazing"
|
||||
|
||||
# Act
|
||||
result = glazing_mix_guard(description)
|
||||
|
||||
# Assert — a mix defers to the cert's per-window glazing, not a whole-dwelling
|
||||
# type.
|
||||
assert result is GlazingType.MIXED
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"description",
|
||||
[
|
||||
"100% double glazing 2002 or later", # uniform — one type
|
||||
"96% double glazing 2002 or later, 4% single glazing", # >= 90% -> uniform
|
||||
"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.
|
||||
|
||||
# Act
|
||||
result = glazing_mix_guard(description)
|
||||
|
||||
# Assert
|
||||
assert result is None
|
||||
|
|
@ -57,6 +57,18 @@ def test_unresolvable_glazing_produces_no_overlay(glazing_value: str) -> None:
|
|||
assert simulation is None
|
||||
|
||||
|
||||
def test_a_glazing_mix_produces_no_overlay_so_the_cert_is_kept() -> None:
|
||||
# A MIXED aggregate resolves to no overlay: a whole-dwelling proportion cannot
|
||||
# say which windows are which, so the cert's per-window `sap_windows` are kept
|
||||
# rather than flattened to one type (ADR-0042 amendment, #1376).
|
||||
|
||||
# Act
|
||||
simulation = glazing_overlay_for(GlazingType.MIXED.value, 0)
|
||||
|
||||
# Assert
|
||||
assert simulation is None
|
||||
|
||||
|
||||
def test_glazing_override_remaps_every_window_and_clears_lodged_u() -> None:
|
||||
# Arrange — baseline windows are double glazed (code 2, lodged U 2.8); the
|
||||
# landlord corrects the whole dwelling to single glazing.
|
||||
|
|
@ -74,14 +86,18 @@ def test_glazing_override_remaps_every_window_and_clears_lodged_u() -> None:
|
|||
assert all(w.window_transmission_details is None for w in result.sap_windows)
|
||||
|
||||
|
||||
_NO_OVERLAY_SENTINELS = {GlazingType.UNKNOWN, GlazingType.MIXED}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"member", [m for m in GlazingType if m is not GlazingType.UNKNOWN]
|
||||
"member", [m for m in GlazingType if m not in _NO_OVERLAY_SENTINELS]
|
||||
)
|
||||
def test_every_resolvable_glazing_value_decodes_to_a_code(
|
||||
member: GlazingType,
|
||||
) -> None:
|
||||
# A classifier emits a GlazingType value; if the overlay can't decode it the
|
||||
# override silently no-ops. Every non-UNKNOWN member must resolve.
|
||||
# override silently no-ops. Every member except the deliberate no-overlay
|
||||
# sentinels (UNKNOWN, MIXED) must resolve to a code.
|
||||
|
||||
# Act
|
||||
simulation = glazing_overlay_for(member.value, 0)
|
||||
|
|
|
|||
35
tests/scripts/test_reclassify_mixed_glazing.py
Normal file
35
tests/scripts/test_reclassify_mixed_glazing.py
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
"""The mixed-glazing reclassify maps a flattened aggregate mix onto MIXED, leaves
|
||||
uniform / already-correct rows alone, and is idempotent (#1376 / ADR-0042)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from scripts.reclassify_mixed_glazing import mixed_glazing_corrections
|
||||
|
||||
|
||||
def test_flattened_mix_rows_are_corrected_to_mixed() -> None:
|
||||
# Arrange — an aggregate mix flattened to Double (the bug), a near-uniform mix
|
||||
# and a clean uniform row (both correctly Double), and a pure single row.
|
||||
stored = [
|
||||
("40% double glazing 2002 or later, 60% single glazing", "Double glazing, 2002 or later"),
|
||||
("96% double glazing 2002 or later, 4% single glazing", "Double glazing, 2002 or later"),
|
||||
("100% double glazing 2002 or later", "Double glazing, 2002 or later"),
|
||||
("100% single glazing", "Single glazing"),
|
||||
]
|
||||
|
||||
# Act
|
||||
corrections = mixed_glazing_corrections(stored)
|
||||
|
||||
# Assert — only the genuine mix is re-mapped; near-uniform and uniform stay.
|
||||
assert corrections == {
|
||||
"40% double glazing 2002 or later, 60% single glazing": "Mixed glazing"
|
||||
}
|
||||
|
||||
|
||||
def test_already_mixed_rows_need_no_change() -> None:
|
||||
# Act / Assert — idempotent.
|
||||
assert (
|
||||
mixed_glazing_corrections(
|
||||
[("40% double glazing 2002 or later, 60% single glazing", "Mixed glazing")]
|
||||
)
|
||||
== {}
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue