fix(cli): /model switch, session restore and /new re-resolve reasoning effort for the new model
The classic CLI resolves `reasoning_config` once at startup for the launch model and passes that field into every lazily built agent. `_stage_and_swap_model` (typed /model and picker), `_restore_session_model` (--resume, /resume) and `new_session` (/new) moved `self.model` without re-running the chokepoint, so the first turn after a switch issued before the first message went out with the OLD model's effort — always-thinking models that accept only their own level set (GLM/ARK: low/high/max) reject that with a non-retryable HTTP 400. `agent.switch_model` already re-resolves its own copy, so only the CLI-level field was stale. - `_resolve_cli_reasoning(cli)` runs `resolve_reasoning_config(CLI_CONFIG, cli.model)` and is called BEFORE the agent branch on all three paths, covering the lazy-build case. - `reasoning_config` joins `_RUNTIME_FIELDS`, so a failed in-place swap and the `/model X --once` restore roll it back with the rest of the route (drops the explicit one-turn restore line the field loop now covers). - `/new` resolves after the config-default model reset, so the default model's per-model override is kept while a `/reasoning` session override is still dropped. - Docs: the override list names the switch-before-first-message, resume and /new paths. Slimmer redo of #96023 (@liuhao1024): same core hunk, without the `--reasoning`-pins- for-the-whole-run flag (the live-agent path already re-resolves on switch, so the CLI field follows the same rule) and with the two entry points #96023 missed (`_restore_session_model`, `_RUNTIME_FIELDS`), which @catecholamin identified on #112921. #112924 (@li-lizhe) covers the same `_stage_and_swap_model` hunk. Fixes #112921 Fixes #96012 Co-authored-by: liuhao1024 <sunsky.lau@gmail.com> Co-authored-by: li-lizhe <li-lizhe@noreply.github.com>
This commit is contained in:
@@ -18,16 +18,31 @@ from rich.markup import escape as _escape
|
||||
from utils import base_url_host_matches
|
||||
|
||||
# CLI-level fields describing the active model route; snapshotted before a switch / one-turn
|
||||
# override and restored wholesale on rollback.
|
||||
# override and restored wholesale on rollback. ``reasoning_config`` rides along because it is
|
||||
# resolved per model: a failed swap or `/model X --reasoning high --once` must not leave the
|
||||
# new model's effort behind with the old route.
|
||||
_RUNTIME_FIELDS = (
|
||||
"model", "provider", "requested_provider", "_explicit_api_key", "_explicit_base_url",
|
||||
"api_key", "base_url", "api_mode")
|
||||
"api_key", "base_url", "api_mode", "reasoning_config")
|
||||
|
||||
|
||||
def _runtime_fields(cli) -> dict:
|
||||
return {key: getattr(cli, key, None) for key in _RUNTIME_FIELDS}
|
||||
|
||||
|
||||
def _resolve_cli_reasoning(cli) -> None:
|
||||
"""Re-resolve the CLI-level ``reasoning_config`` for ``cli.model`` through the shared chokepoint
|
||||
(per-model ``reasoning_overrides`` > global ``agent.reasoning_effort``). Startup resolves it once
|
||||
for the launch model; every path that moves ``cli.model`` must call this BEFORE the agent branch,
|
||||
because a lazily built agent inherits this field — an always-thinking model then goes out with
|
||||
the launch model's effort and 400s (#112921, #96012). ``agent.switch_model`` re-resolves its own
|
||||
copy for the live-agent path."""
|
||||
from cli import CLI_CONFIG
|
||||
from hermes_constants import resolve_reasoning_config
|
||||
# getattr: tests drive /new unbound on a SimpleNamespace without ``model`` (blank -> config default).
|
||||
cli.reasoning_config = resolve_reasoning_config(CLI_CONFIG, getattr(cli, "model", None) or "")
|
||||
|
||||
|
||||
def stored_session_route(session_meta, *, current_model, current_provider):
|
||||
"""The route a resumed session should run on, or ``None`` when the stored one is absent or
|
||||
already current. Returns ``(model, provider, base_url, api_mode, provider_changed)``; the
|
||||
@@ -427,8 +442,9 @@ class CLIModelSwitchMixin:
|
||||
"Credential re-resolution for resumed session provider "
|
||||
"%s failed; keeping ambient credentials",
|
||||
stored_provider, exc_info=True)
|
||||
_resolve_cli_reasoning(self)
|
||||
# Mid-chat /resume swaps the live agent; on startup --resume _init_agent picks up
|
||||
# self.model / self.provider.
|
||||
# self.model / self.provider / self.reasoning_config.
|
||||
if self.agent is not None:
|
||||
try:
|
||||
self.agent.switch_model(
|
||||
@@ -516,9 +532,6 @@ class CLIModelSwitchMixin:
|
||||
for key in _RUNTIME_FIELDS:
|
||||
if key in snapshot:
|
||||
setattr(self, key, snapshot.get(key))
|
||||
# `/model X --reasoning high --once` must not leave the effort behind with the model.
|
||||
if "reasoning_config" in snapshot:
|
||||
self.reasoning_config = snapshot["reasoning_config"]
|
||||
|
||||
agent = getattr(self, "agent", None)
|
||||
if agent is None:
|
||||
@@ -602,6 +615,7 @@ class CLIModelSwitchMixin:
|
||||
self.base_url = result.base_url
|
||||
if result.api_mode:
|
||||
self.api_mode = result.api_mode
|
||||
_resolve_cli_reasoning(self)
|
||||
|
||||
if self.agent is not None:
|
||||
try:
|
||||
|
||||
@@ -489,8 +489,9 @@ class CLISessionMixin:
|
||||
def new_session(self, silent=False, title=None):
|
||||
"""Start a fresh session with a new session ID and cleared agent state."""
|
||||
from cli import (
|
||||
CLI_CONFIG, _parse_reasoning_config, _parse_service_tier_config,
|
||||
CLI_CONFIG, _parse_service_tier_config,
|
||||
_sync_process_session_id, datetime)
|
||||
from hermes_cli.cli_model_switch_mixin import _resolve_cli_reasoning
|
||||
old_session_id = self.session_id
|
||||
_boundary_snapshot = None
|
||||
if self.agent:
|
||||
@@ -527,14 +528,15 @@ class CLISessionMixin:
|
||||
self._resumed = False
|
||||
# An explicit -m/--model was for the previous session only.
|
||||
self._explicit_model_override = False
|
||||
self.reasoning_config = _parse_reasoning_config(
|
||||
CLI_CONFIG["agent"].get("reasoning_effort", ""))
|
||||
# Session-scoped overrides (/model --session, /fast, one-turn restores) don't carry over.
|
||||
# Re-derive model/provider and service tier from config.yaml so a session-only switch never leaks
|
||||
# into the next session (#48055, #23131).
|
||||
self._pending_one_turn_model_restore = None
|
||||
self.service_tier = _parse_service_tier_config(CLI_CONFIG["agent"].get("service_tier", ""))
|
||||
_reset_model_to_config_default(self, silent)
|
||||
# After the model reset: the effort belongs to the model the fresh session lands on (a /reasoning
|
||||
# session override is dropped, the default model's per-model override is kept).
|
||||
_resolve_cli_reasoning(self)
|
||||
_sync_process_session_id(self.session_id)
|
||||
|
||||
if self.agent:
|
||||
|
||||
78
tests/hermes_cli/test_model_switch_reasoning_reresolve.py
Normal file
78
tests/hermes_cli/test_model_switch_reasoning_reresolve.py
Normal file
@@ -0,0 +1,78 @@
|
||||
"""The CLI-level ``reasoning_config`` follows the model on every path that moves ``cli.model``.
|
||||
|
||||
Startup resolves the effort once for the launch model; a lazily built agent (first message after
|
||||
``/model``, ``--resume``, ``/new``) inherits ``cli.reasoning_config`` verbatim. Leaving it stale sends
|
||||
the launch model's effort to the new model — always-thinking models that accept only their own level
|
||||
set reject that with a non-retryable HTTP 400 (#112921, #96012).
|
||||
"""
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
from hermes_cli.cli_model_switch_mixin import CLIModelSwitchMixin
|
||||
|
||||
_CFG = {
|
||||
"agent": {"reasoning_effort": "medium", "reasoning_overrides": {"glm-5.3-flash": "high"}},
|
||||
"model": {"provider": "custom:ark", "default": "deepseek-v4-flash"},
|
||||
}
|
||||
_MEDIUM = {"enabled": True, "effort": "medium"}
|
||||
_HIGH = {"enabled": True, "effort": "high"}
|
||||
|
||||
|
||||
def _cli(model, reasoning_config, agent=None):
|
||||
return SimpleNamespace(
|
||||
model=model, provider="custom:ark", requested_provider="custom:ark", api_key="k",
|
||||
base_url="https://ark.example/v3", api_mode="chat_completions", _explicit_api_key=None,
|
||||
_explicit_base_url=None, agent=agent, _explicit_model_override=False, _credential_pool=None,
|
||||
reasoning_config=reasoning_config, _console_print=lambda *_a, **_k: None)
|
||||
|
||||
|
||||
def _result(model):
|
||||
return SimpleNamespace(new_model=model, target_provider="custom:ark", api_key="k",
|
||||
base_url="https://ark.example/v3", api_mode="chat_completions", success=True)
|
||||
|
||||
|
||||
def test_switch_and_session_restore_re_resolve_effort_before_the_agent_exists():
|
||||
import cli as cli_mod
|
||||
|
||||
with patch.dict(cli_mod.CLI_CONFIG, _CFG):
|
||||
# /model glm-5.3-flash before the first message: per-model override wins.
|
||||
cli = _cli("deepseek-v4-flash", _MEDIUM)
|
||||
assert CLIModelSwitchMixin._stage_and_swap_model(cli, _result("glm-5.3-flash"), "deepseek-v4-flash")
|
||||
assert cli.reasoning_config == _HIGH
|
||||
|
||||
# Control: switching back to a model without an override lands on the global effort.
|
||||
assert CLIModelSwitchMixin._stage_and_swap_model(cli, _result("deepseek-v4-flash"), "glm-5.3-flash")
|
||||
assert cli.reasoning_config == _MEDIUM
|
||||
|
||||
# --resume of a session stored on the override model, agent not built yet.
|
||||
cli = _cli("deepseek-v4-flash", _MEDIUM)
|
||||
CLIModelSwitchMixin._restore_session_model(
|
||||
cli, {"model": "glm-5.3-flash", "model_config": {"gateway_runtime": {
|
||||
"provider": "custom:ark", "base_url": "https://ark.example/v3", "api_mode": "chat_completions"}}},
|
||||
quiet=True)
|
||||
assert cli.model == "glm-5.3-flash"
|
||||
assert cli.reasoning_config == _HIGH
|
||||
|
||||
|
||||
def test_failed_swap_and_new_session_keep_the_effort_with_the_route():
|
||||
import cli as cli_mod
|
||||
from hermes_cli.cli_session_mixin import CLISessionMixin
|
||||
|
||||
def _boom(**_kw):
|
||||
raise RuntimeError("boom")
|
||||
|
||||
with patch.dict(cli_mod.CLI_CONFIG, _CFG):
|
||||
# A failed in-place swap rolls reasoning_config back with the rest of the CLI route.
|
||||
cli = _cli("deepseek-v4-flash", _MEDIUM, agent=SimpleNamespace(switch_model=_boom))
|
||||
assert not CLIModelSwitchMixin._stage_and_swap_model(cli, _result("glm-5.3-flash"), "deepseek-v4-flash")
|
||||
assert (cli.model, cli.reasoning_config) == ("deepseek-v4-flash", _MEDIUM)
|
||||
|
||||
# /new on a session whose config default carries an override: the default model's effort,
|
||||
# not the bare global key, and a /reasoning session override does not carry over.
|
||||
with patch.dict(cli_mod.CLI_CONFIG["model"], {"default": "glm-5.3-flash"}):
|
||||
cli = _cli("glm-5.3-flash", {"enabled": False})
|
||||
cli.conversation_history, cli.session_id, cli._session_db, cli._pending_title = [], "old", None, None
|
||||
cli._resumed, cli._notify_session_boundary = False, lambda *_a, **_k: None
|
||||
CLISessionMixin.new_session(cli, silent=True)
|
||||
assert cli.reasoning_config == _HIGH
|
||||
@@ -1808,7 +1808,7 @@ There is no `hermes config set` support for `reasoning_overrides` keys — edit
|
||||
3. Global `agent.reasoning_effort`
|
||||
4. Provider default
|
||||
|
||||
The override applies automatically everywhere: CLI startup, messaging gateway, Desktop/TUI, cron jobs, `/model` mid-session switches, and fallback model activation.
|
||||
The override applies automatically everywhere: CLI startup, messaging gateway, Desktop/TUI, cron jobs, `/model` mid-session switches (including a switch issued before the first message), session resume (`--resume`, `/resume`), `/new`, and fallback model activation.
|
||||
|
||||
## Fast Mode
|
||||
|
||||
|
||||
Reference in New Issue
Block a user