mirror of
https://github.com/Hestia-Homes/assessment-model.git
synced 2026-07-27 22:45:03 +00:00
docs: reporting glossary terms + ADR-0010 compliance-window filter
Gross/Net cost, Project delivery, Current stock, Likely downgrade/upgrade (band movement) added to CONTEXT.md from the reporting-redesign grilling session. ADR-0010 records the compliance-window filter as a report-view parameter, not a Scenario constraint. Also commits the previously untracked ADR-0001/0002 docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
1344d1a77d
commit
83f65567cf
4 changed files with 218 additions and 0 deletions
26
CONTEXT.md
26
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?"
|
||||
|
|
|
|||
71
docs/adr/0001-data-backfills-outside-drizzle.md
Normal file
71
docs/adr/0001-data-backfills-outside-drizzle.md
Normal file
|
|
@ -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 <join table>` 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).
|
||||
60
docs/adr/0002-effective-performance-is-canonical-current.md
Normal file
60
docs/adr/0002-effective-performance-is-canonical-current.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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
|
||||
Loading…
Add table
Reference in a new issue