From 5866eacedccdd697a071b23183caf44cf56f1dce Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 20 Jul 2026 14:22:31 +0100 Subject: [PATCH] =?UTF-8?q?docs(reporting):=20Scenario=20overlay=20term=20?= =?UTF-8?q?+=20ADRs=200015=E2=80=930017?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CONTEXT.md: add the Scenario overlay glossary term under Reporting. - ADR-0015: the Scenario overlay is one read model behind both metrics endpoints. - ADR-0016: the SAP↔EPC band threshold ladder has a single owner (@/lib/epc). - ADR-0017: Likely downgrade is band-movement only; the SAP-05 signal is retired. Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 4 ++ ...0015-scenario-overlay-is-one-read-model.md | 70 +++++++++++++++++++ ...6-sap-epc-threshold-ladder-single-owner.md | 52 ++++++++++++++ ...-likely-downgrade-is-band-movement-only.md | 54 ++++++++++++++ 4 files changed, 180 insertions(+) create mode 100644 docs/adr/0015-scenario-overlay-is-one-read-model.md create mode 100644 docs/adr/0016-sap-epc-threshold-ladder-single-owner.md create mode 100644 docs/adr/0017-likely-downgrade-is-band-movement-only.md diff --git a/CONTEXT.md b/CONTEXT.md index 09cc2cfd..4737d64e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -134,6 +134,10 @@ _Avoid_: PC cost, prelims 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 +**Scenario overlay**: +The "After scenario" read model — the aggregate reporting picture of a Portfolio once a plan selection is applied: the portfolio-after averages and EPC-band distribution, the works-list ledger (Gross cost, Net cost, homes upgraded, funding) and the genuine SAP uplift. The *after* half of every Current → After comparison. A plan selection is either a specific **Scenario**'s plans or the **recommended-plans default** (the latest `is_default` Plan per Property); the default view is the same overlay with no EPC target and the report-view filters (compliance window, lodged baseline — [ADR-0010](./docs/adr/0010-compliance-window-is-a-report-view-parameter.md)) inert. Computed once in `getScenarioOverlay` (`src/lib/reporting/overlay.ts`) and served by both scenario-metrics endpoints. See [ADR-0015](./docs/adr/0015-scenario-overlay-is-one-read-model.md). +_Avoid_: scenario metrics (the endpoint name, not the concept), after-distribution (only the band part), scenario results (Plans hold results; the overlay aggregates them) + ### Live projects **Project**: diff --git a/docs/adr/0015-scenario-overlay-is-one-read-model.md b/docs/adr/0015-scenario-overlay-is-one-read-model.md new file mode 100644 index 00000000..b970ce72 --- /dev/null +++ b/docs/adr/0015-scenario-overlay-is-one-read-model.md @@ -0,0 +1,70 @@ +# 15. The Scenario overlay is one read model behind both metrics endpoints + +Date: 2026-07-20 + +## Status + +Accepted + +## Context + +The reporting "After scenario" aggregate — the back half of every +Current → After comparison (portfolio-after averages, the after EPC-band +distribution, the works-list ledger, the SAP uplift) — was implemented twice, +in two Next.js route handlers: + +- `api/portfolio/[portfolioId]/scenario/[scenarioId]/metrics` (488 lines) +- `api/portfolio/[portfolioId]/scenario/default/metrics` (316 lines) + +They differed in exactly one thing: the plan selection. The `[scenarioId]` +route scopes to `scenario_id = `; the `default` route scopes to +`is_default = true` (the recommended plan per property). Everything else — the +three-query structure, the `computeLedger` wiring, the SAP band bucketing, the +compliance-window predicate ([ADR-0010]), the response shape — was duplicated +and drifting independently. The heaviest, most business-critical reporting SQL +also lived *outside* the reporting folder, so "the reporting queries" were not +discoverable in one place, and the JSON both routes returned was untyped and +re-described by each of its three consumers (the metrics body, the Compare +table, the PDF). + +## Decision + +**One read model owns the overlay; the routes are thin adapters.** + +- `getScenarioOverlay(portfolioId, scope, filters, tags)` in + `src/lib/reporting/overlay.ts` owns all overlay SQL and shaping. `scope` is a + discriminated union — a specific Scenario, or the recommended-plans default. +- The **default (recommended-plans) view is the scenario view with no EPC + target and every report-view filter inert.** The compliance-window and + lodged-baseline filters are a Scenario concept ([ADR-0010]); the default view + passes an inert filter set, so it flows through the same code path with the + filters switched off. +- The overlay's joins are **conditional on what the active filters read**, so + an unfiltered overlay never joins the lodged-certificate or legacy tables — + strictly leaner than the previous always-join `[scenarioId]` route. +- `ScenarioOverlay` is a single exported type in `src/lib/reporting/model.ts` + (client-safe): the wire contract the producer returns and every consumer + imports. +- The two route handlers now only parse and validate the request, call + `getScenarioOverlay`, and return it (or 404). + +This lands the reporting reads in `src/lib/reporting/` next to the pure +`model.ts` and the `viewState.ts` URL state, following the repo's +`src/lib//server.ts` convention rather than colocating data access in +the route folder. + +## Consequences + +- One definition of the after-scenario figures: band thresholds, the + compliance-window rule and the ledger wiring are fixed once. The unit of test + is one function, not two HTTP handlers. +- The default view gains a formal "inert filters" contract. A future decision + to honour, say, the compliance window in the recommended view becomes a + one-line change to the default adapter, not a second query to edit. +- The plans-only query dropped several `avg_*`/`total_*` columns that were + computed but never read in the response. +- Risk: the overlay is now shared, so a change intended for one view reaches + both. Mitigated by the typed contract and the scope/filters being explicit + inputs. + +[ADR-0010]: ./0010-compliance-window-is-a-report-view-parameter.md diff --git a/docs/adr/0016-sap-epc-threshold-ladder-single-owner.md b/docs/adr/0016-sap-epc-threshold-ladder-single-owner.md new file mode 100644 index 00000000..49dc7d52 --- /dev/null +++ b/docs/adr/0016-sap-epc-threshold-ladder-single-owner.md @@ -0,0 +1,52 @@ +# 16. The SAP↔EPC band threshold ladder has a single owner + +Date: 2026-07-20 + +## Status + +Accepted + +## Context + +The RdSAP band floors — A ≥ 92, B ≥ 81, C ≥ 69, D ≥ 55, E ≥ 39, F ≥ 21, else G +— are the most safety-critical constant in the domain: they decide which band a +home is in, and thus almost every reporting figure. They were written out in at +least five places, in both TypeScript and SQL: + +- `@/app/utils.sapToEpc` — the client ladder. +- `@/app/actions/recommendations.ts` — a hand-copied `sapToEpcLetter`, its own + comment admitting it "mirrors sapToEpc". +- `@/app/utils/epc.ts` — the `EPC_TO_SAP_MIN` / `EPC_TO_SAP_MAX` range records. +- `BAND_BUCKETS_SQL` and `EPC_MIN_SAP` in *both* scenario-metrics routes. + +`@/lib/epc/bands.ts` owned the band *letters* (drift-tested against the schema +enum) but not the *thresholds*. A re-scoring would have meant finding and +editing every copy in lockstep — exactly the kind of change that silently goes +wrong. + +## Decision + +**`@/lib/epc` is the single owner of the threshold ladder.** + +- `src/lib/epc/thresholds.ts` (pure, client-safe) holds `BAND_MIN_SAP` — the one + table — plus `sapToBand`, and derives `EPC_TO_SAP_MIN` / `EPC_TO_SAP_MAX` from + it rather than hand-maintaining them. +- `src/lib/epc/thresholdsSql.ts` (server-only) holds `sapBandBucketsSql`, the + SQL twin, built from the same `BAND_MIN_SAP`. Split from the pure module so + client bundles never pull in Drizzle. +- `@/app/utils.sapToEpc` and `@/app/utils/epc` now delegate to the canonical + ladder (keeping their historic names/paths so call sites are untouched); the + hand-copied `sapToEpcLetter` is deleted; both metrics routes use + `sapBandBucketsSql` / `BAND_MIN_SAP`. + +## Consequences + +- A re-scoring changes `BAND_MIN_SAP` and nothing else — TS and SQL stay in + lockstep by construction. +- `sapToBand` is `null → "Unknown"` and throws on a negative score (SAP is never + negative — surface the bad datum rather than bucket it into G); a test pins it + to the historic `sapToEpc` output for every integer 0–100. +- Follow-up (not done here): `@/lib/scenarios/model` still defines its own + `EPC_BANDS`; the reporting routes were pointed at `@/lib/epc/bands`, but that + second copy of the band letters remains for the scenarios domain to + consolidate. diff --git a/docs/adr/0017-likely-downgrade-is-band-movement-only.md b/docs/adr/0017-likely-downgrade-is-band-movement-only.md new file mode 100644 index 00000000..e05affca --- /dev/null +++ b/docs/adr/0017-likely-downgrade-is-band-movement-only.md @@ -0,0 +1,54 @@ +# 17. Likely downgrade is band-movement only; the SAP-05 signal is retired + +Date: 2026-07-20 + +## Status + +Accepted + +## Context + +**Likely downgrade** (CONTEXT.md) is defined as band movement: a home whose +lodged (register) band sits above its effective (modelled) band, so a re-survey +would likely certificate it lower — judged on band movement only, never SAP +points. + +Reporting had two live implementations over **near-disjoint populations**: + +- The current-stock **confidence strip** counted the legacy **SAP-05 point + comparison** (`current_sap_points < sap_05_score`), restricted to legacy + (pre-cutoff) properties. +- The **data-quality page** and the **drill-down** counted **band movement** + (`lodgedEpcBandSql < effectiveEpcBandSql`). + +These do not measure the same thing. For a legacy property, `lodgedEpcBandSql` +and `effectiveEpcBandSql` both resolve to the same column +(`p.current_epc_rating`), so band movement yields **zero** for exactly the +population the SAP-05 path covers. So the two surfaces could report different +"likely downgrade" counts for the same portfolio, and CONTEXT.md already labels +the point-level SAP signal "the retired signal". + +## Decision + +**Band movement is the single definition; the SAP-05 path is retired.** + +- New shared SQL fragments `likelyDowngradeSql` / `likelyUpgradeSql` in + `@/lib/services/epcSources` — the SQL twin of `model.classifyBandMovement`. +- `server.getLikelyDowngrades` (the confidence strip) now counts band movement, + so it agrees with the data-quality page. +- `getDataQualityMetrics` and the drill-down route use the shared fragments, so + all three surfaces read one definition. + +## Consequences + +- Legacy (pre-cutoff) portfolios now report likely-downgrades via band movement, + which is **0** where lodged and effective band are the same datum. This is a + deliberate, user-visible change: those homes never had a band-movement signal, + and the point-level one is retired. New-approach homes surface real band + movement through Rebaseline ([ADR-0002]). +- The `sap_05_*` columns are no longer read by reporting. They remain on + `property_details_epc` for now; dropping them is separate schema work. +- Likely downgrade/upgrade now has one SQL definition kept in step with the pure + `classifyBandMovement` — change the movement rule in one place. + +[ADR-0002]: ./0002-effective-performance-is-canonical-current.md