mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
fix(model): track explicit models: declarations in section 3 so a singular default_model doesn't suppress live discovery
A providers: entry with only a default_model/model (no explicit models: list) is un-narrowed — the singular field is just the active selection. Section 3 derived has_explicit_models from the merged models list, so the lone default_model entry counted as an explicit catalog and suppressed the /v1/models probe for no-key endpoints, leaving a one-line /model picker menu for local llama.cpp/Ollama/vLLM servers. Track explicit models: declarations separately at group-build time (mirrors section 4's declaration-tracking from #40542 / PR #61928) and gate the probe on that instead. Salvaged from PR #68984 by @vigilancetech-com (the probe_custom_providers gate removal in that PR is not taken — the GUI no-probe gate is intentional).
This commit is contained in:
parent
0f554ce19e
commit
a75ec9278c
3 changed files with 73 additions and 4 deletions
1
contributors/emails/kevinbanjo@gmail.com
Normal file
1
contributors/emails/kevinbanjo@gmail.com
Normal file
|
|
@ -0,0 +1 @@
|
|||
vigilancetech-com
|
||||
|
|
@ -2370,7 +2370,8 @@ def list_authenticated_providers(
|
|||
entry_models: list = []
|
||||
if default_model:
|
||||
entry_models.append(default_model)
|
||||
for model_id in _declared_model_ids(ep_cfg.get("models", [])):
|
||||
entry_declared_models = _declared_model_ids(ep_cfg.get("models", []))
|
||||
for model_id in entry_declared_models:
|
||||
if model_id not in entry_models:
|
||||
entry_models.append(model_id)
|
||||
|
||||
|
|
@ -2404,6 +2405,7 @@ def list_authenticated_providers(
|
|||
"name": grp_display or display_name,
|
||||
"api_url": api_url,
|
||||
"models": [],
|
||||
"has_explicit_models": False,
|
||||
"ep_cfg": ep_cfg, # used below for discover_models / api_key
|
||||
"raw_names": [],
|
||||
}
|
||||
|
|
@ -2411,6 +2413,13 @@ def list_authenticated_providers(
|
|||
for _m in entry_models:
|
||||
if _m and _m not in ep_groups[group_key]["models"]:
|
||||
ep_groups[group_key]["models"].append(_m)
|
||||
# Track explicit ``models:`` declarations separately from the
|
||||
# merged list: a singular ``default_model``/``model`` is only the
|
||||
# active selection and must not be mistaken for the user narrowing
|
||||
# the endpoint to a curated subset (mirrors section 4's
|
||||
# declaration-tracking; see #40542 / PR #61928).
|
||||
if entry_declared_models:
|
||||
ep_groups[group_key]["has_explicit_models"] = True
|
||||
ep_groups[group_key]["raw_names"].append(display_name)
|
||||
|
||||
for grp in ep_groups.values():
|
||||
|
|
@ -2433,8 +2442,11 @@ def list_authenticated_providers(
|
|||
# unless the provider explicitly opts out via discover_models: false.
|
||||
# Policy mirrors Section 4's should_probe logic:
|
||||
# - With an api_key: always probe (user opted into the endpoint).
|
||||
# - Without an api_key but with explicit models: skip — the user
|
||||
# is narrowing a public endpoint to a specific subset.
|
||||
# - Without an api_key but with an explicit ``models:`` list:
|
||||
# skip — the user is narrowing a public endpoint to a specific
|
||||
# subset. A singular ``default_model``/``model`` does NOT count
|
||||
# as narrowing (it's just the active selection) and must not
|
||||
# suppress discovery — mirrors section 4 / #40542.
|
||||
# - Without an api_key AND no explicit models: probe anyway so
|
||||
# bare-endpoint providers (local llama.cpp / Ollama servers)
|
||||
# still show their full model catalog.
|
||||
|
|
@ -2445,7 +2457,7 @@ def list_authenticated_providers(
|
|||
discover = ep_cfg.get("discover_models", True)
|
||||
if isinstance(discover, str):
|
||||
discover = discover.lower() not in {"false", "no", "0"}
|
||||
has_explicit_models = bool(models_list)
|
||||
has_explicit_models = bool(grp.get("has_explicit_models"))
|
||||
_ep_url_norm = str(api_url).strip().rstrip("/").lower()
|
||||
_ep_slug_norm = str(ep_name).strip().lower()
|
||||
_ep_custom_slug_norm = custom_provider_slug(display_name).lower()
|
||||
|
|
|
|||
|
|
@ -390,6 +390,9 @@ def test_list_authenticated_providers_user_openai_official_url_fallback(monkeypa
|
|||
"""User providers: api.openai.com with no models list uses native curated fallback."""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
# No models: list → un-narrowed → section 3 probes; simulate the keyless
|
||||
# probe failing (401) so the curated fallback is exercised hermetically.
|
||||
monkeypatch.setattr("hermes_cli.models.fetch_api_models", lambda *a, **k: None)
|
||||
|
||||
user_providers = {
|
||||
"openai-direct": {
|
||||
|
|
@ -413,6 +416,9 @@ def test_list_authenticated_providers_fallback_to_default_only(monkeypatch):
|
|||
"""When no models array is provided, should fall back to default_model."""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
# default_model-only entries are un-narrowed, so section 3 probes the
|
||||
# live endpoint; simulate it being unreachable to test the fallback.
|
||||
monkeypatch.setattr("hermes_cli.models.fetch_api_models", lambda *a, **k: None)
|
||||
|
||||
user_providers = {
|
||||
"simple-provider": {
|
||||
|
|
@ -554,6 +560,9 @@ def test_list_authenticated_providers_no_duplicate_labels_across_schemas(monkeyp
|
|||
"""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
# Singular ``model:``-only entries are un-narrowed → section 3 now probes
|
||||
# them; stub the probe so the test stays hermetic (endpoints are fake).
|
||||
monkeypatch.setattr("hermes_cli.models.fetch_api_models", lambda *a, **k: None)
|
||||
|
||||
shared_entries = [
|
||||
("endpoint-a", "http://a.local/v1"),
|
||||
|
|
@ -1173,6 +1182,53 @@ def test_section3_probes_no_key_endpoint_without_explicit_models(monkeypatch):
|
|||
assert row["total_models"] == 3
|
||||
|
||||
|
||||
def test_section3_probes_no_key_endpoint_with_singular_default_model(monkeypatch):
|
||||
"""A providers: entry with no api_key and only a singular ``default_model``
|
||||
(no explicit ``models:`` list) must still probe /v1/models — the singular
|
||||
field is just the active selection, not the user narrowing the endpoint.
|
||||
|
||||
Regression for #40554 / PR #68984 (@vigilancetech-com): section 3 derived
|
||||
``has_explicit_models`` from the merged models list, so the lone
|
||||
``default_model`` entry suppressed live discovery and the /model picker
|
||||
showed a one-line menu for local no-auth endpoints.
|
||||
"""
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||
|
||||
probed = {}
|
||||
|
||||
def _fake_fetch(api_key, api_url, **kwargs):
|
||||
probed["called"] = True
|
||||
probed["api_key"] = api_key
|
||||
return ["live-model-1", "live-model-2", "live-model-3"]
|
||||
|
||||
monkeypatch.setattr("hermes_cli.models.fetch_api_models", _fake_fetch)
|
||||
|
||||
user_providers = {
|
||||
"local-ollama": {
|
||||
"name": "Local Ollama",
|
||||
"api": "http://localhost:11434/v1",
|
||||
"default_model": "llama3",
|
||||
# No api_key, no models: list — singular default only.
|
||||
}
|
||||
}
|
||||
|
||||
providers = list_authenticated_providers(
|
||||
current_provider="local-ollama",
|
||||
user_providers=user_providers,
|
||||
custom_providers=[],
|
||||
max_models=50,
|
||||
)
|
||||
|
||||
assert probed.get("called") is True, (
|
||||
"singular default_model must not suppress live discovery"
|
||||
)
|
||||
assert probed["api_key"] == ""
|
||||
row = next(p for p in providers if p["slug"] == "local-ollama")
|
||||
assert row["models"] == ["live-model-1", "live-model-2", "live-model-3"]
|
||||
assert row["total_models"] == 3
|
||||
|
||||
|
||||
def test_section3_skips_probe_when_no_key_but_explicit_models(monkeypatch):
|
||||
"""A no-key endpoint WITH an explicit models: list is the user narrowing a
|
||||
public endpoint to a subset — skip live discovery and keep the list."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue