refactor(model_switch): remove parse_model_flags, which only tests called
Every /model surface parses through parse_model_switch_args. The legacy 5-tuple wrapper had no production caller; its remaining references were two tests of the tuple shape, an unused import, and two tui_gateway patch() calls on a function tui_gateway no longer calls. The en-dash --session normalization those tests covered moves into the parse_model_switch_args keeper. test_model_switch_once_flags and test_model_switch_context_offload are removed: the first re-tests the private parser the keeper already drives, the second the one-line to_thread wrapper that tests/gateway/test_model_command_context_offload.py proves off the event loop through the real /model handler.
This commit is contained in:
@@ -507,12 +507,6 @@ def parse_model_flags_detailed(raw_args: str) -> ModelFlagParseResult:
|
||||
return ModelFlagParseResult(model_input=" ".join(filtered).strip(), **values, **flags)
|
||||
|
||||
|
||||
def parse_model_flags(raw_args: str) -> tuple[str, str, bool, bool, bool]:
|
||||
"""Legacy 5-tuple ``(model_input, explicit_provider, is_global, force_refresh, is_session)``."""
|
||||
p = parse_model_flags_detailed(raw_args)
|
||||
return (p.model_input, p.explicit_provider, p.is_global, p.force_refresh, p.is_session)
|
||||
|
||||
|
||||
def resolve_persist_behavior(
|
||||
is_global: bool, is_session: bool, is_once: bool = False, explicit_provider: str = "") -> bool:
|
||||
"""Decide whether a ``/model`` switch should persist to ``config.yaml``.
|
||||
|
||||
@@ -16,7 +16,6 @@ from unittest.mock import patch
|
||||
|
||||
from hermes_cli.model_switch import (
|
||||
list_authenticated_providers,
|
||||
parse_model_flags,
|
||||
switch_model,
|
||||
)
|
||||
from hermes_cli.providers import resolve_provider_full
|
||||
|
||||
@@ -1,63 +0,0 @@
|
||||
"""``/model`` context-length resolution must not run on the gateway event loop.
|
||||
|
||||
``resolve_display_context_length`` runs two blocking chains — the route
|
||||
comparison in ``should_clear_context_pin`` and the provider probe ladder in
|
||||
``get_model_context_length`` (blocking ``requests`` calls to Anthropic
|
||||
``/v1/models``, Copilot, Nous, Codex, GMI, Ollama, models.dev and OpenRouter).
|
||||
|
||||
The gateway message path already offloads both (``get_model_context_length_async``,
|
||||
``should_clear_context_pin_async``); the ``/model`` slash-command handlers called
|
||||
the sync helper directly, freezing the loop for every user on every platform for
|
||||
the duration of the probe ladder.
|
||||
"""
|
||||
|
||||
import threading
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
import agent.model_metadata as model_meta_mod
|
||||
from hermes_cli import model_switch
|
||||
|
||||
PROBE_SECONDS = 0.05
|
||||
|
||||
RESOLVE_ARGS = dict(
|
||||
model="claude-opus-4",
|
||||
provider="anthropic",
|
||||
base_url="",
|
||||
api_key="",
|
||||
custom_providers=None,
|
||||
config_context_length=None,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def slow_probe(monkeypatch):
|
||||
"""Stand in for one blocking provider probe inside the resolution chain."""
|
||||
calls = {}
|
||||
|
||||
def _probe(model, **kwargs):
|
||||
calls["thread"] = threading.current_thread()
|
||||
time.sleep(PROBE_SECONDS)
|
||||
return 128000
|
||||
|
||||
monkeypatch.setattr(model_meta_mod, "get_model_context_length", _probe)
|
||||
return calls
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_variant_matches_sync(slow_probe):
|
||||
"""The async wrapper resolves the same value as the sync helper."""
|
||||
sync_value = model_switch.resolve_display_context_length(**RESOLVE_ARGS)
|
||||
async_value = await model_switch.resolve_display_context_length_async(
|
||||
**RESOLVE_ARGS
|
||||
)
|
||||
assert async_value == sync_value == 128000
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolution_runs_off_the_event_loop_thread(slow_probe):
|
||||
"""The blocking chain must execute on a worker thread, not the loop thread."""
|
||||
loop_thread = threading.current_thread()
|
||||
await model_switch.resolve_display_context_length_async(**RESOLVE_ARGS)
|
||||
assert slow_probe["thread"] is not loop_thread
|
||||
@@ -1,14 +0,0 @@
|
||||
from hermes_cli.model_switch import parse_model_flags, parse_model_flags_detailed
|
||||
|
||||
|
||||
def test_parse_model_flags_detailed_supports_once():
|
||||
parsed = parse_model_flags_detailed("sonnet --provider anthropic --once")
|
||||
|
||||
assert parsed.model_input == "sonnet"
|
||||
assert parsed.explicit_provider == "anthropic"
|
||||
assert parsed.is_global is False
|
||||
assert parsed.force_refresh is False
|
||||
assert parsed.is_session is False
|
||||
assert parsed.is_once is True
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ def test_provider_flag_and_scopes():
|
||||
assert req.errors == ()
|
||||
|
||||
assert parse_model_switch_args("sonnet --session").scope == "session"
|
||||
assert parse_model_switch_args("sonnet \u2013session").scope == "session" # iOS/Telegram en-dash
|
||||
assert parse_model_switch_args("sonnet --once").scope == "once"
|
||||
assert parse_model_switch_args("--refresh").force_refresh is True
|
||||
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
"""Tests for session-scoped-by-default model switching.
|
||||
|
||||
Covers:
|
||||
- ``parse_model_flags`` recognises ``--session`` (and keeps ``--global``).
|
||||
- ``resolve_persist_behavior`` applies the config-gated default and the
|
||||
``--session`` / ``--global`` overrides.
|
||||
- The default (no flags) is session-only, which is the user-facing fix: a
|
||||
@@ -11,28 +10,7 @@ Covers:
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from hermes_cli.model_switch import parse_model_flags, resolve_persist_behavior
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# parse_model_flags
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestParseModelFlagsSession:
|
||||
def test_no_flags(self):
|
||||
assert parse_model_flags("sonnet") == ("sonnet", "", False, False, False)
|
||||
|
||||
|
||||
def test_unicode_dash_session_normalized(self):
|
||||
# Telegram/iOS auto-converts -- to en/em dashes.
|
||||
assert parse_model_flags("sonnet \u2013session") == (
|
||||
"sonnet",
|
||||
"",
|
||||
False,
|
||||
False,
|
||||
True,
|
||||
)
|
||||
from hermes_cli.model_switch import resolve_persist_behavior
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -635,7 +635,6 @@ class TestFollowProfileConfigRuntimeOverrides:
|
||||
known = set(server._sessions)
|
||||
try:
|
||||
with (
|
||||
patch("hermes_cli.model_switch.parse_model_flags", return_value=("glm-5.1", None, False, False, None)),
|
||||
patch("hermes_cli.model_switch.resolve_persist_behavior", return_value=False),
|
||||
patch("hermes_cli.model_switch.switch_model", return_value=result),
|
||||
server._profile_build_scope(secondary),
|
||||
|
||||
@@ -68,8 +68,6 @@ def test_apply_model_switch_does_not_leak_process_env():
|
||||
|
||||
persisted_composer_profiles = []
|
||||
with (
|
||||
patch("hermes_cli.model_switch.parse_model_flags",
|
||||
return_value=("glm-5.1", None, False, False, True)),
|
||||
patch("hermes_cli.model_switch.resolve_persist_behavior",
|
||||
return_value=False),
|
||||
patch("hermes_cli.model_switch.switch_model", return_value=_FakeResult()),
|
||||
|
||||
Reference in New Issue
Block a user