From 50554ff57e85ae14faee4d986f8be7b9f93783cc Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Thu, 30 Jul 2026 15:58:38 +0000 Subject: [PATCH] review: backfill writes an explicit Unknown band too (matches finaliser) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligns the historical backfill with the go-forward finaliser: it now writes a boiler_efficiency_band row for EVERY main_heating_system row — the parseable band (A-G) where present, else an explicit Unknown — so historical and go-forward data match. Unknown is fine on non-boilers (inert to modelling). Confirmed with Khalim. Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/backfill_boiler_efficiency_band.py | 34 ++++++++++++------- .../test_backfill_boiler_efficiency_band.py | 18 +++++++--- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/scripts/backfill_boiler_efficiency_band.py b/scripts/backfill_boiler_efficiency_band.py index ff083e626..01271d694 100644 --- a/scripts/backfill_boiler_efficiency_band.py +++ b/scripts/backfill_boiler_efficiency_band.py @@ -7,12 +7,14 @@ override rows for the existing stock by re-parsing that text with the SAME guard live classifier uses (``boiler_efficiency_band_guard``), so the backfill and the forward path cannot drift. -One ``boiler_efficiency_band`` row per ``main_heating_system`` row whose description -carries a band (``A``–``G``), keyed to the same ``(property_id, building_part)``. -Descriptions with no band (a plain boiler, a non-boiler heating system) get no row. -The modelling gate (gas/oil boilers only, ADR-0068) is applied at overlay time, not -here — so a band parsed off an electric ``Boiler: A rated NA`` is stored for -fidelity but ignored by the calculator, exactly as the live classifier stores it. +One ``boiler_efficiency_band`` row per ``main_heating_system`` row, keyed to the +same ``(property_id, building_part)`` — the parseable band (``A``–``G``) where the +description carries one, else an explicit ``Unknown``. This mirrors the finaliser +(which records Unknown rather than skipping), so backfilled historical data matches +go-forward data. The modelling gate (gas/oil boilers only, ADR-0068) is applied at +overlay time, not here — so a band parsed off an electric ``Boiler: A rated NA`` is +stored for fidelity but ignored by the calculator, and ``Unknown`` (incl. on a +non-boiler system) is inert to the calculator, exactly as the live path stores them. GATED: the ``boiler_efficiency_band`` value of the FE-owned ``override_component`` pgEnum must exist first (the Drizzle migration in the assessment-model repo). Until @@ -63,24 +65,30 @@ class BandBackfillRow: def band_backfill_rows(rows: Iterable[MainHeatingRow]) -> list[BandBackfillRow]: - """The ``boiler_efficiency_band`` rows to write for a set of - ``main_heating_system`` rows — one per row whose description carries a - parseable SEDBUK band, keyed to the same property + building part. Rows with - no band (or the ``UNKNOWN`` sentinel) produce nothing. + """The ``boiler_efficiency_band`` row to write for **every** + ``main_heating_system`` row, keyed to the same property + building part — the + parseable SEDBUK band (``A``–``G``) where the description carries one, else an + explicit ``Unknown``. This mirrors the finaliser (which records Unknown rather + than skipping), so backfilled historical data matches go-forward data; + ``Unknown`` is inert to modelling (no efficiency anchor) and fine on a + non-boiler system. Pure and DB-free, so the parse/decide logic is unit-tested without a database; the connection wrapper below just feeds it rows and upserts.""" backfill: list[BandBackfillRow] = [] for row in rows: band = boiler_efficiency_band_guard(row.original_spreadsheet_description) - if band is None or band is BoilerEfficiencyBand.UNKNOWN: - continue + value = ( + band.value + if band is not None and band is not BoilerEfficiencyBand.UNKNOWN + else BoilerEfficiencyBand.UNKNOWN.value + ) backfill.append( BandBackfillRow( property_id=row.property_id, portfolio_id=row.portfolio_id, building_part=row.building_part, - override_value=band.value, + override_value=value, original_spreadsheet_description=row.original_spreadsheet_description, ) ) diff --git a/tests/scripts/test_backfill_boiler_efficiency_band.py b/tests/scripts/test_backfill_boiler_efficiency_band.py index c94220201..9319564af 100644 --- a/tests/scripts/test_backfill_boiler_efficiency_band.py +++ b/tests/scripts/test_backfill_boiler_efficiency_band.py @@ -30,14 +30,22 @@ def test_backfills_a_band_row_per_boiler_with_a_parseable_band() -> None: ] -def test_leaves_descriptions_without_a_band_untouched() -> None: +def test_records_unknown_for_descriptions_without_a_band() -> None: + # Aligned with the finaliser: every main_heating_system row gets a band row — + # a description with no determinable band (plain boiler, non-boiler system) is + # recorded as an explicit Unknown, so historical + go-forward data match. + # Unknown is fine for non-boilers (inert to modelling). rows = [ - _row("Gas boiler"), - _row("Community Heating Systems: Community boilers only (RdSAP)"), - _row(""), + _row("Gas boiler", property_id=1), + _row("Community Heating Systems: Community boilers only (RdSAP)", property_id=2), ] - assert band_backfill_rows(rows) == [] + result = band_backfill_rows(rows) + + assert [(r.property_id, r.override_value) for r in result] == [ + (1, "Unknown"), + (2, "Unknown"), + ] def test_uses_the_same_guard_as_the_live_path_for_multi_system_and_electric() -> None: