Repository navigation
Cohorts from long id lists, locked cohorts, and saving cohorts by name - #45
Merged
Merged
Conversation
A sampleList/patientList clause may now name a stored id set
({"kind":"sampleList","set":"<id>"}) instead of carrying its ids. The list
is saved once (POST /api/id-sets, or "+ Add cohort" -> "From a list of
ids"), matched to the studies' own samples or patients first (what matched
nothing is reported), and the document, its ?doc= link and every request
carry only the set's id.
Why: an inline list is written into the SQL. On msk_chord_2024 sample ids
hit ClickHouse's 256 KB max_query_size between 12,000 and 13,000 ids, each
view re-inlined the list, and the readback echoed it: at 10,000 ids every id
appeared four times in an 839 KB response. The ?doc= URL broke first, at
roughly 350 sample ids (Node refuses headers past 16 KB).
How:
- db/migrations/0006_id_sets.sql: id_set (owner, sharing columns as for
custom datasets, members gzipped per study, immutable by trigger, soft
delete) and id_set_access_log. lib/id-set-store.ts is the only module that
touches them; access goes through lib/access.ts. Group deletion unshares
sets too.
- lib/id-sets.ts: the per-request context. The compiler names one ClickHouse
external table per set and study (IN (SELECT id FROM _idset_...)), and
lib/ch.ts uploads it only with a statement that names it, only to the
patient-level connection holding that study.
- lib/id-sets-server.ts: every engine route passes its body as `uses` to
withRequestCustomData, which reads the sets it names as the signed-in
principal. MCP, scripts and signed-out requests do not read sets, and such
a clause is refused by name. carriesPatientIds counts a set as ids.
Answers that read a set bypass the shared result cache (hasIdSets()).
- Inline lists are refused by name past INLINE_ID_SQL_LIMIT (200 KB of ids
per study), pointing at stored sets, instead of ClickHouse's syntax error.
- Sharing an analysis or cohort wider than a set it names warns, as for
custom datasets, and offers to share the reader's own sets the same way.
- DSL 2.7 (no-op upgrade step) so a 2.6 engine refuses a document naming a
set rather than reading it as an empty list. Docs regenerated;
DATA-BOUNDARY.md and PERFORMANCE.md updated.
Verified against the live warehouse with runAnalysis (compare + clinical):
all 25,040 msk_chord_2024 samples in 4.9 s, 416 B document, 23 KB response
(inline: fails); 60,000 samples across msk_chord_2024 + genie_public in
7.3 s, 504 B document, 45 KB response. Saving the 60k set took 0.9 s.
Tests: test/id-sets.test.ts (compile, request context, boundary, inline
limit); idSetContract on PGlite and Postgres (round trip, access, immutable
members, delete and unshare take effect at once: both mutation-checked);
e2e paste -> save -> add, with the set refused signed out, for another user
and after deletion. Gate (GATE_PORT=4111 scripts/gate.sh) passes.
Deploying needs `npm run db:migrate` for 0006 on the configured Postgres.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NvdfHurd9t63GK2ToazMti
Where the reader is signed in, the page no longer puts inline id lists into
documents it saves or builds:
- app/lab/id-sets-client.ts: storeCohortLists / storeDocLists replace every
inline sampleList/patientList (its "<study>:<id>" ids, the only ones an
inline list ever matched) with a stored set, {kind, set, label, n}. A list
that matches no case, or names only studies where patient ids may not go,
stays inline: it selects nobody there either way.
- Lists entering the document: an oncoprint drag-selection and a Cell
Explorer import become cohorts naming a set (the paste tab already did).
Signed out, they stay inline as before.
- Saves: the page's persist / persistSpaces / saveShared wrap every save path
(save to library, save analysis, save over a shared analysis, the
assistant's save, Explore's save as cohort) so what is saved names sets,
and the open document is given the same sets, so a re-save stores nothing
new. saveCohortToLibrary takes the saved definition into the copy, so the
cohort then has nothing left to save.
- db/migrations/0001's comment no longer says identifiers never live in
Postgres: definitions are rules, and a list a definition names is a stored
id set. DSL reference and CLAUDE.md say the same.
Inline lists remain valid DSL for MCP clients, scripts and signed-out
readers. Analyses saved earlier with inline lists convert on their next save.
Verified: new e2e check (save a cohort carrying an inline list -> the saved
definition and the analysis's copy name one new set, nothing left to save,
same count); the oncoprint drag check now asserts the stored set's size
equals the selection and the compiler's count (3,556 = 3,556 = 3,556).
Gate (GATE_PORT=4111 scripts/gate.sh) passes: 255 e2e checks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NvdfHurd9t63GK2ToazMti
…ts it selects now
A cohort can be locked so it stops drifting as patients are added to the
data. Its `lock` names stored id sets of its patients and their samples then,
the moment, the data version and the OncoKB release, and the frozen patients
of each of its split arms (`children`, by "<split> / <arm>"). The document's
`locked` marks an analysis whose cohorts were all locked at one moment.
Engine:
- lib/cohort-lock.ts applyLocks runs after splits expand and before
`extends` resolves (runAnalysis and resolveDocCohorts): a locked cohort or
arm becomes its set alone; an arm added after the lock still extends its
frozen cohort, so arms add up to it. compileCohortDef honours `lock` too, so
Explore, oncoprint specs and links cannot fall back to the live definition.
- The definition is still run beside the set: result.cohorts[name].lock
reports what it selects now, who it would add or drop, frozen patients no
longer in the data, and new samples of the frozen patients
(PatientLevelResolver.lockDrift, one logged query per study). The readback
says "locked <date>: N patients who were <definition>".
- lib/cohort-lock-server.ts + POST /api/lock ({doc, cohorts?}) evaluate each
cohort and arm at one moment and save the sets (empty sets allowed: an arm
may hold nobody). Patient-level studies only; needs a sign-in.
- idSetRefsIn reads a lock's sets for its request; carriesPatientIds counts a
locked cohort as ids; a request may name up to 128 sets.
- DSL 2.8 (no-op upgrade) so a 2.7 engine refuses a locked document instead
of silently running the live definitions. Docs regenerated.
Page: a cohort's menu has "lock to these patients" / "unlock"; "Lock all
cohorts" beside the cohorts heading locks every cohort and arm at once
(then "locked <date> · unlock all"); unlocking one cohort clears the
analysis lock. A locked card says when, and either "unchanged since" or how
the data has moved. "Update to the saved version" asks before it drops a
lock; rename and notes leave it alone.
A lock freezes patients, not samples (new samples of frozen patients are
counted and shown), and not the values recorded about them.
Verified on msk_chord_2024: locking KRAS NSCLC (2,194, with a split) and EGFR
(2,158) took 0.45 s and left counts and views unchanged; with the definition
widened under the lock the cohort still answered 2,194 and reported
now 7,115, +4,921 entered, 0 left; an arm added after locking summed to
2,194 (conserved). test/cohort-lock.test.ts (10 checks); new e2e check (lock
all -> same counts, "unchanged since", one moment for all; unlock one ->
analysis lock cleared). Gate (GATE_PORT=4111 scripts/gate.sh) passes: 256
e2e checks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NvdfHurd9t63GK2ToazMti
- "save to library…" / "save changes to library…" open a dialog prefilled with the cohort's library name, which says what saving does: a new saved cohort, the one it is linked to updated, or (in red, button "Replace") another saved cohort of that name replaced. - Saved under another name, the cohort takes that name in the analysis too, views drawing it included (as "edit…" renames), unless the analysis already has a cohort of that name; then it keeps its name and a message says so. - "save changes to library…" is offered only for a cohort linked to a saved cohort (copied from it, or saved before) and changed since. A cohort that merely shares a saved cohort's name (default names repeat: "selection", "from Cell Explorer") is offered as a new save, and the dialog warns. - The pasted-list tab has no default name: "Match and save" waits for one. - The source badge reads "in library" (was a bare hash), and its tooltip and panel say "linked to your saved cohort" (was "copied from", wrong for a cohort that was saved rather than copied). Verified on the test server (dialog, rename, no page errors) and in the e2e save check: rename on first save, view follows, "in library" badge, "Updates your saved cohort" on re-save, a same-name cohort offered as new with the Replace warning. Gate (GATE_PORT=4111 scripts/gate.sh) passes: 256 e2e checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NvdfHurd9t63GK2ToazMti
A cohort with no `study`/`studies` runs in its analysis's study. Saved to
the library like that, it would silently take the study of whatever analysis
it joined next ("EP300 mutant in MSK-CHORD" becoming "EP300 mutant in
GENIE"). The page's persist, which every cohort save goes through (a cohort's
card, Explore, a library example, the assistant's save), now pins such a
cohort to the study it was running in: the analysis's, or for a template its
parameter's current value. The analysis's copy takes the same definition, so
it stays "in library", not "changed". Saved analyses are untouched, so
templates (`"study": "$study"`) keep working.
Verified: the e2e save check asserts a cohort with no study is saved naming
msk_chord_2024, the analysis's study, and the analysis's copy names it too.
Gate (GATE_PORT=4111 scripts/gate.sh) passes: 256 e2e checks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NvdfHurd9t63GK2ToazMti
…xample cohorts move to Examples
- The paste-ids form is its own component (app/lab/list-cohort.tsx: ListSource,
ListCohortDialog), used in three places:
- the Saved tab: "New cohort from a list of ids…" chooses the study (there
is no analysis to take one from) and saves straight to the library; a name
already taken gets " (2)", never an overwrite;
- the analysis's "Cohorts in this analysis" panel: "From a list of ids…"
adds the cohort before any view exists (the panel now shows with no
cohorts, saying how to add one);
- a view's "+ Add cohort", as before.
A list cohort always names the study it was matched in.
- Example cohorts are only on the Examples page, in a new Cohorts tab (preview,
analyze, save to library); they are gone from the Saved tab and from
"+ Add cohort"'s library.
- User-facing text speaks of list cohorts, not stored id sets.
Verified: new e2e check (library: study must be chosen, saved with its set
and study; panel: cohort added with no view; examples only on the Examples
page). e2e-saved.cjs follows the examples to the Examples page. Gate
(GATE_PORT=4111 scripts/gate.sh) passes: 257 e2e checks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NvdfHurd9t63GK2ToazMti
db/migrations/0007 adds id_set.kind ('list' | 'lock'), parts (sizes) and
parts_gz (members), covered by the immutability trigger. A lock keeps the
cohort's patients in ids_gz as before, and beside them, as named parts, those
patients' samples ("samples") and each split arm's patients
("arm:<split> / <arm>"). The cohort's `lock` names that one set; an arm names
its part (`part`), and compiles to a ClickHouse external table of its own,
uploaded only where the set's ids may go. Locks made before parts (separate
sample and arm sets) are still read as they were (lockSamples, applyLocks).
Verified against the live warehouse: locking KRAS NSCLC (with a split) and
EGFR made exactly one id_set row each (KRAS: 2,194 patients, parts samples
2,225 and three arms); counts, views, drift (+4,921 when widened) and an arm
added after locking (conserved) unchanged. Unit tests for parts (table names,
uploads, drift against the samples part, older locks); store contract for a
lock's parts and their immutability; the e2e lock check asserts one row per
locked cohort. 0007 applied to the shared Supabase (additive; production
unaffected). Gate (GATE_PORT=4111 scripts/gate.sh) passes: 257 e2e checks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NvdfHurd9t63GK2ToazMti
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Groups defined as long lists of ids, cohorts and analyses locked against data drift, and a tidier save flow.
What changes for a researcher
How
id_set,lib/id-set-store.ts, owned and shared bylib/access.ts, immutable members, access log) and named by id in the document, so documents and?doc=links stay small. Each statement that reads one carries it as a ClickHouse external table instead of inlined SQL, removing themax_query_sizewall (inline sample lists failed at ~13k ids; a stored 60,000-sample cross-study set runs in 7.3 s). Inline lists stay valid DSL (MCP, scripts, signed-out readers) up toINLINE_ID_SQL_LIMIT, refused by name past it.id_setrow (kind = lock): the cohort's patients, plus their samples and each arm's patients as parts. The engine evaluates the set and still runs the filter beside it for the drift readout (lib/cohort-lock.ts,POST /api/lock).carriesPatientIdscounts sets and locks); answers that read a set bypass the shared cache. Seedocs/DATA-BOUNDARY.md.Database
Migrations
0006_id_sets.sqland0007_id_set_parts.sql(additive). Both are already applied to the shared Supabase that cbiolab.vercel.app uses. Any other deployment with its own Postgres needsnpm run db:migratebefore this code.Verification
GATE_PORT=4111 scripts/gate.shpasses on this branch rebased onto current master: lint, build, 1,316 unit, 47 Postgres, 6/6 planted access bugs, 465/465 portal comparisons, cohort logic, method scripts, Python conformance, 266 e2e checks (new checks for pasted lists, list storage on save, locking, the save dialog, study pinning, and example cohorts). Exercised by hand on a test server against the live warehouse and Supabase.Preview
No deploy preview: this repo deploys to Vercel from
master.🤖 Generated with Claude Code
https://claude.ai/code/session_01NvdfHurd9t63GK2ToazMti