From 2ffdf08376756771891289b49e40fd2de8c061a4 Mon Sep 17 00:00:00 2001 From: Almurat Date: Fri, 26 Jun 2026 22:19:27 +0800 Subject: [PATCH] fix: show both kimi-coding and kimi-coding-cn in /model picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both providers share the same models.dev ID (kimi-for-coding) but have different API keys (KIMI_API_KEY vs KIMI_CN_API_KEY) and base URLs (moonshot.ai vs moonshot.cn). The /model picker was only showing one because the dedup key was mdev_id alone. Changes in list_authenticated_providers(): - Resolve canonical provider profile name and skip alias hermes_ids (e.g. "kimi", "moonshot" → "kimi-coding") so only canonical entries are processed. - Deduplicate by slug (hermes_id) instead of mdev_id so distinct profiles sharing a models.dev ID (kimi-coding vs kimi-coding-cn) both appear. - Prefer PROVIDER_REGISTRY name for the display label so the CN variant shows "Kimi / Moonshot (China)" instead of the generic models.dev name. Adds test coverage for all three key scenarios: - Only KIMI_CN_API_KEY set → only kimi-coding-cn appears - Only KIMI_API_KEY set → only kimi-coding appears - Both keys set → both providers appear, aliases not duplicated Closes #10526 --- hermes_cli/model_switch.py | 49 ++++---- .../test_kimi_cn_provider_listing.py | 119 ++++++++++++++++++ 2 files changed, 144 insertions(+), 24 deletions(-) create mode 100644 tests/hermes_cli/test_kimi_cn_provider_listing.py diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 268b9ecf15c1..a86b1316e99d 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1720,7 +1720,6 @@ def list_authenticated_providers( results: List[dict] = [] seen_slugs: set = set() # lowercase-normalized to catch case variants (#9545) - seen_mdev_ids: set = set() # prevent duplicate entries for aliases (e.g. kimi-coding + kimi-coding-cn) _current_provider_norm = str(current_provider or "").strip().lower() _current_base_url_norm = str(current_base_url or "").strip().rstrip("/").lower() @@ -1867,10 +1866,30 @@ def list_authenticated_providers( and _alias_target in _AGG_PROVIDERS ): continue - # Skip aliases that map to the same models.dev provider (e.g. - # kimi-coding and kimi-coding-cn both → kimi-for-coding). - # The first one with valid credentials wins (#10526). - if mdev_id in seen_mdev_ids: + # Resolve the canonical provider profile name. Skip hermes_ids + # that are mere aliases resolving to a different canonical profile + # (e.g. "kimi" and "moonshot" both → "kimi-coding"). Only process + # entries whose hermes_id matches the canonical profile name so + # distinct profiles (e.g. kimi-coding, kimi-coding-cn) each get + # their own picker row. + _canonical = hermes_id + try: + from providers import get_provider_profile as _gpp + _prof = _gpp(hermes_id) + if _prof is not None: + _canonical = _prof.name + except Exception: + pass + if _canonical != hermes_id: + continue + + # Skip duplicates: another entry with the same slug was already + # emitted (e.g. two PROVIDER_TO_MODELS_DEV entries routing to the + # same hermes_id). Distinct canonical profiles that share a + # models.dev ID (e.g. kimi-coding and kimi-coding-cn → kimi-for-coding) + # are both allowed through since they have different slugs. + slug = hermes_id + if slug.lower() in seen_slugs: continue if hermes_id.lower() in _excluded or mdev_id.lower() in _excluded: continue @@ -1941,25 +1960,8 @@ def list_authenticated_providers( else: top = model_ids[:max_models] if max_models is not None else model_ids - # Emit under the CANONICAL Hermes slug, not the bare alias. A single - # credential can be reachable under several PROVIDER_TO_MODELS_DEV keys - # that all share one models.dev id (e.g. "kimi", "moonshot" and the - # canonical "kimi-coding" all map to "kimi-for-coding"). The - # seen_mdev_ids guard above already collapses them to the first key — - # but that first key is usually the bare alias ("kimi"), so emitting it - # verbatim leaves section 2b free to re-emit the canonical "kimi-coding" - # from CANONICAL_PROVIDERS: one key, two picker rows (#49439). Resolving - # to the canonical slug here lets 2b's seen_slugs check collapse the - # pair, matches the picker's other alias rows (copilot, gemini, …), and - # keeps the row resolvable to the real provider. - slug = _CANON_ALIASES.get(hermes_id.lower(), hermes_id) - if slug.lower() in seen_slugs: - # Canonical already emitted by an earlier alias in this pass; don't - # add a second row for the same provider. - seen_mdev_ids.add(mdev_id) - continue pinfo = _mdev_pinfo(mdev_id) - display_name = pinfo.name if pinfo else mdev_id + display_name = pconfig.name if pconfig and pconfig.name else (pinfo.name if pinfo else mdev_id) results.append({ "slug": slug, @@ -1975,7 +1977,6 @@ def list_authenticated_providers( "source": "built-in", }) seen_slugs.add(slug.lower()) - seen_mdev_ids.add(mdev_id) _record_builtin_endpoint(slug) # --- 2. Check Hermes-only providers (nous, openai-codex, copilot, opencode-go) --- diff --git a/tests/hermes_cli/test_kimi_cn_provider_listing.py b/tests/hermes_cli/test_kimi_cn_provider_listing.py new file mode 100644 index 000000000000..5d7f473e3249 --- /dev/null +++ b/tests/hermes_cli/test_kimi_cn_provider_listing.py @@ -0,0 +1,119 @@ +"""Test that kimi-coding and kimi-coding-cn both appear in the /model picker. + +Both providers share the same models.dev ID (kimi-for-coding) but are distinct +profiles with different API keys, base URLs, and endpoints. The /model picker +must show both so users can pick the right endpoint for their key type. + +Regression: the original ``seen_mdev_ids`` dedup by mdev_id alone would skip +kimi-coding-cn after kimi-coding was emitted because both map to +``kimi-for-coding`` (#10526). The fix deduplicates by +``(mdev_id, canonical_profile_name)`` instead, allowing distinct profiles +through. +""" + +import os +from unittest.mock import patch + +from hermes_cli.model_switch import list_authenticated_providers + + +# -- Only KIMI_CN_API_KEY set ------------------------------------------------ + + +@patch.dict(os.environ, {"KIMI_CN_API_KEY": "sk-cn-fake"}, clear=False) +def test_kimi_cn_appears_when_only_cn_key_set(): + """kimi-coding-cn should appear when only KIMI_CN_API_KEY is set.""" + providers = list_authenticated_providers(current_provider="kimi-coding-cn") + + # kimi-coding-cn must be listed (it has credentials) + cn = next((p for p in providers if p["slug"] == "kimi-coding-cn"), None) + assert cn is not None, ( + "kimi-coding-cn should appear when KIMI_CN_API_KEY is set" + ) + assert cn["is_current"] is True + assert cn["total_models"] > 0 + + # kimi-coding must NOT appear (no KIMI_API_KEY) + intl = next((p for p in providers if p["slug"] == "kimi-coding"), None) + assert intl is None, ( + "kimi-coding should NOT appear when only KIMI_CN_API_KEY is set" + ) + + +# -- Only KIMI_API_KEY set --------------------------------------------------- + + +@patch.dict(os.environ, {"KIMI_API_KEY": "sk-intl-fake"}, clear=False) +def test_kimi_intl_appears_when_only_intl_key_set(): + """kimi-coding (international) should appear when only KIMI_API_KEY is set.""" + providers = list_authenticated_providers(current_provider="kimi-coding") + + intl = next((p for p in providers if p["slug"] == "kimi-coding"), None) + assert intl is not None, ( + "kimi-coding should appear when KIMI_API_KEY is set" + ) + assert intl["is_current"] is True + + # kimi-coding-cn must NOT appear (no KIMI_CN_API_KEY) + cn = next((p for p in providers if p["slug"] == "kimi-coding-cn"), None) + assert cn is None, ( + "kimi-coding-cn should NOT appear when only KIMI_API_KEY is set" + ) + + +# -- Both keys set ----------------------------------------------------------- + +@patch.dict(os.environ, { + "KIMI_API_KEY": "sk-intl-fake", + "KIMI_CN_API_KEY": "sk-cn-fake", +}, clear=False) +def test_both_kimi_providers_appear_when_both_keys_set(): + """Both kimi-coding and kimi-coding-cn should appear when both keys set. + + They are distinct profiles with different env vars and endpoints. The + existing aliases (kimi, moonshot → kimi-coding; kimi-cn, moonshot-cn → + kimi-coding-cn) must NOT create additional rows. + """ + providers = list_authenticated_providers(current_provider="kimi-coding") + + # Both profile slugs must appear + intl = next((p for p in providers if p["slug"] == "kimi-coding"), None) + assert intl is not None, "kimi-coding should appear when KIMI_API_KEY is set" + assert intl["is_current"] is True + + cn = next((p for p in providers if p["slug"] == "kimi-coding-cn"), None) + assert cn is not None, ( + "kimi-coding-cn should appear when KIMI_CN_API_KEY is set" + ) + assert cn["is_current"] is False # `current_provider` is kimi-coding + + # Exactly 2 Kimi entries — no duplicates for aliases (kimi, moonshot, + # moonshot-cn, kimi-cn) + kimi_slugs = [p["slug"] for p in providers if "kimi" in p["slug"] or "moonshot" in p["slug"]] + assert len(kimi_slugs) == 2, ( + f"Expected exactly 2 Kimi entries (kimi-coding, kimi-coding-cn), " + f"got {kimi_slugs}" + ) + + +# -- Both aliases deduped correctly ------------------------------------------ + +@patch.dict(os.environ, { + "KIMI_API_KEY": "sk-intl-fake", + "KIMI_CN_API_KEY": "sk-cn-fake", +}, clear=False) +def test_kimi_aliases_not_listed_separately(): + """Alias hermes_ids (kimi, moonshot) must NOT create phantom picker rows. + + They resolve to the same canonical profile (kimi-coding) and should be + deduped. Only the canonical slug (kimi-coding) should appear. + """ + providers = list_authenticated_providers(current_provider="kimi-coding-cn") + + slugs = {p["slug"] for p in providers} + # These alias slugs must NOT appear + for bad_slug in ("kimi", "moonshot", "moonshot-cn", "kimi-cn"): + assert bad_slug not in slugs, ( + f"Alias slug '{bad_slug}' must not appear in picker (resolved to " + f"canonical profile)" + )