docs(portfolio): ADR-0012 — list resolves descriptors via override precedence

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) <noreply@anthropic.com>
This commit is contained in:
Khalim Conn-Kowlessar 2026-07-10 23:21:21 +00:00
parent e48a5f4ae2
commit 6871aff62a
2 changed files with 90 additions and 0 deletions

View file

@ -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. "19001929", "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.

View file

@ -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", "19001929", … "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.