Since #1665 landed, the full room-in-roof geometry is stored in and reconstructed
from the epc_room_in_roof / epc_room_in_roof_surface child tables; the two flat
columns on epc_building_part (room_in_roof_floor_area,
room_in_roof_construction_age_band) are redundant duplicates that are written but
never read. Remove the two SQLModel fields and their from_domain writes so the
backend stops populating them — the required step BEFORE the FE migration drops
the columns (dropping them while a backend that still writes them is live would
500 every save).
Reconstruction already reads the child table (_to_room_in_roof), so the
round-trip gate stays a hard zero. The columns remain in the DB (nullable,
unwritten) until the FE drop migration runs.
Deploy AFTER #1674 is stable in every environment; the FE column drop runs after
this deploys everywhere.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SapBuildingPart.wall_u_value (the gov-EPC lodged wall U, authoritative over the
derived default) had no column and dropped on save, so the reloaded dwelling fell
back to the age-band default U — -2.32 SAP on the 2 affected RdSAP corpus certs.
Surfaced by the new DB-round-trip gate. The prior _UNPERSISTED_ALLOWLIST
justification ("deliberate — full-SAP only; RdSAP re-model uses default U-values")
was stale: the cascade DOES honour the lodged U on the API path.
Add wall_u_value to EpcBuildingPartModel, write in from_domain, reconstruct in
_to_building_part, drop the allowlist entry. Verified: certs 39036600 / 38137278
round-trip 72.43 / 69.11 exactly. Needs an added FE column
(epc_building_part.wall_u_value, double precision NULL) on assessment-model#443.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SapRoomInRoof.detailed_surfaces (+ the gable/common-wall scalars) had no columns,
so 107 corpus certs lost their whole §3.9/§3.10 room-in-roof geometry on the DB
round-trip and fell back to the Simplified all-elements age-band default —
worst case -15.21 SAP (#1665 / #1664).
Add EpcRoomInRoofModel (0..1 per building part, unique FK) + EpcRoomInRoofSurface
Model (0..n, surface_index order). Save nests via RETURNING (building part -> RIR
-> surfaces); delete clears bottom-up (surfaces -> room_in_roof -> parts) since
the FE FKs are ON DELETE no action; both read paths group + reconstruct in a new
_to_room_in_roof / _to_rir_surface, replacing the two-field flat rebuild in
_to_building_part. The flat room_in_roof_* columns on epc_building_part are
superseded and dropped in a follow-up (#1664). Nullable is preserved on
insulation_thickness_mm / u_value (null vs 0 is the Table 17-vs-18 branch); area
is unconstrained (a §3.9.2 absent-gable adjustment is signed). Drop the 8
_UNPERSISTED_ALLOWLIST entries. Verified: worst RIR cert 10012119141 round-trip
-15.21 -> 0.0000 with detailed_surfaces at deep equality.
Companion FE migration: assessment-model#443 (epc_room_in_roof +
epc_room_in_roof_surface). Deploy gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EpcPropertyData.sap_roof_windows (List[SapRoofWindow]) had no table, so 55
corpus certs lost their rooflights on the DB round-trip and were re-scored
without them (§3 (27a) heat transmission + §6 solar gain). Add EpcRoofWindowModel
(mirrors EpcPhotovoltaicArrayModel) with a roof_window_index order column and the
resolved SapRoofWindow fields; wire save (batch insert), delete (both delete
paths), both read paths (get + get_many) and _compose reconstruction; drop the
two _UNPERSISTED_ALLOWLIST entries.
u_value_raw / g_perpendicular / frame_factor are stored resolved, not re-derived:
the Table 24 lookup is keyed on glazing_type AND glazing_gap, and glazing_gap is
not retained on SapRoofWindow, so they can't be reconstructed from the persisted
fields. window_location is JSONB (int on API / str on site-notes). Verified: a
roof-window cert round-trips sap_roof_windows to deep equality.
Companion FE migration: assessment-model#443 (epc_roof_window). Deploy gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SapAlternativeWall.u_value (heat_transmission.py:1616), .wall_thickness_mm
(:1645) and .is_basement (:1618) were read by the calculator but had no column,
so they dropped on save. Latent today (0 gov-API certs lodge them — live only on
the Elmhurst/site-notes path), but is_basement is the nastiest shape: it does not
null out, it FLIPS meaning (True -> None -> is_basement_wall reads False),
silently switching off the RdSAP §5.17 / Table 23 basement-wall U path.
Add alt_wall_{1,2}_{u_value,thickness_mm,is_basement} to EpcBuildingPartModel,
write in from_domain, reconstruct in _to_alt_wall, and drop the three
_UNPERSISTED_ALLOWLIST entries so the structural guard enforces them. is_basement
is nullable (None "not stated" != False "not a basement"; a NOT NULL DEFAULT
false re-enables the code-6 heuristic — same reasoning as #1661's wall_is_basement).
Companion assessment-model migration: six nullable columns on epc_building_part
(deploy gate). Round-trip + field-coverage guard green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EpcPropertyModel.uprn was `Optional[int]`, which SQLModel maps to INTEGER, so a
real 11/12-digit UPRN (e.g. 10000452538) overflowed int32 and hard-failed the
insert with NumericValueOutOfRange — a failure on EVERY real-cert save, masked
because the round-trip fixtures use small fake UPRNs. The FE `property.uprn` is
already bigint; pin the column to BigInteger so ours matches (backend-only, no
migration). Verified: uprn 10000452538 saves + reloads.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SapHeating.cylinder_heat_loss is read by the calculator (cert_to_inputs §4
cylinder loss) but had no column, so it was silently dropped on save: a dwelling
reloaded from Postgres lost its lodged cylinder loss and fell back to the Table
2b age-band default — worst case -15.53 SAP over the corpus round-trip (#1665).
Add heating_cylinder_heat_loss (nullable double precision) to EpcPropertyModel,
write it in from_domain and reconstruct it in the SapHeating compose path,
mirroring heating_cylinder_volume_measured_l. Remove the now-false
_UNPERSISTED_ALLOWLIST entry ("dormant — not read by the calculator") so the
ADR structural guard enforces it. Verified: worst cylinder cert 10091630692
round-trip -15.53 -> 0.0000.
Companion assessment-model migration (heating_cylinder_heat_loss column) must
land + be applied per-environment before the backend deploys (deploy gate).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The service was the last one living wholly under backend/. It now follows
the same layering as abri and the other newer services:
domain/pashub_fetcher/ core file classification, subfolders
infrastructure/pashub_fetcher/ PasHub client, token getter, wire DTOs
orchestration/ PashubFetcherOrchestrator (was PashubService)
applications/pashub_fetcher/ lambda handler, trigger request, dev tooling
core_files.py is split along the layer boundary: the domain module keeps the
filename/evidence-category classification rules and no longer imports
infrastructure.postgres, while the CoreFiles -> FileTypeEnum translation moves
to infrastructure/pashub_fetcher/core_file_types.py.
Tests move into the tests/ tree by layer. Note this puts them in the only
suite CI currently runs (unit_tests.yml is disabled), so these 73 tests now
execute on PRs for the first time; they were previously reachable only via
the legacy pytest.ini testpaths.
sharepoint_renamer's image now copies just domain/pashub_fetcher/ rather than
the whole service, since SharepointSubfolders is all it needed.
Behaviour is unchanged. tests/ goes 9927 -> 10000 passed (+73, exactly the
tests that moved in); the legacy suite keeps its same 17 pre-existing failures
and 11 errors. tests/test_lambda_packaging.py confirms both changed
Dockerfiles still copy their handler's full import closure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RdSAP 10 §5.17 / Table 23. `wall_is_basement` is not a lodged U-value — it is a
disambiguation flag selecting WHICH RdSAP default applies, so unlike
wall/roof/floor_u_value it must survive the round-trip.
Code 6 is canonically SYSTEM-BUILT and the gov-API path infers "basement" from a
code-6 heuristic; the Elmhurst site-notes mapper sets wall_is_basement=False
precisely to defeat it. Dropping the field inverted the answer — a system-built
wall came back as a basement wall, dragging the whole ground floor onto the
Table 23 basement-floor U-value via `has_basement`.
Declared on no RdSAP schema, so it never arrives via the gov API; the earlier
corpus scan showing 0 occurrences measured the wrong population. The Elmhurst
site-notes path sets it and those EPCs are persisted
(scripts/wchg_elmhurst_ingest.py).
Nullable is load-bearing: None = "not stated, use the heuristic", False =
"explicitly system-built, do not".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add `rafter_insulation_thickness` to `EpcBuildingPartModel` (JSONB, matching the
sibling thickness columns so Union[str, int] survives) and wire it through
`from_domain` / `_to_building_part`.
Property 749719 lodges "250mm" -> Table 16 col (2) U 0.23. Dropped, the roof
billed at U 2.30 — a 10x error worth -9.8 SAP (lodged 73, calculated 63.2) that
moved the dwelling a whole band, C -> D, and drew a 9.63-point loft-insulation
recommendation on a roof rated 4/5 by the assessor.
The sibling wall/roof/floor_u_value and wall_is_basement fields have the same
drop but are deliberately NOT persisted: they are full-SAP artefacts and we
re-model as RdSAP, which uses default U-values. Their allow-list entries are
re-justified rather than removed — the previous "not read by the calculator" was
stale and is what hid this drop.
Requires the Drizzle migration in Hestia-Homes/assessment-model to land FIRST;
without the column, EPC reads fail outright.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dan's review on #1546:
- Stream the workbook to a /tmp temp file and S3Client.upload_file it, instead
of render_workbook -> bytes -> put_object. render_workbook now saves straight
to a path; the orchestrator renders to a temp file, multipart-uploads it, and
always cleans it up. Restores ADR-0065's "never an in-memory BytesIO" decision
(the OOM path at the 100k-row cap).
- Move the raw `SELECT ... FROM scenario` out of the Lambda handler into
ScenarioNamesPostgresRepository, so the handler stays composition-only and all
SQL lives in repositories/.
- current_sap_points goes through _float, matching its Optional[float] read-model
type and the sibling numeric facts.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The deal's third_party_surveyor_identifier is now required by the log
and amend flows: the trigger contract rejects messages without it
(blank treated as missing, since a blank resource makes OpenHousing
silently drop the appointment), and LogJobRequest/AmendJobRequest carry
it as a required field. The ABRI_RELAY_DEFAULT_RESOURCE fallback and
its config/terraform/workflow wiring are removed as dead code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Carry third_party_surveyor_identifier on the Abri trigger message and
through dispatch into the log and amend flows, booking the job against
that resource. LogJobRequest gains the optional resource field
AmendJobRequest already had; the client falls back to the configured
default surveyor when the deal names no third-party resource, so
existing deals behave unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CalicoParser now tracks how many rows it drops for a blank/N-A covering type
and how many references it drops for not resolving to a portfolio UPRN, exposed
for the load reconciliation report so nothing vanishes silently.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The real Calico export lodges the literal string "N/A" (not an empty cell) for
placeholder rows; _clean_str now treats N/A-family sentinels as no covering, so
the 1,239 placeholders are dropped on parse (5,346 -> 4,107 real observations).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CalicoParser reads the workbook into CalicoAssetCondition rows, drops rows
with no covering type, and groups them by asset reference into CalicoProperty,
resolving each reference to a UPRN via the injected lookup (unmatched dropped).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Behaviour-preserving lift-and-shift of the condition module out of legacy
backend/condition/ into domain/condition, infrastructure/condition,
infrastructure/postgres/condition_tables.py, repositories/condition,
applications/condition, and tests/condition. Imports rewritten to the DDD
paths; ConditionPostgres and the ORM models keep the legacy backend.app.db
Base/db_session bridges so the existing suite proves behaviour is unchanged
(SQLModel + session-DI conversion tracked as a follow-up in ADR-0064).
16 condition tests pass at the new location.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An unrecognised schema_type used to raise ValueError and abort the whole
call; now from_api_response logs a warning and returns None so one
unmapped cert doesn't break a batch. Schema 15.0 already has a mapper
(PR #1531) so it's unaffected; only genuinely unmapped versions skip.
OpenHousing logs a job but silently drops the appointment when no bookable
resource is sent, so the survey date never lands. log_job already sends the
configured default_resource; amend_job omitted the parameter unless the
request carried an explicit surveyor. Default amend's resource to the
configured surveyor too, and fail loudly at config load on a blank
ABRI_RELAY_DEFAULT_RESOURCE so the misconfiguration can't ship an empty
resource again.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
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>
