mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Merge branch 'release_26.1' into dev
This commit is contained in:
@@ -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
|
||||
|
||||
+25
-46
@@ -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 {}
|
||||
|
||||
@@ -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",
|
||||
]
|
||||
|
||||
@@ -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)
|
||||
@@ -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"
|
||||
}
|
||||
]
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user