From bc163d6040f7ab56dd73aa91ff62d3d4d8d5e027 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Sat, 19 Sep 2026 07:24:40 +0000 Subject: [PATCH] Watch the routing decision, because no answer can T005b wanted the observation no test can make. `answer-sweep` cannot see a classifier that stops choosing a collection: neither `complexes` nor `reactions` can be guarded by asking a question, since their content is duplicated in `summations` prose and in the input/output names `reactions` carries. A collection that goes unchosen produces answers that are confident, plausible and slightly worse, and every tracked question still passes. `bin/routing-probe` asks the classifier ten questions, two per collection, and checks what it *selected* rather than what any answer said. One model call each, cheap enough to run on every deploy. Two properties, with the same asymmetry the feature rests on. Coverage: every collection chosen by at least one question -- the guard for `complexes`, which nothing else watches. No wrong narrow: a question that narrows must include the collection its answer lives in. An empty selection is never a failure, because widening is safe and the prompt asks for it when the model is unsure. First run: 10/10 correct, every collection chosen, nothing left open. So `complexes` is being routed to and the risk that argued against deploying is not currently realised. The tests check that the guard fires rather than that the happy path is quiet, and the coverage assertion was verified by sabotage -- forcing `missing` empty fails the test that exists to catch it. Two probes per collection is itself asserted, so one passing is not luck, and so is the existence of a probe per collection: a collection with no probe cannot be reported as never chosen, which would make this file quietly useless for exactly the collection it was written for. T005c is open and matters more than this commit: one run is a snapshot of one model's judgement on one day. The risk was always drift, and the probe is only a guard if it keeps being run. Co-Authored-By: Claude Opus 5 --- bin/routing-probe | 12 ++ specs/009-collection-routing/research.md | 24 +++ specs/009-collection-routing/tasks.md | 3 +- src/evaluation/routing_probe.py | 206 +++++++++++++++++++++++ tests/evaluation/test_routing_probe.py | 65 +++++++ 5 files changed, 309 insertions(+), 1 deletion(-) create mode 100755 bin/routing-probe create mode 100644 src/evaluation/routing_probe.py create mode 100644 tests/evaluation/test_routing_probe.py diff --git a/bin/routing-probe b/bin/routing-probe new file mode 100755 index 0000000..67da08c --- /dev/null +++ b/bin/routing-probe @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +"""Entry point for the routing probe; see src/evaluation/routing_probe.py.""" + +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "src")) + +from evaluation.routing_probe import main + +if __name__ == "__main__": + main() diff --git a/specs/009-collection-routing/research.md b/specs/009-collection-routing/research.md index 51f9647..fcee6ce 100644 --- a/specs/009-collection-routing/research.md +++ b/specs/009-collection-routing/research.md @@ -375,3 +375,27 @@ passes, and the only symptom is answers that are quietly worse. That is smaller than it was -- the plumbing is now pinned, and the nine questions above found no case where `complexes` was needed for a correct answer at all -- but it is not nothing, and it is not what T005a tests. + +## The routing probe, 2026-09-19 + +T005b asked for the observation no test can make. `bin/routing-probe` makes it: +ten questions, two per collection, asking the classifier directly and checking +**what it selected** rather than what any answer said. One model call each, so +it is cheap enough to run on every deploy. + +Two properties, and the asymmetry between them is the same one the feature +rests on: + +- **Coverage** -- every collection is chosen by at least one question. This is + the guard for `complexes`, which nothing else can watch. +- **No wrong narrow** -- a question that narrows must include the collection + its answer lives in. An empty selection is *never* a failure, because + widening is safe and the prompt asks for it whenever the model is unsure. + +First run: **10/10 correct, every collection chosen, nothing left open.** So +`complexes` is being routed to, and the under-routing risk that argued against +deploying is not currently realised. + +What that is not: a guarantee. It is a snapshot of one model's judgement on +one day. The risk was always drift, and the probe is only a guard if it keeps +being run -- which is why T005c exists and is open. diff --git a/specs/009-collection-routing/tasks.md b/specs/009-collection-routing/tasks.md index e4f2766..1448182 100644 --- a/specs/009-collection-routing/tasks.md +++ b/specs/009-collection-routing/tasks.md @@ -21,7 +21,8 @@ acceptance criterion cannot detect the failure the feature can cause. - [x] T004 [P] Add a `summations`-dependent question to `EXPECTATIONS` in src/evaluation/answer_sweep.py (Selective autophagy / lysosome; verified by removal) - [x] T005 [P] ~~Find a `complexes` candidate that fails without the collection~~ **Attempted and abandoned with a reason, 2026-09-19.** Nine questions over six complexes, in two shapes; every one answered just as well without the collection, because complex names appear throughout `reactions` (as input and output names) and `summations` prose. The one thing unique to `complexes` -- the component list -- has answers too variable to assert on: the same configuration returned 1 to 6 of 7 components. See research.md - [x] T005a [P] Assert at retrieval level that `complexes` was searched, in tests/retrievers/test_sync_async_equivalence.py -- parametrised over all five real collection names: each is reachable, and selecting it searches nothing else. Verified by sabotaging `resolve_collections` to ignore the selection, which fails all five. **Catches a plumbing failure, not a routing one**: a classifier that never chooses a collection is still undetectable, and needs a production probe rather than a test (research.md) -- [ ] T005b **The risk T005 was written for is still open.** T005a catches a plumbing failure; a classifier that never *chooses* a collection is undetectable by any deterministic test, and produces confident plausible answers rather than an error. Needs the routing distribution observed over real traffic, or a periodic probe asking known-collection questions and checking what was selected. **Do not treat Phase 2 as closing this** -- a register that reads "handled" is why nobody looks again +- [x] T005b Probe the routing decision, since no answer can: `bin/routing-probe` asks the classifier ten questions, two per collection, and checks **what was selected** rather than what any answer said. Fails if a collection is never chosen, or if a question narrows away from where its answer lives; an open selection is never a failure, because widening is the safe direction. First run 2026-09-19: 10/10 correct, every collection chosen +- [ ] T005c Run the probe on every deploy, beside the sweep. **The residual risk is not that the probe is missing, it is that under-routing drifts in between runs** -- a classifier that stops choosing `complexes` next month produces confident plausible answers and a green sweep. One run is a snapshot; the guard is the repetition - [x] T006 [P] Add an `ewas`-dependent question to `EXPECTATIONS` in src/evaluation/answer_sweep.py (TP53 UniProt P04637; verified by removal) - [x] T007 Assert at retrieval level that `reactions` was searched -- closed by the same parametrised test as T005a, in tests/retrievers/test_sync_async_equivalence.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) diff --git a/src/evaluation/routing_probe.py b/src/evaluation/routing_probe.py new file mode 100644 index 0000000..a9868d5 --- /dev/null +++ b/src/evaluation/routing_probe.py @@ -0,0 +1,206 @@ +"""Does the classifier ever choose each collection? + +`answer-sweep` cannot answer this, and the gap is structural rather than an +omission. Measured 2026-09-19 (specs/009-collection-routing/research.md): +neither `complexes` nor `reactions` can be guarded by asking a question, +because their content is duplicated in `summations` prose and in the +input/output names carried by `reactions`. Every candidate answered just as +well with the collection removed. + +So a classifier that silently stops routing to a collection produces answers +that are confident, plausible and slightly worse, while every tracked question +still passes. That is the failure T005 was written for, T005a does not catch +it -- it catches a collection being unreachable, not unchosen -- and no +deterministic test can, because it is one model call's judgement. + +This is the observation you have to keep making instead. It asks the +classifier directly, one call per question, and checks *what was selected* +rather than what any answer said. Two properties: + +- **Coverage**: every collection in the bundle is chosen by at least one + question. This is the T005b guard -- it is what notices `complexes` going + unchosen. +- **No wrong narrow**: a question that narrows at all must include the + collection its answer lives in. Leaving the selection empty is always + allowed, because widening is the safe direction and the prompt asks for it + whenever the model is unsure. + +Empty selections are reported, not failed. A run where everything is empty is +routing doing nothing, which is worth seeing and is not incorrect. +""" + +import argparse +import asyncio +import sys +from dataclasses import dataclass, field + +from agent.graph import resolve_llm_model +from agent.models import get_llm +from agent.tasks.intent_classifier import ( + SourceName, + create_intent_classifier, +) +from reactome_mcp.session import is_configured +from retrievers.reactome.metadata_info import reactome_descriptions_info + + +@dataclass(frozen=True) +class Probe: + question: str + #: The collection this question's answer lives in. If the classifier + #: narrows at all, this must be among what it chose. + expect: str + why: str + + +PROBES: tuple[Probe, ...] = ( + Probe( + question="What is the UniProt accession for the TP53 protein in Reactome?", + expect="ewas", + why="UniProt links live only in ewas; this is the tracked question too.", + ), + Probe( + question="What is the UniProt identifier for BRCA1 in Reactome?", + expect="ewas", + why="A second accession question, so one passing is not luck.", + ), + Probe( + question="Which diseases involve variants of the PTEN gene in Reactome?", + expect="disease_variants", + why="The variants themselves are only in disease_variants.", + ), + Probe( + question="List the ABCA1 variants in Reactome and the disease each causes.", + expect="disease_variants", + why="Measured: the prose survives narrowing, the variant names do not.", + ), + Probe( + question="What does Reactome's summary of Selective autophagy say?", + expect="summations", + why="The curated prose lives only in summations.", + ), + Probe( + question="How does Reactome describe the Wnt signalling pathway?", + expect="summations", + why="A second prose question.", + ), + Probe( + question="What are the protein components of the Nup107 complex in Reactome?", + expect="complexes", + why="The whole reason this file exists: complexes cannot be guarded by " + "an answer, so it is guarded by the routing decision instead.", + ), + Probe( + question="Which proteins make up the PAM complex in Reactome?", + expect="complexes", + why="A second complexes question, because it is the unguarded one.", + ), + Probe( + question="What are the inputs and outputs of the reaction where CDK1 " + "phosphorylates MCM2?", + expect="reactions", + why="Inputs, outputs and catalysts are the shape reactions holds.", + ), + Probe( + question="Which reaction converts cholesterol in the ABCA1 pathway, and " + "what catalyses it?", + expect="reactions", + why="A second reactions question.", + ), +) + + +@dataclass +class Result: + probe: Probe + chose: list[str] = field(default_factory=list) + error: str = "" + + @property + def widened(self) -> bool: + """Chose nothing, meaning search everything. Always allowed.""" + return not self.chose and not self.error + + @property + def wrong(self) -> bool: + """Narrowed, but not to where the answer lives.""" + return bool(self.chose) and self.probe.expect not in self.chose + + +def available_sources() -> frozenset[SourceName]: + """The prompt the deployment actually uses, so this probes the real one.""" + sources: set[SourceName] = {"reactome", "userguide"} + if is_configured(): + sources.add("live") + return frozenset(sources) + + +async def run(probes: tuple[Probe, ...] = PROBES) -> list[Result]: + provider, model, base_url = resolve_llm_model(None) + classifier = create_intent_classifier( + get_llm(provider, model, base_url=base_url, request_timeout=120.0), + available_sources(), + ) + results = [] + for index, probe in enumerate(probes, start=1): + print(f" [{index}/{len(probes)}] {probe.question[:58]}", file=sys.stderr) + result = Result(probe=probe) + try: + intent = await classifier.ainvoke({"rephrased_input": probe.question}) + result.chose = sorted(intent.collections) + except Exception as exc: + result.error = f"{type(exc).__name__}: {exc}" + results.append(result) + return results + + +def report(results: list[Result]) -> int: + print() + for r in results: + if r.error: + state = "ERROR" + elif r.wrong: + state = "WRONG" + elif r.widened: + state = "open " + else: + state = "ok " + print( + f" {state} {r.probe.expect:17s} {','.join(r.chose) or '(all)':32s} " + f"{r.probe.question[:44]}" + ) + if r.wrong: + print(f" narrowed away from {r.probe.expect}: {r.probe.why}") + if r.error: + print(f" {r.error}") + + chosen = {c for r in results for c in r.chose} + missing = sorted(set(reactome_descriptions_info) - chosen) + wrong = [r for r in results if r.wrong] + errors = [r for r in results if r.error] + widened = [r for r in results if r.widened] + + print() + print( + f" {len(results) - len(widened) - len(errors)} narrowed, " + f"{len(widened)} left open, {len(wrong)} narrowed wrongly, " + f"{len(errors)} errored" + ) + + if missing: + print() + print(f" NEVER CHOSEN: {', '.join(missing)}") + print(" A collection nothing routes to is served worse than before this") + print(" feature existed, and answer-sweep cannot see it -- that is why") + print(" this file exists. See specs/009-collection-routing (T005b).") + else: + print(" every collection was chosen by at least one question") + + return 1 if (missing or wrong or errors) else 0 + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.parse_args() + print(f"Asking the classifier {len(PROBES)} questions\n", file=sys.stderr) + raise SystemExit(report(asyncio.run(run()))) diff --git a/tests/evaluation/test_routing_probe.py b/tests/evaluation/test_routing_probe.py new file mode 100644 index 0000000..2e16a16 --- /dev/null +++ b/tests/evaluation/test_routing_probe.py @@ -0,0 +1,65 @@ +"""The probe that watches for a collection nobody routes to. + +Its whole purpose is the guard `answer-sweep` cannot provide, so the thing to +test is that the guard fires -- not that the happy path is quiet. +""" + +from evaluation.routing_probe import PROBES, Probe, Result, report +from retrievers.reactome.metadata_info import reactome_descriptions_info + + +def _result(expect: str, chose: list[str]) -> Result: + probe = Probe(question=f"about {expect}", expect=expect, why="test") + return Result(probe=probe, chose=sorted(chose)) + + +def _all_collections_covered() -> list[Result]: + return [_result(name, [name]) for name in reactome_descriptions_info] + + +def test_every_collection_has_at_least_one_probe() -> None: + # A collection with no probe cannot be reported as never chosen, which + # would make this file quietly useless for exactly the collection it was + # written for. + for name in reactome_descriptions_info: + assert any(p.expect == name for p in PROBES), f"nothing probes {name}" + + +def test_two_probes_per_collection_so_one_passing_is_not_luck() -> None: + for name in reactome_descriptions_info: + assert sum(p.expect == name for p in PROBES) >= 2, name + + +def test_a_collection_nobody_chose_fails(capsys) -> None: # type: ignore[no-untyped-def] + results = [r for r in _all_collections_covered() if r.probe.expect != "complexes"] + assert report(results) == 1 + assert "NEVER CHOSEN: complexes" in capsys.readouterr().out + + +def test_full_coverage_passes(capsys) -> None: # type: ignore[no-untyped-def] + assert report(_all_collections_covered()) == 0 + assert "every collection was chosen" in capsys.readouterr().out + + +def test_leaving_the_selection_open_is_never_a_failure() -> None: + # Widening is the safe direction and the prompt asks for it when unsure. + # An open selection must not be reported as a wrong narrow. + open_one = _result("ewas", []) + assert open_one.widened + assert not open_one.wrong + + +def test_narrowing_to_the_wrong_collection_fails(capsys) -> None: # type: ignore[no-untyped-def] + results = _all_collections_covered() + results.append(_result("disease_variants", ["summations"])) + assert report(results) == 1 + assert "narrowed away from disease_variants" in capsys.readouterr().out + + +def test_an_error_is_reported_not_raised(capsys) -> None: # type: ignore[no-untyped-def] + results = _all_collections_covered() + failed = _result("ewas", []) + failed.error = "RuntimeError: upstream died" + results.append(failed) + assert report(results) == 1 + assert "upstream died" in capsys.readouterr().out