-
Notifications
You must be signed in to change notification settings - Fork 46.7k
tui: friendlier model display + group same-endpoint providers in picker #36998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
antydizajn
wants to merge
2
commits into
NousResearch:main
from
antydizajn:fix/tui-friendly-model-display
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,51 @@ | |
| ) | ||
|
|
||
|
|
||
| # 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. | ||
|
|
||
|
|
@@ -1504,44 +1549,108 @@ 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 "" | ||
| ) | ||
| # ``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", "") | ||
| 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() | ||
| group_key = (api_url_norm, credential_identity, api_mode) | ||
|
|
||
| # 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. | ||
| # Hermes writes ``models:`` as a dict keyed by model id | ||
| # (see hermes_cli/main.py::_save_custom_provider); older | ||
| # configs or hand-edited files may still use a list. | ||
| default_model = ep_cfg.get("default_model", "") or ep_cfg.get("model", "") | ||
| cfg_models = ep_cfg.get("models", []) | ||
| entry_models: list = [] | ||
| if default_model: | ||
| entry_models.append(default_model) | ||
| if isinstance(cfg_models, dict): | ||
| for m in cfg_models: | ||
| if m and m not in models_list: | ||
| models_list.append(m) | ||
| if m and m not in entry_models: | ||
| entry_models.append(m) | ||
| elif isinstance(cfg_models, list): | ||
| for m in cfg_models: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please aggregate via |
||
| if m and m not in models_list: | ||
| models_list.append(m) | ||
| if m and m not in entry_models: | ||
| entry_models.append(m) | ||
|
|
||
| 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. | ||
|
|
@@ -1584,9 +1693,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) | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include normalized
extra_headersin this identity. Current section 4 does so because same URL/key/mode entries can route to different tenants through headers; collapsing them selects the first entry's headers/configuration for the group.