assessment-model/docs/design/bulk-upload-finaliser-v2-handover.md

Handover: bulk_upload_finaliser v2 — populate property_overrides

Purpose. Self-contained brief to start a fresh context implementing v2 (the per-Property fact layer) in the existing bulk_upload_finaliser Lambda. v1 (async finalise that writes property) is shipped and working end-to-end. This doc assumes no memory of the v1 session.

0. Design resolved — grilling outcome (2026-06-05)

The open questions in §9 were resolved in a design session. This section is now authoritative; the later sections are kept for background but where they conflict with this one, this one wins. The new v2 ADR is docs/adr/0006-property-overrides-join-and-no-uprn-defer.md; ADR-0004 was amended for per-count ordering capture.

Spine. Populate property_overrides at finalise for UPRN-matched rows only. Join the classifier descriptions to the combiner identity by a synthetic UUID source_row_id — not Internal Reference (it is absent from the classifier CSV, and optional anyway) and not by carrying description columns through address2uprn (architecture B, rejected). This is architecture (A) with a purpose-built key.

No-UPRN rows are deferred to v3. v1 currently inserts them as property rows; v2 changes nothing in the property insert and simply writes no overrides for them. The eventual home for unmatched rows is a separate staging table (Model B): property holds only matched rows; unmatched inputs (with their descriptions) live in the staging table until a different UPRN matcher assigns a UPRN and promotes them. "Found vs unfound" is a view across both tables, not a flag on property. v3 owns the property-insert change + the staging table + the matcher-rerun UX together.

Frontend work (/workspaces/assessment-model):

