From 60cc42e38bf6570766b4cc24a8ac673aebb783c7 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sun, 14 Jun 2026 13:03:59 +0800 Subject: [PATCH] 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 --- hermes_cli/inventory.py | 30 +++++++ tests/hermes_cli/test_inventory.py | 124 +++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/hermes_cli/inventory.py b/hermes_cli/inventory.py index 48fc4e928d1..43d3150ccdb 100644 --- a/hermes_cli/inventory.py +++ b/hermes_cli/inventory.py @@ -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: diff --git a/tests/hermes_cli/test_inventory.py b/tests/hermes_cli/test_inventory.py index e51c62a2701..e81288f9ab1 100644 --- a/tests/hermes_cli/test_inventory.py +++ b/tests/hermes_cli/test_inventory.py @@ -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 +