ReactomeGSA client: run an analysis without the data touching the model

[adamjohnwright]

First slice of spec 012 — the layer the chat flow sits on. Load a public dataset, download its matrix, submit, poll, parse. No chat wiring yet, and nothing in reactome-mcp: the public MCP was deliberately narrowed this month, and long-running jobs carrying user data do not belong there.

The sizes are the design

	measured 2026-09-21
matrix, 16-sample example	1.2 MB
submitted payload	1.5 MB
result	2.0 MB, 2,679 pathways
run time	minutes (PADOG, 1,000 permutations)

POST /analysis requires the matrix inline and has no by-reference form — not even for a dataset the service itself just loaded. So download_matrix and submit exist to keep that string server-side, and for_model is the only thing producing something small enough for a prompt.

Two defects the swagger would have caused

Both found by building the fixture from a real response rather than from the schema.

The result key is method_name. The swagger's AnalysisResult says methodName, so parse read None — the same defect reactome-mcp fixed across ten tools in September: a field path asserted rather than verified.

mappings is 8,035 entries, 499 KB, of the user's own row identifiers mapped to UniProt. For an uploaded matrix those are the user's gene names. Now in NEVER_SENT.

Nobody would have thought to exclude it — which is the argument for an allow-list. for_model names the six columns it may emit, so mappings was excluded before I knew it existed; I only had to notice in order to write the test. A denial list would have shipped it.

Those names are ReactomeGSA's, not the Analysis Service's. Reusing analysis/disclosure.py's fileName/sampleName/columnNames here would have looked like protection and provided none.

The test that matters

It plants a marker in every user-supplied place — dataset name, fold_changes, mappings — and asserts it appears nowhere in the model's view. On values, not field names, since a field can be copied into a differently named one.

It is paired with a control that puts the same marker in an allow-listed column and requires it through. An absence assertion proves nothing until the same construction has produced a presence.

Measured behaviours, handled because they happened

• a submission returned 200 and then failed through /status alone, with CONNECTION_FORCED - broker forced connection closure. A receipt is not a result, so finished covers failure as well as success.
• /result answers 406 while running — the happy path — so it raises GsaNotReadyError rather than collapsing "still going" into "broken"
• /data/download needs format=expr; omitting it is 400 and tsv is rejected
• a non-numeric FDR falls back to 1.0, not 0.0, so "NA" cannot sort to the top as the most significant pathway in the analysis

Identifiers are interpolated into URL paths and come from the model, so they are constrained to what real ones look like — a value containing / addresses a different endpoint, which is a bypass rather than a 404. Both POSTs validate the ID they read from the body instead of str()-ing whatever arrived.

No browser user-agent, unlike analysis/client.py. gsa.reactome.org serves hypercorn directly and answers python-httpx with 200 — verified rather than assumed, because testing only with curl, which is exempt from the automation block in front of reactome.org, is how a sibling service was recently published and believed reachable.

Verification

Sabotage: defaulting a bad FDR to 0.0 fails the ranking test; adding the user's dataset name to the model view fails the marker test.

25 new tests, whole suite green, ruff and mypy clean.

Not in this PR

The Chainlit upload flow, the results file, and the async "I'll tell you when it's done" UX. Spec 012 records the constraints those must respect: Chainlit ships a 500 MB upload cap while this host has 4.8 GB free, and the run outlasts a chat turn.

🤖 Generated with Claude Code

---

Adversarial review of the finished branch

Three findings. The first is the one that mattered.

1. The model was sent a capability, not a citation

for_model included browser_links, and the Pathway Browser URL embeds the analysis token:

https://reactome.org/PathwayBrowser/#/DTAB=AN&ANALYSIS=MjAyNj...

Anyone holding that token can fetch the whole result back from the service — including mappings (the user's own gene identifiers) and fold_changes (whose columns are their samples).

So the allow-list stripped the user's content out of the payload and then handed over a key that retrieves it. The same disclosure by a longer route — and the marker test could not see it, because the marker is not in the URL.

The link now goes to for_user, for the chat to render. The person should absolutely have it; the model should not.

2. An empty result was reported as an exact zero

A result whose table did not arrive parses to no pathways, and for_model said pathway_count: 0, counts_are_exact: True. A summary built on that tells the user their data contained no enriched pathways — a confident answer to a question nobody managed to ask. It now reports no_result and stops claiming exactness.

3. repr(GsaResult) printed the whole table

A dataclass generates __repr__ over every field, and raw_table is ~500 KB in a real run. One logger.debug("%s", result), one exception context, or one failing assertion would put the entire pathway table into a log. Now field(repr=False).

Care over what reaches a model is wasted if the same content reaches a log file by default.

Verification

Each finding has a test, each verified by sabotage against the specific test written for it:

sabotage	test that failed
put browser_links back in the model view	test_the_analysis_token_never_reaches_the_model
remove the empty-result branch	test_an_empty_result_is_not_reported_as_an_exact_zero
drop repr=False	test_the_result_does_not_print_its_table

Both absence assertions are paired with controls — the user's link must still reach the user, and a real result must not be flagged as missing.

Also: mypy src is not what CI runs

CI lint failed here while ruff and mypy passed locally, because CI runs bare mypy over 159 files and I ran mypy src over 152. The seven it misses include the tests. Added checks.sh, which runs the four commands from ci.yml verbatim with no pipes around them — an earlier commit in this repo shipped six mypy errors because the exit code read was a pipe's. Verified it can go red: swapping in mypy --strict gives 1 of 4 checks FAILED, exit 1.

30 tests in tests/gsa, whole suite green.