review: finaliser records an explicit Unknown band instead of skipping

kimjunte + Khalim alignment: when the band column is mapped and a description
can't be resolved to A-G, write the row with an explicit Unknown value rather
than silently skipping. The landlord declared the column, so every row gets a
value; 'assessed, no band' is now distinct from 'never assessed'. Still never
fails the finalise (unlike the mandatory components). Modelling reads Unknown as
no band (no efficiency anchor), so it stays inert to SAP.

Requires the FE boiler_efficiency_band pgEnum to include 'Unknown' (assessment-
model#486 updated). 7 finaliser tests green; pyright clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Khalim Conn-Kowlessar 2026-07-30 15:05:27 +00:00
parent eb07b91614
commit 1af3860528
2 changed files with 39 additions and 30 deletions

View file

@ -14,6 +14,7 @@ from typing import Any, Optional
from uuid import UUID
from domain.epc.property_overrides.boiler_efficiency_band import BoilerEfficiencyBand
from domain.epc.property_overrides.built_form_type import BuiltFormType
from domain.epc.property_overrides.property_type import PropertyType
from domain.epc.property_overrides.roof_type import RoofType
@ -61,12 +62,16 @@ UNKNOWN_VALUES = frozenset(
}
)
# Override components that are OPTIONAL per cell: present on some, legitimately
# absent on others. An unresolved/UNKNOWN value is skipped (no row) rather than
# failing the finalise the way a mandatory component does. The Boiler Efficiency
# Band (ADR-0068) exists only for a SEDBUK-rated boiler, so a plain boiler or a
# non-boiler heating cell has none.
_OPTIONAL_COMPONENTS = frozenset({"boiler_efficiency_band"})
# Override components whose unresolved/UNKNOWN value is recorded EXPLICITLY as
# Unknown rather than failing the finalise the way a mandatory component does.
# The landlord declared the column, so we always write a row — an undeterminable
# Boiler Efficiency Band (ADR-0068; a non-boiler heating cell, or a plain boiler
# with no SEDBUK letter) is stored as Unknown, keeping "assessed, no band"
# distinct from "never assessed". The modelling reads Unknown as no band (no
# efficiency anchor), so it is inert to SAP; it is not persisted downstream of the
# override read.
_UNKNOWN_MAPPED_COMPONENTS = frozenset({"boiler_efficiency_band"})
_BOILER_EFFICIENCY_BAND_UNKNOWN = BoilerEfficiencyBand.UNKNOWN.value
def _split_entries(cell: Any) -> list[str]:
@ -316,23 +321,24 @@ class BulkUploadFinaliserOrchestrator:
for building_part, file_pos in enumerate(permutation):
raw = entries[file_pos]
value = component_vocab.get(raw.lower())
if component in _OPTIONAL_COMPONENTS and (
value is None or value in UNKNOWN_VALUES
):
# An OPTIONAL override is present on some cells and absent
# on others by design — a Boiler Efficiency Band only
# exists for a SEDBUK-rated boiler (ADR-0068), so a plain
# boiler or a non-boiler heating cell resolves to UNKNOWN.
# That is a legitimate "no value": skip it (no row), don't
# fail the finalise the way a mandatory component does.
continue
if value is None or value in UNKNOWN_VALUES:
raise ValueError(
f"Unresolved {component} description {raw!r} "
f"(row {row_id}, portfolio {portfolio_id}): no resolved "
f"value{' (UNKNOWN)' if value else ''}. The verify gate "
f"should have mapped it; failing the finalise (ADR-0006)."
)
if component in _UNKNOWN_MAPPED_COMPONENTS:
# Record an EXPLICIT Unknown (ADR-0068; kimjunte): the
# landlord declared the column, so we always write a
# row — an undeterminable band (non-boiler cell, or a
# plain boiler with no SEDBUK letter) is Unknown, not
# skipped and not a hard failure. Modelling reads it as
# no band. Distinguishes "assessed, no band" from
# "never assessed".
value = _BOILER_EFFICIENCY_BAND_UNKNOWN
else:
raise ValueError(
f"Unresolved {component} description {raw!r} "
f"(row {row_id}, portfolio {portfolio_id}): no "
f"resolved value{' (UNKNOWN)' if value else ''}. The "
f"verify gate should have mapped it; failing the "
f"finalise (ADR-0006)."
)
inserts.append(
PropertyOverrideInsert(
property_id=property_id,

View file

@ -262,10 +262,12 @@ def test_finalise_writes_the_boiler_efficiency_band_alongside_the_heating_system
assert (band.building_part, band.override_value) == (0, "D")
def test_finalise_skips_an_unknown_boiler_efficiency_band_without_failing() -> None:
# A non-boiler heating system (or a plain boiler) carries no band, so the band
# classifier returns UNKNOWN. Unlike the mandatory components, an UNKNOWN band
# is a legitimate "no value" — skip it, don't fail the finalise (ADR-0068).
def test_finalise_records_unknown_when_the_band_cannot_be_mapped() -> None:
# A description with no determinable band (a non-boiler heating system, or a
# plain boiler) is recorded as an EXPLICIT Unknown band, not skipped: if the
# landlord declared the band column we always write a row, so "assessed, no
# band" is distinguishable from "never assessed" (ADR-0068; kimjunte). Unlike
# the mandatory components it does NOT fail the finalise.
combiner = [{"address2uprn_uprn": "100023", "source_row_id": "row-a"}]
classifier = [
{"Heating": "Community Heating Systems: Community boilers only (RdSAP)",
@ -294,11 +296,12 @@ def test_finalise_skips_an_unknown_boiler_efficiency_band_without_failing() -> N
},
)
# The heating system row is written; the UNKNOWN band produced no row.
# The heating row is written, AND an explicit Unknown band row alongside it.
assert any(o.override_component == "main_heating_system" for o in overrides.upserted)
assert not any(
o.override_component == "boiler_efficiency_band" for o in overrides.upserted
)
(band,) = [
o for o in overrides.upserted if o.override_component == "boiler_efficiency_band"
]
assert band.override_value == "Unknown"
def test_finalise_fails_loudly_on_unresolved_description() -> None: