From 54459e76ed93223680a8ef89e3cda72aafc138c8 Mon Sep 17 00:00:00 2001 From: ajzrva-sys Date: Sun, 19 Jul 2026 19:33:49 -0400 Subject: [PATCH] fix: speed up CLI /model picker by skipping non-current custom provider probing (#65652) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: speed up CLI /model picker by skipping non-current custom provider probing The CLI /model picker calls build_models_payload() with default probe_custom_providers=True, which live-fetches /v1/models from every saved custom endpoint on every open. The GUI/desktop picker already passes probe_custom_providers=False for snappiness. Match the GUI behavior: skip probing non-current custom providers, but still probe the current one so its model list stays accurate. Users can force a full re-fetch with /model --refresh. Fixes #65650 Related: #63583 * fix(cli): forward force_refresh to model picker probe flags When /model --refresh is used, the CLI model picker must probe all custom providers to refresh their model lists — not skip them. Normal bare /model still skips non-current probes for speed. Mirrors the existing desktop/TUI behavior. Add regression test for both normal and refresh flag forwarding. Fixes #65650 * fix: auto-save discovered models to config for discover-once caching After a successful /v1/models probe, persist the discovered model list back to config.yaml under the matching custom_providers entry. This makes discover_models: false meaningful out of the box — users get a populated cache after the first probe instead of a stale 1-model list. - Add _save_discovered_models_to_config() helper - Call after successful fetch_api_models in section 4 probe path - Skip config write when model list hasn't changed - Idempotent — no-op on empty api_url or model_ids Tests: 4 new tests covering auto-save, empty-probe skip, unchanged skip, and no-op-on-empty-args. All 4 pass. Refs: #65652, #65650 --------- Co-authored-by: ajzrva-sys <302567740+ajzrva-sys@users.noreply.github.com> --- cli.py | 6 +- hermes_cli/model_switch.py | 52 ++++++ tests/hermes_cli/test_inventory.py | 37 +++++ .../test_model_switch_custom_providers.py | 153 ++++++++++++++++++ 4 files changed, 247 insertions(+), 1 deletion(-) diff --git a/cli.py b/cli.py index f2c9773aef7..c86e7a4a329 100644 --- a/cli.py +++ b/cli.py @@ -8210,7 +8210,11 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): try: if ctx is None: raise RuntimeError("inventory context unavailable") - providers = build_models_payload(ctx)["providers"] + providers = build_models_payload( + ctx, + probe_custom_providers=force_refresh, + probe_current_custom_provider=not force_refresh, + )["providers"] except Exception: providers = [] diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index fcb66917162..8af49151d2f 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -101,6 +101,52 @@ def _declared_model_ids(value: Any) -> list[str]: return ids +def _save_discovered_models_to_config( + api_url: str, model_ids: list[str] +) -> None: + """Persist discovered models into ``custom_providers`` in config.yaml. + + Called after a successful ``/v1/models`` probe so that the next read + with ``discover_models: false`` uses the cached list instead of a stale + or minimal manually-configured subset. + + Matches entries by ``base_url`` (trailing-slash-normalised). A failed + config write is swallowed — the picker still shows the live models for + this session. + """ + if not api_url or not model_ids: + return + try: + from hermes_cli.config import load_config, save_config + + cfg = load_config() + providers = cfg.get("custom_providers") or [] + if not isinstance(providers, list): + return + + norm_url = api_url.strip().rstrip("/").lower() + changed = False + for entry in providers: + if not isinstance(entry, dict): + continue + entry_url = (entry.get("base_url", "") or entry.get("url", "") or "").strip() + if entry_url.rstrip("/").lower() != norm_url: + continue + existing = entry.get("models") + # Only update when models are stale — avoids unnecessary + # config writes on every picker open. + if isinstance(existing, list) and existing == model_ids: + continue + entry["models"] = model_ids + changed = True + + if changed: + cfg["custom_providers"] = providers + save_config(cfg) + except Exception: + pass + + def _bare_custom_provider_def(current_base_url: str) -> Optional[ProviderDef]: """ProviderDef for a direct ``model.provider: custom`` endpoint.""" base_url = str(current_base_url or "").strip() @@ -2447,6 +2493,12 @@ def list_authenticated_providers( if live_models: grp["models"] = live_models grp["total_models"] = len(live_models) + # Auto-save discovered models back to config so + # ``discover_models: false`` has a populated cache + # on the next read. A failed save is non-fatal. + _save_discovered_models_to_config( + api_url, live_models + ) except Exception: pass results.append({ diff --git a/tests/hermes_cli/test_inventory.py b/tests/hermes_cli/test_inventory.py index fd9f25fdefb..6c262180b9d 100644 --- a/tests/hermes_cli/test_inventory.py +++ b/tests/hermes_cli/test_inventory.py @@ -276,6 +276,43 @@ def test_build_models_payload_can_probe_only_current_custom_provider(): assert mock_list.call_args.kwargs["probe_current_custom_provider"] is True +def test_cli_model_picker_forwards_force_refresh_to_probe_flags(): + """CLI /model picker must pass force_refresh to probe flags (#65652, #65650). + + Normal open (/model bare) skips non-current probes; /model --refresh probes + all custom providers to freshen their model lists. + """ + ctx = _empty_ctx() + + # Normal open — skip non-current probes + force_refresh = False + with patch( + "hermes_cli.model_switch.list_authenticated_providers", + return_value=[], + ) as mock_list: + build_models_payload( + ctx, + probe_custom_providers=force_refresh, + probe_current_custom_provider=not force_refresh, + ) + assert mock_list.call_args.kwargs["probe_custom_providers"] is False + assert mock_list.call_args.kwargs["probe_current_custom_provider"] is True + + # Refresh open — probe everything + force_refresh = True + with patch( + "hermes_cli.model_switch.list_authenticated_providers", + return_value=[], + ) as mock_list: + build_models_payload( + ctx, + probe_custom_providers=force_refresh, + probe_current_custom_provider=not force_refresh, + ) + assert mock_list.call_args.kwargs["probe_custom_providers"] is True + assert mock_list.call_args.kwargs["probe_current_custom_provider"] is False + + def test_list_authenticated_providers_force_fresh_is_keyword_only(): """``force_fresh_nous_tier`` must be keyword-only on the public listing API. diff --git a/tests/hermes_cli/test_model_switch_custom_providers.py b/tests/hermes_cli/test_model_switch_custom_providers.py index 06722b4f9f1..abd1089623f 100644 --- a/tests/hermes_cli/test_model_switch_custom_providers.py +++ b/tests/hermes_cli/test_model_switch_custom_providers.py @@ -1326,3 +1326,156 @@ def test_resolve_custom_provider_bare_custom_self_heal_passes_key_env(): assert resolved is not None assert resolved.api_key_env_vars == ("XIAOMI_MIMO_API_KEY",) + + +def test_discovered_models_auto_saved_to_cache(monkeypatch): + """Discovered models are persisted to config so ``discover_models: false`` + has a populated cache on the next read (#65652). + + When a successful probe returns live models, ``_save_discovered_models_to_config`` + must be called with the provider's base_url and the discovered model list. + """ + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {}) + monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {}) + + save_calls = [] + + def fake_fetch_api_models(api_key, base_url, **kwargs): + return ["discovered-a", "discovered-b", "discovered-c"] + + monkeypatch.setattr("hermes_cli.models.fetch_api_models", fake_fetch_api_models) + monkeypatch.setattr( + "hermes_cli.model_switch._save_discovered_models_to_config", + lambda api_url, model_ids: save_calls.append((api_url, model_ids)), + ) + + custom_providers = [ + { + "name": "my-gateway", + "api_key": "***", + "base_url": "https://gateway.example.com/v1", + "discover_models": True, + "model": "only-model", + "models": {"only-model": {"context_length": 128000}}, + } + ] + + providers = list_authenticated_providers( + current_provider="my-gateway", + current_base_url="https://gateway.example.com/v1", + custom_providers=custom_providers, + max_models=50, + probe_custom_providers=True, + ) + + assert len(save_calls) == 1, ( + "_save_discovered_models_to_config must be called after a successful probe" + ) + assert save_calls[0][0] == "https://gateway.example.com/v1" + assert save_calls[0][1] == ["discovered-a", "discovered-b", "discovered-c"] + + gateway_prov = next( + (p for p in providers if p.get("api_url") == "https://gateway.example.com/v1"), + None, + ) + assert gateway_prov is not None + assert gateway_prov["models"] == ["discovered-a", "discovered-b", "discovered-c"] + + +def test_discovered_models_not_saved_on_empty_probe(monkeypatch): + """When a probe returns an empty list, no auto-save must happen (#65652).""" + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {}) + monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {}) + + save_calls = [] + + def fake_fetch_api_models(api_key, base_url, **kwargs): + return [] + + monkeypatch.setattr("hermes_cli.models.fetch_api_models", fake_fetch_api_models) + monkeypatch.setattr( + "hermes_cli.model_switch._save_discovered_models_to_config", + lambda api_url, model_ids: save_calls.append((api_url, model_ids)), + ) + + custom_providers = [ + { + "name": "my-gateway", + "api_key": "***", + "base_url": "https://gateway.example.com/v1", + "discover_models": True, + "model": "only-model", + } + ] + + list_authenticated_providers( + current_provider="my-gateway", + current_base_url="https://gateway.example.com/v1", + custom_providers=custom_providers, + max_models=50, + probe_custom_providers=True, + ) + + assert save_calls == [], "Empty probe must not trigger a save" + + +def test_save_discovered_models_skips_unchanged(monkeypatch): + """``_save_discovered_models_to_config`` must not write config when the + model list hasn't changed (#65652).""" + from hermes_cli.model_switch import _save_discovered_models_to_config + + save_calls = [] + + def fake_save(config): + save_calls.append(dict(config)) + + monkeypatch.setattr("hermes_cli.config.save_config", fake_save) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: { + "custom_providers": [ + { + "name": "my-gateway", + "base_url": "https://gateway.example.com/v1", + "models": ["model-a", "model-b"], + } + ] + }, + ) + + # Same list — no write + _save_discovered_models_to_config( + "https://gateway.example.com/v1", + ["model-a", "model-b"], + ) + assert save_calls == [], "Unchanged models must not trigger config write" + + # Changed list — write + _save_discovered_models_to_config( + "https://gateway.example.com/v1", + ["model-a", "model-b", "model-c"], + ) + assert len(save_calls) == 1, "Changed models must trigger config write" + updated = save_calls[0]["custom_providers"][0] + assert updated["models"] == ["model-a", "model-b", "model-c"] + + +def test_save_discovered_models_noop_on_empty_args(monkeypatch): + """``_save_discovered_models_to_config`` is a no-op when api_url or + model_ids are blank (#65652).""" + from hermes_cli.model_switch import _save_discovered_models_to_config + + load_calls = 0 + + def fake_load(): + nonlocal load_calls + load_calls += 1 + return {"custom_providers": []} + + monkeypatch.setattr("hermes_cli.config.load_config", fake_load) + + _save_discovered_models_to_config("", ["a"]) + _save_discovered_models_to_config("https://x.com", []) + _save_discovered_models_to_config("", []) + + assert load_calls == 0, "load_config must not be called for empty args"