Skip to content

Move image reading onto the episode as reading cases - #385

Open
edwardhorsford wants to merge 10 commits into
mainfrom
reading-cases
Open

Move image reading onto the episode as reading cases#385
edwardhorsford wants to merge 10 commits into
mainfrom
reading-cases

Conversation

@edwardhorsford

Copy link
Copy Markdown
Contributor

Moves image reading data off the appointment and onto the episode, as reading cases, and gives past rounds enough detail to be worth looking at.

Why

Reads lived on appointment.imageReading, keyed by user, with the arbitration read implied by happening to be third. That shape doesn't survive contact with arbitration: reads can be withdrawn (deferring after giving an opinion does exactly that), which shifts positions and makes "the third read" mean nothing.

Production models this as reading cases — taking mammograms triggers a case, and the reads belong to the case rather than the episode directly. One case per set of images, so a technical recall's re-screen produces a second case.

What changed

Reading cases. episode.readingCases[], one per image set, keyed to the appointment that produced it. Reads are an ordered array, each carrying its own readType (first / second / arbitration), settled when the read is written — because once there's a manual step in front of arbitration, whether a read was an arbitration read depends on where the case had got to at the time, and nothing can recover that afterwards.

getOutcome split in two. It returned a mix of process states (not_read, arbitration_pending) and clinical answers (normal, technical_recall) in one string. Now:

  • getReadingCaseState — where the case has got to: awaiting_first_read, awaiting_second_read, arbitration_required, in_arbitration, concluded
  • getReadingCaseOutcome — what it found, or null while reading is still under way

arbitration_required and in_arbitration are deliberately separate. Reads disagreeing makes arbitration necessary; a case only becomes arbitratable once someone releases it. Nothing performs that release yet, so nothing reaches in_arbitration — the state exists so the vocabulary is whole rather than growing a value later across every call site. Deferral already works this way: the act is recorded, and the state is read back from its presence.

Module split, one-directional. reading-cases.js is pure case logic and takes a case. episodes.js owns getting cases out of session data and writing them back, because both go through the episode. reading.js is the appointment- and session-shaped layer above — sessions, backlogs, progress — which is why most of its helpers now take data. Cases are resolved once, at the edges: the reading workflow middleware sets res.locals.readingCase, and list-building attaches it per row.

Episode stage comes from the latest case, which fixes the known technical-recall bug where a re-screen didn't supersede the reading that asked for it.

Past rounds are worth looking at now. Each screened historic episode carries a summary reading case and a stand-in appointment (summaryAppointments[]) holding the medical information recorded that day. Both sit on the episode rather than in data.appointments, so they can't drift into a clinic list or a reading queue — there's nothing to open, and the episode page's links deliberately go nowhere. Detail is conditioned on the outcome the round was already seeded with: a round that ended in treatment was recalled for assessment, most clear rounds were read normal, and a few went to arbitration.

Seed profiles reach history. episodes.historicOutcomeWeights existed as a hook nothing set, so "All normals" still generated past referrals for treatment. Now wired, and historic medical information follows the profile's probabilities too.

Bugs found on the way

  • Lazy reading sessions were destroyed by resuming them. /resume looped topUpSession until it stopped adding, to replace slots taken by other readers — but that call grows a session toward its target size, so it loaded every remaining case at once. A session went from 1 case to 10 on a single resume. Pre-existing.
  • The technical-recall seeding was mistyping a read as arbitration, caught by the new checkEpisodes assertions.
  • Two intermittent e2e failures, both pre-existing: full names were built without middle names, and controls low inside a modal can't be scrolled into view (the dialog is overflow: clip and scrolls via an inner element).

Verification

npm test green — lint, 104 routes with no server errors, and 7 journeys, run repeatedly to shake out the flakes above. Three new reading journeys went in first, as a net under the refactor: technical recall, defer + unflag, and the second-reader comparison. Between them the reading journeys now cover all four ways a reader can leave a case. A fourth journey pins the lazy-session fix.

Data regenerates clean with no checkEpisodes warnings, and no appointment retains imageReading.

Deliberately not here

  • The arbitration flow itself — the gate, the arbitration read, the resolution step. The data shape and state vocabulary are ready for it.
  • The reading backlog / case views. The state values map 1:1 onto the planned tabs.
  • Medical information building across a participant's rounds. Each round still generates independently, so history isn't yet coherent round to round; that needs a wider change to generators the live appointment flow also uses.
  • Deferral and priors storage — written up separately as a spike rather than changed here.

…parison

