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