Both flagged mismatches were Elmhurst input errors (same silent-stale-value
contamination class as the earlier chimneys/wall-thickness bugs), not
parser or calculator bugs:
- cylinder_size: build_100010086084.py's water_heating() selected
DropDownListCylinderSize by raw value "2", but this DOM's option values
ARE their visible litre-band text (no "2" among them) — Playwright
silently no-ops on a non-matching value, leaving a prior cert's "Medium
(131-170L)" selection in place. Fixed to match by text ("Normal"), and
added the missing mapper dict entries (Normal/Large litre-suffixed
labels) to _ELMHURST_CYLINDER_SIZE_LABEL_TO_SAP10 (a real mapper-coverage
gap — the calculator raises UnmappedElmhurstLabel rather than silently
mis-mapping).
- boiler_flue_type: the boiler-code search dialog's combined "Balanced/Open
Flue" Table 4b category doesn't drive the separate, independently
selectable RadioButtonListFlueType field, which was left at an inherited
"Balanced" from a prior cert. Fixed space_heating() to explicitly select
"Open" to match the lodged gov-API code. This field isn't consumed by
Sap10Calculator (ML/generator-only), so it had zero effect on the SAP
score.
Re-downloaded elmhurst_summary.pdf/elmhurst_worksheet.pdf after the fixes
(Recommendations page confirmed clean). Elmhurst-PDF-inputs path moved
46->47 SAP from the cylinder-volume correction; gov-API SAP (53) and
Elmhurst's own worksheet (51) are unchanged. Full accuracy suites re-run
clean (67 passed, 67 skipped, 1 xfailed, same as before).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Elmhurst rebuild for this cert had two carryover bugs from the shared
Playwright session: TextBoxOpenChimneys silently inherited "2" from a prior
cert's build (this cert's register lodges open_fireplaces_count=0, which
the gov-API mapper already reads correctly), and wall thickness inherited
a stale 280mm. Also fixed a real bug in the space_heating() PCDB-ref-clearing
logic: input_value() returns the string "0" when already cleared, which is
truthy in Python, causing a spurious navigate-away that broke the next
E.goto() call.
Rebuilt cleanly (chimneys=0, wall thickness=220mm per RdSAP convention for
unrecorded solid-wall thickness, since CheckBoxWallThicknessUnknown does not
persist via Playwright in this tool version). Elmhurst's own worksheet score
moves from a contaminated 46/48 to 51, closing the gap against this engine's
53-on-gov-API-inputs from ~5-7pt to ~2pt. No mapper.py change was needed --
the gov-API mapper's open_chimneys_count=0 was correct all along.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Built UPRN 100010086084 (the largest gov-API-vs-lodged outlier from the
20-cert batch) in Elmhurst's accredited RdSAP10 tool to get real ground
truth, per feedback that lodged rating isn't a valid comparison target.
Elmhurst worksheet: SAP 46-48 (orientation-dependent), vs this engine's
53 on gov-API inputs -- a real ~5-7pt residual gap. Ruled out with
direct empirical tests: total_floor_area, missing party_wall_length,
region/climate (SAP rating always uses UK-average weather per Appendix
U -- confirmed inert), boiler/secondary efficiency (exact match, 66%/
63% both sides). Confirmed real but NOT blind-fixed (need more than one
cert's evidence, and a wall-U-value patch tested the wrong direction in
isolation): a wall-thickness-unknown U-value fallback mismatch (this
engine 1.70, Elmhurst's own "unknown" resolution 1.40) and a window
U-value divergence (1.85 vs 2.52 W/m2K for the same nominal glazing
pick). Single-window orientation confirmed a ~2pt partial contributor.
Also fixes a small, separate, evidenced gap the build surfaced: the
Elmhurst summary parser didn't recognise the live tool's "Medium
(131-170L)" cylinder-size label (only the bare "Medium").
Full write-up is in the RealCertExpectation comment for this cert in
test_real_cert_sap_accuracy.py and the worklist. Residual gap is
intentionally left open for follow-up, not force-closed.
These are 2011 SAP-2009 lodgements; this calculator implements current
RdSAP10, which has since revised several U-value tables. Closeness to
lodged was never evidence of mapper correctness (nor is a gap evidence
against it) -- the prior commit's comments and worklist entries wrongly
framed 16/20 "matching lodged" as validation and the other 4 as
anomalies needing explanation. Corrected the comments to state what the
batch actually shows: all 20 structurally distinct certs map without
error and produce plausible scores. Real accuracy validation still
needs an Elmhurst RdSAP10 rebuild on the same methodology as the
engine, which remains open.
Sampled all 20 unique property_ids that failed with "Unsupported EPC
schema: 'SAP-Schema-15.0'" across the sub_task history (114 total across
72 subtask rows), captured 20 real live certs, and pinned each as a
RealCertExpectation regression in the accuracy corpus.
16/20 land exact or within 1-3 SAP of the lodged rating, confirming the
door_count/WWHRS/FGHRS/draught-proofing defaults generalise beyond the
original two properties. The other 4 (all solid-brick, uninsulated,
age band C) diverge +6 to +12 — traced to a legitimate SAP-2009-vintage-
lodgement vs RdSAP10-recalculation methodology gap (u_wall() checked
directly against RdSAP10's own Table 6, returns the correct 1.70 — no
mapper defect), not a mapper bug, so pinned to the engine's observed
value rather than tuned to lodged.
First successful live run surfaced three issues:
1. Email looked rubbish (a giant raw presigned URL). Now sends a proper HTML
email with a 'Download documents' button plus a plain-text fallback, and a
summary (N documents across M properties, expiry). Email delivery is now
best-effort: a transport failure no longer loses an already-built package
(the link is still on sub_task.outputs), and the SMTP connect has a 30s
timeout so an unreachable SES endpoint fails fast instead of hanging to the
900s Lambda timeout.
2. Every folder was 'address unavailable (...)': the resolver read property.address,
but these are HubSpot deals with no property row. It now uses the deal's
dealname from hubspot_deal_data.
3. No logs / no idea why a run took ~9 minutes: the worker's INFO logs were
dropped (Lambda root logger defaults to WARNING). The handler now raises the
level, and the orchestrator logs per-phase timing and volume (gather+plan,
packaged N files / X MB, upload, email, total) so the slow phase is visible.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
'Cancelled' and 'no show' were separate strings in the outcome vocabulary,
but HubSpot only ever emits the one dropdown value 'Cancelled / No Show' -
so neither phantom string could ever match. Replace both with the real
value (mapped to NOACCESS, best-guess pending client), which also fixes the
long-standing differ mismatch: the abandonment trigger now actually fires on
Cancelled / No Show. Drops the now-unused REQT enum member.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ADR-0034's nearby-postcode broadening degrades to a genuine
NoSameTypeComparablesError whenever the default 1000m/30-postcode reach has no
same-type comparable nearby (e.g. property_id=752685, portfolio 824 — the only
Maisonette within reach). Adds one configurable extra widening step
(EpcComparablePropertiesRepository.candidates_near now accepts
widen_nearby_postcodes), tried only when the normal-radius walk falls short of
`minimum` matches. modelling_e2e's handler wires this to a 3000m/60-postcode
PostcodesIoClient as the single wider step.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Complete the outcome-to-code table so all five UNSUCCESSFUL_OUTCOMES map:
cancelled -> REQT (new enum member) and no show -> NOACCESS join the
existing three. Only no answer -> CARDED is firm; the rest are best-guesses
for client confirmation, noted inline with alternatives.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the CARDED placeholder in abandon_reason_for_outcome with the
real outcome-to-code table from the client: No Answer->CARDED,
Tenant Refusal->CRA (ambiguous, client follow-up pending),
Not Viable->NOACCESS. Only these three outcomes trip the abandonment
trigger today; any empty/unrecognised outcome raises
UnmappableOutcomeError since abandon_reason is mandatory.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A live test returned 'no documents could be packaged' for 91 correctly-resolved
properties. Root cause: no upload source populates uploaded_files.landlord_property_id
(pashub/magic-plan/audit set hubspot_deal_id; ECMK sets hubspot_listing_id), so
matching on landlord_property_id found zero rows.
Match fix: the worker now joins uploaded_files -> hubspot_deal_data (on deal_id) ->
landlord_property_id, taking the property identity from the bridge. The repository
returns a small PropertyDocument read-model instead of the infra ORM row, so the
orchestrator no longer names infrastructure.postgres.* (resolves the leak dancafc
flagged) and the s3_upload_timestamp cast is gone. Coverage is limited to
deal-id-linked sources; listing_id/uprn-only files are a noted follow-up.
Observability: the empty-selection failure now carries stage counts
(selected/matched/planned/skipped) in both the message (-> the worker WARNING log)
and details (-> sub_task.outputs), and the run logs those counts. The next failure
says 'matched 0 documents' instead of failing opaquely.
ADR-0060 + CONTEXT.md updated (matching decision + considered options).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
uploaded_files has no property_id — matching is on landlord_property_id — so the
property_ids path was pointless indirection (look property.id up in property just
to translate back to landlord_property_id). The FE holds landlord_property_id per
row anyway.
task.inputs hand-pick key is now landlord_property_ids: str[] (was property_ids:
int[]). resolve_selection unions two landlord_property_id sources — project_codes
expanded via hubspot_deal_data, and the hand-picked ids taken as given — and no
longer queries the property table at all. Trigger-only change; the domain plan,
orchestrator and matching already work in landlord_property_id.
ADR-0060, CONTEXT.md and the request schema updated to match.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Property 753950 (uprn 100021969385, cert 0141-2860-6891-9124-5625,
SAP-Schema-16.2) hard-failed modelling_e2e: RdSapSchema17_1 requires
multiple_glazed_proportion as a non-optional int, and this 16.x cert omits
the field entirely, lodging only the multiple_glazing_type="ND" sentinel.
The cert's own window.description ("Fully double glazed") states the
glazing extent unambiguously, so _normalize_sap_schema_16_x now derives
0/100 from "single"/"double" wording when the field is absent, mirroring
the existing single-glazed multiple_glazing_type cascade. This is
worklist P4 (.claude/skills/expand-sap-accuracy-corpus/worklist.md) —
a flat default was tried previously and reverted because making such
certs mappable at all pulls them into the EPC-prediction donor pool and
tips near-tie similarity matches; deriving from explicit text (rather
than a blind default) was the suggested unblock.
Re-measured the component-accuracy gate as the worklist asked: it drops
(has_hot_water_cylinder 0.8687->0.8586, cylinder_insulation_type
0.3333->0.1667, door_count residual 0.3131->0.3333) via the same
donor-pool-composition mechanism as the prior #1245/ADR-0037
re-baselines, not a prediction-logic loosening. Re-baselined the floors
with that rationale recorded inline.
The 'all properties' selection resolved against property.portfolio_id, but a
portfolio spans multiple HubSpot projects — so 'all' pulled the whole portfolio,
not the project the user was looking at. The project↔property grain lives on
hubspot_deal_data, not property.
task.inputs is now {project_codes?: str[], property_ids?: int[], portfolio_id?}:
the route resolves the distinct landlord_property_id set as the union of every
property in the named project_codes (from hubspot_deal_data) and the hand-picked
property_ids; portfolio_id is optional and only names the package. Drops the
portfolio-scoped select_all. Cap now applies to the resolved set size.
ADR-0060, CONTEXT.md and the request schema updated to match. New router tests
cover project-code resolution, the union+dedup, null landlord_property_id drop,
and the empty-project rejection.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses PR #1498 review:
- A per-file read failure (missing/deleted object, or a bucket the role can't
reach) is now a SkippedDocument(reason=unreadable), not a whole-run abort;
an all-unreadable selection still fails (no empty package). Best-effort on
the read path (ADR-0060).
- Each ZIP member is streamed to a temp file (S3DocumentDownloader.download,
boto download_file) and added from disk, so a single multi-GB member never
hits the heap — the 'never held whole in memory' claim is now true.
- The 409 double-submit guard is DB-arbitrated: the sub_task insert is
conditional on the task having none, so a race creates only one.
- Local env gets a placeholder recipient email so the route is exercisable.
- PackageEntry carries landlord_property_id so a read failure can be reported.
- TODO noting the UploadedFile.s3_upload_timestamp typing fix.
Two new tests: per-file read failure skips-and-reports; address fallback flows
to the folder name.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per PR review: the FE writes the selection config
({portfolio_id, property_ids?, select_all?}) into the FE-owned task.inputs and
passes only task_id, so a large hand-picked selection never travels in an HTTP
body. The route reads task.inputs, resolves to landlord_property_ids, caps, and
pins the recipe onto sub_task.inputs as before. Declares the FE-owned inputs
column on the TaskRow mirror so the backend can read it and the test schema
builds it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolves the authenticated requester's email (ADR-0059), resolves + caps the
property selection, pins the recipe (landlord_property_ids, recipient_email,
package_name) onto one pre-created sub_task (raw SQL, dodging the mirror
double-registration), and enqueues one message to the worker. Refuses
double-submits (409) and oversized selections (400).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins the best-effort contract (ADR-0060) delivered with the tracer — no red
phase; discriminating: a strict impl would fail the run or omit the report.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins the auth handshake delivered with the tracer green (no red phase);
discriminating: an unauthenticated/cleartext send would be rejected by SES.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins behavior delivered with the tracer slice (the group key is
(landlord_property_id, document_type)) — no red phase; discriminating: a
global-by-type dedupe would drop one property's file.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
date_abandoned now resolves to the third failed attempt's confirmed
survey date, falling back to its last submission date, rather than the
processing date. This dates the OpenHousing cancellation to when the
job actually lapsed even if the trigger message lags or redelivers.
- domain: abandonment_date() encodes the confirmed-survey -> last-
submission fallback
- DealAbandonment carries both dates; abandon_job raises
AbandonmentDateUnknownError when a deal has neither
- last_submission_date now flows through the trigger message and request
- the injected clock is dropped: nothing reads now() any more
Co-Authored-By: Claude Opus 4.8 <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>
