Codex/ward management design - #2289
Merged
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.Contributor
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:20 minutes Limit details: You’ve used the included review currently available. Your 91 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. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (132)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:1813124690
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
marked this pull request as draft
August 22, 2026 14:12
BigSimmo
marked this pull request as ready for review
August 22, 2026 19:02
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:2c9c56e244
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- 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.
BigSimmo
enabled auto-merge (squash)
August 22, 2026 19:36
Contributor
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #13299 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 22, 2026
BigSimmo added a commit
that referenced
this pull request
Aug 24, 2026
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>
7 tasks
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
2 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
Verification
npm run verify:pr-localDuring development, use
npm run verify:cheapas the faster iteration gate before the final PR-local preflight.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsFor retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract changes,
verify:pr-localrunseval:rag:offlineautomatically. Run the offline command directly during iteration before spending a live eval.npm run eval:retrieval:quality(must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed — CI cannot run it (needs live keys), so run it locally and paste the summary. A metadata/governance-weighting change once buried correct docs (recall 1.0→0.76) and only this eval caught it.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-onlywhen answer generation, the synthesis prompt, or answer post-processing changed (grounded-supported must not drop; citation-failure 0)npm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changednpm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedRisk and rollout
Complete this section for clinical, data, API, auth/privacy, workflow, dependency, build, or deployment changes.
src/lib/rag/, retrieval RPCs, golden fixtures, ranking tests; seedocs/rag-behaviour/safeguards.md). Must use one of:RAG impact: no retrieval behaviour change — <reason>RAG impact: behaviour change — canary pair <baseline run> -> <post run>RAG impact: noneor omit for non-RAG PRs)Clinical Governance Preflight
Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes