Land the unlanded Ward Flow Phase 3 work and reconcile the branch with main - #2344
Merged
Conversation
…nd phase 1 plan Baseline for the Phase 1 model rework. Adds the ward-management surface (command, constellation, network, queue, capacity, movements, exceptions, transport, governance, patient workspace), its synthetic fixtures, the domain glossary and decision records, the metro patient-flow design spec, and the Phase 1 implementation plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eligibility, shared derivations
The subagent-driven-development ledger lives in git-ignored scratch and does not survive a session. This records what a later session needs: the rulings taken during Phase 1 with what each costs if wrong, the module map, the deferred findings that Phase 4 must not inherit blindly, and the verification lessons from this run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ules Fixes every finding from the final whole-branch review: gate icons that ignored pass/fail (including MHA authorisation), a five-state bed grid that double-counted held/blocked beds, "open movement" counts that included arrived/closed records, generated movements whose stage contradicted their own fields, negative-instant clock formatting, bed-release blockers that leaked departing-patient detail, a "catchment" label computed from origin ED rather than patient catchment, an "exhausted search" blocker the fixture didn't actually support, a Readiness panel that ticked a breached legal deadline, and an unconditional "eligible candidate" claim. Also cleans up the smaller deferred items: cohort/security gate grammar and pass/fail text, an advertised-but-unrun exception rule, stale glossary prose, and one leftover "AI" label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The whole-branch review found defects the eight per-task reviews could not see, because each looked at one task's diff. One fix wave closed them. This records what the review caught and why it mattered, the findings parked at the close with the ruling on each, the verification actually run, and the two repo traps that make a green result untrustworthy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rief Phase 2 builds the flow coordinator's screen — pressure strip, priority queue, flow diagram, explainable shortlist, exceptions drawer, phone form — and retires Constellation into it. Ten tasks, two of which build the pure derivations the screen reads: the operational score (rebuilt with no urgency component) and per-department pressure. The kickoff brief carries what a cold session needs, including the process calibration for a screen-heavy phase and the three Phase 1 lessons that should shape how it is verified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One self-contained file a session can read cold: the problem, the WA clinical and legal grounding, the ten settled scope decisions, the model with every export and identifier, the non-negotiable rules, the repo conventions and the two gates that report success without running, what Phase 1 learned and why it shapes how Phase 2 is verified, and the decisions taken on the owner's behalf. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nd blocker checks
Review findings against the real fixture:
- Declines detail rendered a self-contradictory fraction ("5 of 3 parallel
referrals declined") — declines.length is cumulative history, the cap
limits simultaneous live referrals, they never shared a denominator.
State only the count.
- Transport delay fired for movements already en route (WF-006, WF-014,
WF-306, WF-313, WF-320, WF-327), contradicting their own blocker text.
Require acceptedAt set and enRouteAt/collectedAt/cancelledAt all unset,
matching buildActionInbox's existing condition, and drop the truthiness
reads on Instant fields in favour of explicit === undefined.
- hasActiveBlocker's "starts with None" match was wide enough to hide a
real blocker like "None of the secure units can take him"; narrowed to
the exact sentinel or "None" + end-of-string/dash/colon.
- queueOrder's exclusion test only asserted a shorter list, which would
still pass if queueOrder dropped everything; pinned to the exact
isOpen-filtered count instead.
- Exported ward-clock's splitDuration (already zero-padding) and reused it
for the wait-time detail instead of a second, non-padded formatter.
- Removed the redundant .slice() after .filter() in queueOrder.
Six new/tightened tests cover the behavioural findings; watched them fail
against the unfixed code before applying each fix.Adds edPressure(now), sorted worst-first by breaching then longest wait then volume. Reuses ward-clock's clockState for the breach definition instead of re-deriving it inline, and clamps per-movement waits at zero so a movement with a future openedAt can never render a negative wait. The sort test's original array-comparison assertion coerced both sides to strings via `>=`, which would pass a genuinely wrong ordering; it is replaced with an explicit numeric tuple comparison.
…ble tests edPressure(now) imported wardMovements directly, so every assertion was forced to key off the one fixture (every department busy, every wait positive). That let three tests pass under mutations that should have failed them: dropping quiet departments, misattributing counts between departments, and removing the future-openedAt wait clamp. Change the signature to edPressure(now, movements = wardMovements), mirroring queueOrder(movements, now) in ward-priority.ts with now kept first for the existing/planned call sites. Add tests that inject a built movement list to prove: quiet departments still report zeros (never drop, never substitute a different EmergencyDepartment), counts attribute to the correct originEdId rather than a rotated neighbour, and a future-dated openedAt clamps to zero rather than going negative. Also cover the clockState "due exactly now" boundary, and correct the pre-existing longest-wait test's own expectation to mirror the clamp (it previously proved the clamp only by fixture coincidence). Use ward-clock's minutesUntil instead of inline subtraction, matching the existing elapsedLabel pattern. Each of the three structural findings was verified red against its named mutation before being accepted.
…ow, labelling Two Criticals, three Importants, three Minors from the Task 3 review. Criticals: - Wire the unused selectedUnitId setter into a real (if currently unreachable) "Clear unit selection" control in the diagram region instead of discarding it, clearing the npm run lint no-unused-vars failure. - Repointing /ward-management broke five tests in ui-ward-management.spec.ts that still targeted WardManagementConsole. Repaired gotoWardFlow and three specs against the coordinator screen, kept the network-diagram spec untouched, and removed the two specs with no coordinator-screen equivalent yet — replaced by test.fixme placeholders in ui-ward-coordinator.spec.ts naming the tasks that will implement them (5/7 and 8). Importants: - The coordinator spec's overflow assertion measured document.documentElement, which .screen's overflow:hidden makes impossible to fail. Now measures the region grid's own scrollWidth/clientWidth via a testid; proved it goes red against the pre-fix CSS (836px overflow at 320px) before applying the fix. - The region grid had no narrow-layout fallback below its shrink breakpoint and overflowed at 1100/820/390/320. Stacked the three regions into one column below 1440px (min-width: 90rem restores the three-column grid), and separately fixed a blank 72px band at <=640px where the hidden rail's grid track was still reserved. Verified zero overflow at 1600/1280/1100/820/390/320. - "N movements" labelled the open-only queue count as a total; now "N open movements". Minors: - Dropped the inert z-index on the statically-positioned exceptions drawer (its position in .main's row order already pins it) and the now-unused --co-z-drawer token, plus two other declared-but-unused tokens. --co-space-4, --co-space-16. - Added a visually-hidden <h1> naming the screen. Also fixed a wrapping regression the open-movements label change introduced in the 14rem queue column header, caught by re-inspecting the recaptured screenshot. RAG impact: no retrieval behaviour change — this only touches the Ward Flow coordinator screen shell (src/components/ward-management/coordinator/**) and its tests; no file under src/lib/rag/**, retrieval-selection, ranking-config, answer-ranking, the eval harness, or the golden fixture was touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…, selection visibility, empty state - Tests could not detect a wrong card order or the queue filtering to the wrong department; both are now pinned by data attributes (data-breaching, data-longest-minutes, data-waiting, data-origin-ed) and asserted directly. - aria-label on each pressure card replaced its whole accessible name, hiding every figure from assistive technology; the name is now composed from the hospital name plus the visible figures. - Selected state was invisible on breaching cards (border/background were overridden by the danger tint); a card outline now survives that and is pinned to Highlight under forced-colors. - A quiet department rendered "0 waiting · longest 0m"; it now shows an explicit "No patients waiting" state, covered by a new dom test. - Removed the click-retry loop around Playwright interactions in favour of a networkidle wait in gotoCoordinator, matching the sibling ward-management spec's helper. - Added the ordering rule to the strip header and cleared the department filter automatically if it ever names a department that cannot be resolved.
…roof, now-prop, origin label, ED lookup hoist Three mutations survived the appended ordering test with the suite green: removing data-tier still passed (empty-array vacuity), inverting the within-tier tiebreak still passed (no machine-readable score on the row), and both a constant score and a deleted breach line still passed. Fixes: - Add a count floor before the tier-sortedness assertion so absence can't satisfy it. - Add data-score to each row and assert non-increasing score within a tier (the property, not the fixture). - Assert the visible score text matches the row's own data-score, and that the breach line is present on a breached-deadline row and absent on a non-breaching one. Also: PriorityQueue now takes now: Instant as a prop (was reading NOW_ANCHOR directly, unlike its now-prop sibling PressureStrip); origin department renders "from JHC" instead of the unlabelled "JHC", which read as a destination next to the patient's required security level; and allEmergencyDepartments() is looked up once per render via a Map instead of once per row.
… as a routed destination Critical 1: eligibleCandidates sorts eligible-first but never filters, so the diagram was drawing solid accent routes and "N shortlisted units" for candidates that had already declined the movement or failed a hard gate (e.g. WF-009: zero of its three nearest candidates were actually eligible). Every shortlisted node now carries data-eligible plus its own candidateReason() text, ineligible routes render dashed and danger-toned, and the hub states the true eligible count instead of the shortlist size. Important 2: destinationUnit() conflated an accepted bed with an outstanding referral into one badge and only looked at referredUnitIds[0]. Split into distinct Accepted destination / Outstanding referral badges, checked directly against the movement's own fields, so every parallel referral is visible. Important 3/4: the routed-set and route-connector assertions now hold for a second movement with a different shortlist (WF-009), and route connectors are marked data-connector-kind so they're provably distinct from demand connectors, closing the gaps a single-movement proof and an unmarked connector left open. Important 5: routed state now reaches the accessible name as real text (candidateReason), not just an outline colour behind an aria-hidden svg. Minor 6/7: a unit that fails its service-group lookup renders as an explicit anomaly instead of vanishing; the window resize listener now attaches unconditionally rather than being gated behind ResizeObserver's own availability check. Controller finding 8: the region grid's row-sizing squeezed the diagram to 208px of 1080px content at 1280x900. `.regionGrid` now sizes to its own content (align-self: start) only in the stacked (<90rem) layout, `.diagramRegion` gets a real min-height floor, and `.queueRegion` gets a matching cap so it doesn't balloon to all 41 unclamped rows — the >=90rem three-column layout is untouched (restored to align-self: stretch there), matching its original, already-working behaviour exactly.
…ates
Task 7: builds ShortlistPanel, the surface Phase 1's whole-branch review found
rendering a green tick beside "is not authorised under the Mental Health Act".
Every gate row now states its own verdict in text ("Met"/"Not met"), all eight
gates render every time (never .slice()'d), the icon reads directly off
gate.pass, and an ineligible candidate is marked with a dashed danger border
(forced-colors safe) and never styled as a recommendation. Confirm/override are
the only way to place a patient — nothing auto-allocates.…hortlist (I2)
RECORD_ESCALATION existed in the events union, the role table, the reducer,
and had a unit test -- but no surface anywhere dispatched it
(grep -rn "RECORD_ESCALATION" src/ returned only the events file and the
reducer). Spec section 11 was moved into Phase 3 from Phase 4 specifically
because "a phase that only proves the loop which succeeds has not proved the
loop", and the plan's own spec-coverage line already claimed this shipped.
Added an Escalation section to shortlist-panel.tsx, placed after Declines
(which already renders what was tried and why each failed, via real decline
rows) and before the operational-score details. The control to record a new
escalation renders only while `topEligible === undefined` -- the same
condition the header already uses for "No eligible destination found yet." A
recorded escalation (`movement.escalation`) always renders as a persistent
fact regardless of that condition, the same way an already-answered override
record stays visible.
`RECORD_ESCALATION`'s reducer branch carries no precondition beyond the role
check, so this control can never be refused -- no *BlockedReason guard is
needed, unlike every other clinical control in this phase.
`triedUnitIds` is `movement.declines.map(d => d.unitId)`, never the panel's
own `shortlist` (which is capped at PARALLEL_REFERRAL_CAP and is a
theoretical eligibility scan, not a record of what was actually attempted).
WF-009's own pre-authored fixture escalation already uses exactly this
shape -- its five triedUnitIds are its five declines, unit for unit -- so
this makes the runtime writer match the fixture's own established
convention. Free text is limited to `contact` (a role or service, never a
person), labelled as such.
Added a Playwright test asserting: the pre-authored fixture escalation on
WF-009 renders correctly before any dispatch; submitting a new escalation
genuinely overwrites it with live contact text and the real 5-decline tried
count (not a placeholder); and the record persists across a selection
change and back (it lives on `movement.escalation`, not local component
state). Mutation-tested: forcing `triedUnitIds` to `[]` turned the test red
("Expected substring: tried 5 unit, Received: tried 0 units"); reverting
turned it green again (8.4s). Full ui-ward-coordinator.spec.ts,
ui-ward-management.spec.ts and ui-ward-roles.spec.ts re-run clean: 41 passed.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 were implemented and tested in the reducer
from Task 3 onward but dispatched only from test-harness buttons -- no
product surface ever raised either event, so bedHeldUntil (always now + 60)
could never actually be watched expiring.
Added WardDemoControls, a self-contained popover component (its own CSS
module, following ward-role-switcher.tsx's own trigger+menu pattern) mounted
once in ClinicalRail next to WardRoleSwitcher, so it is present on every
/ward-management/* route without per-screen wiring -- the clock is shared
provider state, not a per-screen concern.
Made it unmistakably non-clinical with three redundant signals: a distinct
warning-toned palette (never the role switcher's blue), a flask icon
(FlaskConical, unused anywhere else in this phase, carrying no borrowed
clinical meaning), and every visible string saying "demo" or "scenario"
never a clinical verb -- the open menu leads with an explicit "Demo tool,
not part of the clinical record" notice before any button is reachable.
ADVANCE_CLOCK/RESET_SCENARIO carry no reducer precondition beyond the role
check (EVENT_ROLE's "demo" role), so unlike every clinical control in this
phase, neither button needs a *BlockedReason guard -- raised with
role: "demo", the reducer can never refuse either.
Added a Playwright test on the Ward screen: holds a bed on the fixture-pinned
WF-003 (accepted_awaiting_bed at rph-adult-secure), asserts the trigger's
aria-label and title both state "not a clinical action" in words, advances
the clock 45 minutes via three "+15 min" clicks, and confirms the hold's
remaining time moves from "1h 00m left" to "15m left" -- the exact thing
spec §5 says the control exists to demonstrate. Mutation-tested: forcing
ADVANCE_CLOCK's minutes to 0 turned the test red (stuck at "1h 00m left");
reverting turned it green again (6.8s). Full ui-ward-coordinator.spec.ts,
ui-ward-management.spec.ts and ui-ward-roles.spec.ts re-run clean: 42 passed.…ciliation Written ahead of a chat transfer so it exists whenever this session stops. The branch is DIVERGED - 8 local commits, 10 remote - because a second session worked the same defects and pushed. The handover leads with that, names the four things never to do (force-push, hard reset, blind pull, checkout over uncommitted work), and carries ruling R74's per-property reconciliation plan: local for live unit capacity because theirs uses a defaulted fallback to the frozen fixture that the Global Constraints forbid; theirs wholesale for expired bed holds because spec line 201 requires it and my side never implemented it; neither for the Form 1A expiry the owner cancelled. Also records R73 and why it matters more than it looks: an agent invented a statutory figure, the owner's correction exposed fourteen more fabricated Form 1A deadlines nobody had measured, and his second answer removed the category. Task 6A's F17 had replaced one fabrication with another and nobody checked. The standing prohibition is now absolute - no agent cites, paraphrases or infers any figure from the Mental Health Act. Marks clearly which baselines are verified first-hand (through 421d9a6) and which are still only reported (the six commits after it).
- ward-flow-reducer.ts: closing an examination as community_order/ revoked now cancels in-flight transport and releases any bed already held at the accepted unit, and every downstream transition (referral, acceptance, hold, decline, handover, transport, arrival, escalation) now rejects once a movement carries a `closure`. Reproduces and fixes the reported defect where TRANSPORT_EN_ROUTE still succeeded on a movement already recorded as revoked, with its bed still reserved. - ward-screen.tsx: resolves the unit from the provider's live `units` instead of the frozen `unitById()` fixture, so CONFIRM_CAPACITY/ HOLD_BED updates are reflected on this screen instead of the seeded count. - ward-flow-provider.tsx: elapsed time is now accumulated across 30s checkpoints instead of compared against the original mount instant, so a session open past 24h (or any multiple of it) keeps counting forward instead of resetting to zero on an exact-24h wall-clock reading, which previously moved every deadline/wait/hold on every screen backward by up to a day. Each fix carries a regression test; the provider one is confirmed to fail against the pre-fix code (702 vs expected 2142). Addresses the three unresolved P1 review threads on PR #2289 as part of a Run PR sweep.
Every Form 1A `dueAt` in the fixture was an invented number the previous agent (this same task, before a mid-task re-scope) had derived from a 24h/72h rule it attributed to the clinician. Asked directly, the product owner's actual instruction was narrower: "please can you leave the legal part and just start a clock once the patient arrives to ED. Keep it simple for now." So this removes the legal countdown from Form 1A entirely rather than deriving a corrected figure for it — the same move Task 6A already made for Form 3B. - Deleted `EXAMINATION_REFERRAL_WINDOW_MINUTES` and its doc comment asserting what the Mental Health Act requires; that assertion (an earlier agent's own recollection, not the clinician's) is what caused this task. - Every Form 1A — the 5 hand-authored fixture records, the generated fixture, and `RAISE_REFERRAL`'s runtime constructor (`initialLegalForm` in ward-flow-reducer.ts, whole-branch-review finding I5, unchanged) — now carries no `dueAt`. `LegalForm.dueAt` stays optional, as it already was for a Form 3B. - Every reader (priority-queue, shortlist-panel, ward-pressure, buildActionInbox, ward-management-console) already guarded an absent `dueAt` correctly from Task 6A; only their comments needed correcting — including a self-inflicted overclaim caught during this fix: the transport/transfer forms (4A/4C) are out of scope and still carry a real `dueAt`, so "no LegalForm carries a dueAt" was wrong as stated and is now scoped to "neither a 1A nor a 3B" throughout. - `operationalScore`'s "Statutory timing" factor is documented as dormant for 1A/3B specifically, not fully dormant — 4A/4C still score through it today (verified: WF-006, WF-014). - Rewrote the 1A/3B invariant test (`ward-model-phase3.test.ts`) to assert neither code ever carries a `dueAt`, added a WF-303 regression test in `ward-derivations.test.ts`, fixed the `RAISE_REFERRAL` test's dueAt assertion, and repointed the coordinator's F17 breach assertion (`ui-ward-coordinator.spec.ts`) at a whole-page absence of "passed its deadline" — repointing it at `ED_ACCESS_TARGET_MINUTES` instead was considered and rejected because nothing in the fixture currently exceeds that target either. Every added/changed assertion was mutation-tested by reverting the change it proves and confirming the test goes red, then reverting back. RAG impact: no retrieval behaviour change — this touches only the ward-management prototype (src/components/ward-management/**), which is outside src/lib/rag and every other protected retrieval/ranking surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tent
Two sessions worked `codex/ward-management-design` in parallel and both fixed
the same whole-branch-review findings. Divergence at merge: 10 local, 11
remote. Resolved per file rather than per side, because neither side was
wholly right.
Taken from THIS side:
- Critical 1 (live unit capacity). Both sides fixed it; this side's signature
is `eligibleCandidatesAmong(movement, units, now, limit = 3)` with `units`
REQUIRED. The other side used `eligibleCandidates(movement, now, limit = 3,
units = allUnits())` -- a defaulted fallback to the frozen fixture, which
the plan's Global Constraints forbid by name ("no defaulted-parameter
equivalent") because every existing call site keeps compiling while
silently reading frozen capacity. This side also carries the parser-based
static guard and the Playwright test a reverting mutation was confirmed to
kill.
- The Form 1A deadline removal (9c217c8) and `initialLegalForm`.
Taken from the DIVERGED side, because this side had none of it:
- Closure guards in the reducer (6e1300f). Closing an examination as
community_order/revoked left an in-flight transport running and a bed still
reserved, and TRANSPORT_EN_ROUTE still SUCCEEDED on a movement already
recorded as revoked. Ten `if (movement.closure) return reject(...)` guards,
plus bed release and transport cancellation on the closing transition.
- Elapsed-time accumulation in the provider (6e1300f). `wallClockNow()`
returns a minute-of-day, so comparing against the original mount instant
unwraps exactly one midnight; at 24h (or any multiple) elapsed reset to
zero, moving every deadline, wait and hold on every screen backward by up
to a day. Their regression test fails against the pre-fix code, 702 vs 2142.
- Expired bed holds in `buildActionInbox` (a5716d8) and the "Reconfirm or
release bed hold" action label (1a6f163). This is spec line 201 -- "an
expiring hold raises an exception, it never auto-releases the bed" -- which
this side implemented nowhere and which every task and the whole-branch
review both missed. `formatRemaining` was checked rather than assumed: it
renders a negative as "10m overdue", so the row states its own data
honestly.
- Their unit-level live-capacity test, adapted to this side's signature, and
their 1A-to-3B round-trip assertion, which proves the created form is
EXAMINABLE rather than merely present.
Rejected from BOTH sides:
- `FORM_1A_REFERRAL_EXPIRY_MINUTES` / `EXAMINATION_REFERRAL_WINDOW_MINUTES`.
Both sessions independently invented a 72-hour statutory window and
attributed it to the Mental Health Act 2014. The product owner's actual
instruction was narrower: leave the legal part out and just count up from
ED arrival. No Form 1A or 3B carries a `dueAt` anywhere now.
- `...(awaitingExamination ? { formedAt: event.now } : {})` on RAISE_REFERRAL.
`formedAt` means formed in the community BEFORE reaching ED --
`isCommunityFormed` is literally `formedAt !== undefined && formedAt <
openedAt`. Setting it equal to `openedAt` records a community forming that
did not happen. Inert today only because every reader guards on the strict
`<`; dropped rather than left as a false fact in the record.
Also corrected here: `ward-derivations.ts`'s doc comment still pointed at the
frozen `eligibleCandidates` wrapper that R70 deleted -- an incomplete fix on
this side, found while resolving the conflict in that same hunk.
Left deliberately unchanged, and flagged instead: `initialLegalForm` gives a
Form 1A to every non-voluntary draft, including "Involuntary inpatient",
which the ED intake form does offer. A 1A is a referral FOR examination, so
an already-involuntary inpatient arguably should not receive one. The
diverged side's narrower condition is the more defensible reading, but
adopting it would change a landed, reviewed test, and which statuses warrant
a 1A is a clinical question for the product owner rather than a merge
decision.
Verified first-hand, not taken on either session's report:
- `npx tsc --noEmit` -- clean, zero output lines.
- Node-env ward suites, one invocation, 14 files: 207 passed.
- jsdom suites, one invocation each: ward-screen 4, ward-flow-provider 5,
ward-flow-clock-consistency 1, ward-flow-queue-selection 1.
RAG impact: no retrieval behaviour change -- this touches only the
ward-management prototype (src/components/ward-management/**), which is
outside src/lib/rag and every other protected retrieval/ranking surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Land Phase 3 on the live #2140 base without dropping post-merge tap-target, column-header, and forced-colors patches. Retired constellation stays as a network redirect so existing main bookmarks do not 404. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…to codex/ward-management-design
… a11y assertion Two post-merge defects, both found by re-checking invariants AFTER the merge rather than before it. 1. `FORM_1A_REFERRAL_EXPIRY_MINUTES = 7 * 24 * 60` was back in ward-model.ts, doc-commented as "the Mental Health Act's seven-day outer limit". It arrived by AUTO-MERGE from the rejected commit e8f9805 -- ward-model.ts never appeared in either conflict list, so nothing asked about it. It had zero real consumers: the only reference was an orphaned import in tests/ward-flow-reducer.test.ts. Both removed. This is the third invented statutory figure in this phase and the second attributed to the Act in a doc comment. One session invented 72 hours, another invented seven days; neither came from the product owner, whose actual instruction was to leave the legal part out and count up from ED arrival. 2. tests/ward-management.test.ts carried a broken assertion on this branch: expect(modesSource).not.toMatch(/<th(?! scope="col")/); The negative lookahead matches `<thead>` -- `<th` followed by `ead>` is not followed by ` scope="col"` -- so it can never pass on a file containing a table head, regardless of whether every real `<th>` is correctly scoped. All twelve are. main had already replaced it with a version that matches only real `<th>` tags and then checks each one; taking main's file fixes it and keeps this branch aligned with what CI actually runs. Verified: node-env ward suites, one invocation, 14 files -- 208 passed. RAG impact: no retrieval behaviour change -- this touches only the ward-management prototype and its tests, outside src/lib/rag and every other protected retrieval/ranking surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…gged The pre-push guard checks the PUSHED COMMIT in an isolated worktree, not the working copy. On this Windows checkout the working tree carries CRLF while git stores LF, so a local `prettier --check` passes on content the guard then rejects -- exactly the trap AGENTS.md documents. Formatting only; no behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ui-ward-management.spec.ts` "opens every Ward Flow mode" has failed intermittently across sessions, at a different mode link each time, and was recorded as a suspected flake without ever being confirmed. It is not a flake. It is a fixed, sequential, deterministic test whose time budget is too small for the work it does when the machine is loaded. Measured at HEAD 12f17b1. The test performs one page load plus seven sequential route navigations: warm (.next populated) initial 1.63s worst nav 0.55s whole test 3.78s cold (.next deleted) initial 8.58s worst nav 1.52s whole test 15.24s Against budgets of 10s per assertion (the config default, because the loop's assertion set none) and 45s for the whole test. On an idle machine it passed 18 of 18 runs, including fully cold. The sessions where it failed ran the same 42-test Chromium gate in 10.1 and 10.6 minutes; that gate takes 1.8 minutes here, so the machine was roughly six times slower. Six times the cold whole-test cost is ~91s, past the 45s budget; six times the cold worst navigation is ~9.1s, at the edge of the 10s default. The clock therefore expires at whichever link it happens to reach — which is exactly the reported "different link each time". Confirmed by mutation rather than argued: cutting the describe budget to 3s made it fail 4 of 4 runs, always at the loop's assertion, at a different mode link across runs. Restored byte-for-byte, it passed 3 of 3. The loop's assertion was also the only route arrival in this file with no explicit allowance; the file's other two both use 15_000. This weakens nothing. The assertion still fails when a mode route does not render: mutating an expected test id fails the test naming that locator, and Playwright reports "Timeout: 15000ms" rather than the 10000ms config default, which is direct proof the new allowance is in force. CI runs this spec against a production build, which scripts/run-playwright.mjs builds and serves, so no on-demand compilation happens there and a passing test never consumes its timeout. What it does cost, recorded in the comment rather than left implicit: playwright.config.ts sets no actionTimeout or navigationTimeout, so a genuinely hung navigation in this one test now burns 120s rather than 45s before failing — about +75s on a Chromium-only PR shard and about +225s across the three browsers of verify:release, both well inside those jobs' budgets. Verification: single test 1 passed; whole file 5 passed, the other four still inside the unchanged 45s describe budget; prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reconciles the branch with `main` so the unlanded Ward Flow work can reach it. 36 files conflicted; every conflict was resolved per file and per hunk with intent, never per side, because neither side was wholly right. Why 36. PR #2289 squash-merged this branch's Phase 3 work into `main` on 2026-08-22. A squash is not an ancestor of the branch, so git treats identical lineage as unrelated content: most of the conflict was the branch's own earlier work coming back at it. 135 hunks in total, of which 41 were mechanical repetitions of one already-decided rename and the rest genuine judgement. Which side is newer flips by concern, and that is what the resolution turned on: the fabricated Mental Health Act figure, dueAt on a 1A or 3B, and statutory wording -> take main. PR #2340 is the newer correction, reviewed twice, and it carries a guard test the branch has never seen. Phase 3 behaviour main lacks: reducer closure guards, live capacity, the demo clock, expired bed holds, the role switcher -> take HEAD. This is the work the merge exists to deliver. accessibility, forced-colors, CSS, layout and performance -> take main. Ruling R76: main's design-system work must survive the ward add/add conflicts rather than be silently reverted. Source 76 hunks (62 HEAD, 11 main, 3 combined); tests 51 hunks (32 HEAD, 14 main, 5 combined); config and generated docs 8 hunks, all main, with the five generated files regenerated rather than hand-merged. Two hunks genuinely contradicted, and both were ruled rather than guessed. Which legal statuses receive a Form 1A on RAISE_REFERRAL. Taken from main, which gives one only to a patient actually awaiting examination. The branch gave one to anyone not voluntary, including a patient already recorded as an involuntary inpatient, which would let RECORD_EXAMINATION fire on someone already examined. Measured against the fixture: seven involuntary-inpatient records, none carrying a 1A. This settles open question 3 in the handover, in the conservative direction that document itself leaned toward, and it is flagged to the product owner as settled by merge rather than left to be discovered. WF-018's withdrawn referral. Taken from HEAD's empty list: the record carries no acceptedUnitId and no declines, so the withdrawal it displayed can never have happened. No test was dropped: per-file counts confirm every resolved file carries at least as many as the larger side. One assertion was deliberately not kept, main's toContainText(/Parallel referral|referred to 3/i) in ui-ward-roles.spec.ts, which a single badge satisfies while its own comment claims three. HEAD's toHaveCount(3) on the testid replaces it. Keeping both would have kept a check that cannot fail beside one that can. Invariants re-checked AFTER the merge, never only before. Ruling R81 exists because the third fabricated statutory figure in this project arrived through a file that appeared in no conflict list: no statutory duration constant declared anywhere in src no Form 1A or 3B carries a dueAt; the four that remain are 4A and 4C tests/ward-legal-figure-guard.test.ts present and passing both render sites read "no deadline recorded" zero unattributed claims about what the Act imposes, across src, tests, scripts, docs and worker. Ten code sites were fixed, two of them in files that were never conflicted and appeared in no task list eligibleCandidatesAmong still takes units as a required parameter, with its static guard and its Playwright test intact sex remains the only patient attribute main's accessibility and CSS work survived; the only ward .module.css delta from main is additive all three ward specs registered in testMatch, productionSpecPattern and the shard groups Verification: tsc --noEmit clean, exit 0, zero errors. Ward unit suites, 20 files, 239 passed, more than either side carried alone. Adoption and route-reachability, 59 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
BigSimmo
enabled auto-merge (squash)
August 24, 2026 13:06
Contributor
Warning Review limit reachedNext included review available in 40 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 100 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (32)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Approved. Cursor Bugbot was not present on this head after the first poll, so that signal was skipped; no applicable approval policy or unresolved automated-review findings require human review. No reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
💡 Codex Review
Resubmit this clinical workflow review at xhighThis commit changes ward-management prioritization, pressure, movement, and coordinator behavior, which is a clinical-safety-sensitive workflow. Repository policy requires xhigh reasoning before substantive inspection of this scope. Please select xhigh in the Cloud reasoning control and resubmit the same request with the exact marker [xhigh-confirmed]. No PR was generated.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #13648 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
`Static PR checks` failed on `docs:check-links` — three paths referenced by `docs/ward-flow-phase-3-handover.md` did not resolve. All three were pre-existing defects in that document, surfaced rather than caused by the merge, which took the branch's copy of it. Two were truncations, not missing files: the table abbreviated the spec and plan filenames with a literal ellipsis to fit the column, so the link pointed at a path that never existed. Both files are present under their real names and the references now use them. The third was genuinely absent. `docs/ward-flow-phase-3-workspace/whole-branch-review.md` was referenced but never committed — the workspace directory carries all 49 other briefs, reports and reviews, and the whole-branch review was the one record missing from it. Restored from the handover package. Vetted before committing: it asserts no Mental Health Act figure or requirement, and its only mentions of identifying data are the review's own confirmation that none is rendered. Verification: docs link check passes, 3207 repo path references resolve. Prettier clean on both files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…to codex/ward-management-design
Contributor
There was a problem hiding this comment.
Approved. Cursor Bugbot was not present on this head after the first poll, so that signal was skipped; no applicable approval policy or unresolved automated-review findings require human review. No reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
main. PR Codex/ward management design #2289 squash-merged an earlier snapshot of this branch on 2026-08-22 and the branch kept going afterwards. This reconciles the two so roughly a thousand lines across 25 ward files can land: the reducer's closure guards (without them, transport still succeeded on a patient already recorded as revoked, with their bed still reserved), the elapsed-time accumulation fix (a session open past 24 hours reset every clock to zero, moving every deadline backward by up to a day), the live-capacity fix, the demo clock controls, expired bed holds in the action inbox, and role-switcher work.dueAton a Form 1A or 3B, and statutory wording →main, because PR fix(ward-flow): remove the fabricated Mental Health Act figure #2340 is the newer correction, reviewed twice, and carries a guard test this branch has never seen;mainlacks → the branch, since that is the work this merge exists to deliver;main, because ruling R76 exists precisely somain's design-system work survives ward add/add conflicts rather than being silently reverted.main, 3 combined); tests 51 hunks (32 branch, 14main, 5 combined); config and generated docs 8 hunks, allmain, with the five generated files regenerated rather than hand-merged.Two hunks genuinely contradicted, and both were ruled rather than guessed
1. Which legal statuses receive a Form 1A on
RAISE_REFERRAL— this one is clinical and needs the product owner's eye. Taken frommain, which gives a Form 1A only to a patient actually awaiting examination. The branch gave one to anyone not voluntary, including a patient already recorded as an involuntary inpatient — which would letRECORD_EXAMINATIONfire on someone already examined. Measured against the fixture: seven involuntary-inpatient records, none carrying a 1A.This settles open question 3 in the Phase 3 handover, which asked exactly this and noted that a 1A refers someone for examination so arguably an involuntary inpatient should not receive one.
main's narrower behaviour is the conservative direction and the one that document leaned toward. It is flagged here rather than left to be discovered. If the product owner wants it the other way, it is a one-line change and a clinical judgement, not an engineering one.2. WF-018's withdrawn referral. Taken from the branch's empty list: the record carries no
acceptedUnitIdand no declines, so the withdrawal it displayed can never have happened.Test integrity
No test was dropped. Per-file
it(/test(counts confirm every resolved file carries at least as many as the larger side.ward-flow-reducer.test.tsgained one, because the voluntary-referral assertion — whichmainhas nowhere — was lifted into its own test rather than being swallowed by anit.each.One assertion was deliberately not kept:
main'stoContainText(/Parallel referral|referred to 3/i)inui-ward-roles.spec.ts, which a single badge satisfies while its own comment claims three. The branch'stoHaveCount(3)on the testid replaces it. Keeping both would have kept a check that cannot fail beside one that can.Verification
Invariants were re-checked after the merge, never only before. Ruling R81 exists because the third fabricated statutory figure in this project arrived through a file that appeared in no conflict list — a pre-merge check cannot see that, and a clean auto-merge is not evidence that nothing changed, only that git had no question to ask.
srcdueAt; the four that remain are Forms 4A and 4C, which carry real operational deadlines about moving a persontests/ward-legal-figure-guard.test.tspresent and passingno deadline recorded, neverno statutory deadlinesrc,tests,scripts,docsandworker. Ten code sites were fixed — two of them in files that were never conflicted and appeared in no task list, found only because the sweep was made repository-wide rather than per-fileeligibleCandidatesAmongstill takesunitsas a required parameter, with its static guard and Playwright test intactmain's accessibility and CSS work survived; the only ward.module.cssdelta frommainis additivetestMatch,productionSpecPatternand the shard groupsGates, all run first-hand on the committed merge:
npx tsc --noEmit -p tsconfig.json— clean, exit 0, zero errorsui-ward-coordinator,ui-ward-management,ui-ward-roles— 42 passed in 1.5 min, zero failures, against a warmed dev server on this worktree's printed portnpx prettier --checkon all 34 files that differ frommain— cleanVerification not run:
npm run verify:pr-local, and no provider-backed gate was touched — no OpenAI, Supabase, GitHub Actions or live database call was made from this work.Risk and rollout
main. The clinical surface that moves is the Form 1A rule in ruling 1 above, which narrows what the system asserts rather than widening it.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)On the last item: Ward Flow is a synthetic, offline prototype and is not clinical decision support. It touches no live database, no patient data and no OpenAI call; every movement in it is invented, and sex is the only patient attribute the model carries. Nothing here alters that classification, and the one clinical behaviour change narrows what the system claims.
Notes
mainat time of writing (e070afb07, a CI-policy change touching no ward file). Not synced, to avoid cancelling an in-flight CI run for no benefit.Note
Medium Risk
Touches live capacity, referral eligibility, and legal-form attachment on every Ward Flow screen. Wrong wiring would show stale beds or attach the wrong Form 1A, but this remains a synthetic offline prototype.
Overview
Live unit state is now required everywhere. Shortlists, diagrams, ED/ward/officer screens, and role switching resolve units from the provider’s
units, notallUnits()/unitById.eligibleCandidatesis replaced byeligibleCandidatesAmong(movement, units, …)with no default, so frozen capacity cannot silently return.Coordinator honesty. Escalation can be recorded when no eligible destination remains (tried units come from real declines). The exceptions trigger shows a separate refusal badge. Fixture records that could not exist under the reducer (live referral at
placement_requested, withdrawal with no acceptance) are cleared.Demo clock (
+15 min/+1 hour/ reset) is mounted in the clinical rail, labelled as non-clinical, so held beds can be watched expiring.Legal forms. New referrals awaiting examination get a Form 1A with no
dueAt; voluntary referrals get none. 1A/3B statutory scoring stays dormant.Guards and tests: AST allow-list for
allUnits/unitById, fixture-coherence contracts, and a no-reload live-capacity journey.Reviewed by Cursor Bugbot for commit 5188cae. Configure here.