diff --git a/cli.py b/cli.py index c86e7a4a3290d..502957121f0a0 100644 --- a/cli.py +++ b/cli.py @@ -119,6 +119,52 @@ def format_duration_compact(*args, **kwargs): return f"{days:.1f}d" +# Cached reverse map of config.yaml ``model_aliases:`` so the TUI can show +# friendly names instead of full Palantir RIDs / long catalog IDs. Built +# lazily on first call; cache is process-lifetime (config is read once at +# session start, so further invalidation is unnecessary). +_REVERSE_ALIAS_CACHE: dict[str, str] | None = None + + +def _reverse_alias_for_display(model_name: str) -> str: + """Return the shortest configured alias for ``model_name``, or ``model_name``. + + Looks up both ``model_aliases:`` (dict-based, full DirectAlias entries) + and ``model.aliases:`` (string-based, set via ``hermes config set``) + from config.yaml. Multiple aliases pointing at the same model — the + shortest wins, so ``opus47`` beats ``palantir-claude47``. + """ + global _REVERSE_ALIAS_CACHE + if not model_name: + return model_name + if _REVERSE_ALIAS_CACHE is None: + rmap: dict[str, str] = {} + try: + from hermes_cli.config import load_config + cfg = load_config() or {} + ma = cfg.get("model_aliases") + if isinstance(ma, dict): + for alias, entry in ma.items(): + if isinstance(entry, dict): + m = str(entry.get("model", "") or "").strip() + if m and (m not in rmap or len(alias) < len(rmap[m])): + rmap[m] = alias + mdl = cfg.get("model", {}) or {} + if isinstance(mdl, dict): + simple = mdl.get("aliases") + if isinstance(simple, dict): + for alias, val in simple.items(): + if isinstance(val, str) and val.strip(): + v = val.strip() + m = v.split("/", 1)[1] if "/" in v else v + if m and (m not in rmap or len(alias) < len(rmap[m])): + rmap[m] = alias + except Exception: + pass + _REVERSE_ALIAS_CACHE = rmap + return _REVERSE_ALIAS_CACHE.get(model_name, model_name) + + def format_token_count_compact(*args, **kwargs): value = int(args[0] if args else kwargs.get("value", 0)) abs_value = abs(value) @@ -4580,7 +4626,17 @@ def _get_status_bar_snapshot(self) -> Dict[str, Any]: # _try_activate_fallback() switches provider/model. agent = getattr(self, "agent", None) model_name = (getattr(agent, "model", None) or self.model or "unknown") - model_short = model_name.split("/")[-1] if "/" in model_name else model_name + # Friendly display: prefer reverse-alias from config.yaml ``model_aliases:`` + # before slash/length truncation. This turns long Palantir RIDs like + # ``ri.language-model-service..language-model.anthropic-claude-4-7-opus`` + # into the user's chosen short name (e.g. ``opus-4.7``) in the status bar. + model_short = _reverse_alias_for_display(model_name) + if model_short == model_name: + model_short = model_name.split("/")[-1] if "/" in model_name else model_name + # Strip Palantir RID prefixes via the shared display formatter so + # this site and ``ModelSwitchResult`` confirmation can't drift. + from hermes_cli.model_switch import format_model_for_display + model_short = format_model_for_display(model_short) if model_short.endswith(".gguf"): model_short = model_short[:-5] if len(model_short) > 26: @@ -7996,14 +8052,18 @@ def _apply_model_switch_result(self, result, persist_global: bool) -> None: ) return + from hermes_cli.model_switch import format_model_for_display + _display_old = format_model_for_display(old_model) + _display_new = format_model_for_display(result.new_model) + self._pending_model_switch_note = ( - f"[Note: model was just switched from {old_model} to {result.new_model} " + f"[Note: model was just switched from {_display_old} to {_display_new} " f"via {result.provider_label or result.target_provider}. " f"Adjust your self-identification accordingly.]" ) provider_label = result.provider_label or result.target_provider - _cprint(f" ✓ Model switched: {result.new_model}") + _cprint(f" ✓ Model switched: {_display_new}") _cprint(f" Provider: {provider_label}") # Context: always resolve via the provider-aware chain so Codex OAuth, @@ -8327,8 +8387,12 @@ def _handle_model_switch(self, cmd_original: str): # Store a note to prepend to the next user message so the model # knows a switch occurred (avoids injecting system messages mid-history # which breaks providers and prompt caching). + from hermes_cli.model_switch import format_model_for_display + _display_old = format_model_for_display(old_model) + _display_new = format_model_for_display(result.new_model) + self._pending_model_switch_note = ( - f"[Note: model was just switched from {old_model} to {result.new_model} " + f"[Note: model was just switched from {_display_old} to {_display_new} " f"via {result.provider_label or result.target_provider}. " f"{'This override applies to the next turn only. ' if one_turn else ''}" f"Adjust your self-identification accordingly.]" @@ -8340,7 +8404,7 @@ def _handle_model_switch(self, cmd_original: str): # Display confirmation with full metadata provider_label = result.provider_label or result.target_provider - _cprint(f" ✓ Model switched: {result.new_model}") + _cprint(f" ✓ Model switched: {_display_new}") _cprint(f" Provider: {provider_label}") # Context: always resolve via the provider-aware chain so Codex OAuth, diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 03c3017c8baec..419a11ad293f6 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -1663,11 +1663,17 @@ async def _on_model_selected_scoped( "Failed to persist model switch to DB: %s", exc ) - # Store model note + session override + # Store model note + session override. Use display + # form (strips opaque Palantir prefix) for the user- + # visible note; session-override map still gets the + # full opaque ID, which is what the wire needs. + from hermes_cli.model_switch import format_model_for_display + _display_cur = format_model_for_display(_cur_model) + _display_new = format_model_for_display(result.new_model) if not hasattr(_self, "_pending_model_notes"): _self._pending_model_notes = {} _self._pending_model_notes[_session_key] = ( - f"[Note: model was just switched from {_cur_model} to {result.new_model} " + f"[Note: model was just switched from {_display_cur} to {_display_new} " f"via {result.provider_label or result.target_provider}. " f"Adjust your self-identification accordingly.]" ) @@ -1743,9 +1749,11 @@ async def _on_model_selected_scoped( except Exception as e: logger.warning("Failed to persist model switch: %s", e) - # Build confirmation text + # Build confirmation text. Use display form so opaque + # Palantir IDs (ri.language-model-service..*) get + # shortened to their trailing slug for the UI. plabel = result.provider_label or result.target_provider - lines = [t("gateway.model.switched", model=result.new_model)] + lines = [t("gateway.model.switched", model=format_model_for_display(result.new_model))] lines.append(t("gateway.model.provider_label", provider=plabel)) mi = result.model_info from hermes_cli.model_switch import resolve_display_context_length @@ -1939,10 +1947,13 @@ async def _finish_switch() -> str: # Store a note to prepend to the next user message so the model # knows about the switch (avoids system messages mid-history). + # Display form strips opaque Palantir RID prefixes; the override + # map below keeps the full ID for the wire. + from hermes_cli.model_switch import format_model_for_display if not hasattr(self, "_pending_model_notes"): self._pending_model_notes = {} self._pending_model_notes[session_key] = ( - f"[Note: model was just switched from {current_model} to {result.new_model} " + f"[Note: model was just switched from {format_model_for_display(current_model)} to {format_model_for_display(result.new_model)} " f"via {result.provider_label or result.target_provider}. " f"{'This override applies to the next turn only. ' if one_turn else ''}" f"Adjust your self-identification accordingly.]" @@ -2038,7 +2049,7 @@ async def _finish_switch() -> str: # Build confirmation message with full metadata provider_label = result.provider_label or result.target_provider - lines = [t("gateway.model.switched", model=result.new_model)] + lines = [t("gateway.model.switched", model=format_model_for_display(result.new_model))] lines.append(t("gateway.model.provider_label", provider=provider_label)) # Context: always resolve via the provider-aware chain so Codex OAuth, diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index f3fd20875890b..5d58c5911eb9a 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -200,6 +200,51 @@ def _bare_custom_provider_def(current_base_url: str) -> Optional[ProviderDef]: ) +# Opaque internal model-ID display +# --------------------------------------------------------------------------- +# Some proxies (notably Palantir Foundry's LLM-proxy) identify models by +# resource-instance IDs that are deeply nested, verbose, and pure noise to +# read in CLI status output, e.g.: +# +# ri.language-model-service..language-model.anthropic-claude-4-7-opus +# +# The provider_label (e.g. "palantir-claude46") already carries the routing +# context, so the only useful information left in the opaque ID is the +# trailing slug. Strip the boilerplate prefix for *display* — never for +# wire-side comparison, persistence, config writes, alias lookup, or +# anything that round-trips back into the API. +# +# Match by substring on a known prefix so we never accidentally truncate +# a legitimate model name that happens to contain dots. + +_OPAQUE_MODEL_PREFIXES: tuple[str, ...] = ( + "ri.language-model-service..language-model.", +) + + +def format_model_for_display(model_name: str) -> str: + """Return a human-friendly form of *model_name* for CLI status output. + + Strips known opaque proxy prefixes (Palantir Foundry's + ``ri.language-model-service..language-model.*``) and returns the + trailing slug. Falls through to the original string for everything + else, so real model IDs (``claude-4-7-opus-20260101``, + ``gpt-5-4``, ``meta-llama/Llama-3.3-70B-Instruct``) are untouched. + + This is a DISPLAY-ONLY helper. Do NOT use the return value for any + wire-side operation — the proxy expects the full opaque ID, and + callers that compare or persist must keep the original. + """ + if not model_name: + return model_name + for prefix in _OPAQUE_MODEL_PREFIXES: + if model_name.startswith(prefix): + tail = model_name[len(prefix):] + return tail if tail else model_name + return model_name + + +# --------------------------------------------------------------------------- def is_nous_hermes_non_agentic(model_name: str) -> bool: """Return True if *model_name* is a real Nous Hermes 3/4 chat model. @@ -2142,37 +2187,115 @@ def _has_aws_sdk_creds_for_listing(slug: str) -> bool: # and one "custom:openrouter" from section 4, both labelled identically. _section3_emitted_pairs: set = set() if user_providers and isinstance(user_providers, dict): + # Group ``providers:`` entries by (api_url, key_env, api_mode) so that + # multiple keyed providers pointing at the same endpoint with the + # same credential and wire-protocol collapse into one picker row. + # Mirrors section-4's grouping for ``custom_providers:`` lists. + # Concrete case: a Palantir Foundry Anthropic-proxy with two + # configured models (claude-4.6 + claude-4.7) — both share the same + # api/key_env/api_mode and used to produce two near-duplicate rows + # labelled "Palantir Claude 4.6 Opus" and "Palantir Claude 4.7 Opus"; + # now they appear as a single "Palantir Claude" row with both models + # in the dropdown. Same-host entries with different ``key_env`` or + # ``api_mode`` (e.g. an OpenAI-compat gpt-5.4 alongside the Anthropic + # claude-4.7 on the same Palantir host) keep distinct rows since + # the wire protocol differs. + from collections import OrderedDict as _OD3 + + ep_groups: "_OD3[tuple, dict]" = _OD3() for ep_name, ep_cfg in user_providers.items(): if not isinstance(ep_cfg, dict): continue - # Skip if this slug was already emitted (e.g. canonical provider - # with the same name) or will be picked up by section 4. if ep_name.lower() in seen_slugs: continue display_name = ep_cfg.get("name", "") or ep_name - # ``base_url`` is Hermes's canonical write key (matches - # custom_providers and _save_custom_provider); ``api`` / ``url`` - # remain as fallbacks for hand-edited / legacy configs. api_url = ( ep_cfg.get("base_url", "") or ep_cfg.get("api", "") or ep_cfg.get("url", "") or "" ) + key_env = str(ep_cfg.get("key_env", "") or "").strip() + inline_api_key = str(ep_cfg.get("api_key", "") or "").strip() + api_mode = str( + ep_cfg.get("api_mode") + or ep_cfg.get("transport") + or "" + ).strip().lower() + credential_identity = ( + inline_api_key + if inline_api_key + else (f"env:{key_env}" if key_env else "") + ) + api_url_norm = str(api_url).strip().rstrip("/").lower() + # Per-provider extra_headers participate in the group identity + # (same invariant as section 4): two entries sharing + # (api_url, credential, api_mode) but declaring different headers + # are distinct endpoints (e.g. different tenants behind one proxy + # URL, routed by header) and must keep distinct picker rows. + entry_extra_headers = _extra_headers_from_config(ep_cfg) + headers_identity = tuple(sorted(entry_extra_headers.items())) + group_key = (api_url_norm, credential_identity, api_mode, headers_identity) + # ``default_model`` is the legacy key; ``model`` matches what # custom_providers entries use, so accept either. default_model = ep_cfg.get("default_model", "") or ep_cfg.get("model", "") - - # Build models list from both default_model and full models array - models_list = [] - if default_model: - models_list.append(default_model) - # Also include the full models list from config. + # Build models list from both default_model and full models array. # Hermes writes ``models:`` as a dict keyed by model id, but older - # or hand-edited configs may use strings or ``[{id: ...}]`` rows. + # or hand-edited configs may use strings or ``[{id: ...}]`` rows — + # _declared_model_ids() owns that contract. + entry_models: list = [] + if default_model: + entry_models.append(default_model) for model_id in _declared_model_ids(ep_cfg.get("models", [])): - if model_id not in models_list: - models_list.append(model_id) + if model_id not in entry_models: + entry_models.append(model_id) + + if group_key not in ep_groups: + # Strip per-model suffix so "Palantir Claude 4.7 Opus" becomes + # "Palantir Claude". Em dash and " - " are the separators + # Hermes's own writer uses (mirrors section-4 grouping). + grp_display = display_name + for sep in ("—", " - "): + if sep in grp_display: + grp_display = grp_display.split(sep)[0].strip() + break + # Drop trailing numeric/version tokens that distinguish per-model + # entries ("Palantir Claude 4.7 Opus" → "Palantir Claude"). + # Keeps the row label short; the model dropdown carries the + # per-version detail. Heuristic: split at the first token whose + # stripped form contains a digit; keep the prefix only if it + # is at least 2 words (avoids over-trimming single-word names). + _toks = grp_display.split() + _cut_at = None + for _i, _t in enumerate(_toks): + _tl = _t.strip(".,()") + if _tl and any(c.isdigit() for c in _tl): + _cut_at = _i + break + if _cut_at is not None and _cut_at >= 2: + grp_display = " ".join(_toks[:_cut_at]).strip() + grp_slug = ep_name # primary slug is the first ep_name encountered + ep_groups[group_key] = { + "slug": grp_slug, + "name": grp_display or display_name, + "api_url": api_url, + "models": [], + "ep_cfg": ep_cfg, # used below for discover_models / api_key + "raw_names": [], + } + # Aggregate models across all members of the group (preserve order). + for _m in entry_models: + if _m and _m not in ep_groups[group_key]["models"]: + ep_groups[group_key]["models"].append(_m) + ep_groups[group_key]["raw_names"].append(display_name) + + for grp in ep_groups.values(): + ep_cfg = grp["ep_cfg"] + ep_name = grp["slug"] + display_name = grp["name"] + api_url = grp["api_url"] + models_list = list(grp["models"]) # Official OpenAI API rows in providers: often have base_url but no # explicit models: dict — avoid a misleading zero count in /model. @@ -2240,9 +2363,22 @@ def _has_aws_sdk_creds_for_listing(slug: str) -> bool: }) seen_slugs.add(ep_name.lower()) seen_slugs.add(custom_provider_slug(display_name).lower()) + # Record (display_name, api_url) for each raw entry that joined + # this group so section-4's _section3_emitted_pairs dedup can + # match per-model custom_providers rows ("Palantir Claude 4.7 Opus") + # even though we collapsed the group label to "Palantir Claude". + _url_norm_for_pair = str(api_url).strip().rstrip("/").lower() + for _raw_name in grp.get("raw_names") or [display_name]: + _pair = ( + str(_raw_name).strip().lower(), + _url_norm_for_pair, + ) + if _pair[0] and _pair[1]: + _section3_emitted_pairs.add(_pair) + seen_slugs.add(custom_provider_slug(_raw_name).lower()) _pair = ( str(display_name).strip().lower(), - str(api_url).strip().rstrip("/").lower(), + _url_norm_for_pair, ) if _pair[0] and _pair[1]: _section3_emitted_pairs.add(_pair) diff --git a/tests/hermes_cli/test_provider_section3_grouping.py b/tests/hermes_cli/test_provider_section3_grouping.py new file mode 100644 index 0000000000000..6f8c73934d7a1 --- /dev/null +++ b/tests/hermes_cli/test_provider_section3_grouping.py @@ -0,0 +1,149 @@ +"""Regression tests for section-3 (``providers:``) same-endpoint grouping in +``list_authenticated_providers`` and for ``format_model_for_display``. + +Salvaged with PR #36998 (@antydizajn): section 3 folds ``providers:`` entries +that share (api_url, credential, api_mode, extra_headers) into one picker row, +mirroring section 4's grouping for ``custom_providers:``. These are invariant +tests — grouping identity, header-routed separation, list-of-dict model +declarations, and display-only RID stripping. +""" + +import hermes_cli.providers as providers_mod +from hermes_cli.model_switch import ( + format_model_for_display, + list_authenticated_providers, +) + + +def _providers(monkeypatch, user_providers): + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {}) + monkeypatch.setattr(providers_mod, "HERMES_OVERLAYS", {}) + monkeypatch.setattr("hermes_cli.models.fetch_api_models", lambda *a, **k: []) + return list_authenticated_providers( + user_providers=user_providers, + custom_providers=[], + max_models=50, + ) + + +def _user_rows(rows): + return [p for p in rows if p.get("source") == "user-config"] + + +def test_same_endpoint_same_credential_entries_fold_to_one_row(monkeypatch): + """Two providers: entries differing only by model id collapse into one + picker row carrying both models (the Palantir Foundry case).""" + rows = _user_rows(_providers(monkeypatch, { + "palantir-claude46": { + "name": "Palantir Claude 4.6 Opus", + "base_url": "https://foundry.example.com/anthropic", + "key_env": "PALANTIR_TOKEN", + "api_mode": "anthropic_messages", + "model": "ri.language-model-service..language-model.anthropic-claude-4-6-opus", + }, + "palantir-claude47": { + "name": "Palantir Claude 4.7 Opus", + "base_url": "https://foundry.example.com/anthropic", + "key_env": "PALANTIR_TOKEN", + "api_mode": "anthropic_messages", + "model": "ri.language-model-service..language-model.anthropic-claude-4-7-opus", + }, + })) + assert len(rows) == 1 + row = rows[0] + assert row["slug"] == "palantir-claude46" # first member's slug wins + assert row["name"] == "Palantir Claude" # version suffix stripped + assert len(row["models"]) == 2 + + +def test_different_api_mode_keeps_distinct_rows(monkeypatch): + """Same host + credential but a different wire protocol must not fold.""" + rows = _user_rows(_providers(monkeypatch, { + "proxy-claude": { + "name": "Proxy Claude", + "base_url": "https://proxy.example.com/v1", + "key_env": "PROXY_TOKEN", + "api_mode": "anthropic_messages", + "model": "claude-opus-4.6", + }, + "proxy-gpt": { + "name": "Proxy GPT", + "base_url": "https://proxy.example.com/v1", + "key_env": "PROXY_TOKEN", + "api_mode": "openai_chat", + "model": "gpt-5.4", + }, + })) + assert len(rows) == 2 + + +def test_different_extra_headers_keep_distinct_rows(monkeypatch): + """Header-routed tenants behind one proxy URL are distinct endpoints — + extra_headers is part of the group identity (mirrors section 4).""" + rows = _user_rows(_providers(monkeypatch, { + "tenant-a": { + "name": "Tenant A", + "base_url": "https://proxy.example.com/v1", + "key_env": "PROXY_TOKEN", + "api_mode": "openai_chat", + "extra_headers": {"X-Tenant": "a"}, + "model": "model-a", + }, + "tenant-b": { + "name": "Tenant B", + "base_url": "https://proxy.example.com/v1", + "key_env": "PROXY_TOKEN", + "api_mode": "openai_chat", + "extra_headers": {"X-Tenant": "b"}, + "model": "model-b", + }, + })) + assert len(rows) == 2 + + +def test_list_of_dict_model_declarations_are_honored(monkeypatch): + """``models: [{"id": ...}]`` rows go through _declared_model_ids — the + grouped path must not regress that contract.""" + rows = _user_rows(_providers(monkeypatch, { + "dictrows": { + "name": "Dict Rows", + "base_url": "https://dictrows.example.com/v1", + "key_env": "DICTROWS_TOKEN", + "models": [{"id": "model-x"}, {"id": "model-y"}], + }, + })) + assert len(rows) == 1 + assert rows[0]["models"] == ["model-x", "model-y"] + + +def test_single_word_group_name_not_over_trimmed(monkeypatch): + """Version-token stripping only applies when the prefix keeps >= 2 words.""" + rows = _user_rows(_providers(monkeypatch, { + "gpt54-a": { + "name": "GPT 5.4", + "base_url": "https://single.example.com/v1", + "key_env": "SINGLE_TOKEN", + "model": "gpt-5.4", + }, + })) + assert rows[0]["name"] == "GPT 5.4" + + +class TestFormatModelForDisplay: + def test_palantir_rid_stripped_to_trailing_slug(self): + rid = "ri.language-model-service..language-model.anthropic-claude-4-7-opus" + assert format_model_for_display(rid) == "anthropic-claude-4-7-opus" + + def test_plain_names_pass_through(self): + for name in ( + "claude-opus-4.6", + "gpt-5.4", + "meta-llama/Llama-3.3-70B-Instruct", + "some-model.gguf", + "", + ): + assert format_model_for_display(name) == name + + def test_prefix_only_edge_preserved(self): + """A bare prefix with no trailing slug must not become empty.""" + assert format_model_for_display("ri.language-model-service..language-model.") != ""