tui: friendlier model display + group same-endpoint providers in picker - #36998
tui: friendlier model display + group same-endpoint providers in picker#36998antydizajn wants to merge 2 commits into
Conversation
Two related TUI quality-of-life fixes for users running multiple models
behind a single proxy/aggregator (e.g. Palantir Foundry, Bedrock,
self-hosted vLLM behind a single key).
1. _get_status_bar_snapshot() — friendlier model name in the status bar.
Long catalog IDs (Palantir RIDs like
``ri.language-model-service..language-model.anthropic-claude-4-7-opus``)
were truncated to ``ri.language-model-ser...`` by the existing 26-char
slash-split, leaving the user with no way to tell which model is active.
The status bar now:
* Reverse-looks up the model id in config.yaml ``model_aliases:`` /
``model.aliases:`` and shows the shortest configured alias when one
exists (so users who set up a friendly alias get it for free).
* Falls back to stripping Palantir's ``ri.<service>..<ns>.`` RID prefix
before length-truncation, so the truncated label carries the actual
model identity (``anthropic-claude-4-7-opus``) instead of the URN
scheme.
* Reverse-alias map is cached at module level (config is loaded once
per session; no need to re-resolve on every status-bar refresh).
2. list_authenticated_providers() section 3 — group ``providers:`` entries
by (api_url, key_env, api_mode), mirroring section 4's existing grouping
for ``custom_providers:`` lists.
Before: a Palantir Foundry config with two Anthropic-proxy entries
(``palantir-claude46`` + ``palantir-claude47``) produced two near-
duplicate picker rows labelled ``Palantir Claude 4.6 Opus`` and
``Palantir Claude 4.7 Opus`` — same endpoint, same PALANTIR_TOKEN,
same anthropic_messages wire protocol, differing only by model id.
After: those entries collapse into a single ``Palantir Claude`` row
with both models in the dropdown. Same-host entries with a different
``api_mode`` (e.g. an OpenAI-compat ``palantir-gpt54`` alongside the
Anthropic claude rows on the same host) keep distinct rows since
the wire protocol differs — same safety invariant section 4 already
enforced for ``custom_providers:``.
Group display name strips per-version trailing tokens (``Palantir
Claude 4.7 Opus`` → ``Palantir Claude``) only when the prefix has
≥2 words, so single-word names aren't over-trimmed.
The new code records (raw_display_name, api_url) into
_section3_emitted_pairs for every raw entry that joined the group, so
section 4's compatibility-merged ``custom_providers`` view (built by
``get_compatible_custom_providers()`` which calls
``providers_dict_to_custom_providers()`` to convert ``providers:``
into custom-provider shape) still dedupes against this grouped row.
Manual smoke test on a config with three Palantir entries
(claude-4.6, claude-4.7, gpt-5.4): before — 3 picker rows; after — 2
picker rows (1 row "Palantir Claude" with 2 models, 1 row
"Palantir GPT-5.4" with 1 model).
…ch banner Address review on PR NousResearch#36998: the inline ri.<service>..<ns>. stripper in _get_status_bar_snapshot was a one-off heuristic that: * lived in cli.py with no shared call site, so the switch-confirmation banner ("✓ Model switched: ri.language-model-service..…") and the [Note: model was just switched from … to …] system-prompt nudge still printed the full opaque RID — exactly what the screenshot reported; * split on '..' and re-split on '.', which would mis-handle any RID whose namespace token isn't a single dotted segment. Refactor: * New module-level helper hermes_cli.model_switch.format_model_for_display matches on a startswith() allow-list (_OPAQUE_MODEL_PREFIXES) and returns the trailing slug. Falls through to the original string for every non-Palantir id, so HF paths (meta-llama/Llama-3.3-70B-Instruct), plain Claude/GPT names, .gguf paths, and aliased ids are untouched. Allow-list is extensible — add a prefix tuple entry for future proxies that wrap real names in a namespace (Bedrock ARNs are already covered by the slash-split fallback and have a different shape). * _get_status_bar_snapshot() now delegates to the shared helper after the reverse-alias miss (so configured aliases still win over the helper output). * cli.py::_handle_model_command — both confirmation-print blocks (~7720 and ~7975) now run result.new_model AND old_model through the formatter before they hit _cprint() and the _pending_model_switch_note text. * gateway/run.py model-switch handler (~10915) — same treatment for _pending_model_notes[_session_key] and the t('gateway.model.switched', model=…) confirmation line returned to the gateway client. The formatter is DISPLAY-ONLY. The session_model_overrides map, ModelSwitchResult.new_model, persistence to config.yaml, alias lookups, and every wire call still carry the full opaque RID — Palantir's API requires it. Verification: unit reproducer covers (a) all four Palantir model RIDs from this user's config stripped to the trailing slug, (b) plain model names (claude-4-7-opus-20260101, gpt-5.4, HF paths, empty string) passed through unchanged, (c) prefix-only edge preserved (no infinite-loop / empty-output regression). Refs: PR NousResearch#36998 review feedback; screenshot showed model banner still printing the long RID after the original status-bar-only fix landed.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real picker/status-bar usability gap. The current main behavior still has both premises: cli.py:4556-4560 truncates opaque IDs directly, and hermes_cli/model_switch.py:2017-2121 emits one section-3 row per configured provider.
Problems
- The proposed section-3 key at
hermes_cli/model_switch.py:1593omitsextra_headers. Current section 4 deliberately includes normalized headers in its key (hermes_cli/model_switch.py:2222-2231), because same URL/key/mode entries can be distinct tenant-routed endpoints. This would collapse them and use the first entry's configuration for discovery. - The manual list parsing at
hermes_cli/model_switch.py:1600-1607regresses current support formodels: [{"id": ...}]. Main centralizes that contract in_declared_model_ids()(hermes_cli/model_switch.py:55-100) and section 3 uses it at:2046.
Suggested changes
- Salvage the grouping atop current main, include normalized headers in its identity, and use
_declared_model_ids(). - Add focused section-3 regression tests for header-routed endpoints and list-of-dict model declarations.
Automated hermes-sweeper review.
| 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) |
There was a problem hiding this comment.
Please include normalized extra_headers in 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.
| if m and m not in entry_models: | ||
| entry_models.append(m) | ||
| elif isinstance(cfg_models, list): | ||
| for m in cfg_models: |
There was a problem hiding this comment.
Please aggregate via _declared_model_ids(cfg_models) instead. This loop appends dicts for supported models: [{"id": "..."}] declarations, while current main preserves their string IDs through the shared parser.
…ch banner Address review on PR #36998: the inline ri.<service>..<ns>. stripper in _get_status_bar_snapshot was a one-off heuristic that: * lived in cli.py with no shared call site, so the switch-confirmation banner ("✓ Model switched: ri.language-model-service..…") and the [Note: model was just switched from … to …] system-prompt nudge still printed the full opaque RID — exactly what the screenshot reported; * split on '..' and re-split on '.', which would mis-handle any RID whose namespace token isn't a single dotted segment. Refactor: * New module-level helper hermes_cli.model_switch.format_model_for_display matches on a startswith() allow-list (_OPAQUE_MODEL_PREFIXES) and returns the trailing slug. Falls through to the original string for every non-Palantir id, so HF paths (meta-llama/Llama-3.3-70B-Instruct), plain Claude/GPT names, .gguf paths, and aliased ids are untouched. Allow-list is extensible — add a prefix tuple entry for future proxies that wrap real names in a namespace (Bedrock ARNs are already covered by the slash-split fallback and have a different shape). * _get_status_bar_snapshot() now delegates to the shared helper after the reverse-alias miss (so configured aliases still win over the helper output). * cli.py::_handle_model_command — both confirmation-print blocks (~7720 and ~7975) now run result.new_model AND old_model through the formatter before they hit _cprint() and the _pending_model_switch_note text. * gateway/run.py model-switch handler (~10915) — same treatment for _pending_model_notes[_session_key] and the t('gateway.model.switched', model=…) confirmation line returned to the gateway client. The formatter is DISPLAY-ONLY. The session_model_overrides map, ModelSwitchResult.new_model, persistence to config.yaml, alias lookups, and every wire call still carry the full opaque RID — Palantir's API requires it. Verification: unit reproducer covers (a) all four Palantir model RIDs from this user's config stripped to the trailing slug, (b) plain model names (claude-4-7-opus-20260101, gpt-5.4, HF paths, empty string) passed through unchanged, (c) prefix-only edge preserved (no infinite-loop / empty-output regression). Refs: PR #36998 review feedback; screenshot showed model banner still printing the long RID after the original status-bar-only fix landed.
- extra_headers participates in the section-3 group identity (mirrors
section 4 — header-routed tenants behind one proxy URL stay distinct)
- model declarations go through _declared_model_ids() so
models: [{id: ...}] rows keep working
- gateway model-switch handler moved to gateway/slash_commands.py since
the PR branched — re-applied the display-form edits there (both the
legacy picker closure and the current typed path)
- regression tests: same-endpoint fold, api_mode separation,
header-routed separation, list-of-dict models, RID display stripping
|
Merged via PR #67908 — both of your commits were cherry-picked onto current main with your authorship preserved in git log. Two follow-ups were applied on top per the earlier review: the section-3 group key now includes normalized |
…ch banner Address review on PR NousResearch#36998: the inline ri.<service>..<ns>. stripper in _get_status_bar_snapshot was a one-off heuristic that: * lived in cli.py with no shared call site, so the switch-confirmation banner ("✓ Model switched: ri.language-model-service..…") and the [Note: model was just switched from … to …] system-prompt nudge still printed the full opaque RID — exactly what the screenshot reported; * split on '..' and re-split on '.', which would mis-handle any RID whose namespace token isn't a single dotted segment. Refactor: * New module-level helper hermes_cli.model_switch.format_model_for_display matches on a startswith() allow-list (_OPAQUE_MODEL_PREFIXES) and returns the trailing slug. Falls through to the original string for every non-Palantir id, so HF paths (meta-llama/Llama-3.3-70B-Instruct), plain Claude/GPT names, .gguf paths, and aliased ids are untouched. Allow-list is extensible — add a prefix tuple entry for future proxies that wrap real names in a namespace (Bedrock ARNs are already covered by the slash-split fallback and have a different shape). * _get_status_bar_snapshot() now delegates to the shared helper after the reverse-alias miss (so configured aliases still win over the helper output). * cli.py::_handle_model_command — both confirmation-print blocks (~7720 and ~7975) now run result.new_model AND old_model through the formatter before they hit _cprint() and the _pending_model_switch_note text. * gateway/run.py model-switch handler (~10915) — same treatment for _pending_model_notes[_session_key] and the t('gateway.model.switched', model=…) confirmation line returned to the gateway client. The formatter is DISPLAY-ONLY. The session_model_overrides map, ModelSwitchResult.new_model, persistence to config.yaml, alias lookups, and every wire call still carry the full opaque RID — Palantir's API requires it. Verification: unit reproducer covers (a) all four Palantir model RIDs from this user's config stripped to the trailing slug, (b) plain model names (claude-4-7-opus-20260101, gpt-5.4, HF paths, empty string) passed through unchanged, (c) prefix-only edge preserved (no infinite-loop / empty-output regression). Refs: PR NousResearch#36998 review feedback; screenshot showed model banner still printing the long RID after the original status-bar-only fix landed.
- extra_headers participates in the section-3 group identity (mirrors
section 4 — header-routed tenants behind one proxy URL stay distinct)
- model declarations go through _declared_model_ids() so
models: [{id: ...}] rows keep working
- gateway model-switch handler moved to gateway/slash_commands.py since
the PR branched — re-applied the display-form edits there (both the
legacy picker closure and the current typed path)
- regression tests: same-endpoint fold, api_mode separation,
header-routed separation, list-of-dict models, RID display stripping
What
Two related TUI quality-of-life fixes for users running multiple models behind a single proxy/aggregator endpoint (e.g. Palantir Foundry, Bedrock, self-hosted vLLM behind a single key).
1. Friendlier active-model name in the status bar
Long catalog IDs (Palantir RIDs like
ri.language-model-service..language-model.anthropic-claude-4-7-opus) were truncated tori.language-model-ser…by the existing 26-char slash-split path in_get_status_bar_snapshot(), leaving no useful identity in the status bar.The status bar now:
model_aliases:/model.aliases:and shows the shortest configured alias when one exists. So a user who setopus47as an alias getsopus47in the status bar for free.ri.<service>..<ns>.RID prefix before length-truncation, so the truncated label carries the actual model identity (anthropic-claude-4-7-opus) instead of the URN scheme.2. Group same-endpoint
providers:entries in the model pickerlist_authenticated_providers()section 3 now groupsproviders:entries by(api_url, key_env, api_mode), mirroring section 4's existing grouping forcustom_providers:lists.Before: a Palantir Foundry config with two Anthropic-proxy entries (
palantir-claude46+palantir-claude47) produced two near-duplicate picker rows labelled "Palantir Claude 4.6 Opus" and "Palantir Claude 4.7 Opus" — same endpoint, samePALANTIR_TOKEN, sameanthropic_messageswire protocol, differing only by model id.After: those entries collapse into a single "Palantir Claude" row with both models in the dropdown.
Same-host entries with a different
api_mode(e.g. an OpenAI-compatpalantir-gpt54alongside the Anthropic claude rows on the same host) keep distinct rows since the wire protocol differs — the same safety invariant section 4 already enforced forcustom_providers:.Group display name strips per-version trailing tokens ("Palantir Claude 4.7 Opus" → "Palantir Claude") only when the prefix has ≥2 words, so single-word names aren't over-trimmed.
The new code records
(raw_display_name, api_url)into_section3_emitted_pairsfor every raw entry that joined a group, so section 4's compatibility-mergedcustom_providersview (built byget_compatible_custom_providers()callingproviders_dict_to_custom_providers()) still dedupes against the grouped row.Why
Hermes already groups
custom_providers:entries by endpoint+credential+protocol (section 4). The same shape exists inproviders:(the keyed schema), but section 3 emitted one row per key, so any user with multiple models behind a single proxy hit a polluted picker. This brings sections 3 and 4 to parity.The status-bar truncation made the long-RID problem worse because the shown prefix carries no per-model identity at all.
Manual smoke test
Local config with three Palantir entries (
palantir-gpt54,palantir-claude46,palantir-claude47):api_modediffers).Status bar with
provider=palantir-claude47,model=ri.language-model-service..language-model.anthropic-claude-4-7-opus:⚕ ri.language-model-ser…⚕ anthropic-claude-4-7-opus(or any user-configured short alias ifmodel_aliases:is set).No behaviour change for users with the simple case
Single-entry
providers:rows (one model per provider key) keep their existing label and slug — they're a single-member group and the per-version-token strip heuristic only kicks in when the prefix has ≥2 words.