mirror of
https://github.com/Hestia-Homes/assessment-model.git
synced 2026-06-30 12:55:02 +00:00
remove doc files
This commit is contained in:
parent
b387bc24f8
commit
35775164ef
6 changed files with 2 additions and 82 deletions
2
.gitignore
vendored
2
.gitignore
vendored
|
|
@ -39,3 +39,5 @@ cypress.env.json
|
|||
next-env.d.ts
|
||||
|
||||
backlog/**
|
||||
|
||||
docs/adr/**
|
||||
|
|
|
|||
|
|
@ -1,20 +0,0 @@
|
|||
# BulkUpload state machine (v1)
|
||||
|
||||
The BulkUpload lifecycle is:
|
||||
|
||||
```
|
||||
ready_for_processing → mapping_complete → processing → combining → awaiting_review
|
||||
→ complete | needs_review | failed
|
||||
```
|
||||
|
||||
Two writers: Next.js writes the user-driven transitions (`mapping_complete`, `processing`, `complete`, `needs_review`); the FastAPI `bulk_address2uprn_combiner` worker writes `combining` and `awaiting_review` directly to the DB during its run. Any aggregate enforcing the state machine on the Next.js side must treat those two as observed-not-owned.
|
||||
|
||||
Three deliberate "not yet" decisions are baked into this version, each likely to be re-suggested without a record:
|
||||
|
||||
1. **`needs_review` is terminal — no recovery flow.** A finalised upload with missing or duplicate UPRNs ends up here, and that's it. The imported Properties show up in the property table; a proper review/recovery UI is planned but out of scope. Without this note, every future architecture pass will surface "needs_review has no exit" as a bug.
|
||||
|
||||
2. **Re-mapping is rejected after `mapping_complete`.** PATCHing `columnMapping` once Address matching has been triggered returns 409. We considered (B) reset-and-rerun (clear the combiner output, return to `mapping_complete`, force the user to re-trigger) but rejected it for now: it requires cleaning up abandoned Tasks/SubTasks and re-charging a FastAPI run. (A) is the smallest correct thing — the DB column never drifts from what produced the combined output. Revisit when re-run-from-review lands.
|
||||
|
||||
3. **`failed` exists in the schema but is not yet written by any route.** Synchronous trigger failures (route can't reach FastAPI) are surfaced as React Query toasts on 5xx; the BulkUpload stays in its prior status and the user retries. In-flight failures (Combiner crashes silently) currently leave uploads stuck in `processing` — this is known-incomplete, waiting on FastAPI-side callback work to report failure. The status is in the schema now so the seam is ready when that work is scheduled.
|
||||
|
||||
These are the only "no" decisions in v1; everything else (concurrency guards on stage triggers, persisting `failed`, etc.) is intended to be added.
|
||||
|
|
@ -1,17 +0,0 @@
|
|||
# BulkUpload pipeline stays browser-driven for now
|
||||
|
||||
The BulkUpload pipeline chains three stages — address matching, combiner trigger, finalise — and the chain is currently driven by the **frontend polling** in `OnboardingProgress.tsx`. The browser fires `POST /combine` when the Task looks done, then `POST /finalize` when the upload reaches `awaiting_review`. Close the tab → upload gets stuck.
|
||||
|
||||
A server-driven alternative (FastAPI callback into a Next.js webhook, or a sweeper route) would be more robust, but the entire bulk-upload flow is scheduled for redesign. This code path is being kept as **internal tooling for the Domna tech team** to onboard portfolios while the new flow is built — not for end-user use.
|
||||
|
||||
So the cleanup invests only in things that survive the redesign:
|
||||
- A real state machine on the BulkUpload aggregate (so the tech team can debug from the row alone).
|
||||
- Deduplicated FastAPI trigger logic.
|
||||
- Concurrency guards on stage triggers.
|
||||
|
||||
It deliberately does **not** invest in:
|
||||
- Server-side stage orchestration.
|
||||
- Recovery from `failed` / stuck uploads.
|
||||
- A real `awaiting_review` review UI.
|
||||
|
||||
When the redesign lands, browser-driven chaining and the auto-finalise behaviour go with it. Future readers asking "why didn't they fix this" — that's why.
|
||||
|
|
@ -1,12 +0,0 @@
|
|||
# Task creation lives in BulkUpload, not behind a generic Task endpoint
|
||||
|
||||
A `Task` is the orchestration handle for a FastAPI pipeline run; per `CONTEXT.md`, **a BulkUpload has at most one Task**. Today BulkUpload is the only feature that creates Tasks.
|
||||
|
||||
We're collapsing the previous two-step client seam — `POST /api/tasks` to create a Task and SubTask, then `POST /bulk-uploads/[uploadId]/start-address-matching` with the resulting IDs — into a single route. Task + SubTask creation moves inside `triggerAddressMatching` in `src/lib/bulkUpload/server.ts`. The generic `POST /api/tasks` endpoint is deleted; the `GET /api/tasks` listing (admin Tasks UI) stays.
|
||||
|
||||
The trade-off was between:
|
||||
|
||||
- **(rejected) Keep `POST /api/tasks` for future generic use.** No other feature creates Tasks today. By the deletion test, the generic endpoint wasn't earning its keep — every line was overhead carried for hypothetical callers, while the only real caller had to perform a leaky two-call dance to satisfy a contract that didn't reflect the domain.
|
||||
- **(chosen) Collapse Task creation into the BulkUpload module.** The seam matches the domain: BulkUpload owns its Task's lifecycle, including the moment of creation. One route handles the full transition into `processing`. The client surface (`useStartAddressMatching`) is one mutation, not a chain.
|
||||
|
||||
A future reader will see `/api/tasks` exposing GET-only and reasonably wonder where Tasks are created. The answer is: inside whichever feature owns the Task. Today that's BulkUpload only. If a second feature ever needs to create Tasks, re-extract a generic creator at that point — extracting from two real consumers is straightforward, whereas keeping a speculative endpoint live without consumers leaves a stale-by-default seam that drifts from how Tasks are actually created.
|
||||
|
|
@ -1,19 +0,0 @@
|
|||
# Bulk-upload pipeline advances via explicit Run Combiner / Finalise buttons
|
||||
|
||||
[ADR-0002](./0002-bulk-upload-browser-driven-orchestration.md) described the bulk-upload pipeline as **browser-driven and auto-firing**: a polling loop in `OnboardingProgress.tsx` watched the Task summary and fired `POST /combine` when the Task looked done, then `POST /finalize` when the upload reached `awaiting_review`, with mutable `combineFired` / `finalizeFired` / `refreshed` flags gating the side effects.
|
||||
|
||||
This decision keeps the pipeline browser-driven (per ADR-0002) but replaces the auto-fire behaviour with **explicit buttons** the Domna tech team clicks to advance each stage. The state machine in [ADR-0001](./0001-bulk-upload-state-machine.md) is unchanged.
|
||||
|
||||
Concretely:
|
||||
|
||||
- The progress screen polls a single `GET /bulk-uploads/[uploadId]/progress` snapshot (Task summary + BulkUpload row in one query).
|
||||
- "Run Combiner" appears when the Task reaches terminal-non-failed and the Combiner hasn't been triggered yet.
|
||||
- "Finalise" appears when the BulkUpload reaches `awaiting_review`.
|
||||
- Polling stops once the BulkUpload reaches a terminal status; on `useFinalize` success the caller runs `router.refresh()`.
|
||||
|
||||
The trade-off was between:
|
||||
|
||||
- **(rejected) Keep auto-fire and translate the orchestration to react-query with `useEffect`s.** Functional, but the auto-fire UX hides where the pipeline gets stuck — which is the failure mode the Domna tech team needs to debug. CLAUDE.md's "avoid `useEffect`" rule also gets stretched: three effects, one per side-effecting transition, each watching derived eligibility flags.
|
||||
- **(chosen) Explicit buttons.** The "fire once" mutable flags collapse into react-query mutation state (`isIdle` / `isPending` / `isSuccess`) — pure derivations from the cache, no `useEffect`. Each stage is observable, retryable, and reflects the current BulkUpload status directly. Tab close still leaves uploads stuck (browser-driven progression, per ADR-0002), but "stuck" is now legible: the team sees which button is pending.
|
||||
|
||||
When the bulk-upload redesign lands (per ADR-0002, "out of scope"), this entire screen and its buttons go away in favour of server-driven progression. Until then, the buttons are the right level of investment: they survive the redesign as discoverable artifacts of the prior flow, and they make the current flow tractable for the team using it.
|
||||
|
|
@ -1,14 +0,0 @@
|
|||
# Retire the `needs_review` BulkUpload status
|
||||
|
||||
[ADR-0001](./0001-bulk-upload-state-machine.md) preserved `needs_review` as a terminal status (deliberate decision #1): a finalised upload with missing or duplicate UPRNs ended up there, distinct from `complete`, even though no recovery flow existed. The reasoning was that the status flag still carried a UI signal — "Imported with issues" copy on the upload detail page — that told the team manual cleanup was needed in the property table.
|
||||
|
||||
This decision retires `needs_review` entirely. Finalise now always sets `complete`, regardless of whether any rows were missing a UPRN or shared one with another row. The status enum, the `markFinalized` signature, and the per-status UI copy are all simplified accordingly.
|
||||
|
||||
The terminal set is now `complete` and `failed` only. The state machine in ADR-0001 otherwise stands; only the post-Finalise branching changes.
|
||||
|
||||
The trade-off was between:
|
||||
|
||||
- **(rejected) Keep `needs_review` for the UI distinction.** The flag was the only consumer of the missing/duplicate-UPRN counts during Finalise, and the counts themselves were already dead in the response payload. Keeping the flag meant maintaining a state-machine branch and a STATUS_CONFIG entry for a signal that — per ADR-0001 — has no follow-up action. Future readers would continue to surface "needs_review has no exit" as friction.
|
||||
- **(chosen) Always `complete`.** One terminal success state. The Finalise route loses the missing/duplicate-UPRN booleans entirely; rows are inserted with whatever UPRN they have (or none), and the team handles cleanup in the property table directly. If a recovery flow ever lands, it can re-introduce a more meaningful intermediate status at that point — driven by the actual recovery UX rather than a defensive flag set today.
|
||||
|
||||
Legacy `bulk_address_uploads` rows with `status = 'needs_review'` (if any exist) will fall through `STATUS_CONFIG` lookup and render as the default `ready_for_processing` card. This is acceptable because (a) the flow is internal Domna tooling per ADR-0002, (b) any such rows pre-date this change and represent uploads the team has already triaged manually, and (c) the records still link to imported properties via the BulkUpload row's relationships. No migration is run.
|
||||
Loading…
Add table
Reference in a new issue