mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix: speed up CLI /model picker by skipping non-current custom provider probing (#65652)
* 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>
This commit is contained in:
parent
b30108143e
commit
54459e76ed
4 changed files with 247 additions and 1 deletions
6
cli.py
6
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 = []
|
||||
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue