fix(model-picker): dedup overlapping providers: dict and custom_providers: list entries
When a user's config has the same endpoint in both the providers: dict (v12+ keyed schema) and custom_providers: list (legacy schema) — which happens automatically when callers pass the output of get_compatible_custom_providers() alongside the raw providers dict — list_authenticated_providers() emitted two picker rows for the same endpoint: one bare-slug from section 3 and one 'custom:<name>' from section 4. The slug shapes differed, so seen_slugs dedup never fired, and users saw the same endpoint twice with identical display labels. Fix: section 3 records the (display_name, base_url) of each emitted entry in _section3_emitted_pairs; section 4 skips groups whose (name, api_url) pair was already emitted. Preserves existing behaviour for users on either schema alone, and for distinct entries across both. Test: test_list_authenticated_providers_no_duplicate_labels_across_schemas.
This commit is contained in:
@@ -1035,6 +1035,13 @@ def list_authenticated_providers(
|
|||||||
seen_slugs.add(_cp.slug.lower())
|
seen_slugs.add(_cp.slug.lower())
|
||||||
|
|
||||||
# --- 3. User-defined endpoints from config ---
|
# --- 3. User-defined endpoints from config ---
|
||||||
|
# Track (name, base_url) of what section 3 emits so section 4 can skip
|
||||||
|
# any overlapping ``custom_providers:`` entries. Callers typically pass
|
||||||
|
# both (gateway/CLI invoke ``get_compatible_custom_providers()`` which
|
||||||
|
# merges ``providers:`` into the list) — without this, the same endpoint
|
||||||
|
# produces two picker rows: one bare-slug ("openrouter") from section 3
|
||||||
|
# and one "custom:openrouter" from section 4, both labelled identically.
|
||||||
|
_section3_emitted_pairs: set = set()
|
||||||
if user_providers and isinstance(user_providers, dict):
|
if user_providers and isinstance(user_providers, dict):
|
||||||
for ep_name, ep_cfg in user_providers.items():
|
for ep_name, ep_cfg in user_providers.items():
|
||||||
if not isinstance(ep_cfg, dict):
|
if not isinstance(ep_cfg, dict):
|
||||||
@@ -1088,6 +1095,12 @@ def list_authenticated_providers(
|
|||||||
"api_url": api_url,
|
"api_url": api_url,
|
||||||
})
|
})
|
||||||
seen_slugs.add(ep_name.lower())
|
seen_slugs.add(ep_name.lower())
|
||||||
|
_pair = (
|
||||||
|
str(display_name).strip().lower(),
|
||||||
|
str(api_url).strip().rstrip("/").lower(),
|
||||||
|
)
|
||||||
|
if _pair[0] and _pair[1]:
|
||||||
|
_section3_emitted_pairs.add(_pair)
|
||||||
|
|
||||||
# --- 4. Saved custom providers from config ---
|
# --- 4. Saved custom providers from config ---
|
||||||
# Each ``custom_providers`` entry represents one model under a named
|
# Each ``custom_providers`` entry represents one model under a named
|
||||||
@@ -1146,6 +1159,17 @@ def list_authenticated_providers(
|
|||||||
for slug, grp in groups.items():
|
for slug, grp in groups.items():
|
||||||
if slug.lower() in seen_slugs:
|
if slug.lower() in seen_slugs:
|
||||||
continue
|
continue
|
||||||
|
# Skip if section 3 already emitted this endpoint under its
|
||||||
|
# ``providers:`` dict key — matches on (display_name, base_url),
|
||||||
|
# the tuple section 4 groups by. Prevents two picker rows
|
||||||
|
# labelled identically when callers pass both ``user_providers``
|
||||||
|
# and a compatibility-merged ``custom_providers`` list.
|
||||||
|
_pair_key = (
|
||||||
|
str(grp["name"]).strip().lower(),
|
||||||
|
str(grp["api_url"]).strip().rstrip("/").lower(),
|
||||||
|
)
|
||||||
|
if _pair_key[0] and _pair_key[1] and _pair_key in _section3_emitted_pairs:
|
||||||
|
continue
|
||||||
results.append({
|
results.append({
|
||||||
"slug": slug,
|
"slug": slug,
|
||||||
"name": grp["name"],
|
"name": grp["name"],
|
||||||
|
|||||||
@@ -304,6 +304,54 @@ def test_list_authenticated_providers_dedupes_when_user_and_custom_overlap(monke
|
|||||||
assert matches[0]["models"] == ["gpt-5.4", "grok-4.20-beta"]
|
assert matches[0]["models"] == ["gpt-5.4", "grok-4.20-beta"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_list_authenticated_providers_no_duplicate_labels_across_schemas(monkeypatch):
|
||||||
|
"""Regression: same endpoint in both ``providers:`` dict AND ``custom_providers:``
|
||||||
|
list (e.g. via ``get_compatible_custom_providers()``) must not emit two picker
|
||||||
|
rows with identical display names.
|
||||||
|
|
||||||
|
Before the fix, section 3 emitted bare-slug rows ("openrouter") and section 4
|
||||||
|
emitted ``custom:openrouter`` rows for the same endpoint — both labelled
|
||||||
|
identically, bypassing ``seen_slugs`` dedup because the slug shapes differ.
|
||||||
|
"""
|
||||||
|
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||||
|
monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {})
|
||||||
|
|
||||||
|
shared_entries = [
|
||||||
|
("endpoint-a", "http://a.local/v1"),
|
||||||
|
("endpoint-b", "http://b.local/v1"),
|
||||||
|
("endpoint-c", "http://c.local/v1"),
|
||||||
|
]
|
||||||
|
|
||||||
|
user_providers = {
|
||||||
|
name: {"name": name, "base_url": url, "model": "m1"}
|
||||||
|
for name, url in shared_entries
|
||||||
|
}
|
||||||
|
custom_providers = [
|
||||||
|
{"name": name, "base_url": url, "model": "m1"}
|
||||||
|
for name, url in shared_entries
|
||||||
|
]
|
||||||
|
|
||||||
|
providers = list_authenticated_providers(
|
||||||
|
current_provider="none",
|
||||||
|
user_providers=user_providers,
|
||||||
|
custom_providers=custom_providers,
|
||||||
|
max_models=50,
|
||||||
|
)
|
||||||
|
|
||||||
|
user_rows = [p for p in providers if p.get("source") == "user-config"]
|
||||||
|
# Expect one row per shared entry — not two.
|
||||||
|
assert len(user_rows) == len(shared_entries), (
|
||||||
|
f"Expected {len(shared_entries)} rows, got {len(user_rows)}: "
|
||||||
|
f"{[(p['slug'], p['name']) for p in user_rows]}"
|
||||||
|
)
|
||||||
|
|
||||||
|
# And zero duplicate display labels.
|
||||||
|
labels = [p["name"].lower() for p in user_rows]
|
||||||
|
assert len(labels) == len(set(labels)), (
|
||||||
|
f"Duplicate labels across picker rows: {labels}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# =============================================================================
|
# =============================================================================
|
||||||
# Tests for _get_named_custom_provider with providers: dict
|
# Tests for _get_named_custom_provider with providers: dict
|
||||||
# =============================================================================
|
# =============================================================================
|
||||||
|
|||||||
Reference in New Issue
Block a user