mirror of
https://github.com/Hestia-Homes/assessment-model.git
synced 2026-07-27 22:45:03 +00:00
fix(reporting): EPC cards count certificates, not predictions (ADR-0014)
"Homes Without an EPC" and "Expired EPCs" were both derived from estimatedSql,
which asks "was this picture gap-filled?". That conflates two different questions:
provenance (was it modelled?) and coverage (does a certificate exist?).
They used to agree, because a gap-fill implied no certificate. Model ADR-0054 ends
that: the gov EPC API only serves certs registered since 2012, so a pre-2012
dwelling is gap-filled even though a real, stale certificate exists — and gets the
new source='expired'. On portfolio 824, 375 of the 403 "without an EPC" homes have
exactly that: a real certificate, just an out-of-date one. A different, and far more
actionable, problem than never having been certified.
Two defects fell out of the same conflation:
- Every query matched source='predicted' literally, and `source` is plain text
with no enum or CHECK. An 'expired' row matches neither the lodged nor the
predicted alias, so the home would have read as having a VALID CURRENT cert —
silently gone from both cards, no Estimated badge, nothing to catch it.
- isExpiredSql read epl.registration_date alone, but 714 of 824's 4,873 lodged
certs carry only inspection_date. All 714 counted as current; 170 are in fact
>10 years old. lodgementDateSql already coalesced the two; expiry didn't.
The cards now partition on coverage: withoutEpcSql = no lodged AND no historical
record; isExpiredSql = a cert of either kind, out of date. estimatedSql keeps its
own meaning and spans the whole predicted slot, so an expired home is estimated AND
has an EPC — both true. The `AND estimated = false` guard is gone from the Expired
card: an expired home IS estimated, so it excluded the very homes being counted.
Legacy portfolios are untouched (632: 8/1662 before and after). 824 moves to
913 → 1,083 expired immediately (the inspection_date fix), then to 25 / 1,461 once
Model writes expired rows. Safe to deploy BEFORE Model — the predicates just find
none. The reverse was not safe, which is why this goes first.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jun-te Kim
parent
fc5fc80d88
commit
0933f05173
10 changed files with 351 additions and 44 deletions
22
CONTEXT.md
22
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
|
||||
|
|
|
|||
105
docs/adr/0014-epc-cards-count-certificates-not-predictions.md
Normal file
105
docs/adr/0014-epc-cards-count-certificates-not-predictions.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
|
||||
<div className="lg:col-span-2">
|
||||
<EpcQualityCards
|
||||
estimatedCounts={activeMetrics.estimatedCounts}
|
||||
epcCoverage={activeMetrics.epcCoverage}
|
||||
total={activeMetrics.total}
|
||||
expiredEpcs={activeMetrics.expiredEpcs}
|
||||
likelyDowngrades={activeMetrics.likelyDowngrades}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
energyConsumptionSql,
|
||||
estimatedSql,
|
||||
isExpiredSql,
|
||||
withoutEpcSql,
|
||||
effectiveSapSql,
|
||||
effectiveEpcBandSql,
|
||||
propertyTypeSql,
|
||||
|
|
@ -19,7 +20,7 @@ import type {
|
|||
AverageMetrics,
|
||||
AgeBandCount,
|
||||
EpcBandCount,
|
||||
EstimatedCounts,
|
||||
EpcCoverageCounts,
|
||||
PropertyTypeCount,
|
||||
BaselineMetrics,
|
||||
TotalMetrics,
|
||||
|
|
@ -132,13 +133,22 @@ export async function getCountByEpcBand(
|
|||
return result.rows;
|
||||
}
|
||||
|
||||
export async function getEstimatedCounts(
|
||||
/**
|
||||
* The "Homes Without an EPC" card: homes for which we found neither a lodgement
|
||||
* record nor a historical one.
|
||||
*
|
||||
* Deliberately NOT `estimatedSql`. A home whose prediction was conditioned on an
|
||||
* expired certificate is estimated, but it is not *without an EPC* — it has one,
|
||||
* and it belongs in the Expired card instead. The two cards partition the homes
|
||||
* that have some certificate vs none.
|
||||
*/
|
||||
export async function getEpcCoverageCounts(
|
||||
portfolioId: number,
|
||||
): Promise<EstimatedCounts> {
|
||||
const result = await db.execute<EstimatedCounts>(sql`
|
||||
): Promise<EpcCoverageCounts> {
|
||||
const result = await db.execute<EpcCoverageCounts>(sql`
|
||||
SELECT
|
||||
SUM(CASE WHEN ${estimatedSql(sql`e`)} = true THEN 1 ELSE 0 END)::int AS estimated,
|
||||
SUM(CASE WHEN ${estimatedSql(sql`e`)} = false THEN 1 ELSE 0 END)::int AS actual
|
||||
SUM(CASE WHEN ${withoutEpcSql(sql`e`)} = true THEN 1 ELSE 0 END)::int AS without_epc,
|
||||
SUM(CASE WHEN ${withoutEpcSql(sql`e`)} = false THEN 1 ELSE 0 END)::int AS with_epc
|
||||
FROM property p
|
||||
LEFT JOIN property_details_epc e ON e.property_id = p.id
|
||||
${newApproachJoins}
|
||||
|
|
@ -163,16 +173,20 @@ export async function getCountByPropertyType(
|
|||
return result.rows;
|
||||
}
|
||||
|
||||
/**
|
||||
* The "Expired EPCs" card: homes where we found a certificate — lodged or
|
||||
* historical — and it is out of date.
|
||||
*
|
||||
* The old `AND estimated = false` guard is gone, and must stay gone: a home whose
|
||||
* prediction was conditioned on an expired certificate IS estimated, so that guard
|
||||
* would exclude exactly the homes this card exists to count. The "a gap-fill has no
|
||||
* certificate to expire" rule it was really enforcing now lives inside
|
||||
* `isExpiredSql`, where it can't be forgotten by a caller.
|
||||
*/
|
||||
export async function getExpiredEpcCount(portfolioId: number): Promise<number> {
|
||||
const result = await db.execute<{ expired: number }>(sql`
|
||||
SELECT
|
||||
SUM(
|
||||
CASE
|
||||
WHEN ${isExpiredSql(sql`e`)} = true AND ${estimatedSql(sql`e`)} = false
|
||||
THEN 1
|
||||
ELSE 0
|
||||
END
|
||||
)::int AS expired
|
||||
SUM(CASE WHEN ${isExpiredSql(sql`e`)} = true THEN 1 ELSE 0 END)::int AS expired
|
||||
FROM property p
|
||||
LEFT JOIN property_details_epc e ON e.property_id = p.id
|
||||
${newApproachJoins}
|
||||
|
|
@ -213,7 +227,7 @@ export async function loadBaselineMetrics(
|
|||
totals,
|
||||
ageBands,
|
||||
epcBands,
|
||||
estimatedCounts,
|
||||
epcCoverage,
|
||||
expiredEpcs,
|
||||
likelyDowngrades,
|
||||
] = await Promise.all([
|
||||
|
|
@ -222,7 +236,7 @@ export async function loadBaselineMetrics(
|
|||
getTotals(portfolioId),
|
||||
getCountByAgeBand(portfolioId),
|
||||
getCountByEpcBand(portfolioId),
|
||||
getEstimatedCounts(portfolioId),
|
||||
getEpcCoverageCounts(portfolioId),
|
||||
getExpiredEpcCount(portfolioId),
|
||||
getLikelyDowngrades(portfolioId),
|
||||
]);
|
||||
|
|
@ -233,7 +247,7 @@ export async function loadBaselineMetrics(
|
|||
totals,
|
||||
ageBands,
|
||||
epcBands,
|
||||
estimatedCounts,
|
||||
epcCoverage,
|
||||
expiredEpcs,
|
||||
likelyDowngrades,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -175,7 +175,7 @@ export default async function ReportingPdfPage(props: {
|
|||
total={baseline.total}
|
||||
totals={baseline.totals}
|
||||
averages={baseline.averages}
|
||||
estimatedCounts={baseline.estimatedCounts}
|
||||
epcCoverage={baseline.epcCoverage}
|
||||
scenarioOverlay={scenarioOverlay}
|
||||
/>
|
||||
|
||||
|
|
@ -188,7 +188,7 @@ export default async function ReportingPdfPage(props: {
|
|||
/>
|
||||
|
||||
<EpcQualityCards
|
||||
estimatedCounts={baseline.estimatedCounts}
|
||||
epcCoverage={baseline.epcCoverage}
|
||||
total={baseline.total}
|
||||
expiredEpcs={baseline.expiredEpcs}
|
||||
likelyDowngrades={baseline.likelyDowngrades}
|
||||
|
|
|
|||
|
|
@ -23,9 +23,14 @@ export type EpcBandCount = {
|
|||
estimated: number;
|
||||
};
|
||||
|
||||
export type EstimatedCounts = {
|
||||
estimated: number;
|
||||
actual: number;
|
||||
/**
|
||||
* Whether a home has any certificate at all — lodged or historical.
|
||||
* NOT the same question as `estimated` (was the picture modelled?): a home
|
||||
* conditioned on an expired certificate is estimated but is not withoutEpc.
|
||||
*/
|
||||
export type EpcCoverageCounts = {
|
||||
withoutEpc: number;
|
||||
withEpc: number;
|
||||
};
|
||||
|
||||
export type PropertyTypeCount = {
|
||||
|
|
@ -39,7 +44,7 @@ export interface BaselineMetrics {
|
|||
totals: TotalMetrics;
|
||||
ageBands: AgeBandCount[];
|
||||
epcBands: EpcBandCount[];
|
||||
estimatedCounts: EstimatedCounts;
|
||||
epcCoverage: EpcCoverageCounts;
|
||||
expiredEpcs: number;
|
||||
likelyDowngrades: number;
|
||||
}
|
||||
|
|
|
|||
104
src/lib/services/epcSlot.test.ts
Normal file
104
src/lib/services/epcSlot.test.ts
Normal file
|
|
@ -0,0 +1,104 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { PgDialect } from "drizzle-orm/pg-core";
|
||||
import { sql } from "drizzle-orm";
|
||||
import {
|
||||
newApproachJoins,
|
||||
estimatedSql,
|
||||
withoutEpcSql,
|
||||
isExpiredSql,
|
||||
provenanceSignalSql,
|
||||
} from "./epcSources";
|
||||
|
||||
/**
|
||||
* `epc_property.source` has three values: `lodged`, and the two flavours of the
|
||||
* predicted slot — `predicted` (gap-filled blind) and `expired` (gap-filled, but
|
||||
* conditioned on the dwelling's own expired pre-2012 certificate; Model ADR-0054).
|
||||
*
|
||||
* These tests exist because of a specific, silent failure: every EPC expression
|
||||
* used to match `source = 'predicted'` alone. An `expired` row then matched
|
||||
* neither the lodged nor the predicted alias, so the home read as though it had a
|
||||
* valid current certificate and vanished from BOTH the "Homes Without an EPC" and
|
||||
* "Expired EPCs" cards. `epc_property.source` is plain `text` with no enum or CHECK
|
||||
* constraint, so nothing at the database level would have caught it.
|
||||
*
|
||||
* They assert on rendered SQL rather than query results because the repo has no
|
||||
* database harness — structural, but they pin the exact invariant that broke.
|
||||
*/
|
||||
|
||||
const dialect = new PgDialect();
|
||||
const render = (fragment: ReturnType<typeof sql>) =>
|
||||
dialect.sqlToQuery(fragment).sql;
|
||||
|
||||
// A zero-interpolation fragment renders empty on its own, so embed it.
|
||||
const renderJoins = () => render(sql`SELECT 1 ${newApproachJoins}`);
|
||||
|
||||
describe("newApproachJoins", () => {
|
||||
it("joins all three sources, so no epc_property row is invisible", () => {
|
||||
const joins = renderJoins();
|
||||
expect(joins).toContain("epl.source = 'lodged'");
|
||||
expect(joins).toContain("epp.source = 'predicted'");
|
||||
expect(joins).toContain("epx.source = 'expired'");
|
||||
});
|
||||
});
|
||||
|
||||
describe("estimatedSql — 'was this picture modelled?'", () => {
|
||||
it("counts BOTH flavours of the predicted slot as estimated", () => {
|
||||
// An expired-conditioned prediction is still a prediction: it must carry the
|
||||
// Estimated provenance signal, not read as a real certificate.
|
||||
expect(render(estimatedSql(sql`e`))).toContain(
|
||||
"(epl.id IS NULL AND (epp.id IS NOT NULL OR epx.id IS NOT NULL))",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("withoutEpcSql — 'does any certificate exist?'", () => {
|
||||
it("requires neither a lodged nor a historical record", () => {
|
||||
expect(render(withoutEpcSql(sql`e`))).toContain(
|
||||
"(epl.id IS NULL AND epx.id IS NULL)",
|
||||
);
|
||||
});
|
||||
|
||||
it("does not consult the blind-predicted row: a prediction is not a certificate", () => {
|
||||
// A home with only a `predicted` row has no EPC anywhere — that is precisely
|
||||
// what makes it "without an EPC", so `epp` must not appear as evidence of one.
|
||||
expect(render(withoutEpcSql(sql`e`))).not.toContain("epp");
|
||||
});
|
||||
|
||||
it("is a different question from estimatedSql", () => {
|
||||
// The bug was treating these as synonyms. An `expired` home is estimated AND
|
||||
// has an EPC, so the two predicates must disagree about it.
|
||||
expect(render(withoutEpcSql(sql`e`))).not.toEqual(render(estimatedSql(sql`e`)));
|
||||
});
|
||||
});
|
||||
|
||||
describe("isExpiredSql — 'we found a certificate and it is out of date'", () => {
|
||||
it("treats an expired-source row as expired by construction", () => {
|
||||
// ADR-0054: an `expired` row is only ever written when the conditioning
|
||||
// certificate predates 2012, so it is >10 years old by definition.
|
||||
expect(render(isExpiredSql(sql`e`))).toContain("epx.id IS NOT NULL");
|
||||
});
|
||||
|
||||
it("falls back to inspection_date when the lodged cert has no registration_date", () => {
|
||||
// Reading registration_date alone silently counted date-less lodged certs as
|
||||
// current; lodgementDateSql already coalesces the same way.
|
||||
expect(render(isExpiredSql(sql`e`))).toContain(
|
||||
"COALESCE(epl.registration_date, epl.inspection_date)",
|
||||
);
|
||||
});
|
||||
|
||||
it("excludes gap-filled legacy rows — nothing to expire without a certificate", () => {
|
||||
expect(render(isExpiredSql(sql`e`))).toContain(
|
||||
"NOT COALESCE(e.estimated, false)",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("provenanceSignalSql", () => {
|
||||
it("shows the Estimated badge for both flavours of the predicted slot", () => {
|
||||
// Without epx here, an expired-conditioned home would render with no badge at
|
||||
// all, as though its picture came off a real current certificate.
|
||||
expect(render(provenanceSignalSql)).toContain(
|
||||
"(epp.id IS NOT NULL OR epx.id IS NOT NULL) AND epl.id IS NULL",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -69,11 +69,33 @@ type Alias = ReturnType<typeof sql>;
|
|||
/** TRUE for new-approach rows. */
|
||||
export const isNewApproachSql = sql`p.updated_at >= ${NEW_APPROACH_CUTOFF}::date`;
|
||||
|
||||
/** LEFT JOINs that expose the new sources alongside the legacy table. */
|
||||
/**
|
||||
* LEFT JOINs that expose the new sources alongside the legacy table.
|
||||
*
|
||||
* `epc_property.source` has three values, not two. `lodged` is a real certificate.
|
||||
* The other two share one slot — the *predicted slot* — and are both gap-fills:
|
||||
*
|
||||
* - `predicted` — synthesised blind from the postcode cohort. Nothing is known
|
||||
* about this dwelling's own energy performance; there is no certificate for it
|
||||
* anywhere.
|
||||
* - `expired` — synthesised, but conditioned on the dwelling's own expired
|
||||
* pre-2012 certificate from the historic register (Model ADR-0054). The gov EPC
|
||||
* API only serves certificates registered since 2012, so these dwellings look
|
||||
* EPC-less to ingestion even though a real, stale certificate exists.
|
||||
*
|
||||
* That distinction is the whole point: a home with an `expired` row **has** an EPC
|
||||
* (an expired one); a home with only a `predicted` row has none at all.
|
||||
*
|
||||
* Matching `source = 'predicted'` alone would silently drop every `expired` row —
|
||||
* it would match neither `epl` nor `epp`, and the home would read as though it had
|
||||
* a valid current certificate. Address the *slot* (`epp` OR `epx`), never one member.
|
||||
* Mirrors PREDICTED_SLOT_SOURCES in the Model repo.
|
||||
*/
|
||||
export const newApproachJoins = sql`
|
||||
LEFT JOIN property_baseline_performance bp ON bp.property_id = p.id
|
||||
LEFT JOIN epc_property epl ON epl.property_id = p.id AND epl.source = 'lodged'
|
||||
LEFT JOIN epc_property epp ON epp.property_id = p.id AND epp.source = 'predicted'
|
||||
LEFT JOIN epc_property epx ON epx.property_id = p.id AND epx.source = 'expired'
|
||||
`;
|
||||
|
||||
/**
|
||||
|
|
@ -132,7 +154,7 @@ export const lodgedSapSql = sql`CASE WHEN ${isNewApproachSql} THEN (CASE WHEN ep
|
|||
export const provenanceSignalSql = sql`CASE
|
||||
WHEN ${isNewApproachSql} THEN (
|
||||
CASE
|
||||
WHEN epp.id IS NOT NULL AND epl.id IS NULL THEN 'estimated'
|
||||
WHEN (epp.id IS NOT NULL OR epx.id IS NOT NULL) AND epl.id IS NULL THEN 'estimated'
|
||||
WHEN epl.id IS NOT NULL AND bp.rebaseline_reason IS NOT NULL AND bp.rebaseline_reason <> 'none' THEN 'remodelled'
|
||||
ELSE 'none'
|
||||
END
|
||||
|
|
@ -179,18 +201,57 @@ export const lodgementDateSql = (e: Alias) =>
|
|||
sql`CASE WHEN ${isNewApproachSql} THEN COALESCE(epl.registration_date, epl.inspection_date) ELSE ${e}.lodgement_date END`;
|
||||
|
||||
/**
|
||||
* "estimated" boolean. New: the property only has a predicted EPC (no lodged
|
||||
* row). Legacy: e.estimated.
|
||||
* "estimated" — is this property's EPC picture a gap-fill rather than a real
|
||||
* certificate? True for *either* flavour of the predicted slot: a blind
|
||||
* `predicted` and a cert-conditioned `expired` are both synthesised, so both
|
||||
* carry the Estimated provenance signal.
|
||||
*
|
||||
* This answers "was it modelled?", NOT "does a certificate exist?" — an `expired`
|
||||
* property is estimated AND has an (expired) certificate. For the latter question
|
||||
* use `withoutEpcSql`. Conflating the two is what made the reporting cards wrong.
|
||||
*
|
||||
* Legacy: e.estimated.
|
||||
*/
|
||||
export const estimatedSql = (e: Alias) =>
|
||||
sql`CASE WHEN ${isNewApproachSql} THEN (epl.id IS NULL AND epp.id IS NOT NULL) ELSE COALESCE(${e}.estimated, false) END`;
|
||||
sql`CASE WHEN ${isNewApproachSql} THEN (epl.id IS NULL AND (epp.id IS NOT NULL OR epx.id IS NOT NULL)) ELSE COALESCE(${e}.estimated, false) END`;
|
||||
|
||||
/**
|
||||
* "expired" boolean. New: derived from the lodged EPC age (>10 years), since the
|
||||
* new graph stores no is_expired flag. Legacy: e.is_expired.
|
||||
* "no EPC at all" — we found neither a lodgement record nor a historical one, so
|
||||
* nothing has ever been certified for this dwelling and its picture was gap-filled
|
||||
* blind. This is the "Homes Without an EPC" card.
|
||||
*
|
||||
* Note a plain `predicted` row does not make a property "with" an EPC — a
|
||||
* prediction is not a certificate. Only `lodged` (a current cert) or `expired` (a
|
||||
* historical cert we conditioned on) count as having one. A property with no
|
||||
* epc_property rows at all also lands here, which is right: it too has no EPC.
|
||||
*
|
||||
* Legacy: the flat `estimated` flag, which meant exactly this before the register
|
||||
* had a historical tier to distinguish.
|
||||
*/
|
||||
export const withoutEpcSql = (e: Alias) =>
|
||||
sql`CASE WHEN ${isNewApproachSql} THEN (epl.id IS NULL AND epx.id IS NULL) ELSE COALESCE(${e}.estimated, false) END`;
|
||||
|
||||
/**
|
||||
* "expired" — we found a certificate, of one kind or the other, and it is out of
|
||||
* date. Two ways that happens:
|
||||
*
|
||||
* - an `expired` row exists: by construction (Model ADR-0054) it was conditioned
|
||||
* on a pre-2012 certificate, which is >10 years old by definition; or
|
||||
* - the lodged certificate is itself more than 10 years old.
|
||||
*
|
||||
* The lodged date falls back to `inspection_date` when `registration_date` is null —
|
||||
* a sizeable minority of lodged rows carry only the former, and reading
|
||||
* registration_date alone silently counted them as current. `lodgementDateSql`
|
||||
* above already coalesces the same way.
|
||||
*
|
||||
* A blindly-`predicted` property is never expired: it has no certificate to expire.
|
||||
* Legacy: e.is_expired, less the gap-filled rows (same reason).
|
||||
*/
|
||||
export const isExpiredSql = (e: Alias) =>
|
||||
sql`CASE WHEN ${isNewApproachSql} THEN (epl.registration_date IS NOT NULL AND epl.registration_date < (CURRENT_DATE - INTERVAL '10 years')) ELSE ${e}.is_expired END`;
|
||||
sql`CASE WHEN ${isNewApproachSql} THEN (
|
||||
epx.id IS NOT NULL
|
||||
OR COALESCE(epl.registration_date, epl.inspection_date) < (CURRENT_DATE - INTERVAL '10 years')
|
||||
) ELSE (${e}.is_expired AND NOT COALESCE(${e}.estimated, false)) END`;
|
||||
|
||||
/**
|
||||
* The main-fuel scalar for one epc_property alias, read from the JSONB
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue