ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 10 Packages Projects Releases Wiki Activity

fix(auxiliary): skip same-provider retry on a vision full-budget timeout

Browse Source 
Issue #54465 established that a same-provider retry after a full-budget
timeout costs a second whole `timeout` window before the fallback chain is
reached, doubling the user-visible stall, and that compression must not pay
it because it sits on a critical path. The guard added for that is spelled
`task == "compression"`, so vision — which sits on the interactive path —
still retries.

The cost is the same and the stall is more visible: the turn holding the
image cannot answer, and because turns are serialised the following user
messages queue behind it. Two sequential full-budget timeouts on an
unhealthy vision provider is a long stall for something the fallback chain
could have served immediately.

Replaces the string comparison at both retry sites (sync `call_llm` and
`async_call_llm`) with `_TIMEOUT_NO_RETRY_TASKS = {"compression", "vision"}`,
so the two paths cannot drift again. Behaviour is unchanged for every other
task: fast blips (a streaming-close or a 5xx) still retry, and only
full-budget timeouts on those two tasks skip straight to fallback.

Tests: vision now falls straight through to fallback with the primary tried
exactly once, and a non-critical task still gets its one same-provider
retry, so the change stays scoped. Reverting the source change fails the
vision test and leaves the scoping test green.

Not the same as #51513, which fixes five separate defects in the vision
fallback chain (capability detection, sync/async client misuse, geo-block
and RemoteProtocolError classification, and chain iteration). This is about
what happens before that chain is reached.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
xmhua
2026-08-29 09:30:15 +08:00
committed by kshitij
parent 30b83ab7b1
commit 827cf6fa01
2 changed files with 89 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agent/auxiliary_client.py
+25 -14
View File
@@ -5035,6 +5035,14 @@ def _is_invalid_aux_response_error(exc: Exception) -> bool:
)
# Auxiliary tasks that sit on a user-visible critical path. A same-provider
# retry after a full-budget timeout costs another whole ``timeout`` window
# before the fallback chain is reached, so these skip it and fall through
# immediately. Fast blips (a streaming-close or a 5xx) still retry, since
# those are cheap. See issue #54465 for the compression case.
_TIMEOUT_NO_RETRY_TASKS = frozenset({"compression", "vision"})
def _evict_cached_clients(provider: str) -> None:
"""Drop cached auxiliary clients for a provider so fresh creds are used."""
normalized = _normalize_aux_provider(provider)
@@ -10622,14 +10630,17 @@ def _call_llm_impl(
if not _is_transient_transport_error(transient_err):
raise
# Compression is on the critical preflight path: a user cannot
# continue or resume an oversized session until it compacts. A
# same-provider retry on a timeout means another full ``timeout``-
# long wall-clock block before the except-chain below can fall
# back — doubling the user-visible stall (issue #54465). Skip the
# same-provider retry for compression on a full-budget timeout and
# continue or resume an oversized session until it compacts.
# Vision is on the interactive path: the turn holding the image
# cannot answer, and because turns are serialised the following
# user messages stall behind it. In both cases a same-provider
# retry on a timeout means another full ``timeout``-long
# wall-clock block before the except-chain below can fall back —
# doubling the user-visible stall (issue #54465). Skip the
# same-provider retry for those tasks on a full-budget timeout and
# fall straight through to provider/model fallback; fast blips (a
# streaming-close or a 5xx) still retry, since those are cheap.
if task == "compression" and _is_timeout_error(transient_err):
if task in _TIMEOUT_NO_RETRY_TASKS and _is_timeout_error(transient_err):
# A fast first-token fail (dead stream detected within the
# 60s no-progress window, zero output seen) is cheap — take
# the normal same-provider retry chain first; the provider
@@ -10639,9 +10650,9 @@ def _call_llm_impl(
# same provider doubles the user-visible stall (#54465).
if "no-progress timeout" not in str(transient_err):
logger.info(
"Auxiliary compression: timeout on the critical path; "
"Auxiliary %s: timeout on the critical path; "
"skipping same-provider retry and falling back: %s",
transient_err,
task, transient_err,
)
raise
_max_transient_retries = _transient_retry_count()
@@ -11459,14 +11470,14 @@ async def _async_call_llm_impl(
except Exception as transient_err:
if not _is_transient_transport_error(transient_err):
raise
# See call_llm(): compression is on the critical preflight path,
# so skip the same-provider retry on a full-budget timeout and
# fall straight through to fallback (issue #54465).
if task == "compression" and _is_timeout_error(transient_err):
# See call_llm(): compression and vision both sit on a critical
# path, so skip the same-provider retry on a full-budget timeout
# and fall straight through to fallback (issue #54465).
if task in _TIMEOUT_NO_RETRY_TASKS and _is_timeout_error(transient_err):
logger.info(
"Auxiliary compression (async): timeout on the critical "
"Auxiliary %s (async): timeout on the critical "
"path; skipping same-provider retry and falling back: %s",
transient_err,
task, transient_err,
)
raise
logger.info(
tests/agent/test_auxiliary_client.py
+64
View File
@@ -2118,6 +2118,70 @@ class TestTransientTransportRetry:
assert primary.chat.completions.create.call_count == 1
assert fb_client.chat.completions.create.call_count == 1
def test_vision_skips_same_provider_retry_on_timeout(self):
"""Vision is on the interactive critical path: the turn holding the
image cannot answer, and because turns are serialised the following
user messages stall behind it. A full-budget timeout must therefore
fall straight through to fallback rather than spending a second
``timeout`` window on the same provider (same reasoning as #54465).
"""
class _Timeout(Exception):
pass
_Timeout.__name__ = "APITimeoutError"
primary = MagicMock()
primary.base_url = "https://openrouter.ai/api/v1"
primary.chat.completions.create.side_effect = _Timeout("Request timed out.")
fb_client = MagicMock()
fb_client.base_url = "https://api.openai.com/v1"
fb_client.chat.completions.create.return_value = {"fallback": True}
p1, p2, p3 = self._patches(primary)
with (
p1, p2, p3,
# Vision resolves its client through resolve_vision_provider_client(),
# not _get_cached_client(); the retry block under test is shared.
patch(
"agent.auxiliary_client.resolve_vision_provider_client",
return_value=("openrouter", primary, "some-model"),
),
patch(
"agent.auxiliary_client._try_configured_fallback_chain",
return_value=(None, None, ""),
),
patch(
"agent.auxiliary_client._try_main_agent_model_fallback",
return_value=(fb_client, "fb-model", "openai"),
),
):
result = call_llm(task="vision", messages=[{"role": "user", "content": "hi"}])
assert result == {"fallback": True}
assert primary.chat.completions.create.call_count == 1
assert fb_client.chat.completions.create.call_count == 1
def test_non_critical_task_still_retries_same_provider_on_timeout(self):
"""The skip is scoped to critical-path tasks. Everything else keeps the
existing one-shot same-provider retry, so this is not a blanket change.
"""
class _Timeout(Exception):
pass
_Timeout.__name__ = "APITimeoutError"
primary = MagicMock()
primary.base_url = "https://openrouter.ai/api/v1"
primary.chat.completions.create.side_effect = [
_Timeout("Request timed out."),
{"retried": True},
]
p1, p2, p3 = self._patches(primary)
with p1, p2, p3:
result = call_llm(task="title", messages=[{"role": "user", "content": "hi"}])
assert result == {"retried": True}
# Same provider was retried once — unchanged behaviour off the critical path.
assert primary.chat.completions.create.call_count == 2
def test_timeout_forwards_failed_model_to_configured_chain(self):
"""A timeout is model-specific, so call_llm must forward the failed
model to the configured chain (failed_model=<model>, not None). This