diff --git a/CONTEXT.md b/CONTEXT.md index 4016fdcc..30eb39f0 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -187,11 +187,29 @@ The act of substituting a corrected set of performance metrics in place of the L _Avoid_: override, adjustment, correction **EPC provenance** (`epc_property.source`): -Whether a property's EPC picture is a real certificate or a gap-fill. `lodged` = a real public/landlord EPC exists; `predicted` = no certificate, so the EPC was estimated from nearby properties (EPC Prediction gap-fill). Independent of Rebaseline: a `lodged` property may still be rebaselined, and a `predicted` property still carries Lodged-performance figures (mirrored estimates), so the presence of `lodged_*` columns does **not** imply a real certificate — only `source = lodged` does. +Whether a property's EPC picture is a real certificate or a gap-fill. Three values: +- `lodged` — a real, current public/landlord EPC exists. +- `predicted` — no certificate exists anywhere, so the EPC was gap-filled from nearby properties (EPC Prediction). +- `expired` — also gap-filled, but conditioned on **this dwelling's own expired certificate** from the historic register (Model ADR-0054). The gov EPC API only serves certificates registered since 2012, so a pre-2012 dwelling looks EPC-less to ingestion even though a real, stale certificate exists for it. + +Independent of Rebaseline: a `lodged` property may still be rebaselined, and a gap-filled property still carries Lodged-performance figures (mirrored estimates), so the presence of `lodged_*` columns does **not** imply a real certificate — only `source = lodged` does. +_Avoid_: estimated EPC (reserve "estimated" for the UI signal), source EPC + +**Predicted slot**: +`predicted` and `expired` are two flavours of one slot — a property holds at most one of them, and a re-ingestion may flip which. Every query must therefore address the **slot** (`source IN ('predicted','expired')`), never a single member: matching `source = 'predicted'` alone makes `expired` rows invisible, and since `epc_property.source` is plain `text` with no enum or CHECK constraint, nothing catches it. Mirrors `PREDICTED_SLOT_SOURCES` in the Model repo. See [ADR-0014](./docs/adr/0014-epc-cards-count-certificates-not-predictions.md). +_Avoid_: predicted source (it is two sources) + +**EPC coverage** — *does any certificate exist for this dwelling?* +`lodged` **or** `expired` ⇒ yes, one does (current, or out of date). `predicted`, or no `epc_property` rows at all ⇒ no, none does. + +This is a **different question** from provenance's "was the picture modelled?" — an `expired` property is gap-filled *and* has a certificate. Conflating the two is what made the reporting cards wrong: the "Homes Without an EPC" card counted every gap-fill, so homes whose only fault was an out-of-date certificate were reported as having no EPC at all. The cards partition on coverage, not on provenance. +_Avoid_: missing EPC (ambiguous — say "without an EPC" for coverage, "estimated" for provenance) _Avoid_: estimated EPC (reserve "estimated" for the UI signal), source EPC **Provenance signal** (UI): -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. +What the user is told about an EPC's trustworthiness, derived from EPC provenance and Rebaseline together: **Estimated** (`source IN (predicted, expired)` — the whole predicted slot, 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. + +An `expired` property is **Estimated**: its picture was modelled, not read off a current certificate. That it *also* has an (out-of-date) certificate is a matter of EPC coverage, which the reporting cards use — not of this signal. _Avoid_: estimation notification, banner (those are component names) ## Example dialogue diff --git a/docs/adr/0014-epc-cards-count-certificates-not-predictions.md b/docs/adr/0014-epc-cards-count-certificates-not-predictions.md new file mode 100644 index 00000000..c8a8b87c --- /dev/null +++ b/docs/adr/0014-epc-cards-count-certificates-not-predictions.md @@ -0,0 +1,105 @@ +# The EPC reporting cards count certificates, not predictions + +## Status + +accepted + +## Context + +The portfolio reporting page shows two EPC-quality cards: **Homes Without an EPC** +and **Expired EPCs**. Both were derived from `estimatedSql`, which asked *"is this +property's EPC picture a gap-fill?"*: + +```sql +-- "Homes Without an EPC" +SUM(CASE WHEN estimatedSql = true THEN 1 ELSE 0 END) +-- "Expired EPCs" +SUM(CASE WHEN isExpiredSql = true AND estimatedSql = false THEN 1 ELSE 0 END) +``` + +That conflates two genuinely different questions: + +- **Provenance** — was this picture *modelled*, or read off a real certificate? +- **Coverage** — does a certificate *exist* for this dwelling at all? + +They used to give the same answer, because `epc_property.source` had two values and +a gap-fill implied no certificate. That stopped being true. + +The gov EPC API only serves certificates registered since **1 January 2012**. A +dwelling whose only certificate predates that looks EPC-less to ingestion, so its +picture gets gap-filled — even though a real, if stale, certificate exists for it in +the historic register. Model ADR-0054 introduced a third `source` value, `expired`, +for exactly this case: a prediction *conditioned on the dwelling's own expired +certificate*. `predicted` and `expired` share one slot (`PREDICTED_SLOT_SOURCES`). + +Measured on portfolio 824 (5,276 homes): **403** homes were gap-filled, and **375** +of them have a real pre-2012 certificate in the historic backup. So the card claimed +403 homes had no EPC when 375 of them did — they just had an out-of-date one, which +is a different and far more actionable problem. It is also the *wrong* problem to +solve: you do not commission a new EPC survey for a home you believe has never been +certified in the same way you chase a lapsed one. + +Two further defects fell out of the same conflation: + +- `epc_property.source` is plain `text` with **no enum and no CHECK constraint**, and + every query matched `source = 'predicted'` literally. An `expired` row matches + neither the lodged nor the predicted alias, so the home would have read as though it + held a *valid current certificate* — silently absent from **both** cards, with no + Estimated badge. Nothing in the database would have caught this. +- `isExpiredSql` read `epl.registration_date` alone, but 714 of 824's 4,873 lodged + certificates carry only an `inspection_date`. All 714 were treated as current; 170 + of them are in fact over ten years old. (`lodgementDateSql` already coalesced the + two — the expiry predicate simply didn't.) + +## Decision + +**The two cards partition on coverage, not provenance.** A prediction is not a +certificate; an expired certificate is still a certificate. + +- `withoutEpcSql` — *no lodgement record and no historical record*: + `epl.id IS NULL AND epx.id IS NULL`. A blind `predicted` row is not evidence of an + EPC, so it is not consulted. A property with no `epc_property` rows at all also + lands here, correctly. +- `isExpiredSql` — *we found a certificate, of either kind, and it is out of date*: + an `expired` row (>10 years old by construction, since its conditioning certificate + predates 2012), **or** a lodged certificate older than ten years, dating it by + `COALESCE(registration_date, inspection_date)`. +- `estimatedSql` keeps its own, separate meaning — *was this modelled?* — and now + spans the whole predicted slot, so an `expired` home still carries the **Estimated** + provenance badge. It is estimated **and** has an EPC. Both are true. + +The `AND estimated = false` guard is removed from the Expired card and must stay +removed: an `expired` home *is* estimated, so the guard would exclude precisely the +homes the card exists to count. The rule it was really enforcing — *a gap-fill has no +certificate to expire* — now lives inside `isExpiredSql`, where a caller cannot forget +it. + +**Every query addresses the predicted slot (`epp` OR `epx`), never one member.** + +## Consequences + +Legacy properties (`updated_at` before the 2026-06-01 cutoff) are **unaffected** — +they have no `epc_property` rows and keep reading the flat `property_details_epc` +flags. Portfolio 632 reports 8 / 1,662 before and after. + +For new-approach properties the numbers move, in two steps. The `inspection_date` +fallback lands immediately and is a straight bug fix: 824's Expired count goes +**913 → 1,083**. Then, once Model starts writing `expired` rows, the cards settle at +**403 → 25** without an EPC and **1,083 → 1,461** expired. The portfolio has not +changed; we were miscounting it. + +This repo is safe to deploy **before** Model starts writing `expired` rows — the +predicates simply find none. The reverse is not true: had Model shipped first, every +`expired` home would have gone quietly missing from both cards. + +Two things we accept: + +- **The three-way reality is still squashed into booleans per card.** We do not + surface "has an expired certificate" and "has nothing at all" as distinct states + anywhere except by the two cards being disjoint. If a third state ever needs its own + treatment, this is where to add it. +- **Nothing enforces the slot at the database level.** `source` remains `text`. A + future query that matches `source = 'predicted'` alone would reintroduce the bug in + silence. `src/lib/services/epcSlot.test.ts` pins the invariant structurally; a CHECK + constraint or a Postgres enum would be stronger, and is worth doing when the schema + is next touched. diff --git a/src/app/portfolio/[slug]/(portfolio)/reporting/DashboardSummaryCards.tsx b/src/app/portfolio/[slug]/(portfolio)/reporting/DashboardSummaryCards.tsx index 439b54a4..8f2ab4a8 100644 --- a/src/app/portfolio/[slug]/(portfolio)/reporting/DashboardSummaryCards.tsx +++ b/src/app/portfolio/[slug]/(portfolio)/reporting/DashboardSummaryCards.tsx @@ -12,7 +12,7 @@ import { Home, Zap, Leaf, LineChart, FileQuestionIcon } from "lucide-react"; import { formatNumber, sapToEpc } from "@/app/utils"; import type { AverageMetrics, - EstimatedCounts, + EpcCoverageCounts, TotalMetrics, ScenarioOverlayMetrics, MetricKey, @@ -66,18 +66,18 @@ export function DashboardSummaryCards({ total, totals, averages, - estimatedCounts, + epcCoverage, scenarioOverlay, loading = false, }: { total: number; totals: TotalMetrics; averages: AverageMetrics; - estimatedCounts: EstimatedCounts; + epcCoverage: EpcCoverageCounts; scenarioOverlay?: ScenarioOverlayMetrics | null; loading?: boolean; }) { - const missingEpcCount = estimatedCounts.estimated; + const missingEpcCount = epcCoverage.withoutEpc; const missingEpcPercent = total > 0 ? (missingEpcCount / total) * 100 : 0; const averageCurrentEpc = sapToEpc(averages.avg_sap || 0); diff --git a/src/app/portfolio/[slug]/(portfolio)/reporting/EpcQualityCards.tsx b/src/app/portfolio/[slug]/(portfolio)/reporting/EpcQualityCards.tsx index 89893137..6dc57604 100644 --- a/src/app/portfolio/[slug]/(portfolio)/reporting/EpcQualityCards.tsx +++ b/src/app/portfolio/[slug]/(portfolio)/reporting/EpcQualityCards.tsx @@ -9,21 +9,21 @@ import { } from "@/app/shadcn_components/ui/card"; import { motion } from "framer-motion"; import { FileQuestion, AlertTriangle, TrendingDown } from "lucide-react"; -import type { EstimatedCounts } from "./types"; +import type { EpcCoverageCounts } from "./types"; export function EpcQualityCards({ - estimatedCounts, + epcCoverage, total, expiredEpcs, likelyDowngrades, }: { - estimatedCounts: EstimatedCounts; + epcCoverage: EpcCoverageCounts; total: number; expiredEpcs: number; likelyDowngrades: number; }) { - // Missing EPCs (estimated = true) - const missing = estimatedCounts.estimated; + // Homes with no certificate at all — neither lodged nor historical. + const missing = epcCoverage.withoutEpc; const pctMissing = total > 0 ? (missing / total) * 100 : 0; // Expired EPCs @@ -39,7 +39,7 @@ export function EpcQualityCards({ icon: FileQuestion, color: "text-red-600", value: missing, - subtitle: `${pctMissing.toFixed(1)}% missing EPC records`, + subtitle: `${pctMissing.toFixed(1)}% have no EPC on record`, barColor: "bg-red-500", barWidth: pctMissing, gradient: "bg-gradient-to-br from-white to-red-50/20", diff --git a/src/app/portfolio/[slug]/(portfolio)/reporting/ReportingClientArea.tsx b/src/app/portfolio/[slug]/(portfolio)/reporting/ReportingClientArea.tsx index 8d89323b..8dd01dbe 100644 --- a/src/app/portfolio/[slug]/(portfolio)/reporting/ReportingClientArea.tsx +++ b/src/app/portfolio/[slug]/(portfolio)/reporting/ReportingClientArea.tsx @@ -315,7 +315,7 @@ export function ReportingClientArea({ total={activeMetrics.total} totals={activeMetrics.totals} averages={activeMetrics.averages} - estimatedCounts={activeMetrics.estimatedCounts} + epcCoverage={activeMetrics.epcCoverage} scenarioOverlay={scenarioOverlay} loading={scenarioBusy} /> @@ -329,7 +329,7 @@ export function ReportingClientArea({