feat(worklist): a patient-first work list, panel filters by PCP and insurance, and set-based bulk assign - #551
Conversation
… filter that cannot hide half of Medicare
MM-2 PR 1. A patient-first work list, the PCP and primary-payer panel filters the practice
asked for, and one set-based bulk assign replacing a per-case browser loop.
/worklist was a redirect to /cases on the reasoning that the work list IS the open-case queue.
/cases is one row per GAP, so a patient with four open measures is four rows in four places and
the staffer calling them works one and leaves three. It is now a row per patient carrying every
open gap, with the selection acting on all of them. /cases is unchanged and still linked.
Payer arrives as a Source of Payment Typology code on EmployeeProfile (from the corpus's own
Coverage), with GET /api/payers reporting each code present and its count. The filter is a SET:
the typology is hierarchical, so Medicare is `1` plus `11` (managed care) — 3,927 and 2,900 on
the pilot's live seed — and a single-select would let someone ask for Medicare, receive the
smaller set, and never learn the rest were withheld under a heading claiming to contain them.
The predicate still compares exact codes; grouping is something a caller opts into.
Four surfaces each carried their own `providerId || ageBand || sex` copy of "is a filter
active", which would have skipped the predicate entirely for `payer` — an unfiltered list under
a filtered heading on all four. One hasActiveSubjectFilters now answers it.
The default status set becomes ACTIVE_CASE_STATUSES: statusesFor("open") returned OPEN only,
so moving a case to IN_PROGRESS dropped it off the list it was started from.
CaseQuery.employeeIds pre-filters the panel in SQL, but only where the roster is the complete
set of subjects a case can name — on a WebChart deployment the lookup resolves subjects the
directory does not hold, and building the set from the directory would silently shorten the
list by exactly those people. An empty id set means "nobody matches", never "no filter".
POST /api/cases/bulk-assign is one statement, audited before it mutates, reporting what MOVED
rather than what was asked for. No assignment_source — that column is PR 2's owner-written DDL.
Verified: backend 2,659 pass (1 known stale-artifact failure, green in CI), typecheck clean;
the two new store-contract cases ran on both stores against a real postgres:16 (103/103 ceiling,
100/100 floor) with the null-safe assignee comparison mutation-checked on each dialect; frontend
409 pass, lint and build clean.
…nd the collapsed timeline showed the oldest entry Two code reviews on the previous commit. The one that mattered most was in code this PR had just touched. The roster's filter guard was upgraded to the shared hasActiveSubjectFilters — which sees `payer` — while the call beside it still rebuilt an object from three named fields and dropped it. GET /api/compliance/roster?payer=1 therefore reported a filter as active, applied none, and returned all 20,000 patients under a heading that said Medicare. It is the same defect class as the four copied guards this PR removed, one layer up, and the contract paragraph added alongside them was false as written. RosterFilters now EXTENDS SubjectFilters and the call passes `filters` whole, so the next filter added cannot be dropped the same way; the test asserts that general property rather than one more field. The collapsed audit timeline rendered slice(0, 1) of a list ordered occurred_at ASC, so it showed CASE_CREATED — months old — under a control promising the newest entry. The test missed it by using a newest-first fixture the real store never produces. Also, all introduced by the previous commit: the worklist toast claimed "already assigned that way" over gaps a run had closed; the dashboard date range was read, triggered a refetch and was never sent; a page-size change kept the stale offset; select-all at 100 patients on six routed measures builds 600 ids against a 500 cap, so the button offered an action the server refuses; "Mixed (1)" counted only the gaps that HAD an owner, contradicting its own label; `unchanged` double-counted the closed and the missing so the four response numbers summed past the input; the selection survived a successful assign; and a slow earlier response could overwrite a newer one. The panel pre-filter gains a size cap: the SQLite floor spends one bind per id against a per-statement variable limit while Postgres binds the set as one array, so a ~6,800-id payer selection was a cliff only one store could fall off. Skipping is always correct — the post-filter applies the same predicate — and the test pins that the rows are identical. One fix ships deliberately untested and says so in the code: two payer checkboxes clicked before a re-render made the second clobber the first, and the jsdom harness cannot reproduce it — the nav mock updates params synchronously inside replace and the event helpers flush React between clicks, so both spellings passed. The test was deleted rather than kept as a green assertion over a race the harness defines away. Verified: backend 2,684 tests, 2,661 pass, 1 known stale-artifact failure (green in CI), typecheck clean; Postgres ceiling 103/103; frontend 415 pass, lint and build clean. The roster-filter, timeline-collapse and pre-filter-cap fixes were each mutation-checked.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85e7270af8
ℹ️ 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".
| case "": | ||
| case "open": | ||
| return [...ACTIVE_CASE_STATUSES]; |
There was a problem hiding this comment.
The new open mapping makes IN_PROGRESS cases visible in both work lists, but the cases page exports that view with status=open, while backend-ts/src/routes/exports.ts:46-53 still translates it to only ["OPEN"]; the MCP list_noncompliant query at backend-ts/src/mcp/tools.ts:506 has the same mismatch. Consequently, once an appointment moves a case to IN_PROGRESS, it remains visible in the UI but silently disappears from the corresponding CSV and MCP work list, so propagate ACTIVE_CASE_STATUSES to these readers as well.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
| if (changing.length > 0) { | ||
| await stores.events.appendAudits( | ||
| changing.map((c) => ({ |
There was a problem hiding this comment.
Every successful bulk assignment writes only audit_events through appendAudits, whereas the established single-case path uses recordCaseEvent to atomically write both an ASSIGNED case_actions row and its audit row. Thus assignments made from either new bulk UI are absent from the operator-action ledger, contrary to the CaseEventStore contract that every mutating case action writes both records; add a batch equivalent of recordCaseEvent rather than bypassing case_actions.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| const assigned = await stores.cases.assignCases(changing.map((c) => c.id), assignee); |
There was a problem hiding this comment.
When two bulk requests race on the same active case, both can audit from the same pre-read and then both update it: assignCases only checks that the current owner differs from the requested target (case-store-postgres.ts:474-476 and the SQLite equivalent), not that it still equals the owner read here. Different targets therefore overwrite each other while the later state change's audit names the wrong previous owner, and identical targets can produce a second audit even though the second update changes nothing; use a transaction or compare-and-set against each pre-read owner so the audit and returned changed set describe the same mutations.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
… tools, and bulk assign is compare-and-set Three findings from the review on the PR itself. The open status set was widened for the two work lists and not for their readers. The cases CSV export (?status=open) and the MCP list_cases / list_noncompliant tools still mapped "open" to ["OPEN"], so scheduling an appointment moved a case to IN_PROGRESS and it stayed on the screen while disappearing from the CSV taken off that screen and from the tool serving the same list to a client. A row missing from an export is missing without anyone being told. All four readers now share ACTIVE_CASE_STATUSES. Two further readers keep OPEN-only and are deliberately untouched: outreach-campaign targeting and the case attached to an MCP compliance answer. Neither is the work list, and whether a case someone has already picked up should also receive automated outreach is an owner's question rather than a silent widening. Written down in DATA_MODEL_CONTRACTS §4. assignCases checked that the current assignee DIFFERED FROM THE TARGET, not that it still equalled the value the caller read — which is a different test and loses a concurrent write. A reads a case owned by Alice and audits previousAssignee: Alice; B assigns it to Bob; A's update still matches because Bob differs from A's target, so A overwrites B and the ledger names a transition that never happened. It is now a compare-and-set: each entry carries the assignee the caller read and the row moves only while it still holds it, so a row another operator touched is skipped and its already-written audit becomes a recorded-but-unapplied action. Postgres zips the pairs through unnest in one statement; the floor groups by prior owner, a handful of statements because the distinct set is the assignable accounts plus unassigned. Bulk assign wrote only audit_events, satisfying the hard rule but leaving case_actions — canonical operational state — shaped by how many rows the operator happened to tick. recordCaseEvents is the batch form of the existing atomic dual-write. Verified: backend 2,686 tests, 2,663 pass, 1 known stale-artifact failure (green in CI), typecheck clean; Postgres ceiling 103/103, SQLite floor 100/100; frontend 415 pass, lint clean. The compare-and-set was mutation-checked on BOTH dialects, and the case_actions dual-write and the widened export status each on the floor.
|
All three review findings are addressed in P1 — include IN_PROGRESS in every open-case readerCorrect, and it was the half of this PR's own status change that never got propagated. Fixed in three readers — the CSV export, Two more readers keep OPEN-only deliberately, and they are named in Guarded by a test that flips the seeded case to IN_PROGRESS and asserts it survives P2 — make the bulk update agree with its audit snapshotCorrect, and sharper than the two pre-PR reviews got to: one of them judged this the acceptable side of the ordering tradeoff, which conflated two different tests.
Both conditions are kept and neither is decoration: the compare-and-set fails on a concurrent change, The store contract gained a stale-pre-read case and now runs on both stores. Mutation-checked on each dialect — removing the compare fails it on Postgres and on SQLite. P2 — record bulk assignments in the case-action ledgerAgreed. The audit event alone satisfied the hard rule, but it left Added Verification |
1 participant
Summary
MM-2 PR 1 — the patient-first work list, the PCP and primary-payer panel filters the practice asked for on the 09-09/10 calls, and one set-based bulk assign replacing a per-case browser loop.
A work list of GAPS is not a work list of PEOPLE.
/casesis one row per patient-measure, so a patient with four open measures is four rows in four places and the staffer calling them works one and leaves three./worklist— until now a redirect to/cases— is a row per patient carrying every open gap, with the selection acting on all of them./casesis unchanged and still linked: it is the right view when the question is about a measure rather than a person.Medicare is two codes, and a single-select filter would have hidden one of them. The Source of Payment Typology is hierarchical:
1is Medicare and11is its managed-care child. Measured on the pilot's live seed (maui-py2027-v1, 20,000 patients) that is 3,927 and 2,900. MSSP attribution is Medicare, so the population the ACO asks about is both — and a single-choice filter lets someone pick "Medicare", receive the smaller set, and never learn the rest were withheld under a heading claiming to contain them. So the filter is a multi-select,?payer=takes a repeated or comma-joined value,GET /api/payersreports each code's own count, and an "All Medicare" control selects every code in the category the roster actually has. The predicate still compares exact codes; grouping is something a caller opts into.Three defects found along the way that were already live
providerId || ageBand || sexcopy of "is a filter active". Addingpayerwould have left all four silently skipping the predicate — an unfiltered list under a filtered heading, on every one of them. OnehasActiveSubjectFiltersnow answers it.statusesFor("open")returned["OPEN"]whileACTIVE_CASE_STATUSES— the contract's own definition, used by every open-case rollup — is OPEN + IN_PROGRESS. Moving a case to IN_PROGRESS removed it from the list it was started from, which reads as "it got done".DATA_MODEL_CONTRACTS§4 names this hazard for the counts; the list had it too.?payer=entirely — found by review, fixed in the second commit (below).Design notes worth a reviewer's attention
CaseQuery.employeeIdslets the database return the panel instead of the practice. But on a WebChart-configured deployment the route's lookup resolves live subjects the directory does not hold, so building that set from the directory would omit their cases — a work list short by exactly the people the live integration exists to serve, and a missing row reads as "no gaps". The roster is passed only where it is the complete set. An empty id set is a real constraint meaning "nobody matches", never "no filter". It is also size-capped: the SQLite floor spends one bind per id against a per-statement variable limit while Postgres binds the set as one array, so a ~6,800-id payer selection was a cliff only one store could fall off.CASE_ASSIGNEDevents, then applies — so a failure can only leave a recorded-but-unapplied action, never an unaudited state change (the orderingcase-actions.tsestablished). An unchanged row writes no event. The response reports what moved, andassigned + unchanged + closed + missingpartitions the input exactly.assignment_source. That column is owner-written DDL for PR 2. The store method takes no provenance argument rather than accepting one it could not honour.The review round
Two reviews ran before this PR opened;
85e7270ais their result. The most serious finding was in code this PR had just touched:GET /api/compliance/roster?payer=1returned all 20,000 patients under a heading that said Medicare — the guard was upgraded to seepayerwhile the call beside it still rebuilt an object from three named fields and dropped it.RosterFiltersnow extendsSubjectFiltersand the call passesfilterswhole, and the test asserts the general property (for every key the predicate understands, a value nobody satisfies must return nothing) rather than one more field.Also fixed there: the collapsed audit timeline rendered
slice(0, 1)of a list orderedoccurred_at ASC, showingCASE_CREATEDunder a control promising the newest entry — the test had missed it by using a newest-first fixture the real store never produces. Plus a toast claiming "already assigned that way" over gaps a run had closed, a date-range control that refetched and sent nothing, a page-size change keeping a stale offset, a select-all that could build 600 ids against a 500 cap,Mixed (1)counting only the gaps that had an owner, a selection surviving a successful assign, and a missing stale-fetch guard.One fix ships deliberately untested and says so in the code. Two payer checkboxes clicked before a re-render made the second clobber the first; it now builds from the last-written query string held in a ref. The jsdom harness cannot reproduce the race — its
next/navigationmock updates params synchronously insidereplaceand the event helpers flush React between clicks — so both spellings passed the test written for it. The test was deleted rather than kept as a green assertion over a race the harness defines away.Scope
Verification
Mutation-checked, not just green — each of these fails when the fix is reverted:
payer was DROPPED on the way inslice(-1)→slice(0, 1)IS DISTINCT FROM/IS NOT→<>payerbranchSecurity and compliance checks
GET /api/worklist/**carries the same gate asGET /api/cases(AUTHENTICATED), stated explicitly rather than inherited; gating it tighter was tried and reverted, because it would let the read-only VIEWER role open/casesand be refused the same rows on/worklist.POST /api/cases/bulk-assignis CM/ADMIN via the existingPOST /api/cases/**rule (a second rule for it would sit after that one and never fire).CASE_ASSIGNEDbefore the mutation; unchanged rows write nothing.Documentation
README.mdupdated (not needed)docs/JOURNAL.mdupdateddocs/DATA_MODEL_CONTRACTS.md— §6.2/§6.3 columns (providerId,payerappended, never inserted) and the payer-set semanticsdocs/MCP.md— thepayerfilter onlist_noncompliant, including the two-Medicare-codes trapdocs/guide/10-scenarios.md— the by-gap vs by-patient distinctionNot in this PR, named rather than dropped
worklist.spec.tse2e. Both additive; neither gates PR 2.cases.assignment_sourceand the panel→staff mapping are PR 2, which needs the owner's DDL first.Related issues
Part of milestone M-M (MM-2). Follows #550.