fix(copilot): recompute api_mode on fallback and stop stale-mode veto (#46527)
Port of #46542 by @MarkVLK onto current main: - hermes_cli/runtime_provider.py: _copilot_runtime_api_mode must not veto a stale configured codex_responses mode for chat-only Copilot models; recompute from the live model capabilities. - agent/transports/codex.py: request summary:"auto" on the GitHub Responses API so reasoning summaries surface. - agent/agent_init.py: recompute api_mode at init-time fallback so the fallback provider/model gets its own mode (and the primary-runtime snapshot reflects it). - tests: regression tests for the stale-mode veto, the GitHub Responses summary, and the init-time fallback recompute. Conflicts resolved against main's refactored fallback ladder (agent/chat_completion_helpers.py) and its expanded transport tests. Co-authored-by: Mark Vlcek <markvlcek@gmail.com> Fixes #46527
This commit is contained in:
committed by
brooklyn!
parent
152aa695bb
commit
3bbe2a1ce2
@@ -23,6 +23,7 @@ from typing import Any, Callable, Dict, List, Optional
|
||||
from urllib.parse import parse_qs, urlparse, urlunparse
|
||||
|
||||
from agent.context_compressor import ContextCompressor
|
||||
from agent.agent_init_fallback import _fallback_entries, _init_fallback_chain, recompute_init_fallback_api_mode
|
||||
from agent.agent_runtime_helpers import _ra
|
||||
from agent.iteration_budget import IterationBudget, normalize_budget_warning_ratio
|
||||
from agent.memory_manager import StreamingContextScrubber
|
||||
@@ -899,6 +900,7 @@ def _routed_client_kwargs(agent, fallback_model, _provider_timeout) -> Optional[
|
||||
return None
|
||||
agent.provider = agent.requested_provider = _fb["provider"]
|
||||
agent.model = _fb_model or _fb["model"]
|
||||
recompute_init_fallback_api_mode(agent, _fb_client)
|
||||
return _client_kwargs_from_routed(_fb_client, _provider_timeout)
|
||||
# A burned credential pool (#119533) is otherwise indistinguishable from missing config,
|
||||
# so name it even when no fallback entries are configured.
|
||||
@@ -1088,38 +1090,6 @@ def _client_kwargs_from_routed(client, timeout) -> Dict[str, Any]:
|
||||
return kwargs
|
||||
|
||||
|
||||
def _fallback_entries(fallback_model) -> List[Dict[str, Any]]:
|
||||
"""Normalize legacy single-dict ``fallback_model`` / list ``fallback_providers``."""
|
||||
if isinstance(fallback_model, dict):
|
||||
fallback_model = [fallback_model]
|
||||
if not isinstance(fallback_model, list):
|
||||
return []
|
||||
return [
|
||||
f for f in fallback_model if isinstance(f, dict) and f.get("provider") and f.get("model")
|
||||
]
|
||||
|
||||
|
||||
def _init_fallback_chain(agent, fallback_model):
|
||||
# Stable pool-entry identity: OAuth refreshes can replace the token before a failed
|
||||
# request is recovered, so the key value alone can't attribute the failure.
|
||||
from agent.agent_runtime_helpers import sync_credential_pool_entry_id
|
||||
sync_credential_pool_entry_id(agent)
|
||||
|
||||
# Ordered backups tried when the primary is exhausted (legacy single-dict or list).
|
||||
agent._fallback_chain = _fallback_entries(fallback_model)
|
||||
agent._fallback_index = 0
|
||||
agent._fallback_activated = getattr(agent, "_fallback_activated", False)
|
||||
# Legacy attribute kept for backward compat (tests, external callers)
|
||||
agent._fallback_model = agent._fallback_chain[0] if agent._fallback_chain else None
|
||||
chain = agent._fallback_chain
|
||||
if chain and not agent.quiet_mode:
|
||||
labels = [f"{f['model']} ({f['provider']})" for f in chain]
|
||||
if len(chain) == 1:
|
||||
print(f"🔄 Fallback model: {labels[0]}")
|
||||
else:
|
||||
print(f"🔄 Fallback chain ({len(chain)} providers): " + " → ".join(labels))
|
||||
|
||||
|
||||
def _load_tools(agent, enabled_toolsets, disabled_toolsets):
|
||||
# A feature that left core for a catalog plugin (Home Assistant) is installed for a home that
|
||||
# used it, once per process, before discovery so its tools are in this agent's snapshot.
|
||||
|
||||
61
agent/agent_init_fallback.py
Normal file
61
agent/agent_init_fallback.py
Normal file
@@ -0,0 +1,61 @@
|
||||
"""Init-time fallback chain: entry normalization, chain setup, and the fallback's api_mode."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from typing import Any, Dict, List
|
||||
|
||||
logger = logging.getLogger("run_agent")
|
||||
|
||||
|
||||
def _fallback_entries(fallback_model) -> List[Dict[str, Any]]:
|
||||
"""Normalize legacy single-dict ``fallback_model`` / list ``fallback_providers``."""
|
||||
if isinstance(fallback_model, dict):
|
||||
fallback_model = [fallback_model]
|
||||
if not isinstance(fallback_model, list):
|
||||
return []
|
||||
return [
|
||||
f for f in fallback_model if isinstance(f, dict) and f.get("provider") and f.get("model")
|
||||
]
|
||||
|
||||
|
||||
def _init_fallback_chain(agent, fallback_model):
|
||||
# Stable pool-entry identity: OAuth refreshes can replace the token before a failed
|
||||
# request is recovered, so the key value alone can't attribute the failure.
|
||||
from agent.agent_runtime_helpers import sync_credential_pool_entry_id
|
||||
sync_credential_pool_entry_id(agent)
|
||||
|
||||
# Ordered backups tried when the primary is exhausted (legacy single-dict or list).
|
||||
agent._fallback_chain = _fallback_entries(fallback_model)
|
||||
agent._fallback_index = 0
|
||||
agent._fallback_activated = getattr(agent, "_fallback_activated", False)
|
||||
# Legacy attribute kept for backward compat (tests, external callers)
|
||||
agent._fallback_model = agent._fallback_chain[0] if agent._fallback_chain else None
|
||||
chain = agent._fallback_chain
|
||||
if chain and not agent.quiet_mode:
|
||||
labels = [f"{f['model']} ({f['provider']})" for f in chain]
|
||||
if len(chain) == 1:
|
||||
print(f"🔄 Fallback model: {labels[0]}")
|
||||
else:
|
||||
print(f"🔄 Fallback chain ({len(chain)} providers): " + " → ".join(labels))
|
||||
|
||||
|
||||
def recompute_init_fallback_api_mode(agent, fb_client) -> None:
|
||||
"""Give an init-time fallback its own api_mode instead of the unreachable primary's.
|
||||
|
||||
A Nous primary configured with ``api_mode: codex_responses`` otherwise leaves a Copilot
|
||||
gpt-5-mini fallback on the Responses API, which silently drops reasoning content (#46527).
|
||||
The per-turn path (``try_activate_fallback``) already recomputes via
|
||||
``_fallback_api_mode_resolved``; this keeps init in agreement, and runs before the
|
||||
``_primary_runtime`` snapshot so ``restore_primary_runtime`` replays the recomputed mode.
|
||||
A detection failure keeps the primary's mode and never blocks the fallback itself.
|
||||
"""
|
||||
from agent.chat_completion_helpers import _fallback_api_mode_resolved
|
||||
try:
|
||||
agent.api_mode = _fallback_api_mode_resolved(
|
||||
agent, agent.provider, agent.model, str(getattr(fb_client, "base_url", "") or ""))
|
||||
except Exception:
|
||||
logger.debug("Init-time fallback api_mode detection failed for %s", agent.provider, exc_info=True)
|
||||
return
|
||||
if hasattr(agent, "_transport_cache"):
|
||||
agent._transport_cache.clear()
|
||||
@@ -591,7 +591,15 @@ def _reasoning_fields(
|
||||
elif enabled:
|
||||
if is_github_responses:
|
||||
if params.get("github_reasoning_extra") is not None:
|
||||
fields["reasoning"] = params["github_reasoning_extra"]
|
||||
# Request a reasoning summary so the Responses API actually returns
|
||||
# reasoning text. Without ``summary: "auto"`` the GitHub/Copilot
|
||||
# /responses endpoint returns reasoning items with no summary, so Hermes
|
||||
# persists empty reasoning/thinking content for every Copilot model on
|
||||
# the Responses API (gpt-5.4, gpt-5, ...) — the same loss the non-GitHub
|
||||
# branch below already avoids. Verified against api.githubcopilot.com
|
||||
# /responses: the summary is only emitted when summary="auto" is sent.
|
||||
# See #46527.
|
||||
fields["reasoning"] = {**params["github_reasoning_extra"], "summary": "auto"}
|
||||
else:
|
||||
fields["reasoning"] = {"effort": effort, "summary": "auto"}
|
||||
fields["include"] = include
|
||||
|
||||
@@ -224,6 +224,18 @@ def _effective_model(model_cfg: Dict[str, Any], target_model: Optional[str]) ->
|
||||
def _copilot_runtime_api_mode(model_cfg: Dict[str, Any], api_key: str, *, target_model: Optional[str] = None) -> str:
|
||||
configured_mode = _configured_api_mode("copilot", model_cfg)
|
||||
if configured_mode:
|
||||
# A stale/incompatible explicit ``codex_responses`` must not override Copilot's hard
|
||||
# per-model requirement. Copilot serves ``gpt-5-mini`` on chat completions only; a
|
||||
# Responses-API request there silently succeeds but returns no reasoning/thinking
|
||||
# content (#46527). The stale value commonly survives an ordinary flow: the Copilot
|
||||
# OAuth setup writes ``api_mode: codex_responses`` (correct for the gpt-5.4-mini it
|
||||
# initially selects), then the user switches to gpt-5-mini without the mode being
|
||||
# recomputed. The check is the pure-regex Copilot exception — no network call; a
|
||||
# legitimate ``codex_responses`` for GPT-5 variants like gpt-5.4-mini is still honored.
|
||||
model_name = str(_effective_model(model_cfg, target_model)).strip()
|
||||
if (configured_mode == "codex_responses" and model_name
|
||||
and not _models._should_use_copilot_responses_api(model_name)):
|
||||
return "chat_completions"
|
||||
return configured_mode
|
||||
# Use the model being resolved, not the persisted default: a Claude MoA slot inheriting
|
||||
# codex_responses from a GPT-5 default fails with "model ... does not support Responses API".
|
||||
|
||||
@@ -131,3 +131,79 @@ def test_init_openrouter_exhausted_without_chain_keeps_generic_error():
|
||||
fallback_model=[{"provider": "openrouter",
|
||||
"model": "poolside/laguna-s-2.1:free"}],
|
||||
)
|
||||
|
||||
|
||||
def test_init_fallback_recomputes_api_mode_for_copilot_gpt5_mini():
|
||||
"""#46527: Nous primary (api_mode=codex_responses, no creds) -> Copilot gpt-5-mini
|
||||
fallback must end up with api_mode=chat_completions, not codex_responses.
|
||||
|
||||
Copilot's gpt-5-mini is the documented exception that uses Chat Completions,
|
||||
not the Responses API. Before the fix, ``agent.api_mode`` stayed
|
||||
``"codex_responses"`` after the init-time fallback activated, so every request
|
||||
for the fallback model went out via the Responses API and silently returned no
|
||||
reasoning/thinking content.
|
||||
"""
|
||||
fb_client = _mock_client(base_url="https://api.githubcopilot.com")
|
||||
|
||||
def fake_resolve(provider, model=None, raw_codex=False,
|
||||
explicit_base_url=None, explicit_api_key=None):
|
||||
if provider == "copilot":
|
||||
return fb_client, "gpt-5-mini"
|
||||
return None, None # primary (nous) has no usable credentials
|
||||
|
||||
with patch("agent.auxiliary_client.resolve_provider_client", side_effect=fake_resolve), \
|
||||
patch("model_tools.get_tool_definitions", return_value=_make_tool_defs()), \
|
||||
patch("model_tools.check_toolset_requirements", return_value={}), \
|
||||
patch("agent.process_bootstrap.OpenAI", return_value=MagicMock()):
|
||||
|
||||
agent = AIAgent(
|
||||
provider="nous",
|
||||
model="gpt-5.4-mini",
|
||||
api_mode="codex_responses",
|
||||
api_key=None,
|
||||
base_url=None,
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
fallback_model=[{"provider": "copilot", "model": "gpt-5-mini"}],
|
||||
)
|
||||
|
||||
assert agent.provider == "copilot"
|
||||
assert agent.model == "gpt-5-mini"
|
||||
assert agent._fallback_activated is True
|
||||
assert agent.api_mode == "chat_completions"
|
||||
|
||||
|
||||
def test_init_fallback_keeps_codex_responses_for_openai_codex():
|
||||
"""#46527: Nous primary (api_mode=codex_responses, no creds) -> openai-codex
|
||||
fallback must keep api_mode=codex_responses (openai-codex always uses the
|
||||
Responses API)."""
|
||||
fb_client = _mock_client(base_url="https://chatgpt.com/backend-api/codex")
|
||||
|
||||
def fake_resolve(provider, model=None, raw_codex=False,
|
||||
explicit_base_url=None, explicit_api_key=None):
|
||||
if provider == "openai-codex":
|
||||
return fb_client, "gpt-5.4-mini"
|
||||
return None, None
|
||||
|
||||
with patch("agent.auxiliary_client.resolve_provider_client", side_effect=fake_resolve), \
|
||||
patch("model_tools.get_tool_definitions", return_value=_make_tool_defs()), \
|
||||
patch("model_tools.check_toolset_requirements", return_value={}), \
|
||||
patch("agent.process_bootstrap.OpenAI", return_value=MagicMock()):
|
||||
|
||||
agent = AIAgent(
|
||||
provider="nous",
|
||||
model="gpt-5.4-mini",
|
||||
api_mode="codex_responses",
|
||||
api_key=None,
|
||||
base_url=None,
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
fallback_model=[{"provider": "openai-codex", "model": "gpt-5.4-mini"}],
|
||||
)
|
||||
|
||||
assert agent.provider == "openai-codex"
|
||||
assert agent.model == "gpt-5.4-mini"
|
||||
assert agent._fallback_activated is True
|
||||
assert agent.api_mode == "codex_responses"
|
||||
|
||||
@@ -53,7 +53,7 @@ class TestCodexBuildKwargs:
|
||||
reasoning_config=reasoning,
|
||||
github_reasoning_extra=ReasoningParamsMixin._github_models_reasoning_extra_body(agent),
|
||||
)
|
||||
assert kw.get("reasoning") == ({"effort": expected} if expected else None)
|
||||
assert kw.get("reasoning") == ({"effort": expected, "summary": "auto"} if expected else None)
|
||||
|
||||
def test_astra_direct_request_applies_model_contract_after_overrides(self, transport):
|
||||
kw = transport.build_kwargs(
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
"""GitHub/Copilot Responses reasoning fields on the Codex transport.
|
||||
|
||||
Regression for #46527: the GitHub Responses branch must request ``summary: "auto"`` so
|
||||
Copilot returns reasoning text. Without it, reasoning items come back with no summary and
|
||||
Hermes persists no reasoning/thinking content.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.transports import get_transport
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def transport():
|
||||
import agent.transports.codex # noqa: F401
|
||||
return get_transport("codex_responses")
|
||||
|
||||
|
||||
def test_github_responses_requests_reasoning_summary(transport):
|
||||
kw = transport.build_kwargs(
|
||||
model="gpt-5.4", messages=[{"role": "user", "content": "Hi"}], tools=[],
|
||||
is_github_responses=True,
|
||||
github_reasoning_extra={"effort": "medium"},
|
||||
)
|
||||
assert kw.get("reasoning") == {"effort": "medium", "summary": "auto"}
|
||||
6
tests/fixtures/provider_wire_snapshot.json
vendored
6
tests/fixtures/provider_wire_snapshot.json
vendored
@@ -2773,7 +2773,8 @@
|
||||
"model": "gpt-5.5",
|
||||
"parallel_tool_calls": true,
|
||||
"reasoning": {
|
||||
"effort": "high"
|
||||
"effort": "high",
|
||||
"summary": "auto"
|
||||
},
|
||||
"store": false,
|
||||
"timeout": 1800.0,
|
||||
@@ -2877,7 +2878,8 @@
|
||||
"model": "gpt-5.5",
|
||||
"parallel_tool_calls": true,
|
||||
"reasoning": {
|
||||
"effort": "medium"
|
||||
"effort": "medium",
|
||||
"summary": "auto"
|
||||
},
|
||||
"store": false,
|
||||
"timeout": 1800.0,
|
||||
|
||||
53
tests/hermes_cli/test_copilot_runtime_api_mode_stale.py
Normal file
53
tests/hermes_cli/test_copilot_runtime_api_mode_stale.py
Normal file
@@ -0,0 +1,53 @@
|
||||
"""Regression tests for #46527: Copilot primary api_mode resolution must not
|
||||
honor a stale/incompatible explicit ``api_mode``.
|
||||
|
||||
The gateway resolves and snapshots the primary provider/model at startup via
|
||||
``resolve_runtime_provider`` -> ``_copilot_runtime_api_mode``. When the config
|
||||
carries a stale ``api_mode: codex_responses`` (e.g. the Copilot OAuth setup
|
||||
wrote it for the ``gpt-5.4-mini`` it initially selects, then the user switched
|
||||
to ``gpt-5-mini``) the old code honored the stale ``codex_responses`` verbatim.
|
||||
That dispatches gpt-5-mini through the Responses API — which Copilot serves on
|
||||
chat completions only — and the response comes back with no reasoning/thinking
|
||||
content (the symptom in #46527).
|
||||
"""
|
||||
from hermes_cli.runtime_provider import _copilot_runtime_api_mode
|
||||
|
||||
|
||||
def test_stale_codex_responses_vetoed_for_gpt5_mini():
|
||||
"""gpt-5-mini + stale api_mode=codex_responses must resolve to chat_completions."""
|
||||
cfg = {"provider": "copilot", "default": "gpt-5-mini", "api_mode": "codex_responses"}
|
||||
assert _copilot_runtime_api_mode(cfg, api_key="x") == "chat_completions"
|
||||
|
||||
|
||||
def test_explicit_codex_responses_kept_for_gpt5_variant():
|
||||
"""gpt-5.4-mini legitimately uses the Responses API on Copilot — an explicit
|
||||
codex_responses must still be honored (no over-correction)."""
|
||||
cfg = {"provider": "copilot", "default": "gpt-5.4-mini", "api_mode": "codex_responses"}
|
||||
assert _copilot_runtime_api_mode(cfg, api_key="x") == "codex_responses"
|
||||
|
||||
|
||||
def test_no_explicit_mode_derives_chat_completions_for_gpt5_mini():
|
||||
"""Without an explicit api_mode, gpt-5-mini derives chat_completions (unchanged)."""
|
||||
cfg = {"provider": "copilot", "default": "gpt-5-mini"}
|
||||
assert _copilot_runtime_api_mode(cfg, api_key="x") == "chat_completions"
|
||||
|
||||
|
||||
def test_explicit_chat_completions_unchanged():
|
||||
"""An explicit chat_completions stays chat_completions for gpt-5-mini."""
|
||||
cfg = {"provider": "copilot", "default": "gpt-5-mini", "api_mode": "chat_completions"}
|
||||
assert _copilot_runtime_api_mode(cfg, api_key="x") == "chat_completions"
|
||||
|
||||
|
||||
def test_stale_mode_from_different_provider_not_honored():
|
||||
"""When the config's provider doesn't match copilot, the explicit mode is
|
||||
not honored at all (existing _provider_supports_explicit_api_mode guard),
|
||||
so gpt-5-mini still derives chat_completions."""
|
||||
cfg = {"provider": "nous", "default": "gpt-5-mini", "api_mode": "codex_responses"}
|
||||
assert _copilot_runtime_api_mode(cfg, api_key="x") == "chat_completions"
|
||||
|
||||
|
||||
def test_target_model_beats_stale_default_mode():
|
||||
"""A /model switch to gpt-5-mini must not inherit a stale codex_responses
|
||||
recorded for the persisted default."""
|
||||
cfg = {"provider": "copilot", "default": "gpt-5.4-mini", "api_mode": "codex_responses"}
|
||||
assert _copilot_runtime_api_mode(cfg, api_key="x", target_model="gpt-5-mini") == "chat_completions"
|
||||
Reference in New Issue
Block a user