mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-17 09:41:58 +00:00
fix(inventory): deduplicate models between user-defined and aggregator providers
When a user-defined provider (e.g. litellm-proxy) and an aggregator (e.g. openrouter) both advertise the same model name, the Desktop/TUI model picker would show the model under both groups. Selecting it from the aggregator row silently set model.provider to the aggregator, breaking calls because the aggregator doesn't actually serve that model ID. Fix: after list_authenticated_providers() returns, collect all models from user-defined provider rows and filter them out of aggregator rows. Uses is_aggregator() from hermes_cli/providers.py to identify aggregators. Case-insensitive matching. Fixes #45954
This commit is contained in:
parent
9df1a1a8de
commit
60cc42e38b
2 changed files with 154 additions and 0 deletions
|
|
@ -157,6 +157,36 @@ def build_models_payload(
|
|||
max_models=max_models,
|
||||
)
|
||||
|
||||
# --- Deduplicate: remove models from aggregators that overlap with
|
||||
# user-defined providers. When a local proxy (e.g. litellm-proxy)
|
||||
# serves a model whose name also appears in an aggregator's curated
|
||||
# catalog, the picker would show the model under both providers.
|
||||
# Selecting it from the aggregator row sets model.provider to the
|
||||
# aggregator (e.g. openrouter) instead of the user's proxy — silently
|
||||
# breaking the call. Filtering at the payload level keeps the
|
||||
# aggregator rows honest: they only show models the user can't get
|
||||
# from a more-specific provider. (#45954)
|
||||
try:
|
||||
from hermes_cli.providers import is_aggregator as _is_aggregator
|
||||
except Exception:
|
||||
_is_aggregator = None # type: ignore[assignment]
|
||||
|
||||
if _is_aggregator is not None:
|
||||
user_models: set[str] = set()
|
||||
for row in rows:
|
||||
if row.get("is_user_defined"):
|
||||
user_models.update(m.lower() for m in (row.get("models") or []))
|
||||
if user_models:
|
||||
for row in rows:
|
||||
slug = row.get("slug", "")
|
||||
if not _is_aggregator(slug):
|
||||
continue
|
||||
original = row.get("models") or []
|
||||
filtered = [m for m in original if m.lower() not in user_models]
|
||||
if len(filtered) < len(original):
|
||||
row["models"] = filtered
|
||||
row["total_models"] = len(filtered)
|
||||
|
||||
if include_unconfigured:
|
||||
rows = list(rows) + _append_unconfigured_rows(rows, ctx)
|
||||
if picker_hints:
|
||||
|
|
|
|||
|
|
@ -482,3 +482,127 @@ def test_payload_shape_compatible_with_modelpickerdialog_frontend():
|
|||
for row in payload["providers"]:
|
||||
missing = required_keys - row.keys()
|
||||
assert not missing, f"row {row['slug']} missing keys: {missing}"
|
||||
|
||||
|
||||
# ─── Aggregator dedup (issue #45954) ───────────────────────────────────
|
||||
|
||||
|
||||
def _user_provider_row(slug: str, models: list[str]) -> dict:
|
||||
return {
|
||||
"slug": slug,
|
||||
"name": slug.title(),
|
||||
"models": models,
|
||||
"total_models": len(models),
|
||||
"is_current": False,
|
||||
"is_user_defined": True,
|
||||
"source": "user-config",
|
||||
}
|
||||
|
||||
|
||||
def _aggregator_row(slug: str, models: list[str]) -> dict:
|
||||
return {
|
||||
"slug": slug,
|
||||
"name": slug.title(),
|
||||
"models": models,
|
||||
"total_models": len(models),
|
||||
"is_current": False,
|
||||
"is_user_defined": False,
|
||||
"source": "built-in",
|
||||
}
|
||||
|
||||
|
||||
def test_aggregator_dedup_removes_overlapping_models():
|
||||
"""Models served by a user-defined provider are removed from
|
||||
aggregator rows so the picker doesn't show them under the wrong
|
||||
provider. (#45954)"""
|
||||
rows = [
|
||||
_user_provider_row("litellm-proxy", [
|
||||
"nvidia/nim/minimax-m3",
|
||||
"nvidia/nim/kimi-k2.6",
|
||||
]),
|
||||
_aggregator_row("openrouter", [
|
||||
"minimax/minimax-m3",
|
||||
"nvidia/nim/minimax-m3", # overlaps with litellm-proxy
|
||||
"anthropic/claude-sonnet-4.6",
|
||||
]),
|
||||
]
|
||||
ctx = _empty_ctx()
|
||||
with _list_auth_returning(rows):
|
||||
payload = build_models_payload(ctx)
|
||||
|
||||
or_row = next(r for r in payload["providers"] if r["slug"] == "openrouter")
|
||||
proxy_row = next(r for r in payload["providers"] if r["slug"] == "litellm-proxy")
|
||||
|
||||
# User-defined provider keeps all its models
|
||||
assert proxy_row["models"] == ["nvidia/nim/minimax-m3", "nvidia/nim/kimi-k2.6"]
|
||||
|
||||
# Aggregator lost the overlapping model but kept the rest
|
||||
assert "nvidia/nim/minimax-m3" not in or_row["models"]
|
||||
assert "minimax/minimax-m3" in or_row["models"]
|
||||
assert "anthropic/claude-sonnet-4.6" in or_row["models"]
|
||||
assert or_row["total_models"] == 2
|
||||
|
||||
|
||||
def test_aggregator_dedup_case_insensitive():
|
||||
"""Dedup uses case-insensitive matching. (#45954)"""
|
||||
rows = [
|
||||
_user_provider_row("my-proxy", ["NVIDIA/NIM/MiniMax-M3"]),
|
||||
_aggregator_row("openrouter", ["nvidia/nim/minimax-m3", "other/model"]),
|
||||
]
|
||||
ctx = _empty_ctx()
|
||||
with _list_auth_returning(rows):
|
||||
payload = build_models_payload(ctx)
|
||||
|
||||
or_row = next(r for r in payload["providers"] if r["slug"] == "openrouter")
|
||||
assert "nvidia/nim/minimax-m3" not in or_row["models"]
|
||||
assert or_row["total_models"] == 1
|
||||
|
||||
|
||||
def test_aggregator_dedup_no_overlap_unchanged():
|
||||
"""When there's no overlap, aggregator models are untouched. (#45954)"""
|
||||
rows = [
|
||||
_user_provider_row("litellm-proxy", ["custom/model-a"]),
|
||||
_aggregator_row("openrouter", ["anthropic/claude-sonnet-4.6"]),
|
||||
]
|
||||
ctx = _empty_ctx()
|
||||
with _list_auth_returning(rows):
|
||||
payload = build_models_payload(ctx)
|
||||
|
||||
or_row = next(r for r in payload["providers"] if r["slug"] == "openrouter")
|
||||
assert or_row["models"] == ["anthropic/claude-sonnet-4.6"]
|
||||
assert or_row["total_models"] == 1
|
||||
|
||||
|
||||
def test_aggregator_dedup_no_user_providers_unchanged():
|
||||
"""When there are no user-defined providers, nothing is filtered.
|
||||
(#45954)"""
|
||||
rows = [
|
||||
_aggregator_row("openrouter", [
|
||||
"nvidia/nim/minimax-m3",
|
||||
"anthropic/claude-sonnet-4.6",
|
||||
]),
|
||||
]
|
||||
ctx = _empty_ctx()
|
||||
with _list_auth_returning(rows):
|
||||
payload = build_models_payload(ctx)
|
||||
|
||||
or_row = payload["providers"][0]
|
||||
assert len(or_row["models"]) == 2
|
||||
|
||||
|
||||
def test_aggregator_dedup_multiple_user_providers():
|
||||
"""Models from all user-defined providers are excluded from aggregators.
|
||||
(#45954)"""
|
||||
rows = [
|
||||
_user_provider_row("proxy-a", ["model-x"]),
|
||||
_user_provider_row("proxy-b", ["model-y"]),
|
||||
_aggregator_row("openrouter", ["model-x", "model-y", "model-z"]),
|
||||
]
|
||||
ctx = _empty_ctx()
|
||||
with _list_auth_returning(rows):
|
||||
payload = build_models_payload(ctx)
|
||||
|
||||
or_row = next(r for r in payload["providers"] if r["slug"] == "openrouter")
|
||||
assert or_row["models"] == ["model-z"]
|
||||
assert or_row["total_models"] == 1
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue