Review findings on PR #1526:
- tests/domain/modelling/_optimiser_fixtures.py is the one home for the
overlay constants, the ScoredOption builder, the additive per-kind
StubScorer and the forced ventilation dependency; test_optimiser.py and
test_optimiser_fabric_first.py had byte-identical copies of each
(and _StubScorer / _VentStubScorer fold into one parameterised stub).
- Fixture worlds are domain-plausible per team convention: the fabric-vs-
heating contrast is a £12,000 EWI against a £3,200 gas boiler rather
than a £500 heat pump undercutting a £1,000 cavity wall; heating
overlays carry real identities (SAP Table 4a code 104 for the boiler,
a PCDF index for the heat pump) instead of code 201 doubling as both;
whole-dwelling double glazing is £3,500, not £500.
- Dead knobs removed: the unused _ROOF_OVERLAY, the always-zero roof
gain, the duplicate _BOILER_OVERLAY, and the nested conditional
expressions in the interaction stubs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback: cavity and internal wall insulation are competing options,
so a package selecting both read as nonsense even in a synthetic fixture.
The behaviour under test (a dependency triggered in both phases injects
once) now uses an airtightness pair that genuinely coexists — cavity wall
in phase 1, double glazing re-entering in phase 2 — with the same numbers.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documents behaviour already delivered: with no SAP target both phases run
max-gain, so the envelope still consumes the budget first. Test passed on
arrival.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documents behaviour already delivered: an empty phase 1 meets no target, so
phase 2 optimises every group — identical to a plain run. Test passed on
arrival.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documents behaviour already delivered: phase 2 optimises every group phase 1
did not consume, with signals re-scored against the fabric-applied dwelling,
so glazing skipped on raw-baseline merit re-enters when it closes the target.
Test passed on arrival.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documents behaviour already delivered by the phase-2 structure: the leftover
budget is the envelope minus the committed fabric cost, so a heating system
the plain optimiser would buy can be priced out. Test passed on arrival.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
End-to-end pin at the generator seam (recommend_solar) — passed on first
run since option construction is unchanged by design (ADR-0058).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins that the unchanged cap regime (min of 0.7×Google and the ADR-0038
budget) flows into ADR-0058 derivation — no red phase; the tracer
implementation passed the already-min'd cap through.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins the ADR-0058 install floor delivered with the tracer slice (rung_max
< 2 returns empty) — no red phase.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins behavior delivered with the tracer slice (derivation starts from the
north-dropped remainder) — no red phase; the mutation is discriminating
(a kept north panel would win fill-by-generation and shift the yields).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sizing selects the aroTHERM plus rung nearest PSR ~0.8 (0.8 x design heat
loss) — the Appendix-N efficiency peak, reproducing the pump a real
installer fits — rather than the MCS PSR>=1.0 capacity target, which
oversizes. Validated against the relodged Elmhurst ASHP cert (delta 0
held at the 5 kW Vaillant); the two self-snapshot pins re-pin to their
correctly-sized larger pumps, and the orchestrator/harness thread the
calculator's design heat loss so production sizes to the dwelling. Manual
SapResult test stubs carry the new design_heat_loss_kw field.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_ashp_option now selects the aroTHERM plus PCDB record matching the
threaded design heat loss (floor-area proxy fallback), so the overlay's
efficiency anchor is sized to the dwelling instead of a fixed 5 kW unit.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The high-heat-retention storage-heater (HHRSH) recommendation was computed
as LOWERING SAP and wrongly rejected by the Optimiser, leaving electric-
room-heater flats/maisonettes stuck at band E with no sensible plan.
Root cause: `_HHR_STORAGE_OVERLAY` sets `sap_main_heating_code=409` (HHR
storage) + `meter_type="Dual"` but left `main_heating_category` at the
pre-existing value (10 = electric room heaters). The SAP 10.2 Table 12a
space-heating high-rate-fraction resolver classifies by category, so it
misread the off-peak storage heating as direct-acting electric (1.00
high-rate) and priced 100% at the 7-hour peak rate (15.29p) instead of the
storage off-peak split (0.00, ~5.5p).
Verified against accredited Elmhurst RdSAP-10 on UPRN 100020942571
(8 Booth Close, SE28 8BW): engine HHRSH SAP 48 (peak-priced, cost £1087)
vs Elmhurst 71 (cost ~£611); with the fix the engine gives 73 / £576.
Two-part fix (defence in depth, so no future overlay/mapper can
reintroduce it):
- `_HHR_STORAGE_OVERLAY` now sets `main_heating_category=7`, keeping the
overlaid EPC internally consistent.
- `_table_12a_system_for_main` now classifies electric storage heaters by
their Table 4a SAP code (401-409) as well as `category==7`, checked
before the room-heater branch, so a specific storage code is
authoritative over a stale/inconsistent category.
Tests: new regression `test_storage_heater_classified_by_sap_code_over_stale_category`;
updated the HHRSH overlay pin. Full accuracy corpus still passes (no regressions).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Gov-API certs that lodge a gas boiler as main_heating_category=2 + a PCDB
main_heating_index_number but omit sap_main_heating_code were carried with
a null code, so the heating generators (which gated on sap_main_heating_code
alone) never evaluated them for a boiler upgrade or a system tune-up — they
silently got nothing (£0 works, plateau at D). The SAP calculator already
handles this shape (cert_to_inputs._is_wet_boiler_main), so the calculator
modelled these as wet boilers while the recommenders ignored them.
Mirror the calculator's classification in the generator:
- _is_wet_boiler / _is_electric_boiler fall back, when no SAP code is
lodged, to a PCDB Table 105 boiler index or main_heating_category in
{1,2} (heat pumps excluded); electric boilers stay excluded by the code
range when lodged, else by an electricity main fuel.
- _already_condensing reads the boiler's declared PCDB winter efficiency
when there is no Table 4b code, so a modern condensing boiler lodged this
way still isn't offered a pointless like-for-like swap (gets a tune-up).
Verified read-only on portfolio 814 / scenario 1271: pids 742304, 742306,
742322, 742325, 742336 now get a gas_boiler_upgrade (two cross D->C on the
boiler alone) where they had £0 works; predicted properties whose heating
donor is a PCDB-shape boiler (e.g. 742095/742102/742105) also recover a
tune-up. Fixes the A#2 slice of #1388 (and unblocks 742302/742367).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Gate PV generation/credit in cert_to_inputs on gov-API pv_connection:
credit only when ==2 ('connected'); ==1 ('present but not connected to the
dwelling's meter') contributes zero to the dwelling's cost/CO2/PE per
RdSAP 10 §11.1 / SAP 10.2 Appendix M. Non-int (None / site-notes str) keeps
the credit-if-array behaviour, so the Elmhurst/Summary + synthetic paths are
unchanged (no regression).
Corpus: all 5 pv_connection=1 PV certs move inside ±0.5 (e.g. 100051118081
+6.5→+0.5); MAE 0.760→0.740, within-0.5 73.8→74.3%, no regression
(pv_connection=2 certs keep their credit).
Also corrects a now-load-bearing latent bug: the solar-recommendation
overlay tagged recommended arrays pv_connection=1 ('not connected') — which
the new gate would zero. A new install connects to the dwelling's meter, so
it must be 2; pinned by the overlay test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three corrections found by re-running property 742003 end-to-end:
- roofSegmentStats are POSITIONAL — real responses omit the segmentIndex field
the fixture happened to carry; key the centre/area lookup by array position.
- Base the cap on ground_floor_area (the footprint the roof covers), not the
greatest per-storey area; roof_area is the fallback.
- Clamp the basis by total_floor_area: predicted EPCs borrow the structural
template's geometry (742003: a 118.62 m² MAIN ground floor) decoupled from
the predicted 55 m² (ADR-0029), so without the clamp the cap reads the
template's larger footprint.
Result: 742003 plan A/92.4 (16 kWp) -> C/74.4 (6.4 kWp). 29 solar tests +
orchestration threading + products green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
select_conservative_configs must accept the dwelling's roof area and cap panels
to its usable roof (ADR-0038) — bounding a 55m² dwelling to ~16 panels under
Google footprint conflation, while staying a no-op on correctly-matched homes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Dwelling-Roof Cap (ADR-0038) sizes by usable roof area and ranks segments
by distance from the dwelling, so the projection must carry each panel's
footprint and each segment's centre + area (from roofSegmentStats).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 modelling_e2e properties failed with KeyError: 'maxArrayPanelsCount'.
Google returns a `solarPotential` block with no array-level sizing fields
(`maxArrayPanelsCount` / `panelCapacityWatts`) for buildings with no usable
solar estimate. `SolarPotential.from_building_insights` hard-indexed those keys
and crashed the whole property.
Fix: the projection now returns Optional and yields None when those fields are
absent — the established "no solar potential" outcome (the orchestrator and
recommendation path already type it Optional and skip solar on None). Existing
callers (`_solar_potential_for`, harness) already assign to Optional.
Regression test + `assert is not None` narrowing on the valid-fixture tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
