diff --git a/CONTEXT.md b/CONTEXT.md index 32ce5c3bc..e0b4a8493 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -105,6 +105,10 @@ _Avoid_: patches (deprecated), corrections, manual EPC, edits Resolving a **Landlord Description** (unbounded free-text a landlord supplies for one component — "CWI" / "Cav filled" / "cavity insulated" all name one thing) onto a **Recognised Internal Description** via an LLM classifier, persisted in the `landlord_*_overrides` table (`source=classifier`) as a reviewed cache. Four vocabularies are kept **distinct** and must not be conflated: a **Landlord Description** (unbounded input); a **Recognised Internal Description** (the closed target taxonomy — e.g. a `MainHeatingSystemType` archetype — each binding to a Simulation Overlay); a **Lodged Description** (the gov-EPC `main_heating[].description` rendering, e.g. "Room heaters, electric" — only an example of which system *types* occur, never a map key); and the **SAP main heating code** (Table 4a/4b, what the calculator consumes). The classifier maps Landlord → Recognised Internal → SAP code. When it cannot confidently place the text it emits **`None`** (no overlay → the lodged EPC stands, surfaced to the user as "no suitable match"), **never the nearest wrong archetype** — the target taxonomy must be complete enough that a real system always has a correct home, so the classifier never overflows into a garbage-drawer archetype (ADR-0041). _Avoid_: "the LLM mapper is unreliable" (the failure mode is a too-small target taxonomy, not LLM language ability); conflating the landlord input vocabulary with the gov-EPC lodged rendering or the RdSAP entry-tool catalogue; treating a deterministic dict as a *replacement* for the LLM rather than a reviewed cache of its output +**Lodgement Sentinels (ND / NI / AB)**: +The RdSAP string sentinels a cert may lodge in place of a numeric value (e.g. `roof_insulation_thickness`). They are data, not noise, and are **not interchangeable**: **`ND`** (Not Defined) — no data; resolve to the construction age band's **as-built** state (roof as-built: A–D uninsulated, E–F limited insulation, G+ insulated). **`NI`** — insulation **present** but thickness unknown (RdSAP 10 §5.11.4: score as 50 mm); never "no insulation". **`AB`** — As Built; resolve by age band like ND. The mapper deliberately passes roof sentinels through raw — the calculator's resolution depends on distinguishing them (ADR-0047). Scoring and eligibility may legitimately resolve the same sentinel differently (`NI`: 50 mm to the U-cascade, not-eligible to the roof generator). +_Avoid_: reading `ND` as "no insulation"; reading `NI` as "no insulation" (it means the opposite); normalising sentinels at the mapper boundary; comparing a sentinel string against an int (`"ND" != 0` is always True) + ### Modelling **Effective EPC**: diff --git a/docs/adr/0047-roof-eligibility-resolves-sentinel-thickness-by-age-band.md b/docs/adr/0047-roof-eligibility-resolves-sentinel-thickness-by-age-band.md new file mode 100644 index 000000000..b582f99c5 --- /dev/null +++ b/docs/adr/0047-roof-eligibility-resolves-sentinel-thickness-by-age-band.md @@ -0,0 +1,119 @@ +# Roof Eligibility Resolves Sentinel Thickness by Age Band (vaulted joins sloping; ND ≠ NI) + +Property 742265 (uprn 10033526327, cert 2780-3922-3202-6042-9200) lodges a +`Pitched (vaulted ceiling)` roof (gov-API `roof_construction` 5) with +`roof_insulation_thickness = "ND"` and got **no roof Recommendation**, leaving +its plan short of the scenario's goal band on an unlimited budget. Two +generator gaps compound (ADR-0021's dispatch table has no vaulted row, and its +uninsulated trigger `!= 0` compares the `"ND"` *string* against int 0, so it +never fires), while the SAP10 Calculation already handles both: it treats +vaulted as a slope-following ceiling and resolves the `"ND"` sentinel via the +roof description / age-band defaults (`heat_transmission.py` — the +`is_sloping_ceiling` disjunction and the raw-sentinel inspection). The +generator diverged from the calculator — the exact split ADR-0021 set out to +prevent. + +## Decision + +**Amends ADR-0021.** The roof Recommendation Generator changes in two ways; +the mapper and the calculator change in none. + +**1. Vaulted is a sloping ceiling.** The dispatch matches the same substring +disjunction the calculator uses (`"sloping ceiling" in roof_type or "vaulted" +in roof_type`): a vaulted ceiling has no loft void, so its one applicable +Measure is `sloping_ceiling_insulation` (rafters, 100 mm) — never the loft +fallback. ADR-0021's dispatch table gains the row: + +| Roof type (`roof_construction_type` substring) | Measure | Uninsulated trigger | Overlay | +|---|---|---|---| +| `sloping ceiling` **or `vaulted`** | `sloping_ceiling_insulation` | resolved thickness 0 (below) | → 100 mm | + +**2. Sentinel-aware uninsulated trigger, resolved by age band — codes, not +descriptions.** `roof_insulation_thickness` is `Optional[Union[str, int]]`; a +lodged string sentinel is data, not noise. The generator resolves eligibility +for the two pitched branches (sloping/vaulted and loft) as: + +| Lodged value | Meaning (RdSAP 10) | Eligibility | +|---|---|---| +| int `0` | explicitly uninsulated | eligible (unchanged) | +| int `> 0` | measured thickness | not eligible (unchanged) | +| `"NI"` | insulation **present**, thickness unknown (§5.11.4: assume 50 mm) | **not eligible** | +| `"ND"` / `None` | not defined — resolve to the age band's **as-built** state | eligible iff as-built uninsulated (bands **A–D**) | + +Age-band as-built states (construction age code): **A–D uninsulated · E–F +limited insulation · G+ insulated.** Only A–D resolves to eligible. This +extends the codebase's existing precedent (`_PRE_1950_AGE_CODES = {A,B,C,D}` +in the mapper's code-8 rule; pinned by cert 001479 Ext2 at U=2.30) from the +scoring path to eligibility. + +**Sentinel resolution requires a KNOWN pitched/thatched roof type.** An +unlodged `roof_construction_type` reaching the loft fallback can be a **party +ceiling** — gov-API codes 6/7 ("(another dwelling above)" / thatch with +additional insulation) map the type to `None`, and real code-7 certs lodge +`"ND"` — with zero roof heat loss (RdSAP 10 Table 18), so insulating it +contradicts the dwelling. An unlodged type therefore keeps the explicit-0-only +trigger (the pre-ADR behaviour for that shape); only a type containing +`pitched`/`thatch` (or the sloping/vaulted branch, known by construction) +resolves sentinels by age band. + +**Coherence rule (load-bearing):** sentinel resolution must never make an +undefined cert *more* eligible than an explicitly-lodged one. An explicit +50 mm lodgement is not eligible today, so `"ND"` on a band-E/F ("limited +insulation") dwelling must also resolve to not-eligible — recommending a +top-up to the dwelling whose insulation we *don't* know while withholding it +from the identical dwelling whose thin insulation we *do* know would be +incoherent. Whether limited-insulation **top-up** should be a Measure at all +is a separate product question that applies equally to explicit thin +lodgements; it is out of scope here (see Consequences). + +**The mapper stays untouched — the `"ND"` passthrough is load-bearing.** The +calculator inspects the *raw* lodgement to distinguish explicit `int(0)` +(drop the description; uninsulated) from `"NI"` (keep the description; §5.11.4 +retrofit-50 mm) from `"ND"` (defer to description / age default). Normalising +sentinels at the mapper boundary would change scoring inputs for every ND-roof +cert in the corpus for zero eligibility benefit. + +**Two homes for sentinel resolution, deliberately.** Scoring asks "what +thickness should the U-cascade see" (`NI` → 50 mm); eligibility asks "is this +roof a candidate" (`NI` → no). Same sentinel, different questions, different +answers — a single shared resolver would just relocate the split into its call +sites. Each home carries a regression test pinning the *disagreement* as +intentional. Consolidate only if a third consumer appears (rule of three). + +## Considered options + +- **Normalise `ND` → 0 in the mapper** (extend the code-8 rule to code 5 and + to the `"ND"` string). Rejected: changes calculator inputs corpus-wide — + `roof_thickness_explicitly_zero` would fire and drop the roof description + the §5.11 resolution depends on — risking the pinned Elmhurst accuracy + corpus to fix an eligibility bug. +- **Defer to `roofs[].description`** ("no insulation (assumed)") for ND + resolution, mirroring the calculator exactly. Rejected: eligibility should + key on codes, not free-text description parsing; descriptions drift by + lodgement software and the age-band code carries the same as-built signal + deterministically. +- **Gate eligibility on the calculator's effective roof U-value.** Rejected: + no other generator gates on scored output (walls/floors gate on lodged + attributes), and it re-couples eligibility to scoring against ADR-0016's + separation. +- **Treat `"ND"` alone as uninsulated** (sentinel-literal). Rejected: ND means + *not defined*, not *none* — a band-G ND roof is almost certainly insulated + as built and would get a junk Recommendation. + +## Consequences + +- 742265 (band B, vaulted, ND) becomes eligible: `sloping_ceiling_insulation` + over 55.82 m² (~£2.2k, verified by local re-model). Any pitched-loft cert + lodging `"ND"` with age band A–D is likewise un-withheld. +- The **flat branch is deliberately untouched**: its inverted convention + (`None` = uninsulated, per the Elmhurst mapping) means an `"ND"` flat roof + is *not eligible* today and stays so — a separately-verifiable follow-up, + not silently folded into this change. +- **Open follow-up (product):** should limited-insulation dwellings (explicit + thin lodgements *and* ND + bands E–F alike) be offered an insulation + top-up? Today neither is; changing that is a trigger redesign, not a + sentinel question. +- ADR-0021's "one dispatching generator, one Measure per roof" and its + dispatch order (sloping → flat → no-access → loft) are unchanged; the + sloping test simply matches one more substring, ahead of the fallbacks as + before. diff --git a/domain/modelling/generators/roof_recommendation.py b/domain/modelling/generators/roof_recommendation.py index 2595e3f93..5b234a41d 100644 --- a/domain/modelling/generators/roof_recommendation.py +++ b/domain/modelling/generators/roof_recommendation.py @@ -1,16 +1,18 @@ """The roof Recommendation Generator. Dispatches the MAIN roof to its single applicable insulation Measure by roof -type (ADR-0021): a sloping ceiling (rafters, 100 mm), or — the fallback — a -loft / thatched / unlodged pitched roof (joists, 300 mm); a no-access roof gets -nothing. Each emits one "Roof" Recommendation whose Option carries the +type (ADR-0021): a sloping or vaulted ceiling (rafters, 100 mm), a flat roof +(200 mm), or — the fallback — a loft / thatched / unlodged pitched roof +(joists, 300 mm); a no-access roof gets nothing. The pitched branches resolve +a lodged sentinel thickness ("ND" / nothing) to the age band's as-built state +(ADR-0047). Each emits one "Roof" Recommendation whose Option carries the insulation Simulation Overlay (raising `roof_insulation_thickness`) and a priced Cost (roof area x the Product's fully-loaded unit cost, with its contingency). -Flat-roof and room-in-roof branches land in later slices. No scoring, no -persistence — impact is produced later by scoring (ADR-0016). +The room-in-roof branch lands in a later slice. No scoring, no persistence — +impact is produced later by scoring (ADR-0016). """ -from typing import Optional +from typing import Final, Optional, Union from datatypes.epc.domain.epc_property_data import ( BuildingPartIdentifier, @@ -24,15 +26,42 @@ from repositories.product.product_repository import ProductRepository _LOFT_MEASURE_TYPE = MeasureType.LOFT_INSULATION _SLOPING_CEILING_MEASURE_TYPE = MeasureType.SLOPING_CEILING_INSULATION -# RdSAP 10 Table 16: 0 mm lodged roof insulation is an uninsulated loft. The -# Elmhurst mapper resolves "As Built" to 0 for pitched/sloping/loft roofs. -_ROOF_UNINSULATED_MM = 0 # Recommended loft-insulation depth (mm). Elmhurst re-lodges a loft-insulation # measure at 300 mm; pinning the before→after cascade (000490/001431) requires # the overlay to match that depth exactly (see test_elmhurst_cascade_pins). _RECOMMENDED_LOFT_THICKNESS_MM = 300 # Recommended sloping-ceiling depth (mm); Elmhurst re-lodges 100 mm (ADR-0021). _RECOMMENDED_SLOPING_CEILING_THICKNESS_MM = 100 +# Age bands whose as-built pitched roof is uninsulated (ADR-0047: A-D +# uninsulated, E-F limited insulation, G+ insulated) — the resolution for a +# cert lodging the "ND" (Not Defined) sentinel or no thickness at all. +_AS_BUILT_UNINSULATED_AGE_BANDS: Final[frozenset[str]] = frozenset( + {"A", "B", "C", "D"} +) + + +def _pitched_roof_is_uninsulated( + thickness: Union[str, int, None], age_band: Optional[str] +) -> bool: + """The pitched-roof uninsulated trigger, sentinel-aware (ADR-0047). + + `roof_insulation_thickness` lodges numerics AND string sentinels; the + sentinels are data, not noise, and must never make an undefined cert MORE + eligible than an explicitly-lodged one. An explicit 0 is uninsulated; the + "ND" (Not Defined) sentinel — or nothing lodged — resolves to the age + band's as-built state (only A-D built without pitched-roof insulation). + Anything else ("NI" = insulation present at unknown thickness per RdSAP 10 + §5.11.4, or a measured depth) means insulation is there. Eligibility only + — the calculator resolves the same sentinels separately for the U-cascade + (deliberate two-home split, ADR-0047).""" + if isinstance(thickness, int): + return thickness == 0 + if thickness is None or thickness == "ND": + return ( + age_band is not None + and age_band.upper() in _AS_BUILT_UNINSULATED_AGE_BANDS + ) + return False _FLAT_ROOF_MEASURE_TYPE = MeasureType.FLAT_ROOF_INSULATION # Recommended flat-roof depth (mm); Elmhurst re-lodges 200 mm (ADR-0021). _RECOMMENDED_FLAT_ROOF_THICKNESS_MM = 200 @@ -68,8 +97,13 @@ def recommend_roof_insulation( # plain pitched loft, a thatched roof (the covering doesn't block insulating # the loft floor), and an unlodged roof type (the modal UK case), matching # the pre-dispatcher behaviour of firing on `roof_insulation_thickness == 0`. - if "sloping ceiling" in roof_type: - if main.roof_insulation_thickness != _ROOF_UNINSULATED_MM: + # A vaulted ceiling follows the slope like a sloping ceiling (no loft + # void) — same disjunction the calculator's `is_sloping_ceiling` uses + # (heat_transmission.py), so generator and scorer read one story (ADR-0047). + if "sloping ceiling" in roof_type or "vaulted" in roof_type: + if not _pitched_roof_is_uninsulated( + main.roof_insulation_thickness, main.construction_age_band + ): return None return _roof_recommendation( epc, @@ -95,7 +129,20 @@ def recommend_roof_insulation( if "no access" in roof_type: return None # the roof void can't be reached to insulate it - if main.roof_insulation_thickness != _ROOF_UNINSULATED_MM: + # Sentinel resolution needs a KNOWN pitched/thatched roof (ADR-0047). An + # unlodged type reaching this fallback can be a party ceiling — gov-API + # codes 6/7 ("(another dwelling above)" / insulated thatch) map the type + # to None and real code-7 certs lodge "ND" — with zero roof heat loss + # (RdSAP 10 Table 18), so only an explicit 0 may trigger it. + known_pitched: bool = "pitched" in roof_type or "thatch" in roof_type + uninsulated: bool = ( + _pitched_roof_is_uninsulated( + main.roof_insulation_thickness, main.construction_age_band + ) + if known_pitched + else main.roof_insulation_thickness == 0 + ) + if not uninsulated: return None return _roof_recommendation( epc, diff --git a/tests/domain/modelling/test_roof_recommendation.py b/tests/domain/modelling/test_roof_recommendation.py index 63f28ce68..6759341ed 100644 --- a/tests/domain/modelling/test_roof_recommendation.py +++ b/tests/domain/modelling/test_roof_recommendation.py @@ -83,6 +83,139 @@ def test_room_in_roof_yields_no_recommendation_pending_a_dedicated_branch() -> N assert recommendation is None +def test_vaulted_ceiling_with_nd_thickness_on_pre_1950_dwelling_yields_sloping_ceiling_insulation() -> None: + # Arrange — the 742265 shape (ADR-0047): a "Pitched (vaulted ceiling)" MAIN + # roof lodging the "ND" sentinel on an age-band-B dwelling. ND resolves to + # the age band's as-built state (A-D: uninsulated), and a vaulted ceiling + # has no loft void, so the one applicable Measure is rafter insulation. + baseline: EpcPropertyData = build_epc() + main: SapBuildingPart = _part(baseline, BuildingPartIdentifier.MAIN) + main.roof_construction_type = "Pitched (vaulted ceiling)" + main.roof_insulation_thickness = "ND" + main.construction_age_band = "B" + + # Act + recommendation: Recommendation | None = recommend_roof_insulation( + baseline, _StubProducts() + ) + + # Assert + assert recommendation is not None + assert recommendation.surface == "Roof" + assert len(recommendation.options) == 1 + option = recommendation.options[0] + assert option.measure_type == "sloping_ceiling_insulation" + simulated: EpcPropertyData = apply_simulations(baseline, [option.overlay]) + assert ( + _part(simulated, BuildingPartIdentifier.MAIN).roof_insulation_thickness == 100 + ) + + +def test_vaulted_ceiling_with_nd_thickness_on_limited_insulation_age_band_yields_no_recommendation() -> None: + # Arrange — same vaulted-ND shape, but age band E: as-built carries limited + # insulation (ADR-0047), and ND must never make an undefined cert MORE + # eligible than an explicit thin lodgement (which is not eligible today). + baseline: EpcPropertyData = build_epc() + main: SapBuildingPart = _part(baseline, BuildingPartIdentifier.MAIN) + main.roof_construction_type = "Pitched (vaulted ceiling)" + main.roof_insulation_thickness = "ND" + main.construction_age_band = "E" + + # Act + recommendation: Recommendation | None = recommend_roof_insulation( + baseline, _StubProducts() + ) + + # Assert + assert recommendation is None + + +def test_vaulted_ceiling_with_ni_thickness_yields_no_recommendation() -> None: + # Arrange — "NI" means insulation PRESENT at unknown thickness (RdSAP 10 + # §5.11.4: score as 50 mm), never "no insulation" — even on an age band + # whose as-built state is uninsulated (ADR-0047, the ND ≠ NI distinction). + baseline: EpcPropertyData = build_epc() + main: SapBuildingPart = _part(baseline, BuildingPartIdentifier.MAIN) + main.roof_construction_type = "Pitched (vaulted ceiling)" + main.roof_insulation_thickness = "NI" + main.construction_age_band = "B" + + # Act + recommendation: Recommendation | None = recommend_roof_insulation( + baseline, _StubProducts() + ) + + # Assert + assert recommendation is None + + +def test_vaulted_ceiling_with_measured_thickness_yields_no_recommendation() -> None: + # Arrange — an explicitly-lodged depth means the rafters are already + # insulated; the sentinel-aware trigger must not loosen the numeric rule. + baseline: EpcPropertyData = build_epc() + main: SapBuildingPart = _part(baseline, BuildingPartIdentifier.MAIN) + main.roof_construction_type = "Pitched (vaulted ceiling)" + main.roof_insulation_thickness = 100 + main.construction_age_band = "B" + + # Act + recommendation: Recommendation | None = recommend_roof_insulation( + baseline, _StubProducts() + ) + + # Assert + assert recommendation is None + + +def test_loft_roof_with_nd_thickness_on_pre_1950_dwelling_yields_loft_insulation() -> None: + # Arrange — the loft fallback shares the sentinel semantics (ADR-0047): + # "ND" on an age-band-C pitched loft resolves to as-built uninsulated, so + # the joist measure fires just as it does for an explicit 0. The lodged + # type is the gov-API code-4 string — sentinel resolution needs a KNOWN + # pitched roof (an unlodged type could be a party ceiling; see below). + baseline: EpcPropertyData = build_epc() + main: SapBuildingPart = _part(baseline, BuildingPartIdentifier.MAIN) + main.roof_construction_type = "Pitched (slates/tiles), access to loft" + main.roof_insulation_thickness = "ND" + main.construction_age_band = "C" + + # Act + recommendation: Recommendation | None = recommend_roof_insulation( + baseline, _StubProducts() + ) + + # Assert + assert recommendation is not None + option = recommendation.options[0] + assert option.measure_type == "loft_insulation" + simulated: EpcPropertyData = apply_simulations(baseline, [option.overlay]) + assert ( + _part(simulated, BuildingPartIdentifier.MAIN).roof_insulation_thickness == 300 + ) + + +def test_unlodged_roof_type_with_nd_thickness_yields_no_recommendation() -> None: + # Arrange — a part with NO lodged roof type reaching the loft fallback can + # be a party ceiling ("(another dwelling above)", gov-API codes 6/7 map the + # type to None and real code-7 certs lodge "ND"): zero roof heat loss, so + # insulating it contradicts the dwelling. Sentinel resolution therefore + # requires a KNOWN pitched/thatched roof; an unlodged type keeps the + # explicit-0-only trigger (ADR-0047). + baseline: EpcPropertyData = build_epc() + main: SapBuildingPart = _part(baseline, BuildingPartIdentifier.MAIN) + main.roof_construction_type = None + main.roof_insulation_thickness = "ND" + main.construction_age_band = "B" + + # Act + recommendation: Recommendation | None = recommend_roof_insulation( + baseline, _StubProducts() + ) + + # Assert + assert recommendation is None + + def test_loft_option_carries_cost_from_roof_area_and_product() -> None: # Arrange baseline: EpcPropertyData = build_epc() # MAIN roof area 14.85 m^2