From 0a2c245cd6af3dfcdde3f61077f7429bb9c8a48a Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:20:17 -0700 Subject: [PATCH] fix(auxiliary): reach the main agent model when a sibling aux model fails on the same provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Widen okalentiev's failed_model narrowing (#59561) to _try_main_agent_model_fallback. The safety-net layer still skipped on a provider-label match alone, so single-provider users whose aux compression model and main model share one custom endpoint had ZERO fallbacks: the aux model timing out exhausted the chain in one hop and compression aborted. Real incident (0.19.0 debug dump): aux zai-org/glm-5.2 hung 324s and timed out while main mindai/macaron-v1-venti on the SAME endpoint was serving 448K-token turns — the label-only skip discarded the one viable summarizer, the session wedged over threshold, and the anti-thrash breaker tripped. Same convention as the chain fix: model-specific failures (timeout, connection, rate limit) pass failed_model so only the exact failed model is skipped; provider-wide failures (auth 401 / payment 402) pass None and keep the whole-provider skip. Both sync and async call_llm sites pass it. Sabotage-verified: the new regression test fails on the provider-only skip. --- agent/auxiliary_client.py | 34 ++++++++++++++++---- contributors/emails/okalentiev@gmail.com | 1 + tests/agent/test_auxiliary_client.py | 41 ++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 contributors/emails/okalentiev@gmail.com diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index cbc4254e6bea..910130daa3a0 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -4188,6 +4188,7 @@ def _try_main_agent_model_fallback( failed_provider: str, task: str = None, reason: str = "error", + failed_model: Optional[str] = None, ) -> Tuple[Optional[Any], Optional[str], str]: """Last-resort fallback to the user's main agent provider + model. @@ -4196,8 +4197,23 @@ def _try_main_agent_model_fallback( layer: if nothing the user asked for can serve the request, try the main chat model before giving up. - Skips when the failed provider already IS the main provider (no point - retrying the same backend that just failed). + ``failed_model`` narrows the same-provider skip to the exact + (provider, model) pair that just failed, mirroring + :func:`_try_configured_fallback_chain`. This matters for self-hosted / + custom endpoints serving several models behind one provider label: the + aux compression model timing out says nothing about the health of the + main agent model deployed on the same URL (real incident: aux + ``glm-5.2`` hung and timed out while main ``macaron-v1-venti`` on the + identical endpoint was serving 448K-token turns fine — the + provider-label skip discarded the one fallback that would have worked). + + - Model-specific runtime failures (timeout, connection, rate limit, + model-incompatible, invalid response) pass ``failed_model``: skip the + main model only when it IS the exact model that failed. + - Provider-wide failures (auth 401, payment 402) and legacy callers + leave ``failed_model`` as None, keeping the whole-provider skip — + the shared credentials/account are broken, so the main model on the + same provider cannot help either. Returns: (client, model, provider_label) or (None, None, "") if no fallback. @@ -4215,8 +4231,12 @@ def _try_main_agent_model_fallback( return None, None, "" skip = (failed_provider or "").lower().strip() - if main_provider.lower() == skip: - # The thing that failed IS the main model — nothing to fall back to. + skip_model = (failed_model or "").strip().lower() or None + if main_provider.lower() == skip and ( + skip_model is None or main_model.lower() == skip_model + ): + # The thing that failed IS the main model (or the failure was + # provider-wide) — nothing to fall back to. return None, None, "" if _is_provider_unhealthy(main_provider): _log_skip_unhealthy(main_provider, task) @@ -8135,7 +8155,8 @@ def call_llm( failed_model=_chain_failed_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_agent_model_fallback( - resolved_provider, task, reason=reason) + resolved_provider, task, reason=reason, + failed_model=_chain_failed_model) if fb_client is not None: fb_resp = _call_fallback_candidate_sync( @@ -8680,7 +8701,8 @@ async def async_call_llm( failed_model=_chain_failed_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_agent_model_fallback( - resolved_provider, task, reason=reason) + resolved_provider, task, reason=reason, + failed_model=_chain_failed_model) if fb_client is not None: # Convert sync fallback client to async diff --git a/contributors/emails/okalentiev@gmail.com b/contributors/emails/okalentiev@gmail.com new file mode 100644 index 000000000000..3445cdf65363 --- /dev/null +++ b/contributors/emails/okalentiev@gmail.com @@ -0,0 +1 @@ +okalentiev diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index 5fece4c4f783..8da9bd1edf5f 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -3167,6 +3167,47 @@ class TestTryMainAgentModelFallback: client, model, label = _try_main_agent_model_fallback("glm", task="vision") assert client is None + def test_same_provider_different_model_falls_back_when_failed_model_given(self): + """Self-hosted shape: aux model and main model share one custom + provider label. A timeout on the aux model must still reach the main + model — same provider, DIFFERENT model (real incident: aux glm-5.2 + timed out while main macaron-v1-venti on the same endpoint was + healthy; the provider-label skip discarded the working fallback).""" + from agent.auxiliary_client import _try_main_agent_model_fallback + fake_client = MagicMock() + with patch("agent.auxiliary_client._read_main_provider", return_value="custom"), \ + patch("agent.auxiliary_client._read_main_model", return_value="mindai/macaron-v1-venti"), \ + patch("agent.auxiliary_client._is_provider_unhealthy", return_value=False), \ + patch("agent.auxiliary_client.resolve_provider_client", + return_value=(fake_client, "mindai/macaron-v1-venti")): + client, model, label = _try_main_agent_model_fallback( + "custom", task="compression", failed_model="zai-org/glm-5.2") + assert client is fake_client + assert model == "mindai/macaron-v1-venti" + assert label == "main-agent(custom)" + + def test_same_provider_same_model_still_skips_with_failed_model(self): + """When the model that failed IS the main model, there is nothing to + fall back to — the narrowed skip must not regress into a self-retry.""" + from agent.auxiliary_client import _try_main_agent_model_fallback + with patch("agent.auxiliary_client._read_main_provider", return_value="custom"), \ + patch("agent.auxiliary_client._read_main_model", return_value="mindai/macaron-v1-venti"): + client, model, label = _try_main_agent_model_fallback( + "custom", task="compression", + failed_model="MindAI/Macaron-V1-Venti") # case-insensitive match + assert client is None and model is None and label == "" + + def test_same_provider_no_failed_model_keeps_provider_wide_skip(self): + """Legacy / provider-wide callers (auth 401, payment 402) pass no + failed_model: the whole-provider skip must be preserved so broken + shared credentials don't trigger a doomed main-model attempt.""" + from agent.auxiliary_client import _try_main_agent_model_fallback + with patch("agent.auxiliary_client._read_main_provider", return_value="custom"), \ + patch("agent.auxiliary_client._read_main_model", return_value="mindai/macaron-v1-venti"): + client, model, label = _try_main_agent_model_fallback( + "custom", task="compression") + assert client is None and model is None and label == "" + # --------------------------------------------------------------------------- # Gate: _resolve_api_key_provider must skip anthropic when not configured