diff --git a/EvoScientist/cli/tui_interactive.py b/EvoScientist/cli/tui_interactive.py index 3523970..f784907 100644 --- a/EvoScientist/cli/tui_interactive.py +++ b/EvoScientist/cli/tui_interactive.py @@ -316,6 +316,159 @@ def _stopped_response_after_narration( return display_current, display_stopped, full_stopped +# (kind, payload, item_index): kind is "header"/"sep"/"item"; payload is the +# category name for headers or the candidate for items; item_index is the +# candidate's position in the source list (-1 for non-item rows). +_CompletionRow = tuple[str, Any, int] + + +def _build_completion_rows(items: list[Any]) -> list[_CompletionRow]: + """Flatten completion candidates into render rows with category headers.""" + rows: list[_CompletionRow] = [] + last_cat = "" + for i, candidate in enumerate(items): + cat = getattr(candidate, "category", "") + if cat and cat != last_cat: + if last_cat: + rows.append(("sep", "", -1)) + rows.append(("header", cat, -1)) + last_cat = cat + rows.append(("item", candidate, i)) + return rows + + +def _window_completion_rows( + rows: list[_CompletionRow], + selected: int, + max_rows: int, +) -> tuple[list[_CompletionRow], int, int]: + """Slice *rows* to a window of at most *max_rows* total display lines. + + The window always contains the selected item (top of the list when + nothing is selected) and reserves one line per overflow indicator. + Returns ``(visible_rows, hidden_items_above, hidden_items_below)``. + """ + max_rows = max(max_rows, 5) + if len(rows) <= max_rows: + return list(rows), 0, 0 + + sel_row = 0 + if selected >= 0: + for r, (kind, _payload, idx) in enumerate(rows): + if kind == "item" and idx == selected: + sel_row = r + break + + # Center the selection; centering keeps it clear of the indicator + # lines that replace the window's edge rows when content is clipped. + start = min(max(sel_row - max_rows // 2, 0), len(rows) - max_rows) + end = start + max_rows + content_start = start + (1 if start > 0 else 0) + content_end = end - (1 if end < len(rows) else 0) + + above = sum(1 for kind, _p, _i in rows[:content_start] if kind == "item") + below = sum(1 for kind, _p, _i in rows[content_end:] if kind == "item") + return rows[content_start:content_end], above, below + + +def _render_completion_text(items: list[Any], selected: int, max_rows: int) -> Text: + """Render the completion popup content bounded to *max_rows* lines.""" + rows = _build_completion_rows(items) + visible, above, below = _window_completion_rows(rows, selected, max_rows) + + # Blank separator lines are cosmetic — drop them at the window edges. + while visible and visible[0][0] == "sep": + visible = visible[1:] + while visible and visible[-1][0] == "sep": + visible = visible[:-1] + + lines: list[Text] = [] + if above: + lines.append(Text(f" ↑ {above} more", style="dim italic")) + for kind, payload, idx in visible: + if kind == "sep": + lines.append(Text()) + elif kind == "header": + lines.append(Text(f" {payload}", style="bold #6b7280")) + elif idx == selected: + lines.append( + Text.assemble( + (" ▸ ", "bold"), + (f"{payload.text:<28}", "bold"), + (payload.description, "bold"), + ) + ) + else: + lines.append( + Text.assemble( + (" ", "#888888"), + (f"{payload.text:<28}", "#888888"), + (payload.description, "#888888"), + ) + ) + if below: + lines.append(Text(f" ↓ {below} more", style="dim italic")) + return Text("\n").join(lines) + + +# Hard cap on popup lines so the popup never dwarfs the chat area +# (mainstream CLI behavior); matches the pre-#354 max-height. +_COMPLETION_MAX_VISIBLE_ROWS = 15 +# Rows kept free for the input row, status bar and a slice of chat. On +# terminals shorter than ~17 rows the 5-row floor wins over this +# reservation — a smaller popup would be unusable. +_COMPLETION_RESERVED_ROWS = 12 + + +def _completion_row_budget(height: int) -> int: + """Popup line budget for a terminal of *height* rows.""" + if height <= 0: + return _COMPLETION_MAX_VISIBLE_ROWS + return max(5, min(height - _COMPLETION_RESERVED_ROWS, _COMPLETION_MAX_VISIBLE_ROWS)) + + +# Textual converts rich Text to Content and drops rich no_wrap/overflow +# attributes, so line cropping must be enforced here in CSS. +_COMPLETIONS_CSS = """ +#completions { + display: none; + height: auto; + background: #1e1f26; + padding: 0 1; + border-bottom: solid #0284c7; + text-wrap: nowrap; + text-overflow: ellipsis; +} +""" + + +def _normalize_chat_scroll(container: Any) -> None: + """Repair the chat scroll state after the popup resized the viewport. + + Textual's compositor recomputes ``scroll_y`` for anchored containers + bypassing the validator, so when the popup hides and the content fits + again, ``scroll_y`` can go negative — the scrollbar then renders as if + scrolled to the bottom while the content sits at the top (issue #301 + family). Runs after refresh so sizes are current. + """ + # force=True: with the content fitting, the scrollbar is hidden and + # allow_vertical_scroll is False — an unforced scroll_home would + # silently no-op and leave the negative scroll_y in place. + if container.is_anchored: + if container.max_scroll_y <= 0: + container.anchor(False) + container.scroll_home(animate=False, immediate=True, force=True) + elif container.scroll_y < 0: + container.scroll_home(animate=False, immediate=True, force=True) + # Resync the scrollbar thumb: watch_scroll_y skips the update while + # the scrollbar is hidden (or when the compositor wrote scroll_y via + # set_reactive), so a stale position survives until the scrollbar + # reappears — rendering as "scrolled to bottom" at the top. + scrollbar = getattr(container, "vertical_scrollbar", None) + if scrollbar is not None and scrollbar.position != container.scroll_y: + scrollbar.position = container.scroll_y + + def run_textual_interactive( *, show_thinking: bool, @@ -384,7 +537,8 @@ def run_textual_interactive( def supports_interactive(self) -> bool: return True - CSS = """ + CSS = ( + """ Screen { layout: vertical; background: #16161a; @@ -436,14 +590,9 @@ def run_textual_interactive( padding: 0 2; color: #9ca3af; } - #completions { - display: none; - height: auto; - max-height: 15; - background: #1e1f26; - padding: 0 1; - border-bottom: solid #0284c7; - } + """ + + _COMPLETIONS_CSS + + """ #status { height: 1; min-height: 1; @@ -452,6 +601,7 @@ def run_textual_interactive( padding: 0 1; } """ + ) BINDINGS: ClassVar[list[Binding]] = [ Binding("ctrl+c", "request_quit", "Quit", show=False, priority=True), Binding("ctrl+v", "paste_clipboard", "Paste", show=False), @@ -501,6 +651,7 @@ def run_textual_interactive( ] = [] # queued messages to send after current turn self._comp_items: list = [] self._comp_index: int = -1 + self._comp_last_height: int = 0 self._comp_base: str = "" self._hitl_auto_approve: bool = False self._approval_future: asyncio.Future | None = None @@ -912,6 +1063,17 @@ def run_textual_interactive( self._background_tasks.add(ch_task) ch_task.add_done_callback(self._background_tasks.discard) + def on_resize(self, event: Any) -> None: + """Re-window the completion popup for the new terminal height.""" + try: + comp_widget = self.query_one("#completions", Static) + except Exception: + return + if comp_widget.display and self._comp_items: + # Deferred: this handler can run before the base App + # handler updates self.size with the new dimensions. + self.call_after_refresh(self._render_completions) + # ── Update check ────────────────────────────────────── async def _check_for_updates(self) -> None: @@ -1149,7 +1311,9 @@ def run_textual_interactive( go negative and pushes the welcome banner out of view (issue #301). """ container.anchor(False) - container.scroll_home(animate=False, immediate=True) + # force=True: with content fitting, the scrollbar is hidden and + # allow_vertical_scroll is False — unforced scroll_home no-ops. + container.scroll_home(animate=False, immediate=True, force=True) def _append_system(self, text: str, style: str = "dim") -> None: """Mount a SystemMessage widget into #chat.""" @@ -2858,30 +3022,36 @@ def run_textual_interactive( def _hide_completions(self) -> None: self._comp_items = [] self._comp_index = -1 - self.query_one("#completions", Static).display = False + self._comp_last_height = 0 + comp_widget = self.query_one("#completions", Static) + was_visible = comp_widget.display + comp_widget.display = False + # Called on every ordinary input change — only a popup that was + # actually visible changed the chat viewport. + if was_visible: + self.call_after_refresh(self._normalize_chat_after_popup) + + def _completion_max_rows(self) -> int: + return _completion_row_budget(int(getattr(self.size, "height", 0) or 0)) def _render_completions(self) -> None: - comp_text = Text() - last_cat = "" - for i, candidate in enumerate(self._comp_items): - cmd, desc = candidate.text, candidate.description - cat = getattr(candidate, "category", "") - if cat and cat != last_cat: - if last_cat: - comp_text.append("\n") - comp_text.append(f" {cat}\n", style="bold #6b7280") - last_cat = cat - if i == self._comp_index: - comp_text.append(" \u25b8 ", style="bold") - comp_text.append(f"{cmd:<28}", style="bold") - comp_text.append(desc, style="bold") - else: - comp_text.append(" ", style="#888888") - comp_text.append(f"{cmd:<28}", style="#888888") - comp_text.append(desc, style="#888888") - if i < len(self._comp_items) - 1: - comp_text.append("\n") + comp_text = _render_completion_text( + self._comp_items, self._comp_index, self._completion_max_rows() + ) self.query_one("#completions", Static).update(comp_text) + # Selection-only navigation keeps the height — skip the (cheap + # but per-keystroke) normalize unless the viewport can change. + n_lines = len(comp_text.plain.splitlines()) if comp_text.plain else 0 + if n_lines != self._comp_last_height: + self._comp_last_height = n_lines + self.call_after_refresh(self._normalize_chat_after_popup) + + def _normalize_chat_after_popup(self) -> None: + try: + container = self.query_one("#chat", VerticalScroll) + except Exception: + return + _normalize_chat_scroll(container) # ── Slash commands ───────────────────────────────────── diff --git a/EvoScientist/cli/widgets/mcp_browser.py b/EvoScientist/cli/widgets/mcp_browser.py index 9efda52..a4f0ec0 100644 --- a/EvoScientist/cli/widgets/mcp_browser.py +++ b/EvoScientist/cli/widgets/mcp_browser.py @@ -1,6 +1,7 @@ """Inline MCP server browser widget for /install-mcp in TUI. -Two-phase keyboard-driven widget (mirrors SkillBrowserWidget): +Two-phase keyboard-driven widget built on the shared picker engine +(``picker_base.TagCheckboxBrowserBase``): Phase 1 — tag picker (arrow keys + Enter to select, or Esc for all) Phase 2 — server checkbox (arrow keys to navigate, Space to toggle, Enter to confirm) @@ -12,73 +13,20 @@ from __future__ import annotations from typing import TYPE_CHECKING, Any, ClassVar -from rich.text import Text -from textual.binding import Binding, BindingType -from textual.containers import Container from textual.message import Message -from textual.widget import Widget -from textual.widgets import Static + +from .picker_base import TagCheckboxBrowserBase if TYPE_CHECKING: - from textual import events - from textual.app import ComposeResult - from ...mcp.registry import MCPServerEntry -class MCPBrowserWidget(Widget): - """Inline MCP server browser — mounts in chat, keyboard-driven. +class MCPBrowserWidget(TagCheckboxBrowserBase): + """Inline MCP server browser — mounts in chat, keyboard-driven.""" - Phase 1: Tag picker (select a tag filter or "All"). - Phase 2: Server checkbox (toggle servers, confirm to install). - """ - - can_focus = True - can_focus_children = False - - DEFAULT_CSS = """ - MCPBrowserWidget { - height: auto; - max-height: 30; - margin: 1 0; - padding: 0 1; - background: $surface; - border: solid $primary; - } - MCPBrowserWidget .browser-title { - height: 1; - text-style: bold; - color: $primary; - } - MCPBrowserWidget .browser-rows { - height: auto; - max-height: 20; - overflow-y: auto; - } - MCPBrowserWidget .browser-row { - height: 1; - padding: 0 1; - } - MCPBrowserWidget .browser-row-selected { - background: $primary; - text-style: bold; - } - MCPBrowserWidget .browser-help { - height: 1; - color: $text-muted; - text-style: italic; - } - """ - - BINDINGS: ClassVar[list[BindingType]] = [ - Binding("up", "move_up", "Up", show=False), - Binding("k", "move_up", "Up", show=False), - Binding("down", "move_down", "Down", show=False), - Binding("j", "move_down", "Down", show=False), - Binding("enter", "confirm", "Confirm", show=False), - Binding("space", "toggle", "Toggle", show=False), - Binding("escape", "cancel", "Cancel", show=False), - ] + _INSTALLED_SUFFIX: ClassVar[str] = " (configured)" + _PHASE2_TITLE: ClassVar[str] = "Select MCP servers to install" + _PHASE2_CONFIRM_LABEL: ClassVar[str] = "install" class Confirmed(Message): """Posted when user confirms server selection.""" @@ -90,242 +38,17 @@ class MCPBrowserWidget(Widget): class Cancelled(Message): """Posted when user cancels.""" - def __init__( - self, - servers: list[MCPServerEntry], - installed_names: set[str], - *, - pre_filter_tag: str = "", - **kwargs: Any, - ) -> None: - super().__init__(**kwargs) - self._servers = servers - self._installed_names = installed_names - self._pre_filter_tag = pre_filter_tag.lower() - self._selected = 0 - self._row_widgets: list[Static] = [] - self._title_widget: Static | None = None - self._help_widget: Static | None = None + def _item_name(self, item: Any) -> str: + return item.name - # Phase 1: tag picker - # Phase 2: server checkbox - self._phase: int = 1 - self._tag_items: list[tuple[str, int]] = [] - self._server_items: list[MCPServerEntry] = [] - self._checked: set[int] = set() + def _item_tags(self, item: Any) -> list[str]: + return item.tags - # Build tag list - from collections import Counter + def _item_desc(self, item: Any) -> str: + return item.description or item.label - tag_counter: Counter[str] = Counter() - for s in self._servers: - for t in s.tags: - tag_counter[t.lower()] += 1 - sorted_tags = sorted(tag_counter.items(), key=lambda x: (-x[1], x[0])) - self._tag_items = [("all", len(self._servers)), *sorted_tags] + def _post_confirmed(self, items: list[Any]) -> None: + self.post_message(self.Confirmed(items)) - # If pre-filtered, skip to phase 2 - if self._pre_filter_tag: - self._server_items = [ - s - for s in self._servers - if self._pre_filter_tag in [t.lower() for t in s.tags] - ] - if self._server_items: - self._phase = 2 - else: - self._pre_filter_tag = "" - - def compose(self) -> ComposeResult: - self._title_widget = Static("", classes="browser-title") - yield self._title_widget - with Container(classes="browser-rows"): - max_rows = max(len(self._tag_items), len(self._servers)) - for _ in range(max_rows): - widget = Static("", classes="browser-row") - self._row_widgets.append(widget) - yield widget - self._help_widget = Static("", classes="browser-help") - yield self._help_widget - - def on_mount(self) -> None: - self.call_after_refresh(self._update_display) - self.call_later(self.focus) - - def _update_display(self) -> None: - if self._phase == 1: - self._render_tag_picker() - else: - self._render_server_checkbox() - - def _render_tag_picker(self) -> None: - if self._title_widget: - self._title_widget.update("Filter by tag:") - if self._help_widget: - self._help_widget.update( - "\u2191/\u2193 navigate \u00b7 Enter select \u00b7 Esc cancel" - ) - - for i, widget in enumerate(self._row_widgets): - if i < len(self._tag_items): - tag, count = self._tag_items[i] - is_selected = i == self._selected - text = Text() - cursor = "\u25b8 " if is_selected else " " - text.append(cursor, style="bold cyan" if is_selected else "dim") - label = f"{tag} ({count})" - text.append(label, style="bold" if is_selected else "") - widget.update(text) - widget.display = True - widget.remove_class("browser-row-selected") - if is_selected: - widget.add_class("browser-row-selected") - widget.scroll_visible() - else: - widget.update("") - widget.display = False - - def _row_content_width(self) -> int: - try: - w = self.size.width - if w > 0: - return w - 6 - except Exception: - pass - try: - return self.app.size.width - 10 - except Exception: - return 100 - - def _truncate(self, desc: str, name: str, *, suffix: str = "") -> str: - overhead = 2 + 2 + len(name) + 3 + len(suffix) - max_len = max(20, self._row_content_width() - overhead) - if len(desc) <= max_len: - return desc - return desc[: max_len - 1] + "\u2026" - - def _render_server_checkbox(self) -> None: - n_checked = len( - [ - i - for i in self._checked - if self._server_items[i].name not in self._installed_names - ] - ) - if self._title_widget: - self._title_widget.update( - f"Select MCP servers to install ({n_checked} selected):" - ) - if self._help_widget: - self._help_widget.update( - "\u2191/\u2193 navigate \u00b7 Space toggle \u00b7 Enter install \u00b7 Esc cancel" - ) - - for i, widget in enumerate(self._row_widgets): - if i < len(self._server_items): - entry = self._server_items[i] - is_selected = i == self._selected - is_installed = entry.name in self._installed_names - is_checked = i in self._checked - - text = Text() - cursor = "\u25b8 " if is_selected else " " - text.append(cursor, style="bold cyan" if is_selected else "dim") - - desc = entry.description or entry.label - - if is_installed: - suffix = " (configured)" - desc = self._truncate(desc, entry.name, suffix=suffix) - text.append("\u2713 ", style="green") - text.append(entry.name, style="green dim") - text.append(f" \u2014 {desc}", style="dim") - text.append(suffix, style="dim italic") - elif is_checked: - desc = self._truncate(desc, entry.name) - text.append("\u25cf ", style="green bold") - text.append(entry.name, style="bold") - text.append(f" \u2014 {desc}", style="") - else: - desc = self._truncate(desc, entry.name) - text.append("\u25cb ", style="dim") - text.append(entry.name, style="bold" if is_selected else "") - text.append(f" \u2014 {desc}", style="dim") - - widget.update(text) - widget.display = True - widget.remove_class("browser-row-selected") - if is_selected: - widget.add_class("browser-row-selected") - widget.scroll_visible() - else: - widget.update("") - widget.display = False - - def _current_items_count(self) -> int: - if self._phase == 1: - return len(self._tag_items) - return len(self._server_items) - - def action_move_up(self) -> None: - n = self._current_items_count() - if not n: - return - self._selected = (self._selected - 1) % n - self._update_display() - - def action_move_down(self) -> None: - n = self._current_items_count() - if not n: - return - self._selected = (self._selected + 1) % n - self._update_display() - - def action_toggle(self) -> None: - if self._phase != 2: - return - if not self._server_items: - return - entry = self._server_items[self._selected] - if entry.name in self._installed_names: - return - if self._selected in self._checked: - self._checked.discard(self._selected) - else: - self._checked.add(self._selected) - self._update_display() - - def action_confirm(self) -> None: - if self._phase == 1: - if not self._tag_items: - return - tag, _ = self._tag_items[self._selected] - if tag == "all": - self._server_items = list(self._servers) - else: - self._server_items = [ - s for s in self._servers if tag in [t.lower() for t in s.tags] - ] - self._phase = 2 - self._selected = 0 - self._checked = set() - self._update_display() - else: - entries = [ - self._server_items[i] - for i in sorted(self._checked) - if self._server_items[i].name not in self._installed_names - ] - self.post_message(self.Confirmed(entries)) - - def action_cancel(self) -> None: - if self._phase == 2 and not self._pre_filter_tag: - self._phase = 1 - self._selected = 0 - self._checked = set() - self._update_display() - else: - self.post_message(self.Cancelled()) - - def on_blur(self, event: events.Blur) -> None: - self.call_after_refresh(self.focus) + def _post_cancelled(self) -> None: + self.post_message(self.Cancelled()) diff --git a/EvoScientist/cli/widgets/model_picker.py b/EvoScientist/cli/widgets/model_picker.py index b7a44ee..a710b5b 100644 --- a/EvoScientist/cli/widgets/model_picker.py +++ b/EvoScientist/cli/widgets/model_picker.py @@ -12,9 +12,10 @@ from rich.text import Text from textual.binding import Binding, BindingType from textual.containers import Container from textual.message import Message -from textual.widget import Widget from textual.widgets import Input, Static +from .picker_base import PickerWidgetBase, first_selectable_index, move_selection + if TYPE_CHECKING: from textual import events from textual.app import ComposeResult @@ -81,14 +82,13 @@ def _build_items( return items -class ModelPickerWidget(Widget): +class ModelPickerWidget(PickerWidgetBase): """Inline model picker -- mounts in chat, keyboard-driven. Posts ``Picked(name, provider)`` on Enter, ``Cancelled()`` on Esc. Type to filter models. """ - can_focus = True # Required so the Custom Ollama ``Input`` child can hold focus when the # user is typing a model name. can_focus_children = True @@ -188,22 +188,19 @@ class ModelPickerWidget(Widget): self._mode: Literal["list", "input"] = "list" self._custom_input: Input | None = None + @staticmethod + def _is_model(item: dict) -> bool: + return item["type"] == "model" + def _first_model_index(self) -> int: - for i, item in enumerate(self._items): - if item["type"] == "model": - return i - return 0 + return first_selectable_index(self._items, self._is_model) def _move(self, direction: int) -> None: if not self._items: return - i = (self._selected + direction) % len(self._items) - steps = 0 - while self._items[i]["type"] != "model" and steps < len(self._items): - i = (i + direction) % len(self._items) - steps += 1 - if self._items[i]["type"] == "model": - self._selected = i + new = move_selection(self._items, self._selected, direction, self._is_model) + if self._is_model(self._items[new]): + self._selected = new self._update_rows() def _rebuild(self) -> None: @@ -251,10 +248,9 @@ class ModelPickerWidget(Widget): classes="picker-help", ) - def on_mount(self) -> None: + def _refresh_view(self) -> None: self._update_rows() self._update_filter() - self.call_later(self.focus) def _update_filter(self) -> None: if self._filter_widget is not None: @@ -273,8 +269,8 @@ class ModelPickerWidget(Widget): for i, (item, widget) in enumerate( zip(self._items, self._row_widgets, strict=False) ): - widget.remove_class("picker-row-selected") if item["type"] == "header": + widget.remove_class("picker-row-selected") t = Text() t.append("\u2500\u2500 ", style="bold cyan") t.append(item["label"], style="bold cyan") @@ -289,9 +285,7 @@ class ModelPickerWidget(Widget): t.append(" *", style="bold green") t.append(f" ({item['provider']})", style="dim italic") widget.update(t) - if is_selected: - widget.add_class("picker-row-selected") - widget.scroll_visible() + self.apply_row_highlight(widget, is_selected) def on_key(self, event: events.Key) -> None: # In input mode, the Input child owns printable keys + backspace. @@ -350,11 +344,9 @@ class ModelPickerWidget(Widget): return self.post_message(self.Cancelled()) - def on_blur(self, event: events.Blur) -> None: + def _should_refocus_on_blur(self) -> bool: # When the Input child has focus we must NOT steal it back. - if self._mode == "input": - return - self.call_after_refresh(self.focus) + return self._mode != "input" def on_input_submitted(self, event: Input.Submitted) -> None: """Safety net: Enter fired inside the Input widget rather than diff --git a/EvoScientist/cli/widgets/picker_base.py b/EvoScientist/cli/widgets/picker_base.py new file mode 100644 index 0000000..3907469 --- /dev/null +++ b/EvoScientist/cli/widgets/picker_base.py @@ -0,0 +1,427 @@ +"""Shared engine for the TUI's keyboard-driven picker/browser widgets. + +Every inline picker (model picker, thread picker, skill/MCP browsers) +follows the same pattern: a flat item list where some rows are +selectable, a wrapping highlight cursor, Enter/Esc terminal messages, +and focus trapped inside the widget until a decision is made. This +module owns that machinery so the widgets only provide their data +model and row rendering. + +Subclassing contract: Textual dispatches same-named handlers at EVERY +level of the MRO, so subclasses must NOT define ``on_mount``/``on_blur`` +— they implement the ``_refresh_view()`` hook (and override +``_should_refocus_on_blur()`` if focus may legitimately leave, e.g. a +child ``Input``). Message classes (``Picked``/``Confirmed``/ +``Cancelled``) stay defined in each widget: their handler names +(``on__``) derive from the defining class. +""" + +from __future__ import annotations + +from collections.abc import Callable +from typing import TYPE_CHECKING, Any, ClassVar + +from rich.text import Text +from textual.binding import Binding, BindingType +from textual.containers import Container +from textual.widget import Widget +from textual.widgets import Static + +if TYPE_CHECKING: + from textual import events + from textual.app import ComposeResult + +# --------------------------------------------------------------------------- +# Pure selection helpers +# --------------------------------------------------------------------------- + + +def first_selectable_index( + items: list[Any], is_selectable: Callable[[Any], bool] +) -> int: + """Index of the first selectable item, or 0 when none qualifies.""" + for i, item in enumerate(items): + if is_selectable(item): + return i + return 0 + + +def move_selection( + items: list[Any], + current: int, + direction: int, + is_selectable: Callable[[Any], bool], +) -> int: + """Next selectable index from *current*, wrapping around the list. + + Non-selectable rows (headers, separators) are skipped; when no + selectable row exists the *current* index is returned unchanged. + """ + if not items: + return current + i = (current + direction) % len(items) + steps = 0 + while not is_selectable(items[i]) and steps < len(items): + i = (i + direction) % len(items) + steps += 1 + return i if is_selectable(items[i]) else current + + +# --------------------------------------------------------------------------- +# Widget base +# --------------------------------------------------------------------------- + + +class PickerWidgetBase(Widget): + """Focus-trapped inline picker: mount-focus, blur-refocus, row + highlight bookkeeping and description truncation.""" + + can_focus = True + can_focus_children = False + + def _refresh_view(self) -> None: + """Render the current state into the row widgets.""" + raise NotImplementedError + + def _should_refocus_on_blur(self) -> bool: + """Whether blur should snap focus back (focus trap).""" + return True + + def on_mount(self) -> None: + # Deferred so self.size is populated for width-aware rendering. + self.call_after_refresh(self._refresh_view) + self.call_later(self.focus) + + def on_blur(self, event: events.Blur) -> None: + if self._should_refocus_on_blur(): + self.call_after_refresh(self.focus) + + @staticmethod + def apply_row_highlight( + widget: Static, selected: bool, css_class: str = "picker-row-selected" + ) -> None: + """Toggle the selected-row CSS class and keep the row in view.""" + widget.remove_class(css_class) + if selected: + widget.add_class(css_class) + widget.scroll_visible() + + def _row_content_width(self) -> int: + """Usable character width for a row's text content (accounts for + widget border/padding; falls back to terminal width pre-layout).""" + try: + w = self.size.width + if w > 0: + # border (2) + widget padding (2) + row padding (2) + return w - 6 + except Exception: + pass + try: + return self.app.size.width - 10 + except Exception: + return 100 + + def _truncate(self, desc: str, name: str, *, suffix: str = "") -> str: + """Truncate a description to fit the row, adding ellipsis.""" + # cursor(2) + indicator(2) + name + " — "(3) + suffix + overhead = 2 + 2 + len(name) + 3 + len(suffix) + max_len = max(20, self._row_content_width() - overhead) + if len(desc) <= max_len: + return desc + return desc[: max_len - 1] + "…" + + +# --------------------------------------------------------------------------- +# Two-phase tag-filter → checkbox browser +# --------------------------------------------------------------------------- + + +class TagCheckboxBrowserBase(PickerWidgetBase): + """Two-phase multi-select browser shared by the skill and MCP browsers. + + Phase 1 — tag picker (Enter selects a tag filter, "all" included). + Phase 2 — checkbox list (Space toggles, Enter confirms, Esc goes back + to phase 1 unless the widget was constructed pre-filtered). + + Subclasses provide the data adapters (``_item_name`` / ``_item_tags`` + / ``_item_desc``), the phase-2 texts, and ``_post_confirmed()``. + """ + + DEFAULT_CSS = """ + TagCheckboxBrowserBase { + height: auto; + max-height: 30; + margin: 1 0; + padding: 0 1; + background: $surface; + border: solid $primary; + } + TagCheckboxBrowserBase .browser-title { + height: 1; + text-style: bold; + color: $primary; + } + TagCheckboxBrowserBase .browser-rows { + height: auto; + max-height: 20; + overflow-y: auto; + } + TagCheckboxBrowserBase .browser-row { + height: 1; + padding: 0 1; + } + TagCheckboxBrowserBase .browser-row-selected { + background: $primary; + text-style: bold; + } + TagCheckboxBrowserBase .browser-help { + height: 1; + color: $text-muted; + text-style: italic; + } + """ + + BINDINGS: ClassVar[list[BindingType]] = [ + Binding("up", "move_up", "Up", show=False), + Binding("k", "move_up", "Up", show=False), + Binding("down", "move_down", "Down", show=False), + Binding("j", "move_down", "Down", show=False), + Binding("enter", "confirm", "Confirm", show=False), + Binding("space", "toggle", "Toggle", show=False), + Binding("escape", "cancel", "Cancel", show=False), + ] + + # -- subclass adapters -------------------------------------------- + + _INSTALLED_SUFFIX: ClassVar[str] = " (installed)" + _PHASE2_TITLE: ClassVar[str] = "Select items to install" + _PHASE2_CONFIRM_LABEL: ClassVar[str] = "install" + + def _item_name(self, item: Any) -> str: + raise NotImplementedError + + def _item_tags(self, item: Any) -> list[str]: + raise NotImplementedError + + def _item_desc(self, item: Any) -> str: + raise NotImplementedError + + def _post_confirmed(self, items: list[Any]) -> None: + """Post the widget-specific ``Confirmed`` message.""" + raise NotImplementedError + + def _post_cancelled(self) -> None: + """Post the widget-specific ``Cancelled`` message.""" + raise NotImplementedError + + # -- state --------------------------------------------------------- + + def __init__( + self, + items: list[Any], + installed_names: set[str], + *, + pre_filter_tag: str = "", + **kwargs: Any, + ) -> None: + super().__init__(**kwargs) + self._all_items = items + self._installed_names = installed_names + self._pre_filter_tag = pre_filter_tag.lower() + self._selected = 0 + self._row_widgets: list[Static] = [] + self._title_widget: Static | None = None + self._help_widget: Static | None = None + + self._phase: int = 1 + self._filtered_items: list[Any] = [] + self._checked: set[int] = set() + + # Build tag list (sorted by count desc, then alphabetically) + from collections import Counter + + tag_counter: Counter[str] = Counter() + for item in self._all_items: + for t in self._item_tags(item): + tag_counter[t.lower()] += 1 + sorted_tags = sorted(tag_counter.items(), key=lambda x: (-x[1], x[0])) + self._tag_items: list[tuple[str, int]] = [ + ("all", len(self._all_items)), + *sorted_tags, + ] + + # If pre-filtered, skip to phase 2 + if self._pre_filter_tag: + self._filtered_items = self._items_with_tag(self._pre_filter_tag) + if self._filtered_items: + self._phase = 2 + else: + self._pre_filter_tag = "" + + def _items_with_tag(self, tag: str) -> list[Any]: + if tag == "all": + return list(self._all_items) + return [ + item + for item in self._all_items + if tag in [t.lower() for t in self._item_tags(item)] + ] + + # -- layout --------------------------------------------------------- + + def compose(self) -> ComposeResult: + self._title_widget = Static("", classes="browser-title") + yield self._title_widget + with Container(classes="browser-rows"): + max_rows = max(len(self._tag_items), len(self._all_items)) + for _ in range(max_rows): + widget = Static("", classes="browser-row") + self._row_widgets.append(widget) + yield widget + self._help_widget = Static("", classes="browser-help") + yield self._help_widget + + # -- rendering ------------------------------------------------------ + + def _refresh_view(self) -> None: + if self._phase == 1: + self._render_tag_picker() + else: + self._render_checkbox_list() + + def _render_tag_picker(self) -> None: + if self._title_widget: + self._title_widget.update("Filter by tag:") + if self._help_widget: + self._help_widget.update("↑/↓ navigate · Enter select · Esc cancel") + + for i, widget in enumerate(self._row_widgets): + if i < len(self._tag_items): + tag, count = self._tag_items[i] + is_selected = i == self._selected + text = Text() + cursor = "▸ " if is_selected else " " + text.append(cursor, style="bold cyan" if is_selected else "dim") + text.append(f"{tag} ({count})", style="bold" if is_selected else "") + widget.update(text) + widget.display = True + self.apply_row_highlight(widget, is_selected, "browser-row-selected") + else: + widget.update("") + widget.display = False + + def _render_checkbox_list(self) -> None: + n_checked = len( + [ + i + for i in self._checked + if self._item_name(self._filtered_items[i]) not in self._installed_names + ] + ) + if self._title_widget: + self._title_widget.update(f"{self._PHASE2_TITLE} ({n_checked} selected):") + if self._help_widget: + self._help_widget.update( + "↑/↓ navigate · Space toggle · " + f"Enter {self._PHASE2_CONFIRM_LABEL} · Esc cancel" + ) + + for i, widget in enumerate(self._row_widgets): + if i < len(self._filtered_items): + item = self._filtered_items[i] + name = self._item_name(item) + is_selected = i == self._selected + is_installed = name in self._installed_names + is_checked = i in self._checked + + text = Text() + cursor = "▸ " if is_selected else " " + text.append(cursor, style="bold cyan" if is_selected else "dim") + + if is_installed: + suffix = self._INSTALLED_SUFFIX + desc = self._truncate(self._item_desc(item), name, suffix=suffix) + text.append("✓ ", style="green") + text.append(name, style="green dim") + text.append(f" — {desc}", style="dim") + text.append(suffix, style="dim italic") + elif is_checked: + desc = self._truncate(self._item_desc(item), name) + text.append("● ", style="green bold") + text.append(name, style="bold") + text.append(f" — {desc}", style="") + else: + desc = self._truncate(self._item_desc(item), name) + text.append("○ ", style="dim") + text.append(name, style="bold" if is_selected else "") + text.append(f" — {desc}", style="dim") + + widget.update(text) + widget.display = True + self.apply_row_highlight(widget, is_selected, "browser-row-selected") + else: + widget.update("") + widget.display = False + + # -- actions ---------------------------------------------------------- + + def _current_items_count(self) -> int: + if self._phase == 1: + return len(self._tag_items) + return len(self._filtered_items) + + def action_move_up(self) -> None: + n = self._current_items_count() + if not n: + return + self._selected = (self._selected - 1) % n + self._refresh_view() + + def action_move_down(self) -> None: + n = self._current_items_count() + if not n: + return + self._selected = (self._selected + 1) % n + self._refresh_view() + + def action_toggle(self) -> None: + """Toggle checkbox selection (phase 2 only).""" + if self._phase != 2 or not self._filtered_items: + return + if self._item_name(self._filtered_items[self._selected]) in ( + self._installed_names + ): + return # Can't toggle already-installed items + if self._selected in self._checked: + self._checked.discard(self._selected) + else: + self._checked.add(self._selected) + self._refresh_view() + + def action_confirm(self) -> None: + if self._phase == 1: + if not self._tag_items: + return + tag, _ = self._tag_items[self._selected] + self._filtered_items = self._items_with_tag(tag) + self._phase = 2 + self._selected = 0 + self._checked = set() + self._refresh_view() + else: + items = [ + self._filtered_items[i] + for i in sorted(self._checked) + if self._item_name(self._filtered_items[i]) not in self._installed_names + ] + self._post_confirmed(items) + + def action_cancel(self) -> None: + if self._phase == 2 and not self._pre_filter_tag: + # Go back to tag picker + self._phase = 1 + self._selected = 0 + self._checked = set() + self._refresh_view() + else: + self._post_cancelled() diff --git a/EvoScientist/cli/widgets/skill_browser.py b/EvoScientist/cli/widgets/skill_browser.py index e6de017..be88794 100644 --- a/EvoScientist/cli/widgets/skill_browser.py +++ b/EvoScientist/cli/widgets/skill_browser.py @@ -1,6 +1,7 @@ """Inline skill browser widget for /evoskills in TUI. -Two-phase keyboard-driven widget: +Two-phase keyboard-driven widget built on the shared picker engine +(``picker_base.TagCheckboxBrowserBase``): Phase 1 — tag picker (arrow keys + Enter to select, or Esc for all) Phase 2 — skill checkbox (arrow keys to navigate, Space to toggle, Enter to confirm) @@ -10,73 +11,19 @@ or ``SkillBrowserWidget.Cancelled`` on Esc. from __future__ import annotations -from typing import TYPE_CHECKING, Any, ClassVar +from typing import Any, ClassVar -from rich.text import Text -from textual.binding import Binding, BindingType -from textual.containers import Container from textual.message import Message -from textual.widget import Widget -from textual.widgets import Static -if TYPE_CHECKING: - from textual import events - from textual.app import ComposeResult +from .picker_base import TagCheckboxBrowserBase -class SkillBrowserWidget(Widget): - """Inline skill browser — mounts in chat, keyboard-driven. +class SkillBrowserWidget(TagCheckboxBrowserBase): + """Inline skill browser — mounts in chat, keyboard-driven.""" - Phase 1: Tag picker (select a tag filter or "All"). - Phase 2: Skill checkbox (toggle skills, confirm to install). - """ - - can_focus = True - can_focus_children = False - - DEFAULT_CSS = """ - SkillBrowserWidget { - height: auto; - max-height: 30; - margin: 1 0; - padding: 0 1; - background: $surface; - border: solid $primary; - } - SkillBrowserWidget .browser-title { - height: 1; - text-style: bold; - color: $primary; - } - SkillBrowserWidget .browser-rows { - height: auto; - max-height: 20; - overflow-y: auto; - } - SkillBrowserWidget .browser-row { - height: 1; - padding: 0 1; - } - SkillBrowserWidget .browser-row-selected { - background: $primary; - text-style: bold; - } - SkillBrowserWidget .browser-help { - height: 1; - color: $text-muted; - text-style: italic; - } - """ - - BINDINGS: ClassVar[list[BindingType]] = [ - Binding("up", "move_up", "Up", show=False), - Binding("k", "move_up", "Up", show=False), - Binding("down", "move_down", "Down", show=False), - Binding("j", "move_down", "Down", show=False), - Binding("enter", "confirm", "Confirm", show=False), - Binding("space", "toggle", "Toggle", show=False), - Binding("escape", "cancel", "Cancel", show=False), - ] + _INSTALLED_SUFFIX: ClassVar[str] = " (installed)" + _PHASE2_TITLE: ClassVar[str] = "Select skills to install" + _PHASE2_CONFIRM_LABEL: ClassVar[str] = "install" class Confirmed(Message): """Posted when user confirms skill selection.""" @@ -88,261 +35,17 @@ class SkillBrowserWidget(Widget): class Cancelled(Message): """Posted when user cancels.""" - def __init__( - self, - index: list[dict], - installed_names: set[str], - *, - pre_filter_tag: str = "", - **kwargs: Any, - ) -> None: - super().__init__(**kwargs) - self._index = index - self._installed_names = installed_names - self._pre_filter_tag = pre_filter_tag.lower() - self._selected = 0 - self._row_widgets: list[Static] = [] - self._title_widget: Static | None = None - self._help_widget: Static | None = None + def _item_name(self, item: Any) -> str: + return item["name"] - # Phase 1: tag picker - # Phase 2: skill checkbox - self._phase: int = 1 - self._tag_items: list[tuple[str, int]] = [] # (tag, count) - self._skill_items: list[dict] = [] # filtered skills - self._checked: set[int] = set() # indices of checked skills + def _item_tags(self, item: Any) -> list[str]: + return item.get("tags", []) - # Build tag list (sorted by count desc, then alphabetically) - from collections import Counter + def _item_desc(self, item: Any) -> str: + return item["description"] - tag_counter: Counter[str] = Counter() - for s in self._index: - for t in s.get("tags", []): - tag_counter[t.lower()] += 1 - sorted_tags = sorted(tag_counter.items(), key=lambda x: (-x[1], x[0])) - self._tag_items = [("all", len(self._index)), *sorted_tags] + def _post_confirmed(self, items: list[Any]) -> None: + self.post_message(self.Confirmed([s["install_source"] for s in items])) - # If pre-filtered, skip to phase 2 - if self._pre_filter_tag: - self._skill_items = [ - s - for s in self._index - if self._pre_filter_tag in [t.lower() for t in s.get("tags", [])] - ] - if self._skill_items: - self._phase = 2 - else: - # No matches — show tag picker anyway - self._pre_filter_tag = "" - - def compose(self) -> ComposeResult: - self._title_widget = Static("", classes="browser-title") - yield self._title_widget - with Container(classes="browser-rows"): - # Pre-allocate enough rows for the larger of tag list or skill list - max_rows = max(len(self._tag_items), len(self._index)) - for _ in range(max_rows): - widget = Static("", classes="browser-row") - self._row_widgets.append(widget) - yield widget - self._help_widget = Static("", classes="browser-help") - yield self._help_widget - - def on_mount(self) -> None: - # Defer rendering until after layout so self.size is populated - self.call_after_refresh(self._update_display) - self.call_later(self.focus) - - def _update_display(self) -> None: - if self._phase == 1: - self._render_tag_picker() - else: - self._render_skill_checkbox() - - def _render_tag_picker(self) -> None: - if self._title_widget: - self._title_widget.update("Filter by tag:") - if self._help_widget: - self._help_widget.update("↑/↓ navigate · Enter select · Esc cancel") - - for i, widget in enumerate(self._row_widgets): - if i < len(self._tag_items): - tag, count = self._tag_items[i] - is_selected = i == self._selected - text = Text() - cursor = "▸ " if is_selected else " " - text.append(cursor, style="bold cyan" if is_selected else "dim") - label = f"{tag} ({count})" - text.append(label, style="bold" if is_selected else "") - widget.update(text) - widget.display = True - widget.remove_class("browser-row-selected") - if is_selected: - widget.add_class("browser-row-selected") - widget.scroll_visible() - else: - widget.update("") - widget.display = False - - def _row_content_width(self) -> int: - """Get the usable character width for a row's text content. - - Accounts for widget border, widget padding, and row padding. - Falls back to terminal width if the widget hasn't been laid out yet. - """ - try: - w = self.size.width - if w > 0: - # border (2) + widget padding-left/right (2) + row padding-left/right (2) - return w - 6 - except Exception: - pass - # Fallback: use terminal width minus reasonable chrome - try: - return self.app.size.width - 10 - except Exception: - return 100 - - def _truncate(self, desc: str, name: str, *, suffix: str = "") -> str: - """Truncate a description to fit the row, adding ellipsis if needed.""" - # cursor(2) + indicator(2) + name + " — "(3) + suffix - overhead = 2 + 2 + len(name) + 3 + len(suffix) - max_len = max(20, self._row_content_width() - overhead) - if len(desc) <= max_len: - return desc - return desc[: max_len - 1] + "…" - - def _render_skill_checkbox(self) -> None: - n_checked = len( - [ - i - for i in self._checked - if self._skill_items[i]["name"] not in self._installed_names - ] - ) - if self._title_widget: - self._title_widget.update( - f"Select skills to install ({n_checked} selected):" - ) - if self._help_widget: - self._help_widget.update( - "↑/↓ navigate · Space toggle · Enter install · Esc cancel" - ) - - for i, widget in enumerate(self._row_widgets): - if i < len(self._skill_items): - skill = self._skill_items[i] - is_selected = i == self._selected - is_installed = skill["name"] in self._installed_names - is_checked = i in self._checked - - text = Text() - cursor = "▸ " if is_selected else " " - text.append(cursor, style="bold cyan" if is_selected else "dim") - - if is_installed: - suffix = " (installed)" - desc = self._truncate( - desc=skill["description"], - name=skill["name"], - suffix=suffix, - ) - text.append("✓ ", style="green") - text.append(skill["name"], style="green dim") - text.append(f" — {desc}", style="dim") - text.append(suffix, style="dim italic") - elif is_checked: - desc = self._truncate(skill["description"], skill["name"]) - text.append("● ", style="green bold") - text.append(skill["name"], style="bold") - text.append(f" — {desc}", style="") - else: - desc = self._truncate(skill["description"], skill["name"]) - text.append("○ ", style="dim") - text.append(skill["name"], style="bold" if is_selected else "") - text.append(f" — {desc}", style="dim") - - widget.update(text) - widget.display = True - widget.remove_class("browser-row-selected") - if is_selected: - widget.add_class("browser-row-selected") - widget.scroll_visible() - else: - widget.update("") - widget.display = False - - def _current_items_count(self) -> int: - if self._phase == 1: - return len(self._tag_items) - return len(self._skill_items) - - def action_move_up(self) -> None: - n = self._current_items_count() - if not n: - return - self._selected = (self._selected - 1) % n - self._update_display() - - def action_move_down(self) -> None: - n = self._current_items_count() - if not n: - return - self._selected = (self._selected + 1) % n - self._update_display() - - def action_toggle(self) -> None: - """Toggle skill selection (phase 2 only).""" - if self._phase != 2: - return - if not self._skill_items: - return - skill = self._skill_items[self._selected] - if skill["name"] in self._installed_names: - return # Can't toggle installed skills - if self._selected in self._checked: - self._checked.discard(self._selected) - else: - self._checked.add(self._selected) - self._update_display() - - def action_confirm(self) -> None: - if self._phase == 1: - # Transition to phase 2 - if not self._tag_items: - return - tag, _ = self._tag_items[self._selected] - if tag == "all": - self._skill_items = list(self._index) - else: - self._skill_items = [ - s - for s in self._index - if tag in [t.lower() for t in s.get("tags", [])] - ] - self._phase = 2 - self._selected = 0 - self._checked = set() - self._update_display() - else: - # Confirm selection - sources = [ - self._skill_items[i]["install_source"] - for i in sorted(self._checked) - if self._skill_items[i]["name"] not in self._installed_names - ] - self.post_message(self.Confirmed(sources)) - - def action_cancel(self) -> None: - if self._phase == 2 and not self._pre_filter_tag: - # Go back to tag picker - self._phase = 1 - self._selected = 0 - self._checked = set() - self._update_display() - else: - self.post_message(self.Cancelled()) - - def on_blur(self, event: events.Blur) -> None: - """Re-focus to keep focus trapped until decision is made.""" - self.call_after_refresh(self.focus) + def _post_cancelled(self) -> None: + self.post_message(self.Cancelled()) diff --git a/EvoScientist/cli/widgets/thread_selector.py b/EvoScientist/cli/widgets/thread_selector.py index b79b7f8..f5473ad 100644 --- a/EvoScientist/cli/widgets/thread_selector.py +++ b/EvoScientist/cli/widgets/thread_selector.py @@ -23,11 +23,11 @@ from rich.text import Text from textual.binding import Binding, BindingType from textual.containers import Container from textual.message import Message -from textual.widget import Widget from textual.widgets import Static +from .picker_base import PickerWidgetBase, first_selectable_index, move_selection + if TYPE_CHECKING: - from textual import events from textual.app import ComposeResult @@ -237,16 +237,13 @@ def build_row_text( # --------------------------------------------------------------------------- -class ThreadPickerWidget(Widget): +class ThreadPickerWidget(PickerWidgetBase): """Inline thread picker — mounts in chat, keyboard-driven. Posts ``Picked(thread_id)`` on Enter, ``Cancelled()`` on Esc. Threads are displayed in a two-level workspace hierarchy. """ - can_focus = True - can_focus_children = False - DEFAULT_CSS = """ ThreadPickerWidget { height: auto; @@ -323,22 +320,19 @@ class ThreadPickerWidget(Widget): self._selected = self._first_thread_index() self._row_widgets: list[Static] = [] + @staticmethod + def _is_thread(item: dict) -> bool: + return item["type"] == "thread" + def _first_thread_index(self) -> int: - for i, item in enumerate(self._items): - if item["type"] == "thread": - return i - return 0 + return first_selectable_index(self._items, self._is_thread) def _move(self, direction: int) -> None: if not self._items: return - i = (self._selected + direction) % len(self._items) - steps = 0 - while self._items[i]["type"] != "thread" and steps < len(self._items): - i = (i + direction) % len(self._items) - steps += 1 - if self._items[i]["type"] == "thread": - self._selected = i + new = move_selection(self._items, self._selected, direction, self._is_thread) + if self._is_thread(self._items[new]): + self._selected = new self._update_rows() def compose(self) -> ComposeResult: @@ -358,18 +352,18 @@ class ThreadPickerWidget(Widget): classes="picker-help", ) - def on_mount(self) -> None: + def _refresh_view(self) -> None: self._update_rows() - self.call_later(self.focus) def _update_rows(self) -> None: for i, (item, widget) in enumerate( zip(self._items, self._row_widgets, strict=False) ): - widget.remove_class("picker-row-selected") if item["type"] == "header": + widget.remove_class("picker-row-selected") widget.update(build_header_text(item["label"])) elif item["type"] == "subheader": + widget.remove_class("picker-row-selected") widget.update(build_subheader_text(item["label"])) else: thread = item["thread"] @@ -381,9 +375,7 @@ class ThreadPickerWidget(Widget): indented=item.get("indented", False), ) widget.update(text) - if is_selected: - widget.add_class("picker-row-selected") - widget.scroll_visible() + self.apply_row_highlight(widget, is_selected) def action_move_up(self) -> None: self._move(-1) @@ -403,6 +395,3 @@ class ThreadPickerWidget(Widget): def action_cancel(self) -> None: self.post_message(self.Cancelled()) - - def on_blur(self, event: events.Blur) -> None: - self.call_after_refresh(self.focus) diff --git a/tests/test_picker_widgets.py b/tests/test_picker_widgets.py new file mode 100644 index 0000000..fe2cc1f --- /dev/null +++ b/tests/test_picker_widgets.py @@ -0,0 +1,255 @@ +"""Behavior tests for the picker/browser widgets and their shared engine. + +Written against the pre-refactor SkillBrowserWidget / MCPBrowserWidget +behavior to pin the two-phase tag->checkbox flow, then kept green while +the widgets migrated onto the shared picker engine (PR #362 follow-up +discussion). Widgets are driven without a running app: ``__init__`` and +the ``action_*`` methods only touch widget state, and DOM-dependent +rendering no-ops before ``compose()``. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import ClassVar +from unittest.mock import MagicMock + +from EvoScientist.cli.widgets.mcp_browser import MCPBrowserWidget +from EvoScientist.cli.widgets.picker_base import ( + first_selectable_index, + move_selection, +) +from EvoScientist.cli.widgets.skill_browser import SkillBrowserWidget + +_SKILL_INDEX = [ + { + "name": "paper-writing", + "description": "Write papers", + "tags": ["writing"], + "install_source": "src/paper-writing", + }, + { + "name": "paper-review", + "description": "Review papers", + "tags": ["writing", "review"], + "install_source": "src/paper-review", + }, + { + "name": "experiment-pipeline", + "description": "Run experiments", + "tags": ["experiments"], + "install_source": "src/experiment-pipeline", + }, +] + + +@dataclass +class _Server: + name: str + tags: list[str] = field(default_factory=list) + description: str = "" + label: str = "" + + +_SERVERS = [ + _Server("tavily", tags=["search"], description="Web search"), + _Server("github", tags=["dev", "search"], description="GitHub API"), + _Server("filesystem", tags=["dev"], description="Local files"), +] + + +def _skill_browser(**kwargs) -> SkillBrowserWidget: + w = SkillBrowserWidget(_SKILL_INDEX, {"paper-review"}, **kwargs) + w.post_message = MagicMock() + w.focus = MagicMock() + return w + + +def _mcp_browser(**kwargs) -> MCPBrowserWidget: + w = MCPBrowserWidget(_SERVERS, {"github"}, **kwargs) + w.post_message = MagicMock() + w.focus = MagicMock() + return w + + +class TestSelectionHelpers: + """Pure navigation helpers shared by all pickers (headers skipped, + wrapping cursor).""" + + _ITEMS: ClassVar[list[str]] = ["header", "item", "item", "header", "item"] + + @staticmethod + def _selectable(item: str) -> bool: + return item == "item" + + def test_first_selectable_skips_headers(self): + assert first_selectable_index(self._ITEMS, self._selectable) == 1 + + def test_first_selectable_empty_or_none_returns_zero(self): + assert first_selectable_index([], self._selectable) == 0 + assert first_selectable_index(["header"], self._selectable) == 0 + + def test_move_skips_headers_and_wraps(self): + assert move_selection(self._ITEMS, 1, 1, self._selectable) == 2 + assert move_selection(self._ITEMS, 2, 1, self._selectable) == 4 + # wraps past the trailing header back to the first item + assert move_selection(self._ITEMS, 4, 1, self._selectable) == 1 + assert move_selection(self._ITEMS, 1, -1, self._selectable) == 4 + + def test_move_with_no_selectable_keeps_current(self): + items = ["header", "header"] + assert move_selection(items, 0, 1, self._selectable) == 0 + + def test_move_single_selectable_stays_put(self): + items = ["header", "item"] + assert move_selection(items, 1, 1, self._selectable) == 1 + + +class TestSkillBrowserFlow: + def test_tag_items_sorted_by_count_with_all_first(self): + w = _skill_browser() + assert w._tag_items[0] == ("all", 3) + # "writing" appears twice -> first among real tags + assert w._tag_items[1] == ("writing", 2) + + def test_phase1_enter_filters_and_enters_phase2(self): + w = _skill_browser() + w._selected = 1 # "writing" + w.action_confirm() + assert w._phase == 2 + assert [s["name"] for s in w._filtered_items] == [ + "paper-writing", + "paper-review", + ] + assert w._selected == 0 + + def test_phase1_all_selects_everything(self): + w = _skill_browser() + w._selected = 0 # "all" + w.action_confirm() + assert len(w._filtered_items) == 3 + + def test_space_toggles_uninstalled_only(self): + w = _skill_browser() + w.action_confirm() # all -> phase 2 + w._selected = 0 # paper-writing (not installed) + w.action_toggle() + assert 0 in w._checked + w.action_toggle() + assert 0 not in w._checked + w._selected = 1 # paper-review (installed) + w.action_toggle() + assert 1 not in w._checked + + def test_confirm_posts_confirmed_with_sources(self): + w = _skill_browser() + w.action_confirm() # all -> phase 2 + w._selected = 0 + w.action_toggle() + w._selected = 2 + w.action_toggle() + w.action_confirm() + msg = w.post_message.call_args[0][0] + assert isinstance(msg, SkillBrowserWidget.Confirmed) + assert msg.install_sources == ["src/paper-writing", "src/experiment-pipeline"] + + def test_escape_returns_to_phase1_then_cancels(self): + w = _skill_browser() + w.action_confirm() # phase 2 + w.action_cancel() + assert w._phase == 1 + w.action_cancel() + msg = w.post_message.call_args[0][0] + assert isinstance(msg, SkillBrowserWidget.Cancelled) + + def test_pre_filter_tag_skips_to_phase2(self): + w = _skill_browser(pre_filter_tag="experiments") + assert w._phase == 2 + assert [s["name"] for s in w._filtered_items] == ["experiment-pipeline"] + # Esc cancels directly (no tag-picker phase to return to) + w.action_cancel() + msg = w.post_message.call_args[0][0] + assert isinstance(msg, SkillBrowserWidget.Cancelled) + + def test_pre_filter_without_match_falls_back_to_phase1(self): + w = _skill_browser(pre_filter_tag="nonexistent") + assert w._phase == 1 + + def test_move_wraps_around(self): + w = _skill_browser() + n = len(w._tag_items) + w._selected = 0 + w.action_move_up() + assert w._selected == n - 1 + w.action_move_down() + assert w._selected == 0 + + +class TestMountedBrowser: + """End-to-end smoke test: the engine's mount hook, bindings and + base-class CSS must all work on a real mounted widget.""" + + async def test_skill_browser_keyboard_flow(self): + from textual.app import App, ComposeResult + + posted: list = [] + + class BrowserApp(App[None]): + def compose(self) -> ComposeResult: + yield SkillBrowserWidget(_SKILL_INDEX, {"paper-review"}) + + def on_skill_browser_widget_confirmed(self, event) -> None: + posted.append(event) + + app = BrowserApp() + async with app.run_test(size=(80, 30)) as pilot: + widget = app.query_one(SkillBrowserWidget) + await pilot.pause() + # Base CSS (via TagCheckboxBrowserBase type selector) applied + assert widget.styles.border.top is not None + # Phase 1 renders the tag rows + assert widget._phase == 1 + # Enter on "all" -> phase 2; space toggles; enter confirms + await pilot.press("enter") + assert widget._phase == 2 + await pilot.press("space") + await pilot.press("enter") + await pilot.pause() + assert len(posted) == 1 + assert posted[0].install_sources == ["src/paper-writing"] + + +class TestMCPBrowserFlow: + def test_tag_items_sorted_by_count_with_all_first(self): + w = _mcp_browser() + assert w._tag_items[0] == ("all", 3) + assert {t for t, _ in w._tag_items[1:]} == {"dev", "search"} + + def test_confirm_posts_confirmed_with_entries(self): + w = _mcp_browser() + w.action_confirm() # all -> phase 2 + w._selected = 0 # tavily (not installed) + w.action_toggle() + w._selected = 1 # github (installed -> ignored) + w.action_toggle() + w.action_confirm() + msg = w.post_message.call_args[0][0] + assert isinstance(msg, MCPBrowserWidget.Confirmed) + assert [e.name for e in msg.entries] == ["tavily"] + + def test_tag_filter_selects_matching_servers(self): + w = _mcp_browser() + tag_index = [t for t, _ in w._tag_items].index("dev") + w._selected = tag_index + w.action_confirm() + assert w._phase == 2 + assert [e.name for e in w._filtered_items] == ["github", "filesystem"] + + def test_escape_returns_to_phase1_then_cancels(self): + w = _mcp_browser() + w.action_confirm() + w.action_cancel() + assert w._phase == 1 + w.action_cancel() + msg = w.post_message.call_args[0][0] + assert isinstance(msg, MCPBrowserWidget.Cancelled) diff --git a/tests/test_tui_completion_window.py b/tests/test_tui_completion_window.py new file mode 100644 index 0000000..ebd1bc6 --- /dev/null +++ b/tests/test_tui_completion_window.py @@ -0,0 +1,310 @@ +"""Tests for the TUI completion popup windowing (issue #354). + +The ``#completions`` Static used to be clipped by a fixed CSS +``max-height: 15`` — the flattened command list for ``/`` renders ~30 +rows (items + category headers + separators), so trailing categories +(``General``) were silently cut off. The fix renders a height-bounded +window that always keeps the selected item visible and marks hidden +items with overflow indicators. +""" + +from __future__ import annotations + +from EvoScientist.cli.tui_interactive import ( + _COMPLETIONS_CSS, + _build_completion_rows, + _completion_row_budget, + _normalize_chat_scroll, + _render_completion_text, + _window_completion_rows, +) +from EvoScientist.commands._completion_engine import ( + CompletionCandidate, + compute_completions, +) + + +def _candidates(spec: list[tuple[str, str]]) -> list[CompletionCandidate]: + """Build candidates from (text, category) pairs.""" + return [ + CompletionCandidate( + text=text, + description=f"desc of {text}", + replace_start=0, + replace_end=1, + category=category, + ) + for text, category in spec + ] + + +def _long_candidates() -> list[CompletionCandidate]: + """Three candidates whose lines far exceed a 40-column terminal.""" + return [ + CompletionCandidate( + text=f"/long-command-{i}", + description="x" * 120, + replace_start=0, + replace_end=1, + category="Session", + ) + for i in range(3) + ] + + +class TestBuildCompletionRows: + def test_headers_and_separators_between_categories(self): + items = _candidates([("/a", "Session"), ("/b", "Session"), ("/c", "General")]) + rows = _build_completion_rows(items) + kinds = [kind for kind, _payload, _idx in rows] + assert kinds == ["header", "item", "item", "sep", "header", "item"] + + def test_item_rows_keep_original_indices(self): + items = _candidates([("/a", "Session"), ("/b", "General")]) + rows = _build_completion_rows(items) + item_indices = [idx for kind, _payload, idx in rows if kind == "item"] + assert item_indices == [0, 1] + + def test_no_category_items_have_no_headers(self): + items = _candidates([("@f1", ""), ("@f2", "")]) + rows = _build_completion_rows(items) + assert [kind for kind, _p, _i in rows] == ["item", "item"] + + +class TestWindowCompletionRows: + def test_fitting_rows_are_untouched(self): + rows = _build_completion_rows( + _candidates([("/a", "Session"), ("/b", "General")]) + ) + visible, above, below = _window_completion_rows(rows, -1, 10) + assert visible == rows + assert (above, below) == (0, 0) + + def test_no_selection_windows_from_top(self): + items = _candidates([(f"/c{i}", "Session") for i in range(20)]) + rows = _build_completion_rows(items) + visible, above, below = _window_completion_rows(rows, -1, 8) + assert above == 0 + assert below > 0 + # First item visible at the top of the window + first_items = [idx for kind, _p, idx in visible if kind == "item"] + assert first_items[0] == 0 + + def test_last_selection_windows_to_bottom(self): + items = _candidates([(f"/c{i}", "Session") for i in range(20)]) + rows = _build_completion_rows(items) + visible, above, below = _window_completion_rows(rows, 19, 8) + assert below == 0 + assert above > 0 + assert any(kind == "item" and idx == 19 for kind, _p, idx in visible) + + def test_selected_item_always_visible(self): + items = _candidates( + [(f"/c{i}", cat) for i, cat in enumerate(["A"] * 7 + ["B"] * 7 + ["C"] * 6)] + ) + rows = _build_completion_rows(items) + for selected in range(len(items)): + visible, above, below = _window_completion_rows(rows, selected, 9) + assert any( + kind == "item" and idx == selected for kind, _p, idx in visible + ), f"selected {selected} not in window" + # Window body + indicator rows never exceed the budget + total = len(visible) + (1 if above else 0) + (1 if below else 0) + assert total <= 9 + + def test_hidden_counts_only_count_items(self): + items = _candidates( + [("/a", "A"), ("/b", "A"), ("/c", "B"), ("/d", "B"), ("/e", "C")] + ) + rows = _build_completion_rows(items) # 8 rows + visible, above, below = _window_completion_rows(rows, 4, 5) + assert above + below == 5 - sum(1 for k, _p, _i in visible if k == "item") + + +class TestCompletionRowBudget: + """The popup line budget follows the terminal but is hard-capped so + the popup never dwarfs the chat area (mainstream CLI behavior).""" + + def test_capped_on_tall_terminals(self): + assert _completion_row_budget(100) == 15 + + def test_shrinks_with_terminal(self): + assert _completion_row_budget(20) == 8 + + def test_floor_on_tiny_terminals(self): + assert _completion_row_budget(10) == 5 + + def test_unknown_height_falls_back_to_cap(self): + assert _completion_row_budget(0) == 15 + + +class TestRenderCompletionText: + def test_generous_budget_renders_all_commands(self): + """Regression for #354: with enough rows every category renders.""" + result = compute_completions("/", 1) + text = _render_completion_text(result.candidates, -1, 100).plain + assert "General" in text + assert "/help" in text + for candidate in result.candidates: + assert candidate.text in text + + def test_short_terminal_shows_overflow_indicator(self): + result = compute_completions("/", 1) + text = _render_completion_text(result.candidates, -1, 10).plain + assert "more" in text + assert "↓" in text # ↓ indicator for hidden items below + assert result.candidates[0].text in text + + def test_navigation_reaches_every_command(self): + """Every candidate becomes visible when selected, even clipped ones.""" + result = compute_completions("/", 1) + for i, candidate in enumerate(result.candidates): + text = _render_completion_text(result.candidates, i, 10).plain + assert candidate.text in text, f"{candidate.text} hidden when selected" + + def test_selected_candidate_marked(self): + items = _candidates([("/a", "Session"), ("/b", "Session")]) + text = _render_completion_text(items, 1, 20).plain + marked = [ln for ln in text.splitlines() if "▸" in ln] + assert len(marked) == 1 + assert "/b" in marked[0] + + def test_no_indicators_when_everything_fits(self): + items = _candidates([("/a", "Session"), ("/b", "General")]) + text = _render_completion_text(items, -1, 20).plain + assert "more" not in text + + +class TestCompletionPopupCss: + """Textual converts rich Text to Content and drops rich no_wrap and + overflow attributes, so cropping is enforced by the widget CSS. + """ + + async def test_static_crops_long_lines_with_real_css(self): + """Mount a Static with the real popup CSS and verify visual rows + equal logical rows on a narrow terminal (no wrapping).""" + from textual.app import App, ComposeResult + from textual.widgets import Static + + class PopupApp(App[None]): + CSS = _COMPLETIONS_CSS + + def compose(self) -> ComposeResult: + yield Static("", id="completions") + + text = _render_completion_text(_long_candidates(), 0, 20) + logical_rows = len(text.plain.splitlines()) + + app = PopupApp() + async with app.run_test(size=(40, 30)) as pilot: + widget = app.query_one("#completions", Static) + widget.display = True + widget.update(text) + await pilot.pause() + assert widget.size.height == logical_rows + rendered = [ + "".join(seg.text for seg in widget.render_line(y)) + for y in range(widget.size.height) + ] + assert any("…" in line for line in rendered) + + +def _make_chat_app(n_lines: int): + """Minimal app mirroring the real chat + completion popup layout.""" + from textual.app import App, ComposeResult + from textual.containers import Container, VerticalScroll + from textual.widgets import Static + + class ChatApp(App[None]): + CSS = ( + "Screen { layout: vertical; }\n" + "#chat { height: 1fr; }\n" + "#input-shell { height: auto; }\n" + ) + _COMPLETIONS_CSS + + def compose(self) -> ComposeResult: + with VerticalScroll(id="chat"): + for i in range(n_lines): + yield Static(f"line {i}") + with Container(id="input-shell"): + yield Static("", id="completions") + + return ChatApp() + + +async def _toggle_popup_cycle(app, pilot): + """din0s's repro: popup open -> scroll to bottom -> hide -> reopen.""" + from textual.containers import VerticalScroll + from textual.widgets import Static + + chat = app.query_one("#chat", VerticalScroll) + comp = app.query_one("#completions", Static) + text = _render_completion_text( + _candidates([(f"/c{i}", "Session") for i in range(14)]), -1, 15 + ) + comp.update(text) + comp.display = True + await pilot.pause() + chat.scroll_end(animate=False) + await pilot.pause() + comp.display = False + await pilot.pause() + return chat, comp + + +class TestNormalizeChatScroll: + """Popup show/hide resizes the chat viewport. For an anchored chat + whose content then fits, Textual's compositor pushes ``scroll_y`` + negative (bypasses the validator, issue #301 family) — the scrollbar + then renders as if scrolled to the bottom while the content sits at + the top. ``_normalize_chat_scroll`` repairs the state. + """ + + async def test_releases_anchor_and_pins_top_when_content_fits(self): + from textual.widget import Widget + + app = _make_chat_app(18) + async with app.run_test(size=(80, 30)) as pilot: + chat, comp = await _toggle_popup_cycle(app, pilot) + chat.anchor() + await pilot.pause() + # Deterministically inject the compositor's anchored-scroll + # bypass (set_reactive skips validator AND watcher — see + # textual _compositor.py) instead of racing its layout pass. + chat.set_reactive(Widget.scroll_y, -12.0) + chat.set_reactive(Widget.scroll_target_y, -12.0) + _normalize_chat_scroll(chat) + await pilot.pause() + assert chat.scroll_y == 0 + assert not chat.is_anchored + # The scrollbar thumb must not keep a stale position: when it + # becomes visible again (popup reopens) it would render as if + # scrolled to the bottom while the content sits at the top. + comp.display = True + await pilot.pause() + _normalize_chat_scroll(chat) + await pilot.pause() + assert chat.vertical_scrollbar.position == chat.scroll_y + + async def test_keeps_anchored_overflowing_chat_pinned(self): + app = _make_chat_app(60) + async with app.run_test(size=(80, 30)) as pilot: + chat, _comp = await _toggle_popup_cycle(app, pilot) + chat.anchor() + await pilot.pause() + _normalize_chat_scroll(chat) + await pilot.pause() + assert chat.is_anchored + assert chat.scroll_y == chat.max_scroll_y + + async def test_preserves_position_when_user_scrolled_up(self): + app = _make_chat_app(60) + async with app.run_test(size=(80, 30)) as pilot: + chat, comp = await _toggle_popup_cycle(app, pilot) + chat.scroll_to(y=10, animate=False) + await pilot.pause() + comp.display = True + await pilot.pause() + _normalize_chat_scroll(chat) + await pilot.pause() + assert chat.scroll_y == 10 diff --git a/tests/test_tui_widgets.py b/tests/test_tui_widgets.py index 9015968..8bf7173 100644 --- a/tests/test_tui_widgets.py +++ b/tests/test_tui_widgets.py @@ -695,7 +695,6 @@ class TestCompletionLogic(unittest.TestCase): def _make_app(self, comp_items=None, comp_index=-1): """Return a stub app-like object with completion state.""" - from rich.text import Text # Fake Input widget ------------------------------------------------- class _FakeInput: @@ -795,29 +794,12 @@ class TestCompletionLogic(unittest.TestCase): self.query_one("#completions").display = False def _render_completions(self): + from EvoScientist.cli.tui_interactive import _render_completion_text + comp_widget = self.query_one("#completions") - comp_text = Text() - for i, candidate in enumerate(self._comp_items): - cmd, desc = candidate.text, candidate.description - cat = getattr(candidate, "category", "") - if cat and ( - i == 0 - or getattr(self._comp_items[i - 1], "category", "") != cat - ): - if i > 0: - comp_text.append("\n") - comp_text.append(f" {cat}\n", style="bold #6b7280") - if i == self._comp_index: - comp_text.append(" \u25b8 ", style="bold") - comp_text.append(f"{cmd:<28}", style="bold") - comp_text.append(desc, style="bold") - else: - comp_text.append(" ", style="#888888") - comp_text.append(f"{cmd:<28}", style="#888888") - comp_text.append(desc, style="#888888") - if i < len(self._comp_items) - 1: - comp_text.append("\n") - comp_widget.update(comp_text) + comp_widget.update( + _render_completion_text(self._comp_items, self._comp_index, 15) + ) def on_key(self, key: str): comp_widget = self.query_one("#completions") diff --git a/uv.lock b/uv.lock index 5254262..320e872 100644 --- a/uv.lock +++ b/uv.lock @@ -961,6 +961,7 @@ dependencies = [ { name = "langchain-openrouter" }, { name = "langgraph-checkpoint-sqlite" }, { name = "langgraph-cli", extra = ["inmem"] }, + { name = "langgraph-sdk" }, { name = "lazy-loader" }, { name = "markdownify" }, { name = "nest-asyncio" }, @@ -1068,6 +1069,7 @@ requires-dist = [ { name = "langchain-openrouter", specifier = ">=0.2.5" }, { name = "langgraph-checkpoint-sqlite", specifier = ">=3.0" }, { name = "langgraph-cli", extras = ["inmem"], specifier = ">=0.4" }, + { name = "langgraph-sdk", specifier = ">=0.4" }, { name = "lark-oapi", marker = "extra == 'all-channels'", specifier = ">=1.4.0" }, { name = "lark-oapi", marker = "extra == 'feishu'", specifier = ">=1.4.0" }, { name = "lazy-loader", specifier = ">=0.5" },