Ports the §6.1 conservatory split (declare the four glazed fields; exclude the
glazed BP from the fabric loop; carry it as sap_conservatory) to the remaining
gov mappers that lacked it. Fixes the modelling_e2e construction_age_band
NOT-NULL failures: 5 type-4 conservatory certs on RdSAP-Schema-20.0.0 (e.g.
property 725213) mapped the conservatory to a null-age fabric part that crashed
on persist. 17.0/17.1/18.0 ported preventively (no-op for non-conservatory
certs). 21.0.0 deferred — it never adopted the ADR-0027 all-Optional
SapBuildingPart, so a sparse conservatory part needs that broader change first.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review (Khalim): lift the full-SAP detection out of the SAP-Schema-16.x
branch into a single top-level gate in from_api_response, and key it on the
gov-API's own assessment_type declaration rather than the structural
sap_opening_types proxy.
- _is_full_sap_assessment(data): assessment_type == "SAP" — the authoritative
SAP-vs-RdSAP classification. Verified to separate the entire fixture corpus:
every full-SAP schema (SAP-Schema-17.x/18.x + the broken LIG 16.x) is "SAP";
every reduced cert (RdSAP-Schema-* and reduced SAP-Schema-16.x, incl.
sap_16_0.json) is "RdSAP". data_type / sap_opening_types agree but are derived
shape artifacts; assessment_type is the meaning.
- A cert that is full-SAP by assessment_type but whose schema_type LABEL is not a
recognised full-SAP schema is a *broken schema type* (label disagrees with the
assessment). _record_broken_schema_type logs it — visible, not silently
rerouted — so unreliable labels surface and coverage grows as new mislabels
appear. Generalises beyond 16.x to any future mislabel.
- _from_full_sap maps it via the full-SAP 17.1 mapper (real measured openings,
no defaulting; only `tenure` defaulted). Correctly-labelled full-SAP certs keep
their dedicated branches (one-mapper-per-schema convention); reduced certs are
unchanged.
Tests: broken cert routed AND recorded; correctly-labelled full-SAP not recorded.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A full-SAP cert (assessment_type=SAP, the as-designed LIG-* new-builds) lodged
under a SAP-Schema-16.x version failed to map with:
ValueError: RdSapSchema17_1: missing required field 'door_count'
— crashing the property's modelling_e2e subtask. 89 distinct properties in the
2026-06-24 run hit this; ~22 of every 30 sampled are this full-SAP shape.
Root cause: from_api_response dispatches on the schema_type STRING only, and the
SAP-Schema-16.x branch assumed a single shape — "reduced-field (RdSAP-shaped)" —
sending every 16.x cert through _normalize_sap_schema_16_x -> RdSapSchema17_1.
But the SAP-Schema-16.x name covers two structurally different certs:
* reduced RdSAP (assessment_type=RdSAP): top-level door_count / glazed_area
band / construction-code building parts.
* full SAP (assessment_type=SAP): measured shape — sap_opening_types +
structured sap_building_parts carrying measured U-values and door/window
OPENINGS, with NO top-level door_count (the door is an opening). These omit
the reduced-only count fields, so the reduced normaliser failed loud on the
first one it checks (door_count) — and 14 others behind it.
Force-fitting a full-SAP cert to the reduced RdSAP schema was the bug: the data
was never missing, it was being validated against the wrong schema.
Fix: discriminate on shape (_is_full_sap_cert: assessment_type=SAP AND
sap_opening_types present) and route full-SAP 16.x certs to the existing
full-SAP 17.1 mapper, which reads the real measured openings (the lodged
0.9x2.1m door -> door_count 1) — no reduced-field defaulting. The only field the
16.x full-SAP shape omits that SapSchema17_1 needs is `tenure` (register
metadata, no SAP effect), defaulted. Reduced 16.x certs (assessment_type=RdSAP,
no opening types) are untouched — all pinned reduced 16.x fixtures stay on the
RdSAP path.
Regression test pins a real full-SAP-16.0 cert (0293-...-8795, lodged 83) mapping
via the full-SAP path + the reduced 16.2 cert staying on the RdSAP path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror the merged 21.0.1 fix (d501535c): declare the four glazed conservatory
fields on the 19.0 SapBuildingPart so from_dict stops dropping them, exclude the
glazed BP from the fabric loop, and carry it as EpcPropertyData.sap_conservatory
(§6.1). Fixes cert 718138 (conservatory_type=4) mis-scoring as a fabric part and
failing to persist on the NOT-NULL construction_age_band.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5 modelling_e2e cohort certs (full-SAP 19.1.0) were skipped with
"unmapped API sap_opening_type code: 6". Code 6 is "Ext Rooflight"; SAP 10.2
treats roof windows and rooflights as the same inclined-glazing family.
Fix: add 6 to the known opening-type taxonomy and route it onto the roof-window
path (`sap_roof_windows`) alongside code 5, via a `_SAP_ROOF_WINDOW_TYPES`
{5, 6} set. Genuinely-unknown codes (e.g. 99) still raise.
Cert 9878-3908-6309-6714-8200 now maps + calculates (sap 81). Regression test:
a type-6 opening maps onto sap_roof_windows.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A SAP-Schema-17.0 full-SAP cohort cert (8265-7433-3220-9736-7902) omits the
top-level `has_hot_water_cylinder` and lodges it only under `sap_heating`. The
required top-level field on SapSchema17_1 made `from_dict` raise and skip the
cert.
Fix: make the top-level field Optional, and have the full-SAP mapper
(`from_sap_schema_17_1`, also used by 17.0/18.0.0 via delegation) fall back to
`sap_heating.has_hot_water_cylinder`. RdSAP mappers are unchanged (they keep the
top-level required field; their SapHeating has no such attribute).
Cert now maps + calculates (sap 76). Regression tests for the nested true/false
fallback.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A SAP-16.x cohort cert (9258-4062-7265-2844-7954) lodges a bare int building-
part identifier (the second part as `1` after "Main Dwelling").
`BuildingPartIdentifier.from_api_string` regex-matched it assuming a string and
raised "TypeError: expected string or bytes-like object, got 'int'", failing the
whole property.
Fix: guard the match on `isinstance(api_identifier, str)` so a non-string
identifier falls to OTHER, matching the documented "anything unrecognised ->
OTHER" contract. The baseline SAP fabric sums all building parts regardless of
identifier, so OTHER is SAP-neutral; the identifier only labels parts for
measure targeting. Fixes every mapper (all route through from_api_string).
Cert now maps + calculates (sap 63). Regression test added.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up (Khalim): the first pass made far more optional than needed —
notably the whole SapBuildingPart block — and a buggy 21.0.0↔21.0.1 diff also
MISSED open_chimneys_count / cfl_/led_fixed_lighting_bulbs_count / suggested_
improvements, so the original change actually mapped only 3 of the 33 skipped
certs (the rest still failed on open_chimneys_count).
Re-derived the exact set empirically from all 33 skipped cohort certs:
widen only fields that are (a) required in 21.0.0, (b) already optional in
21.0.1, AND (c) genuinely omitted by ≥1 of those certs. Result:
- KEEP optional: the 4 SapWindow refinements, the top-level vent/lighting/
door/pressure-test block (incl. the 3 previously-missed fields), 2
SapEnergySource fields, Addendum.addendum_numbers, PhotovoltaicSupply.
none_or_no_details, and exactly ONE building-part field
(SapBuildingPart.roof_insulation_thickness — omitted by 7 certs).
- REVERT to required: the other 12 SapBuildingPart fields (construction_age_
band, wall_construction, …), MainHeatingDetail.emitter_temperature,
PvBatteries.pv_battery, ShowerOutlets.shower_outlet — none of the 33 certs
omit these, so they stay strict.
Mapper: coalesce the count fields (wet_rooms_count, open_chimneys_count,
cfl_/led_fixed_lighting_bulbs_count) to 0 like every other mapper, so the now-
optional values can't reach a NOT-NULL column (also drops 4 pyright ignores).
Now maps 32/33 (up from 3); the last cert hits a pre-existing pv_batteries-
shape AttributeError and degrades via the ADR-0031 skip path. pyright net
unchanged (43, no new errors); regression test rewritten to the real omitted set.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
