mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Harden agent retries config resolution
Two follow-ups on the configurable-retries change. A shared `default` block `retries` was silently overriding custom_tool's producer-0, which would re-enable pydantic-ai's generic retry loop inside the producer's own reflection loop. The producer's 0 is a correctness requirement, not a tunable, so it now honors only an explicit `custom_tool.retries` and ignores the default block. Separately, a non-numeric or blank `retries` value used to blow up with a bare TypeError/ValueError at agent construction -- which the manager mistook for an unknown-agent fallback and double-faulted on -- so it now raises a clear ConfigurationError.
This commit is contained in:
@@ -5688,8 +5688,10 @@
|
||||
``retries`` sets the pydantic-ai retry budget (tool calls and
|
||||
output validation); it defaults to 3. Raise it if a model
|
||||
intermittently fails to produce conforming output ("Exceeded
|
||||
maximum output retries"); custom_tool's producer defaults to 0
|
||||
since it runs its own reflection loop.
|
||||
maximum output retries"). custom_tool's producer keeps a budget of
|
||||
0 because it runs its own reflection loop; a shared ``default``
|
||||
block does not change that -- set ``custom_tool.retries``
|
||||
explicitly to override it.
|
||||
:Default: ``None``
|
||||
:Type: any
|
||||
|
||||
|
||||
@@ -29,6 +29,7 @@ from typing import (
|
||||
|
||||
import yaml
|
||||
|
||||
from galaxy.exceptions import ConfigurationError
|
||||
from galaxy.managers.context import ProvidesUserContext
|
||||
from galaxy.model import User
|
||||
from galaxy.schema.agents import (
|
||||
@@ -817,6 +818,19 @@ class BaseGalaxyAgent(ABC):
|
||||
return self.deps.config.ai_api_base_url
|
||||
return default
|
||||
|
||||
def _get_agent_specific_config(self, key: str, default: Any = None) -> Any:
|
||||
"""Read a value only from this agent's own ``inference_services`` block.
|
||||
|
||||
Unlike :meth:`_get_agent_config`, this skips the shared ``default`` block so a
|
||||
caller-pinned builtin is overridden only by an explicit per-agent entry.
|
||||
"""
|
||||
inference_config = getattr(self.deps.config, "inference_services", {})
|
||||
if isinstance(inference_config, dict):
|
||||
agent_specific = inference_config.get(self.agent_type, {})
|
||||
if isinstance(agent_specific, dict) and key in agent_specific:
|
||||
return agent_specific[key]
|
||||
return default
|
||||
|
||||
def _get_model_name(self) -> str:
|
||||
return self._get_agent_config("model", "gpt-4o-mini")
|
||||
|
||||
@@ -864,10 +878,26 @@ class BaseGalaxyAgent(ABC):
|
||||
def _get_retries(self, default: Optional[int] = None) -> int:
|
||||
"""Retry budget for the agent's pydantic-ai ``Agent(retries=...)``.
|
||||
|
||||
``default`` lets a caller override the builtin (e.g. custom_tool's producer
|
||||
keeps 0 so its own reflection loop owns the retry).
|
||||
With no ``default``, the budget resolves per-agent > ``default`` block >
|
||||
builtin (:attr:`DEFAULT_AGENT_RETRIES`). A caller-pinned ``default`` (e.g.
|
||||
custom_tool's producer keeps 0 so its own reflection loop owns the retry) is
|
||||
a correctness requirement, not a tunable: only an explicit per-agent
|
||||
``retries`` overrides it -- a shared ``default`` block must not silently
|
||||
re-enable pydantic-ai retries there.
|
||||
"""
|
||||
return int(self._get_agent_config("retries", self.DEFAULT_AGENT_RETRIES if default is None else default))
|
||||
if default is None:
|
||||
raw = self._get_agent_config("retries", self.DEFAULT_AGENT_RETRIES)
|
||||
else:
|
||||
raw = self._get_agent_specific_config("retries", default)
|
||||
try:
|
||||
retries = int(raw)
|
||||
except (TypeError, ValueError):
|
||||
retries = None
|
||||
if retries is None or retries < 0:
|
||||
raise ConfigurationError(
|
||||
f"inference_services 'retries' for agent '{self.agent_type}' must be a non-negative integer, got {raw!r}"
|
||||
)
|
||||
return retries
|
||||
|
||||
async def _call_agent_from_tool(
|
||||
self,
|
||||
|
||||
@@ -3084,8 +3084,10 @@ galaxy:
|
||||
# ``retries`` sets the pydantic-ai retry budget (tool calls and output
|
||||
# validation); it defaults to 3. Raise it if a model intermittently
|
||||
# fails to produce conforming output ("Exceeded maximum output
|
||||
# retries"); custom_tool's producer defaults to 0 since it runs its
|
||||
# own reflection loop.
|
||||
# retries"). custom_tool's producer keeps a budget of 0 because it
|
||||
# runs its own reflection loop; a shared ``default`` block does not
|
||||
# change that -- set ``custom_tool.retries`` explicitly to override
|
||||
# it.
|
||||
#inference_services: null
|
||||
|
||||
# YAML file with capability hints for agent inference models. Maps
|
||||
|
||||
@@ -4211,8 +4211,9 @@ mapping:
|
||||
Per-agent or default-block ``retries`` sets the pydantic-ai retry budget
|
||||
(tool calls and output validation); it defaults to 3. Raise it if a model
|
||||
intermittently fails to produce conforming output ("Exceeded maximum output
|
||||
retries"); custom_tool's producer defaults to 0 since it runs its own
|
||||
reflection loop.
|
||||
retries"). custom_tool's producer keeps a budget of 0 because it runs its
|
||||
own reflection loop; a shared ``default`` block does not change that -- set
|
||||
``custom_tool.retries`` explicitly to override it.
|
||||
|
||||
agent_model_capabilities_file:
|
||||
type: str
|
||||
|
||||
@@ -84,6 +84,7 @@ from galaxy.agents.page_assistant import (
|
||||
FullReplacementEdit,
|
||||
SectionPatchEdit,
|
||||
)
|
||||
from galaxy.exceptions import ConfigurationError
|
||||
from galaxy.schema.agents import ConfidenceLevel
|
||||
from galaxy.tool_util_models import UserToolSource
|
||||
from galaxy.util.unittest_utils import pytestmark_live_llm
|
||||
@@ -191,6 +192,37 @@ class TestAgentUnitMocked:
|
||||
producer = CustomToolAgent(self.deps)
|
||||
assert producer.agent._max_output_retries == 0
|
||||
|
||||
def test_producer_retries_default_ignores_default_block(self):
|
||||
# custom_tool's producer pins retries=0 so its own reflection loop owns
|
||||
# the retry. A shared `default` block must NOT silently re-enable
|
||||
# pydantic-ai retries there -- only an explicit per-agent entry may.
|
||||
producer = CustomToolAgent(self.deps)
|
||||
|
||||
self.mock_config.inference_services = {"default": {"retries": 5}}
|
||||
assert producer._get_retries(default=0) == 0
|
||||
# The normal (critic) lookup still honors the default block.
|
||||
assert producer._get_retries() == 5
|
||||
|
||||
# An explicit custom_tool entry still overrides the pinned builtin.
|
||||
self.mock_config.inference_services = {"custom_tool": {"retries": 7}}
|
||||
assert producer._get_retries(default=0) == 7
|
||||
|
||||
def test_invalid_retries_config_raises_configuration_error(self):
|
||||
# Non-numeric, blank, or negative `retries` is operator misconfiguration;
|
||||
# it must surface as a clear ConfigurationError rather than a bare
|
||||
# TypeError/ValueError (which the manager mistakes for an unknown-agent
|
||||
# fallback) or a silently-broken negative budget that fails every request.
|
||||
router = QueryRouterAgent(self.deps)
|
||||
|
||||
for bad in ("three", None, -1):
|
||||
self.mock_config.inference_services = {"default": {"retries": bad}}
|
||||
with pytest.raises(ConfigurationError, match="retries"):
|
||||
router._get_retries()
|
||||
|
||||
# 0 is valid (custom_tool's producer relies on it) and must not raise.
|
||||
self.mock_config.inference_services = {"default": {"retries": 0}}
|
||||
assert router._get_retries() == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_router_falls_back_on_output_retry_exhaustion(self):
|
||||
# When the model never produces a valid structured output, pydantic-ai
|
||||
|
||||
Reference in New Issue
Block a user