1. Mint source_row_id (UUID) in start-address-matching right after readRows(), and explicitly emit it as a column in both buildAddressCsv and buildClassifierCsv — both project a fixed column set, so attaching it to the row object is not enough. It survives address2uprn→combiner like any input column (carried as additional_info); verify against a real combiner output.
2. Per-count ordering capture (supersedes ADR-0004's largest-count-only): detectMultiEntry keeps a sample per distinct count; OnboardingProgress renders one ordering panel per count ≥ 2. The jsonb type and setMultiEntryOrdering validation already accept all counts — no migration, no backend-validation change.
3. Verify gate hardened: Finalise is blocked while any description is still UNKNOWN. UNKNOWN is now a transient "needs review" marker, never a final value (this retires the old "UNKNOWN is legitimate" line in §7).
4. dispatchFinaliser adds two fields to the trigger body: classifier_s3_uri and multi_entry_ordering (it already reads the bulk_address_uploads row, and dispatch happens after the user confirms ordering, so the value is final). The classifier S3 key comes from a shared classifierCsvKey(portfolioId, uploadId) helper used by both the writer and the dispatcher (the key is not stored anywhere today — convention only).

Backend work (/workspaces/home/github/Model): 5. Grow the trigger schema in two places — FastAPI FinaliserTriggerRequest and Lambda BulkUploadFinaliserTriggerBody — with classifier_s3_uri + multi_entry_ordering. Handler stays trigger-driven (no new bulk_address_uploads coupling). 6. PropertyOverrideRow table mirror + a sibling PropertyOverrideRepository (own aggregate; upsert on (property_id, override_component, building_part)), and a read-only LandlordOverrideRepository that loads a portfolio's vocabulary per component into dicts once (the vocabulary is deduplicated, not per-row). 7. Orchestrator step, in the same commit_scope:

• bulk SELECT (portfolio_id, uprn) → id for the run's UPRN rows → in-memory map;
• join classifier↔combiner rows by source_row_id;
• uniform comma-split all four components → permutations[count] → parts (count-1 cell → building_part = 0); the finaliser needs no fallback because every count ≥ 2 has a confirmed permutation;
• resolve each part's normalized description against the override dicts;
• original_spreadsheet_description = the raw entry text (un-normalized);
• empty cell → write no row; non-empty but unresolved (or UNKNOWN) → raise → commit_scope rolls back → _mark_failed flips the upload to failed (fail loudly, no partial writes);
• write only the classifier components actually mapped in columnMapping;
• no source column in v2 — upsert is unconditional for now.

Locked assumptions (load-bearing — see ADR-0006).

• One real upload per user. A re-upload only adds new properties (ones not previously included), never re-describes existing ones → part-keys are append-only across uploads → upsert-only, no delete-orphans is correct and complete.
• Per-count consistency. One ordering per count, confirmed from one sample, applies to every cell of that count in the file (extends ADR-0004's bet to all counts).
• Per-cell count. Walls may split into 3 while Roofs splits into 2 in the same row; each cell is ordered by its own entry count.
• Classification completes before awaiting_review, and the hardened verify gate forces every UNKNOWN to be resolved — so an unresolved description at finalise is a genuine defect, hence fail-loud.

1. Where v1 left things (read first)

v1 made Finalise an async dispatched Lambda that writes property rows. The full flow works in dev: dispatch 202 → SQS → Lambda inserts properties + writes terminal status → UI advances to "Processing complete".

Authoritative background — read these before coding:

• docs/design/bulk-upload-finaliser.md — the full grilling/design doc (schema Q6–Q9, snapshot-not-FK, recalculate-on-rerun, the v2 input hazards).
• docs/adr/0005-async-bulk-upload-finaliser.md (frontend) — state machine + property_overrides shape.
• /workspaces/home/github/Model/docs/adr/0013-bulk-upload-finaliser-writes-properties.md (backend) — the Lambda write path + DDD layering.
• docs/adr/0004-multi-entry-building-part-ordering.md — critical for v2: how building parts and multiEntryOrdering work.
• docs/adr/0002-landlord-override-vocabulary.md — the vocabulary (landlord_*_overrides) layer v2 resolves against.
• CONTEXT.md — glossary: Property override, Building part, Main building, Extension, Multi-entry, Building-part ordering, VocabularyMapping.

Convention that must hold (it was corrected hard in v1): in the Model repo, business logic lives in orchestration/*_orchestrator.py; the Lambda applications/*/handler.py stays thin (parse trigger, wire infra, delegate). One repository per aggregate; orchestrators never commit (the handler owns the transaction via commit_scope). See memory model-ddd-layering.

2. v2 goal

Populate property_overrides during finalise: for each property, write one row per (building_part, override_component) carrying the resolved enum snapshot of the landlord's description for that part.

3. The target table (already shipped — migration 0221, do NOT re-migrate)

Drizzle: src/app/db/schema/property_overrides.ts.

property_overrides
  id                              uuid pk
  property_id                     bigint NOT NULL  FK → property.id   ON DELETE CASCADE
  portfolio_id                    bigint NOT NULL  FK → portfolio.id  ON DELETE CASCADE
  building_part                   smallint NOT NULL          -- 0 = main, 1 = ext 1, 2 = ext 2, …
  override_component              override_component NOT NULL -- pgEnum {wall_type, roof_type, property_type, built_form_type}
  override_value                  text NOT NULL              -- snapshot of the resolved enum value
  original_spreadsheet_description text NOT NULL             -- raw cell text it resolved from
  created_at / updated_at         timestamptz NOT NULL
  UNIQUE (property_id, override_component, building_part)

4. Design decisions already locked (do not relitigate)

• Snapshot, not FK. override_value is a denormalised text copy of the resolved enum, taken at materialise time — not an FK to landlord_*_overrides. This is what lets two properties sharing a description diverge later, and is required because there are four polymorphic vocabulary tables. Lineage is the natural key (portfolio_id, override_component, original_spreadsheet_description).
• Re-run = recalculate. Write with onConflictDoUpdate on (property_id, override_component, building_part), refreshing override_value + original_spreadsheet_description + updated_at. (Contrast property, which is onConflictDoNothing.) When a per-property user-edit path eventually exists, this upsert will need a WHERE source='classifier' guard — but there is no source column in v1; add it as a nullable column only when that path is built.
• override_component values are exactly the classifier category keys (wall_type, roof_type, property_type, built_form_type) used in both src/lib/bulkUpload/columnFields.ts and the Model ClassifiableColumn.name — no translation.
• building_part indexing: 0 = Main building, 1 = Extension 1, … per ADR-0004.
• Whole-dwelling components (property_type, built_form_type) are per-part- capable but today's files supply them once → usually written at building_part = 0.

5. The hard part: assembling the inputs (this is the real v2 work)

The combiner output (what the v1 finaliser reads) carries only address/UPRN columns — Address 1/2/3, postcode, Internal Reference, address2uprn_uprn, address2uprn_address, address2uprn_lexiscore. The raw Walls/Roofs/ Property Type/Built Form cells are NOT in it. They live only in:

• the classifier CSV on S3 — bulk_onboarding_inputs/{portfolioId}/{uploadId}-classifier.csv (original landlord headers), and
• landlord_*_overrides in Postgres — the resolved values keyed by (portfolio_id, normalized description).

To write one property_overrides row, v2 must assemble four inputs:

Need	Source
property_id (identity)	combiner output → (portfolio_id, uprn) … but no-UPRN rows have no key
raw cell text per row	the classifier CSV (not the combiner output)
split a multi-valued cell → building parts	multiEntryOrdering on bulk_address_uploads
description → override_value	landlord_*_overrides (resolve by normalized description)

Two open hazards — both RESOLVED (see §0)

1. Join key (RESOLVED). Investigation confirmed Internal Reference is in the address CSV + combiner output but NOT in the classifier CSV, and is optional. So architecture (A)-by-Internal Reference is dead. Resolution: mint a synthetic UUID source_row_id in start-address-matching after readRows(), emitted as an explicit column in both CSVs. It is the join key. (Architecture (A) with a purpose-built key; (B) "carry descriptions through address2uprn" was rejected.)

2. property_id for no-UPRN rows (RESOLVED by descoping). v2 writes overrides only for UPRN rows, whose property.id is re-found by (portfolio_id, uprn) — so no RETURNING correlation is needed. No-UPRN rows are deferred to v3 (Model B staging table); v2 leaves the property insert untouched.

6. multiEntryOrdering — how to split cells into parts

Persisted on bulk_address_uploads (src/app/db/schema/bulk_address_uploads.ts):

MultiEntryOrdering { permutations: Record<string, number[]>; confirmed: boolean }
// permutations[count][k] = the 0-based FILE position holding building part k
//   where 0 = Main building, 1..N-1 = Extension 1..N-1.
// e.g. { "2": [1, 0] } => for 2-entry rows, the main building is file position 1.

A multi-valued cell (e.g. Walls = "Cavity: …, Solid brick: …") splits on commas into entries by file position; permutations[count] maps file position → building part. Caveat (ADR-0004): only the largest count permutation is captured this iteration; other counts need a derivation rule — decide it in v2. multiEntrySummary holds the detected multi-valued columns + normalized description keys (the normalization that matches the classifier's stored keys: split → strip → lower).

7. Resolving description → value (landlord_*_overrides)

Four per-component tables in src/app/db/schema/landlord_overrides.ts (landlord_wall_type_overrides, …_roof_type_…, …_property_type_…, …_built_form_type_…), each UNIQUE (portfolio_id, description), value typed by the component's pgEnum, plus a source (classifier|user). Resolve a normalized description → value. The frontend already does this read in src/lib/bulkUpload/server.ts (lookupOverrides) — mirror that mapping on the backend. UNKNOWN is now a transient "needs review" marker, never a final value (resolved in §0): the verify gate forces the user to map every UNKNOWN before Finalise, so a UNKNOWN (or unresolvable description) reaching the finaliser is a defect and fails the run loudly.

8. Backend pieces to build (DDD, mirror v1)

In /workspaces/home/github/Model:

• PropertyOverrideRow SQLModel mirror → infrastructure/postgres/property_override_table.py (mirror the pattern in property_table.py / landlord_*_override_table.py; reuse a shared override_component SAEnum like landlord_override_enums.py).
• Repository for the override write (one per aggregate): add to repositories/property/ (e.g. extend the property repo or a sibling property_override repo), with an upsert_all using on_conflict_do_update(index_elements=[property_id, override_component, building_part], …).
• Orchestrator logic in orchestration/bulk_upload_finaliser_orchestrator.py: extend finalise(...) (or add a step) to, after inserting properties and getting ids, build the override rows (join → split by part → resolve) and persist them in the same commit_scope.
• Handler stays thin — it already wires S3 + engine + repos. It will need the extra input (classifier CSV and/or multiEntryOrdering); decide how those reach the Lambda (extend BulkUploadFinaliserTriggerBody, or read bulk_address_uploads for multiEntryOrdering + the classifier S3 URI). The trigger currently carries task_id, sub_task_id, s3_uri (combiner output), portfolio_id, bulk_upload_id.

Key v1 files to extend (all in the Model repo):

• applications/bulk_upload_finaliser/handler.py
• orchestration/bulk_upload_finaliser_orchestrator.py
• repositories/property/property_repository.py + property_postgres_repository.py
• infrastructure/postgres/property_table.py (reference for the new mirror)
• infrastructure/s3/csv_s3_client.py (read_rows)
• Packaging test: tests/test_lambda_packaging.py will flag any new top-level import the Dockerfile doesn't COPY (v1 hit this with datatypes/).

9. Open questions — all RESOLVED (see §0 + ADR-0006)

• Join key → synthetic UUID source_row_id in both CSVs (not Internal Reference, not architecture B).
• property_id for no-UPRN rows → out of scope; v2 is UPRN-only, no-UPRN deferred to v3 (Model B). UPRN rows re-found by (portfolio_id, uprn); no RETURNING.
• Non-largest-count multiEntryOrdering → capture a confirmed permutation for every count ≥ 2 in the UI (supersedes ADR-0004); finaliser needs no fallback.
• Trigger body vs handler-reads-DB → grow the trigger body (classifier_s3_uri
  • multi_entry_ordering), built in dispatchFinaliser.
• Re-materialise → recalculate every finalise via upsert-only on (property_id, override_component, building_part); no delete-orphans (justified by the one-real-upload assumption); property rows untouched.

10. Implementation order (design is settled — build it)

Frontend first (the finaliser depends on source_row_id + per-count ordering):

1. source_row_id: shared classifierCsvKey helper; mint the UUID in start-address-matching after readRows(); emit it as an explicit column in both buildAddressCsv and buildClassifierCsv. Verify it lands in a real combiner output.
2. Per-count ordering: detectMultiEntry keeps a sample per count; OnboardingProgress renders one ordering panel per count ≥ 2. Drop the largest-count-only assumption in setMultiEntryOrdering if it requires the largest.
3. Verify gate: block Finalise while any classification is UNKNOWN.
4. dispatchFinaliser: add classifier_s3_uri + multi_entry_ordering to the trigger payload.

Backend:

5. Grow FinaliserTriggerRequest (FastAPI) + BulkUploadFinaliserTriggerBody (Lambda).
6. PropertyOverrideRow mirror + sibling PropertyOverrideRepository (upsert) + read-only LandlordOverrideRepository.
7. Orchestrator step (join → split → resolve → upsert; fail-loud on unresolved), TDD against fakes (mirror tests/orchestration/test_bulk_upload_finaliser_orchestrator.py).
8. Handler wiring; watch tests/test_lambda_packaging.py for Dockerfile COPY gaps.

Docs (done in this session): ADR-0004 amended, ADR-0006 added, CONTEXT.md "Property override" updated.

11. Verification notes (environment)

• Frontend: npx tsc --noEmit (was 0 errors at v1 close).
• Model repo: mypy/pytest need a deps-installed env (the v1 session couldn't run them locally; /app Docker config runs the full suite). terraform plan needs the CLI. Watch tests/test_lambda_packaging.py for Dockerfile COPY gaps.
• v1 is committed; dev Lambda + SQS queue are deployed and working (FINALISER_SQS_URL wired in backend/.env for local, and in terraform/fast-api).