From 71fe5fccad0b104f91c529cb2f2254dfad0cfbd4 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:55:44 +0530 Subject: [PATCH] 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. --- hermes_cli/model_switch.py | 6 -- .../test_kimi_cn_provider_listing.py | 1 - .../test_model_switch_context_offload.py | 63 ------------------- .../test_model_switch_once_flags.py | 14 ----- tests/hermes_cli/test_model_switch_parsing.py | 1 + .../test_model_switch_persist_default.py | 24 +------ ...est_custom_provider_session_persistence.py | 1 - tests/tui_gateway/test_make_agent_provider.py | 2 - 8 files changed, 2 insertions(+), 110 deletions(-) delete mode 100644 tests/hermes_cli/test_model_switch_context_offload.py delete mode 100644 tests/hermes_cli/test_model_switch_once_flags.py diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index d0c0e3b5a4..3274c985d3 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -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``. diff --git a/tests/hermes_cli/test_kimi_cn_provider_listing.py b/tests/hermes_cli/test_kimi_cn_provider_listing.py index 79d1995012..d02eb54993 100644 --- a/tests/hermes_cli/test_kimi_cn_provider_listing.py +++ b/tests/hermes_cli/test_kimi_cn_provider_listing.py @@ -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 diff --git a/tests/hermes_cli/test_model_switch_context_offload.py b/tests/hermes_cli/test_model_switch_context_offload.py deleted file mode 100644 index fd63bd181a..0000000000 --- a/tests/hermes_cli/test_model_switch_context_offload.py +++ /dev/null @@ -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 diff --git a/tests/hermes_cli/test_model_switch_once_flags.py b/tests/hermes_cli/test_model_switch_once_flags.py deleted file mode 100644 index fd461e2658..0000000000 --- a/tests/hermes_cli/test_model_switch_once_flags.py +++ /dev/null @@ -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 - - diff --git a/tests/hermes_cli/test_model_switch_parsing.py b/tests/hermes_cli/test_model_switch_parsing.py index 4310ec62cf..2b398ef651 100644 --- a/tests/hermes_cli/test_model_switch_parsing.py +++ b/tests/hermes_cli/test_model_switch_parsing.py @@ -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 diff --git a/tests/hermes_cli/test_model_switch_persist_default.py b/tests/hermes_cli/test_model_switch_persist_default.py index b53c8913e1..ce2c78bc42 100644 --- a/tests/hermes_cli/test_model_switch_persist_default.py +++ b/tests/hermes_cli/test_model_switch_persist_default.py @@ -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 # --------------------------------------------------------------------------- diff --git a/tests/tui_gateway/test_custom_provider_session_persistence.py b/tests/tui_gateway/test_custom_provider_session_persistence.py index f49c79b0c0..349c528d4e 100644 --- a/tests/tui_gateway/test_custom_provider_session_persistence.py +++ b/tests/tui_gateway/test_custom_provider_session_persistence.py @@ -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), diff --git a/tests/tui_gateway/test_make_agent_provider.py b/tests/tui_gateway/test_make_agent_provider.py index 37a5defc0c..1ca1df5c46 100644 --- a/tests/tui_gateway/test_make_agent_provider.py +++ b/tests/tui_gateway/test_make_agent_provider.py @@ -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()),