From 1af3860528ff742372447d387e6cbea1ad080e42 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 30 Jul 2026 15:05:27 +0000 Subject: [PATCH] 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) --- .../bulk_upload_finaliser_orchestrator.py | 50 +++++++++++-------- ...test_bulk_upload_finaliser_orchestrator.py | 19 ++++--- 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/orchestration/bulk_upload_finaliser_orchestrator.py b/orchestration/bulk_upload_finaliser_orchestrator.py index 5d0bd9568..77a545856 100644 --- a/orchestration/bulk_upload_finaliser_orchestrator.py +++ b/orchestration/bulk_upload_finaliser_orchestrator.py @@ -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, diff --git a/tests/orchestration/test_bulk_upload_finaliser_orchestrator.py b/tests/orchestration/test_bulk_upload_finaliser_orchestrator.py index 3ce8b8dcc..38dba15d2 100644 --- a/tests/orchestration/test_bulk_upload_finaliser_orchestrator.py +++ b/tests/orchestration/test_bulk_upload_finaliser_orchestrator.py @@ -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: