The deterministic calculator reads sap_ventilation.extract_fans_count (which
already round-trips); the top-level epc.extract_fans_count is its mirror (the
mapper sets both from one source). Reconstruct it from the same column so
EpcPropertyData round-trips complete, dropping the allow-list exception.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
community heating fuel + CHP fraction, alt-wall is_sheltered, wall
insulation thermal conductivity, pv_diverter_present, measured cylinder
volume, AP50 air permeability — all calculator-read, all silently dropped on
save. FE columns now live; assert deep-equal round-trip and drop their
coverage-guard allow-list entries so the guard enforces reconstruction.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ADR-0036 guard only inspected EpcPropertyData's top-level fields, so a
dropped field on a NESTED object (the PV-array list, the floor heat-loss
flags) slipped straight through. Generalise it to walk every domain
dataclass reachable from EpcPropertyData and check each field is
reconstructed by a _compose/_to_* mapper or allow-listed (per-field or
whole-class), keyed by Class.field.
Surfaced 14 pre-existing nested gaps the old guard was blind to: 7 are
calculator-read with no FE column (scoring-relevant silent-drop, same class
as the PV bug — tracked follow-up), the rest dormant or awaiting FE tables.
Each is now explicit and justified.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sap_energy_source.photovoltaic_arrays has no table, so every array is
dropped on save — worth ~12 SAP points on an electrically-heated dwelling
(persist != score). Inject two ordered arrays onto a PV-free fixture.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
is_exposed_floor / is_above_partially_heated_space have no
epc_floor_dimension column, so a True flag round-trips back to the False
default and silently flips the floor's heat-loss path (persist != score).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Even after batching the data writes, the handler still wrote to the DB per
property through the orchestrator's SubTask bookkeeping: create + start +
complete each self-committed, and _cascade re-listed every sibling and re-saved
the parent on every transition — ~5 writes per property plus an O(N^2) cascade.
- TaskOrchestrator.run_subtasks: create all children in one INSERT, run each
(failures isolated per child), then persist all terminal states in one bulk
save and cascade the parent once. Children go WAITING -> terminal; the
transient IN_PROGRESS row is never written.
- SubTaskRepository.create_many / save_many (bulk INSERT / bulk fetch + update).
- _cascade short-circuits when the Task is already FAILED (terminal) — skips the
sibling roll-up entirely.
- modelling_e2e handler fans out via run_subtasks instead of per-property
create_child_subtask + run_subtask.
Per N-property batch the SubTask bookkeeping drops from ~5N writes + an O(N^2)
cascade to ~2 writes + 1 cascade.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The handler fired ~2+2N read round-trips and N+N write transactions per
SQS batch, pinning RDS CPU under ~32 concurrent containers on pool_size=1.
Reads: merge the duplicate property query and add overrides_for_many /
SolarRepository.get_many so overrides, solar, and property rows each load
in one query (2+2N -> 3).
Writes: buffer each modelled property's persistence intent in memory
(_PropertyWrite) during the loop, then flush the whole batch in one
PostgresUnitOfWork with a single commit, and run the baseline orchestrator
once for all written ids (N+N -> 2 transactions). Per-property modelling
failures stay isolated in the loop; the batch write is all-or-nothing and
retried via SQS (saves are idempotent upserts).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The modelling_e2e Lambda runs on a single-connection pool (pool_size=1,
max_overflow=0) so one invocation uses one Postgres connection. But re-hydrating
a Property through PostgresUnitOfWork resolved its Landlord Overrides through a
PropertyOverridesPostgresReader built from the unit's session *factory* — which
opens a brand-new Session per call. While the unit's own read transaction was
still open (PropertyPostgresRepository.get_many had checked out the connection),
that second Session asked the pool for a second connection, found none, and timed
out after 30s:
QueuePool limit of size 1 overflow 0 reached, connection timed out, timeout 30.00
The baseline stage (PropertyBaselineOrchestrator.run -> uow.property.get_many ->
landlord overrides) hit this on every invocation.
Read the overrides on the unit's OWN session instead. property_overrides is
committed reference data, so reading it inside the unit's transaction sees the
same rows and keeps the invocation on one connection. Extract the query/mapping
into a shared helper and add OpenSessionPropertyOverridesReader (reads on a
caller-owned, already-open session without closing it) for the unit; the
standalone PropertyOverridesPostgresReader still opens its own short session for
use outside a unit.
Regression test pins the invariant with a real pool_size=1/max_overflow=0 engine:
without the fix it reproduces the exact QueuePool timeout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The modelling_e2e Lambda held up to ~4 concurrent Postgres connections per
invocation: the read Session stayed open across the write loop (the catalogue
was queried live and overrides were read per-Property), each per-Property Unit
of Work opened a second, and the TaskOrchestrator ran on its own NullPool
engine — so the pool needed pool_size=2 + max_overflow=1 just for the modelling
work. Under 32 concurrent containers that approached RDS max_connections.
Restructure the handler to read everything up front — overrides, Scenario, an
in-memory catalogue snapshot, and stored Solar — through one short-lived read
Session, close it, then write each Property in a sequential Unit of Work. The
read and write Sessions no longer overlap, so the engine drops to pool_size=1,
max_overflow=0. Fold the orchestrator onto the same pooled engine: its repos
commit on every save, releasing the connection between bookkeeping calls, so it
holds none during the work. One invocation now uses one connection at a time.
The catalogue becomes a per-invocation snapshot (MaterialSnapshotRepository),
mirroring ProductPostgresRepository.get exactly — same drift mapping, lowest-id
pick, and errors — but priced after the Session closes. Transaction isolation
is preserved: per-Property writes and orchestrator bookkeeping keep their own
independent transactions, just drawn sequentially from a single connection.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fail if any EpcPropertyData field is neither reconstructed by _compose nor on a
documented allow-list, turning latent persistence gaps into explicit decisions
(would have caught the conservatory and roof-window drops). ADR-0036.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Persist SapConservatory as five nullable conservatory_* columns on epc_property
(1:1 with the dwelling) and rebuild it in _compose, so the §6.1 fold survives
save -> reload -> score. Without this the scored (re-hydrated) EPC silently
dropped the conservatory (persist != score) — a latent gap shared with the
21.0.1 path. Adds a deep-equality round-trip test. ADR-0036.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
10 modelling_e2e properties failed with "unmapped SAP code in fuel_code: 10":
the billing layer (`sap_code_to_fuel`) had no carrier for Table-32 code 10
(dual fuel, mineral + wood) and raised rather than guess one.
SAP 10.2 treats dual fuel as its OWN fuel (its own Table-12 factors), so model
it as its own billing carrier rather than collapsing onto wood or coal:
- New `Fuel.DUAL_FUEL_MINERAL_AND_WOOD`.
- `_CODE_TO_FUEL[10]` -> that carrier.
- Fuel Rates snapshot prices it at 7.69 p/kWh — the midpoint of the COAL proxy
(7.13) and WOOD_LOGS (8.25). This mirrors SAP's own construction: Table-32
dual fuel (3.99) ~= midpoint of house coal (3.67) and wood logs (4.23).
Marked `derived` with a documented _note/_gap/_assumption (like the COAL and
HEAT_NETWORK proxies), since there is no retail blend price.
A dedicated carrier + rate (vs a one-line map to an existing carrier) keeps the
fuel identity faithful to SAP and avoids mispricing dual fuel as pure wood/coal.
Tests: code 10 -> DUAL_FUEL carrier; snapshot prices it at 7.69; grid-export
codes (36/60) still raise (the genuine no-carrier case).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PostgresUnitOfWork built its PropertyPostgresRepository without an overrides
reader, so a Property re-hydrated through the unit silently dropped its
Landlord Overrides (ADR-0032). The Baseline orchestrator runs through the UoW,
so it scored the bare lodged EPC while the Plan modelled the override-folded
Effective EPC — the two diverged (e.g. baseline effective 71/C vs plan
baseline 62/D), producing "already at band C yet recommends reaching C".
Wire PropertyOverridesPostgresReader into the unit's property repo (uow-
independent committed reference data, read via the same session factory) so
every re-hydration folds overrides, matching the live modelling path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two reconciliations to make the modelling_e2e Lambda handler production-ready.
1. Price through the off-catalogue overlay, drop the workarounds
The handler priced through a plain ProductPostgresRepository and excluded
secondary_heating_removal / system_tune_up / system_tune_up_zoned to dodge
ProductNotFound (and a poisoning pgEnum DataError). Those measures are now
priced by catalogue_with_off_catalogue_overrides (already used by the e2e
runner and PostgresUnitOfWork), so the exclusions are removed and ALL measure
types are considered. This also fixes gas-boiler / single-glazed properties,
which Dan's handler never excluded and so still crashed (the standard
system_tune_up option is built unconditionally — the considered-measures
exclusion never actually gated it).
2. Broaden the EPC-Prediction cohort to nearby real postcodes (ADR-0031)
A property with no lodged EPC and no same-type comparable in its own postcode
(e.g. the only flat among houses) used to gate out and fail the subtask. The
gov EPC API cannot search by radius/outcode, so we resolve the real unit
postcodes physically nearest the target via postcodes.io (keyless; already a
trusted in-repo dependency) and walk them nearest-first until enough same-type
comparables surface. New PostcodesIoClient (transient-failure retry with
exponential backoff, soft-failing to the seed so broadening never breaks
prediction) and EpcComparablePropertiesRepository.candidates_near. Wired into
the handler and e2e runner; broadening is lazy (only on gate-out) and memoised
per (postcode, property_type).
Validated live: property 728476 (gas boiler) prices system_tune_up at GBP295;
property 718580 (lone flat in BR6 6BS) now predicts via nearby BR6 postcodes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make run_modelling_e2e the single script that does everything for a portfolio,
so the 291-property run needs one invocation with per-property recovery (no
all-or-nothing chunking):
- On --persist, a lodged-EPC Property now also gets its Baseline Performance
row written via PropertyBaselineOrchestrator (per Property, so one bad cert
does not abort the batch). Predicted (EPC-less) Properties have no lodged
figures, so they get a Plan but no baseline row.
- The run CSV gains api_sap (register) vs baseline_sap (calculator) + sap_delta,
so calculator-vs-API divergence is reviewable per property.
Fill the off-catalogue overlay for the measures the live material catalogue
cannot price, so they stop crashing the run:
- double_glazing (£550/window) and secondary_glazing (£400/window): priced
per window (the generator multiplies by single-glazed window count, matching
the legacy window_glazing). Grounded in 2025/26 UK installed costs; per-window
is the right unit for windows (fixed per-unit install dominates) — per-m2 fits
walls/floors, not glazing.
- gas_boiler_upgrade / system_tune_up / system_tune_up_zoned: these are priced
off the heating rate sheet (Products()), with get() reading the catalogue only
for an id — so the overlay entry exists to satisfy that lookup (material_id
stays None, as with ASHP); the rate sheet remains authoritative.
Validated on a 12-property sample (incl. a secondary-glazing case and a
SAP-Schema-16.2 cert): 12/12 baseline rows + plans, 0 errors.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Caught live writing property_overrides on portfolio 796: the Python
override_component SAEnum lagged the DB enum, so reading a new-component row
back threw LookupError. Guard it with a consistency test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The FE-owned `material.type` pgEnum cannot carry `secondary_heating_removal`,
so pricing it through the DB catalogue raises a DataError that poisons the
session — the modelling pipeline crashed on any property with a lodged
secondary heater unless the measure was excluded on the Scenario.
Realise the `ProductRepository` docstring's intent (DB catalogue today, a JSON
file for costs the ETL does not yet supply, behind the same port): add a
`CompositeProductRepository` that resolves an override source first, then the
catalogue. Checking the override first keeps that Measure Type away from the DB
entirely; every other type misses the override and falls through unchanged.
- off_catalogue_costs.json prices it at £270 flat per-dwelling — the legacy
`Costs.heater_removal` ported to the new flat model (ADR-0028):
(£25 + £200 baseline) x 1.2 VAT, for the single fixed secondary a cert lodges.
Contingency (0.25) is joined from config, not the file.
- Wire the composite into PostgresUnitOfWork.product and run_modelling_e2e, so
the first-run pipeline and the local runner both honour the overlay.
- Integration test: drop the unrealistic seeded secondary_heating_removal DB
rows (the pgEnum can't hold the type) and assert it is JSON-sourced
(material_id is None, cost £270) end-to-end through a real Unit of Work.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds whole-dwelling property_type/built_form to EpcSimulation (folded by
apply_simulations) and maps those override components. property_type drives
party-wall heat loss + ASHP/solar/wall eligibility, so a landlord correction now
moves both the SAP calc and the measure menu; built_form has no calculator
consumer today (feeds the ML transform). Written as the landlord text value
(park-home check is text-only). Refines ADR-0032 dec-4.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extends WallType coverage to timber/stone/system-built/cob/park-home/curtain and
adds RoofType "Pitched, N mm loft insulation" -> roof_insulation_thickness. The
"(assumed) insulated"/"partial" wall states stay deferred (ambiguous code, needs
Elmhurst validation per ADR-0032); property_type/built_form carry no SAP weight.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The new pipeline left no per-Property record of a run (the old engine set
property.has_recommendations and populated property_details_epc). Restore the
marker: PropertyRepository.mark_modelled sets has_recommendations (true when the
Plan carries measures, mirroring the old engine) and bumps updated_at, so a
first-run under the new process is identifiable as updated_at >= 2026-06-01.
ModellingOrchestrator marks each Property after its Scenarios (true if any
Scenario yielded a measure); run_modelling_e2e's --persist path marks it too
(its compute runs on in-memory fakes, so the DB UoW sets it directly). Adds the
has_recommendations/updated_at columns to the PropertyRow mirror.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The measures a run considers should come from the Scenario, not a CLI flag.
The live scenario table persists exclusions only (no inclusions column), as a
Postgres text-array of exact MeasureType values.
- Scenario gains `exclusions: frozenset[MeasureType]` + `considered_measures()`
(all measures minus the excluded ones, or None when none are excluded).
- ScenarioModel.to_domain parses the `{a,b,c}` exclusions array into
MeasureTypes, raising on a token that is not an exact MeasureType value
(no high-level category expansion), per the strict-enum convention.
- ModellingOrchestrator._plan_for derives the allowlist from the Scenario's
exclusions, combined (intersection) with any explicit considered_measures
override via the new `combine_considered_measures`.
- run_modelling_e2e sources the allowlist from the Scenario; --measures /
--exclude-measures become optional overlays (e.g. the technical
secondary_heating_removal exclusion the catalogue cannot yet stock).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two review points from @dancafc:
1) Rename the `Comparable` dataclass → `ComparableProperty` (it models one
comparable *property*; the collection stays `ComparableProperties`). Applied
across domain, repositories, orchestration, harness, scripts, and tests with a
word-boundary rename so `ComparableProperties` is untouched.
2) Move `PredictionTarget` out of comparable_properties.py into prediction_target.py
(where `PredictionTargetAttributes` + `build_prediction_target` already live).
comparable_properties.py now imports it; no import cycle (prediction_target no
longer depends on comparable_properties). Importers updated.
92 tests pass across the touched suites; pyright strict clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a `source` discriminator (lodged | predicted) to the EPC store so a Property
holds a lodged EPC and a predicted one (EPC Prediction gap-fill) at once
(ADR-0031). EpcRepository.save gains source="lodged"; idempotent delete is now
per-source (a predicted save no longer wipes lodged, and vice versa);
get_for_property/get_for_properties filter lodged; new get_predicted_for_property
/ get_predicted_for_properties read predicted. PropertyPostgresRepository.get +
get_many hydrate Property.predicted_epc, so the predicted picture reaches the
modelling read (both load via get_many). FakeEpcRepo mirrors the dual slot.
EpcPropertyModel gains `source` (default "lodged"); the test DB builds from the
SQLModel mirror so this is exercised without the prod migration. The matching
Drizzle change (column + per-(property_id,source) uniqueness) is the team's to
action before merge — docs/MIGRATION_NOTE_predicted_epc_source.md.
3 store tests (coexist, idempotent predicted re-save leaves lodged, lodged-only
has no predicted) + property-repo wiring; 85 pass across affected suites; new
code pyright-clean (2 pre-existing wwhrs errors in epc_property_table untouched).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Build the cohort IO port ADR-0029 deferred (ADR-0031 slice-5b):
`ComparablePropertiesRepository.candidates_for(postcode) -> list[Comparable]`,
with an EPC-API + geospatial adapter that lists the postcode's lodged certs
(search_by_postcode), fetches + maps each (get_by_certificate_number), and
resolves their UPRNs to coordinates in ONE batched read. Register metadata the
cert doesn't carry (address, registration date) is threaded off the search row;
a UPRN-less or unparseable-date cert is kept, just uncoordinated / unweighted.
The domain select_comparables then filters these candidates into the cohort.
Thin CohortEpcClient / CohortGeospatial Protocols keep the adapter testable
against fakes; EpcClientService + GeospatialS3Repository satisfy them
structurally (no changes). 3 tests; pyright strict clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
