From d8831a83d2a7c7ff7265ea69d886b74952ca7e89 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Fri, 19 Jun 2026 12:36:14 -0400 Subject: [PATCH 1/2] Add failing test for router losing follow-up context After a normal answer, an elliptical follow-up ("what about a workflow for this?") routes on the current message alone, so its referent is lost and the router asks for clarification on context it already had. These tests pin the behavior we want instead: forward the whole previous turn -- request and reply -- so the referent survives, capped at the last turn so deep history stays out. --- test/unit/app/test_agents.py | 55 ++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/test/unit/app/test_agents.py b/test/unit/app/test_agents.py index 343171fdc7f..632770d4c7a 100644 --- a/test/unit/app/test_agents.py +++ b/test/unit/app/test_agents.py @@ -629,32 +629,63 @@ class TestAgentUnitMocked: ) @pytest.mark.asyncio - async def test_router_withholds_history_for_routing(self): - """Router routes on the current message, withholding conversation history from the - model. Feeding history dilutes the routing signal (evals show it degrades routing - monotonically), so ``message_history`` is not forwarded; specialists still get the - full history via the handoff context.""" + async def test_router_forwards_prior_turn_for_followup(self): + """A follow-up to a normal answer ("what about a workflow for this?") needs the prior + turn for its referent, so the router forwards the whole last turn -- the prior request + AND its assistant reply. (A bare user message with no reply reads as a dangling request + and mis-routes.) Deep history is still withheld; specialists get it via the handoff.""" router = QueryRouterAgent(self.deps) history: list[ModelMessage] = [ - ModelRequest(parts=[UserPromptPart(content="What histories do I have?")]), - ModelResponse(parts=[TextPart(content="You have 3.")]), + ModelRequest(parts=[UserPromptPart(content="Is there a tutorial for quantifying histological staining?")]), + ModelResponse(parts=[TextPart(content="Yes -- here is a GTN tutorial on color deconvolution.")]), ] with mock.patch.object(router, "_run_with_retry") as mock_run: mock_result = mock.Mock(spec=["output"]) - mock_result.output = "Following up: here is more detail." + mock_result.output = "Routed." mock_run.return_value = mock_result await router.process( - "Tell me more about the second one", + "What about a workflow for this?", context={"conversation_history": history}, ) mock_run.assert_called_once() args, kwargs = mock_run.call_args - # Routing decision is made on the current message, not the accumulated history. - assert kwargs["message_history"] is None - assert args[0] == "Tell me more about the second one" + assert args[0] == "What about a workflow for this?" + forwarded = kwargs["message_history"] + assert forwarded is not None + # The whole prior turn rides along -- request and reply both. + contents = [part.content for message in forwarded for part in message.parts] + assert any("histological staining" in content for content in contents) + assert any("color deconvolution" in content for content in contents) + + @pytest.mark.asyncio + async def test_router_followup_history_capped_to_last_turn(self): + """Across a deeper conversation the router forwards only the most recent turn + (ROUTING_HISTORY_TURNS) -- both its messages, not the older turns -- enough to resolve + the referent without re-diluting the routing signal that #22791 protects.""" + router = QueryRouterAgent(self.deps) + history: list[ModelMessage] = [ + ModelRequest(parts=[UserPromptPart(content="What histories do I have?")]), + ModelResponse(parts=[TextPart(content="You have 3.")]), + ModelRequest(parts=[UserPromptPart(content="Tell me about RNA-seq alignment tools")]), + ModelResponse(parts=[TextPart(content="HISAT2 and STAR are common aligners.")]), + ] + + with mock.patch.object(router, "_run_with_retry") as mock_run: + mock_result = mock.Mock(spec=["output"]) + mock_result.output = "Routed." + mock_run.return_value = mock_result + + await router.process("Is there a workflow for that?", context={"conversation_history": history}) + + _, kwargs = mock_run.call_args + forwarded = kwargs["message_history"] + assert forwarded is not None + # Only the last turn (both messages), not the earlier "What histories" turn. + contents = [part.content for message in forwarded for part in message.parts] + assert contents == ["Tell me about RNA-seq alignment tools", "HISAT2 and STAR are common aligners."] @pytest.mark.asyncio async def test_router_injects_interface_context_into_prompt(self): From 38e64675649408b9dd29677814e3630c3b7c4eb5 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Fri, 19 Jun 2026 12:36:29 -0400 Subject: [PATCH 2/2] Route follow-ups using the previous turn The router decides routing on the current message -- deep history biases it toward answering directly, which is the regression #22791 fixed by withholding history. But that left elliptical follow-ups with no referent: "what about a workflow for this?" got bounced back with a clarifying question for context we already had. #22791 carved out one exception, the answer to a clarifying question (responding_to_clarification); a follow-up to a normal answer hit the same wall but wasn't covered. So forward the most recent turn for every routing decision (ROUTING_HISTORY_TURNS=1). It's the whole turn, request and reply: a bare user message with no reply reads as a dangling/compound request and mis-routes to the orchestrator. That makes the follow-up and clarification paths identical, so the responding_to_clarification branch in the router collapses into one path (the flag plumbing in chat.py/managers stays for now). Evals on gpt-oss-120b: follow-ups recover from 1/6 to ~5-6/6, and routing_depth holds at 18/18 -- forwarding a single turn doesn't reintroduce the deep-history bias. Adds a routing_followup eval with a nofix A/B for the quadrant routing_clarification_followup missed. For #22973. --- lib/galaxy/agents/prompts/router.md | 8 ++- lib/galaxy/agents/router.py | 71 +++++++------------ test/evals/datasets/__init__.py | 2 + test/evals/datasets/routing_followup.py | 63 ++++++++++++++++ .../datasets/routing_followup_scenarios.json | 50 +++++++++++++ test/evals/specs.py | 45 ++++++++++++ test/evals/tasks.py | 33 +++++++++ 7 files changed, 225 insertions(+), 47 deletions(-) create mode 100644 test/evals/datasets/routing_followup.py create mode 100644 test/evals/datasets/routing_followup_scenarios.json diff --git a/lib/galaxy/agents/prompts/router.md b/lib/galaxy/agents/prompts/router.md index ef351721466..a612abd417d 100644 --- a/lib/galaxy/agents/prompts/router.md +++ b/lib/galaxy/agents/prompts/router.md @@ -54,7 +54,7 @@ Ask when: - The message names no analysis, tool, dataset, or goal ("Can you help with my data?", "What should I do next?") - A failure is reported with no error text, exit code, or tool name ("It keeps failing", "My job isn't working") - The intent could plausibly mean several different things -- a tool, a tutorial, usage help, or debugging ("I need help with variant calling") -- A follow-up's referent cannot be determined from the message itself ("Is there a better one?") +- A follow-up's referent cannot be resolved even from the recent turn you were given Do NOT ask when the current message is clear enough to route or answer on its own. A confident route or answer is always better than an unnecessary question -- over-asking is @@ -62,6 +62,12 @@ as harmful as mis-routing. When you do ask, name the options where you can ("Do tool recommendation or a tutorial?") rather than a generic "can you clarify?". You may pass 2-4 short `options` so the user can pick an answer directly. +You are given the most recent turn of the conversation (the previous message and its reply) +as context. When the current message is a follow-up -- "what about a workflow for this?", +"is there a better one?", "and for paired-end reads?" -- resolve "this" / "it" / "one" from +that prior turn and route accordingly. Do not ask what it refers to when the prior turn makes +it clear. + If the user's message is answering a clarifying question you just asked, route using that question together with their original request -- e.g. after you asked "tool recommendation or a tutorial?", a reply of "the second one" or "a tutorial" means hand off to the tutorial diff --git a/lib/galaxy/agents/router.py b/lib/galaxy/agents/router.py index 2a65cf057cd..a5a3c3ad001 100644 --- a/lib/galaxy/agents/router.py +++ b/lib/galaxy/agents/router.py @@ -51,13 +51,15 @@ class QueryRouterAgent(BaseGalaxyAgent): agent_type = AgentType.ROUTER _handoff_context: Optional[dict[str, Any]] = None - # How many recent conversation turns the router sees when making its routing - # decision. Evals show routing accuracy degrades monotonically as more history is - # fed to the router -- a tool/workflow question that routes correctly on turn 1 gets - # answered directly deep in a conversation. Routing on the current message alone - # (0 turns) recovers it; specialists still receive the full conversation_history via - # the handoff context, so nothing downstream loses information. - ROUTING_HISTORY_TURNS = 0 + # The current message drives routing, plus the most recent conversation turn(s) so an + # elliptical follow-up ("what about a workflow for this?", or the answer to a clarifying + # question -- "the second one") keeps its referent. Capped at ROUTING_HISTORY_TURNS: + # forwarding the whole deep conversation degrades routing -- it biases the model toward + # answering directly (the #22791 finding) -- but forwarding the last turn does not. It's + # the whole turn, request and reply: a bare user message with no assistant reply reads as + # a dangling/compound request and mis-routes. Specialists always receive the full + # conversation_history via the handoff context. + ROUTING_HISTORY_TURNS = 1 def _create_agent(self) -> Agent[GalaxyAgentDependencies, str]: model_name = self._get_agent_config("model", "") @@ -499,7 +501,7 @@ class QueryRouterAgent(BaseGalaxyAgent): - A failure is reported with no error text, exit code, or tool name ("It keeps failing") - The intent could plausibly mean several different things -- a tool, a tutorial, usage help, or debugging ("I need help with variant calling") - - A follow-up's referent cannot be determined from the message itself + - A follow-up's referent cannot be resolved even from the recent turn provided Do NOT use this when the current message is clear enough to route or answer on its own -- a confident route or answer is always better than an unnecessary question. @@ -523,33 +525,16 @@ class QueryRouterAgent(BaseGalaxyAgent): return [i for i, message in enumerate(full_history) if _is_turn_start(message)] def _routing_history(self, full_history: Optional[list]) -> Optional[list]: - """The history the router uses for its routing decision. - - Capped at ``ROUTING_HISTORY_TURNS`` recent turns (0 -> none). Conversation history - dilutes the routing signal and biases the model toward answering directly, so the - router routes on the current message while specialists still get the full history. - """ + """The most recent ``ROUTING_HISTORY_TURNS`` turn(s) of ``full_history``, or None when + capped to 0 or there's no history. See ``ROUTING_HISTORY_TURNS`` for the rationale.""" if not full_history or self.ROUTING_HISTORY_TURNS <= 0: return None - - turn_starts = self._turn_start_indices(full_history) - if len(turn_starts) <= self.ROUTING_HISTORY_TURNS: - return full_history - return full_history[turn_starts[-self.ROUTING_HISTORY_TURNS] :] - - def _clarification_routing_history(self, full_history: Optional[list]) -> list: - """The last conversation turn (original request + the clarifying question we asked). - - A narrow exception to history-withholding: when the user is answering a clarification, - the router needs that turn to route an elliptical answer like "the second one" -- on - its own it has no referent. Specialists still receive the full history via the handoff. - """ - if not full_history: - return [] turn_starts = self._turn_start_indices(full_history) if not turn_starts: - return full_history - return full_history[turn_starts[-1] :] + return None + if len(turn_starts) > self.ROUTING_HISTORY_TURNS: + return full_history[turn_starts[-self.ROUTING_HISTORY_TURNS] :] + return full_history async def process(self, query: str, context: Optional[dict[str, Any]] = None) -> AgentResponse: validation_error = self._validate_query(query) @@ -558,22 +543,16 @@ class QueryRouterAgent(BaseGalaxyAgent): try: full_history = self._extract_message_history(context) - # Route on the current message; deep history degrades the routing decision. - # Specialists still receive the full conversation_history via _handoff_context. - message_history: Optional[list] - if context and context.get("responding_to_clarification") and full_history: - # The previous turn asked a clarifying question -- route the answer using - # that one turn so an elliptical reply ("the second one") has a referent. - message_history = self._clarification_routing_history(full_history) - log.info(f"Router: answering a clarification, routing on the last turn ({len(message_history)} msgs)") + # Route on the current message plus the most recent turn(s) (see ROUTING_HISTORY_TURNS). + # The responding_to_clarification context flag is intentionally not consulted: forwarding + # the last turn handles clarification answers and ordinary follow-ups the same way. + message_history = self._routing_history(full_history) + if message_history: + log.info(f"Router: routing on the current message plus the last turn ({len(message_history)} msgs)") + elif full_history: + log.info(f"Router: routing on the current message, withholding {len(full_history)} history messages") else: - message_history = self._routing_history(full_history) - if full_history and not message_history: - log.info(f"Router: routing on current message, withholding {len(full_history)} history messages") - elif message_history: - log.info(f"Router: routing on {len(message_history)} recent messages") - else: - log.info("Router: processing query with no conversation history") + log.info("Router: processing query with no conversation history") previous_handoff_context = self._handoff_context self._handoff_context = context.copy() if context else {} diff --git a/test/evals/datasets/__init__.py b/test/evals/datasets/__init__.py index 8a5a528dfbe..d1ad4b3125a 100644 --- a/test/evals/datasets/__init__.py +++ b/test/evals/datasets/__init__.py @@ -13,6 +13,7 @@ from .routing_depth import ( build_history, routing_depth_dataset, ) +from .routing_followup import routing_followup_dataset from .staining_quantification import staining_quantification_dataset from .tool_recommendation import tool_recommendation_dataset @@ -28,6 +29,7 @@ __all__ = [ "routing_clarification_followup_dataset", "routing_dataset", "routing_depth_dataset", + "routing_followup_dataset", "staining_quantification_dataset", "tool_recommendation_dataset", ] diff --git a/test/evals/datasets/routing_followup.py b/test/evals/datasets/routing_followup.py new file mode 100644 index 00000000000..bebfb6e961e --- /dev/null +++ b/test/evals/datasets/routing_followup.py @@ -0,0 +1,63 @@ +"""Routing followup dataset: route a follow-up to a NORMAL (non-clarification) answer. + +The router routes on the current message and forwards the prior user turn(s) so an elliptical +follow-up keeps its referent. ``routing_clarification_followup`` covers the case where the +previous assistant turn was a clarifying question; this dataset covers the adjacent case the +clarification carve-out missed: a follow-up to an ordinary answer -- "what about a workflow +for this?", "is there a tutorial for that?" -- whose "this"/"that"/"it" points at the prior +*user* message, not the assistant's prose. + +Each case reconstructs the prior turn (the user's request + a normal assistant answer) and +provides the follow-up as the current message. Scored by HandoffMatch on the router's chosen +``agent_type`` against the gold specialist. Run with the fix OFF (``ROUTING_HISTORY_TURNS = 0``) +vs ON to quantify its value: without it the referent is lost and the router asks for +clarification (agent_type "clarification") instead of routing. + +Scenarios are generated data in ``routing_followup_scenarios.json``. +""" + +import json +from pathlib import Path +from typing import ( + Any, + Optional, +) + +from pydantic_evals import ( + Case, + Dataset, +) + +_SCENARIOS_PATH = Path(__file__).parent / "routing_followup_scenarios.json" + + +def _load_scenarios() -> list[dict[str, Any]]: + return json.loads(_SCENARIOS_PATH.read_text()) + + +def routing_followup_dataset( + only: Optional[list[str]] = None, +) -> Dataset[dict[str, Any], str, dict[str, Any]]: + """Build the followup Dataset. + + Case input is ``{"original_query", "assistant_answer", "followup"}``; the task + reconstructs the prior turn into conversation_history and routes the follow-up. + """ + cases: list[Case[dict[str, Any], str, dict[str, Any]]] = [] + for scenario in _load_scenarios(): + cases.append( + Case( + name=scenario["name"], + inputs={ + "original_query": scenario["original_query"], + "assistant_answer": scenario["assistant_answer"], + "followup": scenario["followup"], + }, + expected_output=scenario["expected"], + metadata={"followup_kind": scenario.get("followup_kind", ""), "requires_galaxy": False}, + ) + ) + if only: + wanted = set(only) + cases = [c for c in cases if c.name in wanted] + return Dataset(name="routing_followup", cases=cases) diff --git a/test/evals/datasets/routing_followup_scenarios.json b/test/evals/datasets/routing_followup_scenarios.json new file mode 100644 index 00000000000..df098f94324 --- /dev/null +++ b/test/evals/datasets/routing_followup_scenarios.json @@ -0,0 +1,50 @@ +[ + { + "name": "staining_tutorial_then_workflow", + "original_query": "Is there a tutorial for quantifying histological staining?", + "assistant_answer": "Yes -- the GTN has a tutorial on quantitative analysis of histological staining using color deconvolution that walks through the full process with example data.", + "followup": "What about a workflow for this?", + "expected": "tool_recommendation", + "followup_kind": "referential" + }, + { + "name": "trim_tool_then_tutorial", + "original_query": "What tool can I use to trim adapters from my reads?", + "assistant_answer": "Trimmomatic and Cutadapt both trim adapter sequences from sequencing reads.", + "followup": "Is there a tutorial for that?", + "expected": "gtn_training", + "followup_kind": "referential" + }, + { + "name": "rnaseq_then_workflow", + "original_query": "How do I do RNA-seq differential expression in Galaxy?", + "assistant_answer": "You align the reads, count features, and run a differential expression test such as DESeq2.", + "followup": "Is there a ready-made workflow for it?", + "expected": "tool_recommendation", + "followup_kind": "referential" + }, + { + "name": "variant_tutorial_better_one", + "original_query": "Can you recommend a tutorial for variant calling?", + "assistant_answer": "The GTN variant calling tutorial walks through mapping, calling, and filtering variants.", + "followup": "Is there a better one for bacterial genomes?", + "expected": "gtn_training", + "followup_kind": "referential" + }, + { + "name": "longread_tool_then_workflow", + "original_query": "Which tool aligns long reads?", + "assistant_answer": "Minimap2 is commonly used to align long reads against a reference.", + "followup": "And a full workflow that uses it?", + "expected": "tool_recommendation", + "followup_kind": "referential" + }, + { + "name": "assembly_then_tutorial", + "original_query": "What can I use to assemble a bacterial genome?", + "assistant_answer": "Unicycler and SPAdes are widely used for bacterial genome assembly.", + "followup": "How do I learn the whole process end to end?", + "expected": "gtn_training", + "followup_kind": "named" + } +] diff --git a/test/evals/specs.py b/test/evals/specs.py index 2f9ef6f62fd..4a3f289dfe8 100644 --- a/test/evals/specs.py +++ b/test/evals/specs.py @@ -30,6 +30,7 @@ from .datasets import ( routing_clarification_followup_dataset, routing_dataset, routing_depth_dataset, + routing_followup_dataset, staining_quantification_dataset, tool_recommendation_dataset, ) @@ -49,6 +50,7 @@ from .tasks import ( make_orchestrator_plan_task, make_router_clarification_task, make_router_content_task, + make_router_followup_task, make_router_inspect_task, make_router_multiturn_task, make_router_task, @@ -183,6 +185,47 @@ def build_routing_clarification_followup_nofix( return _build_routing_clarification_followup(deps, False, only, usage_buffer) +def _build_routing_followup( + deps: GalaxyAgentDependencies, + route_followup: bool, + only: Optional[list[str]], + usage_buffer: Optional[list[dict[str, int]]], +) -> BuiltDataset: + dataset = routing_followup_dataset(only=only) + dataset.add_evaluator(HandoffMatch()) + return BuiltDataset( + dataset=dataset, + task=make_router_followup_task(deps, route_followup=route_followup, usage_buffer=usage_buffer), + primary_score="HandoffMatch", + ) + + +def build_routing_followup( + deps: GalaxyAgentDependencies, + judge_model: Optional[Model] = None, + only: Optional[list[str]] = None, + include_galaxy_required: bool = False, + usage_buffer: Optional[list[dict[str, int]]] = None, +) -> BuiltDataset: + """Route a follow-up to a normal answer WITH the fix (the shipped behavior): the router + sees the prior user turn, so "what about a workflow for this?" routes to the right + specialist instead of asking what "this" means.""" + return _build_routing_followup(deps, True, only, usage_buffer) + + +def build_routing_followup_nofix( + deps: GalaxyAgentDependencies, + judge_model: Optional[Model] = None, + only: Optional[list[str]] = None, + include_galaxy_required: bool = False, + usage_buffer: Optional[list[dict[str, int]]] = None, +) -> BuiltDataset: + """A/B control: route the same follow-ups WITHOUT the fix (prior user turn withheld). The + elliptical follow-up has no referent, so this should score well below the fixed variant -- + that gap is the fix's value.""" + return _build_routing_followup(deps, False, only, usage_buffer) + + def build_error_analysis( deps: GalaxyAgentDependencies, judge_model: Optional[Model] = None, @@ -323,6 +366,8 @@ SPECS: dict[str, Callable[..., BuiltDataset]] = { "routing_ambiguous": build_routing_ambiguous, "routing_clarification_followup": build_routing_clarification_followup, "routing_clarification_followup_nofix": build_routing_clarification_followup_nofix, + "routing_followup": build_routing_followup, + "routing_followup_nofix": build_routing_followup_nofix, "error_analysis": build_error_analysis, "tool_recommendation": build_tool_recommendation, "custom_tool": build_custom_tool, diff --git a/test/evals/tasks.py b/test/evals/tasks.py index 110aed7ddd9..818e215ce07 100644 --- a/test/evals/tasks.py +++ b/test/evals/tasks.py @@ -257,6 +257,39 @@ def make_router_clarification_task( return router_clarification_task +def make_router_followup_task( + deps: GalaxyAgentDependencies, + route_followup: bool = True, + usage_buffer: UsageBuffer = None, +) -> Callable[[dict], Awaitable[str]]: + """Build an async callable for the followup dataset. + + The case input is ``{"original_query", "assistant_answer", "followup"}``. Reconstructs the + prior turn (the user's request + a normal assistant answer) as conversation_history and + routes the elliptical ``followup``. With ``route_followup=True`` (the shipped default) the + router forwards the prior turn so "this"/"that" has a referent; with ``False`` it sets + ``ROUTING_HISTORY_TURNS = 0`` to withhold it -- the A/B that quantifies the fix's value. + Returns the router's chosen agent_type. + """ + + async def router_followup_task(case_input: dict) -> str: + history = [ + {"role": "user", "content": case_input["original_query"]}, + {"role": "assistant", "content": case_input["assistant_answer"]}, + ] + router = QueryRouterAgent(deps) + if not route_followup: + router.ROUTING_HISTORY_TURNS = 0 + response = await router.process( + case_input["followup"], + context={"conversation_history": history}, + ) + _record_response_usage(usage_buffer, response) + return response.agent_type + + return router_followup_task + + def make_router_content_task( deps: GalaxyAgentDependencies, context: Optional[dict] = None,