FIELD_MAP and upsert_deal already map planning_authority, designated_area,
article_pd_rights, listed_building, design_constraints, planning_comments,
planning_status, and planning_suggested_approach, but from_deal_id_get_info
never requested them from the HubSpot API. Since HubSpot only returns
explicitly requested properties, these always came back None regardless
of the deal's actual data, so refreshes never populated them.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Foundation for PRD #1555: runs each PAS Hub site-note PDF through the
extractor -> EpcPropertyData -> Sap10Calculator and gauges the computed
SAP against pashub's own SAP-10.2 `pre_sap` (from hubspot_deal_data).
- test_pashub_sap_accuracy.py: hybrid gate. Per-fixture "must compute"
(xfail on the known in-progress mapper gaps MissingMainFuelType /
UnmappedSapCode / UnmappedPasHubLabel) + aggregate within-0.5 ratchet
floor, mirroring test_sap_accuracy_corpus.py.
- 205 image-stripped site-note PDFs + manifest.json. Images stripped so
the repo footprint stays ~52MB while the text layer the extractor reads
is byte-identical.
- build_pashub_accuracy_fixtures.py: provenance/rebuild from S3 +
hubspot_deal_data.
All 206 currently xfail on the known heating-string mapper gaps; each fix
(#1556-1568) flips its fixtures to computing and ratchets the floor.
Refs #1555#1568
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`from_site_notes` copied the raw `party_wall_construction_type` survey
label onto the building part; the calculator read `None` and applied the
U=0.25 house default to every party wall — phantom heat loss on the
~165/205 solid party walls that RdSAP 10 Table 15 rates U=0.0.
Add `_pashub_party_wall_construction_int` (mirroring `_pashub_main_fuel_code`)
mapping the six surveyed labels to the SAP10 codes `u_party_wall` consumes,
strict-raising `UnmappedPasHubLabel` on an unknown label, and wire it into
both site-note building-part mappers. On the Guinness GMCA cohort this
halves the systematic SAP under-rate (mean signed -1.58 -> -0.85).
Refs #1559
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_map_sap_heating resolves the raw emitter label (e.g. Radiators) to its
SAP10 emitter code via _pashub_heat_emitter_code, reusing the Elmhurst
emitter map. Blank passes through; an unrecognised label strict-raises
UnmappedPasHubLabel at the mapper boundary (ADR-0015) instead of
resurfacing as the calculator's UnmappedSapCode: heat_emitter_type.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sets HISTORIC_EPC_S3_ROOT, the env var the handler reads to build the historic-EPC
reader. With it set, an EPC-less Property whose expired pre-2012 certificate is in
the backup has its prediction conditioned on that certificate and persisted as
source="expired" rather than "predicted" — which is what lets reporting tell "no EPC
at all" apart from "only an expired one".
The bucket comes from the shared remote state rather than a rebuilt string:
`retrofit_sap_data_bucket_name` is — despite the name — the retrofit-data-<stage>
bucket (shared/main.tf:167), and engine and fast-api already read it from that output.
modelling_e2e was the outlier hardcoding "retrofit-data-${var.stage}".
No IAM change: modelling_e2e_s3_read already grants GetObject + ListBucket across the
whole retrofit-data-<stage> bucket, which covers historical_epc/.
Note this makes 'expired' rows start appearing once deployed. assessment-model's
epcSources.ts joins only 'lodged' and 'predicted', so until it addresses the predicted
slot — source IN ('predicted','expired') — an 'expired' row matches neither and the
home drops out of both the "Homes Without an EPC" and "Expired EPCs" cards.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_lambda_image_copies_full_import_closure caught this: importing the Historic
EPC S3 repo drags datatypes/epc/domain/historic_epc_matching.py into the handler's
init-time closure, and that reaches back into the legacy address matcher —
backend/address2UPRN/scoring.py and utils/pandas_utils.py. The image COPYed
neither, so the Lambda would have died at cold start with Runtime.ImportModuleError.
Copied file-by-file rather than `COPY backend/ backend/`: backend/ is the whole
legacy engine and the closure needs only these seven files. Their third-party
deps (pandas, requests) are already in requirements.txt, so no new pip installs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ADR-0054 gives a prediction conditioned by an expired Historic EPC the source
"expired", so reporting can tell "no EPC at all" apart from "only an expired
one". It was implemented in IngestionOrchestrator — but the pipeline that
actually runs is modelling_e2e, which predicts through its own _predict_epc and
never adopted it. Result: zero `expired` rows have ever been written, and
`epc_property.source` holds only 'lodged' and 'predicted'.
Two things kept the flavour unreachable here, and both had to go:
- _predict_epc never looked at the historic backup at all, so no prediction
was ever conditioned.
- _flush_writes passed the literal source="predicted", and _PropertyWrite had
no field to carry a flavour, so one would have been dropped before the write
even if computed.
_predict_epc now mirrors IngestionOrchestrator._predict: the expired cert's
stable attributes fill the gaps Landlord Overrides left (overrides still win
where both speak) and condition the cohort, and it returns the source alongside
the EPC. _PropertyWrite carries it; _flush_writes persists it. save_batch
already groups deletes by source family, so a mixed predicted/expired batch
clears the shared slot correctly.
Ships dark: the reader is built only when HISTORIC_EPC_S3_ROOT is set, so
without it every prediction stays plain "predicted", exactly as today.
DEFAULT_S3_ROOT names the dev bucket, so defaulting to it would have a prod
lambda silently reading dev data — Terraform sets the var per environment.
Note this also rescues properties that currently fail to model outright: an
expired cert can supply the property_type no override resolved, where today
that raises UnresolvedPropertyTypeError.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
inspection_date is non-null on epc_property, so it is a better winner for
a shared-UPRN collision than insertion order: the latest-surveyed lodged
row wins. id DESC stays as the deterministic secondary, so a same-UPRN,
same-inspection_date tie falls to the highest id.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An epc_property row can carry a null property_id (FE/historic ingestion
persists EPC rows never linked to a property row), so a property_id-keyed
read silently misses them. UPRN is the durable key both the property row
and the epc_property row share. Repoint the lodged reads (get_for_property
+ get_for_properties) onto UPRN; the predicted reads stay on property_id
because a predicted EPC deep-copies a neighbour's UPRN, so its UPRN column
is never the property's own.
UPRN is not unique on epc_property, so the read tie-break is now load-
bearing (it was dormant under property_id, where the write path guarantees
one lodged row per property): the most recently ingested (highest-id) row
wins. Write path stays keyed on (property_id, source).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Locks the gate edges — a lodged pitched loft below 270mm is recommended, a
loft at 270mm is left alone, and the 'Nmm+' form ('300mm+') parses to its
number and stays ineligible. These passed on arrival (they fell out of the
numeric-parse gate added in the previous commit); pinned as regression guards.
ADR-0063.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A lodged numeric loft depth below the 270mm building-regs compliance gate is
now eligible (topped up to the 300mm install depth), regardless of whether the
roof type is lodged, since a measured depth is a positive statement of a real
loft. Sentinels keep their ADR-0047 resolution. Fixes property 724702 (50mm
loft) which previously returned no recommendation. ADR-0063.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The roof generator offered loft insulation only when genuinely uninsulated
(ADR-0021), so a loft with a shallow lodged depth (e.g. 724702's 50mm) got
nothing and never entered the optimiser pool. Grilled with Khalim: the loft
branch becomes eligible when the lodged depth is a known numeric value below
the 270mm building-regs compliance gate, topped up to the 300mm install depth.
Numeric-lodged-depths only; sentinels keep their ADR-0047 resolution. Loft
branch only. CONTEXT 'Roof Insulation Eligibility' updated to match.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Properties 741843/741872 (uprns 100060714155/100021944594) failed
modelling_e2e subtasks 6ac7841a-9338-4efe-97e5-7e0d19b3055d and
c5450f03-5b0d-4068-a3b3-d1470bc0af57 with a bare StopIteration: their
SAP-Schema-15.0 (2011-era LIG-lodged) certs identify the main dwelling's
building part as "Main building" rather than "Main Dwelling", so
from_api_string fell to OTHER and left the property with no MAIN part —
crashing wall_recommendation.py's unguarded next(...).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Thread the UPRN the PashubService has already resolved for a job into the
PasHub site-notes mapper so the EpcPropertyData aggregate is born with its
uprn set, and the existing save_epc_property_data path persists it.
- EpcPropertyDataMapper.from_site_notes gains uprn: Optional[int] = None and
sets it unconditionally (site notes never carry a UPRN natively).
- parse_site_notes_pdf / _parse_pashub forward the uprn; the Elmhurst branch
is untouched.
- PashubService coerces uprn str -> int in one place, carries it on the
internal upload record, and passes it into parse_site_notes_pdf.
- No new lookups; jobs with no known UPRN still persist null, unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The prior commit derived a Python bool (`cylinder_size != 1`), but
`from_rdsap_schema_17_1` reads the field via `schema.has_hot_water_cylinder
== "true"` — every real 15.0/16.x fixture lodges the lowercase string
"true"/"false", not a JSON boolean. A bare bool compares False against
that string check either way, so the derivation silently mapped every
cylinder-present cert to has_hot_water_cylinder=False too.
Caught by the requested true-branch test (mutating sap_16_2.json, which
lodges cylinder_size=2/has_hot_water_cylinder="true", to omit the field)
— it failed under the original bool-typed fix. Now emits "true"/"false"
strings to match the lodged convention.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Task b9fcd354-2a33-4fc4-a64e-388c060a055c (portfolio 824 / scenario 1278)
also failed property_id 749229 (UPRN 100010349400) with "RdSapSchema17_1:
missing required field 'has_hot_water_cylinder'". Cert
9568-3034-6211-6089-5960 (SAP-Schema-15.0) lodges `sap_heating.cylinder_size`
but omits the separate top-level `has_hot_water_cylinder` boolean
RdSapSchema17_1 also requires.
Unlike `multiple_glazed_proportion` (deliberately left un-defaulted a few
lines above — a prior guessed default regressed the accuracy gate),
`cylinder_size` isn't a proxy needing inference: RdSAP 10 Table 28 defines
code 1 as "no cylinder" — literally the same fact `has_hot_water_cylinder`
encodes. Confirmed 1:1 across every real 15.0/16.x fixture that lodges both
fields (cylinder_size == 1 <-> false, every other code <-> true).
Added to the shared `_normalize_sap_schema_16_x`, so it covers the whole
15.0/16.0/16.1/16.2/16.3 reduced-field family, not just 15.0.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
