Merge pull request #1422 from Hestia-Homes/fix/roof-vaulted-nd-sentinel-eligibility

Roof eligibility resolves sentinel thickness by age band (vaulted joins sloping)
This commit is contained in:
KhalimCK 2026-07-02 12:45:51 +01:00 committed by GitHub
commit 61321d0d47
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 315 additions and 12 deletions

View file

@ -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: AD uninsulated, EF 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**:

View file

@ -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 **AD**) |
Age-band as-built states (construction age code): **AD uninsulated · EF
limited insulation · G+ insulated.** Only AD 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 AD 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 EF 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.

View file

@ -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,

View file

@ -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