Merge pull request #40080 from kshitijk4poor/salvage/discover-models-section4-29810

feat(model_switch): honor discover_models in custom_providers section 4 (salvage #29810)
This commit is contained in:
kshitij 2026-06-05 13:05:34 -07:00 committed by GitHub
commit b5d42daa53
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 130 additions and 3 deletions

View file

@ -1790,6 +1790,13 @@ def list_authenticated_providers(
else (f"env:{key_env}" if key_env else "")
)
# Read discover_models from the entry (same semantics as
# section 3: true by default, set false to keep the explicit
# ``models:`` list instead of replacing it with live /models).
discover = entry.get("discover_models", True)
if isinstance(discover, str):
discover = discover.lower() not in {"false", "no", "0"}
group_key = (api_url, credential_identity, api_mode)
if group_key not in groups:
# Strip per-model suffix so "Ollama — GLM 5.1" becomes
@ -1810,9 +1817,15 @@ def list_authenticated_providers(
"api_url": api_url,
"api_key": api_key,
"models": [],
"discover_models": discover,
}
elif api_key and not groups[group_key].get("api_key"):
groups[group_key]["api_key"] = api_key
else:
if api_key and not groups[group_key].get("api_key"):
groups[group_key]["api_key"] = api_key
# If any entry in this group opts out of discovery,
# honour that for the whole grouped row.
if not discover:
groups[group_key]["discover_models"] = False
# The singular ``model:`` field only holds the currently
# active model. Hermes's own writer (main.py::_save_custom_provider)
@ -1901,7 +1914,16 @@ def list_authenticated_providers(
# - Without an api_key AND no explicit models, fall through to
# live discovery so bare-endpoint custom providers (local
# llama.cpp / Ollama servers) still appear populated.
should_probe = bool(api_url) and (bool(api_key) or not grp["models"])
# - When discover_models: false is set, skip live discovery and
# keep the explicit ``models:`` list regardless of whether an
# api_key is present. This supports endpoints that expose a
# full aggregator catalog via /models but only serve a subset
# (parity with section 3's user ``providers:`` behaviour).
should_probe = (
bool(api_url)
and (bool(api_key) or not grp["models"])
and grp.get("discover_models", True)
)
if should_probe:
try:
from hermes_cli.models import fetch_api_models

View file

@ -1448,6 +1448,7 @@ AUTHOR_MAP = {
"nicsequenzy@gmail.com": "polnikale", # PR #35717 (discover Playwright headless_shell browser)
"wasdhkzk@gmail.com": "whyhkzk", # PR #32407 (sandbox-mirror inner-container guard; commits authored as whyhkzk + zhukun)
"leonard@sellem.me": "leonardsellem", # PR #37405 (desktop WS origin guard on remote/Tailscale binds)
"42903577+ohMyJason@users.noreply.github.com": "ohMyJason", # PR #29810 (discover_models in custom_providers section 4)
}

View file

@ -606,3 +606,107 @@ def test_custom_providers_uses_live_models_for_multi_model_endpoint(monkeypatch)
"gateway-model-c",
], "Live models must replace the static subset"
assert gateway_prov["total_models"] == 3
def test_custom_providers_discover_models_false_keeps_explicit_subset(monkeypatch):
"""Custom providers (section 4) with ``discover_models: false`` must keep
their explicit ``models:`` subset instead of replacing it with live
/models, even when an api_key is present.
This mirrors section 3 (user ``providers:``) behaviour and supports
endpoints that expose a full aggregator catalog via /models but only
serve a configured subset.
"""
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
calls = []
def fake_fetch_api_models(api_key, base_url):
calls.append((api_key, base_url))
return ["gateway-model-a", "gateway-model-b", "gateway-model-c"]
monkeypatch.setattr("hermes_cli.models.fetch_api_models", fake_fetch_api_models)
custom_providers = [
{
"name": "my-gateway",
"api_key": "***",
"base_url": "https://gateway.example.com/v1",
"discover_models": False,
"model": "gateway-model-a",
"models": {
"gateway-model-a": {"context_length": 128000},
"gateway-model-b": {"context_length": 128000},
},
}
]
providers = list_authenticated_providers(
current_provider="openrouter",
current_base_url="https://openrouter.ai/api/v1",
custom_providers=custom_providers,
max_models=50,
)
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, "Custom provider group not found in results"
assert calls == [], (
"fetch_api_models must NOT be called when discover_models is false"
)
assert gateway_prov["models"] == [
"gateway-model-a",
"gateway-model-b",
], "Explicit models: subset must be preserved when discovery is disabled"
assert gateway_prov["total_models"] == 2
def test_custom_providers_discover_models_false_string_is_normalised(monkeypatch):
"""String ``discover_models: "false"`` (hand-edited / env-style configs)
must be treated as a disable, same as the boolean ``False`` and section 3.
"""
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
calls = []
def fake_fetch_api_models(api_key, base_url):
calls.append((api_key, base_url))
return ["live-a", "live-b"]
monkeypatch.setattr("hermes_cli.models.fetch_api_models", fake_fetch_api_models)
custom_providers = [
{
"name": "my-gateway",
"api_key": "***",
"base_url": "https://gateway.example.com/v1",
"discover_models": "false",
"model": "only-model",
"models": {"only-model": {"context_length": 128000}},
}
]
providers = list_authenticated_providers(
current_provider="openrouter",
current_base_url="https://openrouter.ai/api/v1",
custom_providers=custom_providers,
max_models=50,
)
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 calls == [], "string 'false' must disable live discovery"
assert gateway_prov["models"] == ["only-model"]