Skip to content

fix(model-picker): add saved-model pre-menu and preserve list order - #24829

Open
imnotdev25 wants to merge 1 commit into
NousResearch:mainfrom
imnotdev25:fix/model-picker-auto-select-and-reorder
Open

fix(model-picker): add saved-model pre-menu and preserve list order#24829
imnotdev25 wants to merge 1 commit into
NousResearch:mainfrom
imnotdev25:fix/model-picker-auto-select-and-reorder

Conversation

@imnotdev25

Copy link
Copy Markdown

What does this PR do?

Fixes two behavioral issues in hermes model that prevented users from seeing the full model probe list and making an informed model selection for custom providers.

Related Issue

Fixes #5248

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

hermes_cli/main.py_model_flow_named_custom()

Before: If saved_model existed, immediately called _save_model_choice() and returned — no probing, no picker.

After: When a saved model exists, the user sees a choice menu:

-> Use saved model (llama-3.1-70b)
   Browse available models
   Cancel
  • "Use saved model" — quick re-activation (preserves the convenience)
  • "Browse available models" — probes the endpoint and shows the full model picker via _prompt_model_selection, with the saved model marked ← currently in use
  • When no model is saved, goes straight to probing (unchanged behavior)

This delegates model selection to the shared _prompt_model_selection helper instead of using an inline simple_term_menu, ensuring consistent UX across all providers.

hermes_cli/auth.py_prompt_model_selection()

Before: Reordered model list to put current_model at index 0, cursor on it:

ordered = []
if current_model and current_model in model_ids:
    ordered.append(current_model)  # moved to front
for mid in model_ids:
    if mid not in ordered:
        ordered.append(mid)
default_idx = 0  # cursor on the moved model

After: Preserves original list order, only deduplicates:

seen = set()
ordered = []
for mid in model_ids:
    if mid not in seen:
        seen.add(mid)
        ordered.append(mid)
default_idx = 0  # cursor at top of original list

The current model still gets the ← currently in use marker wherever it naturally appears — it's just no longer moved to the front.

How to Test

  1. Add a custom provider with a saved model to ~/.hermes/config.yaml:
custom_providers:
  - name: "My Local LLM"
    base_url: "http://localhost:8080/v1"
    model: "llama-3.1-70b"
  1. Run hermes model and select "My Local LLM"
  2. Expected: See a menu with "Use saved model (llama-3.1-70b)" and "Browse available models" options instead of immediate auto-selection
  3. Select "Browse available models" → verify models appear in probe order, not reordered
  4. Run hermes model and select any provider with models (e.g. OpenRouter) → verify model list is in its natural order with cursor at position 0, current model marked in-place

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard labels May 13, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the model-picker ordering concern. The order-preserving deduplication remains relevant on current main, but the saved-model flow needs rework.

Problems

  • The new default “Use saved model” path returns before probing (hermes_cli/main.py:3621-3647 in this PR). Current main deliberately probes saved-model custom providers in hermes_cli/model_setup_flows.py:1427-1431, and tests/hermes_cli/test_custom_provider_model_switch.py:119-142 requires that call.
  • The pre-menu adds simple_term_menu at hermes_cli/main.py:3590; AGENTS.md:1221-1226 requires new interactive menus to use hermes_cli/curses_ui.py.
  • The two-file diff adds no regression tests for ordering or the new interaction.

Suggested changes

  • Port only the order-preserving deduplication to current hermes_cli/auth.py with a picker-order regression test.
  • If retaining a saved-model choice, adapt it in hermes_cli/model_setup_flows.py without bypassing the endpoint probe, and use curses_radiolist.

Automated hermes-sweeper review.

Comment thread hermes_cli/main.py Outdated
return
if pre_idx == 0:
model_name = saved_model
# Activate immediately without probing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This early activation skips the probe at lines 3651-3655. Current main has a regression contract requiring saved-model custom providers to call fetch_api_models (tests/hermes_cli/test_custom_provider_model_switch.py:119-142); retain that probe rather than returning here.

Comment thread hermes_cli/main.py Outdated
" Cancel",
]
try:
from simple_term_menu import TerminalMenu

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use hermes_cli.curses_ui.curses_radiolist instead. AGENTS.md:1221-1226 prohibits introducing new simple_term_menu usage because of terminal rendering and key-handling failures.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
`_prompt_model_selection` hoisted `current_model` to index 0 and left the
cursor there. Two problems (NousResearch#5248):

- The provider's natural order (probe order / catalog order) was hidden, so
  users could not find models where they expected them.
- The cursor sat on the already-active model, so ENTER was a silent no-op
  re-selection of the model the user opened the picker to change away from.

Keep the caller's order and only deduplicate. `current_model` still gets its
"← currently in use" marker wherever it naturally lands, and the cursor now
starts at the top of the list.

The companion issue — named custom providers auto-selecting a saved model
without probing — is already fixed on main: `_model_flow_named_custom` in
`hermes_cli/model_setup_flows.py` always probes and renders the picker via
`curses_radiolist`. No change needed there.

Adds `tests/hermes_cli/test_model_picker_order.py` covering picker order,
deduplication, cursor position, and index-to-model mapping.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@imnotdev25
imnotdev25 force-pushed the fix/model-picker-auto-select-and-reorder branch from 9bd8789 to e862dab Compare July 29, 2026 14:16
@imnotdev25

Copy link
Copy Markdown
Author

Thanks for the review — all three points addressed. The branch was based on a main that had drifted ~9.4k commits, which is what produced both the conflict and the stale file references. Rebased onto current main and re-scoped.

Probe bypass / simple_term_menu — both resolved by dropping the main.py hunk entirely rather than reworking it. That change is obsolete on current main: _model_flow_named_custom now lives in hermes_cli/model_setup_flows.py and already does what the issue asked for — it always probes, renders through curses_radiolist, and pre-selects the saved model with a (current) marker. Nothing left to port, so there is no early return and no new simple_term_menu call site. test_saved_model_still_probes_endpoint passes unchanged.

Order-preserving deduplication — ported to hermes_cli/auth.py::_prompt_model_selection. It keeps the caller's order and dedupes via a set instead of the previous mid not in ordered list scan. current_model keeps its ← currently in use marker wherever it naturally lands, and the cursor stays at index 0 so ENTER is no longer a silent no-op re-selection of the model the user opened the picker to change away from.

Regression tests — added tests/hermes_cli/test_model_picker_order.py (5 tests): picker order preserved with current_model mid-list, dedup keeps first occurrence, cursor at top, selected index maps to the displayed row, and order preserved when current_model isn't in the list. Two of them fail against the pre-fix implementation, so they're actually pinning the behavior rather than passing vacuously.

The diff is now one file plus tests. 257 tests pass across every suite that touches _prompt_model_selection. The repo has some unrelated pre-existing failures in this environment (cron, kanban, mcp, update); I confirmed identical pass/fail counts with and without this change.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Model selection picker bypasses user choice — saved model auto-selected, list reordered unpredictably

3 participants