diff --git a/hermes_cli/cli_tui_mixin.py b/hermes_cli/cli_tui_mixin.py index 27b668f0f4..4dc8b363e4 100644 --- a/hermes_cli/cli_tui_mixin.py +++ b/hermes_cli/cli_tui_mixin.py @@ -631,7 +631,10 @@ class CLITuiMixin: panel.blank() for idx in range(scroll_offset, min(scroll_offset + visible, len(labels))): style = 'class:clarify-selected' if idx == selected else 'class:clarify-choice' - lead = '❯ ' if idx == selected else indent + # The cursor cell is always two columns wide, so unselected rows get two spaces + # regardless of ``indent`` (the palette's continuation indent is four) — otherwise + # the selected label starts two columns left of its neighbours. + lead = '❯ ' if idx == selected else ' ' for wrapped in _prefix_wrapped_rows( _wrap_panel_text, labels[idx], label_width, lead, indent ): diff --git a/tests/hermes_cli/test_scroll_list_panel_row_layout.py b/tests/hermes_cli/test_scroll_list_panel_row_layout.py index 38c7941b36..b7f426fd64 100644 --- a/tests/hermes_cli/test_scroll_list_panel_row_layout.py +++ b/tests/hermes_cli/test_scroll_list_panel_row_layout.py @@ -1,139 +1,60 @@ -"""Regression: long labels mis-rendered in the scrollable list panel. +"""Invariants for ``CLITuiMixin._render_scroll_list_panel`` (the ``/model`` picker's model +stage and the command palette) with labels long enough to fill the panel body. -Covers ``CLITuiMixin._render_scroll_list_panel`` (the ``/model`` picker's model -stage and the command palette) and its ``_prefix_wrapped_rows`` helper. - -Symptom, with a provider whose model ids are long — e.g. MLX Core's -``peculiar-ragdoll/Cyber-Tiel-Coder-35B-A3B-MLX-oQ4e-MTP`` (54 chars): - -* moving the cursor onto such a row left ``❯`` alone on its own line, with the - model name pushed to the row below; and -* unselected long rows lost their leading indent and rendered flush against the - panel border, out of alignment with every other row. - -Cause: the 2-char cursor/indent prefix was concatenated onto the label *before* -wrapping, so it was charged against the label's width budget, and the wrapper's -whitespace trimming removed the leading indent. +The prefix (cursor or indent) must never be charged against the label's wrap budget: a 54-char +model id such as MLX Core's ``peculiar-ragdoll/Cyber-Tiel-Coder-35B-A3B-MLX-oQ4e-MTP`` used to +strand ``❯`` on a row of its own when selected, and lose its indent when not. """ -import pytest - from cli import _panel_box_width -from hermes_cli.cli_tui_mixin import CLITuiMixin, _prefix_wrapped_rows +from hermes_cli.cli_tui_mixin import CLITuiMixin - -# A real long model id from the MLX Core (LAN) provider. LONG_LABEL = "peculiar-ragdoll/Cyber-Tiel-Coder-35B-A3B-MLX-oQ4e-MTP" -SHORT_LABELS = ["MiniMax-M3", "MiniMax-M2.7", "MiniMax-M2.1"] PICKER_TITLE = "⚙ Model Picker — MLX Core (LAN)" +HINT = "Select a model (7 available) — type to filter" class _Host(CLITuiMixin): - """Minimal host: the renderer only touches `state` + module helpers.""" + """Minimal host: the renderer only touches ``state`` + module helpers.""" -def _panel_rows(fragments): - """Reconstruct the padded body text of each bordered panel row.""" +def _label_rows(fragments, hint): text = "".join(t for _style, t in fragments) - rows = [] - for line in text.split("\n"): - if line.startswith("│") and line.rstrip().endswith("│"): - rows.append(line[2:-2]) - return rows + rows = [line[2:-2] for line in text.split("\n") if line.startswith("│ ") and line.endswith(" │")] + return [r for r in rows if r.strip() and not r.startswith(hint)] -def _label_rows(fragments): - """Panel rows that carry a list label (skip blanks and the hint row).""" - return [r for r in _panel_rows(fragments) if r.strip() and not r.startswith("Select a model")] +def _render(labels, selected, *, indent=" ", min_width=46, max_width=84, title=PICKER_TITLE, hint=HINT): + frags = _Host()._render_scroll_list_panel( + {"selected": selected}, title, hint, labels, min_width=min_width, max_width=max_width, indent=indent) + return _label_rows(frags, hint) -def _render(labels, selected=0, indent=" ", title=PICKER_TITLE, hint="Select a model"): - host = _Host() - return host._render_scroll_list_panel( - {"selected": selected}, title, hint, labels, - min_width=46, max_width=84, indent=indent, - ) +def test_long_model_id_stays_on_the_cursor_row_and_keeps_its_indent(): + labels = ["root4k/Huihui-Qwen3.6-35B-A3B-abliterated-oQ4e-mtp", LONG_LABEL, "← Back", "Cancel"] + body = _panel_box_width(PICKER_TITLE, [HINT] + labels, min_width=46, max_width=84) - 2 + + selected = [r.rstrip() for r in _render(labels, 1)] + assert selected[1] == f"❯ {LONG_LABEL}", selected # cursor and full label on ONE row + unselected = [r.rstrip() for r in _render(labels, 0)] + assert unselected[1] == f" {LONG_LABEL}", unselected # indent kept, aligned with neighbours + assert unselected[0] == f"❯ {labels[0]}" + for rows in (selected, unselected): + assert len(rows) == len(labels) + assert all(len(r) <= body for r in rows), rows -def _box_width(title, labels, hint): - return _panel_box_width(title, [hint] + labels, min_width=46, max_width=84) +def test_palette_rows_align_on_the_cursor_cell_and_wrap_with_its_indent(): + hint = "Type to filter 3 commands — ↑/↓ then Enter inserts, Esc cancels" + long_label = "/very-long-command-name — " + "x" * 60 + labels = ["/model — Switch the active model", long_label, "/help — Help"] + rows = [r.rstrip() for r in _render(labels, 1, indent=" ", min_width=50, max_width=90, + title="⚙ Command Palette", hint=hint)] + assert rows[0] == f" {labels[0]}" # unselected: two-column lead, same column as the cursor row + assert rows[1].startswith("❯ /very-long-command-name") + assert rows[2] == " " + "x" * 60 # continuation row carries the palette's 4-space indent + assert rows[3] == f" {labels[2]}" - -# --------------------------------------------------------------------------- -# Helper contract -# --------------------------------------------------------------------------- - -class TestPrefixWrappedRows: - def test_prefix_is_not_charged_against_the_wrap_width(self): - wrap = lambda text, width: [text[:width], text[width:]] if len(text) > width else [text] - rows = _prefix_wrapped_rows(wrap, "abcdefgh", 8, "❯ ", " ") - assert rows == ["❯ abcdefgh"] - - def test_continuation_rows_use_the_indent(self): - # A wrapper that splits into two rows of 4 chars. - wrap = lambda text, width: [text[i:i + 4] for i in range(0, len(text), 4)] - rows = _prefix_wrapped_rows(wrap, "abcdefgh", 4, "❯ ", " ") - assert rows == ["❯ abcd", " efgh"] - - def test_first_prefix_only_applies_to_the_first_row(self): - wrap = lambda text, width: ["a", "b", "c"] - rows = _prefix_wrapped_rows(wrap, "x", 4, "❯ ", " ") - assert rows == ["❯ a", " b", " c"] - - -# --------------------------------------------------------------------------- -# Real renderer: long labels -# --------------------------------------------------------------------------- - -class TestLongLabelRendering: - def test_selected_long_label_keeps_cursor_and_name_on_one_row(self): - rows = _label_rows(_render([LONG_LABEL], selected=0)) - assert len(rows) == 1, f"label split across rows: {rows}" - assert rows[0].startswith("❯ ") - assert rows[0][2:].strip() == LONG_LABEL - - def test_unselected_long_label_keeps_its_indent(self): - labels = [SHORT_LABELS[0], LONG_LABEL, SHORT_LABELS[1]] - rows = _label_rows(_render(labels, selected=0)) - long_row = next(r for r in rows if LONG_LABEL in r) - assert long_row.startswith(" "), repr(long_row) - assert not long_row.startswith("❯"), repr(long_row) - - @pytest.mark.parametrize("selected", [0, 1, 2]) - def test_every_row_keeps_a_two_column_lead(self, selected): - labels = [SHORT_LABELS[0], LONG_LABEL, SHORT_LABELS[1]] - for row in _label_rows(_render(labels, selected=selected)): - assert row[:2] in ("❯ ", " "), repr(row) - - def test_no_row_exceeds_the_panel_body(self): - labels = [LONG_LABEL] + SHORT_LABELS + ["← Back", "Cancel"] - body = _box_width(PICKER_TITLE, labels, "Select a model") - 2 - for selected in range(len(labels)): - for row in _label_rows(_render(labels, selected=selected)): - assert len(row) <= body, repr(row) - - -class TestShortLabelsUnchanged: - def test_short_labels_render_with_two_column_lead(self): - # Rows are ljust-padded to the panel body width by _Panel.row. - rows = [r.rstrip() for r in - _label_rows(_render(SHORT_LABELS + ["← Back", "Cancel"], selected=1))] - assert rows[0] == " MiniMax-M3" - assert rows[1] == "❯ MiniMax-M2.7" - assert rows[-2] == " ← Back" - assert rows[-1] == " Cancel" - - -class TestPaletteIndent: - """The palette uses a 4-space indent instead of the picker's 2.""" - - def test_long_palette_label_keeps_cursor_and_text_on_one_row(self): - label = "a" * 60 - rows = _label_rows(_render([label], selected=0, indent=" ")) - assert len(rows) == 1, rows - assert rows[0].startswith("❯ ") - - def test_palette_rows_respect_the_wider_indent(self): - label = "alpha beta gamma delta epsilon zeta eta theta iota kappa lambda mu" - rows = _label_rows(_render([label], selected=0, indent=" ")) - for row in rows: - assert row.startswith(("❯ ", " ")), repr(row) + token = "/" + "y" * 69 # one unbreakable token that fills the body: cursor must stay on its row + rows = [r.rstrip() for r in _render([token, "/help — Help"], 0, indent=" ", min_width=50, + max_width=90, title="⚙ Command Palette", hint=hint)] + assert rows == [f"❯ {token}", " /help — Help"], rows