From 6b496c81a43a1b576ca965defd6ebb765cb86145 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Tue, 15 Sep 2026 15:11:56 +0000 Subject: [PATCH] Fix a live answer that was correct and invisible #216 shipped a `live` destination that answers these questions correctly and shows the user nothing. The Chainlit UI displays only what the callback handlers stream -- `bin/chat-chainlit.py` reads `chainlit_cb.final_stream` and has no path that posts `result["answer"]`. The callbacks travel in the RunnableConfig. The live path took no config at all: `_answer_from_live_services(state)` never received one, `answer_from_live_services` never passed one to any model call, and the model was the shared non-streaming instance rather than a streaming copy like every other answer path uses. Measured through the graph, the way the app calls it: Q: What species are included in the Reactome database? answer present : True 'Reactome covers a total of 96 species...' streamed tokens : 0 <-- what the user sees After: 103 tokens, and the streamed text matches the answer exactly. Three things, one cause: - `config` is threaded from the graph node through the tool loop into every model call, including the forced final answer after the round cap -- that call is the one that produces the text. - the live path uses a streaming copy of the llm, the same `model_copy(update={"streaming": True})` the RAG chains use. - the unreachable-MCP fallback passed a freshly constructed `RunnableConfig()` instead of the real one, so even the fallback would have been invisible. Also fixed while here: `chat_history` was a parameter of `answer_from_live_services` that nothing ever passed, so every live answer was asked cold. A follow-up like "can you tell me?" -- which is a real question from a real session -- had no context at all. Typed `Sequence` rather than `list` so a caller holding `list[HumanMessage]` can pass it. Three regression tests; two of them fail against the shipped code. **How this got merged is worth more than the fix.** I tested `answer_from_live_services` directly, it returned the right text, and I reported it working. The constitution's first principle says a component test passing is not evidence the feature works, and to run it the way a user or the server would. I wrote that principle into a repository this week and then did not follow it. Co-Authored-By: Claude Opus 5 --- src/agent/profiles/react_to_me.py | 21 +++++--- src/reactome_mcp/answer.py | 17 +++++-- tests/reactome_mcp/test_live_answer.py | 66 ++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 11 deletions(-) diff --git a/src/agent/profiles/react_to_me.py b/src/agent/profiles/react_to_me.py index c119736..e9037c5 100644 --- a/src/agent/profiles/react_to_me.py +++ b/src/agent/profiles/react_to_me.py @@ -36,7 +36,10 @@ def __init__( embedding: Embeddings, ) -> None: super().__init__(llm, embedding) - self.llm = llm + # A streaming copy, the same way the RAG chains get theirs. The UI + # displays only what the callback handler streams, so an answer + # produced without streaming is invisible however correct it is. + self.live_llm = llm.model_copy(update={"streaming": True}) self.unsafe_answer_generator: Runnable = create_unsafe_answer_generator( llm, streaming=True @@ -149,7 +152,9 @@ async def preprocess( active_sources=active_sources, ) - async def _answer_from_live_services(self, state: ReactToMeState) -> ReactToMeState: + async def _answer_from_live_services( + self, state: ReactToMeState, config: RunnableConfig + ) -> ReactToMeState: """Answer from the MCP tools instead of the vector store. If the server is unreachable the question falls back to retrieval, with @@ -166,17 +171,19 @@ async def _answer_from_live_services(self, state: ReactToMeState) -> ReactToMeSt ) fallback = dict(state) fallback["active_sources"] = ["reactome"] - return await self.generate_answer( - ReactToMeState(**fallback), RunnableConfig() - ) + # The real config, not a fresh one: it carries the callbacks the UI + # streams through, and a fallback nobody can see is not a fallback. + return await self.generate_answer(ReactToMeState(**fallback), config) answer = await answer_from_live_services( # BaseChatModel satisfies ToolCallingModel at runtime; its # bind_tools signature is wider than the Protocol restates. - cast(ToolCallingModel, self.llm), + cast(ToolCallingModel, self.live_llm), tools, state["rephrased_input"], language=state["detected_language"], + chat_history=state["chat_history"] or None, + config=config, ) return ReactToMeState( chat_history=[HumanMessage(state["user_input"]), AIMessage(answer)], @@ -207,7 +214,7 @@ async def generate_answer( ) -> ReactToMeState: source = state["active_sources"][0] if source == "live": - return await self._answer_from_live_services(state) + return await self._answer_from_live_services(state, config) rag = self.rags[source] result: dict[str, Any] = await rag.ainvoke( { diff --git a/src/reactome_mcp/answer.py b/src/reactome_mcp/answer.py index b2a181f..daf95b1 100644 --- a/src/reactome_mcp/answer.py +++ b/src/reactome_mcp/answer.py @@ -12,6 +12,7 @@ """ import logging +from collections.abc import Sequence from typing import Any, Protocol, runtime_checkable from langchain_core.messages import ( @@ -20,6 +21,7 @@ SystemMessage, ToolMessage, ) +from langchain_core.runnables import RunnableConfig from langchain_core.tools import BaseTool logger = logging.getLogger(__name__) @@ -60,9 +62,16 @@ async def answer_from_live_services( tools: list[BaseTool], question: str, language: str = "English", - chat_history: list[BaseMessage] | None = None, + chat_history: Sequence[BaseMessage] | None = None, + config: RunnableConfig | None = None, ) -> str: - """Run the tool-calling loop and return the answer text.""" + """Run the tool-calling loop and return the answer text. + + `config` is not optional in practice. It carries the callback handlers, and + the Chainlit UI displays only what those handlers stream -- `chat-chainlit.py` + reads `chainlit_cb.final_stream` and has no path that posts the returned + string. Without it this function answers correctly into the void. + """ by_name = {tool.name: tool for tool in tools} bound = llm.bind_tools(tools) @@ -73,7 +82,7 @@ async def answer_from_live_services( ] for _round in range(MAX_TOOL_ROUNDS): - reply = await bound.ainvoke(messages) + reply = await bound.ainvoke(messages, config) messages.append(reply) calls = getattr(reply, "tool_calls", None) or [] @@ -111,7 +120,7 @@ async def answer_from_live_services( "Answer now, from the tool results above. Do not call more tools." ) ) - reply = await llm.ainvoke(messages) + reply = await llm.ainvoke(messages, config) content: Any = reply.content if isinstance(content, list): diff --git a/tests/reactome_mcp/test_live_answer.py b/tests/reactome_mcp/test_live_answer.py index ffc5f75..1713adb 100644 --- a/tests/reactome_mcp/test_live_answer.py +++ b/tests/reactome_mcp/test_live_answer.py @@ -36,6 +36,7 @@ def __init__(self, replies: list[AIMessage]) -> None: self._replies = list(replies) self.seen: list[list[Any]] = [] self.bound_tools: list[Any] = [] + self.configs: list[Any] = [] def bind_tools(self, tools: Any, **kwargs: Any) -> "_FakeLLM": self.bound_tools = tools @@ -43,6 +44,7 @@ def bind_tools(self, tools: Any, **kwargs: Any) -> "_FakeLLM": async def ainvoke(self, messages: Any, *args: Any, **kwargs: Any) -> AIMessage: self.seen.append(list(messages)) + self.configs.append(args[0] if args else kwargs.get("config")) return self._replies.pop(0) if self._replies else AIMessage("out of replies") @@ -152,3 +154,67 @@ def test_content_blocks_are_flattened() -> None: llm = _FakeLLM([AIMessage([{"type": "text", "text": "96 species."}])]) answer = asyncio.run(answer_from_live_services(llm, [reactome_species], "x")) assert answer == "96 species." + + +def test_the_config_reaches_every_model_call() -> None: + """Without this the answer is correct and invisible. + + The Chainlit UI displays only what the callback handlers stream -- + `chat-chainlit.py` reads `chainlit_cb.final_stream` and has no path that + posts the returned string. The callbacks travel in the RunnableConfig, so a + config that is dropped anywhere in this loop means the user sees nothing at + all while the tests pass. + + Shipped exactly that way on 2026-09-15: measured 0 streamed tokens for a + live answer, against 103 after the fix. + """ + config = {"callbacks": ["sentinel"]} + llm = _FakeLLM( + [ + AIMessage("", tool_calls=[_call("reactome_species")]), + AIMessage("Reactome covers 96 species."), + ] + ) + asyncio.run( + answer_from_live_services(llm, [reactome_species], "x", config=config) # type: ignore[arg-type] + ) + + assert llm.configs, "the model was never invoked" + assert all( + c == config for c in llm.configs + ), f"a model call lost the config: {llm.configs}" + + +def test_the_config_reaches_the_forced_final_answer_too() -> None: + """The round cap takes a different path out of the loop, and it needs the + config just as much -- that call is the one that produces the text.""" + config = {"callbacks": ["sentinel"]} + llm = _FakeLLM( + [ + AIMessage("", tool_calls=[_call("reactome_species", str(i))]) + for i in range(10) + ] + ) + asyncio.run( + answer_from_live_services(llm, [reactome_species], "x", config=config) # type: ignore[arg-type] + ) + + assert all(c == config for c in llm.configs) + + +def test_chat_history_is_carried_into_the_conversation() -> None: + """A follow-up such as "can you tell me?" is meaningless without it.""" + from langchain_core.messages import HumanMessage + + history = [HumanMessage("what species are in reactome")] + llm = _FakeLLM([AIMessage("96 species.")]) + asyncio.run( + answer_from_live_services( + llm, [reactome_species], "can you tell me?", chat_history=history + ) + ) + + sent = llm.seen[0] + assert any( + isinstance(m, HumanMessage) and "what species" in str(m.content) for m in sent + ), "the history never reached the model"