Merge pull request #13622 from NousResearch/bb/tui-model-switch-sticks
fix(model-switch): /model --provider X sticks instead of silently falling back
This commit is contained in:
16
run_agent.py
16
run_agent.py
@@ -2005,6 +2005,22 @@ class AIAgent:
|
|||||||
self._fallback_activated = False
|
self._fallback_activated = False
|
||||||
self._fallback_index = 0
|
self._fallback_index = 0
|
||||||
|
|
||||||
|
# When the user deliberately swaps primary providers (e.g. openrouter
|
||||||
|
# → anthropic), drop any fallback entries that target the OLD primary
|
||||||
|
# or the NEW one. The chain was seeded from config at agent init for
|
||||||
|
# the original provider — without pruning, a failed turn on the new
|
||||||
|
# primary silently re-activates the provider the user just rejected,
|
||||||
|
# which is exactly what was reported during TUI v2 blitz testing
|
||||||
|
# ("switched to anthropic, tui keeps trying openrouter").
|
||||||
|
old_norm = (old_provider or "").strip().lower()
|
||||||
|
new_norm = (new_provider or "").strip().lower()
|
||||||
|
if old_norm and new_norm and old_norm != new_norm:
|
||||||
|
self._fallback_chain = [
|
||||||
|
entry for entry in self._fallback_chain
|
||||||
|
if (entry.get("provider") or "").strip().lower() not in {old_norm, new_norm}
|
||||||
|
]
|
||||||
|
self._fallback_model = self._fallback_chain[0] if self._fallback_chain else None
|
||||||
|
|
||||||
logging.info(
|
logging.info(
|
||||||
"Model switched in-place: %s (%s) -> %s (%s)",
|
"Model switched in-place: %s (%s) -> %s (%s)",
|
||||||
old_model, old_provider, new_model, new_provider,
|
old_model, old_provider, new_model, new_provider,
|
||||||
|
|||||||
93
tests/run_agent/test_switch_model_fallback_prune.py
Normal file
93
tests/run_agent/test_switch_model_fallback_prune.py
Normal file
@@ -0,0 +1,93 @@
|
|||||||
|
"""Regression test for TUI v2 blitz bug: explicit /model --provider switch
|
||||||
|
silently fell back to the old primary provider on the next turn because the
|
||||||
|
fallback chain — seeded from config at agent __init__ — kept entries for the
|
||||||
|
provider the user just moved away from.
|
||||||
|
|
||||||
|
Reported: "switched from openrouter provider to anthropic api key via hermes
|
||||||
|
model and the tui keeps trying openrouter".
|
||||||
|
"""
|
||||||
|
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
from run_agent import AIAgent
|
||||||
|
|
||||||
|
|
||||||
|
def _make_agent(chain):
|
||||||
|
agent = AIAgent.__new__(AIAgent)
|
||||||
|
|
||||||
|
agent.provider = "openrouter"
|
||||||
|
agent.model = "x-ai/grok-4"
|
||||||
|
agent.base_url = "https://openrouter.ai/api/v1"
|
||||||
|
agent.api_key = "or-key"
|
||||||
|
agent.api_mode = "chat_completions"
|
||||||
|
agent.client = MagicMock()
|
||||||
|
agent._client_kwargs = {"api_key": "or-key", "base_url": "https://openrouter.ai/api/v1"}
|
||||||
|
agent.context_compressor = None
|
||||||
|
agent._anthropic_api_key = ""
|
||||||
|
agent._anthropic_base_url = None
|
||||||
|
agent._anthropic_client = None
|
||||||
|
agent._is_anthropic_oauth = False
|
||||||
|
agent._cached_system_prompt = "cached"
|
||||||
|
agent._primary_runtime = {}
|
||||||
|
agent._fallback_activated = False
|
||||||
|
agent._fallback_index = 0
|
||||||
|
agent._fallback_chain = list(chain)
|
||||||
|
agent._fallback_model = chain[0] if chain else None
|
||||||
|
|
||||||
|
return agent
|
||||||
|
|
||||||
|
|
||||||
|
def _switch_to_anthropic(agent):
|
||||||
|
with (
|
||||||
|
patch("agent.anthropic_adapter.build_anthropic_client", return_value=MagicMock()),
|
||||||
|
patch("agent.anthropic_adapter.resolve_anthropic_token", return_value="sk-ant-xyz"),
|
||||||
|
patch("agent.anthropic_adapter._is_oauth_token", return_value=False),
|
||||||
|
patch("hermes_cli.timeouts.get_provider_request_timeout", return_value=None),
|
||||||
|
):
|
||||||
|
agent.switch_model(
|
||||||
|
new_model="claude-sonnet-4-5",
|
||||||
|
new_provider="anthropic",
|
||||||
|
api_key="sk-ant-xyz",
|
||||||
|
base_url="https://api.anthropic.com",
|
||||||
|
api_mode="anthropic_messages",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_switch_drops_old_primary_from_fallback_chain():
|
||||||
|
agent = _make_agent([
|
||||||
|
{"provider": "openrouter", "model": "x-ai/grok-4"},
|
||||||
|
{"provider": "nous", "model": "hermes-4"},
|
||||||
|
])
|
||||||
|
|
||||||
|
_switch_to_anthropic(agent)
|
||||||
|
|
||||||
|
providers = [entry["provider"] for entry in agent._fallback_chain]
|
||||||
|
|
||||||
|
assert "openrouter" not in providers, "old primary must be pruned"
|
||||||
|
assert "anthropic" not in providers, "new primary is redundant in the chain"
|
||||||
|
assert providers == ["nous"]
|
||||||
|
assert agent._fallback_model == {"provider": "nous", "model": "hermes-4"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_switch_with_empty_chain_stays_empty():
|
||||||
|
agent = _make_agent([])
|
||||||
|
|
||||||
|
_switch_to_anthropic(agent)
|
||||||
|
|
||||||
|
assert agent._fallback_chain == []
|
||||||
|
assert agent._fallback_model is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_switch_within_same_provider_preserves_chain():
|
||||||
|
chain = [{"provider": "openrouter", "model": "x-ai/grok-4"}]
|
||||||
|
agent = _make_agent(chain)
|
||||||
|
|
||||||
|
with patch("hermes_cli.timeouts.get_provider_request_timeout", return_value=None):
|
||||||
|
agent.switch_model(
|
||||||
|
new_model="openai/gpt-5",
|
||||||
|
new_provider="openrouter",
|
||||||
|
api_key="or-key",
|
||||||
|
base_url="https://openrouter.ai/api/v1",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert agent._fallback_chain == chain
|
||||||
@@ -1,4 +1,5 @@
|
|||||||
import json
|
import json
|
||||||
|
import os
|
||||||
import sys
|
import sys
|
||||||
import threading
|
import threading
|
||||||
import time
|
import time
|
||||||
@@ -230,6 +231,48 @@ def test_config_set_model_global_persists(monkeypatch):
|
|||||||
assert saved["model"]["base_url"] == "https://api.anthropic.com"
|
assert saved["model"]["base_url"] == "https://api.anthropic.com"
|
||||||
|
|
||||||
|
|
||||||
|
def test_config_set_model_syncs_inference_provider_env(monkeypatch):
|
||||||
|
"""After an explicit provider switch, HERMES_INFERENCE_PROVIDER must
|
||||||
|
reflect the user's choice so ambient re-resolution (credential pool
|
||||||
|
refresh, aux clients) picks up the new provider instead of the original
|
||||||
|
one persisted in config or shell env.
|
||||||
|
|
||||||
|
Regression: a TUI user switched openrouter → anthropic and the TUI kept
|
||||||
|
trying openrouter because the env-var-backed resolvers still saw the old
|
||||||
|
provider.
|
||||||
|
"""
|
||||||
|
class _Agent:
|
||||||
|
provider = "openrouter"
|
||||||
|
model = "old/model"
|
||||||
|
base_url = ""
|
||||||
|
api_key = "sk-or"
|
||||||
|
|
||||||
|
def switch_model(self, **_kwargs):
|
||||||
|
return None
|
||||||
|
|
||||||
|
result = types.SimpleNamespace(
|
||||||
|
success=True,
|
||||||
|
new_model="claude-sonnet-4.6",
|
||||||
|
target_provider="anthropic",
|
||||||
|
api_key="sk-ant",
|
||||||
|
base_url="https://api.anthropic.com",
|
||||||
|
api_mode="anthropic_messages",
|
||||||
|
warning_message="",
|
||||||
|
)
|
||||||
|
|
||||||
|
server._sessions["sid"] = _session(agent=_Agent())
|
||||||
|
monkeypatch.setenv("HERMES_INFERENCE_PROVIDER", "openrouter")
|
||||||
|
monkeypatch.setattr("hermes_cli.model_switch.switch_model", lambda **_kwargs: result)
|
||||||
|
monkeypatch.setattr(server, "_restart_slash_worker", lambda session: None)
|
||||||
|
monkeypatch.setattr(server, "_emit", lambda *args, **kwargs: None)
|
||||||
|
|
||||||
|
server.handle_request(
|
||||||
|
{"id": "1", "method": "config.set", "params": {"session_id": "sid", "key": "model", "value": "claude-sonnet-4.6 --provider anthropic"}}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert os.environ["HERMES_INFERENCE_PROVIDER"] == "anthropic"
|
||||||
|
|
||||||
|
|
||||||
def test_config_set_personality_rejects_unknown_name(monkeypatch):
|
def test_config_set_personality_rejects_unknown_name(monkeypatch):
|
||||||
monkeypatch.setattr(server, "_available_personalities", lambda cfg=None: {"helpful": "You are helpful."})
|
monkeypatch.setattr(server, "_available_personalities", lambda cfg=None: {"helpful": "You are helpful."})
|
||||||
resp = server.handle_request(
|
resp = server.handle_request(
|
||||||
|
|||||||
@@ -538,6 +538,12 @@ def _apply_model_switch(sid: str, session: dict, raw_input: str) -> dict:
|
|||||||
_emit("session.info", sid, _session_info(agent))
|
_emit("session.info", sid, _session_info(agent))
|
||||||
|
|
||||||
os.environ["HERMES_MODEL"] = result.new_model
|
os.environ["HERMES_MODEL"] = result.new_model
|
||||||
|
# Keep the process-level provider env var in sync with the user's explicit
|
||||||
|
# choice so any ambient re-resolution (credential pool refresh, compressor
|
||||||
|
# rebuild, aux clients) resolves to the new provider instead of the
|
||||||
|
# original one persisted in config or env.
|
||||||
|
if result.target_provider:
|
||||||
|
os.environ["HERMES_INFERENCE_PROVIDER"] = result.target_provider
|
||||||
if persist_global:
|
if persist_global:
|
||||||
_persist_model_switch(result)
|
_persist_model_switch(result)
|
||||||
return {"value": result.new_model, "warning": result.warning_message or ""}
|
return {"value": result.new_model, "warning": result.warning_message or ""}
|
||||||
|
|||||||
Reference in New Issue
Block a user