From e8f8b34b0c534af8e018abf2a707d04079a4108e Mon Sep 17 00:00:00 2001 From: Oleksii Kalentiev Date: Mon, 6 Jul 2026 12:28:05 +0200 Subject: [PATCH] fix(auxiliary): don't skip sibling models when a configured fallback_chain reuses the same provider _try_configured_fallback_chain skipped every fallback_chain entry whose provider matched the one that just failed. A chain that intentionally lists several models under the same provider (e.g. two more NVIDIA NIM models after the primary NIM model times out) was therefore skipped wholesale, falling straight through to the main-agent-model safety net instead of trying the other configured models on that provider. Add failed_model so the skip narrows to the exact (provider, model) pair that failed. Callers that only know the provider (client-build failures, where the whole provider is unreachable regardless of model) keep the old provider-wide skip; the two runtime request-error call sites (call_llm, async_call_llm) now pass the model that just failed. Co-Authored-By: Claude Opus 4.7 (1M context) --- agent/auxiliary_client.py | 37 ++++++++-- tests/agent/test_auxiliary_client.py | 105 ++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 7 deletions(-) diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 74ccd5781297..054ccdf3bce5 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -4326,6 +4326,7 @@ def _try_configured_fallback_chain( task: str, failed_provider: str, reason: str = "error", + failed_model: Optional[str] = None, ) -> Tuple[Optional[Any], Optional[str], str]: """Try user-configured fallback_chain for a specific auxiliary task. @@ -4333,6 +4334,20 @@ def _try_configured_fallback_chain( entry in order. Each entry must have at least ``provider``; ``model``, ``base_url``, and ``api_key`` are optional. + ``failed_model`` narrows the skip check to the exact (provider, model) + pair that just failed, rather than the whole provider. Without it (the + "no client could be built" callers, where credentials/auth are broken + for the whole provider regardless of model) every entry sharing that + provider is skipped, same as before. With it (the runtime request-error + callers), a chain that intentionally lists several models under the + same provider — e.g. two more NVIDIA NIM models after the primary NIM + model times out — is no longer skipped wholesale; only the entry that + is an exact match for what just failed is skipped, so the chain can + still serve the request from the same provider instead of jumping + straight to the main-agent-model safety net. See issue where an NVIDIA + NIM timeout on the primary compression model fell through to the main + Codex model instead of trying the two other configured NIM fallbacks. + Returns: (client, model, provider_label) or (None, None, "") if no fallback. """ @@ -4345,6 +4360,7 @@ def _try_configured_fallback_chain( return None, None, "" skip = failed_provider.lower().strip() + skip_model = (failed_model or "").strip().lower() or None tried = [] min_ctx = _task_minimum_context_length(task) @@ -4352,9 +4368,14 @@ def _try_configured_fallback_chain( if not isinstance(entry, dict): continue fb_provider = str(entry.get("provider", "")).strip() - if not fb_provider or fb_provider.lower() == skip: + if not fb_provider: continue - fb_model = str(entry.get("model", "")).strip() or None + fb_model_raw = str(entry.get("model", "")).strip() + if fb_provider.lower() == skip and ( + skip_model is None or fb_model_raw.lower() == skip_model + ): + continue + fb_model = fb_model_raw or None label = f"fallback_chain[{i}]({fb_provider})" @@ -8086,7 +8107,8 @@ def call_llm( fb_client, fb_model, fb_label = (None, None, "") if is_auto: fb_client, fb_model, fb_label = _try_configured_fallback_chain( - task, resolved_provider or "auto", reason=reason) + task, resolved_provider or "auto", reason=reason, + failed_model=final_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_fallback_chain( task, resolved_provider or "auto", reason=reason) @@ -8095,7 +8117,8 @@ def call_llm( resolved_provider, task, reason=reason) else: fb_client, fb_model, fb_label = _try_configured_fallback_chain( - task, resolved_provider or "auto", reason=reason) + task, resolved_provider or "auto", reason=reason, + failed_model=final_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_agent_model_fallback( resolved_provider, task, reason=reason) @@ -8620,7 +8643,8 @@ async def async_call_llm( fb_client, fb_model, fb_label = (None, None, "") if is_auto: fb_client, fb_model, fb_label = _try_configured_fallback_chain( - task, resolved_provider or "auto", reason=reason) + task, resolved_provider or "auto", reason=reason, + failed_model=final_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_fallback_chain( task, resolved_provider or "auto", reason=reason) @@ -8629,7 +8653,8 @@ async def async_call_llm( resolved_provider, task, reason=reason) else: fb_client, fb_model, fb_label = _try_configured_fallback_chain( - task, resolved_provider or "auto", reason=reason) + task, resolved_provider or "auto", reason=reason, + failed_model=final_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_agent_model_fallback( resolved_provider, task, reason=reason) diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index abe1e7d23bdc..3aa982ea8266 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -2858,6 +2858,7 @@ class TestAuxiliaryFallbackLayering: "title_generation", "nvidia", reason="invalid provider response", + failed_model="minimaxai/minimax-m3", ) mock_main.assert_not_called() @@ -2891,6 +2892,7 @@ class TestAuxiliaryFallbackLayering: "compression", "nvidia", reason="invalid provider response", + failed_model="minimaxai/minimax-m3", ) def test_auto_provider_uses_task_then_main_chain_before_builtin_chain(self, monkeypatch): @@ -2919,7 +2921,8 @@ class TestAuxiliaryFallbackLayering: assert main_chain_client.chat.completions.create.called mock_task_chain.assert_called_once_with( - "title_generation", "auto", reason="payment error") + "title_generation", "auto", reason="payment error", + failed_model="qwen/qwen3.5-122b-a10b") mock_main_chain.assert_called_once_with( "title_generation", "auto", reason="payment error") mock_builtin_chain.assert_not_called() @@ -6307,6 +6310,106 @@ class TestCompressionFallbackContextFilter: assert client is large_client assert model == "large-512k" + # ── same-provider, different-model chain entries ──────────────────── + # A configured fallback_chain may legitimately list several models + # under the *same* provider (e.g. two more NVIDIA NIM models after the + # primary NIM model). failed_provider alone must not skip those + # sibling entries — only failed_model narrows the skip to the exact + # (provider, model) pair that just failed. + + def test_same_provider_sibling_model_not_skipped_when_failed_model_given( + self, monkeypatch + ): + from agent.auxiliary_client import _try_configured_fallback_chain + + sibling_client = MagicMock(name="sibling_nim_client") + entries = [ + self._make_chain_entry("nvidia", "minimaxai/minimax-m3"), + self._make_chain_entry("nvidia", "deepseek-ai/deepseek-v4-flash"), + ] + + def fake_resolve(entry): + if entry is entries[0]: + return sibling_client, "minimaxai/minimax-m3" + raise AssertionError("second entry should not be reached") + + monkeypatch.setattr( + "agent.auxiliary_client._get_auxiliary_task_config", + lambda task: {"fallback_chain": entries} if task == "compression" else {}, + ) + + with patch("agent.auxiliary_client._resolve_fallback_entry", + side_effect=fake_resolve), \ + patch("agent.auxiliary_client.get_model_context_length", + return_value=1_048_576): + client, model, label = _try_configured_fallback_chain( + task="compression", + failed_provider="nvidia", + failed_model="deepseek-ai/deepseek-v4-pro", + ) + + assert client is sibling_client, ( + "A same-provider entry with a DIFFERENT model must still be " + "tried when failed_model narrows the skip — regression for the " + "bug where an NVIDIA NIM timeout fell straight through to the " + "main Codex model instead of trying the other two configured " + "NIM models." + ) + assert model == "minimaxai/minimax-m3" + assert "nvidia" in label + + def test_same_provider_same_model_still_skipped(self, monkeypatch): + """The exact (provider, model) pair that just failed is still + skipped — failed_model narrows the skip, it doesn't disable it.""" + from agent.auxiliary_client import _try_configured_fallback_chain + + entries = [ + self._make_chain_entry("nvidia", "deepseek-ai/deepseek-v4-pro"), + ] + + monkeypatch.setattr( + "agent.auxiliary_client._get_auxiliary_task_config", + lambda task: {"fallback_chain": entries} if task == "compression" else {}, + ) + + with patch("agent.auxiliary_client._resolve_fallback_entry", + side_effect=AssertionError("must not be resolved")): + client, model, label = _try_configured_fallback_chain( + task="compression", + failed_provider="nvidia", + failed_model="deepseek-ai/deepseek-v4-pro", + ) + + assert client is None + assert model is None + assert label == "" + + def test_same_provider_skipped_wholesale_without_failed_model(self, monkeypatch): + """Backward compat: callers that don't know the failed model (e.g. + client-build failures where the whole provider is unreachable) + still skip every entry sharing that provider, as before.""" + from agent.auxiliary_client import _try_configured_fallback_chain + + entries = [ + self._make_chain_entry("nvidia", "minimaxai/minimax-m3"), + self._make_chain_entry("nvidia", "deepseek-ai/deepseek-v4-flash"), + ] + + monkeypatch.setattr( + "agent.auxiliary_client._get_auxiliary_task_config", + lambda task: {"fallback_chain": entries} if task == "compression" else {}, + ) + + with patch("agent.auxiliary_client._resolve_fallback_entry", + side_effect=AssertionError("must not be resolved")): + client, model, label = _try_configured_fallback_chain( + task="compression", failed_provider="nvidia", + ) + + assert client is None + assert model is None + assert label == "" + # ── L3: main fallback chain ──────────────────────────────────────── def test_main_chain_skips_too_small_candidate_for_compression(self, monkeypatch):