From 3bf816130ca240c6fcbc67de61e1a2f89a65732f Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 10 Jul 2026 23:21:22 +0000 Subject: [PATCH] feat(portfolio): resolve Property Type column + filter via override precedence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 1 of the portfolio-list descriptor fix. Property Type was read from the raw property row (p.property_type), which is NULL for new-approach properties (updated_at >= 2026-06-01), so the column and filter broke for new portfolios. - New pure resolver resolvePropertyType (descriptorResolution.ts, mirrors the provenance.ts split) owning override → EPC-derived (lodged over predicted, RdSAP codes mapped, legacy fallback) → 'Unknown', with 9 unit tests. - SQL twin resolvedPropertyTypeSql + propertyTypeOverrideJoin in epcSources.ts. - Wired into getProperties SELECT, the filter colMap, and the count query's conditional join set (propertyType added to EPC_JOIN_FILTER_FIELDS). - PROPERTY_TYPE_OPTIONS gains Park home + Unknown. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/portfolio/[slug]/utils.ts | 16 ++- src/app/utils/propertyFilters.ts | 6 + src/lib/services/descriptorResolution.test.ts | 132 ++++++++++++++++++ src/lib/services/descriptorResolution.ts | 49 +++++++ src/lib/services/epcSources.ts | 25 ++++ 5 files changed, 225 insertions(+), 3 deletions(-) create mode 100644 src/lib/services/descriptorResolution.test.ts create mode 100644 src/lib/services/descriptorResolution.ts diff --git a/src/app/portfolio/[slug]/utils.ts b/src/app/portfolio/[slug]/utils.ts index 0eb1b88e..a3228f17 100644 --- a/src/app/portfolio/[slug]/utils.ts +++ b/src/app/portfolio/[slug]/utils.ts @@ -31,6 +31,8 @@ import { lodgedEpcBandSql, lodgedSapSql, provenanceSignalSql, + propertyTypeOverrideJoin, + resolvedPropertyTypeSql, } from "@/lib/services/epcSources"; import { FilterGroups, @@ -595,7 +597,8 @@ function buildConditionSql(filter: PropertyFilter): ReturnType | nul const options = ENUM_FIELD_DB_OPTIONS[filter.field]; const colMap: Record> = { - propertyType: sql`p.property_type`, + // Override-resolved to match the column + the Run filter (ADR-0012). + propertyType: resolvedPropertyTypeSql, builtForm: sql`p.built_form`, tenure: sql`p.tenure`, yearBuilt: sql`p.year_built`, @@ -695,6 +698,8 @@ const EPC_JOIN_FILTER_FIELDS = new Set([ "floorArea", "epcExpiryDate", "mainfuel", + // Override-resolved (ADR-0012): needs the EPC graph joins + the override join. + "propertyType", ]); const PLAN_JOIN_FILTER_FIELDS = new Set(["expectedEpc"]); @@ -720,7 +725,8 @@ export async function getPropertiesCount( const epcJoins = needsEpcJoins ? sql`LEFT JOIN property_details_epc epc ON epc.property_id = p.id - ${newApproachJoins}` + ${newApproachJoins} + ${propertyTypeOverrideJoin}` : sql``; const planJoin = needsPlanJoin ? sql`LEFT JOIN LATERAL ( @@ -789,7 +795,10 @@ export async function getProperties( COALESCE(rec.cost, 0) AS "totalRecommendationCost", p.landlord_property_id AS "landlordPropertyId", p.original_sap_points AS "originalSapPoints", - p.property_type AS "propertyType", + -- Property Type is resolved through the landlord-override precedence + -- (override → EPC-derived → 'Unknown'), matching the modelling Run filter. + -- See resolvePropertyType / ADR-0012. + ${resolvedPropertyTypeSql} AS "propertyType", p.built_form AS "builtForm", p.tenure AS tenure, p.year_built AS "yearBuilt", @@ -833,6 +842,7 @@ export async function getProperties( LEFT JOIN property_details_epc epc ON epc.property_id = p.id ${newApproachJoins} + ${propertyTypeOverrideJoin} WHERE p.portfolio_id = ${portfolioId} -- Unmatched (no-UPRN) properties live in the "Needs attention" tab until -- resolved, so they're excluded from the main table. diff --git a/src/app/utils/propertyFilters.ts b/src/app/utils/propertyFilters.ts index 1f85675e..f62181e5 100644 --- a/src/app/utils/propertyFilters.ts +++ b/src/app/utils/propertyFilters.ts @@ -64,11 +64,17 @@ export interface EnumOption { dbValues: string[]; } +// dbValues are the label vocabulary emitted by resolvedPropertyTypeSql (override +// snapshot / EPC-derived label). "Park home" is reachable from an RdSAP code (4) +// or an override; "Unknown" is the never-null terminal + an explicit override — +// it selects properties with no resolvable value at all (ADR-0012). export const PROPERTY_TYPE_OPTIONS: EnumOption[] = [ { label: "House", dbValues: ["House"] }, { label: "Flat", dbValues: ["Flat"] }, { label: "Bungalow", dbValues: ["Bungalow"] }, { label: "Maisonette", dbValues: ["Maisonette"] }, + { label: "Park home", dbValues: ["Park home"] }, + { label: "Unknown", dbValues: ["Unknown"] }, ]; export const BUILT_FORM_OPTIONS: EnumOption[] = [ diff --git a/src/lib/services/descriptorResolution.test.ts b/src/lib/services/descriptorResolution.test.ts new file mode 100644 index 00000000..69c68a51 --- /dev/null +++ b/src/lib/services/descriptorResolution.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from "vitest"; +import { resolvePropertyType } from "./descriptorResolution"; + +describe("resolvePropertyType", () => { + it("prefers a landlord override over the EPC-derived value", () => { + expect( + resolvePropertyType({ + override: "Maisonette", + isNewApproach: true, + lodgedPropertyType: "2", // EPC code for Flat + lodgedDwellingType: null, + predictedPropertyType: null, + predictedDwellingType: null, + legacyPropertyType: null, + }), + ).toBe("Maisonette"); + }); + + it("maps an RdSAP property-type code to its label when there is no override", () => { + expect( + resolvePropertyType({ + override: null, + isNewApproach: true, + lodgedPropertyType: "0", // RdSAP code for House + lodgedDwellingType: null, + predictedPropertyType: null, + predictedDwellingType: null, + legacyPropertyType: null, + }), + ).toBe("House"); + }); + + it("falls back to the predicted EPC when there is no lodged value", () => { + expect( + resolvePropertyType({ + override: null, + isNewApproach: true, + lodgedPropertyType: null, + lodgedDwellingType: null, + predictedPropertyType: "2", // RdSAP code for Flat + predictedDwellingType: null, + legacyPropertyType: null, + }), + ).toBe("Flat"); + }); + + it("prefers the lodged EPC over the predicted EPC when both exist", () => { + expect( + resolvePropertyType({ + override: null, + isNewApproach: true, + lodgedPropertyType: "0", // House + lodgedDwellingType: null, + predictedPropertyType: "2", // Flat + predictedDwellingType: null, + legacyPropertyType: null, + }), + ).toBe("House"); + }); + + it("falls back to dwelling_type when the lodged property_type is absent", () => { + expect( + resolvePropertyType({ + override: null, + isNewApproach: true, + lodgedPropertyType: null, + lodgedDwellingType: "Flat", + predictedPropertyType: null, + predictedDwellingType: null, + legacyPropertyType: null, + }), + ).toBe("Flat"); + }); + + it("reads the legacy property-row value for a legacy property", () => { + expect( + resolvePropertyType({ + override: null, + isNewApproach: false, + lodgedPropertyType: "0", // must be ignored for legacy + lodgedDwellingType: null, + predictedPropertyType: null, + predictedDwellingType: null, + legacyPropertyType: "Bungalow", + }), + ).toBe("Bungalow"); + }); + + it("never reads the EPC graph nor the legacy row across the cutoff (new-approach ignores the legacy row)", () => { + expect( + resolvePropertyType({ + override: null, + isNewApproach: true, + lodgedPropertyType: null, + lodgedDwellingType: null, + predictedPropertyType: null, + predictedDwellingType: null, + legacyPropertyType: "Bungalow", // legacy row must NOT surface for a new-approach property + }), + ).toBe("Unknown"); + }); + + // Spec locks for decisions taken in ADR-0012 — an override always wins (even an + // explicit "Unknown"), and an unrecognised RdSAP code is passed through as-is. + it("lets an explicit 'Unknown' override win over a known EPC value", () => { + expect( + resolvePropertyType({ + override: "Unknown", + isNewApproach: true, + lodgedPropertyType: "0", // EPC knows "House" — the override still wins + lodgedDwellingType: null, + predictedPropertyType: null, + predictedDwellingType: null, + legacyPropertyType: null, + }), + ).toBe("Unknown"); + }); + + it("passes an unrecognised RdSAP code through unchanged", () => { + expect( + resolvePropertyType({ + override: null, + isNewApproach: true, + lodgedPropertyType: "9", // not in PROPERTY_TYPE_LABELS + lodgedDwellingType: null, + predictedPropertyType: null, + predictedDwellingType: null, + legacyPropertyType: null, + }), + ).toBe("9"); + }); +}); diff --git a/src/lib/services/descriptorResolution.ts b/src/lib/services/descriptorResolution.ts new file mode 100644 index 00000000..18dc015b --- /dev/null +++ b/src/lib/services/descriptorResolution.ts @@ -0,0 +1,49 @@ +/** + * Pure precedence resolvers for the portfolio-list property descriptors. + * + * Mirrors the provenance.ts split: the DB query gathers the raw inputs (landlord + * override snapshot, lodged/predicted EPC values, legacy property-row value) and + * these functions own the precedence + RdSAP code→label mapping. The SQL twins + * in epcSources.ts (resolvedPropertyTypeSql, …) mirror this logic — keep in sync. + * + * Precedence (ADR-0008, extended to the list by ADR-0012): + * landlord override → EPC-derived (lodged over predicted, codes mapped; + * legacy property-row fallback) → 'Unknown' (never null) + */ +import { PROPERTY_TYPE_LABELS } from "./epcSources"; + +export interface PropertyTypeInputs { + /** property_overrides.override_value for component 'property_type', building_part 0. */ + override: string | null; + /** Whether the property reads the new EPC graph (updated_at >= cutoff). */ + isNewApproach: boolean; + /** epc_property(source='lodged').property_type — RdSAP code or text. */ + lodgedPropertyType: string | null; + /** epc_property(source='lodged').dwelling_type — fallback when property_type is absent. */ + lodgedDwellingType: string | null; + predictedPropertyType: string | null; + predictedDwellingType: string | null; + /** property.property_type — the legacy row value. */ + legacyPropertyType: string | null; +} + +export function resolvePropertyType(i: PropertyTypeInputs): string { + if (i.override) return i.override; + const derived = i.isNewApproach + ? epcPropertyType(i.lodgedPropertyType, i.lodgedDwellingType) ?? + epcPropertyType(i.predictedPropertyType, i.predictedDwellingType) + : i.legacyPropertyType; + return derived ?? "Unknown"; +} + +/** + * One EPC source's property-type label: the RdSAP code mapped to a label (text + * passes through), falling back to dwelling_type. Mirrors epcPropertyTypeSql. + */ +function epcPropertyType( + propertyType: string | null, + dwellingType: string | null, +): string | null { + if (propertyType != null) return PROPERTY_TYPE_LABELS[propertyType] ?? propertyType; + return dwellingType; +} diff --git a/src/lib/services/epcSources.ts b/src/lib/services/epcSources.ts index 3a75bfd9..3392d667 100644 --- a/src/lib/services/epcSources.ts +++ b/src/lib/services/epcSources.ts @@ -283,6 +283,31 @@ export const builtFormTypeSql = sql`CASE ELSE p.built_form END`; +// ───────────────────────────────────────────────────────────────────────────── +// Landlord-override-resolved descriptors (ADR-0012). Each fragment wraps the +// EPC-derived expression above with the property_overrides snapshot and a +// never-null 'Unknown' terminal — the same precedence previewModellingRun +// applies (override → EPC-derived → Unknown). The TS twins in +// descriptorResolution.ts (resolvePropertyType, …) own and unit-test this +// precedence; keep the two in sync. +// +// A resolved fragment references an override alias that its join provides — add +// the matching *OverrideJoin to any query that selects/filters on the fragment. +// Overrides are whole-dwelling for these components, so the join pins +// building_part = 0 (the main building) and never multiplies rows. +// ───────────────────────────────────────────────────────────────────────────── + +/** LEFT JOIN exposing the property_type override snapshot (alias `pot`). */ +export const propertyTypeOverrideJoin = sql` + LEFT JOIN property_overrides pot + ON pot.property_id = p.id + AND pot.override_component = 'property_type' + AND pot.building_part = 0 +`; + +/** property_type: override → EPC-derived → 'Unknown'. Mirrors resolvePropertyType. */ +export const resolvedPropertyTypeSql = sql`COALESCE(pot.override_value, ${propertyTypeSql}, 'Unknown')`; + /** * The main building part's construction age band for one epc_property alias, as * a correlated scalar subquery — properties can also have extension parts, so a