From 6e30ca311d10a834bdfa9c85ec216d43b30e6677 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 16:00:01 +0000 Subject: [PATCH] fix(scenarios): exclusion vocabulary = backend MeasureType exactly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scenario 1275's modelling run crashed the engine: its exclusions carried 'room_roof_insulation', which isn't a backend MeasureType — the strict _parse_exclusions raises for every property in the scenario. The authoring vocabulary had seven such tokens (room_roof_insulation, boiler_upgrade, secondary_heating, ventilation, fireplace, hot_water_tank_insulation, cylinder_thermostat). Scenario authoring now uses its own SCENARIO_MEASURES vocabulary — exactly the backend MeasureType set (drifted names corrected, unimplemented measures dropped, missing system_tune_up/_zoned + sloping_ceiling gained), pinned by a test that fails if either side moves alone (the backend's ADR-0056 shared-contract rule, applied app-side). room_roof_insulation leaves DISRUPTIVE_MEASURES until the backend implements the measure; legacy tokens on existing rows still display in constraint summaries. The global measuresDisplayLabels vocabulary (recommendations display, old file-based trigger) is untouched. Co-Authored-By: Claude Fable 5 --- .../scenarios/new/NewScenarioJourney.tsx | 48 ++++++----- src/lib/scenarios/model.test.ts | 53 +++++++++++- src/lib/scenarios/model.ts | 82 +++++++++++++++---- 3 files changed, 139 insertions(+), 44 deletions(-) diff --git a/src/app/portfolio/[slug]/(portfolio)/scenarios/new/NewScenarioJourney.tsx b/src/app/portfolio/[slug]/(portfolio)/scenarios/new/NewScenarioJourney.tsx index 3fc372bc..f5208d07 100644 --- a/src/app/portfolio/[slug]/(portfolio)/scenarios/new/NewScenarioJourney.tsx +++ b/src/app/portfolio/[slug]/(portfolio)/scenarios/new/NewScenarioJourney.tsx @@ -6,11 +6,14 @@ import { useRouter } from "next/navigation"; import { useMutation } from "@tanstack/react-query"; import { PORTFOLIO_GOALS } from "@/app/db/schema/portfolio"; import { - measuresDisplayLabels, - measuresList, - type MeasureKey, -} from "@/app/db/schema/recommendations"; -import { DISRUPTIVE_MEASURES, EPC_BANDS, findExactDuplicate, SCENARIO_GOALS } from "@/lib/scenarios/model"; + DISRUPTIVE_MEASURES, + EPC_BANDS, + findExactDuplicate, + SCENARIO_GOALS, + SCENARIO_MEASURES, + scenarioMeasuresList, + type ScenarioMeasureKey, +} from "@/lib/scenarios/model"; import { BandChip, EPC_SOFT, GOAL_DESCRIPTIONS, GOAL_SHORT_LABELS, GoalIcon, bandInk, bandTint, formatMoney } from "../ui"; interface ExistingScenario { @@ -24,20 +27,21 @@ interface ExistingScenario { modelled: boolean; } -const MEASURE_GROUPS: [string, MeasureKey[]][] = [ - ["Insulation", ["internal_wall_insulation", "external_wall_insulation", "cavity_wall_insulation", "loft_insulation", "flat_roof_insulation", "room_roof_insulation", "suspended_floor_insulation", "solid_floor_insulation"]], - ["Heating & hot water", ["boiler_upgrade", "high_heat_retention_storage_heaters", "air_source_heat_pump", "secondary_heating", "hot_water_tank_insulation", "cylinder_thermostat"]], - ["Glazing & ventilation", ["double_glazing", "secondary_glazing", "ventilation"]], - ["Renewables & other", ["solar_pv", "low_energy_lighting", "fireplace"]], +// Grouped presentation of SCENARIO_MEASURES (the backend MeasureType vocabulary) +const MEASURE_GROUPS: [string, ScenarioMeasureKey[]][] = [ + ["Insulation", ["internal_wall_insulation", "external_wall_insulation", "cavity_wall_insulation", "loft_insulation", "flat_roof_insulation", "sloping_ceiling_insulation", "suspended_floor_insulation", "solid_floor_insulation"]], + ["Heating & hot water", ["gas_boiler_upgrade", "high_heat_retention_storage_heaters", "air_source_heat_pump", "secondary_heating_removal", "system_tune_up", "system_tune_up_zoned"]], + ["Glazing & ventilation", ["double_glazing", "secondary_glazing", "mechanical_ventilation"]], + ["Renewables & other", ["solar_pv", "low_energy_lighting"]], ]; -const PRESETS: { label: string; hint: string; excl: MeasureKey[] }[] = [ +const PRESETS: { label: string; hint: string; excl: ScenarioMeasureKey[] }[] = [ { label: "No wet trades", hint: "Rules out plaster-and-screed work: wall and solid-floor insulation", excl: ["internal_wall_insulation", "external_wall_insulation", "solid_floor_insulation"] }, { label: "Fabric only", hint: "Insulation and glazing only — no heating systems or renewables", - excl: ["boiler_upgrade", "high_heat_retention_storage_heaters", "air_source_heat_pump", "secondary_heating", "hot_water_tank_insulation", "cylinder_thermostat", "solar_pv", "low_energy_lighting", "fireplace", "ventilation"] }, + excl: ["gas_boiler_upgrade", "high_heat_retention_storage_heaters", "air_source_heat_pump", "secondary_heating_removal", "system_tune_up", "system_tune_up_zoned", "solar_pv", "low_energy_lighting", "mechanical_ventilation"] }, { label: "Low disruption", hint: "Excludes every measure marked as disruptive", - excl: Object.keys(DISRUPTIVE_MEASURES) as MeasureKey[] }, + excl: Object.keys(DISRUPTIVE_MEASURES) as ScenarioMeasureKey[] }, ]; const fieldLabel = @@ -102,7 +106,7 @@ export function NewScenarioJourney({ if (!band) return null; let s = `Reach EPC ${band}`; const ex = [...excluded]; - if (ex.length === 1) s += ` — excl. ${measuresDisplayLabels[ex[0] as MeasureKey]}`; + if (ex.length === 1) s += ` — excl. ${SCENARIO_MEASURES[ex[0] as ScenarioMeasureKey] ?? ex[0]}`; else if (ex.length > 1) s += ` — ${ex.length} exclusions`; if (budgetNumber) s += ` — ${formatMoney(budgetNumber)}/property`; return s; @@ -141,7 +145,7 @@ export function NewScenarioJourney({ setErrors(e); if (Object.keys(e).length) return; } - if (step === 2 && excluded.size === measuresList.length) { + if (step === 2 && excluded.size === scenarioMeasuresList.length) { setErrors({ excl: "You've excluded every measure — the engine would have nothing to work with. Allow at least one." }); return; } @@ -385,7 +389,7 @@ export function NewScenarioJourney({ ))}
- {excluded.size === 0 - ? `All ${measuresList.length} measures allowed` - : `${excluded.size} excluded · ${measuresList.length - excluded.size} allowed`} + ? `All ${scenarioMeasuresList.length} measures allowed` + : `${excluded.size} excluded · ${scenarioMeasuresList.length - excluded.size} allowed`}
{MEASURE_GROUPS.map(([group, keys]) => ( @@ -434,7 +438,7 @@ export function NewScenarioJourney({ {off ? "✕" : "✓"} - {measuresDisplayLabels[k]} + {SCENARIO_MEASURES[k]} {disruptiveReason && !off && ( disruptive @@ -461,14 +465,14 @@ export function NewScenarioJourney({

{(() => { const allowedDisruptive = ( - Object.keys(DISRUPTIVE_MEASURES) as MeasureKey[] + Object.keys(DISRUPTIVE_MEASURES) as ScenarioMeasureKey[] ).filter((k) => !excluded.has(k)); return allowedDisruptive.length > 0 ? (
This scenario still allows{" "} {allowedDisruptive - .map((k) => measuresDisplayLabels[k]) + .map((k) => SCENARIO_MEASURES[k]) .join(", ")} {" "} — disruptive measure{allowedDisruptive.length > 1 ? "s" : ""}{" "} @@ -527,7 +531,7 @@ export function NewScenarioJourney({ key={k} className="rounded-full border border-red-100 bg-white px-3 py-0.5 text-[13px] text-red-800 line-through" > - {measuresDisplayLabels[k as MeasureKey] ?? k} + {SCENARIO_MEASURES[k as ScenarioMeasureKey] ?? k} ))} diff --git a/src/lib/scenarios/model.test.ts b/src/lib/scenarios/model.test.ts index 52b80303..c901b236 100644 --- a/src/lib/scenarios/model.test.ts +++ b/src/lib/scenarios/model.test.ts @@ -1,16 +1,61 @@ import { describe, expect, it } from "vitest"; -import { measuresList } from "@/app/db/schema/recommendations"; import { constraintSummary, deriveScenarioStatus, DISRUPTIVE_MEASURES, findExactDuplicate, parseExclusions, + scenarioMeasuresList, scenarioToInsertRow, serializeExclusions, validateScenarioConfig, } from "./model"; +describe("SCENARIO_MEASURES", () => { + // Mirror of MeasureType in Hestia-Homes/Model domain/modelling/measure_type.py. + // The engine CRASHES on exclusion tokens outside this set at modelling time + // (scenario_table._parse_exclusions is strict), so the two lists must change + // together — the backend's ADR-0056 shared-contract rule, applied here. + const BACKEND_MEASURE_TYPES = [ + "cavity_wall_insulation", + "external_wall_insulation", + "internal_wall_insulation", + "loft_insulation", + "sloping_ceiling_insulation", + "flat_roof_insulation", + "suspended_floor_insulation", + "solid_floor_insulation", + "double_glazing", + "secondary_glazing", + "low_energy_lighting", + "mechanical_ventilation", + "high_heat_retention_storage_heaters", + "air_source_heat_pump", + "gas_boiler_upgrade", + "system_tune_up", + "system_tune_up_zoned", + "solar_pv", + "secondary_heating_removal", + ]; + + it("offers exactly the backend MeasureType vocabulary", () => { + expect([...scenarioMeasuresList].sort()).toEqual([...BACKEND_MEASURE_TYPES].sort()); + }); + + it("rejects engine-unparseable tokens at authoring time (the 1275 crash)", () => { + const result = validateScenarioConfig({ + name: "Low disruption", + housingType: "Social", + goal: "Increasing EPC", + goalValue: "C", + budgetPerProperty: null, + exclusions: ["room_roof_insulation"], + }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.errors.exclusions).toMatch(/room_roof_insulation/); + }); +}); + describe("validateScenarioConfig", () => { it("requires a target band when the goal is Increasing EPC", () => { const result = validateScenarioConfig({ @@ -148,7 +193,7 @@ describe("validateScenarioConfig", () => { }); it("rejects excluding every measure — the engine needs at least one option", () => { - const result = validateScenarioConfig({ ...validBase, exclusions: [...measuresList] }); + const result = validateScenarioConfig({ ...validBase, exclusions: [...scenarioMeasuresList] }); expect(result.ok).toBe(false); if (!result.ok) expect(result.errors.exclusions).toBeTruthy(); }); @@ -231,7 +276,7 @@ describe("findExactDuplicate", () => { describe("DISRUPTIVE_MEASURES", () => { it("only marks real measure keys (a typo would silently drop the badge)", () => { for (const key of Object.keys(DISRUPTIVE_MEASURES)) { - expect(measuresList).toContain(key); + expect(scenarioMeasuresList as readonly string[]).toContain(key); } }); }); @@ -249,7 +294,7 @@ describe("display helpers", () => { measures: ["Solid Floor Insulation"], }); // 18 exclusions → say what's allowed instead (in canonical measure order) - const onlyTwo = measuresList.filter( + const onlyTwo = scenarioMeasuresList.filter( (k) => k !== "solar_pv" && k !== "air_source_heat_pump", ); expect(constraintSummary(onlyTwo)).toEqual({ diff --git a/src/lib/scenarios/model.ts b/src/lib/scenarios/model.ts index 5b596420..fe2e3820 100644 --- a/src/lib/scenarios/model.ts +++ b/src/lib/scenarios/model.ts @@ -8,12 +8,51 @@ */ import { PORTFOLIO_GOALS } from "@/app/db/schema/portfolio"; -import { - HousingType, - measuresDisplayLabels, - measuresList, - type MeasureKey, -} from "@/app/db/schema/recommendations"; +import { HousingType } from "@/app/db/schema/recommendations"; + +/** + * The measures a Scenario may exclude — EXACTLY the backend's `MeasureType` + * values (Hestia-Homes/Model, domain/modelling/measure_type.py). The engine's + * exclusion parsing is strict: a token outside this set crashes the modelling + * run for every property in the scenario, so this list and the backend enum + * must change together (the backend's ADR-0056 shared-contract rule; pinned + * by a test). Tokens are the backend's; labels are ours. + * + * Deliberately narrower than the legacy `measuresDisplayLabels` vocabulary + * (recommendations display / the old file-based trigger): measures the new + * engine cannot recommend (room-in-roof insulation, fireplace sealing, + * hot-water tank insulation, cylinder thermostats) are not offered — + * excluding the unrecommendable is meaningless, and storing the token is a + * time bomb. Room-in-roof insulation returns here when the backend + * implements it. + */ +export const SCENARIO_MEASURES = { + cavity_wall_insulation: "Cavity Wall Insulation", + external_wall_insulation: "External Wall Insulation", + internal_wall_insulation: "Internal Wall Insulation", + loft_insulation: "Loft Insulation", + sloping_ceiling_insulation: "Sloping Ceiling Insulation", + flat_roof_insulation: "Flat Roof Insulation", + suspended_floor_insulation: "Suspended Floor Insulation", + solid_floor_insulation: "Solid Floor Insulation", + double_glazing: "Double Glazing", + secondary_glazing: "Secondary Glazing", + low_energy_lighting: "Low Energy Lighting", + mechanical_ventilation: "Mechanical Ventilation", + high_heat_retention_storage_heaters: "High Heat Retention Storage Heaters", + air_source_heat_pump: "Air Source Heat Pump", + gas_boiler_upgrade: "Gas Boiler Upgrade", + system_tune_up: "Heating System Tune-up", + system_tune_up_zoned: "Heating System Tune-up (zoned)", + solar_pv: "Solar PV", + secondary_heating_removal: "Secondary Heating Removal", +} as const; + +export type ScenarioMeasureKey = keyof typeof SCENARIO_MEASURES; + +export const scenarioMeasuresList = Object.keys( + SCENARIO_MEASURES, +) as ScenarioMeasureKey[]; export const EPC_BANDS = ["A", "B", "C", "D", "E", "F", "G"] as const; export type EpcBand = (typeof EPC_BANDS)[number]; @@ -78,10 +117,12 @@ export function validateScenarioConfig( // value (any stray input is dropped); the only brake is the budget. const exclusions = [...new Set(input.exclusions)].sort(); - const unknown = exclusions.filter((k) => !(measuresList as string[]).includes(k)); + const unknown = exclusions.filter( + (k) => !(scenarioMeasuresList as string[]).includes(k), + ); if (unknown.length > 0) { errors.exclusions = `Unknown measures: ${unknown.join(", ")}`; - } else if (exclusions.length === measuresList.length) { + } else if (exclusions.length === scenarioMeasuresList.length) { errors.exclusions = "Every measure is excluded — the engine would have nothing to work with. Allow at least one."; } @@ -136,14 +177,16 @@ export function parseExclusions(text: string | null): string[] { * frequently exclude them. The value is the plain-language reason shown to * users. The "Low disruption" quick-start excludes exactly this set. */ -export const DISRUPTIVE_MEASURES: Partial> = { +export const DISRUPTIVE_MEASURES: Partial> = { internal_wall_insulation: "Rooms need clearing and replastering while the work is done", solid_floor_insulation: "Floors are dug up or raised — usually needs the home empty", suspended_floor_insulation: "Floorboards come up in every affected room", - room_roof_insulation: "Loft rooms are stripped back to the rafters", + // room-in-roof insulation belongs here ("loft rooms are stripped back to + // the rafters") but is not yet a backend MeasureType — restore it when the + // backend implements the measure. }; export type ScenarioStatus = "modelled" | "awaiting_modelling"; @@ -166,16 +209,19 @@ export type ConstraintSummary = export function constraintSummary(exclusions: string[]): ConstraintSummary { if (exclusions.length === 0) return { kind: "all" }; const excluded = new Set(exclusions); - const allowed = measuresList.filter((k) => !excluded.has(k)); + const allowed = scenarioMeasuresList.filter((k) => !excluded.has(k)); if (allowed.length <= 3) { - return { kind: "only", measures: allowed.map((k) => measuresDisplayLabels[k]) }; + return { kind: "only", measures: allowed.map((k) => SCENARIO_MEASURES[k]) }; } - return { - kind: "excludes", - measures: measuresList - .filter((k) => excluded.has(k)) - .map((k) => measuresDisplayLabels[k as MeasureKey]), - }; + const known = scenarioMeasuresList + .filter((k) => excluded.has(k)) + .map((k) => SCENARIO_MEASURES[k]); + // Legacy rows may hold tokens outside today's vocabulary — display them + // as themselves rather than dropping them silently. + const legacy = exclusions.filter( + (k) => !(scenarioMeasuresList as string[]).includes(k), + ); + return { kind: "excludes", measures: [...known, ...legacy] }; } /** The stored shape a duplicate check compares against (subset of the scenario row). */