From 5fbe16635b8ff91ec9c620458ff0f35973af9969 Mon Sep 17 00:00:00 2001 From: Jorge Date: Fri, 17 Apr 2026 21:24:19 +0930 Subject: [PATCH] fix(cli): scroll the /model picker viewport so long catalogs aren't clipped The /model picker rendered every choice into a prompt_toolkit Window with no max height. Providers with many models (e.g. Ollama Cloud's 36+) overflowed the terminal, clipping the bottom border and the last items. - Add HermesCLI._compute_model_picker_viewport() to slide a scroll offset that keeps the cursor on screen, sized from the live terminal rows minus chrome reserved for input/status/border. - Render only the visible slice in _get_model_picker_display() and persist the offset on _model_picker_state across redraws. - Bind ESC (eager) to close the picker, matching the Cancel button. - Cover the viewport math with 8 unit tests in tests/hermes_cli/test_model_picker_viewport.py. --- cli.py | 53 +++++++++++++++- .../hermes_cli/test_model_picker_viewport.py | 63 +++++++++++++++++++ 2 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 tests/hermes_cli/test_model_picker_viewport.py diff --git a/cli.py b/cli.py index 85a7b508..9855327e 100644 --- a/cli.py +++ b/cli.py @@ -4514,6 +4514,32 @@ class HermesCLI: self._restore_modal_input_snapshot() self._invalidate(min_interval=0.0) + @staticmethod + def _compute_model_picker_viewport( + selected: int, + scroll_offset: int, + n: int, + term_rows: int, + chrome_reserve: int = 14, + ) -> tuple[int, int]: + """Resolve (scroll_offset, visible_count) for the /model picker panel. + + ``term_rows - chrome_reserve`` caps how many rows the panel may use for + items; when the list overflows we slide the offset to keep ``selected`` + on screen. The position counter sits in the bottom border, so no extra + row is reserved for it. + """ + max_visible = max(3, term_rows - chrome_reserve) + if n <= max_visible: + return 0, n + visible = max_visible + if selected < scroll_offset: + scroll_offset = selected + elif selected >= scroll_offset + visible: + scroll_offset = selected - visible + 1 + scroll_offset = max(0, min(scroll_offset, n - visible)) + return scroll_offset, visible + def _apply_model_switch_result(self, result, persist_global: bool) -> None: if not result.success: _cprint(f" ✗ {result.error_message}") @@ -8693,6 +8719,13 @@ class HermesCLI: state["selected"] = min(max_idx, state.get("selected", 0) + 1) event.app.invalidate() + @kb.add('escape', filter=Condition(lambda: bool(self._model_picker_state)), eager=True) + def model_picker_escape(event): + """ESC closes the /model picker.""" + self._close_model_picker() + event.app.current_buffer.reset() + event.app.invalidate() + # --- History navigation: up/down browse history in normal input mode --- # The TextArea is multiline, so by default up/down only move the cursor. # Buffer.auto_up/auto_down handle both: cursor movement when multi-line, @@ -9494,6 +9527,22 @@ class HermesCLI: box_width = _panel_box_width(title, [hint] + choices, min_width=46, max_width=84) inner_text_width = max(8, box_width - 6) + selected = state.get("selected", 0) + + # Scrolling viewport: the panel renders into a Window with no max + # height, so without limiting visible items the bottom border and + # any items past the available terminal rows get clipped on long + # provider catalogs (e.g. Ollama Cloud's 36+ models). + try: + from prompt_toolkit.application import get_app + term_rows = get_app().output.get_size().rows + except Exception: + term_rows = shutil.get_terminal_size((80, 24)).lines + scroll_offset, visible = HermesCLI._compute_model_picker_viewport( + selected, state.get("_scroll_offset", 0), len(choices), term_rows, + ) + state["_scroll_offset"] = scroll_offset + lines = [] lines.append(('class:clarify-border', '╭─ ')) lines.append(('class:clarify-title', title)) @@ -9501,8 +9550,8 @@ class HermesCLI: _append_blank_panel_line(lines, 'class:clarify-border', box_width) _append_panel_line(lines, 'class:clarify-border', 'class:clarify-hint', hint, box_width) _append_blank_panel_line(lines, 'class:clarify-border', box_width) - selected = state.get("selected", 0) - for idx, choice in enumerate(choices): + for idx in range(scroll_offset, scroll_offset + visible): + choice = choices[idx] style = 'class:clarify-selected' if idx == selected else 'class:clarify-choice' prefix = '❯ ' if idx == selected else ' ' for wrapped in _wrap_panel_text(prefix + choice, inner_text_width, subsequent_indent=' '): diff --git a/tests/hermes_cli/test_model_picker_viewport.py b/tests/hermes_cli/test_model_picker_viewport.py new file mode 100644 index 00000000..161d15ee --- /dev/null +++ b/tests/hermes_cli/test_model_picker_viewport.py @@ -0,0 +1,63 @@ +"""Tests for the prompt_toolkit /model picker scroll viewport. + +Regression for: when a provider exposes many models (e.g. Ollama Cloud's +36+), the picker rendered every choice into a Window with no max height, +clipping the bottom border and any items past the terminal's last row. +The viewport helper now caps visible items and slides the offset to keep +the cursor on screen. +""" +from cli import HermesCLI + + +_compute = HermesCLI._compute_model_picker_viewport + + +class TestPickerViewport: + def test_short_list_no_scroll(self): + offset, visible = _compute(selected=0, scroll_offset=0, n=5, term_rows=30) + assert offset == 0 + assert visible == 5 + + def test_long_list_caps_visible_to_chrome_budget(self): + # 36 models, 30 terminal rows, chrome_reserve=14 → max_visible=16. + # Position counter lives in the bottom border, no row reserved. + offset, visible = _compute(selected=0, scroll_offset=0, n=36, term_rows=30) + assert visible == 16 + assert offset == 0 + + def test_cursor_past_window_scrolls_down(self): + offset, visible = _compute(selected=20, scroll_offset=0, n=36, term_rows=30) + assert visible == 16 + assert 20 in range(offset, offset + visible) + + def test_cursor_above_window_scrolls_up(self): + offset, visible = _compute(selected=3, scroll_offset=15, n=36, term_rows=30) + assert offset == 3 + assert 3 in range(offset, offset + visible) + + def test_offset_clamped_to_bottom(self): + # Selected on last item — offset must keep visible window full, not + # walk past the end of the list. + offset, visible = _compute(selected=35, scroll_offset=0, n=36, term_rows=30) + assert offset + visible == 36 + assert 35 in range(offset, offset + visible) + + def test_tiny_terminal_uses_minimum_visible(self): + # term_rows below chrome_reserve falls back to the floor of 3 rows. + _, visible = _compute(selected=0, scroll_offset=0, n=20, term_rows=10) + assert visible == 3 # max(3, 10 - 14) == 3 + + def test_offset_recovers_after_stage_switch(self): + # When the user backs out of the model stage and re-enters with + # selected=0, a stale offset from the previous stage must collapse. + offset, visible = _compute(selected=0, scroll_offset=25, n=36, term_rows=30) + assert offset == 0 + assert 0 in range(offset, offset + visible) + + def test_full_navigation_keeps_cursor_visible(self): + offset = 0 + for cursor in list(range(36)) + list(range(35, -1, -1)): + offset, visible = _compute(cursor, offset, n=36, term_rows=30) + assert cursor in range(offset, offset + visible), ( + f"cursor={cursor} out of view: offset={offset} visible={visible}" + )