Adversarial review: the T023 signal was only tested against a stub graph

specs/010-search-page-answers/tasks.md

@@ -62,7 +62,7 @@ state.
 - [ ] T020c Establish whether a search-page question needs all four preprocessing calls. The sequential half of this is answered and done (T019a): they run in two rounds and cost 2.6s at the median, not the ~16s recorded here, which never reproduced
 - [ ] T021 Land spec 009 collection routing and re-measure. **Still open — I marked this done on 2026-09-18 and was wrong.** What landed is *source* routing (`resolve_active_sources` picks reactome / userguide / live). Collection routing is selecting among the five collections *within* the reactome bundle, and it is not implemented: `QueryIntent` has no `collections` field, `resolve_collections` does not exist, and `retrieve_documents` still loops over every collection
 - [ ] T022 Re-assess FR-005 against the result and say plainly whether 2s/10s is reachable
-- [x] T023 Propagate a real failure signal out of the live path. `answer_from_live_services` takes an optional `LiveReport` and records a tool exception **before** stringifying it into the model's context -- the last point at which it is still a fact rather than a paraphrase. The graph carries it out as `live_tool_failed`, and `answer_sweep` retries on that instead of matching prose. `TRANSIENT` and `_looks_transient` are gone, with a test asserting they do not come back: if prose matching returns it will return as a list of markers. The report is optional, so none of the twelve existing call sites changed
+- [x] T023 Propagate a real failure signal out of the live path. `answer_from_live_services` takes an optional `LiveReport` and records a tool exception **before** stringifying it into the model's context -- the last point at which it is still a fact rather than a paraphrase. The graph carries it out as `live_tool_failed`, and `answer_sweep` retries on that instead of matching prose. `TRANSIENT` and `_looks_transient` are gone, with a test asserting they do not come back: if prose matching returns it will return as a list of markers. The report is optional, so none of the twelve existing call sites changed. **The no-tools fallback deliberately does not set it**: `get_mcp_tools` remembers a failed start, so a retry could never succeed and marking it transient would buy an attempt guaranteed to fail. Pinned by a test, because the absence of the flag there reads like an oversight
 
 ## Phase 6: Handover
 

src/agent/profiles/react_to_me.py

@@ -198,6 +198,15 @@ async def _answer_from_live_services(
         """
         tools = await get_mcp_tools()
         if not tools:
+            # Deliberately NOT reported as an upstream failure, and this is
+            # the kind of omission that looks like a bug later.
+            #
+            # `get_mcp_tools` returns None both when no server is configured
+            # and when starting one failed -- and it *remembers* the failure,
+            # so every later call returns None too. A retry could therefore
+            # never succeed, and marking this transient would buy a second
+            # attempt guaranteed to fail. It is a persistent condition, so the
+            # sweep should fail loudly on it rather than retry (Principle IV).
             logger.warning(
                 "Question routed to live lookup but no MCP tools are available; "
                 "falling back to retrieval."

tests/agent/test_collection_routing.py

@@ -10,13 +10,16 @@
 import asyncio
 from typing import Any, cast
 
+import pytest
 from langchain_core.runnables import RunnableConfig
 
 from agent.profiles.react_to_me import ReactToMeGraphBuilder, ReactToMeState
 from agent.tasks.intent_classifier import QueryIntent, build_classifier_message
-from retrievers.csv_chroma import selected_collections
+from retrievers.csv_chroma import resolve_collections, selected_collections
 from retrievers.reactome.metadata_info import reactome_descriptions_info
 
+ALL = sorted(reactome_descriptions_info)
+
 
 def test_an_omitted_selection_means_every_collection() -> None:
     # The field is optional in the schema, so a model that ignores it entirely
@@ -135,3 +138,32 @@ def test_a_state_without_the_field_searches_everything() -> None:
     del state["collections"]
     asyncio.run(_builder(rag, "reactome").generate_answer(state, RunnableConfig()))
     assert rag.seen == [[]], "a pre-routing checkpoint must not crash or narrow"
+
+
+def test_an_unreachable_mcp_is_not_reported_as_a_transient_failure() -> None:
+    # `get_mcp_tools` remembers a failed start, so every later call returns
+    # None too and a retry could never succeed. Marking this transient would
+    # buy the sweep a second attempt guaranteed to fail. Pinned because the
+    # absence of the flag here reads like an oversight.
+    import agent.profiles.react_to_me as profile
+
+    rag = _RecordingRag()
+    builder = _builder(rag, "reactome")
+
+    async def _no_tools() -> None:
+        return None
+
+    with pytest.MonkeyPatch.context() as patch:
+        patch.setattr(profile, "get_mcp_tools", _no_tools)
+        state = _state("live", [])
+        state["active_sources"] = ["live"]
+        result = asyncio.run(
+            builder._answer_from_live_services(state, RunnableConfig())
+        )
+
+    assert result.get("live_tool_failed") is not True
+    # Either representation means "search everything": the fallback sets an
+    # empty selection explicitly, and `resolve_collections` widens both empty
+    # and absent to every collection. Asserting the meaning, not the shape.
+    assert rag.seen, "the fallback never reached retrieval"
+    assert resolve_collections(rag.seen[0], ALL) == ALL