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):