diff --git a/CONTEXT.md b/CONTEXT.md index 33bf900a..09f36a20 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -112,6 +112,24 @@ _Avoid_: job, batch, trigger request, modelling task (the pipeline's execution r The property-selecting constraints of a Modelling run: postcode, property type, and built form (each multi-select; tags later). An absent filter means *unconstrained* — "all postcodes" is the absence of a postcode filter, never an enumeration. Type and built form are the canonical enum vocabularies plus **Unknown**, which selects properties with no resolvable value at all. Resolution follows the Landlord-override rule: an override fact beats an EPC-derived value; the modelling backend is the single authority for resolving filters to properties (the app never re-implements the rule — matched counts come from the backend). _Avoid_: property query, segment, search (the postcode-search journey is unrelated) +### Reporting + +**Gross cost**: +The all-in price of delivering a Scenario's works: construction works + project delivery + contingency. Nothing a landlord pays for sits outside it. +_Avoid_: total cost, capex, cost of works (that's the construction line only) + +**Net cost**: +Gross cost minus secured funding. The figure a board commits to. +_Avoid_: net investment, cost after grants + +**Project delivery**: +The professional and management costs of delivering works, as distinct from the construction works themselves. +_Avoid_: PC cost, prelims + +**Current stock**: +The reporting view of a Portfolio with no Scenario applied — every figure is Effective performance, aggregated. Also the "before" side of any scenario comparison ("Current → After scenario"). +_Avoid_: baseline (reserved for property-level performance concepts), portfolio overview + ## Lifecycle A **BulkUpload** moves through these statuses: @@ -170,6 +188,14 @@ _Avoid_: estimated EPC (reserve "estimated" for the UI signal), source EPC What the user is told about an EPC's trustworthiness, derived from EPC provenance and Rebaseline together: **Estimated** (`source = predicted` — dominant; "estimated based on nearby homes") › **Re-modelled** (`source = lodged AND rebaseline_reason != none` — effective diverged from the lodged certificate under SAP 10) › **none** (`source = lodged AND rebaseline_reason = none` — effective equals lodged). Estimated always wins when both could apply. _Avoid_: estimation notification, banner (those are component names) +**Likely downgrade**: +A property whose Lodged band sits **above** its Effective band — a re-survey would likely certificate it lower. Judged on **band movement only**; SAP-point differences within a band don't qualify. +_Avoid_: SAP downgrade (point-level, the retired signal), at-risk property + +**Likely upgrade**: +A property whose Effective band sits **above** its Lodged band — Landlord overrides or SAP 10.2 scoring improved the modelled picture, so a re-survey alone would likely certificate it higher, without works. The cheapest compliance move in a portfolio. +_Avoid_: quick win, free upgrade + ## Example dialogue > **Dev:** "If the **Combiner** finishes but the user hasn't clicked Finalise, what does the user see?" diff --git a/docs/adr/0001-data-backfills-outside-drizzle.md b/docs/adr/0001-data-backfills-outside-drizzle.md new file mode 100644 index 00000000..a35d1818 --- /dev/null +++ b/docs/adr/0001-data-backfills-outside-drizzle.md @@ -0,0 +1,71 @@ +# 1. Data backfills run outside drizzle migrations + +Date: 2026-06-07 + +## Status + +Accepted + +## Context + +We needed to denormalise two join tables onto `recommendation` — `plan_id` +(from `plan_recommendations`) and `material_id` + measure columns (from +`recommendation_materials`) — as the first step toward making those columns the +source of truth and dropping the join tables. + +The original migrations (0222–0225) did everything inside drizzle: + +1. `ADD COLUMN` + validating `ADD CONSTRAINT FK` + `CREATE INDEX` +2. a single full-table `UPDATE ... FROM ` to backfill + +Run against the production `recommendation` table (millions of rows) it ran for +**3.5+ hours with no end in sight** and had to be killed. Diagnosis: + +- **drizzle wraps every pending migration in ONE transaction** + (`session.transaction(...)` in `pg-core/dialect`). So: + - the `ADD COLUMN` from 0222 took an `AccessExclusiveLock` on `recommendation` + on the very first statement and **held it for the entire run** — the whole + table was unreadable/unwritable for hours. + - an unrelated migration that happened to be pending in the same batch was + coupled to this one and would have rolled back with it. + - you cannot `COMMIT` between batches, so the backfill could only be one giant + statement — huge WAL, table bloat (~2×), and **EBS IO burst balance was + being drained** with no way to report progress or resume. + - `CREATE INDEX CONCURRENTLY` is impossible (it can't run in a transaction), + so the index was built non-concurrently *before* the backfill — meaning the + `UPDATE` also had to maintain that index for every row. + +The `2026_01_06_recommendation_cover.sql` file already in the repo (a manual, +un-journaled `CREATE INDEX CONCURRENTLY`) showed a prior author hitting the same +wall and stepping outside drizzle for it. + +## Decision + +**Schema DDL stays in drizzle migrations; data backfills and online operations +do not.** + +- drizzle migrations contain only instant, metadata-only DDL: `ADD COLUMN` + (nullable, no default) and `ADD CONSTRAINT FK ... NOT VALID`. These commit in + milliseconds and release the exclusive lock immediately. +- Backfills run as standalone, idempotent, resumable `tsx` scripts that **commit + in batches** (keyset-paginated by `id`), with a configurable inter-batch pause + to protect IO burst balance. +- Index creation (`CREATE INDEX CONCURRENTLY IF NOT EXISTS`) and FK validation + (`VALIDATE CONSTRAINT`) happen in that script, online, after the data is in. + +See `src/app/db/backfill-recommendation-denormalization.ts`. + +## Consequences + +- Deploys gain a step: after `npm run migration:migrate`, run the relevant + backfill script. This is a manual ops step, not part of the migrate command. +- Backfills no longer hold long locks, no longer block unrelated migrations, stay + within IO budget, and report progress + can be resumed after interruption. +- Because the columns are populated out-of-band, any later `SET NOT NULL` / + drop-join-table step must be its own migration, gated on the backfill having + completed and verified zero unexpected NULLs (note: `material_id` is expected + to remain nullable — not every recommendation has a material). +- Trade-off: backfills are no longer atomic with their schema change, and the + schema can briefly exist with unpopulated columns. The read paths must tolerate + NULLs until the backfill finishes — which is acceptable and was already the + case (the columns are new). diff --git a/docs/adr/0002-effective-performance-is-canonical-current.md b/docs/adr/0002-effective-performance-is-canonical-current.md new file mode 100644 index 00000000..2443b0eb --- /dev/null +++ b/docs/adr/0002-effective-performance-is-canonical-current.md @@ -0,0 +1,60 @@ +# 2. Effective performance is the canonical "current" for display and reporting + +Date: 2026-06-23 + +## Status + +Accepted (supersedes the "reporting stays on lodged" position recorded mid-migration in `docs/wip/new-modelling-ui-handover.md`). + +## Context + +`property_baseline_performance` carries two baselines per property: **Lodged +performance** (`lodged_*` — what was submitted to the government EPC register) +and **Effective performance** (`effective_*` — what the modelling engine +actually scored against, after a Rebaseline). See the glossary in +[CONTEXT.md](../../CONTEXT.md#baseline-performance). They diverge whenever +`rebaseline_reason != none` — most commonly `pre_sap10`, where the lodged +certificate used an older SAP version. In today's data 509 of 510 new-approach +properties are rebaselined, so the two baselines disagree for almost everyone. + +The plans (post-retrofit SAP/EPC, savings, valuation uplift) are all scored +against the **effective** baseline. If a property's displayed "current" is the +**lodged** figure, a plan reads as flat or as a downgrade — e.g. lodged C/78 vs +an effective baseline of C/71 with a post-retrofit of C/71.4. The first cut of +the migration switched only the per-property building-passport views to +effective and deliberately left the portfolio list + reporting aggregates on +lodged "to match the gov register." That left reporting's "current" disagreeing +with every property row's "current." + +## Decision + +**Effective performance is the canonical "current" everywhere we show or +aggregate a property's present-day state — per-property views *and* reporting. +Lodged performance is shown only as an explicit secondary "Lodged EPC" surface.** + +- Per-property resolvers (`resolvePropertyHeadline`, `resolveDetailsEpcMeta`, + `resolveNewConditionReport`) return `effective_*` for new-approach properties. +- Set-based fragments gain `effectiveSapSql` / `effectiveEpcBandSql`; the + portfolio property table and the reporting analytics (`AVG(sap)`, EPC-band + distribution) both read them. The original `lodged_*` fragments remain solely + to feed the dedicated "Lodged EPC" column/badge. +- The "Lodged EPC" surface renders only when a real certificate exists + (`source = lodged`); a `predicted` property shows the "estimated from nearby + homes" signal and no lodged figure, even though its `lodged_*` columns are + populated with mirrored estimates. +- Legacy (pre-cutoff) properties are unaffected: the fragments fall back to the + `property` row columns, and the provenance signal is `none`. + +## Consequences + +- Reporting totals no longer reconcile to the gov EPC register; they reflect the + modelled (effective) baseline. This is intentional — the register view is + available per-property via the Lodged EPC column. If a register-fidelity + reporting view is ever needed, it is a *separate* surface built on the retained + `lodged_*` fragments, not a reversion. +- "Current" in the UI now means Effective performance, contradicting the + glossary's advice to avoid "current performance" for Effective. Recorded as a + flagged ambiguity in CONTEXT.md so the next reader doesn't "correct" it. +- The split is the seam that matters: per-property object resolvers and the two + display fragments are effective; the lodged fragments still exist and must not + be deleted while the Lodged EPC surfaces use them. diff --git a/docs/adr/0010-compliance-window-is-a-report-view-parameter.md b/docs/adr/0010-compliance-window-is-a-report-view-parameter.md new file mode 100644 index 00000000..84d39f19 --- /dev/null +++ b/docs/adr/0010-compliance-window-is-a-report-view-parameter.md @@ -0,0 +1,61 @@ +# 10. The compliance-window filter is a report-view parameter, not a Scenario constraint + +Date: 2026-07-07 + +## Status + +Accepted + +## Context + +A recurring landlord question on Reporting: *"what if we don't do works on +properties whose EPC keeps them compliant, from a government perspective, +beyond 2030?"* — i.e. exclude homes that can demonstrate the target band with +an in-date certificate past a cut-off date, and see what the scenario costs +without them. + +Two places this could live: as a **constraint on the Scenario** (the optimiser +never plans works for those homes) or as a **filter on the report view** (the +modelling is unchanged; the report recomputes as if those homes' plans were +dropped). + +## Decision + +**A report-view parameter** on the scenario metrics endpoint (alongside +`hideNonCompliant` and the lodged-baseline toggle), configurable in the UI: + +- **Membership rule**: a home is "compliant beyond the window" iff its + **lodged** certificate shows the selected band or better (default **C**) + **and** remains valid past the selected date (default **1 Jan 2030**). Both + band and date are user-configurable. `predicted` homes never qualify — there + is no certificate to rely on (see EPC provenance in CONTEXT.md). +- **Scenario-side only**: skipped homes' plans drop out of the works-list + aggregates (costs, funding, benefits, homes upgraded) and they hold their + current band in the after-distribution. Current-stock figures are untouched; + the filter only exists when a scenario is selected. + +## Alternatives considered + +- **Scenario constraint (optimiser skips the homes)**: the "true" answer — the + engine would redistribute budget across the remaining stock. Rejected here + because Scenario configuration is immutable ([ADR-0003]) and adding a + post-hoc exclusion would either mutate that config or mint a near-duplicate + Scenario per date/band permutation. It also couples an exploratory, + slider-like question to a full modelling round-trip. +- **Whole-report filter**: hide the homes from every figure including current + stock. Rejected: the portfolio's home count would disagree with every other + surface, and the landlord's question is about works, not about the stock. + +## Consequences + +- The filtered works list is an **approximation**: plans were optimised with + the skipped homes in scope, so shared-cost effects (e.g. scaffolding + batching) are not re-optimised away. Acceptable for an exploratory filter; + the exact answer is future work — a re-model with an explicit exclusion set, + which would be a new Scenario capability, not this filter. +- The parameter travels in the URL, so filtered views are shareable and the + PDF can reproduce them. +- If landlords start committing budgets against filtered numbers rather than + exploring with them, that is the signal to build the re-model path. + +[ADR-0003]: ./0003-app-authored-scenarios.md