From 6871aff62ad633c57bec0c0d6e9034d8bf2c5bda Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 10 Jul 2026 23:21:21 +0000 Subject: [PATCH] =?UTF-8?q?docs(portfolio):=20ADR-0012=20=E2=80=94=20list?= =?UTF-8?q?=20resolves=20descriptors=20via=20override=20precedence?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the decision to make the portfolio property-list override-aware for Property Type / Built Form / Construction Age / Main Fuel, extending ADR-0008's override → EPC-derived → Unknown precedence to a second surface via shared epcSources fragments. Adds a CONTEXT.md Flagged-ambiguities note. Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 1 + ...ves-descriptors-via-override-precedence.md | 89 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 docs/adr/0012-portfolio-list-resolves-descriptors-via-override-precedence.md diff --git a/CONTEXT.md b/CONTEXT.md index de164f7b..da92a07b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -192,6 +192,7 @@ _Avoid_: estimation notification, banner (those are component names) ## Flagged ambiguities +- The portfolio **Property table**'s **Property Type**, **Built Form**, **Construction Age** and **Main Fuel** columns and filters show the **landlord-override-resolved** value — the same precedence the modelling **Run filter** applies (override fact → EPC-derived → **Unknown**) — not the raw `epc_property` value or the legacy property-row column. See [ADR-0012](./docs/adr/0012-portfolio-list-resolves-descriptors-via-override-precedence.md). Two label subtleties follow from this: **Construction Age** is an RdSAP band (e.g. "1900–1929", "before 1900"), never a single year (it was formerly the "Year Built" column); and **Main Fuel** is override-only for new-approach properties, so it reads **Unknown** unless a landlord override supplies it. - The UI surfaces labelled **"Current EPC"** and **"Current Efficiency State"** (property table, building-passport card) show **Effective performance**, not Lodged — despite the glossary advising against "current performance" for Effective. The label is a product choice ("current" = the property's true present-day modelled state); the underlying datum is still **Effective performance**. The **"Lodged EPC"** column/badge is the only surface showing **Lodged performance**, and only when a real certificate exists (`source = lodged`). - "Upload" is used in the codebase to mean both the file-on-S3 and the BulkUpload row. We standardise on **BulkUpload** for the row; the file is just "the source file." - "Onboarding" appears in some route paths (`bulk_onboarding_inputs/...`) but isn't part of this glossary — we use **BulkUpload** end-to-end. diff --git a/docs/adr/0012-portfolio-list-resolves-descriptors-via-override-precedence.md b/docs/adr/0012-portfolio-list-resolves-descriptors-via-override-precedence.md new file mode 100644 index 00000000..08f4c461 --- /dev/null +++ b/docs/adr/0012-portfolio-list-resolves-descriptors-via-override-precedence.md @@ -0,0 +1,89 @@ +# 12. The portfolio property-list resolves descriptors through the landlord-override precedence + +Date: 2026-07-10 + +## Status + +Accepted + +## Context + +The portfolio property-list's **Property Type**, **Built Form**, **Year Built** +and **Main Fuel** columns and filters read raw `property` row columns (and, for +main fuel, the legacy `property_details_epc` table). Those columns are NULL for +**new-approach** properties (`property.updated_at >= 2026-06-01`, the EPC-graph +cutoff — see `epcSources.ts`), so for any new portfolio the columns render blank +and the filters match nothing. The already-migrated columns (current/lodged EPC, +floor area, CO₂, provenance, expiry) route through `epcSources` fragments and +work; these four were simply never migrated. + +Separately, [ADR-0008](./0008-modelling-runs-filters-to-distributor.md) +established the resolution precedence for the modelling **Run filter**: +**landlord-override fact → EPC-derived value (new-approach graph, lodged over +predicted, RdSAP codes mapped; legacy property-row fallback) → Unknown**. It is +implemented in-app inline in `previewModellingRun` and mirrored by the +distributor; divergence between the two is defined as a bug. + +## Decision + +The portfolio list resolves Property Type, Built Form, Construction Age and Main +Fuel through the **same override precedence as the Run filter**, so the browse +surface agrees with what a modelling run would actually select. Concretely: + +- **Shared fragments, one home for the rule.** `epcSources.ts` gains + `overrideJoins` (LEFT JOINs of `property_overrides` aliased per component at + `building_part = 0`) and `resolvedPropertyTypeSql` / `resolvedBuiltFormSql` / + `resolvedConstructionBandSql` / `resolvedMainFuelSql`. Both the list query and + `previewModellingRun` consume them; the inline COALESCE in + `previewModellingRun` is refactored onto the shared fragments. +- **Never-null.** Each fragment is `COALESCE(override, epcSql, 'Unknown')`. Every + property resolves to a concrete, filterable string; the **Unknown** filter chip + matches "no resolvable value at all" — character-for-character the semantics + CONTEXT.md already gives the Run filter's Unknown. The list columns render an + explicit **Unknown** pill instead of a blank `—`. +- **Construction Age is band-native, not a year.** Override and EPC both store + the RdSAP band **letter** (`A..M`, `Unknown`); the field resolves in letter + space and displays the band **label** ("before 1900", "1900–1929", … "2023 + onwards"). A numeric year cannot represent an `Unknown` override and misreads a + band as a precise year. The column is **renamed "Year Built" → "Construction + Age"** and its filter options become the bands, not years; legacy raw years are + bucketed into bands. +- **Main Fuel is override-only for new-approach.** The EPC graph exposes no clean + main-fuel string (a documented `epcSources` gap → NULL), so for new-approach + properties the override is the *only* source; unoverridden properties read + **Unknown**. The override vocabulary (`"mains gas"`, `"LPG (bulk)"`, …) is + folded into the existing collapsed filter labels (Mains Gas, LPG, …). +- **`tenure` is removed from the list**, not fixed. It has no override component, + and its `epc_property.tenure` values don't cleanly reconcile with the filter's + `dbValues`. It is dropped from the list column and filter only; + `resolvePropertyMeta`/`resolvePropertyDescriptors` still resolve it for the + per-property building passport, and `property.tenure` is retained. + +## Alternatives considered + +- **EPC-derived only, no overrides.** Smaller, and consistent with how the other + already-migrated columns behave. Rejected: the list would then disagree with + the modelling Run filter — e.g. show "Flat" for a property a landlord override + makes (and modelling treats as) "Maisonette" — reintroducing exactly the + browse-vs-model mismatch this work set out to remove. +- **Construction Age as a numeric year.** Familiar and a smaller diff. Rejected: + cannot represent an `Unknown` override (collapses to `—`, breaking the + never-null contract) and misrepresents a band as a point year. +- **Keep the precedence rule inline in `previewModellingRun` and duplicate it in + the list.** Avoids touching a working feature. Rejected: it re-creates the + two-homes divergence ADR-0008 explicitly warns against — now within the app. + +## Consequences + +- The count query (`getPropertiesCount`) must join `property_overrides` when any + of these fields is filtered — `EPC_JOIN_FILTER_FIELDS` (or a sibling set) is + extended. Joins are perf-sensitive (the count path already brushes Vercel's + 15s limit), so the joins stay conditional on an active filter. +- Every new-approach property without a main-fuel override shows **Unknown** — a + lot of Unknown pills, but an honest signal of the EPC gap rather than a blank + implying "not applicable". +- Filter option lists grow: Property Type gains **Park home** + **Unknown**; + Built Form gains **Unknown** (keeping **Not Recorded** for the explicit + override); Main Fuel folds the override vocab into existing labels. +- `previewModellingRun` and the portfolio list are now provably in sync — the + shared fragment is the single reference implementation ADR-0008 asked for.