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/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`):

```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).