Safety net before moving reading data onto the episode. Between them the
reading journeys now cover all four ways a reader can leave a case.
Reads lived on appointment.imageReading, keyed by user, with the arbitration
read implied by being third. They now live on episode.readingCases[] - one
case per set of images, keyed to the appointment that produced it.

- reads are an ordered array, each carrying readType (first/second/arbitration)
  settled when the read is written, since a withdrawn read shifts positions
- deferral moves to the case; priors stay on the appointment
- getOutcome splits into getReadingCaseState (where the case has got to) and
  getReadingCaseOutcome (what it found, null while reading is under way).
  arbitration_required and in_arbitration are separate states, ready for the
  manual gate that releases a case into arbitration
- episode stage derives from the latest case, fixing the technical-recall snag
- reading-cases.js is pure and takes a case; episodes.js resolves and writes
  them; reading.js is the appointment/session layer above

checkEpisodes gains assertions for the new shape. Docs updated.
/resume looped topUpSession until it stopped adding, to replace dead slots -
cases fully read by other readers, which can leave a session with nothing
readable while the backlog still has cases. But topUpSession grows a session
towards its target size, so running it to exhaustion loaded every remaining
case at once and defeated lazy sessions entirely.

Top up only until a readable case appears. Normally there already is one, so
nothing is added.

Pre-existing, not from the reading cases move.
Past rounds recorded an outcome but nothing about how they got there, so an
episode page for a previous round had no reading to show. Each screened
historic episode now carries one reading case in the same shape as a live one,
with appointmentId null - there is no appointment record behind a past round.

Reads are conditioned on the outcome the round was already seeded with: a round
ending in treatment was recalled for assessment, a clear round was usually read
as normal, and a minority were recalled and then cleared at assessment.
checkEpisodes enforces the coherence.

Also fixes the e2e seed helper building full names without middle names, which
made the appointment journey fail whenever it happened to pick a participant
who had one.
…them

A past round recorded an outcome and, since the last commit, its reading - but
nothing about what was recorded on the day. Each screened historic episode now
carries episode.summaryAppointment: the round's date and the medical
information taken at it.

It sits on the episode rather than in data.appointments on purpose. Nothing
links to it yet, and keeping it out of the appointments collection means it
can't drift into a clinic list, a reading queue, or a route expecting a real
appointment record.

Recorded dates are stamped back to the round's screening date. The medical
information generators date everything "now", which is right for a live
appointment and wrong for a past one.

Seed profiles now also shape history: episodes.historicOutcomeWeights was a
hook nothing set, so "all normals" still produced past referrals for treatment.
It now defaults explicitly and is overridden by allNormals and
highAbnormalities. Historic medical information follows the profile's
probabilities too.
Past rounds held their stand-in appointment as a single object, and showed it
only as a block of medical information. It is now episode.summaryAppointments[]
- a list, like the episode's other per-appointment records - and the episode
page renders it as an appointments table the same way a live round does, with
"Summary only" where a live round has a link. An episode can hold more than one
appointment, and a past one should have somewhere to show that.

Medical information was only surfaced on historic episodes, which left the two
kinds of episode page inconsistent. Open episodes now show it too, from the
latest appointment that recorded any.

Also fixes a flake in the reading spec: the annotation modal scrolls via
.app-modal__content and the dialog itself is overflow: clip, so Playwright
could not bring Save into view on a long form. The test now scrolls the content
area the way a user would.
A few percent of past rounds now went to arbitration - the two readers
disagreed and a third settled it - so the arbitration work has seeded examples
rather than only cases made by hand. The deciding read carries the round's
conclusion, and checkEpisodes allows the third read.

Episode pages show reading as a table of cases, each linking to the case view
the reading backlog work will serve, rather than listing individual reads. The
reads belong on the case view. Past appointments link too; both are dead links
alluding to pages that don't exist yet.

Snags cleared: deleted the unused reading/batch.html, renamed the seed profile's
`imageReading` group to `reads` (nothing in the data model is called
imageReading any more), and removed the arbitration and newly-arrived-priors
counts in index-simple.html that were computed and never displayed.

Two e2e fragilities fixed, both intermittent and both pre-existing: the normal
opinion helper only handled cases without symptoms, and controls low in a modal
could not be scrolled into view - now a shared revealInModal helper.
They allude to pages that don't exist yet, so an anchor is more honest than a
speculative URL that would 404.
@edwardhorsford
edwardhorsford temporarily deployed to manage-breas-reading-ca-dsirks July 28, 2026 16:15 Inactive
@edwardhorsford
edwardhorsford temporarily deployed to manage-breas-reading-ca-dsirks July 29, 2026 09:19 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant