diff --git a/docs/design-system/adoption-manifest.json b/docs/design-system/adoption-manifest.json index 108814b979..79f580df32 100644 --- a/docs/design-system/adoption-manifest.json +++ b/docs/design-system/adoption-manifest.json @@ -1564,7 +1564,8 @@ "tests/ui-formulation.spec.ts", "tests/ui-smoke.spec.ts", "tests/ui-v2-components.dom.test.tsx", - "tests/ui-ward-coordinator.spec.ts" + "tests/ui-ward-coordinator.spec.ts", + "tests/ui-ward-roles.spec.ts" ], "baseline": { "targetLayer": "v2", diff --git a/docs/ward-flow-phase-3-handover.md b/docs/ward-flow-phase-3-handover.md index 10bf31096d..73f2d0e058 100644 --- a/docs/ward-flow-phase-3-handover.md +++ b/docs/ward-flow-phase-3-handover.md @@ -1,290 +1,252 @@ # Ward Flow Phase 3 — session handover -Refreshed 2026-08-22, with Tasks 1 to 6A complete and Tasks 7 to 12 outstanding, and the -branch now pushed to GitHub. Everything a fresh session needs to continue with full context. Read this -file first, then `docs/ward-flow-phase-3-ledger.md`, then the plan. +Rewritten 2026-08-23, at the end of session 3. **All twelve tasks are built.** A whole-branch review +found 2 Critical, 6 Important and 5 Minor; the Criticals and all six Importants are fixed. What +remains is reconciliation, verification, the `main` merge, and closing out. + +**Read this file first, then `docs/ward-flow-phase-3-ledger.md`, then +`docs/ward-flow-phase-3-rulings.md`.** + +--- + +## 1. STOP AND READ THIS FIRST — the branch is DIVERGED + +`git rev-list --left-right --count HEAD...@{u}` reports **8 local, 10 remote.** + +**A second Claude session worked on this same branch and pushed ten commits.** It independently fixed +the same two defects my agents were fixing. Both sides are real work. `git merge-tree` reports +**8 conflicting files**. + +### DO NOT + +- **Do not force-push.** This repo's own memory records work destroyed twice by cleanup of exactly + this kind. +- **Do not `git reset --hard`, and do not discard either side.** +- **Do not blindly `git pull`.** The merge needs a decision per hunk; see the plan below. +- **Do not `git checkout --` any file with uncommitted changes without backing it up first.** That + destroyed a completed fix round earlier in this phase. + +### The reconciliation plan (ruling R74, decided by reading both sides) + +| Concern | Take | Why | +| ----------------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **Live unit capacity** (Critical 1) | **Local (mine)** | Theirs is `eligibleCandidates(…, units = allUnits())` — a **defaulted parameter falling back to the frozen fixture**. The Global Constraints forbid "no defaulted-parameter equivalent", and Task 6's reviewer was assigned to hunt exactly that because "a default is how a frozen value creeps back". Mine requires `units`, adds a parser-based guard barring `allUnits()` outside an allow-list, and adds a permanent Playwright test. | +| **Expired bed holds** | **Remote (theirs), wholesale** | Spec line 201: _"An expiring hold raises an exception. It never auto-releases the bed."_ **My side never implemented it** — `bedHeldUntil` appears nowhere in `ward-derivations.ts` or `exception-drawer.tsx`. Their `a5716d832` / `1a6f163e0` add it to the action inbox. Missed by every task and by the whole-branch review. | +| **Form 1A expiry** | **Neither** | The product owner cancelled the whole category (ruling R73, below). Their `e8f9805fb` builds it; a local agent was mid-way through removing mine. | + +Their other commits (`57e33749d` attaching examination forms to non-voluntary referrals) overlap with +local finding I5 — compare before choosing. + +**Local commits are UNPUSHED** because the divergence blocks a fast-forward. They are safe in the +shared object store (worktrees share `D:\Repos\Database\.git`), but they are not on GitHub. + +--- + +## 2. Where the work is + +| What | Where | +| ------------ | ----------------------------------------------------------------- | +| Worktree | `C:\Users\joshs\.codex\worktrees\ward-management-design\Database` | +| Branch | `codex/ward-management-design` | +| **NOT** here | `D:\Repos\Database` contains none of this work | +| Dev server | `npm run ensure` prints the URL. **Never assume a port.** | + +--- + +## 3. What remains, in order + +1. **Finish removing the legal deadlines** (ruling R73). An agent was mid-task when this was written; + check `git status` for uncommitted work before re-dispatching — it may be complete and uncommitted. +2. **Reconcile the divergence** per the table above. Roughly an hour, eight files. +3. **Verify the last seven fixes independently.** Their test counts were taken on report and have + **not** been re-run by the controller: node-env **153**, Chromium **42**. Everything before + `2db08e045` was verified first-hand. +4. **Merge `origin/main`** — 568 commits behind, 33 conflicting files, mostly a squash-merge artefact + of this branch's own earlier work (PR #2140). The product owner decided this happens after the + phase (R35). **Bundle ruling R55's fix into it** — see §6. +5. **Handover / close out.** + +--- + +## 4. The clinical decisions that shaped this phase + +Four answers from the product owner, all of which changed the software. + +**"It is just counting how long they have been in ED determining priority. So counting up."** +Deleted a fabricated Form 3B statutory deadline that seven surfaces rendered as legal timing. + +**"For question 3… the reality is in ED that a patient needs review before they are referred for a +bed as they may not need a bed."** Produced the "Bed need confirmed" score factor: a patient +confirmed to need a bed now outranks one nobody has assessed. **Known gap, documented in code with no +invented proxy:** 21 of 41 open movements are voluntary, never receive a Mental Health Act +examination, and can therefore never earn it. + +**"change the 4 hour limit to 24 for patients in ED."** `ED_ACCESS_TARGET_MINUTES` is 1440. + +**"please can you leave the legal part and just start a clock once the patient arrives to ED. Keep +it simple for now."** — ruling **R73**, and the most important instruction of the session. + +### Why R73 matters more than it looks + +An agent invented a statutory figure ("72 hours… the real statutory figure under the WA Mental +Health Act 2014") from its own recollection. Asked about it, the owner gave the real two-part rule — +which then exposed that **fourteen of the fifteen Form 1A deadlines in the fixture were invented +numbers rendered as statutory timing, and all four "passed its deadline" warnings on the board were +fabricated breaches.** + +That is the Form 3B disaster repeated for Form 1A, undetected through the entire phase. Worse: +Task 6A's ruling F17 had deliberately re-pointed the breach assertion at "a genuinely breached 1A" — +those were not genuine either. **One fabrication was replaced with another and nobody measured the +second.** + +He then removed the whole category. **There is now no legal countdown in this model.** What remains +is time since arrival, counting up, and a departmental access target that has never claimed to be +legal. + +**The standing prohibition, absolute: no agent may cite, paraphrase or infer any figure from the +Mental Health Act.** If a legal quantity is needed, it comes from the product owner or it does not +exist. + +--- + +## 5. Still open with the product owner + +1. **Does the Form 1A countdown stay gone?** R73 said "for now". If real timeframes arrive, Task 6A's + precedent is that they return as an optional field plus one derivation — `LegalForm.dueAt` is + already optional and `operationalScore`'s "Statutory timing" factor is dormant, not deleted. +2. **Voluntary patients cannot evidence review at all**, so they can never earn "Bed need confirmed". + Closing it needs a general notion of "reviewed" the model does not have — the same work as gating + referral on review, which he raised and which was measured as out of scope (only 2 of 17 referable + movements are examined; 23 more already past that stage are not). +3. **Both 24-hour clocks now run from `openedAt`** — the departmental access target and, formerly, + the legal window. They will read the same length. The surfaces must keep saying which is which. + +--- + +## 6. Known and deliberately unfixed + +- **R55 — no Ward Flow browser spec has ever run in CI's Production UI lane.** `scripts/playwright-pr-shards.mjs` + holds its own copy of the spec pattern with **no ward alternation at all**, and the test whose job + is to catch that drift has been failing rather than being fixed. Pre-existing on `main`. Correcting + it needs hosted timing measurements unavailable locally — **bundle it with the `main` merge**, which + is the only point it can be verified. +- **Four repository tests fail and are pre-existing**: a stale hard-coded route count in + `design-system-adoption.test.ts` (expects 51, actual 61 — nine stale before this phase added one); + shard-matcher drift; a `contextual-back-navigation-contract.test.ts` offender in an untouched file; + and a Windows-environmental `session-start-hook` failure. +- **M1** — no dark, forced-colours or print coverage for any of the four new screens, which spec §14 + requires. +- **M2** — the voluntary-patient scoring gap is documented in a code comment where no user reads it. +- **M4** — `CONFIRM_CAPACITY` is role-gated but not unit-gated; the event carries no ward identity, so + it cannot be enforced today. +- **R62** — the tracker's `Accepted` and `Collected` badges render identically. +- **`TransportView` / `mode === "transport"`** is unreachable after Task 10, deliberately not deleted + (`AGENTS.md`: "nothing imports it" is insufficient grounds in this repository). --- -## 1. Where the work is - -**The work is NOT in `D:\Repos\Database`.** It lives in a separate Codex worktree: - -| What | Where | -| ---------------- | ----------------------------------------------------------------- | -| Worktree | `C:\Users\joshs\.codex\worktrees\ward-management-design\Database` | -| Branch | `codex/ward-management-design` | -| HEAD at handover | see `git rev-parse --short HEAD` — pushed and in sync with origin | -| State | Clean tree, **72 commits ahead of `origin/main`, pushed** | -| Dev server | `npm run ensure` → prints the URL. Never assume a port. | - -**The branch is now on GitHub** at `origin/codex/ward-management-design`, pushed 2026-08-22 at the -user's explicit request, which supersedes the earlier "no push" instruction. **No PR exists and -none should be opened** — that part of the instruction stands. The work is no longer only on this -machine. - -## 2. The documents that carry the context - -| Document | Committed | Purpose | -| ---------------------------------------------------------------------------- | --------- | ----------------------------------------------------------------------------------------- | -| `docs/superpowers/specs/2026-08-19-ward-flow-phase-3-role-screens-design.md` | yes | **The binding authority.** 19 sections. Every conflict resolves against this. | -| `docs/superpowers/plans/2026-08-19-ward-flow-phase-3-role-screens.md` | yes | 12 tasks with the exact test code for each. Corrected in several places during execution. | -| `docs/ward-flow-phase-3-ledger.md` | yes | The execution record: every task's commits, every ruling, what was verified and how. | -| `docs/ward-flow-context.md` | yes | Cold-start orientation for the whole project, Phases 1 to 3. | - -**The live ledger is `.superpowers/sdd/2026-08-19-ward-flow-phase-3-role-screens/progress.md`, -and that whole directory is gitignored.** It does not travel with a clone or a push. -`docs/ward-flow-phase-3-ledger.md` is a committed copy refreshed at each handover. A continuing -session keeps appending to the `.superpowers` copy — the skill's scripts expect it there — and -re-copies to `docs/` at the next handover. - -That directory also holds, all gitignored and all verified present at this handover: a brief for -every task (1 to 6, 6A, 7 to 12), implementer reports for 1 to 6 and 6A, reviews and re-reviews, a -Task 7 controller addendum, and eleven review diffs. Roughly 800 KB. Useful but reconstructible — -briefs regenerate from the plan with `scripts/task-brief`, diffs regenerate from git. - -## 3. State — what is done and what is next - -**Tasks 1 to 6A are complete**: implemented, reviewed, every finding fixed, every fix verified -independently by the controller rather than accepted on report. - -| Task | State | Commits | -| -------------------------------- | --------------------------------- | --------------------------------------------- | -| 1 — model and fixture | complete | `f3b1f74f0`, `39042cd61`, `2d59219d0` | -| 2 — the reducer | complete | `3b76b093e`, `e7faa7b5a` | -| 3 — the contracts | complete | `f01a4f8f3`, `cbdd47f71` (+ plan `e2b72a300`) | -| 4 — provider, clock, layout | complete | `0612fdfa0`, `9ae334230` | -| 5 — coordinator rewire | complete | `4d36099ca`, `868853b58` | -| 6 — the other nine routes | complete, **5 fix rounds** | `af90428ce` … `f4963f28a` | -| 6A — the ED clock counts up | complete, 2 fix rounds | `2d8200a09`, `f1e32dcd4`, `496039d87` | -| 7 — coordinator phone pin | **next — brief + addendum ready** | — | -| 8 — the ward screen | not started | — | -| 9 — transport officer phone | not started | — | -| 10 — live tracker | not started | — | -| 11 — emergency department screen | not started | — | -| 12 — role switcher + journey | not started | — | - -**Task 7 is dispatch-ready.** Its brief was written before Tasks 6 and 6A landed and its test code -no longer holds, so a controller addendum at `.superpowers/sdd/.../task-7-addendum.md` carries -four corrections (rulings R24 to R27) and **takes precedence over the brief where they differ**. -Read both. - -## 4. The clinical question that was answered, and what it changed - -The phase carried a standing open question: when a patient is examined in the emergency department -and ordered to an inpatient bed, what should the on-screen countdown represent? - -**The clinician answered, verbatim:** - -> "It is just counting how long they have been in ED determining priority. So counting up." - -So there is **no post-examination deadline**. The prototype had been rendering one — a Form 3B -carrying a `dueAt` derived from `examination.at + 240`, which seven surfaces displayed as statutory -timing and counted as a legal breach. That is the worst defect class this project has: a clinical -surface stating something the law does not impose. - -Task 6A deleted it. Two things must be carried forward: - -- **The four hours were real; they were attached to the wrong quantity.** Spec §7 requires the - **emergency department access target** — a departmental performance measure counted up from - `openedAt`, "the number a department is judged on, and mental health patients are its largest - breachers". It now lives as `ED_ACCESS_TARGET_MINUTES`, quarantined from `LegalForm`. - **Task 11 is its only consumer and must render it against `openedAt`, never as a legal clock.** -- **`LegalForm.dueAt` is now optional.** A Form 1A always carries its statutory examination window; - a Form 3B carries none. Absence is rendered explicitly everywhere, and an absent `dueAt` reaching - arithmetic is now a **compile error**, not merely a convention. - -Confirmed on the running app: the coordinator page contains the string "3B" zero times and "due in" -zero times, and all four remaining breach lines are genuine Form 1A examination breaches. -Screenshot: `artifacts/ward-management/phase3-6a-coordinator.png`. - -## 5. Open with the user — three assumptions, none confirmed - -State these rather than bury them. Each was put to the user; none has been answered. - -1. **The Form 1A countdown stays a countdown.** His answer was scoped to the post-examination case. - The pre-examination examination window is still modelled as a deadline. -2. **Being detained and examined confers no priority bonus of its own.** Such a patient's priority - now rides purely on elapsed time, exactly as he described. The practical effect is that patients - still awaiting examination tend to rank above them. -3. **Four hours is the correct ED access target for WA metro.** It is the national figure; it has - not been confirmed for this context. One constant, one screen — trivial to change. - -A fourth, raised but not decided: **the demo now leads with an accident.** The top of the -coordinator queue is `WF-303`, a _generated_ movement whose breach comes from -`NOW_ANCHOR + (((index * 53) % 400) - 60)` in `routineMovements` — arithmetic, not authorship. -Nothing in the fixture says so, and Task 12's guided journey may walk a user straight into it. -Worth deciding whether the demo should lead with a deliberately authored case. - -## 6. How the user wants this run - -Standing instructions, carried from Phase 2 and reaffirmed this session. They are not optional and -they are why this phase has found what it has found. - -- **Verify every claim a subagent makes.** Re-run the typecheck and the suites yourself after every - task. Never accept a pasted number. Phase 2's worst defects all passed their own tests, and this - session caught an implementer report asserting a fixture check it had never run. -- **Mutation-test; do not trust green.** For each new test, make the single change that should kill - it, **print the edited line back from the file**, run, watch it fail, revert. Mutations have - silently failed to apply repeatedly; each near-miss nearly became a recorded false negative. +## 7. How this session was run, and why + +Standing instructions from the product owner. They are not optional and they are why this phase found +what it found. + +- **Verify every claim a subagent makes.** Re-run its gates yourself. Never accept a pasted number. +- **Mutation-test every test**: make the change that should kill it, **print the edited line back from + the file**, run, watch it fail, revert. _A mutation you did not read back did not happen._ - **Read gate output, never exit codes.** Several commands here exit 0 without running. -- **Run the browser gate after any task touching the fixture, the reducer, or a screen** — not only - screen tasks. -- **Send the user screenshots on every screen task** and say what to look at. Look at the screen - yourself, not only at test output. -- **Do not run `verify:ui`, `verify:release`, or any provider-backed gate.** -- **Rule, do not stall.** Decide ambiguities and plan defects yourself, record each ruling with what - it costs if wrong, and keep going. Stop only for something irreversible, destructive, - security-sensitive, or a plan so broken every path forward is a guess. **The full ruling list is - owed to the user at the end of the phase.** -- **Work in this worktree on this branch. No new branch, no PR.** Pushing to - `origin/codex/ward-management-design` is now authorised — but read the push warning in §8 first. -- **Talk to the user in plain English.** He is a psychiatrist, not a software engineer. Lead with - the answer, numbered steps for anything he must do, no jargon, no file paths unless they change - his decision. Full style rules in `C:\Users\joshs\.claude\CLAUDE.md`. - -## 7. Verification — the current baselines - -Measured by the controller on 2026-08-22 at HEAD, after a full `npm ci --include=dev`, not taken -from a report: - -| Gate | Result | -| ---------------------------------------------- | ------------------------ | -| `npx tsc --noEmit -p tsconfig.json` | **clean** | -| Node-environment suites (10 files) | **118 passed** | -| jsdom suites (3 files, **one per invocation**) | **6 passed** (1 + 4 + 1) | -| Ward Chromium journeys | **24 passed** | +- **Look at the screen.** Three of this session's defects were found by looking at a screenshot and + none by a test. +- **Rule, do not stall.** 74 rulings are recorded with what each costs if wrong. + +**Eleven implementers volunteered a surviving mutation this session.** Two were genuinely untestable +assertions; nine were mistimed mutations correctly diagnosed. _A mistimed mutation and an untestable +assertion look identical from outside — only the diagnosis separates them._ + +--- + +## 8. Environment traps — every one hit for real + +- **`node_modules` empties to zero.** The symptom is **not** a dependency error: it is `tsc` unable to + find `process` and most test files failing at once. **Run `ls node_modules | wc -l` before debugging + anything broad.** Recovery: `npm ci --include=dev`, about 7 minutes. Cause is ambient to the machine + (another worktree's push guard borrowing this one's tree), not this branch. +- **The dev server is reaped when the shell that launched it exits.** Start it as a **backgrounded + task**, never `nohup … & disown`. **Prove liveness with Node, not `curl`** — the Playwright config + guard uses Node, and `curl` succeeding against a dead server sends you hunting an IPv6 problem that + does not exist. +- **`npm run lint` exits 0 without running** when the repo lock is held, printing + `DATABASE_HEAVY_RUN_ADMISSION_BUSY`, and has also failed with `EPERM … owner.json`. **A real pass + echoes the inner `lint:internal` eslint command with no busy marker.** +- **`git push` can succeed with its own static gate not run**, for the same reason. Read the output. +- **Vitest reports `Test Files no tests` at exit 0** under load — six recorded occurrences. **The count + is the evidence, never the word "passed".** Run jsdom files one per invocation. +- **A bare `npx playwright test` is rejected by a config guard while still looking like it ran.** + Always pass `PLAYWRIGHT_BASE_URL`. +- **Warm every route with `curl` before Playwright.** `cpus: 1` means a first compile can exceed + Playwright's waits and produce a failure identical to a regression. +- **The Browser pane cannot composite frames here.** Drive headless Chromium directly; put the script + inside the repo so it resolves `playwright`. +- **`.next/dev/types/validator.ts` goes corrupt** and turns `tsc` red for no source reason. Delete it. +- **`git commit` can exceed two minutes** on the pre-commit docs hook. +- **`npm run format` can hang.** Use `npx prettier --write `. +- **Do not use the Monitor tool for gates.** Events routinely never fire; five agents stalled on them. + +--- + +## 9. Verification baselines + +Measured first-hand by the controller at `421d9a666`: + +| Gate | Result | +| ---------------------------------------------- | ------------------------------------------------------------------------------------------------------- | +| `npx tsc --noEmit -p tsconfig.json` | clean | +| Node-env ward suites, 11 files, one invocation | **152 passed** | +| jsdom, one file per invocation | `ward-screen` 3, `ward-flow-clock-consistency` 1, `ward-flow-provider` 4, `ward-flow-queue-selection` 1 | +| Ward Chromium, chromium only, 3 spec files | **40 passed** | +| `npm run lint` | genuine pass | + +**Not yet verified first-hand:** the six commits after `421d9a666` report node-env **153** and +Chromium **42**. Re-run before trusting. ```bash -npx vitest run tests/ward-flow-reducer.test.ts tests/ward-flow-contracts.test.ts tests/ward-model-phase3.test.ts tests/ward-model.test.ts tests/ward-flow-single-source.test.ts tests/ward-clock.test.ts tests/ward-priority.test.ts tests/ward-pressure.test.ts tests/ward-derivations.test.ts tests/ward-management.test.ts +npx vitest run tests/ward-flow-reducer.test.ts tests/ward-flow-contracts.test.ts tests/ward-model-phase3.test.ts tests/ward-model.test.ts tests/ward-flow-single-source.test.ts tests/ward-clock.test.ts tests/ward-priority.test.ts tests/ward-pressure.test.ts tests/ward-derivations.test.ts tests/ward-management.test.ts tests/tracker-derivations.test.ts ``` ```bash -PLAYWRIGHT_BASE_URL= npx playwright test tests/ui-ward-coordinator.spec.ts tests/ui-ward-management.spec.ts --project=chromium --reporter=line +PLAYWRIGHT_BASE_URL= npx playwright test tests/ui-ward-coordinator.spec.ts tests/ui-ward-management.spec.ts tests/ui-ward-roles.spec.ts --project=chromium --reporter=line ``` -**The browser gate is verified at HEAD: 24 passed (3.9 min).** It took two runs to get there -and the first one is worth knowing about. After a fresh `npm ci` the dev server rebuilds cold, and -this project's dev config pins `cpus: 1`, so it took **~16 minutes** to become ready — far longer -than `npm run ensure` waits before declaring failure. The first gate run then came back -**23 passed, 1 failed**, and the failure was `ui-ward-management.spec.ts:54 "opens every Ward Flow -mode"` — the one test that visits every route, so the one paying first-compile cost on each. - -It was confirmed as a cold-start artefact rather than assumed: re-run alone it passed in 43 s, and -the full gate re-run warm passed **24/24**. If you see that single test fail on a cold server, -warm the routes and re-run before believing it. - -**Do NOT run `npx vitest run tests/guard-push.test.ts`** — see the first environment trap below. - -## 8. Environment traps — all hit for real, all cost time - -- **`node_modules` gets emptied to ZERO entries — twice on 2026-08-22, cause UNCONFIRMED.** - This is the most disruptive thing on this machine and the most deceptive. **The symptom is not a - dependency error.** It is `tsc` reporting it cannot find `process`, and 8 of 10 unrelated test - files failing at once — which reads exactly like a code regression and is not one. Recovery is - `npm ci --include=dev`, roughly 7 minutes. - - **First check when any broad, unexplained failure appears: `ls node_modules | wc -l`.** Before - reading a single line of code. - - What correlates, from two occurrences: the first followed `git push`; the second followed - `npx vitest run tests/guard-push.test.ts`. Both exercise `scripts/guard-push.mjs`, whose format - guard links a real dependency tree into a scratch checkout as a Windows junction and then - force-deletes the checkout. **Treat both as unsafe on this branch. Do not run the guard-push - test suite.** The ward suites, `tsc`, and the browser gate were all re-run afterwards and left - `node_modules` intact at 523 entries, so they are not implicated. - - **The mechanism is a hypothesis, not a finding, and an earlier draft of this file wrongly stated - it as fact.** Both candidate force-deletes were probed directly here and **neither destroyed the - junction's target**: Node's recursive `rmSync` did not follow the junction, nor did - `git worktree remove --force`. A live alternative is that `findPrettierBin` deliberately borrows - _another_ worktree's tree when this checkout has none, and this repo has dozens of siblings. - - **Strongest explanation, found 2026-08-22 by inspecting what was actually running.** At the - moment of writing, PID 22400 was `D:/Worktrees/Database/care-plan-impl/scripts/guard-push.mjs` - — **a different worktree running its own push guard.** `findPrettierBin` in that script - deliberately borrows _another_ worktree's real `node_modules` when its own checkout has none, - and this machine has dozens of sibling worktrees. That fits every fact the earlier theory could - not: why isolated probes here never reproduced the damage (the destroyer was in another - worktree), and why it happened a second time when this session did not push at all. - - **The practical consequence: any session pushing from any worktree can empty this one's - `node_modules`.** It is not something this branch can fully defend against alone, and it is not - caused by anything in the ward-flow code. Treat it as ambient, check for it early, and recover - with `npm ci --include=dev`. - - **Do not hand-write a fix for this.** One was written and reverted: a mutation reintroducing the - supposed bug on purpose failed no test at all, proving the fix was untestable against an unknown - mechanism. The reviewed fix exists upstream at `a04330ea0` (PR #2244), which is **not** an - ancestor of this branch. Bringing `main` in is the correct route and is the user's call. - -- **The vitest worker pool is unreliable under load.** Six recorded occurrences of a run reporting - `Test Files no tests / Tests no tests`, or a truncated file count, **at exit code 0**, on suites - that pass on an immediate re-run. `VITEST_MAX_WORKERS=1` does not help. **Run jsdom - `.dom.test.tsx` files one per invocation and read the counts.** The count is the evidence, never - the word "passed". This is the most dangerous trap here: it makes a check that never ran look - like a check that passed. -- **Do not mix jsdom and node-environment suites in one invocation.** Two of seven files silently - fail to start. -- **The Browser pane cannot composite frames in this environment.** Three agents and the controller - hit it independently; `mcp__Claude_Browser__` screenshots time out. **Drive headless Chromium - directly instead** — place the script inside the repo (`artifacts/` is gitignored) so it resolves - `playwright`. The working recipe is in `task-7-addendum.md` §R27. -- **`.next/dev/types/validator.ts` goes corrupt** when the dev server is killed mid-write, turning - `tsc` red for no source reason. Delete it and re-run. -- **`npm run ensure` can time out on a cold start under load** — the server took 94 s to become - ready once. Check `dev-server.log` for `✓ Ready` before assuming it failed. -- **`git commit` can exceed two minutes** on the pre-commit documentation-sync hook. Retry with a - longer timeout rather than assuming a lock. -- **`npm run lint` exits 0 without running** when the repo lock is held; it prints - `DATABASE_HEAVY_RUN_ADMISSION_BUSY`. -- **`npm run format` can hang for minutes.** Use `npx prettier --write `. -- **A Playwright `page.goto()` re-mounts the provider and resets all state.** Any test asserting the - effect of a dispatch must stay on the same page and navigate by clicking. -- **Never `git checkout --` a file with uncommitted changes without backing it up first.** That - destroyed a completed fix round this session; it was recovered only because the diff was still in - context and could be re-applied as a git-verified patch. -- **The machine can run out of memory after hours of agent work.** Measured at this handover: - 1.3 GB free of 32 GB, with 45 `claude` processes holding 7 GB. The symptom is not an error — it - is everything getting slow, then the dev server failing to become ready, then a Playwright run - aborting as `N did not run` at exit 0. If gates start behaving strangely late in a session, - check free memory before debugging the code. -- **A hard usage ceiling killed two agents mid-work.** Both left recoverable state. If an agent - dies, inspect the working tree before re-dispatching — the work may be complete and uncommitted. - -## 9. What has repeatedly gone wrong, and the lesson - -Three of Task 6's five fix rounds went to a single static guard — the one asserting that no screen -reads the frozen epoch `NOW_ANCHOR` instead of the live clock. It overclaimed in three successive -forms, each found by someone deliberately trying to defeat it rather than by running it: - -1. **Co-occurrence scoping** — flagged a file only if it both called `useWardFlow()` and - named-imported `NOW_ANCHOR`. Helper indirection, namespace imports, and any component outside - the rule all walked past. -2. **Directory scoping** — rebuilt as a named allow-list, but walking one directory while its test - name claimed "every read". -3. **A hand-rolled scanner** — stripped comments and strings character by character with no concept - of a regex literal, so a quote inside a regex desynced it and blinded it to the rest of the file. - -It is now a substring pre-filter plus a real TypeScript-parser walk. The same pattern recurred a -fourth time with the `ED_ACCESS_TARGET_MINUTES` quarantine, where the answer was different: -**narrow the claim rather than chase completeness**, and move real enforcement into the brief and -review for the one task that will exercise it. - -**The lesson, and it is the phase's most important finding: a check that claims more than it -delivers is worse than no check, because it stops anyone looking harder.** Name a guard for what it -actually does. - -## 10. Resuming — the opening move - -1. Confirm you are in `C:\Users\joshs\.codex\worktrees\ward-management-design\Database` on - `codex/ward-management-design`, that `git status` is clean, and that local and `origin/codex/ward-management-design` agree. -2. Read `docs/ward-flow-phase-3-ledger.md` in full. It is the recovery map — the commits it names - exist in git even when nothing else remembers them. -3. Invoke `superpowers:subagent-driven-development` with the plan file. It will find the live ledger - and resume. **Do not re-dispatch Tasks 1 to 6A**, which are recorded complete. -4. Dispatch **Task 7** from its brief **plus the addendum**, verify it yourself, send the user the - phone screenshot, then continue through Tasks 8 to 12. -5. **Before dispatching each remaining task, scan its brief against the branch as it now stands.** - Every brief was extracted before Tasks 6 and 6A landed. Task 7's needed four corrections; - Task 11's needed one that would otherwise have deleted a feature the spec requires. +**Never run** `verify:ui`, `verify:release`, `eval:*`, `check:supabase-project`, `test:live`, or +anything touching OpenAI, Supabase, GitHub Actions or the live database. + +--- + +## 10. Where everything is + +| Need | File | +| -------------------------------------------- | ---------------------------------------------------------------------------- | +| **State and how to resume** | this file | +| Execution record, every ruling in full | `docs/ward-flow-phase-3-ledger.md` | +| **All 74 rulings, owner-facing, with costs** | `docs/ward-flow-phase-3-rulings.md` | +| The whole-branch review | `docs/ward-flow-phase-3-workspace/whole-branch-review.md` | +| Briefs, reports, addenda | `docs/ward-flow-phase-3-workspace/` | +| Binding authority | `docs/superpowers/specs/2026-08-19-ward-flow-phase-3-role-screens-design.md` | +| The 12-task plan | `docs/superpowers/plans/2026-08-19-ward-flow-phase-3-role-screens.md` | +| Cross-phase map, Phases 1–3 | `docs/ward-flow-complete-ledger.md` | + +The live workspace at `.superpowers/sdd/2026-08-19-ward-flow-phase-3-role-screens/` is **gitignored** +and does not survive a clone. `docs/ward-flow-phase-3-workspace/` is the committed copy — refresh it +before any handover. + +--- + +## 11. The lesson, stated once + +**A check that claims more than it delivers is worse than no check, because it stops anyone looking +harder.** It has appeared in every phase in a different disguise: a privacy guard that read no +strings; three unfalsifiable tests; a guard whose loops ran zero times; a scanner blinded by a quote +inside a regex; a scroll assertion comparing zero to zero; a vacuity tripwire counting iterations +instead of matches; and a fixture invariant derived correctly and then only half written down. + +Every one was found by someone deliberately trying to defeat the check, by measuring the data, or by +looking at the screen. **Not one was found by running the suite.** diff --git a/docs/ward-flow-phase-3-workspace/whole-branch-review.md b/docs/ward-flow-phase-3-workspace/whole-branch-review.md new file mode 100644 index 0000000000..d3d0f87f7a --- /dev/null +++ b/docs/ward-flow-phase-3-workspace/whole-branch-review.md @@ -0,0 +1,458 @@ +# Ward Flow Phase 3 — whole-branch review + +**Reviewer:** independent whole-branch pass at `916816089`, branch `codex/ward-management-design`. +**Scope:** the 47-commit diff `cf751504f..916816089`, the design spec (19 sections), the plan +(12 tasks), the ledger's rulings, and the per-task reports. +**Method:** read the diff and the spec first, then targeted live probes against the running dev +server at `http://localhost:3718` (headless Chromium driving the real app, clicking real controls), +plus `tsx` probes over the real fixture. No file was modified; scratch scripts lived in the +gitignored `artifacts/` and were deleted. `git status --porcelain` clean before and after. + +**Counts: 2 Critical, 6 Important, 5 Minor.** + +--- + +## Critical + +### C1 — Unit state is live in the reducer, but every capacity figure a decision is made on is read from the frozen fixture + +**In this phase's diff.** Root cause specified in `docs/ward-flow-phase-3-workspace/task-8-brief.md:12,66`. + +Spec §4 is explicit that this is the single correction that most changes the phase: _"Units are in +the state… a ward could accept a patient, hold a bed and receive them while its free-bed count never +moved… frozen capacity makes the primary screen less true the more it is used."_ Spec §7 repeats it: +_"It reads live state. Every figure moves when another role acts, including bed counts."_ + +The reducer does hold units in state (`ward-flow-reducer.ts:26,39`) and mutates them correctly. +But three of the surfaces that read unit capacity never touch that state: + +- `src/components/ward-management/ward/ward-screen.tsx:71-72` — destructures only + `{ movements, now, dispatch }` from `useWardFlow()` and then `const unit = unitById(unitId)`, + i.e. `ward-sites.ts`'s frozen module constant. Every figure on the ward's own screen — the bed + grid, the "Currently confirmed N at HH:MM" line (`:201`), the initial value of the capacity input + (`:78`) and `holdBlockedReason`'s `unit.allocatable.value <= 0` check (`:49`) — comes from it. +- `src/components/ward-management/ward-derivations.ts:269` — `eligibleCandidates()` builds its + candidate list from `allUnits()`, so **every eligibility verdict on the coordinator's shortlist + and flow diagram is computed against frozen capacity**. `ward-eligibility.ts` reads + `unit.allocatable.value`, `unit.allocatable.confirmedAt` and `unit.sexMix` — all three are + reducer-mutated fields. +- `ward-management-network.tsx:225` and `ward-management-modes.tsx:165` — frozen unit detail panels + (lower consequence: no control attached). + +`officer-screen.tsx:56-58` gets this right and its comment names the exact failure mode +("never `unitById` from `ward-sites.ts`, which reads the frozen fixture"). `live-tracker.tsx` and +`CapacityView` also use live units. So the application now holds two different truths about the +same ward at the same instant. + +**Proof 1 — the ward's own screen ignores the ward's own action.** On +`/ward-management/ward/bty-adult-secure`, typing `0` into "Confirm allocatable beds" and clicking +"Confirm capacity": + +``` +--- seed + beds: Ready 2 | Held 0 | Blocked 1 | Occupied 14 | Potential 1 + Currently confirmed 2 at 10:28. +--- after CONFIRM_CAPACITY = 0 + beds: Ready 2 | Held 0 | Blocked 1 | Occupied 14 | Potential 1 + Currently confirmed 2 at 10:28. +``` + +The reducer accepted the event (no rejection is filed for a valid unit id and the `ward` role). +Nothing on the screen that raised it moves. Reproduced identically on `fre-adult-open`. + +**Proof 2 — a control advertises an action the reducer refuses.** Same session, on +`/ward-management/ward/rph-adult-secure` (fixture `allocatable = 1`), after confirming `0`: + +``` +hold button ward-hold-WF-003: aria-disabled = null, title = null <- fully live +card text after clicking Hold: "WF-003 / Accepted, awaiting bed / Hold a bed" <- nothing happened +``` + +Then clicking through the rail to the coordinator (no `goto`, provider intact) and opening the +exceptions drawer: + +``` +Refused actions +HOLD_BED +no allocatable bed remains at RPH Adult Secure (bed_held_for_earlier_referral) +10:42 +``` + +This is the phase's own headline defect recurring in a new disguise: the ward reports nothing while +the reducer has refused. `holdBlockedReason` was written specifically to prevent this +(`ward-screen.tsx:43-45`: _"named so the Hold button can never advertise an action the reducer would +refuse"_) and fails because it reads the wrong `unit`. + +**Proof 3 — the coordinator's primary decision surface contradicts the ward.** Same session, after +RPH Adult Secure confirmed `0`, selecting WF-012 on the coordinator queue: + +``` +RPH Adult Secure / Ready 1 · Held 1 · Blocked 0 · Occupied 18 / Eligible now + Allocatable bed Met 1 allocatable + Capacity freshness Met Confirmed 20 min ago +``` + +The ward said "no beds" seconds earlier. The explainable shortlist — the screen the spec calls +"where the placement decision is actually made" — says one bed is allocatable, that the ward +confirmed it 20 minutes ago, and that the destination is "Eligible now". A coordinator would refer a +patient there. `REFER_TO_UNITS` carries no capacity precondition, so the referral succeeds; the +refusal only surfaces later, at the ward's Hold. + +**Fix shape:** thread the provider's `units` into `WardScreen` and give `eligibleCandidates` and +`unitCapacity`'s callers an injected unit list, the same way `queueOrder` / `edPressure` / +`buildActionInbox` already take injected movements (spec §1 names that as the Phase 2 decision that +pays off here). Then extend the single-source guard (see I1) so it cannot regress. + +--- + +### C2 — WF-012 holds a live referral the receiving ward can never see or answer + +**Fixture data is pre-existing on `cf751504f`; the harm and the missed invariant are in this diff.** + +`src/components/ward-management/ward-movements.ts:347-349`: + +```ts +stage: "placement_requested", +referredUnitIds: ["gry-adult-secure"], +``` + +`RAISE_REFERRAL` is the only reducer branch that produces `placement_requested`, and it writes +`referredUnitIds: []` (`ward-flow-reducer.ts:126-142`). No branch ever returns a movement to that +stage. So this combination is unreachable — and it is unreachable in a way three Phase 3 surfaces +now disagree about: + +- Coordinator shortlist for WF-012: **"Parallel referral: Graylands Adult Secure"** +- ED screen `/ward-management/ed/rgh-ed`, WF-012's card: **"Referred to 1 unit"** +- `/ward-management/ward/gry-adult-secure`: no incoming card, and **the string "WF-012" does not + appear anywhere on the page** — `ward-screen.tsx:104-107` filters `incoming` on + `stage === "destination_review"`. + +Failure scenario: a coordinator sees WF-012 out at Graylands and waits for an answer. Graylands has +no card to accept or decline. Nothing in the system will ever resolve it, and no refusal is recorded +because nobody can raise an event to be refused. Spec §16 names this precisely — _"Four screens +describing one patient… every defect this project has found is a surface stating something the data +does not support."_ + +**Why the R63 invariant did not catch it.** The R63/R64 table in +`tests/ward-flow-contracts.test.ts:196-200` is genuinely exhaustive — its `RAISE_REFERRAL` row +records the write as `referredUnitIds: []`, `declines: []`, `withdrawnReferrals: []`. But no test +asserts any of those three implications. The invariant block asserts `handover_ready ⇒ transport`, +`{post-acceptance stages} ⇒ acceptedUnitId`, `bed_held ⇒ bedHeldUntil`, `moving ⇒ collectedAt`, +`arrived + transport ⇒ arrivedAt`, stamp ordering, and `pre-acceptance ⇒ no acceptedUnitId`. The +table was derived correctly and then only partly discharged. This is the third instance of the class +and the brief predicted it. + +I enumerated the reducer's eight stage-producing branches independently and measured the fixture +against every implication. Two violations exist, both from the un-asserted `RAISE_REFERRAL` row: +WF-012 here, and WF-018 (see I6). Everything else — transport before handover, `bedHeldUntil` before +`bed_held`, `arrived` without `closure`, cap overruns, declined-unit-still-referred, accepted-unit- +also-declined, 1A-with-examination, 3B-without, 1A-without-`dueAt`, 3B-with-`dueAt`, non-voluntary +without a form — measured clean. + +--- + +## Important + +### I1 — The "one source of truth" guard covers only half the frozen fixture + +**In this phase's diff.** `tests/ward-flow-single-source.test.ts:349-356`. + +The test is named `has no component reading the frozen fixture directly` inside a describe block +named `one source of truth`, and its `ALLOWED` comment says _"Everything else must read the +provider, or two surfaces will disagree."_ Its actual predicate is: + +```ts +.filter(({ source }) => /from "[^"]*ward-movements"/.test(source)) +``` + +It matches one module. `ward-sites.ts` — which exports `allUnits()`, `unitById()` and the entire +frozen unit fixture, i.e. the half of the state that spec §4 calls "the correction that most changes +the phase" — is not covered, is not in `ALLOWED`, and is imported freely by eight ward-management +components. This is the guard that would have caught C1, under a name that says it did. + +What one change to the product would make it fail? Only re-adding a `ward-movements` import. A +component switching from live `units` back to `allUnits()` — the exact regression it exists to +prevent — passes silently. Widening the pattern to `ward-(movements|sites)` with an explicit +allow-list for identity-only lookups (`edById`, `siteByCode`, `NOW_ANCHOR`) makes the name true. + +### I2 — Spec §11 escalation was never wired to a control, and the plan records it as delivered + +**In this phase's diff.** + +`RECORD_ESCALATION` exists in the events union, the `EVENT_ROLE` table and the reducer, and is +covered by a unit test. `grep -rn "RECORD_ESCALATION" src/` returns **only** `ward-flow-events.ts` +and `ward-flow-reducer.ts`. No surface anywhere dispatches it. + +Spec §11 is not an optional flourish: it was _moved into Phase 3 from Phase 4_ and given its own +section, on the reasoning that _"a phase that only proves the loop which succeeds has not proved the +loop."_ The plan's own spec-coverage line +(`docs/superpowers/plans/2026-08-19-ward-flow-phase-3-role-screens.md:1544`) states: _"§11 escalation +→ Task 5's `RECORD_ESCALATION` dispatch and the shortlist's existing no-eligible-destination +state."_ That dispatch does not exist. A reader auditing spec coverage from the plan would conclude +§11 shipped. + +### I3 — The jump-forward clock control and scenario reset were never built + +**In this phase's diff.** + +Spec §2 decision 5 ("Does the clock move? **Yes**, with a jump-forward control") and §5 ("+15 min, ++1 hour… so a held bed can be watched expiring in seconds rather than in an hour") are settled +product decisions. `ADVANCE_CLOCK` and `RESET_SCENARIO` are implemented and tested in the reducer, +and dispatched **only from test harness buttons**. Several comments refer to "Task 12's demo +controls" (`ward-flow-clock-consistency.dom.test.tsx:21,38`, and the ledger's F10 ruling) as though +they exist. + +Consequence for the person this is being handed to: `bedHeldUntil` is `now + 60`, so a bed hold +cannot be watched expiring at all, and the one thing spec §5 says the control is for cannot be +demonstrated. + +### I4 — A refusal changes nothing visible on any screen by default + +**In this phase's diff.** `coordinator-screen.tsx:65`, `exception-drawer.tsx:47-51`. + +Spec §7.4: _"It owns the refusals surface. Every transition the reducer refuses appears here, in the +exceptions drawer, **persistently** — not in a toast that vanishes."_ + +The drawer is `useState(false)` — closed on load — and its collapsed trigger renders +`{items.length}`, the action-inbox count only. Refusals are not counted there and appear nowhere +else. In Proof 2 above, the `HOLD_BED` refusal was invisible until I explicitly clicked "Exceptions" +open; the badge did not move. A closed drawer whose badge excludes refusals is functionally the +toast the spec forbids: the record persists, but nothing ever tells anyone to look for it. + +Smallest honest fix: include `rejections.length` in the collapsed badge (or add a second badge), so +the count moves the moment something is refused. + +### I5 — Every referral the ED raises is born unable to have its examination recorded + +**In this phase's diff.** `ward-flow-reducer.ts:126-142` vs `:145-152`. + +`RAISE_REFERRAL` creates the movement with no `legalForm`, whatever `legalStatus` the draft carries. +`RECORD_EXAMINATION` refuses unless `legalForm?.code === "1A"`. The ED's two spec-§6 actions +therefore cannot be used in sequence on a patient that ED raised itself. + +Measured live on `/ward-management/ed/peel-ed`, raising a referral with legal status "Referred for +psychiatric examination": + +``` +WF-901 · Adult · Open · Female · Referred for psychiatric examination +… Placement requested +Form — No legal form recorded for this movement. +[Record examination] aria-disabled=true + title: "WF-901 cannot have an examination recorded while its form is none, not 1A." +``` + +One card asserts the patient _has been referred for psychiatric examination_ and, three lines later, +that no legal form is recorded and the examination can never be recorded. Spec §3 defines Form 1A as +exactly "the person has been referred and not yet examined" — the status and the missing form are +the same fact, disagreeing. All 48 fixture movements honour "non-voluntary ⇒ carries a `legalForm`" +(measured); the only runtime creator of movements breaks it. + +This also undercuts ruling R67 ("a patient is reviewed before a bed is sought"), which the +end-to-end journey makes its first step: a freshly raised patient can never be reviewed. + +The control is correctly disabled with a stated reason, which is why this is Important and not +Critical. The fix is small: derive the 1A in `RAISE_REFERRAL` from a non-voluntary `legalStatus`, or +drop the detained statuses from the draft form. + +### I6 — WF-018 tells a ward its referral was withdrawn, for a referral that never existed + +**In this phase's diff** (the `withdrawnReferrals` entry was added by the Task 1 fixture commit). + +`ward-movements.ts:492-503` — WF-018 is at `placement_requested`, `referredUnitIds: []`, +`declines: []`, no `acceptedUnitId`, and carries: + +```ts +withdrawnReferrals: [ + { + unitId: "scgh-older-adult", + at: NOW_ANCHOR - 10, + reason: "Referral withdrawn — the unit filled the bed from an earlier request", + }, +]; +``` + +`ACCEPT_IN_PRINCIPLE` is the only branch that writes `withdrawnReferrals`, and it does so while +moving the movement to `accepted_awaiting_bed`. So this state is unreachable, and the field is being +used for something it is not: `ward-model.ts` defines it as _"Referrals ended because another unit +accepted"_, while this entry describes a unit-side refusal — which is what `declines` and the +`bed_held_for_earlier_referral` reason exist for. + +Rendered live on `/ward-management/ward/scgh-older-adult`: + +``` +Withdrawn from SCGH Older Adult +WF-018 — Referral withdrawn — the unit filled the bed from an earlier request — 10:32 +``` + +The ward is told a referral it never received has been withdrawn. Same missed invariant as C2. + +--- + +## Minor + +### M1 — No dark, forced-colours or print coverage for any of the four new screens + +Spec §14 requires _"One browser journey per role screen, plus phone, dark, forced-colours and +print."_ `tests/ui-ward-roles.spec.ts` is the only spec touching `/ward-management/ward/*`, +`/ward-management/ed/*` and `/transport/officer`. It sets a 390×844 viewport for the officer and +tracker only; the ward and ED screens are exercised at 1440 only, and no test in the repo calls +`emulateMedia` or a dark / forced-colors variant for any of them. + +### M2 — The "Bed need confirmed" gap is documented where no user reads it + +The factor itself is correct against the constraint: `ward-priority.ts:80-84` labels it +"Bed need confirmed" with an operational detail, never severity / acuity / risk, and `queueOrder` +still sorts urgency-first so it only reorders within a tier. But the KNOWN GAP — 21 of 41 open +movements are voluntary, never receive a Mental Health Act examination, and can therefore never earn +the factor — lives only in a source comment, while the factor is rendered to the coordinator +(`shortlist-panel.tsx:528-529`: `**Bed need confirmed** +25 — Examination outcome: inpatient +order`). The visible effect is that detained patients systematically outrank voluntary ones inside a +tier for a reason that is an artefact of what the model can record. One sentence in the score's +expandable panel would close it. I accept R66's weight; this is disclosure, not arithmetic. + +### M3 — The journey's three-referral assertion passes on one referral + +`tests/ui-ward-roles.spec.ts:390` — +`await expect(shortlist).toContainText(/Parallel referral|referred to 3/i)` under a comment reading +"Three live referrals". One badge satisfies it. `toHaveCount(3)` on the badge locator would make the +assertion match its stated claim. + +### M4 — `CONFIRM_CAPACITY` is role-gated but not unit-gated + +Spec §6 says the ward "Writes to its own unit only." The reducer checks only `role === "ward"` and +then writes whatever `unitId` the event carries (`ward-flow-reducer.ts:361-369`); only the UI +constrains it. Spec §6 also says the role column must be _"enforced by the reducer, not merely +documented"_. The event carries no ward identity, so this cannot be enforced today — worth recording +as a known simplification under §12 rather than left implicit. + +### M5 — Comment attributes the referral-clearing to the wrong reducer branch + +`ward-role-switcher.tsx:73-75` says "REFER_TO_UNITS's own reducer case empties `referredUnitIds` the +moment one unit accepts". `ACCEPT_IN_PRINCIPLE` does that. The code is right; the comment would +mislead the next reader of exactly the branch that matters. + +--- + +## Things I looked for specifically and did not find + +Stated so the absence is evidence rather than silence. + +- **The access-target quarantine (brief item 1) is clean, checked by reading, not by trusting the + guards.** `ED_ACCESS_TARGET_MINUTES` is referenced in exactly two source files. In `ed-screen.tsx` + it reaches only `accessTargetLine(minutesInDepartment: number)` (`:149-155`) and one `data-state` + comparison (`:414`). `accessTargetLine` takes a plain number, constructs no object, and its output + is `" over|under the 24h 00m departmental access target"` — no "due", "deadline", "breach", + "overdue" or "legal". I traced every indirection R28 warned the guards cannot see: no intermediate + local, no aliased import, no spread onto a `LegalForm`, no cross-file helper, no post-construction + mutation. The only `LegalForm` producers anywhere are `ward-movements.ts` and the reducer's + `RECORD_EXAMINATION`, neither of which references the constant. **R28's judgement was right and + its residual risk did not materialise.** +- **`LegalForm.dueAt` absence (brief item 2).** All 25 `dueAt` sites in `src/` handle `undefined` + explicitly; there is no `??` fallback and no non-null assertion on it anywhere. Both rendering + surfaces state the absence in words: `"… · no statutory deadline"` + (`ward-management-console.tsx:52-54`) and `"… — no statutory deadline; in the emergency +department"` (`shortlist-panel.tsx:78-80`). `operationalScore`, `buildActionInbox`, `edPressure` + and `priority-queue` all gate on `!== undefined` before any arithmetic. Fixture measured: 0 × 1A + without `dueAt`, 0 × 3B with one. +- **`ward-eligibility.ts` (brief item 4).** + `git diff cf751504f..916816089 -- src/components/ward-management/ward-eligibility.ts` is empty. No + gate's pass/fail semantics changed anywhere in the diff. The two restriction warnings are a + separate pure function in `ward-derivations.ts` and touch no gate. +- **The end-to-end journey navigates by clicking (brief item 5).** `tests/ui-ward-roles.spec.ts:367` + is the only `page.goto` in the journey — the initial ED load the spec permits. All ten subsequent + hops go through `switchTo()`, which clicks a real `` in the role switcher, and each name is + proved to resolve to exactly one menu item before it is clicked. I independently confirmed the + provider genuinely survives a rail/switcher click: in Proof 2, a `CONFIRM_CAPACITY` and a refused + `HOLD_BED` raised on the ward screen were both still present in the coordinator's exceptions + drawer after clicking through. +- **Controls versus reducer preconditions (brief item 3), for everything except the ward's Hold.** + `examinationBlockedReason` / `handoverBlockedReason` (ED), the officer's four `*BlockedReason` + helpers, `referralAnswerBlocked` (ward), and `canRefer` / `overrideSucceeded` (coordinator) each + mirror their reducer branch clause-for-clause, in the same order, and the override path proves + success by re-reading `movement.referredUnitIds` rather than a local flag. The officer's + `arrivedBlockedReason` deliberately resolves its unit from the **live** `units` array. The single + exception is `holdBlockedReason`, which is C1. +- **Tests that cannot fail.** I checked every test file added or changed in the diff for the house + defect. `ward-flow-contracts.test.ts` counts matches rather than iterations in all six fixture + invariants and asserts the true count (including the two honest `toBe(0)` cases R59 ruled on — + both would still go red on a real violation). `ward-flow-single-source.test.ts` carries three + separate non-empty-scan guards and a positive-control test proving `constructsLegalForm` fires on + a known-real construction. The two jsdom suites both mutate state after mount and assert the _old_ + value is gone as well as the new one present. `tracker-derivations.test.ts` constructs its own + jobs rather than relying on fixture coverage. `ward-priority.test.ts`'s new cases each name a + falsifiable difference. I found no assertion in this diff that no product change could break — + I1 is a guard narrower than its name, not one that cannot fail. +- **Patient-identifying data.** No name, DOB, MRN, address or diagnosis in any rendered string; the + privacy invariant now accumulates and counts the strings it inspects before checking them. + +--- + +## Spec-compliance verdict by section + +| § | Verdict | Basis | +| --------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **3 — Model changes** | **PASS** | All six `Movement` fields present and typed as specified; `DECLINE_REASONS` has seven entries with `out_of_catchment` offered in the ward's decline form; the two restriction warnings are a pure function with distinct levels and distinct prominence, rendered on the shortlist, the diagram and the ward screen; `ward-eligibility.ts` untouched; the 1A/3B invariant is pinned in both directions and measured clean. Fixture requirements met: 3 movements with `formedAt` before `openedAt`, 1 police arrival (WF-009), 3 examined. | +| **6 — The events** | **PASS as a reducer contract** | All fifteen events exist with the specified effects; `EVENT_ROLE` is checked before the payload is inspected at all; withdrawal is automatic and recorded with a reason, not implied by a shrinking list; the hold sets `now + 60` and never auto-releases. Two caveats that do not overturn the verdict: `CONFIRM_CAPACITY` is not unit-gated (M4), and three of the fifteen have no caller in the product (I2, I3) — the reducer conforms, the surfaces do not. | +| **7 — The screens** | **FAIL** | The four screens exist with the right shapes, scoping and empty states, and the access target is worded exactly as required. But §7's first numbered promise for the coordinator — _"Every figure moves when another role acts, including bed counts"_ — is not met (C1): the shortlist's capacity line, all eight gate rows and the ward's own bed grid are frozen. §7's ward paragraph — _"Confirm what beds are genuinely allocatable"_ — has a control that dispatches and a screen that never responds. §7.4's persistent refusals surface is closed by default with a badge that excludes refusals (I4). | +| **9 — Role switching** | **PASS** | Identity is in the URL; the switcher offers all four roles; ward and ED are inferred from the shared `focusMovementId`, read from live `movements`; the plural case offers every candidate rather than picking one (R52 honoured); the coordinator's placelessness is rendered as a fact ("Statewide — no ward or department") rather than invented; unavailable entries use `aria-disabled` + `title` + `sr-only` + an inert handler, per the repo's wiring convention. Every destination is a real ``. | +| **10 — Failure behaviour** | **PARTIAL** | The reducer refuses rather than absorbing, and every named case is implemented and tested: already-placed acceptance names the withdrawal, two-patients-one-bed refuses with `bed_held_for_earlier_referral`, wrong role refuses before the payload is read, out-of-order stages refuse. Conservative display holds throughout (unresolved ids are named, never substituted; absent `dueAt` is stated, never defaulted; `transportLeg` returns `undefined` rather than collapsing absence into a leg). It fails on the second half of the sentence — _"every refusal is recorded in `rejections` **and rendered on the coordinator screen**"_ — because the rendering is behind a closed drawer whose count ignores them (I4), and because C1 manufactures refusals no user can connect to the action that caused them. | +| **14 — How it gets proved** | **PARTIAL** | Reducer coverage is genuinely exhaustive and mutation-proved. The contract tests cover the cap, the declined-unit exclusion, ownerlessness, the recorded withdrawal, the form/examination agreement and privacy. Three shortfalls: _"beds always account for, before and after every event"_ is asserted for one unit across one ten-event walk, not as an invariant over every state (and the earlier `unitCapacity`-based version was already found to survive corruption); the per-role browser journeys exist and the end-to-end journey correctly navigates by clicking, but **dark, forced-colours and print are absent for all four new screens** (M1); and the fixture-coherence invariants were derived from a correct exhaustive table that was then only partly asserted, leaving C2 and I6 live. Time is injected everywhere except the live provider, as required. | + +--- + +## Were any of the controller's rulings wrong? + +**No ruling I would call wrong. One was correct but not carried through, and one brief-level +decision — never recorded as a ruling — is the root of C1.** + +- **R63 (derive the fixture invariant from the reducer's complete set of stage-producing + transitions) was the right call and the method was right.** The table it produced + (`ward-flow-contracts.test.ts:196-200`) is, as far as I can verify by re-deriving it + independently, genuinely exhaustive over the reducer's eight stage-assigning branches. It was then + **incompletely discharged**: the `RAISE_REFERRAL` row records three writes (`referredUnitIds: []`, + `declines: []`, `withdrawnReferrals: []`) and none of the three became an assertion. Both + remaining fixture violations in the whole model — WF-012 (C2) and WF-018 (I6) — sit in exactly + that un-asserted row. The ledger's own diagnosis of the R58 failure ("the instance was fixed and + the class was left alone") applies one level up: this time the class was correctly identified and + then not fully written down. +- **The Task 8 brief's instruction to resolve the ward's unit with `unitById(unitId)`** + (`task-8-brief.md:12,66`) is, in my view, the wrong call, and it is the root of C1. It was written + into the brief rather than introduced by the implementer, and it sends the one screen whose stated + purpose is confirming allocatable beds to the frozen fixture that spec §4 exists to replace. + Task 9 got the same decision right one task later and its code comment names the exact hazard, + which suggests an oversight rather than a considered trade-off — but it was never surfaced as a + ruling, so nothing weighed it. +- **R28 (narrow the access-target guard's claim; move real enforcement to Task 11's brief and + review) I would affirm.** I checked the constant by reading every indirection R28 said the guards + cannot see, and the quarantine holds. The named risk did not materialise, and the guards' names + now match their reach. +- **R59 (keep the two `toBe(0)` assertions, name them honestly, do not manufacture a red) I would + affirm.** Both remain falsifiable; asserting the real count is the honest instrument. +- **R66 (accept the 25-point weight without re-litigating it) I would affirm.** The weight is + defensible, the wording is operational, and the score still orders only within an urgency tier. My + only residual concern is disclosure, not arithmetic (M2). +- **R55, R62, R32, R35** — recorded, scoped and deferred correctly; nothing to add. + +--- + +## Verification actually performed by me + +Read: the full diff, the spec (all 19 sections), the plan's coverage table, the ledger's rulings +R24–R67, and the Task 8/9/10/11/12 briefs and reports for the surfaces in question. + +Ran (all read-only, all against the already-running dev server or the fixture): + +- Six headless-Chromium probes driving real controls: ward capacity confirmation on two units; the + Hold-bed advertise-then-refuse sequence with a click-through to the coordinator's exceptions + drawer; the coordinator shortlist after a ward drops capacity to zero; WF-012 across three + screens; WF-018's withdrawn card; and an ED-raised referral through to its blocked examination. +- Three `tsx` probes measuring the real fixture against every implication I derived independently + from the reducer's eight stage-producing branches (about twenty properties). +- Static reads: every `dueAt` site, every `ED_ACCESS_TARGET_MINUTES` site, every `allUnits()` / + `unitById()` site, every `useWardFlow()` consumer, and every dispatcher of each of the fifteen + events. + +Not run, and why: `npx tsc --noEmit`, the node-env ward suites, the jsdom files and the ward +Chromium gate were all run by the controller at this exact HEAD, and I made no change that could +move them. `npm run verify:ui`, `verify:release`, the guard-push suite and anything provider-backed +were prohibited. No file was modified; `git status --porcelain` is empty. diff --git a/src/components/ward-management/coordinator/coordinator-screen.tsx b/src/components/ward-management/coordinator/coordinator-screen.tsx index 4515851059..39b874de0c 100644 --- a/src/components/ward-management/coordinator/coordinator-screen.tsx +++ b/src/components/ward-management/coordinator/coordinator-screen.tsx @@ -36,10 +36,13 @@ const PHONE_DIAGRAM_MEDIA_QUERY = "(max-width: 48rem)"; export function CoordinatorScreen() { // Task 5: the screen's props stop being derived from the frozen `wardMovements` fixture and // `NOW_ANCHOR` constant and start coming from the shared provider (`WardFlowProvider`, already - // wrapping every `/ward-management` route via `src/app/ward-management/layout.tsx`). `units` - // is not destructured here — nothing this screen renders yet reads live unit state, and an - // unused destructured value would be dead weight rather than real wiring. - const { movements, rejections, now, dispatch, focusMovementId, setFocusMovementId } = useWardFlow(); + // wrapping every `/ward-management` route via `src/app/ward-management/layout.tsx`). + // + // Whole-branch review Critical 1: `units` IS now destructured and threaded into `FlowDiagram` + // and `ShortlistPanel` below. It was deliberately left out here on the original claim that + // "nothing this screen renders yet reads live unit state" — false: both child components read + // unit capacity, and both were doing it from the frozen `ward-sites.ts` fixture instead. + const { movements, units, rejections, now, dispatch, focusMovementId, setFocusMovementId } = useWardFlow(); // Task 12: seeded from the shared `focusMovementId` (not always `undefined`) so a coordinator // who switched away to answer a referral as another role and switches back finds the same // patient still selected — this screen remounts on every route change (it is a route @@ -189,6 +192,7 @@ export function CoordinatorScreen() { setSelectedUnitId((current) => (current === unitId ? undefined : unitId))} /> @@ -203,6 +207,7 @@ export function CoordinatorScreen() { setSelectedUnitId((current) => (current === unitId ? undefined : unitId))} dispatch={dispatch} diff --git a/src/components/ward-management/coordinator/coordinator.module.css b/src/components/ward-management/coordinator/coordinator.module.css index d4df141a41..8f71663597 100644 --- a/src/components/ward-management/coordinator/coordinator.module.css +++ b/src/components/ward-management/coordinator/coordinator.module.css @@ -547,7 +547,7 @@ } /* - * `eligibleCandidates` sorts eligible-first but never filters, so a shortlisted unit can still + * `eligibleCandidatesAmong` sorts eligible-first but never filters, so a shortlisted unit can still * fail a gate (Critical 1 — already declined, wrong security tier, stale capacity, ...). This * line must never look like the eligible route above: dashed, and danger-toned rather than * accent-toned, so the arrow itself does not read as an endorsement of a route the data denies. @@ -1453,6 +1453,25 @@ line-height: var(--co-leading-body); } +/* + * Whole-branch review I2: a recorded escalation is a real, persistent fact — the network really + * was exhausted for this movement at this instant — never a success message, so it deliberately + * does not reuse `.shortlistConfirmationRecord`'s success tokens above. `--warning-*`, not + * `--danger-*`: an escalation is a recorded operational fact a coordinator is expected to act on + * next, not a refusal or an error state. + */ +.shortlistEscalationRecord { + margin: 0 0 var(--co-space-8); + border: 0.0625rem solid var(--warning-border); + border-radius: var(--radius-md); + background: var(--warning-bg); + padding: var(--co-space-8) var(--co-space-10); + color: var(--warning-text); + font-size: var(--text-2xs); + font-weight: 650; + line-height: var(--co-leading-body); +} + .shortlistActionRow { display: flex; flex-wrap: wrap; @@ -1596,6 +1615,30 @@ font-weight: 700; } +/* + * Whole-branch review I4: a separate, danger-toned badge for refused transitions — never merged + * into `.exceptionsToggleCount` above, which is `items.length`, a different fact (the action + * inbox). Rendered only while `rejections.length > 0` (see exception-drawer.tsx), so the trigger + * carries no extra chrome until the first refusal and then never goes quiet again — the same + * "present the moment the fact is real" rule `.refusalsSection` already applies to the panel + * beneath it. Danger tokens, not the neutral accent above: a refusal is the reducer stating an + * action did NOT happen, which is a sharper fact than an open work-list item. + */ +.exceptionsToggleRefusalCount { + display: inline-flex; + min-height: var(--co-space-20); + align-items: center; + gap: var(--co-space-6); + border: 0.0625rem solid var(--danger-border); + border-radius: var(--radius-pill, 9999px); + background: var(--danger-bg); + padding: 0 var(--co-space-8); + color: var(--danger-text); + font-size: var(--text-3xs); + font-weight: 700; + white-space: nowrap; +} + .exceptionsDrawer[data-open="true"] .placeholder { margin-top: var(--co-space-8); } diff --git a/src/components/ward-management/coordinator/exception-drawer.tsx b/src/components/ward-management/coordinator/exception-drawer.tsx index 9a3296eee3..f941541aba 100644 --- a/src/components/ward-management/coordinator/exception-drawer.tsx +++ b/src/components/ward-management/coordinator/exception-drawer.tsx @@ -47,7 +47,30 @@ export function ExceptionDrawer({ items, rejections, open, onToggle, onSelectMov {open ? (
diff --git a/src/components/ward-management/coordinator/flow-diagram.tsx b/src/components/ward-management/coordinator/flow-diagram.tsx index b799797ca5..c1aac43566 100644 --- a/src/components/ward-management/coordinator/flow-diagram.tsx +++ b/src/components/ward-management/coordinator/flow-diagram.tsx @@ -6,27 +6,27 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } fr import type { Instant } from "@/components/ward-management/ward-clock"; import { candidateReason, - eligibleCandidates, + eligibleCandidatesAmong, restrictionNotice, unitCapacity, wardServiceOrder, } from "@/components/ward-management/ward-derivations"; -import { useWardFlow } from "@/components/ward-management/ward-flow-provider"; import { PARALLEL_REFERRAL_CAP, type Movement, type Unit } from "@/components/ward-management/ward-model"; import { edPressure } from "@/components/ward-management/ward-pressure"; -import { allUnits, siteByCode, unitById } from "@/components/ward-management/ward-sites"; +import { siteByCode } from "@/components/ward-management/ward-sites"; import styles from "./coordinator.module.css"; type FlowDiagramProps = { movement: Movement | undefined; now: Instant; + units: Unit[]; selectedUnitId: string | undefined; onSelectUnit: (unitId: string) => void; }; type Point = { x: number; y: number }; -type ShortlistCandidate = ReturnType[number]; +type ShortlistCandidate = ReturnType[number]; type Connector = | { id: string; path: string; kind: "demand" } | { id: string; path: string; kind: "route"; eligible: boolean } @@ -66,7 +66,7 @@ function recordedDestinationIds(movement: Movement | undefined): Set { * * Two further corrections from the whole-branch review: * - * Critical 1 -- the word "nearest" is gone. `eligibleCandidates` ranks cohort-matching units + * Critical 1 -- the word "nearest" is gone. `eligibleCandidatesAmong` ranks cohort-matching units * eligible-first and breaks ties on array order; the model holds no distance data at all, so a * proximity claim here was simply false. * @@ -74,8 +74,12 @@ function recordedDestinationIds(movement: Movement | undefined): Set { * them. WF-004 sits at stage `bed_held` with a bed held at BTY Adult Secure, and this line read * "WF-004 -- 3 eligible destinations". The recorded fact now leads; the candidate count follows * as secondary context, because a coordinator may still be considering alternatives. + * + * Whole-branch review Critical 1: `units` is the caller's live provider `units`, never + * `ward-sites.ts`'s `unitById`/`allUnits` — a name lookup here still must reflect a unit that + * exists in the live world the same as any capacity figure would. */ -function hubStatusText(movement: Movement | undefined, shortlist: ShortlistCandidate[]) { +function hubStatusText(movement: Movement | undefined, shortlist: ShortlistCandidate[], units: Unit[]) { if (!movement) return "Select a movement from the priority queue to route it"; // "Other" means other than the units already recorded against this movement -- a candidate that @@ -85,14 +89,14 @@ function hubStatusText(movement: Movement | undefined, shortlist: ShortlistCandi const candidateTail = otherCount === 0 ? "" : `; ${otherCount} other candidate${otherCount === 1 ? "" : "s"} shown`; if (movement.acceptedUnitId) { - const accepted = unitById(movement.acceptedUnitId); + const accepted = units.find((unit) => unit.id === movement.acceptedUnitId); return accepted ? `${movement.id} — accepted destination: ${accepted.name}${candidateTail}` : `${movement.id} — an accepted destination is recorded but could not be resolved`; } if (movement.referredUnitIds.length > 0) { const names = movement.referredUnitIds - .map((id) => unitById(id)?.name) + .map((id) => units.find((unit) => unit.id === id)?.name) .filter((name): name is string => Boolean(name)); return names.length === movement.referredUnitIds.length && names.length > 0 ? `${movement.id} — outstanding referral${names.length === 1 ? "" : "s"}: ${names.join(", ")}${candidateTail}` @@ -120,7 +124,7 @@ function hubStatusText(movement: Movement | undefined, shortlist: ShortlistCandi * is already recorded against; with nothing selected, this renders the network with nothing routed * rather than a guessed selection (ruling: display less rather than something plausible). * - * `eligibleCandidates` sorts eligible-first but never filters -- it can and does return units + * `eligibleCandidatesAmong` sorts eligible-first but never filters -- it can and does return units * that fail a gate (already declined the movement, wrong security tier, stale capacity, ...). * Every shortlisted node therefore carries its own verdict (`data-eligible`, plus * `candidateReason` rendered as real text) and an ineligible route is drawn visually distinct @@ -133,12 +137,11 @@ function hubStatusText(movement: Movement | undefined, shortlist: ShortlistCandi * and reruns on a `ResizeObserver` plus a window resize listener, so the diagram survives a * resize rather than only ever being screenshotted once. */ -export function FlowDiagram({ movement, now, selectedUnitId, onSelectUnit }: FlowDiagramProps) { - const { units } = useWardFlow(); +export function FlowDiagram({ movement, now, units, selectedUnitId, onSelectUnit }: FlowDiagramProps) { const pressure = useMemo(() => edPressure(now), [now]); const shortlist = useMemo( - () => (movement ? eligibleCandidates(movement, now, PARALLEL_REFERRAL_CAP, units) : []), - [movement, now, units], + () => (movement ? eligibleCandidatesAmong(movement, units, now, PARALLEL_REFERRAL_CAP) : []), + [movement, units, now], ); const shortlistByUnitId = useMemo( () => new Map(shortlist.map((candidate) => [candidate.unit.id, candidate])), @@ -151,20 +154,24 @@ export function FlowDiagram({ movement, now, selectedUnitId, onSelectUnit }: Flo // group rather than guessing one -- conservative failure, not a crash. `unplacedUnits` below // catches exactly that case so the unit still renders (as an explicit anomaly) rather than // silently vanishing from the board (review Minor 6). + // + // Whole-branch review Critical 1: grouped from the caller's live `units`, never `allUnits()` — + // every unit NODE on this board (its bed grid, via `unitCapacity` in `UnitNode` below) must + // move the instant a ward confirms new capacity, not only at first paint. const serviceGroups = useMemo( () => wardServiceOrder .map((service) => ({ service, - units: allUnits().filter((unit) => siteByCode(unit.siteCode)?.service === service), + units: units.filter((unit) => siteByCode(unit.siteCode)?.service === service), })) .filter((group) => group.units.length > 0), - [], + [units], ); const unplacedUnits = useMemo(() => { const grouped = new Set(serviceGroups.flatMap((group) => group.units.map((unit) => unit.id))); - return allUnits().filter((unit) => !grouped.has(unit.id)); - }, [serviceGroups]); + return units.filter((unit) => !grouped.has(unit.id)); + }, [serviceGroups, units]); const canvasRef = useRef(null); const hubRef = useRef(null); @@ -380,7 +387,7 @@ export function FlowDiagram({ movement, now, selectedUnitId, onSelectUnit }: Flo
diff --git a/src/components/ward-management/coordinator/priority-queue.tsx b/src/components/ward-management/coordinator/priority-queue.tsx index 1f1b91efd3..5ee495638e 100644 --- a/src/components/ward-management/coordinator/priority-queue.tsx +++ b/src/components/ward-management/coordinator/priority-queue.tsx @@ -81,8 +81,12 @@ export function PriorityQueue({ movements, now, selectedId, onSelect, filterEdId // breached" — the factor list is scoped to Task 7's expandable shortlist, not this // row, but a breached statutory deadline is the one thing this row must never let a // coordinator miss, so it always renders here regardless of what Task 7 later shows. - // A form with no `dueAt` (Task 6A: a Form 3B honestly carries none) is never breached — - // `undefined` must never reach `clockState`'s arithmetic. + // A form with no `dueAt` is never breached — `undefined` must never reach + // `clockState`'s arithmetic. As of the 2026-08-23 product-owner correction, neither a + // Form 1A nor a Form 3B carries one any longer (Task 6A first established this for 3B; + // see `LegalForm`'s doc comment in ward-model.ts) — only the transport/transfer forms + // (4A/4C) still do, and none of those are due in the past on today's fixture, so + // `legalBreached` below is false today. const legalDueAt = movement.legalForm?.dueAt; const legalBreached = legalDueAt !== undefined && clockState(legalDueAt, now) === "breached"; const legalFactor = factors.find((factor) => factor.label === "Statutory timing"); diff --git a/src/components/ward-management/coordinator/shortlist-panel.tsx b/src/components/ward-management/coordinator/shortlist-panel.tsx index 67c3e5c126..468e164624 100644 --- a/src/components/ward-management/coordinator/shortlist-panel.tsx +++ b/src/components/ward-management/coordinator/shortlist-panel.tsx @@ -8,7 +8,7 @@ import { candidateReason, destinationUnit, elapsedLabel, - eligibleCandidates, + eligibleCandidatesAmong, referralBlockedReason, restrictionNotice, unitCapacity, @@ -16,9 +16,8 @@ import { import { eligibility, type GateResult } from "@/components/ward-management/ward-eligibility"; import type { WardFlowEvent } from "@/components/ward-management/ward-flow-events"; import { PARALLEL_REFERRAL_CAP, type Movement, type Unit } from "@/components/ward-management/ward-model"; -import { useWardFlow } from "@/components/ward-management/ward-flow-provider"; import { operationalScore } from "@/components/ward-management/ward-priority"; -import { allEmergencyDepartments, unitById } from "@/components/ward-management/ward-sites"; +import { allEmergencyDepartments } from "@/components/ward-management/ward-sites"; import { ignoreUnavailableActivation } from "@/components/ui-primitives"; import styles from "./coordinator.module.css"; @@ -26,6 +25,7 @@ import styles from "./coordinator.module.css"; type ShortlistPanelProps = { movement: Movement | undefined; now: Instant; + units: Unit[]; selectedUnitId: string | undefined; onSelectUnit: (unitId: string) => void; dispatch: Dispatch; @@ -102,11 +102,10 @@ function legalFormLine(movement: Movement, now: Instant) { * driven by something other than the gate's own `pass` boolean. Every icon below reads directly * off `gate.pass`; nothing else is permitted to decide it (see the report's red/green proof). */ -export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, dispatch }: ShortlistPanelProps) { - const { units } = useWardFlow(); +export function ShortlistPanel({ movement, now, units, selectedUnitId, onSelectUnit, dispatch }: ShortlistPanelProps) { const shortlist = useMemo( - () => (movement ? eligibleCandidates(movement, now, PARALLEL_REFERRAL_CAP, units) : []), - [movement, now, units], + () => (movement ? eligibleCandidatesAmong(movement, units, now, PARALLEL_REFERRAL_CAP) : []), + [movement, units, now], ); // The unit whose gates this panel currently explains. A selection carried over from another @@ -118,10 +117,14 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di // // Whole-branch review Critical 2: this default is ORIENTATION ONLY. It may never be the thing // Refer acts on — see `canRefer` below. + // + // Whole-branch review Critical 1: resolved from the live `units` the provider hands back — + // never `unitById`, which reads the frozen fixture and would still call this ward "Eligible + // now" after it confirmed zero allocatable beds on its own screen. const activeUnit = useMemo(() => { - if (selectedUnitId) return unitById(selectedUnitId); + if (selectedUnitId) return units.find((unit) => unit.id === selectedUnitId); return shortlist[0]?.unit; - }, [selectedUnitId, shortlist]); + }, [selectedUnitId, shortlist, units]); const activeVerdict = useMemo(() => { if (!movement || !activeUnit) return undefined; @@ -145,6 +148,11 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di // shown) — a coordinator can refer to up to three wards at once, but the diagram and the gate // list can only ever explain one at a time. const [referTargets, setReferTargets] = useState([]); + // Whole-branch review I2 (spec §11): the escalation form's own open/typed-contact state — never + // the recorded fact itself, which lives on `movement.escalation` and is read fresh on every + // render, the same discipline `overrideSucceeded` already holds to for the override record. + const [escalationOpen, setEscalationOpen] = useState(false); + const [escalationContact, setEscalationContact] = useState(""); // A confirmation, an open override form, or a referral selection all belong to the movement // they were made against — moving to a different movement must never leave a stale "Referred" @@ -158,6 +166,8 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di setOverrideOpen(false); setOverrideReason(""); setReferTargets([]); + setEscalationOpen(false); + setEscalationContact(""); } if (!movement) { @@ -167,9 +177,10 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di } // TypeScript's narrowing of `movement` above does not reach into the `handleRefer` / - // `handleOverrideSubmit` closures defined further down, so this plain string is what they - // close over instead of re-checking `movement` themselves. + // `handleOverrideSubmit` / `submitEscalation` closures defined further down, so these plain + // values are what they close over instead of re-checking `movement` themselves. const movementId = movement.id; + const declinedUnitIds = movement.declines.map((decline) => decline.unitId); const originEd = allEmergencyDepartments().find((ed) => ed.id === movement.originEdId); // Neutral "currently at" language, never framed as an authorisation requirement — authorisation @@ -178,8 +189,11 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di ? `Currently at ${originEd.siteCode} — ${originEd.name}` : "Currently at an unresolved department"; - // A form with no `dueAt` (Task 6A: a Form 3B honestly carries none) is never breached — - // `undefined` must never reach `clockState`'s arithmetic. + // A form with no `dueAt` is never breached — `undefined` must never reach `clockState`'s + // arithmetic. As of the 2026-08-23 product-owner correction, neither a Form 1A nor a Form 3B + // carries one any longer (Task 6A first established this for 3B; see `LegalForm`'s doc + // comment in ward-model.ts) — only the transport/transfer forms (4A/4C) still do, and none of + // those are due in the past on today's fixture, so `legalBreached` is false today. const legalDueAt = movement.legalForm?.dueAt; const legalBreached = legalDueAt !== undefined && clockState(legalDueAt, now) === "breached"; @@ -192,9 +206,9 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di // one — is offered instead, and only ever labelled "Suggested destination": a computed // suggestion must never sit unlabelled in the destination slot, and an ineligible candidate must // never be presented as a suggestion at all (review Important 3). - const acceptedUnit = movement.acceptedUnitId ? unitById(movement.acceptedUnitId) : undefined; - const referredUnits = movement.referredUnitIds.map((id) => ({ id, unit: unitById(id) })); - const recordedDestination = destinationUnit(movement); + const acceptedUnit = movement.acceptedUnitId ? units.find((unit) => unit.id === movement.acceptedUnitId) : undefined; + const referredUnits = movement.referredUnitIds.map((id) => ({ id, unit: units.find((unit) => unit.id === id) })); + const recordedDestination = destinationUnit(movement, units); const hasRecordedReferral = recordedDestination !== undefined || Boolean(movement.acceptedUnitId) || movement.referredUnitIds.length > 0; const topEligible = shortlist.find((candidate) => candidate.verdict.eligible); @@ -278,6 +292,38 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di setOverrideReason(""); } + /** + * Whole-branch review I2 (spec §11). `RECORD_ESCALATION`'s own reducer branch + * (`ward-flow-reducer.ts`) carries no precondition beyond the role check — it stamps + * `escalation` on any movement that resolves — so unlike Refer/Override this control never + * needs a `*BlockedReason` guard: nothing here can be refused. `triedUnitIds` is never typed by + * a human — it is `movement.declines`, the units genuinely referred to and declined, exactly + * what the "Declines" section immediately above already renders (each with its own real reason + * — the shortlist's own "what was tried, why each failed"). Deliberately NOT the panel's + * `shortlist` candidate list: that is capped at `PARALLEL_REFERRAL_CAP` and is a theoretical + * eligibility scan, not a record of what was actually attempted — using it would let a + * genuinely untried unit (never referred, only eligibility-checked) be named as "tried". + * WF-009's own pre-authored fixture escalation (`ward-movements.ts`) uses exactly this shape: + * its five `triedUnitIds` are its five `declines`, unit for unit. Only `contact` (a role or + * service, never a person — synthetic data only, the same rule every other free-text field in + * this prototype follows) is typed. + */ + function submitEscalation(event: FormEvent) { + event.preventDefault(); + const contact = escalationContact.trim(); + if (contact.length === 0) return; + dispatch({ + type: "RECORD_ESCALATION", + role: "coordinator", + now, + movementId, + triedUnitIds: declinedUnitIds, + contact, + }); + setEscalationOpen(false); + setEscalationContact(""); + } + // Structurally incapable of claiming an override succeeded when it did not: this checks the // movement's OWN post-dispatch `referredUnitIds` — read fresh on every render from the live // provider — not a flag captured once at click time. Override is not stage-gated (see the @@ -290,7 +336,7 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di overrideRecord.unitIds.length > 0 && overrideRecord.unitIds.every((id) => movement.referredUnitIds.includes(id)); const overrideRecordUnits = overrideRecord - ? overrideRecord.unitIds.map((id) => unitById(id)?.name ?? "an unresolved unit") + ? overrideRecord.unitIds.map((id) => units.find((unit) => unit.id === id)?.name ?? "an unresolved unit") : []; return ( @@ -323,14 +369,17 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di to PARALLEL_REFERRAL_CAP live referrals at once, and each is a fact a coordinator acts on (review Minor 6: a hidden parallel referral is exactly the trust failure the cap and this record exist to prevent). "Parallel referral" is the label Task - 5's Refer action uses everywhere this fact is surfaced. */} + 5's Refer action uses everywhere this fact is surfaced. Whole-branch review M3: + `data-testid` here (never present before) is what lets a test assert the real + COUNT of these badges — the journey's own "Three live referrals" comment used to + sit over an assertion one badge alone could satisfy. */} {referredUnits.map(({ id, unit }) => unit ? ( - + Parallel referral: {unit.name} ) : ( - + Parallel referral to an unresolved unit. ), @@ -369,8 +418,9 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di
{/* Whole-branch review Critical 1: this list was headed "Nearest candidates", a proximity claim the model cannot support — `Unit` has no distance, geo, locality or catchment - field, and `eligibleCandidates` filters on cohort and sorts eligible-first, breaking - ties on `allUnits()` array order. WF-018, sitting in SCGH's own emergency department, + field, and `eligibleCandidatesAmong` filters on cohort and sorts eligible-first, + breaking ties on the live `units` array's own order. WF-018, sitting in SCGH's own + emergency department, was shown RPH Older Adult above SCGH Older Adult under that heading. The subtitle states the real ordering rather than leaving the reader to assume one. */}

Candidates

@@ -502,7 +552,7 @@ export function ShortlistPanel({ movement, now, selectedUnitId, onSelectUnit, di ) : (
    {movement.declines.map((decline, index) => { - const unit = unitById(decline.unitId); + const unit = units.find((candidate) => candidate.id === decline.unitId); return (
  • + {/* Whole-branch review I2 (spec §11): moved into Phase 3 from Phase 4 on the reasoning + that "a phase that only proves the loop which succeeds has not proved the loop." The + shortlist above already renders what was tried (every candidate row) and why each + failed (`candidateReason` on each one); this section adds the two facts nothing else on + screen records: that the network really was exhausted, stamped on the movement, and who + is being contacted next. Rendered whenever a recorded escalation exists (a persistent + fact, never a toast), and the control to record a new one only while there genuinely is + no eligible destination — the same `topEligible === undefined` condition the header + above already uses for "No eligible destination found yet." */} +
    +

    Escalation

    + {movement.escalation ? ( +

    + {`Escalated at ${formatInstant(movement.escalation.at)} — tried ${movement.escalation.triedUnitIds.length} unit${movement.escalation.triedUnitIds.length === 1 ? "" : "s"} — contact: "${movement.escalation.contact}".`} +

    + ) : null} + {topEligible === undefined ? ( + <> + {!movement.escalation ? ( +

    + No eligible destination is currently available for {movement.id}. Record what was tried and who is being + contacted next. +

    + ) : null} + + {escalationOpen ? ( +
    + +