From 7210b4910a20c27472fdf014f2896a8a19085b2f Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Thu, 17 Sep 2026 14:38:02 +0000 Subject: [PATCH] Two collection guards that survive removal, and why the other two did not Phase 2 of spec 009 set out to add one sweep question per collection, each failing if its collection stopped being searched. Building them changed what the spec claims. Method, since the obvious approach does not work: ask a candidate against the full bundle and against a copy with exactly one collection removed. A question guards a collection only if it answers with it and fails without it. Checking string-uniqueness in the CSVs is not enough -- P04637 looked like an ewas marker and is in disease_variants too. Two of four candidates survived that test and are added here. ewas is guarded by TP53's UniProt accession, summations by the curated prose for Selective autophagy; both verified to fail with their collection removed. The other two did not, and the reason is structural rather than a matter of picking better words. Every reaction name also appears in summations, because the summations query covers Pathway OR ReactionLikeEvent -- so the two collections overlap by construction and no answer-level question can distinguish them. reactions answered correctly with its collection removed entirely. T007 is rewritten as a retrieval-level assertion; T005 needs a better complexes candidate and is left open rather than claimed. This cuts both ways for the feature and the spec now says so. Redundancy means routing away from a collection costs less recall than the spec assumed, which argues for the change. It also means the gate hole is harder to close than the plan implied: a collection whose content is duplicated elsewhere cannot be guarded by asking a question at all. Co-Authored-By: Claude Opus 5 --- specs/009-collection-routing/spec.md | 36 +++++++++++++++++++++++++++ specs/009-collection-routing/tasks.md | 10 ++++---- src/evaluation/answer_sweep.py | 20 +++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/specs/009-collection-routing/spec.md b/specs/009-collection-routing/spec.md index 33e83ba..4533790 100644 --- a/specs/009-collection-routing/spec.md +++ b/specs/009-collection-routing/spec.md @@ -129,6 +129,42 @@ this change is a real difference in behaviour rather than noise. That last one matters: the point of this change is a number going down, and it should be reported as one. +## Finding: the collections overlap more than this spec assumed + +Phase 2 set out to add one sweep question per collection, each failing if its +collection stopped being searched. Building them changed the picture. + +Method: ask a candidate against the full bundle and against a copy with exactly one +collection removed. A question guards a collection only if it answers with it and +fails without it. + +| collection | with | without | result | +|---|---|---|---| +| `ewas` | yes | no | **guards it** -- UniProt accessions live only there | +| `summations` | yes | no | **guards it** -- the curated prose lives only there | +| `complexes` | -- | -- | no question found yet | +| `reactions` | yes | **yes** | answered with the collection removed entirely | + +**No reaction name is unique to `reactions`.** The `summations` query covers +`Pathway OR ReactionLikeEvent`, so every event name appears in both collections by +construction. They overlap by design. + +Two consequences for this feature. + +The recall risk is **lower** than this spec assumed. If a question can be answered +with a whole collection removed, routing away from that collection costs little -- +which argues for the change rather than against it. + +But the gate hole is **harder to close** than the plan implies, and for a reason the +plan had wrong. It is not that nobody wrote the questions; it is that a collection +whose content is duplicated elsewhere cannot be guarded by asking a question. For +`reactions` the guard would have to assert on retrieval -- which documents were +searched -- rather than on the answer. + +Recorded rather than worked around: `tasks.md` T004-T008 are written as though four +such questions exist. Two do. T007 (`reactions`) needs rewriting as a retrieval-level +assertion, and T005 (`complexes`) needs a better candidate before it can be claimed. + ## Scope In: collection selection for the `reactome` source, defaulting to all; the diff --git a/specs/009-collection-routing/tasks.md b/specs/009-collection-routing/tasks.md index 5478aa7..8218148 100644 --- a/specs/009-collection-routing/tasks.md +++ b/specs/009-collection-routing/tasks.md @@ -18,11 +18,11 @@ before it is changed. routing stops searching them. Routing must not land before this closes, or the acceptance criterion cannot detect the failure the feature can cause. -- [ ] T004 [P] Add a `summations`-dependent question to `EXPECTATIONS` in src/evaluation/answer_sweep.py, with a `must`/`must_match` that fails if that collection is not searched -- [ ] T005 [P] Add a `complexes`-dependent question to `EXPECTATIONS` in src/evaluation/answer_sweep.py -- [ ] T006 [P] Add an `ewas`-dependent question to `EXPECTATIONS` in src/evaluation/answer_sweep.py -- [ ] T007 [P] Add a `reactions`-dependent question to `EXPECTATIONS` in src/evaluation/answer_sweep.py -- [ ] T008 Verify each new question FAILS when its collection is removed from the bundle copy, and passes with it present; record the evidence in the PR +- [x] T004 [P] Add a `summations`-dependent question to `EXPECTATIONS` in src/evaluation/answer_sweep.py (Selective autophagy / lysosome; verified by removal) +- [ ] T005 [P] Find a `complexes` candidate that fails without the collection (the first attempt did not), then add the question to `EXPECTATIONS` in src/evaluation/answer_sweep.py +- [x] T006 [P] Add an `ewas`-dependent question to `EXPECTATIONS` in src/evaluation/answer_sweep.py (TP53 UniProt P04637; verified by removal) +- [ ] T007 Assert at retrieval level that `reactions` was searched, in tests/retrievers/test_collection_selection.py -- no answer-level question can guard it, because every reaction name also appears in `summations` +- [x] T008 Verify each new question FAILS when its collection is removed from the bundle copy, and passes with it present; record the evidence in the PR (method established; two of four candidates survived it) - [ ] T009 Pin current behaviour: a characterization test in tests/retrievers/test_collection_selection.py asserting that with no selection every collection in the bundle is searched - [ ] T010 Run `./bin/answer-sweep` against Release97 and confirm green before any behaviour change diff --git a/src/evaluation/answer_sweep.py b/src/evaluation/answer_sweep.py index 91e3271..cfd459e 100644 --- a/src/evaluation/answer_sweep.py +++ b/src/evaluation/answer_sweep.py @@ -91,6 +91,26 @@ class Expectation: why="Preferring the web tool must not bury the plugin for someone who wants it.", must=("FIViz",), ), + # --- one question per collection, so routing cannot quietly skip one ---- + # Each was chosen by removing its collection from a copy of the bundle and + # confirming the answer changes. A question that still answers without its + # collection guards nothing, and three of the first four candidates were + # exactly that -- see specs/009-collection-routing/spec.md. + Expectation( + question="What is the UniProt accession for the TP53 protein in Reactome?", + why="Guards the `ewas` collection: it is the only one holding UniProt " + "links. Verified by removing ewas from a bundle copy, after which the " + "accession is no longer answered.", + must_match=(r"\bP04637\b",), + ), + Expectation( + question="What does Reactome's summary of Selective autophagy say about " + "where cargo is degraded?", + why="Guards the `summations` collection: the curated prose summaries live " + "only there. Without it the chatbot says no summary is available.", + must=("lysosom",), + must_not=("does not provide", "not currently available"), + ), # --- disease variants, which only the new collection can name ----------- Expectation( question="List the ABCA1 variants in Reactome and the disease each one causes.",