diff --git a/CHANGELOG.md b/CHANGELOG.md index 449284450..c1aafe0c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,23 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Added + +- `zeph-tui`: inline, non-modal `@` mention picker (issues #6647, #6648), replacing the + old modal file picker. Typing `@` at word-start inserts the character and opens a popup + with `All | Files | Skills | Agents` category tabs (Left/Right to cycle, Up/Down to + select, Tab/Enter to accept without submitting, Esc to dismiss without touching the + buffer). Typing never gets captured away — every keystroke lands in the input buffer, + and the popup only reflects/filters it. Accepting a File inserts the bare repo-relative + path, a Skill inserts the plain name, and an Agent retains the `@` sigil for + slash-command dispatch. The Files category reuses the existing background-built file + index; Skills are delivered via a new `Channel::send_skill_catalog` event emitted at + agent startup and on skill hot-reload; Agents read directly from already-available + runtime metrics. +- `zeph-core`: new `Channel::send_skill_catalog` method (default no-op) and + `AgentEvent::SkillCatalog`, forwarded by `AnyChannel`, `GatewayChannel`, and + `AppChannel`, overridden by `TuiChannel` to feed the mention picker's Skills tab. + ### Changed - `zeph-tui`: moved the busy spinner and humanized activity verb from the bottom status diff --git a/crates/zeph-channels/src/any.rs b/crates/zeph-channels/src/any.rs index 625442226..39f402f11 100644 --- a/crates/zeph-channels/src/any.rs +++ b/crates/zeph-channels/src/any.rs @@ -11,8 +11,8 @@ //! that expands each trait method into a `match` over the active variant. use zeph_core::channel::{ - Channel, ChannelError, ChannelMessage, ElicitationRequest, ElicitationResponse, StopHint, - ToolOutputEvent, ToolStartEvent, + Channel, ChannelError, ChannelMessage, ElicitationRequest, ElicitationResponse, + SkillCatalogItem, StopHint, ToolOutputEvent, ToolStartEvent, }; use crate::cli::CliChannel; @@ -148,6 +148,10 @@ impl Channel for AnyChannel { dispatch_channel!(self, send_status, text) } + async fn send_skill_catalog(&mut self, items: &[SkillCatalogItem]) -> Result<(), ChannelError> { + dispatch_channel!(self, send_skill_catalog, items) + } + async fn send_queue_count(&mut self, count: usize) -> Result<(), ChannelError> { dispatch_channel!(self, send_queue_count, count) } diff --git a/crates/zeph-core/src/agent/mod.rs b/crates/zeph-core/src/agent/mod.rs index d2fae6e85..8671fc321 100644 --- a/crates/zeph-core/src/agent/mod.rs +++ b/crates/zeph-core/src/agent/mod.rs @@ -428,6 +428,18 @@ impl Agent { self.load_and_cache_session_digest().await; self.maybe_send_resume_recap().await; + // Emit the initial skill catalog once at startup (spec 084 §6, issue #6648) so a + // TUI mention picker opened before the first hot-reload still sees real skill + // names/descriptions instead of an indefinite "loading skills…" placeholder. + let initial_skill_catalog = self.skill_catalog_items().await; + if let Err(e) = self + .channel + .send_skill_catalog(&initial_skill_catalog) + .await + { + tracing::warn!("failed to emit initial skill catalog: {e}"); + } + // AutoSkill A6: start periodic heuristic promotion task at session startup so it runs // even when the main loop exits early due to an error (spec 061). The function guards // against double-spawn via a heuristic_promotion_handle.is_some() check. diff --git a/crates/zeph-core/src/agent/skill_reload.rs b/crates/zeph-core/src/agent/skill_reload.rs index e92b90673..f9ccc83ef 100644 --- a/crates/zeph-core/src/agent/skill_reload.rs +++ b/crates/zeph-core/src/agent/skill_reload.rs @@ -16,6 +16,45 @@ use zeph_skills::matcher::{SkillMatcher, SkillMatcherBackend}; use zeph_skills::registry::SkillRegistry; impl Agent { + /// Builds the current skill catalog (name + description) for + /// [`Channel::send_skill_catalog`], excluding blocked skills. + /// + /// Shared by the startup emit (`Agent::run`) and the hot-reload emit below so both + /// apply the same [`zeph_common::SkillTrustLevel::Blocked`] filter — reading + /// `registry.read().all_meta()` raw at only one of the two sites would let blocked + /// skills appear in the mention picker until the first hot-reload, then silently + /// vanish (M2). + pub(super) async fn skill_catalog_items(&mut self) -> Vec { + let all_meta: Vec = self + .services + .skill + .registry + .read() + .all_meta() + .into_iter() + .cloned() + .collect(); + let trust_map = match self.build_skill_trust_map().await { + crate::agent::trust_commands::SkillTrustMapLoad::Fresh(map) => map, + crate::agent::trust_commands::SkillTrustMapLoad::LoadFailed => { + self.services.skill.trust_snapshot.read().clone() + } + }; + all_meta + .into_iter() + .filter(|m| { + !matches!( + trust_map.get(&m.name), + Some(snap) if snap.trust_level == zeph_common::SkillTrustLevel::Blocked + ) + }) + .map(|m| crate::channel::SkillCatalogItem { + name: m.name, + description: m.description, + }) + .collect() + } + /// Update trust DB records for all reloaded skills. async fn update_trust_for_reloaded_skills( &mut self, @@ -287,6 +326,14 @@ impl Agent { // next turn's `rebuild_system_prompt` overwrites it (#6413). self.recompute_prompt_tokens(); + // Re-emit the catalog so an open TUI mention picker's Skills tab (spec 084 §6) + // picks up additions/removals/blocked-status changes on hot-reload, not just at + // startup. + let catalog_items = self.skill_catalog_items().await; + if let Err(e) = self.channel.send_skill_catalog(&catalog_items).await { + tracing::warn!("failed to re-emit skill catalog after reload: {e}"); + } + self.channel.send_status_best_effort("").await; tracing::info!( "reloaded {} skill(s)", diff --git a/crates/zeph-core/src/channel.rs b/crates/zeph-core/src/channel.rs index ead743b84..fa4d8c620 100644 --- a/crates/zeph-core/src/channel.rs +++ b/crates/zeph-core/src/channel.rs @@ -212,6 +212,33 @@ pub struct ChannelMessage { pub owner_key: Option, } +/// One entry in the skill catalog delivered to channels via +/// [`Channel::send_skill_catalog`] (spec 084 §6, issue #6648). +/// +/// Carries just enough to populate a discovery UI (e.g. the TUI's inline `@` mention +/// picker Skills tab) — name plus a human-readable description — without pulling in +/// `zeph-skills`' full `SkillMeta` (which also carries filesystem paths, trust +/// metadata, and resource lists that channels have no business seeing). +/// +/// # Examples +/// +/// ``` +/// use zeph_core::channel::SkillCatalogItem; +/// +/// let item = SkillCatalogItem { +/// name: "web_search".to_owned(), +/// description: "Search the web for current information".to_owned(), +/// }; +/// assert_eq!(item.name, "web_search"); +/// ``` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillCatalogItem { + /// Skill name, as registered in `SKILL.md` frontmatter. + pub name: String, + /// Human-readable description from the skill's frontmatter. + pub description: String, +} + /// Upper bound on [`Channel::send_status_best_effort`]. Status sends are a UX nicety, not a /// value the agent turn depends on, so a slow or rate-limited channel (see issue #6094 — Discord /// and Slack's 429 retry loop can otherwise take minutes) must never stall the turn loop past @@ -332,6 +359,21 @@ pub trait Channel: Send { async { Ok(()) } } + /// Send the full skill catalog (name + description), delivered once at agent + /// startup and re-emitted on skill hot-reload (spec 084 §6, issue #6648). No-op by + /// default — most channels have no discovery UI to populate. `TuiChannel` overrides + /// this to feed the inline `@` mention picker's Skills tab. + /// + /// # Errors + /// + /// Returns an error if the underlying I/O fails. + fn send_skill_catalog( + &mut self, + _items: &[SkillCatalogItem], + ) -> impl Future> + Send { + async { Ok(()) } + } + /// Send a bounded transcript slice for `/history` backfill (spec-068 §13.6-§13.7). /// /// Default: renders `entries` into one flat string via diff --git a/crates/zeph-tui/src/app/action.rs b/crates/zeph-tui/src/app/action.rs index 00f2d22e9..102e687af 100644 --- a/crates/zeph-tui/src/app/action.rs +++ b/crates/zeph-tui/src/app/action.rs @@ -22,6 +22,14 @@ pub(crate) enum VertDir { Down, } +/// Horizontal tab-cycling direction for the mention picker (FR-004). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(dead_code)] +pub(crate) enum HorizDir { + Left, + Right, +} + /// Sub-edits for the command-palette text field. #[derive(Debug, Clone, PartialEq, Eq)] #[allow(dead_code)] @@ -147,17 +155,19 @@ pub(crate) enum Action { /// Accept the currently selected palette entry. PaletteAccept, - // ── File picker ──────────────────────────────────────────────────────────── - /// Open the file picker. - OpenFilePicker, - /// Close the file picker without selecting. - CloseFilePicker, - /// Move the file picker selection. - FilePickerMove(VertDir), - /// Type in the file picker filter field. - FilePickerInput(PaletteEdit), - /// Accept the currently selected file. - FilePickerAccept, + // ── Mention picker (#6647) ────────────────────────────────────────────────── + // No `OpenMentionPicker`/`MentionPickerInput` — opening is a side effect of + // `InsertChar('@')`, and typing continues to flow through the existing + // `InsertChar`/`Delete*`/`MoveCursor` arms (the query is derived from the buffer, + // never mirrored into a second string; see `reducer::mention_picker_query`). + /// Close the mention picker without accepting. + CloseMentionPicker, + /// Move the mention picker selection within the active tab. + MentionPickerMove(VertDir), + /// Cycle the active category tab (FR-004). + MentionPickerTabChange(HorizDir), + /// Accept the currently selected entry (Tab or Enter). + MentionPickerAccept, // ── Slash autocomplete ───────────────────────────────────────────────────── /// Move the autocomplete selection. @@ -237,6 +247,50 @@ pub(crate) enum Action { Dispatch(TuiCommand), } +impl Action { + /// Returns `true` only for actions that provably cannot mutate the input buffer, + /// the cursor position, or the active session — i.e. actions the mention-picker + /// resync (`reducer::sync_mention_picker`) can safely skip. + /// + /// This is a **fail-closed denylist**, not an allowlist of "buffer-touching" + /// actions: everything not listed here defaults to `false` (resync runs), so a + /// future `Action` variant added by someone unaware of the mention picker is safe + /// by construction. An allowlist shape was tried first and missed `Dispatch` + /// (reaches `prefill_input`/`TuiCommand::PrefillVerbatim`, both buffer-mutating) + /// and session switch (`mention_picker` is a global `App` field that must not + /// survive into a different session's buffer) — see spec 084 R1 S1/S2. + #[must_use] + pub(crate) fn preserves_mention_span(&self) -> bool { + matches!( + self, + Self::ScrollLines(_) + | Self::ScrollPage(_) + | Self::ScrollToTop + | Self::ScrollToBottom + | Self::ToggleToolExpanded + | Self::CycleToolDensity + | Self::ToggleSidePanels + | Self::ToggleHelp + | Self::SetHelp(_) + | Self::CyclePanelFocus + | Self::SetActivePanel(_) + | Self::TogglePanelCollapse(_) + | Self::ToggleTaskPanel + | Self::TogglePlanView + | Self::SetViewTarget(_) + | Self::SettingsTabNext + | Self::SettingsTabPrev + | Self::SettingsSelectMove(_) + | Self::SetMouse(_) + | Self::CopyLastAssistant + | Self::CopyLastCodeBlock(_) + | Self::CloseMentionPicker + | Self::MentionPickerMove(_) + | Self::MentionPickerTabChange(_) + ) + } +} + /// Cursor movement kinds for [`Action::MoveCursor`]. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[allow(dead_code)] diff --git a/crates/zeph-tui/src/app/draw.rs b/crates/zeph-tui/src/app/draw.rs index 6847a200b..9c802fd4f 100644 --- a/crates/zeph-tui/src/app/draw.rs +++ b/crates/zeph-tui/src/app/draw.rs @@ -95,8 +95,8 @@ impl App { widgets::input::render(self, frame, layout.input, busy, spinner_idx, motion); widgets::status::render(self, &self.metrics, frame, layout.status); - if let Some(state) = &self.file_picker_state { - widgets::file_picker::render(state, frame, layout.input, &self.theme); + if let Some(state) = &self.mention_picker { + widgets::mention_picker::render(self, state, frame, layout.input, &self.theme); } if let Some(state) = &self.slash_autocomplete { diff --git a/crates/zeph-tui/src/app/events.rs b/crates/zeph-tui/src/app/events.rs index 7d1687365..da3e3ea04 100644 --- a/crates/zeph-tui/src/app/events.rs +++ b/crates/zeph-tui/src/app/events.rs @@ -25,7 +25,16 @@ impl App { self.sessions.current_mut().render_cache.clear(); } AppEvent::Agent(agent_event) => self.handle_agent_event(agent_event), - AppEvent::Paste(text) => self.handle_paste(&text), + // Routed through `reduce` (S1, spec 084) rather than calling `handle_paste` + // directly: this is behaviour-identical (the `InsertText` reducer arm just + // calls `handle_paste`) but gives paste the mention-picker resync for free — + // without it, pasting before the `@` shifts the buffer while `at_char_index` + // stays fixed, which can invert `MentionPickerAccept`'s replacement range. + AppEvent::Paste(text) => { + let effects = + crate::app::reducer::reduce(self, crate::app::action::Action::InsertText(text)); + crate::app::reducer::run_effects(self, effects); + } AppEvent::Mouse(m) => self.handle_mouse(m), } } @@ -304,6 +313,17 @@ impl App { AgentEvent::HistoryBackfill(entries) => { self.backfill_history_display_only(&entries); } + AgentEvent::SkillCatalog(items) => { + self.skill_catalog = Some(items); + if self.mention_picker.is_some() { + let query = crate::app::reducer::mention_picker_query(self); + let skills = self.skill_catalog.clone(); + if let Some(picker) = self.mention_picker.as_mut() { + picker.catalog.skills = skills; + picker.refilter(&query); + } + } + } } } diff --git a/crates/zeph-tui/src/app/keys.rs b/crates/zeph-tui/src/app/keys.rs index 28d51fcaf..b648400b9 100644 --- a/crates/zeph-tui/src/app/keys.rs +++ b/crates/zeph-tui/src/app/keys.rs @@ -5,10 +5,12 @@ use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; pub(super) const SCROLL_STEP_PAGE: usize = 10; -use crate::app::action::{Action, CursorMove, ElicitationEdit, PaletteEdit, ScrollDir, VertDir}; +use crate::app::action::{ + Action, CursorMove, ElicitationEdit, HorizDir, PaletteEdit, ScrollDir, VertDir, +}; use crate::app::reducer::{reduce, run_effects}; use crate::command::TuiCommand; -use crate::file_picker::{FileIndex, FilePickerState}; +use crate::file_picker::FileIndex; use crate::layout::truncate_to_width; use super::{ @@ -64,11 +66,6 @@ impl App { return Self::decode_palette_key(key); } - // File picker - if self.file_picker_state.is_some() { - return Self::decode_file_picker_key(key); - } - // Transcript search (issue #6023): routed mode-agnostically at the top level // (unlike reverse-search, which is Insert-only) so Ctrl+F works whether it was // opened from Normal or Insert mode, and so the two overlays are mutually @@ -123,18 +120,6 @@ impl App { } } - fn decode_file_picker_key(key: KeyEvent) -> Option { - match key.code { - KeyCode::Esc => Some(Action::CloseFilePicker), - KeyCode::Enter | KeyCode::Tab => Some(Action::FilePickerAccept), - KeyCode::Up => Some(Action::FilePickerMove(VertDir::Up)), - KeyCode::Down => Some(Action::FilePickerMove(VertDir::Down)), - KeyCode::Char(c) => Some(Action::FilePickerInput(PaletteEdit::PushChar(c))), - KeyCode::Backspace => Some(Action::FilePickerInput(PaletteEdit::PopChar)), - _ => None, - } - } - #[allow(clippy::too_many_lines)] // large match over all TuiCommand variants pub(super) fn execute_command(&mut self, cmd: TuiCommand) { match cmd { @@ -389,8 +374,12 @@ impl App { return; } // Pure-UI overlays carry no response channel — safe to dismiss silently. + // mention_picker joins this block (not the resync predicate) because + // `input`/`cursor_position` are session-local while `mention_picker` is a + // global `App` field — a resync could otherwise re-derive a valid-looking span + // from the *other* session's buffer after the switch (S2). self.command_palette = None; - self.file_picker_state = None; + self.mention_picker = None; self.slash_autocomplete = None; let prev = self.sessions.active(); match cmd { @@ -919,6 +908,15 @@ impl App { if self.slash_autocomplete.is_some() { return Self::decode_slash_autocomplete_key(key); } + // Mention picker is an Insert-mode overlay, not a modal: only Esc/Tab/Enter/ + // arrows are intercepted here (`None` for anything else), so Space, Backspace, + // Delete, Home/End, Alt+arrows and Ctrl+* all fall through to normal Insert + // decoding below and rely on `sync_mention_picker` to close/refilter as needed. + if self.mention_picker.is_some() + && let Some(a) = Self::decode_mention_picker_key(key) + { + return Some(a); + } if let Some(a) = Self::decode_insert_text_key(key) { return Some(a); } @@ -1056,12 +1054,33 @@ impl App { None } } - KeyCode::Char('@') => Some(Action::OpenFilePicker), KeyCode::Char(c) => Some(Action::InsertChar(c)), _ => None, } } + /// Key routing for the open mention-picker popup (NFR-005/invariant 4): `Esc` + /// closes without submitting, is checked before `decode_insert_text_key`'s + /// `Esc → EnterNormal` fallback so Insert mode is retained. `Tab`/`Enter` accept + /// (unlike slash-autocomplete, accepting a mention never auto-submits). Plain + /// `Left`/`Right` cycle tabs (FR-004/D2) rather than moving the cursor — but + /// `Alt+Left`/`Alt+Right` (word-boundary cursor movement) are deliberately + /// excluded here so they fall through to normal Insert-mode decoding, where + /// `sync_mention_picker` closes the popup if they exit the `@query` span. + /// Everything else returns `None` and falls through the same way. + fn decode_mention_picker_key(key: KeyEvent) -> Option { + let is_alt = key.modifiers.contains(KeyModifiers::ALT); + match key.code { + KeyCode::Esc => Some(Action::CloseMentionPicker), + KeyCode::Tab | KeyCode::Enter => Some(Action::MentionPickerAccept), + KeyCode::Up => Some(Action::MentionPickerMove(VertDir::Up)), + KeyCode::Down => Some(Action::MentionPickerMove(VertDir::Down)), + KeyCode::Left if !is_alt => Some(Action::MentionPickerTabChange(HorizDir::Left)), + KeyCode::Right if !is_alt => Some(Action::MentionPickerTabChange(HorizDir::Right)), + _ => None, + } + } + fn decode_slash_autocomplete_key(key: KeyEvent) -> Option { match key.code { KeyCode::Esc => Some(Action::CloseSlashAutocomplete), @@ -1171,39 +1190,42 @@ impl App { self.sessions.current_mut().cursor_position = self.char_count(); } - pub(super) fn open_file_picker(&mut self) { + /// Kicks off the background file-index build if needed. Never opens the mention + /// picker itself (that already happened synchronously in the reducer's `InsertChar` + /// arm) — this is the race-free fix for FR-011/NFR-004: no keystroke path ever + /// depends on the index arriving. + pub(super) fn ensure_file_index(&mut self) { use std::sync::Arc; let root = std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")); let needs_rebuild = self.file_index.as_ref().is_none_or(FileIndex::is_stale); - if needs_rebuild && self.pending_file_index.is_none() { - self.sessions.current_mut().status_label = Some("indexing files...".to_owned()); - // Status change counts as progress so the wave animates (never reads Stalled). - self.last_progress_at = std::time::Instant::now(); - let pending = if let Some(sup) = &self.task_supervisor { - let handle = sup.spawn_blocking(Arc::from("tui.file_index.build"), move || { - FileIndex::build(&root) - }); - super::PendingFileIndex::Supervised(handle) - } else { - // EXEMPT: supervisor not wired (test environments); bare spawn is acceptable here - // because the oneshot receiver is stored in pending_file_index and polled every tick. - let (tx, rx) = oneshot::channel(); - tokio::task::spawn_blocking(move || { - let _ = tx.send(FileIndex::build(&root)); - }); - super::PendingFileIndex::Bare(rx) - }; - self.pending_file_index = Some(pending); + if !needs_rebuild || self.pending_file_index.is_some() { return; } - if let Some(idx) = &self.file_index { - self.file_picker_state = Some(FilePickerState::new(idx)); - } + self.sessions.current_mut().status_label = Some("indexing files...".to_owned()); + // Status change counts as progress so the wave animates (never reads Stalled). + self.last_progress_at = std::time::Instant::now(); + let pending = if let Some(sup) = &self.task_supervisor { + let handle = sup.spawn_blocking(Arc::from("tui.file_index.build"), move || { + FileIndex::build(&root) + }); + super::PendingFileIndex::Supervised(handle) + } else { + // EXEMPT: supervisor not wired (test environments); bare spawn is acceptable here + // because the oneshot receiver is stored in pending_file_index and polled every tick. + let (tx, rx) = oneshot::channel(); + tokio::task::spawn_blocking(move || { + let _ = tx.send(FileIndex::build(&root)); + }); + super::PendingFileIndex::Bare(rx) + }; + self.pending_file_index = Some(pending); } - /// Checks if the background file index build has completed and, if so, - /// installs the result and opens the picker. + /// Checks if the background file index build has completed and, if so, installs + /// the result and refreshes an open mention picker's Files category (FR-011/ + /// NFR-004) — seamless transition, no input loss even if the popup opened before + /// the index was ready. pub fn poll_pending_file_index(&mut self) { let Some(pending) = self.pending_file_index.take() else { return; @@ -1228,10 +1250,16 @@ impl App { }; match poll_result { Some(Ok(idx)) => { - let picker = FilePickerState::new(&idx); + let files_arc = idx.paths_arc(); self.file_index = Some(idx); - self.file_picker_state = Some(picker); self.sessions.current_mut().status_label = None; + if self.mention_picker.is_some() { + let query = crate::app::reducer::mention_picker_query(self); + if let Some(picker) = self.mention_picker.as_mut() { + picker.catalog.files = Some(files_arc); + picker.refilter(&query); + } + } } Some(Err(())) | None => { self.sessions.current_mut().status_label = None; diff --git a/crates/zeph-tui/src/app/mod.rs b/crates/zeph-tui/src/app/mod.rs index 558f149fe..f287d9ad9 100644 --- a/crates/zeph-tui/src/app/mod.rs +++ b/crates/zeph-tui/src/app/mod.rs @@ -13,11 +13,12 @@ use zeph_common::task_supervisor::{BlockingHandle, TaskSupervisor}; use crate::command::TuiCommand; use crate::event::AgentEvent; -use crate::file_picker::{FileIndex, FilePickerState}; +use crate::file_picker::FileIndex; use crate::hyperlink::HyperlinkSpan; use crate::metrics::MetricsSnapshot; use crate::session::SessionRegistry; use crate::widgets::command_palette::CommandPaletteState; +use crate::widgets::mention_picker::MentionPickerState; use crate::widgets::slash_autocomplete::SlashAutocompleteState; use crate::widgets::tool_view::ToolDensity; @@ -398,7 +399,12 @@ pub struct App { elicitation_state: Option, command_palette: Option, command_tx: Option>, - file_picker_state: Option, + pub(crate) mention_picker: Option, + /// Full skill catalog (name + description) delivered once at startup and + /// re-emitted on hot-reload (`AgentEvent::SkillCatalog`, spec 084 §6/D1). `None` + /// until the first emit arrives — distinguishes "still loading" from "loaded and + /// genuinely empty" for the mention picker's Skills tab (FR-011/FR-019). + pub(crate) skill_catalog: Option>, file_index: Option, slash_autocomplete: Option, reverse_search: Option, diff --git a/crates/zeph-tui/src/app/reducer.rs b/crates/zeph-tui/src/app/reducer.rs index 2762294e4..60ddaf866 100644 --- a/crates/zeph-tui/src/app/reducer.rs +++ b/crates/zeph-tui/src/app/reducer.rs @@ -10,12 +10,14 @@ use zeph_core::channel::ElicitationResponse; -use super::action::{Action, CursorMove, ElicitationEdit, PaletteEdit, ScrollDir, VertDir}; +use super::action::{ + Action, CursorMove, ElicitationEdit, HorizDir, PaletteEdit, ScrollDir, VertDir, +}; use super::state::CTRL_C_DOUBLE_PRESS_TICKS; use super::{App, ChatMessage, InputMode, MessageRole, Panel, format_security_report}; use crate::command::TuiCommand; -use crate::file_picker::FilePickerState; use crate::widgets::command_palette::CommandPaletteState; +use crate::widgets::mention_picker::{MentionKind, MentionPickerState}; use crate::widgets::slash_autocomplete::SlashAutocompleteState; const MAX_INPUT_HISTORY: usize = 500; @@ -31,8 +33,11 @@ pub(crate) enum Effect { SendUserInput(String), /// Copy `text` to the system clipboard. CopyToClipboard(String), - /// Trigger the file indexer for the file picker. - StartFileIndex, + /// Kick off the background file-index build if needed. Unlike the old + /// `StartFileIndex`, this never opens the picker itself (FR-011/NFR-004) — the + /// mention picker already opened synchronously in `reduce_inner`'s `InsertChar` + /// arm, so no keystroke path ever depends on the index arriving. + EnsureFileIndex, /// Enable or disable mouse capture in the terminal backend. /// /// Stored in `pending_mouse_capture` and drained by the `tui_loop` @@ -44,11 +49,105 @@ pub(crate) enum Effect { /// Apply `action` to `app` and return any side-effects to run. /// -/// This is the single mutation point for keyboard and mouse paths (INV-R1). -/// The function must not perform I/O, channel sends, or any blocking work -/// (INV-R2). Callers must pass the returned effects to [`run_effects`]. -#[allow(clippy::too_many_lines)] +/// This is the single mutation point for keyboard and mouse paths (INV-R1). The +/// function must not perform I/O, channel sends, or any blocking work (INV-R2). +/// Callers must pass the returned effects to [`run_effects`]. +/// +/// Thin wrapper around [`reduce_inner`] that reconciles the mention picker +/// (`sync_mention_picker`) after every action except the small, fail-closed set of +/// actions [`Action::preserves_mention_span`] proves cannot touch the input buffer, +/// cursor, or active session. This is deliberately a denylist-shaped allowlist: a +/// future `Action` variant nobody remembers to add here defaults to resyncing, which +/// is always safe (`sync_mention_picker` is a no-op when no picker is open) — the +/// alternative (an allowlist of "safe to skip" actions) was tried first and missed +/// `Action::Dispatch` reaching `prefill_input`/`PrefillVerbatim` and session-switch, +/// both of which mutate the buffer while looking like inert dispatch wrappers. pub(crate) fn reduce(app: &mut App, action: Action) -> Vec { + let inert = action.preserves_mention_span(); + let effects = reduce_inner(app, action); + if !inert { + sync_mention_picker(app); + } + effects +} + +/// Recomputes the mention query from the current buffer/cursor state and reconciles +/// the popup: closes it if the `@query` span was invalidated by whatever `reduce_inner` +/// just did (S1/S2), otherwise re-filters using the freshly derived query. First line +/// is a fast return since this now runs on nearly every action. +fn sync_mention_picker(app: &mut App) { + let Some(at_char_index) = app.mention_picker.as_ref().map(|p| p.at_char_index) else { + return; + }; + let cursor = app.sessions.current().cursor_position; + let at_char = app.sessions.current().input.chars().nth(at_char_index); + if at_char != Some('@') || cursor <= at_char_index { + app.mention_picker = None; + return; + } + let query = mention_picker_query(app); + if query.chars().any(char::is_whitespace) { + app.mention_picker = None; + return; + } + if let Some(picker) = app.mention_picker.as_mut() { + picker.refilter(&query); + } +} + +/// Derives the mention query — the text between the triggering `@` and the cursor — +/// without maintaining a second copy of it in `MentionPickerState`. Uses char-iterator +/// extraction throughout (never direct byte indexing) so a `cursor_position > +/// char_count()` buffer (reachable via `prefill_input`'s pre-existing byte/char-index +/// bug, `keys.rs`) cannot panic here (M10). +pub(super) fn mention_picker_query(app: &App) -> String { + let Some(at_char_index) = app.mention_picker.as_ref().map(|p| p.at_char_index) else { + return String::new(); + }; + let cursor = app.sessions.current().cursor_position; + if cursor <= at_char_index { + return String::new(); + } + app.sessions + .current() + .input + .chars() + .skip(at_char_index + 1) + .take(cursor - at_char_index - 1) + .collect() +} + +/// Char index of the first whitespace at or after `from`, or the buffer's char count if +/// none is found — the end of the "mention token" for [`Action::MentionPickerAccept`] +/// (M4/M9). Uses the same `char::is_whitespace` definition as `is_mention_word_start` +/// so both ends of the token agree on what a boundary is. +fn mention_token_end(app: &App, from: usize) -> usize { + app.sessions + .current() + .input + .chars() + .enumerate() + .skip(from) + .find(|(_, c)| c.is_whitespace()) + .map_or_else(|| app.char_count(), |(i, _)| i) +} + +/// Word-start rule for opening the picker (FR-001/FR-002/FR-020): position 0, or the +/// preceding char is whitespace. +fn is_mention_word_start(app: &App, pos: usize) -> bool { + if pos == 0 { + return true; + } + app.sessions + .current() + .input + .chars() + .nth(pos - 1) + .is_some_and(char::is_whitespace) +} + +#[allow(clippy::too_many_lines)] +fn reduce_inner(app: &mut App, action: Action) -> Vec { match action { // ── Scroll ───────────────────────────────────────────────────────────── Action::ScrollLines(delta) => { @@ -212,6 +311,18 @@ pub(crate) fn reduce(app: &mut App, action: Action) -> Vec { if c == '/' && was_empty && app.slash_autocomplete.is_none() { app.slash_autocomplete = Some(SlashAutocompleteState::new()); } + // Word-start `@` opens the mention picker (FR-001/FR-002/FR-020); mid-word + // `@` (e.g. `user@example.com`) is always inserted as a literal character. + // A single active mention at a time, mirroring slash-autocomplete's exclusion. + if c == '@' + && app.mention_picker.is_none() + && app.slash_autocomplete.is_none() + && is_mention_word_start(app, pos) + { + let catalog = app.mention_catalog(); + app.mention_picker = Some(MentionPickerState::new(pos, catalog)); + return vec![Effect::EnsureFileIndex]; + } vec![] } Action::InsertNewline => { @@ -407,55 +518,76 @@ pub(crate) fn reduce(app: &mut App, action: Action) -> Vec { vec![] } - // ── File picker ───────────────────────────────────────────────────────── - Action::OpenFilePicker => { - vec![Effect::StartFileIndex] - } - Action::CloseFilePicker => { - app.file_picker_state = None; + // ── Mention picker (#6647) ────────────────────────────────────────────── + Action::CloseMentionPicker => { + app.mention_picker = None; vec![] } - Action::FilePickerMove(dir) => { - if let Some(ref mut s) = app.file_picker_state { - match dir { - VertDir::Up => s.move_selection(-1), - VertDir::Down => s.move_selection(1), - } + Action::MentionPickerMove(dir) => { + if let Some(picker) = app.mention_picker.as_mut() { + picker.move_selection(match dir { + VertDir::Up => -1, + VertDir::Down => 1, + }); } vec![] } - Action::FilePickerInput(edit) => { - match edit { - PaletteEdit::PushChar(c) => { - if let Some(ref mut s) = app.file_picker_state { - s.push_char(c); - } - } - PaletteEdit::PopChar => { - let dismissed = app.file_picker_state.as_mut().is_none_or(|s| !s.pop_char()); - if dismissed { - app.file_picker_state = None; - } - } + Action::MentionPickerTabChange(dir) => { + // Query is unaffected by a tab change — only which category it filters. + let query = mention_picker_query(app); + if let Some(picker) = app.mention_picker.as_mut() { + picker.active_tab = match dir { + HorizDir::Left => picker.active_tab.prev(), + HorizDir::Right => picker.active_tab.next(), + }; + picker.refilter(&query); } vec![] } - Action::FilePickerAccept => { - let selected = app - .file_picker_state - .as_ref() - .and_then(FilePickerState::selected_path) - .map(str::to_owned); - app.file_picker_state = None; - if let Some(path_str) = selected { - let pos = app.sessions.current().cursor_position; - let byte_offset = app.byte_offset_of_char(pos); - app.sessions - .current_mut() - .input - .insert_str(byte_offset, &path_str); - app.sessions.current_mut().cursor_position += path_str.chars().count(); + Action::MentionPickerAccept => { + let Some(picker) = app.mention_picker.take() else { + return vec![]; + }; + let at_char_index = picker.at_char_index; + let cursor = app.sessions.current().cursor_position; + let char_count = app.char_count(); + // Defensive guard (S1): an inverted range here would panic in + // `String::replace_range`. Reachable if the buffer shrank or the cursor + // moved left of `@` through some path `sync_mention_picker` didn't catch. + if at_char_index >= char_count || cursor <= at_char_index { + return vec![]; } + let Some(entry) = picker.filtered.get(picker.selected) else { + return vec![]; + }; + // M4: accept replaces the whole mention *token* (through the next + // whitespace), not just up to the cursor — otherwise `"@foo"` with the + // cursor inside the word (e.g. after Alt+Left) would only replace `"@f"` + // and mangle the buffer to `"src/main.rs oo"`. + let token_end = mention_token_end(app, cursor).min(char_count); + let insertion = match entry.kind { + MentionKind::Agent => format!("@{}", entry.display), + MentionKind::File | MentionKind::Skill => entry.display.clone(), + }; + let next_is_space = app + .sessions + .current() + .input + .chars() + .nth(token_end) + .is_some_and(char::is_whitespace); + let mut replacement = insertion; + if !next_is_space { + replacement.push(' '); + } + let start = app.byte_offset_of_char(at_char_index); + let end = app.byte_offset_of_char(token_end); + let inserted_chars = replacement.chars().count(); + app.sessions + .current_mut() + .input + .replace_range(start..end, &replacement); + app.sessions.current_mut().cursor_position = at_char_index + inserted_chars; vec![] } @@ -1058,8 +1190,8 @@ pub(crate) fn run_effects(app: &mut App, effects: Vec) { Ok(()) => app.push_system_message_pub("Copied to clipboard.".to_owned()), Err(e) => app.push_system_message_pub(format!("Copy failed: {e}")), }, - Effect::StartFileIndex => { - app.open_file_picker(); + Effect::EnsureFileIndex => { + app.ensure_file_index(); } Effect::SetMouseCapture(b) => { app.pending_mouse_capture = Some(b); diff --git a/crates/zeph-tui/src/app/state.rs b/crates/zeph-tui/src/app/state.rs index b645af283..0e71ff149 100644 --- a/crates/zeph-tui/src/app/state.rs +++ b/crates/zeph-tui/src/app/state.rs @@ -12,6 +12,7 @@ use zeph_common::task_supervisor::TaskSupervisor; use crate::command::TuiCommand; use crate::event::AgentEvent; +use crate::file_picker::FileIndex; use crate::hyperlink::HyperlinkSpan; use crate::metrics::MetricsSnapshot; use crate::session::SessionRegistry; @@ -77,7 +78,8 @@ impl App { elicitation_state: None, command_palette: None, command_tx: None, - file_picker_state: None, + mention_picker: None, + skill_catalog: None, file_index: None, slash_autocomplete: None, reverse_search: None, @@ -1528,6 +1530,19 @@ impl App { } blocks } + + /// Builds the current [`crate::widgets::mention_picker::MentionCatalog`] snapshot: + /// files from the (possibly not-yet-built) file index, skills from the last + /// `AgentEvent::SkillCatalog` emit, and agents straight from + /// `MetricsSnapshot::agent_definitions` (D1 — no new plumbing needed for agents, + /// spec 084 §6). Every field is an `Arc` clone — O(1), no per-open allocation. + pub(crate) fn mention_catalog(&self) -> crate::widgets::mention_picker::MentionCatalog { + crate::widgets::mention_picker::MentionCatalog { + files: self.file_index.as_ref().map(FileIndex::paths_arc), + skills: self.skill_catalog.clone(), + agents: self.metrics.agent_definitions.clone(), + } + } } #[cfg(test)] diff --git a/crates/zeph-tui/src/app/tests.rs b/crates/zeph-tui/src/app/tests.rs index 1437a26ba..e6ed0b2ad 100644 --- a/crates/zeph-tui/src/app/tests.rs +++ b/crates/zeph-tui/src/app/tests.rs @@ -1754,15 +1754,16 @@ mod command_palette_tests { } } -mod file_picker_tests { +mod mention_picker_tests { use std::fs; + use std::sync::Arc; use super::*; use crate::file_picker::FileIndex; + use crate::widgets::mention_picker::{MentionCatalog, MentionPickerState, MentionTab}; fn make_app_with_index() -> (App, mpsc::Receiver, mpsc::Sender) { - let (app, rx, tx) = make_app(); - (app, rx, tx) + make_app() } fn build_temp_index(files: &[&str]) -> (FileIndex, tempfile::TempDir) { @@ -1778,253 +1779,610 @@ mod file_picker_tests { (idx, dir) } - fn open_picker_with_index(app: &mut App, idx: &FileIndex) { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().to_owned(); - drop(dir.keep()); - app.file_index = Some(FileIndex::build(&path)); - // Replace with our controlled index - app.file_picker_state = Some(crate::file_picker::FilePickerState::new(idx)); + fn catalog_with_files(files: &[&str]) -> MentionCatalog { + MentionCatalog { + files: Some(Arc::new(files.iter().map(|s| (*s).to_owned()).collect())), + skills: Some(Arc::from(Vec::new())), + agents: Arc::from(Vec::new()), + } } - #[test] - fn at_sign_opens_picker_and_does_not_insert_into_input() { - let (mut app, _rx, _tx) = make_app_with_index(); - // Pre-populate a fresh index so open_file_picker can open the picker immediately - // without spawning a background build (which requires a Tokio runtime). - let (idx, _dir) = build_temp_index(&["a.rs"]); + /// Opens the picker through the real key-decode path (not a direct field + /// assignment) so every test exercises the same production code the user does. + fn open_picker_via_key(app: &mut App, files: &[&str]) { + let (idx, _dir) = build_temp_index(files); app.file_index = Some(idx); app.sessions.current_mut().input_mode = InputMode::Insert; let key = KeyEvent::new(KeyCode::Char('@'), KeyModifiers::NONE); app.handle_event(AppEvent::Key(key)); + // The catalog installed by `Effect::EnsureFileIndex` only arrives via + // `poll_pending_file_index`; install it synchronously here for tests that + // don't specifically exercise the "still loading" path. + if let Some(picker) = app.mention_picker.as_mut() { + picker.catalog = catalog_with_files(files); + picker.refilter(""); + } + } + + #[test] + fn at_sign_inserts_char_and_opens_mention_picker() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); assert!( - !app.sessions.current_mut().input.contains('@'), - "@ should not be in input after opening picker" - ); - assert!( - app.file_picker_state.is_some(), - "file_picker_state should be Some after @" + app.sessions.current_mut().input.contains('@'), + "@ must be inserted into the input (FR-001)" ); + assert!(app.mention_picker.is_some()); } #[test] - fn esc_dismisses_picker() { + fn at_sign_mid_word_does_not_open_mention_picker() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["a.rs", "b.rs"]); - open_picker_with_index(&mut app, &idx); - assert!(app.file_picker_state.is_some()); + app.sessions.current_mut().input_mode = InputMode::Insert; + app.sessions.current_mut().input = "user".to_owned(); + app.sessions.current_mut().cursor_position = 4; - let key = KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE); + let key = KeyEvent::new(KeyCode::Char('@'), KeyModifiers::NONE); app.handle_event(AppEvent::Key(key)); - assert!(app.file_picker_state.is_none()); - assert!(app.sessions.current_mut().input.is_empty()); + + assert_eq!(app.sessions.current().input, "user@"); + assert!(app.mention_picker.is_none()); } #[test] - fn enter_inserts_selected_path_and_closes_picker() { + fn at_sign_after_whitespace_opens_picker() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["src/main.rs"]); - open_picker_with_index(&mut app, &idx); + // Pre-populate the file index so `Effect::EnsureFileIndex` does not spawn a + // background build, which requires a Tokio runtime this plain #[test] lacks. + let (idx, _dir) = build_temp_index(&["a.rs"]); + app.file_index = Some(idx); + app.sessions.current_mut().input_mode = InputMode::Insert; + app.sessions.current_mut().input = "hello ".to_owned(); + app.sessions.current_mut().cursor_position = 6; - let selected = app - .file_picker_state - .as_ref() - .unwrap() - .selected_path() - .map(ToOwned::to_owned) - .unwrap(); + let key = KeyEvent::new(KeyCode::Char('@'), KeyModifiers::NONE); + app.handle_event(AppEvent::Key(key)); - let key = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE); + assert!(app.mention_picker.is_some()); + } + + #[test] + fn double_at_sign_second_is_appended_to_query_not_a_new_picker() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + + let key = KeyEvent::new(KeyCode::Char('@'), KeyModifiers::NONE); app.handle_event(AppEvent::Key(key)); - assert!(app.file_picker_state.is_none()); - assert!( - app.sessions.current_mut().input.contains(&selected), - "input should contain selected path" + assert!(app.mention_picker.is_some()); + assert_eq!(app.sessions.current().input, "@@"); + } + + #[test] + fn esc_closes_picker_retains_insert_mode_and_input() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs", "b.rs"]); + assert!(app.mention_picker.is_some()); + let input_before = app.sessions.current().input.clone(); + + let key = KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE); + app.handle_event(AppEvent::Key(key)); + + assert!(app.mention_picker.is_none()); + assert_eq!( + app.sessions.current().input_mode, + InputMode::Insert, + "Esc must not fall through to Normal mode (invariant 4)" ); assert_eq!( - app.sessions.current_mut().cursor_position, - selected.chars().count() + app.sessions.current().input, + input_before, + "Esc must not modify the input" ); } #[test] - fn tab_inserts_selected_path_and_closes_picker() { + fn tab_accepts_file_entry_bare_path_plus_trailing_space() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["README.md"]); - open_picker_with_index(&mut app, &idx); - - let selected = app - .file_picker_state - .as_ref() - .unwrap() - .selected_path() - .map(ToOwned::to_owned) - .unwrap(); + open_picker_via_key(&mut app, &["src/main.rs"]); let key = KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE); app.handle_event(AppEvent::Key(key)); - assert!(app.file_picker_state.is_none()); - assert!(app.sessions.current_mut().input.contains(&selected)); + assert!(app.mention_picker.is_none()); + assert_eq!(app.sessions.current().input, "src/main.rs "); } #[test] - fn enter_with_no_matches_closes_picker_without_modifying_input() { + fn enter_accepts_skill_entry_plain_name_plus_trailing_space() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["a.rs"]); - open_picker_with_index(&mut app, &idx); + open_picker_via_key(&mut app, &["irrelevant.rs"]); + { + let picker = app.mention_picker.as_mut().unwrap(); + picker.catalog = MentionCatalog { + files: Some(Arc::new(Vec::new())), + skills: Some(Arc::from(vec![zeph_core::channel::SkillCatalogItem { + name: "web_search".to_owned(), + description: "desc".to_owned(), + }])), + agents: Arc::from(Vec::new()), + }; + picker.active_tab = MentionTab::Skills; + picker.refilter(""); + } - let state = app.file_picker_state.as_mut().unwrap(); - state.update_query("xyznotfound"); + let key = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE); + app.handle_event(AppEvent::Key(key)); - assert!(app.file_picker_state.as_ref().unwrap().matches().is_empty()); + assert!(app.mention_picker.is_none()); + assert_eq!(app.sessions.current().input, "web_search "); + } + + #[test] + fn accept_agent_entry_retains_at_sigil() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["irrelevant.rs"]); + { + let picker = app.mention_picker.as_mut().unwrap(); + picker.catalog = MentionCatalog { + files: Some(Arc::new(Vec::new())), + skills: Some(Arc::from(Vec::new())), + agents: Arc::from(vec![zeph_core::metrics::AgentDefSummary { + name: "my_agent".to_owned(), + description: "desc".to_owned(), + ..Default::default() + }]), + }; + picker.active_tab = MentionTab::Agents; + picker.refilter(""); + } let key = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE); app.handle_event(AppEvent::Key(key)); - assert!(app.file_picker_state.is_none()); - assert!( - app.sessions.current_mut().input.is_empty(), - "input must be unchanged" - ); + assert_eq!(app.sessions.current().input, "@my_agent "); } #[test] - fn down_key_advances_selection() { + fn enter_with_no_matches_closes_picker_without_modifying_input() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["a.rs", "b.rs", "c.rs"]); - open_picker_with_index(&mut app, &idx); - - assert_eq!(app.file_picker_state.as_ref().unwrap().selected, 0); + open_picker_via_key(&mut app, &["a.rs"]); + for c in "xyznotfound".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + assert!(app.mention_picker.as_ref().unwrap().filtered.is_empty()); - let key = KeyEvent::new(KeyCode::Down, KeyModifiers::NONE); + let key = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE); app.handle_event(AppEvent::Key(key)); - assert_eq!(app.file_picker_state.as_ref().unwrap().selected, 1); + + assert!(app.mention_picker.is_none()); + assert_eq!( + app.sessions.current().input, + "@xyznotfound", + "no entry to accept — the typed query stays in the buffer unchanged" + ); } #[test] - fn up_key_wraps_selection_to_last() { + fn up_down_move_selection_with_wrap() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["a.rs", "b.rs", "c.rs"]); - open_picker_with_index(&mut app, &idx); + open_picker_via_key(&mut app, &["a.rs", "b.rs", "c.rs"]); + { + let picker = app.mention_picker.as_mut().unwrap(); + picker.active_tab = MentionTab::Files; + picker.refilter(""); + } + assert_eq!(app.mention_picker.as_ref().unwrap().selected, 0); - let key = KeyEvent::new(KeyCode::Up, KeyModifiers::NONE); - app.handle_event(AppEvent::Key(key)); - let state = app.file_picker_state.as_ref().unwrap(); - assert_eq!(state.selected, state.matches().len() - 1); + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Up, + KeyModifiers::NONE, + ))); + assert_eq!( + app.mention_picker.as_ref().unwrap().selected, + 2, + "Up on the first entry must wrap to the last (AC-005)" + ); + + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Down, + KeyModifiers::NONE, + ))); + assert_eq!(app.mention_picker.as_ref().unwrap().selected, 0); } #[test] - fn typing_filters_matches() { + fn left_right_cycle_tabs_without_moving_cursor() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["src/main.rs", "src/lib.rs"]); - open_picker_with_index(&mut app, &idx); + open_picker_via_key(&mut app, &["a.rs"]); + let cursor_before = app.sessions.current().cursor_position; + assert_eq!( + app.mention_picker.as_ref().unwrap().active_tab, + MentionTab::All + ); - let initial_count = app.file_picker_state.as_ref().unwrap().matches().len(); + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Right, + KeyModifiers::NONE, + ))); + assert_eq!( + app.mention_picker.as_ref().unwrap().active_tab, + MentionTab::Files + ); + assert_eq!(app.sessions.current().cursor_position, cursor_before); - let key = KeyEvent::new(KeyCode::Char('m'), KeyModifiers::NONE); - app.handle_event(AppEvent::Key(key)); + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Left, + KeyModifiers::NONE, + ))); + assert_eq!( + app.mention_picker.as_ref().unwrap().active_tab, + MentionTab::All + ); + assert_eq!(app.sessions.current().cursor_position, cursor_before); + } + + #[test] + fn typing_filters_matches_across_categories() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["src/main.rs", "src/lib.rs"]); + let initial_count = app.mention_picker.as_ref().unwrap().filtered.len(); + + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char('m'), + KeyModifiers::NONE, + ))); - let filtered_count = app.file_picker_state.as_ref().unwrap().matches().len(); + let filtered_count = app.mention_picker.as_ref().unwrap().filtered.len(); assert!(filtered_count <= initial_count); - assert_eq!(app.file_picker_state.as_ref().unwrap().query, "m"); + assert_eq!(app.sessions.current().input, "@m"); } #[test] - fn backspace_with_nonempty_query_removes_char() { + fn backspace_with_nonempty_query_keeps_picker_open_and_refilters() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["a.rs"]); - open_picker_with_index(&mut app, &idx); + open_picker_via_key(&mut app, &["a.rs"]); + for c in "ma".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + assert_eq!(app.sessions.current().input, "@ma"); - app.file_picker_state.as_mut().unwrap().update_query("ma"); + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Backspace, + KeyModifiers::NONE, + ))); - let key = KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE); - app.handle_event(AppEvent::Key(key)); + assert!(app.mention_picker.is_some()); + assert_eq!(app.sessions.current().input, "@m"); + } + + #[test] + fn backspace_over_at_sign_closes_picker() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + assert_eq!(app.sessions.current().input, "@"); + + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Backspace, + KeyModifiers::NONE, + ))); - assert!(app.file_picker_state.is_some()); - assert_eq!(app.file_picker_state.as_ref().unwrap().query, "m"); + assert!(app.mention_picker.is_none()); + assert_eq!(app.sessions.current().input, ""); } #[test] - fn backspace_on_empty_query_dismisses_picker() { + fn space_closes_picker_and_inserts_as_plain_prose() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["a.rs"]); - open_picker_with_index(&mut app, &idx); + open_picker_via_key(&mut app, &["a.rs"]); + for c in "file".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + assert_eq!(app.sessions.current().input, "@file"); + assert!(app.mention_picker.is_some()); - assert!(app.file_picker_state.as_ref().unwrap().query.is_empty()); + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(' '), + KeyModifiers::NONE, + ))); - let key = KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE); - app.handle_event(AppEvent::Key(key)); + assert!(app.mention_picker.is_none()); + assert_eq!(app.sessions.current().input, "@file "); + } + + #[test] + fn home_key_exits_span_and_closes_picker() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + for c in "foo".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + assert!(app.mention_picker.is_some()); - assert!(app.file_picker_state.is_none()); + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Home, + KeyModifiers::NONE, + ))); + + assert!(app.mention_picker.is_none()); } #[test] - fn picker_blocks_other_keys() { + fn end_key_crossing_trailing_text_closes_picker() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + for c in "foo".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + assert!(app.mention_picker.is_some()); + app.sessions.current_mut().input.push_str(" bar"); + app.sessions.current_mut().cursor_position = 3; + + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::End, + KeyModifiers::NONE, + ))); + + assert!( + app.mention_picker.is_none(), + "End crossing into ` bar` must close the picker" + ); + } + + #[test] + fn alt_left_past_at_sign_closes_picker_but_not_at_the_boundary() { let (mut app, _rx, _tx) = make_app_with_index(); let (idx, _dir) = build_temp_index(&["a.rs"]); - open_picker_with_index(&mut app, &idx); + app.file_index = Some(idx); + app.sessions.current_mut().input_mode = InputMode::Insert; + app.sessions.current_mut().input = "hello ".to_owned(); + app.sessions.current_mut().cursor_position = 6; + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char('@'), + KeyModifiers::NONE, + ))); + for c in "foo".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + assert_eq!(app.sessions.current().input, "hello @foo"); + assert!(app.mention_picker.is_some()); - app.sessions.current_mut().input = "hello".into(); - app.sessions.current_mut().cursor_position = 5; - let key = KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL); - app.handle_event(AppEvent::Key(key)); - assert_eq!( - app.sessions.current_mut().input, - "hello", - "input should be unchanged while picker is open" + let alt_left = KeyEvent::new(KeyCode::Left, KeyModifiers::ALT); + app.handle_event(AppEvent::Key(alt_left)); + assert!( + app.mention_picker.is_some(), + "cursor landing right after @ is still within the span (M4 boundary case)" + ); + + app.handle_event(AppEvent::Key(alt_left)); + assert!( + app.mention_picker.is_none(), + "moving left past @ into the previous word must close the picker" ); } #[test] - fn enter_inserts_at_cursor_mid_input() { + fn ctrl_u_clears_input_and_closes_picker() { let (mut app, _rx, _tx) = make_app_with_index(); - let (idx, _dir) = build_temp_index(&["src/lib.rs"]); - open_picker_with_index(&mut app, &idx); + open_picker_via_key(&mut app, &["a.rs"]); + assert!(app.mention_picker.is_some()); - app.sessions.current_mut().input = "ab".into(); - app.sessions.current_mut().cursor_position = 1; + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char('u'), + KeyModifiers::CONTROL, + ))); + + assert_eq!(app.sessions.current().input, ""); + assert!(app.mention_picker.is_none()); + } - let selected = app - .file_picker_state + #[test] + fn picker_opens_with_files_none_and_reports_ensure_file_index_effect() { + let (mut app, _rx, _tx) = make_app_with_index(); + app.sessions.current_mut().input_mode = InputMode::Insert; + + let effects = + crate::app::reducer::reduce(&mut app, crate::app::action::Action::InsertChar('@')); + + assert!(matches!( + effects.as_slice(), + [crate::app::reducer::Effect::EnsureFileIndex] + )); + let picker = app + .mention_picker .as_ref() - .unwrap() - .selected_path() - .map(ToOwned::to_owned) - .unwrap(); + .expect("picker opens synchronously"); + assert!( + picker.catalog.files.is_none(), + "files category starts in loading state (AC-012/NFR-004)" + ); + } - let key = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE); - app.handle_event(AppEvent::Key(key)); + // ── S1 regression: paste must route through `reduce`, not `handle_paste` directly ── + + #[test] + fn paste_containing_whitespace_closes_picker() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + for c in "fi".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + assert_eq!(app.sessions.current().input, "@fi"); + assert!(app.mention_picker.is_some()); - assert!(app.sessions.current_mut().input.contains(&selected)); - assert!(app.sessions.current_mut().input.starts_with('a')); - assert!(app.sessions.current_mut().input.ends_with('b')); + app.handle_event(AppEvent::Paste("le extra".to_owned())); + + assert_eq!(app.sessions.current().input, "@file extra"); + assert!( + app.mention_picker.is_none(), + "S1: paste must route through reduce so the span-whitespace check runs" + ); } + #[test] + fn accept_with_shrunk_buffer_does_not_panic_and_closes_cleanly() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + for c in "fi".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + // Simulate the buffer shrinking out from under a stale at_char_index — the + // defensive guard in `MentionPickerAccept` must not panic (S1 belt-and-braces). + app.sessions.current_mut().input.clear(); + app.sessions.current_mut().cursor_position = 0; + + let effects = + crate::app::reducer::reduce(&mut app, crate::app::action::Action::MentionPickerAccept); + + assert!(effects.is_empty()); + assert!(app.mention_picker.is_none()); + } + + // ── S2 regression: Dispatch/session-switch must close a stale picker ─────────────── + + #[test] + fn dispatch_prefill_verbatim_closes_stale_picker() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + assert!(app.mention_picker.is_some()); + + let effects = crate::app::reducer::reduce( + &mut app, + crate::app::action::Action::Dispatch(TuiCommand::PrefillVerbatim( + "/new text".to_owned(), + )), + ); + + assert!(effects.is_empty()); + assert!( + app.mention_picker.is_none(), + "S2: Dispatch reaching PrefillVerbatim must resync and close the stale picker" + ); + assert_eq!(app.sessions.current().input, "/new text"); + } + + #[test] + fn session_switch_closes_mention_picker_structurally() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + assert!(app.mention_picker.is_some()); + + app.execute_command(TuiCommand::SessionSwitchNext); + + assert!(app.mention_picker.is_none()); + } + + // ── M4: token-bounded accept ───────────────────────────────────────────────────── + + #[test] + fn m4_accept_replaces_whole_token_when_cursor_is_inside_it() { + let (mut app, _rx, _tx) = make_app_with_index(); + open_picker_via_key(&mut app, &["a.rs"]); + for c in "foo".chars() { + app.handle_event(AppEvent::Key(KeyEvent::new( + KeyCode::Char(c), + KeyModifiers::NONE, + ))); + } + assert_eq!(app.sessions.current().input, "@foo"); + // Cursor lands right after `@` (reachable via Alt+Left/Ctrl+A/mouse) — the query + // there is empty, but accept must still replace the *whole* mention word. + app.sessions.current_mut().cursor_position = 1; + { + let picker = app.mention_picker.as_mut().unwrap(); + picker.catalog = catalog_with_files(&["src/main.rs"]); + picker.refilter(""); + } + + let effects = + crate::app::reducer::reduce(&mut app, crate::app::action::Action::MentionPickerAccept); + + assert!(effects.is_empty()); + assert_eq!(app.sessions.current().input, "src/main.rs "); + } + + #[test] + fn m4_accept_token_end_at_existing_space_suppresses_double_space() { + let (mut app, _rx, _tx) = make_app_with_index(); + let catalog = catalog_with_files(&["src/main.rs"]); + app.sessions.current_mut().input = "@foo bar".to_owned(); + app.sessions.current_mut().cursor_position = 4; // right after "foo", before the space + app.mention_picker = Some(MentionPickerState::new(0, catalog)); + + let effects = + crate::app::reducer::reduce(&mut app, crate::app::action::Action::MentionPickerAccept); + + assert!(effects.is_empty()); + assert_eq!( + app.sessions.current().input, + "src/main.rs bar", + "token_end lands on the existing space; exactly one space must separate" + ); + } + + #[test] + fn m4_accept_replaces_mid_query_mention_leaving_surrounding_prose() { + let (mut app, _rx, _tx) = make_app_with_index(); + let catalog = catalog_with_files(&["src/main.rs"]); + app.sessions.current_mut().input = "hello @file".to_owned(); + app.sessions.current_mut().cursor_position = 11; + app.mention_picker = Some(MentionPickerState::new(6, catalog)); + + let effects = + crate::app::reducer::reduce(&mut app, crate::app::action::Action::MentionPickerAccept); + + assert!(effects.is_empty()); + assert_eq!(app.sessions.current().input, "hello src/main.rs "); + } + + // ── File-index polling: no-input-loss race (FR-011/NFR-004) ───────────────────── + #[tokio::test] - async fn poll_pending_file_index_installs_index_and_opens_picker() { + async fn poll_pending_file_index_installs_index_and_refreshes_open_picker() { let (user_tx, _user_rx) = tokio::sync::mpsc::channel(1); let (_agent_tx, agent_rx) = tokio::sync::mpsc::channel(1); let mut app = App::new(user_tx, agent_rx); + app.sessions.current_mut().input_mode = InputMode::Insert; + app.sessions.current_mut().input = "@".to_owned(); + app.sessions.current_mut().cursor_position = 1; + app.mention_picker = Some(MentionPickerState::new(0, MentionCatalog::default())); + assert!(app.mention_picker.as_ref().unwrap().catalog.files.is_none()); - // Simulate: status is set, pending_file_index is Some (already resolved) let (tx, rx) = tokio::sync::oneshot::channel(); let (idx, _dir) = build_temp_index(&["foo.rs"]); let _ = tx.send(idx); app.pending_file_index = Some(PendingFileIndex::Bare(rx)); app.sessions.current_mut().status_label = Some("indexing files...".to_owned()); - // Give the oneshot a moment to be ready (it already is since we sent before assigning) tokio::task::yield_now().await; app.poll_pending_file_index(); assert!(app.file_index.is_some(), "file_index should be installed"); - assert!( - app.file_picker_state.is_some(), - "picker should open after index ready" - ); assert!( app.sessions.current_mut().status_label.is_none(), "status should be cleared after index ready" @@ -2033,6 +2391,10 @@ mod file_picker_tests { app.pending_file_index.is_none(), "pending handle should be consumed" ); + assert!( + app.mention_picker.as_ref().unwrap().catalog.files.is_some(), + "an open picker's Files category must refresh once the index arrives" + ); } #[tokio::test] @@ -2041,11 +2403,10 @@ mod file_picker_tests { let (_agent_tx, agent_rx) = tokio::sync::mpsc::channel(1); let mut app = App::new(user_tx, agent_rx); - // No pending handle — should be a no-op app.poll_pending_file_index(); assert!(app.file_index.is_none()); - assert!(app.file_picker_state.is_none()); + assert!(app.mention_picker.is_none()); } #[tokio::test] @@ -2055,7 +2416,6 @@ mod file_picker_tests { let mut app = App::new(user_tx, agent_rx); let (tx, rx) = tokio::sync::oneshot::channel::(); - // Drop sender without sending — simulates spawn_blocking panic drop(tx); app.pending_file_index = Some(PendingFileIndex::Bare(rx)); app.sessions.current_mut().status_label = Some("indexing files...".to_owned()); @@ -2666,7 +3026,7 @@ mod slash_autocomplete_tests { } #[test] - fn at_char_while_autocomplete_open_does_not_open_file_picker() { + fn at_char_while_autocomplete_open_does_not_open_mention_picker() { let (mut app, _rx, _tx) = make_app(); app.sessions.current_mut().input_mode = InputMode::Insert; app.slash_autocomplete = @@ -2676,7 +3036,7 @@ mod slash_autocomplete_tests { let key = KeyEvent::new(KeyCode::Char('@'), KeyModifiers::NONE); app.handle_event(AppEvent::Key(key)); - assert!(app.file_picker_state.is_none()); + assert!(app.mention_picker.is_none()); } #[test] diff --git a/crates/zeph-tui/src/channel.rs b/crates/zeph-tui/src/channel.rs index 334cff584..047609653 100644 --- a/crates/zeph-tui/src/channel.rs +++ b/crates/zeph-tui/src/channel.rs @@ -1,10 +1,12 @@ // SPDX-FileCopyrightText: 2026 Andrei G // SPDX-License-Identifier: MIT OR Apache-2.0 +use std::sync::Arc; + use tokio::sync::mpsc; use zeph_core::channel::{ Channel, ChannelError, ChannelMessage, ElicitationRequest, ElicitationResponse, - ToolOutputEvent, ToolStartEvent, + SkillCatalogItem, ToolOutputEvent, ToolStartEvent, }; use crate::command::TuiCommand; @@ -244,6 +246,19 @@ impl Channel for TuiChannel { Ok(()) } + #[cfg_attr( + feature = "profiling", + tracing::instrument(name = "tui.channel.send_skill_catalog", skip_all) + )] + async fn send_skill_catalog(&mut self, items: &[SkillCatalogItem]) -> Result<(), ChannelError> { + // Non-critical: refreshes the mention picker's Skills tab if one is open; + // startup/hot-reload emits arrive well before any user interaction. + let _ = self + .agent_event_tx + .try_send(AgentEvent::SkillCatalog(Arc::from(items.to_vec()))); + Ok(()) + } + #[cfg_attr( feature = "profiling", tracing::instrument(name = "tui.channel.send_transcript_backfill", skip_all) @@ -616,6 +631,24 @@ mod tests { assert!(ch.accumulated.is_empty()); } + #[tokio::test] + async fn send_skill_catalog_forwards_event() { + let (mut ch, _user_tx, mut agent_rx) = make_channel(); + let items = vec![SkillCatalogItem { + name: "web_search".into(), + description: "Search the web".into(), + }]; + ch.send_skill_catalog(&items).await.unwrap(); + let evt = agent_rx.recv().await.unwrap(); + match evt { + AgentEvent::SkillCatalog(got) => { + assert_eq!(got.len(), 1); + assert_eq!(got[0].name, "web_search"); + } + other => panic!("expected SkillCatalog, got {other:?}"), + } + } + #[tokio::test] async fn send_queue_count_forwards_event() { let (mut ch, _user_tx, mut agent_rx) = make_channel(); diff --git a/crates/zeph-tui/src/event.rs b/crates/zeph-tui/src/event.rs index 010030445..697024027 100644 --- a/crates/zeph-tui/src/event.rs +++ b/crates/zeph-tui/src/event.rs @@ -293,6 +293,10 @@ pub enum AgentEvent { /// `input_history`/up-arrow recall (INV-SP-6, AC-20) — never routed through /// `App::load_history`, which also feeds `input_history`. HistoryBackfill(Vec), + /// Full skill catalog (name + description), emitted once at agent startup and + /// re-emitted on skill hot-reload (spec 084 §6, issue #6648). Stored into + /// `App::skill_catalog` and used to refresh an open mention picker's Skills tab. + SkillCatalog(Arc<[zeph_core::channel::SkillCatalogItem]>), } /// Blocking event pump that forwards terminal events to the async [`AppEvent`] channel. diff --git a/crates/zeph-tui/src/file_picker.rs b/crates/zeph-tui/src/file_picker.rs index a8ecf265d..89b067c59 100644 --- a/crates/zeph-tui/src/file_picker.rs +++ b/crates/zeph-tui/src/file_picker.rs @@ -5,11 +5,7 @@ use std::path::Path; use std::sync::Arc; use std::time::{Duration, Instant}; -use nucleo_matcher::pattern::{AtomKind, CaseMatching, Normalization, Pattern}; -use nucleo_matcher::{Config, Matcher, Utf32Str}; - const TTL: Duration = Duration::from_secs(30); -const MAX_RESULTS: usize = 10; /// Hard cap on indexed paths to prevent unbounded memory usage on repos with /// large unignored directories. const MAX_INDEXED: usize = 50_000; @@ -79,122 +75,6 @@ impl FileIndex { } } -#[derive(Clone)] -pub struct PickerMatch { - pub path: String, - pub score: u32, -} - -pub struct FilePickerState { - pub query: String, - pub selected: usize, - matches: Vec, - /// Shared ownership of the file index — no clone on picker open. - index: Arc>, - /// Reused across `refilter` calls to avoid per-keystroke heap allocation. - matcher: Matcher, -} - -impl FilePickerState { - #[must_use] - pub fn new(index: &FileIndex) -> Self { - let mut state = Self { - query: String::new(), - selected: 0, - matches: Vec::new(), - index: index.paths_arc(), - matcher: Matcher::new(Config::DEFAULT), - }; - state.refilter(); - state - } - - pub fn update_query(&mut self, query: &str) { - query.clone_into(&mut self.query); - self.refilter(); - } - - /// Appends a character to the query and re-filters. - pub fn push_char(&mut self, c: char) { - self.query.push(c); - self.refilter(); - } - - /// Removes the last character from the query and re-filters. - /// Returns `true` if a character was removed, `false` if the query was already empty. - pub fn pop_char(&mut self) -> bool { - if self.query.pop().is_some() { - self.refilter(); - true - } else { - false - } - } - - #[must_use] - pub fn matches(&self) -> &[PickerMatch] { - &self.matches - } - - #[must_use] - pub fn selected_path(&self) -> Option<&str> { - self.matches.get(self.selected).map(|m| m.path.as_str()) - } - - pub fn move_selection(&mut self, delta: i32) { - let len = self.matches.len(); - if len == 0 { - return; - } - let len_i = i32::try_from(len).unwrap_or(i32::MAX); - let cur_i = i32::try_from(self.selected).unwrap_or(0); - let new_i = (cur_i + delta).rem_euclid(len_i); - self.selected = usize::try_from(new_i).unwrap_or(0); - } - - fn refilter(&mut self) { - self.selected = 0; - if self.query.is_empty() { - self.matches = self - .index - .iter() - .take(MAX_RESULTS) - .map(|p| PickerMatch { - path: p.clone(), - score: 0, - }) - .collect(); - return; - } - - let pattern = Pattern::new( - &self.query, - CaseMatching::Smart, - Normalization::Smart, - AtomKind::Fuzzy, - ); - - let mut scored: Vec = self - .index - .iter() - .filter_map(|p| { - let mut buf = Vec::new(); - let haystack = Utf32Str::new(p, &mut buf); - pattern - .score(haystack, &mut self.matcher) - .map(|score| PickerMatch { - path: p.clone(), - score, - }) - }) - .collect(); - - scored.sort_unstable_by_key(|entry| std::cmp::Reverse(entry.score)); - scored.truncate(MAX_RESULTS); - self.matches = scored; - } -} - #[cfg(test)] mod tests { use std::fs; @@ -226,123 +106,19 @@ mod tests { assert!(!idx.is_stale()); } - #[test] - fn empty_query_returns_up_to_10_files() { - let files: Vec = (0..15).map(|i| format!("file{i}.rs")).collect(); - let refs: Vec<&str> = files.iter().map(String::as_str).collect(); - let idx = make_index(&refs); - let state = FilePickerState::new(&idx); - assert_eq!(state.matches().len(), 10); - } - - #[test] - fn fuzzy_query_filters_results() { - let idx = make_index(&["src/main.rs", "src/lib.rs", "tests/foo.rs"]); - let mut state = FilePickerState::new(&idx); - state.update_query("main"); - assert!(!state.matches().is_empty()); - assert!(state.matches().iter().any(|m| m.path.contains("main"))); - } - - #[test] - fn selected_path_returns_first_match() { - let idx = make_index(&["alpha.rs", "beta.rs"]); - let state = FilePickerState::new(&idx); - assert!(state.selected_path().is_some()); - } - - #[test] - fn move_selection_wraps_around() { - let idx = make_index(&["a.rs", "b.rs", "c.rs"]); - let mut state = FilePickerState::new(&idx); - assert_eq!(state.selected, 0); - state.move_selection(-1); - assert_eq!(state.selected, state.matches().len() - 1); - } - - #[test] - fn move_selection_noop_when_empty() { - let idx = make_index(&["a.rs"]); - let mut state = FilePickerState::new(&idx); - state.matches = vec![]; - state.move_selection(1); - assert_eq!(state.selected, 0); - } - - #[test] - fn no_match_query_returns_empty_and_selected_path_none() { - let idx = make_index(&["src/main.rs", "src/lib.rs"]); - let mut state = FilePickerState::new(&idx); - state.update_query("xyznotfound"); - assert!(state.matches().is_empty()); - assert!(state.selected_path().is_none()); - } - #[test] fn unicode_paths_are_indexed_and_searchable() { let idx = make_index(&["src/данные.rs", "データ/main.rs", "normal.rs"]); assert!(idx.paths().iter().any(|p| p.contains("данные"))); assert!(idx.paths().iter().any(|p| p.contains("main"))); - - let mut state = FilePickerState::new(&idx); - state.update_query("данные"); - assert!( - !state.matches().is_empty(), - "expected match for unicode query" - ); - } - - #[test] - fn push_char_appends_and_refilters() { - let idx = make_index(&["src/main.rs", "src/lib.rs"]); - let mut state = FilePickerState::new(&idx); - state.push_char('m'); - state.push_char('a'); - assert!(state.matches().iter().any(|m| m.path.contains("main"))); - } - - #[test] - fn pop_char_removes_last_and_refilters() { - let idx = make_index(&["src/main.rs", "src/lib.rs"]); - let mut state = FilePickerState::new(&idx); - state.push_char('m'); - let removed = state.pop_char(); - assert!(removed); - assert!(state.query.is_empty()); - } - - #[test] - fn pop_char_on_empty_returns_false() { - let idx = make_index(&["a.rs"]); - let mut state = FilePickerState::new(&idx); - assert!(!state.pop_char()); } #[test] - fn arc_index_shared_not_cloned() { + fn arc_paths_shared_not_cloned() { let idx = make_index(&["a.rs", "b.rs"]); let arc1 = idx.paths_arc(); - let state = FilePickerState::new(&idx); + let arc2 = idx.paths_arc(); // Both should point to the same allocation - assert!(Arc::ptr_eq(&arc1, &state.index)); - } - - use proptest::prelude::*; - - proptest! { - #![proptest_config(proptest::test_runner::Config::with_cases(200))] - - #[test] - fn move_selection_never_panics( - n in 1usize..20, - delta in -10i32..10, - ) { - let files: Vec = (0..n).map(|i| format!("f{i}.rs")).collect(); - let refs: Vec<&str> = files.iter().map(String::as_str).collect(); - let idx = make_index(&refs); - let mut state = FilePickerState::new(&idx); - state.move_selection(delta); - prop_assert!(state.selected < state.matches().len().max(1)); - } + assert!(Arc::ptr_eq(&arc1, &arc2)); } } diff --git a/crates/zeph-tui/src/widgets/file_picker.rs b/crates/zeph-tui/src/widgets/file_picker.rs deleted file mode 100644 index d5f085830..000000000 --- a/crates/zeph-tui/src/widgets/file_picker.rs +++ /dev/null @@ -1,118 +0,0 @@ -// SPDX-FileCopyrightText: 2026 Andrei G -// SPDX-License-Identifier: MIT OR Apache-2.0 - -use ratatui::Frame; -use ratatui::layout::Rect; -use ratatui::style::{Color, Modifier, Style}; -use ratatui::text::{Line, Span}; -use ratatui::widgets::{Block, BorderType, Borders, Clear, List, ListItem, ListState, Paragraph}; - -use crate::file_picker::FilePickerState; -use crate::theme::Theme; - -pub fn render(state: &FilePickerState, frame: &mut Frame, input_area: Rect, theme: &Theme) { - let match_count = state.matches().len(); - let visible_items = u16::try_from(match_count.min(10)).unwrap_or(10); - // border top + query line + border bottom = 3 overhead; items in between - let height = visible_items + 3; - let y = input_area.y.saturating_sub(height); - let popup = Rect::new(input_area.x, y, input_area.width, height); - - frame.render_widget(Clear, popup); - - // Split popup: first line for query, rest for list - let query_area = Rect::new(popup.x + 1, popup.y + 1, popup.width.saturating_sub(2), 1); - let list_area = Rect::new( - popup.x + 1, - popup.y + 2, - popup.width.saturating_sub(2), - visible_items, - ); - - // Outer block - let block = Block::default() - .borders(Borders::ALL) - .border_type(BorderType::Rounded) - .border_style(theme.panel_border) - .title(" Files ") - .title_style(theme.panel_title); - frame.render_widget(block, popup); - - // Query line - let query_text = format!("> {}", state.query); - let query_para = Paragraph::new(Span::styled(query_text, theme.highlight)); - frame.render_widget(query_para, query_area); - - // File list — borrow path strings to avoid allocation per render frame - let items: Vec = state - .matches() - .iter() - .map(|m| ListItem::new(Line::from(Span::raw(m.path.as_str())))) - .collect(); - - let selected_style = Style::default() - .fg(Color::Black) - .bg(Color::Cyan) - .add_modifier(Modifier::BOLD); - - let list = List::new(items) - .highlight_style(selected_style) - .highlight_symbol("> "); - - let mut list_state = ListState::default(); - if match_count > 0 { - list_state.select(Some(state.selected)); - } - - frame.render_stateful_widget(list, list_area, &mut list_state); -} - -#[cfg(test)] -mod tests { - use std::fs; - - use insta::assert_snapshot; - - use crate::file_picker::{FileIndex, FilePickerState}; - use crate::test_utils::render_to_string; - - fn make_state(files: &[&str], query: &str) -> (FilePickerState, tempfile::TempDir) { - let dir = tempfile::tempdir().unwrap(); - for &f in files { - let path = dir.path().join(f); - if let Some(parent) = path.parent() { - fs::create_dir_all(parent).unwrap(); - } - fs::write(&path, "").unwrap(); - } - let idx = FileIndex::build(dir.path()); - let mut state = FilePickerState::new(&idx); - if !query.is_empty() { - state.update_query(query); - } - (state, dir) - } - - #[test] - fn file_picker_empty_query_snapshot() { - let (state, _dir) = make_state(&["src/main.rs", "src/lib.rs", "README.md"], ""); - let input_area = ratatui::layout::Rect::new(0, 15, 60, 3); - let output = render_to_string(60, 20, |frame, _area| { - let theme = crate::theme::Theme::default(); - super::render(&state, frame, input_area, &theme); - }); - assert_snapshot!(output); - } - - #[test] - fn file_picker_with_query_snapshot() { - let (mut state, _dir) = make_state(&["src/main.rs", "src/lib.rs", "README.md"], ""); - state.update_query("main"); - let input_area = ratatui::layout::Rect::new(0, 15, 60, 3); - let output = render_to_string(60, 20, |frame, _area| { - let theme = crate::theme::Theme::default(); - super::render(&state, frame, input_area, &theme); - }); - assert_snapshot!(output); - } -} diff --git a/crates/zeph-tui/src/widgets/input.rs b/crates/zeph-tui/src/widgets/input.rs index 66893c6ef..3c53e4c04 100644 --- a/crates/zeph-tui/src/widgets/input.rs +++ b/crates/zeph-tui/src/widgets/input.rs @@ -243,17 +243,36 @@ fn render_text_area(app: &App, frame: &mut Frame, text_area: Rect, busy: bool) { // Do not show cursor when paste indicator is active — the user interacts // with the indicator as a whole unit, not individual characters. if app.paste_state().is_none() && matches!(app.input_mode(), InputMode::Insert) { - let prefix: String = app.input().chars().take(app.cursor_position()).collect(); - let last_line = prefix.rsplit('\n').next().unwrap_or(&prefix); - #[allow(clippy::cast_possible_truncation)] - let cursor_x = text_area.x + last_line.width() as u16; - let line_count = u16::try_from(prefix.matches('\n').count()).unwrap_or(u16::MAX); - #[allow(clippy::cast_possible_truncation)] - let cursor_y = text_area.y + line_count.saturating_sub(scroll); + let (cursor_x, cursor_y) = caret_xy(app, text_area, app.cursor_position()); frame.set_cursor_position((cursor_x, cursor_y)); } } +/// Computes the on-screen `(x, y)` position of `char_index` within `text_area`. +/// +/// Shared by the real terminal cursor (above) and the mention-picker popup anchor +/// (`crate::widgets::mention_picker::render`) so the popup never disagrees with where +/// the cursor is actually drawn (M3). Splits the buffer on `'\n'` only — the paragraph +/// renders with `Wrap { trim: false }`, so on a visually-wrapped line the computed `x` +/// can overshoot `text_area.width`; this is a pre-existing limitation shared identically +/// by both call sites, not something this helper newly introduces. +pub(crate) fn caret_xy(app: &App, text_area: Rect, char_index: usize) -> (u16, u16) { + let input = app.input(); + let byte_idx = input + .char_indices() + .nth(char_index) + .map_or(input.len(), |(idx, _)| idx); + let prefix = &input[..byte_idx]; + let last_line = prefix.rsplit('\n').next().unwrap_or(prefix); + #[allow(clippy::cast_possible_truncation)] + let cursor_x = text_area.x + last_line.width() as u16; + let visible_lines = text_area.height; + let cursor_line = u16::try_from(prefix.matches('\n').count()).unwrap_or(u16::MAX); + let scroll = cursor_line.saturating_sub(visible_lines.saturating_sub(1)); + let cursor_y = text_area.y + cursor_line.saturating_sub(scroll); + (cursor_x, cursor_y) +} + pub fn render( app: &App, frame: &mut Frame, diff --git a/crates/zeph-tui/src/widgets/mention_picker.rs b/crates/zeph-tui/src/widgets/mention_picker.rs new file mode 100644 index 000000000..d8deacc7e --- /dev/null +++ b/crates/zeph-tui/src/widgets/mention_picker.rs @@ -0,0 +1,677 @@ +// SPDX-FileCopyrightText: 2026 Andrei G +// SPDX-License-Identifier: MIT OR Apache-2.0 + +//! Inline, non-modal `@` mention picker (spec 084, issues #6647/#6648). +//! +//! Replaces the old modal file picker (`file_picker.rs`, pre-#6647) with a popup that +//! never steals keystrokes from the input buffer: every character still lands in +//! `SessionSlot::input`, and this widget only reflects/filters what is already there. +//! The query itself is never duplicated into `MentionPickerState` — it is always derived +//! from the buffer by the reducer (`crate::app::reducer::mention_picker_query`), which is +//! what makes cursor movement, paste, and backspace "just work" without a second +//! keystroke-mirroring state machine (the bug class `SlashAutocomplete*PushChar/PopChar` +//! lives with). + +use std::sync::Arc; + +use ratatui::Frame; +use ratatui::layout::Rect; +use ratatui::style::{Color, Modifier, Style}; +use ratatui::text::{Line, Span}; +use ratatui::widgets::{Block, BorderType, Borders, Clear, List, ListItem, ListState, Paragraph}; + +use nucleo_matcher::pattern::{AtomKind, CaseMatching, Normalization, Pattern}; +use nucleo_matcher::{Config, Matcher, Utf32Str}; + +use zeph_core::channel::SkillCatalogItem; +use zeph_core::metrics::AgentDefSummary; + +use crate::app::App; +use crate::theme::Theme; + +/// Hard cap on rendered/selectable results, mirroring the old file picker's limit. +const MAX_RESULTS: usize = 10; + +/// Category tab. `Left`/`Right` cycle through these while the popup is open (FR-004); +/// they never move the input cursor while the picker is open (D2). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum MentionTab { + All, + Files, + Skills, + Agents, +} + +impl MentionTab { + #[must_use] + pub(crate) fn next(self) -> Self { + match self { + Self::All => Self::Files, + Self::Files => Self::Skills, + Self::Skills => Self::Agents, + Self::Agents => Self::All, + } + } + + #[must_use] + pub(crate) fn prev(self) -> Self { + match self { + Self::All => Self::Agents, + Self::Files => Self::All, + Self::Skills => Self::Files, + Self::Agents => Self::Skills, + } + } + + fn label(self) -> &'static str { + match self { + Self::All => "All", + Self::Files => "Files", + Self::Skills => "Skills", + Self::Agents => "Agents", + } + } +} + +/// Discriminates a [`MentionEntry`]'s source category. Drives accept format +/// (FR-015/016/017) and the All-tab row prefix (FR-018). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum MentionKind { + File, + Skill, + Agent, +} + +/// One rendered/selectable row in the popup. +pub(crate) struct MentionEntry { + pub(crate) kind: MentionKind, + pub(crate) display: String, + pub(crate) description: Option, + /// Char indices into `display` for match highlighting (FR-013). Sorted and + /// deduplicated at construction time (`Pattern::indices` appends per-atom, + /// unsorted and un-deduped — see `nucleo_matcher::pattern::Pattern::indices` docs). + pub(crate) indices: Vec, +} + +/// The three data sources backing the picker, kept deliberately heterogeneous +/// (no unified item type) since `files`/`skills` are `Option` (loading vs. loaded-empty, +/// FR-011/FR-019) while `agents` reads straight from the always-populated +/// `MetricsSnapshot::agent_definitions` (D1 — no new plumbing needed for agents). +#[derive(Clone, Default)] +pub(crate) struct MentionCatalog { + pub(crate) files: Option>>, + pub(crate) skills: Option>, + pub(crate) agents: Arc<[AgentDefSummary]>, +} + +/// Popup state for the inline `@` mention picker. +/// +/// `at_char_index` is the char index of the triggering `@` in the current session's +/// input buffer. The query (text between `@` and the cursor) is deliberately **not** +/// stored here — see the module doc comment. +pub(crate) struct MentionPickerState { + pub(crate) at_char_index: usize, + pub(crate) active_tab: MentionTab, + pub(crate) selected: usize, + pub(crate) filtered: Vec, + pub(crate) catalog: MentionCatalog, + matcher: Matcher, +} + +impl MentionPickerState { + #[must_use] + pub(crate) fn new(at_char_index: usize, catalog: MentionCatalog) -> Self { + let mut state = Self { + at_char_index, + active_tab: MentionTab::All, + selected: 0, + filtered: Vec::new(), + catalog, + matcher: Matcher::new(Config::DEFAULT), + }; + state.refilter(""); + state + } + + /// Re-derives `filtered` for the active tab from the given query. Called by the + /// reducer after every buffer/cursor mutation and after every tab change — never + /// mutates `query` itself since none is stored (see module doc comment). + pub(crate) fn refilter(&mut self, query: &str) { + self.selected = 0; + // Borrow `catalog` (not `self`) so the immutable candidate borrows below don't + // conflict with the `&mut self.matcher` borrow needed by the scored path. + let candidates = candidates_for_tab(&self.catalog, self.active_tab); + self.filtered = if query.is_empty() { + Self::round_robin(candidates, self.active_tab) + } else { + Self::scored(candidates, query, &mut self.matcher) + }; + } + + pub(crate) fn move_selection(&mut self, delta: i32) { + let len = self.filtered.len(); + if len == 0 { + return; + } + let len_i = i32::try_from(len).unwrap_or(i32::MAX); + let cur_i = i32::try_from(self.selected).unwrap_or(0); + let new_i = (cur_i + delta).rem_euclid(len_i); + self.selected = usize::try_from(new_i).unwrap_or(0); + } + + /// Empty-query path: round-robins across non-empty categories on the `All` tab so + /// files (up to 50 000 candidates) never crowd out skills/agents before either gets a + /// slot (#6651 tracks refinement for the *typed*-query case, which is score-ranked and + /// therefore not round-robined). Single-category tabs just take the first `MAX_RESULTS`. + fn round_robin(candidates: Vec>, tab: MentionTab) -> Vec { + if tab != MentionTab::All { + return candidates + .into_iter() + .take(MAX_RESULTS) + .map(Candidate::into_entry) + .collect(); + } + let mut groups: [Vec>; 3] = [Vec::new(), Vec::new(), Vec::new()]; + for c in candidates { + let slot = match c.kind { + MentionKind::File => 0, + MentionKind::Skill => 1, + MentionKind::Agent => 2, + }; + groups[slot].push(c); + } + let max_len = groups.iter().map(Vec::len).max().unwrap_or(0); + let mut out = Vec::new(); + 'outer: for round in 0..max_len { + for group in &mut groups { + if round < group.len() { + // Swap-free ownership grab: replace with a sentinel-free take via index. + let c = std::mem::replace( + &mut group[round], + Candidate { + kind: MentionKind::File, + name: "", + description: None, + }, + ); + out.push(Candidate::into_entry(c)); + if out.len() >= MAX_RESULTS { + break 'outer; + } + } + } + } + out + } + + /// Typed-query path: phase 1 scores every candidate (no allocation beyond the score + /// itself), truncates to `MAX_RESULTS`, then phase 2 materializes `display`/ + /// `description`/`indices` only for the survivors (`Pattern::indices` allocates, so it + /// must never run across the full candidate set). + fn scored( + candidates: Vec>, + query: &str, + matcher: &mut Matcher, + ) -> Vec { + let pattern = Pattern::new( + query, + CaseMatching::Smart, + Normalization::Smart, + AtomKind::Fuzzy, + ); + let mut scored: Vec<(u32, Candidate<'_>)> = candidates + .into_iter() + .filter_map(|c| { + let mut buf = Vec::new(); + let haystack = Utf32Str::new(c.name, &mut buf); + pattern.score(haystack, matcher).map(|score| (score, c)) + }) + .collect(); + scored.sort_unstable_by_key(|(score, _)| std::cmp::Reverse(*score)); + scored.truncate(MAX_RESULTS); + + scored + .into_iter() + .map(|(_, c)| { + let mut buf = Vec::new(); + let haystack = Utf32Str::new(c.name, &mut buf); + let mut indices = Vec::new(); + pattern.indices(haystack, matcher, &mut indices); + indices.sort_unstable(); + indices.dedup(); + MentionEntry { + kind: c.kind, + display: c.name.to_owned(), + description: c.description.map(str::to_owned), + indices, + } + }) + .collect() + } +} + +struct Candidate<'a> { + kind: MentionKind, + name: &'a str, + description: Option<&'a str>, +} + +/// Collects borrowed candidates from every category relevant to `active_tab`. Phase 1 +/// of the two-phase refilter (NFR-001): no `String`/`Vec` allocation here — only +/// `&str` borrows from the underlying `Arc` catalogs. A free function (not a +/// `&self` method) so callers can borrow `catalog` and `matcher` disjointly. +fn candidates_for_tab(catalog: &MentionCatalog, active_tab: MentionTab) -> Vec> { + let mut out = Vec::new(); + let want_files = matches!(active_tab, MentionTab::All | MentionTab::Files); + let want_skills = matches!(active_tab, MentionTab::All | MentionTab::Skills); + let want_agents = matches!(active_tab, MentionTab::All | MentionTab::Agents); + + if want_files && let Some(files) = &catalog.files { + out.extend(files.iter().map(|p| Candidate { + kind: MentionKind::File, + name: p.as_str(), + description: None, + })); + } + if want_skills && let Some(skills) = &catalog.skills { + out.extend(skills.iter().map(|s| Candidate { + kind: MentionKind::Skill, + name: s.name.as_str(), + description: Some(s.description.as_str()), + })); + } + if want_agents { + out.extend(catalog.agents.iter().map(|a| Candidate { + kind: MentionKind::Agent, + name: a.name.as_str(), + description: Some(a.description.as_str()), + })); + } + out +} + +impl Candidate<'_> { + fn into_entry(self) -> MentionEntry { + MentionEntry { + kind: self.kind, + display: self.name.to_owned(), + description: self.description.map(str::to_owned), + indices: Vec::new(), + } + } +} + +fn category_total(state: &MentionPickerState) -> usize { + match state.active_tab { + MentionTab::Files => state.catalog.files.as_ref().map_or(0, |f| f.len()), + MentionTab::Skills => state.catalog.skills.as_ref().map_or(0, |s| s.len()), + MentionTab::Agents => state.catalog.agents.len(), + MentionTab::All => { + state.catalog.files.as_ref().map_or(0, |f| f.len()) + + state.catalog.skills.as_ref().map_or(0, |s| s.len()) + + state.catalog.agents.len() + } + } +} + +/// Placeholder text shown when `filtered` is empty (FR-011/FR-019): distinguishes +/// "still loading" (`Option::None`) from "loaded and genuinely empty" (`Some(empty)`). +fn placeholder_text(state: &MentionPickerState) -> &'static str { + match state.active_tab { + MentionTab::Files => { + if state.catalog.files.is_none() { + "indexing files…" + } else { + "no files found" + } + } + MentionTab::Skills => { + if state.catalog.skills.is_none() { + "loading skills…" + } else { + "no skills loaded" + } + } + MentionTab::Agents => "no agents loaded", + MentionTab::All => "no results", + } +} + +fn render_tab_bar(active: MentionTab, frame: &mut Frame, area: Rect, theme: &Theme) { + let tabs = [ + MentionTab::All, + MentionTab::Files, + MentionTab::Skills, + MentionTab::Agents, + ]; + let mut spans = Vec::with_capacity(tabs.len() * 2); + for (i, tab) in tabs.into_iter().enumerate() { + if i > 0 { + spans.push(Span::raw(" | ")); + } + let style = if tab == active { + theme.highlight + } else { + theme.panel_title + }; + spans.push(Span::styled(tab.label(), style)); + } + frame.render_widget(Paragraph::new(Line::from(spans)), area); +} + +/// Splits `entry.display` into highlighted/plain `Span`s at the (sorted, deduped) +/// nucleo match indices, then appends the dimmed, width-truncated description. +fn render_row(entry: &MentionEntry, active_tab: MentionTab, theme: &Theme) -> ListItem<'static> { + let prefix = match (active_tab, entry.kind) { + (MentionTab::All, MentionKind::File) => "[F] ", + (MentionTab::All, MentionKind::Skill) => "[S] ", + (MentionTab::All, MentionKind::Agent) => "[A] ", + _ => "", + }; + let mut spans = Vec::new(); + if !prefix.is_empty() { + spans.push(Span::raw(prefix)); + } + let mut buf = String::new(); + let mut highlighted = false; + for (i, c) in entry.display.chars().enumerate() { + #[allow(clippy::cast_possible_truncation)] + let is_hl = entry.indices.binary_search(&(i as u32)).is_ok(); + if is_hl != highlighted && !buf.is_empty() { + let style = if highlighted { + theme.highlight + } else { + Style::default() + }; + spans.push(Span::styled(std::mem::take(&mut buf), style)); + } + highlighted = is_hl; + buf.push(c); + } + if !buf.is_empty() { + let style = if highlighted { + theme.highlight + } else { + Style::default() + }; + spans.push(Span::styled(buf, style)); + } + if let Some(desc) = &entry.description { + let truncated = crate::layout::truncate_to_width(desc, 30); + spans.push(Span::styled( + format!(" — {truncated}"), + theme.system_message, + )); + } + ListItem::new(Line::from(spans)) +} + +/// Renders the popup anchored to the triggering `@` character, flipping above/below +/// `input_area` based on available space. Reuses [`crate::widgets::input::caret_xy`] so +/// the popup never disagrees with where the terminal cursor is actually drawn (M3). +pub(crate) fn render( + app: &App, + state: &MentionPickerState, + frame: &mut Frame, + input_area: Rect, + theme: &Theme, +) { + const TAB_BAR_H: u16 = 1; + const BORDER_H: u16 = 2; + + let (anchor_x, _anchor_y) = + crate::widgets::input::caret_xy(app, input_area, state.at_char_index); + + let visible_rows = state.filtered.len().clamp(1, MAX_RESULTS); + #[allow(clippy::cast_possible_truncation)] + let list_h = visible_rows as u16; + let height = list_h + TAB_BAR_H + BORDER_H; + + let width: u16 = 50.min(input_area.width.max(1)); + let max_x = input_area.x + input_area.width.saturating_sub(width); + let x = anchor_x.min(max_x); + + let frame_height = frame.area().height; + let y = if input_area.y >= height { + input_area.y - height + } else { + (input_area.y + input_area.height).min(frame_height.saturating_sub(height)) + }; + + let popup = Rect { + x, + y, + width, + height, + }; + frame.render_widget(Clear, popup); + + let total = category_total(state); + let title = format!( + " {} ({}/{total}) ", + state.active_tab.label(), + state.filtered.len() + ); + let block = Block::default() + .borders(Borders::ALL) + .border_type(BorderType::Rounded) + .border_style(theme.panel_border) + .title(title) + .title_style(theme.panel_title); + frame.render_widget(block, popup); + + let inner = Rect::new( + popup.x + 1, + popup.y + 1, + popup.width.saturating_sub(2), + popup.height.saturating_sub(2), + ); + let tab_area = Rect::new(inner.x, inner.y, inner.width, 1); + let list_area = Rect::new( + inner.x, + inner.y + 1, + inner.width, + inner.height.saturating_sub(1), + ); + + render_tab_bar(state.active_tab, frame, tab_area, theme); + + if state.filtered.is_empty() { + let msg = Paragraph::new(placeholder_text(state)).style(theme.system_message); + frame.render_widget(msg, list_area); + return; + } + + let items: Vec = state + .filtered + .iter() + .map(|entry| render_row(entry, state.active_tab, theme)) + .collect(); + let selected_style = Style::default() + .fg(Color::Black) + .bg(Color::Cyan) + .add_modifier(Modifier::BOLD); + let list = List::new(items) + .highlight_style(selected_style) + .highlight_symbol("> "); + let mut list_state = ListState::default(); + list_state.select(Some(state.selected)); + frame.render_stateful_widget(list, list_area, &mut list_state); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_utils::render_to_string; + + fn catalog(files: &[&str], skills: &[(&str, &str)], agents: &[(&str, &str)]) -> MentionCatalog { + MentionCatalog { + files: Some(Arc::new(files.iter().map(|s| (*s).to_owned()).collect())), + skills: Some(Arc::from( + skills + .iter() + .map(|(name, description)| SkillCatalogItem { + name: (*name).to_owned(), + description: (*description).to_owned(), + }) + .collect::>(), + )), + agents: Arc::from( + agents + .iter() + .map(|(name, description)| AgentDefSummary { + name: (*name).to_owned(), + description: (*description).to_owned(), + ..AgentDefSummary::default() + }) + .collect::>(), + ), + } + } + + fn make_app() -> App { + let (user_tx, _user_rx) = tokio::sync::mpsc::channel(1); + let (_agent_tx, agent_rx) = tokio::sync::mpsc::channel(1); + App::new(user_tx, agent_rx) + } + + #[test] + fn new_opens_on_all_tab_with_empty_query() { + let state = MentionPickerState::new(0, catalog(&["a.rs", "b.rs"], &[], &[])); + assert_eq!(state.active_tab, MentionTab::All); + assert!(!state.filtered.is_empty()); + } + + #[test] + fn empty_query_round_robins_across_categories() { + let cat = catalog( + &["a.rs", "b.rs", "c.rs", "d.rs", "e.rs"], + &[("skill_one", "desc")], + &[("agent_one", "desc")], + ); + let mut state = MentionPickerState::new(0, cat); + state.refilter(""); + let kinds: Vec = state.filtered.iter().map(|e| e.kind).collect(); + assert!( + kinds.contains(&MentionKind::Skill), + "skills must not be starved by files on the All tab: {kinds:?}" + ); + assert!( + kinds.contains(&MentionKind::Agent), + "agents must not be starved by files on the All tab: {kinds:?}" + ); + } + + #[test] + fn all_tab_omits_empty_categories() { + let cat = catalog(&["a.rs"], &[], &[]); + let mut state = MentionPickerState::new(0, cat); + state.refilter(""); + assert!(state.filtered.iter().all(|e| e.kind == MentionKind::File)); + } + + #[test] + fn single_tab_filters_only_that_category() { + let cat = catalog(&["main.rs"], &[("main_skill", "desc")], &[]); + let mut state = MentionPickerState::new(0, cat); + state.active_tab = MentionTab::Skills; + state.refilter("main"); + assert!(state.filtered.iter().all(|e| e.kind == MentionKind::Skill)); + } + + #[test] + fn typed_query_filters_and_sorts_by_score() { + let cat = catalog(&["src/main.rs", "src/lib.rs", "tests/foo.rs"], &[], &[]); + let mut state = MentionPickerState::new(0, cat); + state.refilter("main"); + assert!(!state.filtered.is_empty()); + assert!(state.filtered.iter().any(|e| e.display.contains("main"))); + } + + #[test] + fn indices_are_sorted_and_deduped() { + let cat = catalog(&["aabbaabb.rs"], &[], &[]); + let mut state = MentionPickerState::new(0, cat); + state.refilter("ab"); + for entry in &state.filtered { + let mut sorted = entry.indices.clone(); + sorted.sort_unstable(); + sorted.dedup(); + assert_eq!( + entry.indices, sorted, + "indices must already be sorted+deduped" + ); + } + } + + #[test] + fn move_selection_wraps() { + let cat = catalog(&["a.rs", "b.rs", "c.rs"], &[], &[]); + let mut state = MentionPickerState::new(0, cat); + assert_eq!(state.selected, 0); + state.move_selection(-1); + assert_eq!(state.selected, state.filtered.len() - 1); + } + + #[test] + fn move_selection_noop_on_empty() { + let mut state = MentionPickerState::new(0, MentionCatalog::default()); + state.move_selection(1); + assert_eq!(state.selected, 0); + } + + #[test] + fn files_loading_shows_placeholder() { + let cat = MentionCatalog { + files: None, + skills: None, + agents: Arc::from(Vec::::new()), + }; + let mut state = MentionPickerState::new(0, cat); + state.active_tab = MentionTab::Files; + state.refilter(""); + assert!(state.filtered.is_empty()); + assert_eq!(placeholder_text(&state), "indexing files…"); + } + + #[test] + fn files_loaded_empty_shows_different_placeholder() { + let cat = catalog(&[], &[], &[]); + let mut state = MentionPickerState::new(0, cat); + state.active_tab = MentionTab::Files; + state.refilter(""); + assert_eq!(placeholder_text(&state), "no files found"); + } + + #[test] + fn render_shows_tabs_and_counter() { + let cat = catalog(&["src/main.rs", "src/lib.rs"], &[], &[]); + let state = MentionPickerState::new(0, cat); + let input_area = Rect::new(0, 15, 60, 3); + let app = make_app(); + let output = render_to_string(60, 20, |frame, _area| { + let theme = crate::theme::Theme::default(); + render(&app, &state, frame, input_area, &theme); + }); + assert!(output.contains("All")); + assert!(output.contains("Files")); + assert!(output.contains("Skills")); + assert!(output.contains("Agents")); + assert!(output.contains("main.rs")); + } + + #[test] + fn render_multi_byte_path_highlights_without_panic() { + let cat = catalog(&["src/данные.rs"], &[], &[]); + let mut state = MentionPickerState::new(0, cat); + state.active_tab = MentionTab::Files; + state.refilter("дан"); + let input_area = Rect::new(0, 15, 60, 3); + let app = make_app(); + let output = render_to_string(60, 20, |frame, _area| { + let theme = crate::theme::Theme::default(); + render(&app, &state, frame, input_area, &theme); + }); + assert!(output.contains("данные")); + } +} diff --git a/crates/zeph-tui/src/widgets/mod.rs b/crates/zeph-tui/src/widgets/mod.rs index 701c497e7..891270b1f 100644 --- a/crates/zeph-tui/src/widgets/mod.rs +++ b/crates/zeph-tui/src/widgets/mod.rs @@ -9,11 +9,11 @@ pub mod context_gauge; pub mod diff; pub mod durable; pub mod elicitation; -pub mod file_picker; pub mod fleet; pub mod help; pub mod input; pub mod memory; +pub mod mention_picker; pub mod plan_view; pub mod resources; pub mod reverse_search; diff --git a/crates/zeph-tui/src/widgets/snapshots/zeph_tui__widgets__file_picker__tests__file_picker_empty_query_snapshot.snap b/crates/zeph-tui/src/widgets/snapshots/zeph_tui__widgets__file_picker__tests__file_picker_empty_query_snapshot.snap deleted file mode 100644 index fba504421..000000000 --- a/crates/zeph-tui/src/widgets/snapshots/zeph_tui__widgets__file_picker__tests__file_picker_empty_query_snapshot.snap +++ /dev/null @@ -1,19 +0,0 @@ ---- -source: crates/zeph-tui/src/widgets/file_picker.rs -expression: output ---- - - - - - - - - - -╭ Files ───────────────────────────────────────────────────╮ -│> │ -│> README.md │ -│ src/lib.rs │ -│ src/main.rs │ -╰──────────────────────────────────────────────────────────╯ diff --git a/crates/zeph-tui/src/widgets/snapshots/zeph_tui__widgets__file_picker__tests__file_picker_with_query_snapshot.snap b/crates/zeph-tui/src/widgets/snapshots/zeph_tui__widgets__file_picker__tests__file_picker_with_query_snapshot.snap deleted file mode 100644 index bb768835a..000000000 --- a/crates/zeph-tui/src/widgets/snapshots/zeph_tui__widgets__file_picker__tests__file_picker_with_query_snapshot.snap +++ /dev/null @@ -1,19 +0,0 @@ ---- -source: crates/zeph-tui/src/widgets/file_picker.rs -expression: output ---- - - - - - - - - - - - -╭ Files ───────────────────────────────────────────────────╮ -│> main │ -│> src/main.rs │ -╰──────────────────────────────────────────────────────────╯ diff --git a/specs/084-tui-mention-picker/spec.md b/specs/084-tui-mention-picker/spec.md index f6e4e575b..afed76ba5 100644 --- a/specs/084-tui-mention-picker/spec.md +++ b/specs/084-tui-mention-picker/spec.md @@ -56,7 +56,10 @@ agent mentions with `@` sigil preserved). - Multi-line skill/agent descriptions in the popup - Force-opening remote agent registries (A2A peer lookup) — only local definitions -- Cursor-movement shortcuts inside the popup (use Up/Down only) +- **Amended**: cursor-movement shortcuts inside the popup beyond Up/Down (selection) and + Left/Right (tab cycling, FR-004/D2) — Left/Right were originally scoped out here but + are a `must` requirement per FR-004; both are implemented. Genuinely out of scope: + any additional in-popup cursor gesture beyond those two pairs - Changing the existing file index TTL (30s) or MAX_RESULTS (10) --- @@ -92,7 +95,8 @@ GIVEN the mention picker popup is visible WHEN the user presses Left/Right arrow THEN the active tab changes AND the list re-filters to show only entries from that category -AND the All tab shows mixed results ranked by fuzzy match score +AND the All tab shows mixed results — ranked by fuzzy match score when a query is typed, + or round-robined across non-empty categories when the query is empty (see FR-018 amendment) ``` ### US-003: Complete a mention and continue typing @@ -106,7 +110,8 @@ SO THAT it is inserted into the input and I can continue typing ``` GIVEN the mention picker is visible with a selection WHEN the user presses Tab or Enter -THEN the `@query` text is replaced with the chosen entry +THEN the whole mention token (`@` through the next whitespace, not just the text up to + the cursor — see M4 in "Accepting a Selection") is replaced with the chosen entry AND a trailing space is inserted (unless already present) AND the popup closes AND the cursor is positioned after the space, ready to continue typing @@ -164,11 +169,11 @@ THEN a message "no results" appears and the popup remains open | FR-003 | WHEN the popup is visible every keystroke appends/deletes from the input buffer (not captured away); the popup reflects the buffer text after the `@` in real time | must | | FR-004 | WHEN the user presses Left/Right while the popup is visible THE SYSTEM SHALL cycle through tabs (All → Files → Skills → Agents → All) | must | | FR-005 | WHEN the user presses Up/Down while the popup is visible THE SYSTEM SHALL move the selection highlight within the current tab, wrapping at boundaries | must | -| FR-006 | WHEN Tab or Enter is pressed on a selected entry THE SYSTEM SHALL replace the typed query with the selected entry plus one trailing space; the popup closes | must | +| FR-006 | **(Amended, M4)** WHEN Tab or Enter is pressed on a selected entry THE SYSTEM SHALL replace the whole mention *token* (not just the typed query up to the cursor — see "Accepting a Selection") with the selected entry, appending one trailing space unless the next character is already whitespace; the popup closes | must | | FR-007 | WHEN Space is pressed while the popup is visible THE SYSTEM SHALL close the popup; the Space is inserted at the cursor position as an ordinary character and the `@query` text stays in the buffer as plain prose | must | | FR-008 | WHEN Esc is pressed THE SYSTEM SHALL close only the popup, retain Insert mode, keep the input buffer intact | must | | FR-009 | WHEN Backspace is pressed and the `@` is deleted THE SYSTEM SHALL close the popup automatically | must | -| FR-010 | WHEN cursor movement (arrow keys outside Up/Down for selection) leaves the `@query` span THE SYSTEM SHALL close the popup | must | +| FR-010 | **(Amended, D2)** WHEN cursor-mutating input other than Up/Down (selection) or Left/Right (tab cycling, FR-004) leaves the `@query` span — i.e. Home/End, Alt+Left/Alt+Right, Ctrl+A/Ctrl+E, or a mouse click — THE SYSTEM SHALL close the popup. Plain Left/Right never move the cursor while the popup is open, so they cannot trigger this rule. A cursor landing exactly one position after `@` (empty query, not yet past `@`) is still inside the span and does not close the popup by itself (see "Cursor Movement") | must | | FR-011 | WHEN the file index is (re)building (first build or stale-TTL rebuild) THE SYSTEM SHALL show an "indexing files…" placeholder row in the Files tab with no input loss race | must | | FR-012 | WHEN the mention picker renders THE SYSTEM SHALL use nucleo fuzzy matching for all three categories | must | | FR-013 | THE SYSTEM SHALL render match-character highlighting (nucleo indices) on all results | must | @@ -176,7 +181,7 @@ THEN a message "no results" appears and the popup remains open | FR-015 | WHEN a File entry is accepted THE SYSTEM SHALL insert the bare repo-relative path WITHOUT a file:// prefix or quotes | must | | FR-016 | WHEN a Skill entry is accepted THE SYSTEM SHALL insert the skill name as plain text (no `/skill` prefix, no forced activation) | must | | FR-017 | WHEN an Agent entry is accepted THE SYSTEM SHALL insert the agent name with the `@` sigil (e.g., `@my_agent`) | must | -| FR-018 | WHEN the All tab is active THE SYSTEM SHALL rank results by fuzzy match score across all categories, with per-row type indicators | must | +| FR-018 | **(Amended)** WHEN the All tab is active with a non-empty typed query THE SYSTEM SHALL rank results by fuzzy match score across all categories, with per-row type indicators. WHEN the query is empty, results are instead round-robined across non-empty categories (not score-ranked — an empty query scores every candidate equally, so score-ranking would let files, the largest category, crowd out Skills/Agents entirely; see "Empty-Query All-Tab Ordering", #6651 tracks further refinement) | must | | FR-019 | WHEN a Skills or Agents category is empty THE SYSTEM SHALL show a dimmed placeholder row; the All tab shall omit that category entirely | must | | FR-020 | THE SYSTEM SHALL apply the word-start trigger rule consistently: the popup opens ONLY when `@` is at position 0 or preceded by whitespace | must | @@ -227,22 +232,32 @@ The "query" is the text between the `@` character and the cursor, not including - Input: `"hello @search"`, cursor at end → query = `"search"` - Input: `"@foo bar"`, cursor after `@foo` → query = `"foo"` (space ends the span) -Cursor movement that leaves this span (e.g., moving left past the `@`, or moving to another word) closes the popup. +Cursor movement that leaves this span (e.g., moving left past the `@`, or moving to another word) closes the popup — via Home/End, Alt+Left/Alt+Right, Ctrl+A/Ctrl+E, or a mouse click; plain Left/Right do not move the cursor at all while the popup is open (they cycle tabs instead, FR-004/D2) and so cannot trigger this rule. See §7 "Cursor Movement" for the full, amended rule including the post-`@` boundary case. --- ## 6. Data Model -### Mention Picker State +### Mention Picker State (as implemented — corrects the original draft below) + +> **Amended (post-implementation).** The original draft stored `query: String` alongside +> `all_entries: MentionEntries`. The approved architecture (2026-07-27 R1) deliberately +> does **not** mirror the query into a second field: `at_char_index` (the char index of +> the triggering `@`) is the only position stored, and the query is always the buffer +> slice `input[at_char_index+1..cursor_position]`, re-derived by the reducer +> (`reducer::mention_picker_query`) after every action. This avoids the parallel-string +> bug class visible in `SlashAutocomplete*PushChar/PopChar`. Catalogs are also +> heterogeneous (`MentionCatalog`, not a single `MentionEntries`) since Files/Skills are +> `Option` (loading vs. loaded-empty) while Agents is not (see Data Sources below). ```rust struct MentionPickerState { - query: String, // text after `@` and before cursor - selected: usize, // current selection index in filtered list - filtered: Vec, // filtered results for active tab + at_char_index: usize, // char index of the triggering `@`; query is derived, never stored active_tab: MentionTab, // All | Files | Skills | Agents - scroll_offset: usize, // for scrolling when >MAX_VISIBLE results - all_entries: MentionEntries, // cached: Files + Skills + Agents + selected: usize, // current selection index in filtered list + filtered: Vec, // filtered results for active tab (≤ MAX_RESULTS) + catalog: MentionCatalog, // Files/Skills/Agents sources + matcher: Matcher, // nucleo matcher, reused across refilters } enum MentionTab { @@ -253,22 +268,22 @@ enum MentionTab { } struct MentionEntry { - entry_type: MentionEntryType, + kind: MentionKind, display: String, // e.g., "src/main.rs", "web_search", "my_agent" description: Option, // e.g., skill description, agent description (dimmed in popup) - match_indices: Vec, // nucleo match positions for highlighting + indices: Vec, // nucleo match char indices, sorted+deduped, for highlighting } -enum MentionEntryType { +enum MentionKind { File, Skill, Agent, } -struct MentionEntries { - files: Vec, - skills: Vec, - agents: Vec, +struct MentionCatalog { + files: Option>>, // None = index still building; Some(empty) = loaded, no files + skills: Option>, // None = catalog not yet delivered; Some(empty) = loaded, no skills + agents: Arc<[AgentDefSummary]>, // always populated from MetricsSnapshot (D1) — never "loading" } ``` @@ -285,11 +300,13 @@ Initialization: `None` (popup is closed). When `@` opens it at word-start, a new | Category | Source | API / Method | |----------|--------|------| -| **Files** | File index (existing `FileIndex` in `zeph-tui`) | `FileIndex::build()` (TTL 30s, supervised task), `FileIndex::search(query)` | -| **Skills** | Skill registry | `SkillRegistry::all_meta()` from `crates/zeph-skills/src/registry.rs:330` → yields `SkillMeta { name, description, … }` | -| **Agents** | Sub-agent definitions | `SubAgentManager::definitions()` or `SubAgentDef::load_all()` from `crates/zeph-subagent/src/def.rs:635` → yields `SubAgentDef { name, description, … }` | +| **Files** | File index (existing `FileIndex` in `zeph-tui`) | `FileIndex::build()` (TTL 30s, supervised task), `FileIndex::paths_arc()` | +| **Skills** | Skill registry, via a new event | `Channel::send_skill_catalog(&[SkillCatalogItem])` → `AgentEvent::SkillCatalog`, built from `SkillRegistry::all_meta()` (`crates/zeph-skills/src/registry.rs:330`), filtered to exclude `SkillTrustLevel::Blocked` | +| **Agents** | `MetricsSnapshot::agent_definitions` (D1 — no new plumbing) | `App.metrics.agent_definitions: Arc<[AgentDefSummary]>`, already populated and refreshed every render frame | -> **Data plumbing decision (fixed): catalog delivery via a dedicated event.** The TUI currently receives only runtime-active names via `MetricsSnapshot`. Full skill/agent catalogs (name + description) are delivered over the existing agent-event channel as a dedicated catalog event emitted once at startup and re-emitted on registry hot-reload — NOT embedded into the per-tick `MetricsSnapshot` (avoids bloating every metrics frame with static catalog data). +> **Data plumbing decision (amended, D1 — 2026-07-27 architecture review): Agents need no new plumbing.** The original text below asserted "the TUI currently receives only runtime-active names via `MetricsSnapshot`" — false for agents: `MetricsSnapshot::agent_definitions: Arc<[AgentDefSummary]>` already carries name + description for every `.zeph/agents/*.md` definition, is already in `App.metrics`, and is already consumed by the Settings view's Agents tab. It is an `Arc<[…]>` refreshed once per render frame (`poll_metrics`), so cloning it per picker-open is a refcount bump, not a reallocation — the "avoids bloating every metrics frame" rationale below does not apply to it. It is also re-derived on config reload, whereas a startup-only catalog event would go stale there. **Skills genuinely have no equivalent path** (`MetricsSnapshot` carries only `active_skills: Vec` names, no descriptions, and hot-reload never refreshes even that) — the dedicated-event decision below stands for Skills only. +> +> **Skills catalog delivery via a dedicated event.** Full skill catalogs (name + description) are delivered over the existing agent-event channel as a dedicated catalog event (`Channel::send_skill_catalog` / `AgentEvent::SkillCatalog`) emitted once at agent startup and re-emitted on skill hot-reload — NOT embedded into the per-tick `MetricsSnapshot` (avoids bloating every metrics frame with static catalog data). `Channel::send_skill_catalog` must be explicitly forwarded by every `impl Channel` wrapper (`AnyChannel`, `GatewayChannel`, and — the easiest one to miss — `AppChannel`, the binary's actual TUI-mode dispatcher) or the trait's no-op default silently wins and the Skills tab stays empty in the real binary while `TuiChannel`-level unit tests still pass. > > **Rationale**: `zeph-tui` has no dependency on `zeph-skills` (verified in `crates/zeph-tui/Cargo.toml`); event-based delivery keeps it that way and reuses the channel the TUI already consumes. Rejected alternative: direct supervised load in `zeph-tui` mirroring `FileIndex::build()` — would require adding a `zeph-skills` dependency and duplicate catalog-loading logic that `zeph-core` already performs at bootstrap. @@ -299,25 +316,34 @@ Initialization: `None` (popup is closed). When `@` opens it at word-start, a new ### Opening the Popup -When the word-start trigger is satisfied: +**Amended (matches the approved R1 naming deviation, §6/§9 above — no `Action::OpenMentionPicker` exists).** +When the word-start trigger is satisfied, inside the existing `Action::InsertChar('@')` reducer arm: -1. An `Action::OpenMentionPicker` is routed through the reducer -2. `MentionPickerState::new()` is created with: - - `query = ""` - - `filtered = all_entries` (All tab, no filter) - - `selected = 0` - - `active_tab = MentionTab::All` -3. The first keystroke after `@` appends to `query` and re-filters +1. The `@` is inserted into the input buffer (as any other character would be) +2. `MentionPickerState::new(at_char_index, app.mention_catalog())` is created, where + `at_char_index` is the char index of the just-inserted `@`; no `query` field is + stored (see §6) — the query starts implicitly empty since `cursor_position == + at_char_index + 1` +3. `refilter("")` runs immediately, populating `filtered` via round-robin (§3 FR-018 amendment) +4. Further keystrokes flow through the ordinary `InsertChar`/`Delete*` reducer arms; + each is followed by `sync_mention_picker`, which re-derives the query from the + buffer and re-filters (see §6) — there is no separate "append to query" step ### Typing & Filtering +**Amended** — no `scroll_offset` field exists; `MAX_RESULTS = 10` caps `filtered` at a +size that always fits the popup's fixed height, so ratatui's `ListState` selection alone +keeps `selected` visible with no separate scroll bookkeeping. + On every character typed (while the popup is open): -1. The character is appended to `query` and to the input buffer simultaneously -2. The `filtered` list is re-computed using `nucleo_matcher::Matcher` across the active tab -3. `selected` resets to 0 (or stays at 0 if already there) -4. `scroll_offset` is adjusted if needed to keep `selected` in view -5. The popup re-renders with highlighted match indices and result count +1. The character is inserted into the input buffer via the ordinary `InsertChar` + reducer arm (never a separate `query` field — see §6) +2. `sync_mention_picker` re-derives the query from the buffer and calls + `refilter(&query)`, which re-computes `filtered` for the active tab using + `nucleo_matcher::Matcher` +3. `selected` resets to 0 inside `refilter` +4. The popup re-renders with highlighted match indices and result count ### Tab Cycling (Left/Right) @@ -329,12 +355,25 @@ On every character typed (while the popup is open): - Up/Down arrows move `selected` within the current `filtered` list - Wraps at boundaries (down on last → wraps to 0; up on 0 → wraps to last) -- `scroll_offset` adjusts to keep selection visible +- **Amended**: no separate `scroll_offset` bookkeeping — `filtered` is always ≤ + `MAX_RESULTS` (10), so every row fits the popup and `ListState::select` alone + drives ratatui's highlight ### Accepting a Selection (Tab or Enter) -1. The `@query` text is replaced with the selected entry **including the `@` sigil** (for agents) or as plain text (for files/skills) -2. A trailing space is inserted (for chaining) +1. **(Amended, M4 — 2026-07-27 architecture review)** The replacement range is the whole + **mention token** — `[at_char_index .. token_end]`, where `token_end` is the first + whitespace char at or after the cursor (or end of buffer) — not just `[at_char_index .. + cursor_position]`. The token is replaced with the selected entry **including the `@` + sigil** (for agents) or as plain text (for files/skills). This matters when the cursor + sits *inside* the mention word (reachable via Alt+Left, Ctrl+A, or a mouse click) — + e.g. `"@foo"` with the cursor after `@f` still replaces the whole `"@foo"`, not just + `"@f"`, which would otherwise mangle the buffer to `"src/main.rs oo"`. The *query* used + for filtering is unaffected by this and remains `[at_char_index+1 .. cursor_position]` + exactly as defined above — only the accept-time replacement range is token-bounded. +2. A trailing space is inserted (for chaining) unless the character immediately after + `token_end` is already whitespace (avoids a double space, e.g. `"@foo bar"` accepting + to `"src/main.rs bar"`, not `"src/main.rs bar"`) 3. The popup closes: `mention_picker = None` 4. For Tab: the cursor is positioned after the space; Insert mode continues 5. For Enter: same insertion + behavior as Tab (unlike slash-autocomplete, Enter does NOT auto-submit when accepting a mention) @@ -374,9 +413,23 @@ Example: ### Cursor Movement -If the user presses arrow keys (Left/Right/Home/End) and the cursor moves **outside the `@query` span**, the popup closes. - -- Example: `"@file"`, cursor at end, user presses Left twice → cursor now before `fil`, popup closes +**Amended (D2 — 2026-07-27 architecture review): supersedes the original text below.** +Plain `Left`/`Right` are claimed by Tab Cycling (FR-004) while the popup is open — they +never move the cursor and never close the popup by themselves. Span-exit closure instead +applies to **`Home`/`End`, `Alt+Left`/`Alt+Right` (word-boundary movement), `Ctrl+A`/ +`Ctrl+E`, and mouse clicks** — any cursor mutation that lands outside `[at_char_index+1 .. +cursor_position]`'s valid span (i.e. at or before `at_char_index`, or past a whitespace +character) closes the popup. Note the boundary case: a cursor landing exactly one position +after `@` (e.g. after a single `Alt+Left` from the end of `"@foo"`) is *still inside* the +span (empty query, not yet past `@`) and does **not** close the popup by itself — a second +`Alt+Left` (or equivalent) that moves further left, past the `@`, does close it. This is +the same state Accepting a Selection's M4 amendment discusses for the *accept* semantics +at that same boundary — see above. + +- Superseded example (was: "`Left` twice closes the popup") — no longer applicable, since + plain `Left`/`Right` cycle tabs instead. Use `Alt+Left` for the word-boundary-exit case: + `"@foo"`, cursor at end, `Alt+Left` once → cursor after `@` (still inside span, popup + stays open); `Alt+Left` again → cursor moves into whatever precedes `@`, popup closes. ### File Index Building (Race-Free) @@ -384,7 +437,7 @@ The file index build runs as a supervised task (see spec-039 / `TaskSupervisor`) - **Before index is ready**: The popup opens immediately with an "indexing files…" placeholder row in the Files tab - **No input loss**: Keystrokes are never lost; they append to `query` and continue filtering (even if only one placeholder row is shown until the index arrives) -- **Seamless transition**: Once `FileIndex::search()` returns real results, the filtered list updates on the next keystroke +- **Seamless transition (amended)**: `FileIndex::search` does not exist in the shipped code — `PickerMatch`/`FilePickerState` (and their `search`/`update_query` methods) were deleted as dead code, superseded by `MentionPickerState::refilter`. Once the background build resolves, `App::poll_pending_file_index` installs `FileIndex::paths_arc()` into `MentionCatalog.files` and calls `refilter` immediately — no need to wait for the next keystroke --- @@ -393,9 +446,9 @@ The file index build runs as a supervised task (see spec-039 / `TaskSupervisor`) | Scenario | Expected Behavior | |----------|-------------------| | User types `@@` (two `@` symbols) | First `@` opens picker; second `@` is appended to `query` and searched (query = "@") | -| Cursor in middle of `@query` (e.g., `@file`, cursor after `@fi`) | Up/Down/Left/Right at this position closes popup (cursor leaving span); typed chars are inserted mid-query | +| Cursor in middle of `@query` (e.g., `@file`, cursor after `@fi`) | **Amended (M4/D2)**: Up/Down move selection (do not affect the cursor); Left/Right cycle tabs (do not affect the cursor); the popup stays **open** in this position — it does not close merely because the cursor is inside the token. Accepting here (Tab/Enter) replaces the *whole* mention token (`@file`, not just `@fi`), per the M4 amendment to "Accepting a Selection" above. Typed chars are inserted mid-query and the popup re-filters normally | | Terminal is very narrow (<30 cols) | Popup clips gracefully (ratatui `Rect` clamping); no panic | -| File index build takes >30s (timeout) | FileIndex::search fails; Files tab shows placeholder "index unavailable" or empty | +| File index build takes >30s (timeout) | **Amended**: there is no explicit build timeout and no "index unavailable" placeholder in the shipped code. While `MentionCatalog.files` is `None` (build not yet complete) the Files tab shows "indexing files…"; once loaded, an empty result set shows "no files found" | | Skills category empty, All tab open | All tab shows only files + agents; Skills section is omitted | | User accepts an unknown agent name (e.g., `@nonexistent`) | Mention is inserted as `@nonexistent`; it flows to the LLM or slash-command dispatch (see spec-044 dispatch behavior) — never an error in the picker | | Match highlighting on multi-byte UTF-8 | Use nucleo indices directly; no byte-boundary truncation issues | @@ -408,11 +461,13 @@ The file index build runs as a supervised task (see spec-039 / `TaskSupervisor`) ``` crates/zeph-tui/src/widgets/mention_picker.rs - — MentionPickerState struct - — MentionEntry, MentionTab enums - — open() / refilter() / move_up() / move_down() / accept() methods - — render(state, frame, area) function with tabs, highlight, result counter - — nucleo integration for fuzzy matching + — MentionPickerState, MentionCatalog, MentionEntry structs; MentionTab, MentionKind enums + — **Amended**: `MentionPickerState::new()` / `refilter()` / `move_selection(delta: i32)` + methods; accept and close are reducer-side (`Action::MentionPickerAccept`/ + `CloseMentionPicker` in `app/reducer.rs`), not widget methods — matches the + reducer-purity invariant (§11 inv. 8) + — render(app, state, frame, input_area, theme) function with tabs, highlight, result counter + — nucleo integration for fuzzy matching (two-phase: score-only, then materialize top MAX_RESULTS) ``` ### Modified Files @@ -420,9 +475,15 @@ crates/zeph-tui/src/widgets/mention_picker.rs ``` crates/zeph-tui/src/app/ — add field: mention_picker: Option - — add Action variants: OpenMentionPicker, MentionPickerMove(VertDir), - MentionPickerInput(PaletteEdit), MentionPickerTabChange(Direction), - MentionPickerAccept, CloseMentionPicker + — **Amended (approved naming deviation, R1)**: no `OpenMentionPicker`/`MentionPickerInput` + variants — opening is a side effect of the existing `Action::InsertChar('@')` arm + (mirroring how `/` opens slash-autocomplete), and all text edits continue to flow + through the existing `InsertChar`/`Delete*`/`MoveCursor` arms rather than a + parallel input action. Adding those two variants would require duplicating buffer + mutation and reintroduce the parallel-string bug class this design deliberately + avoids (see the amended §6 Mention Picker State note). Action variants actually + added: `CloseMentionPicker`, `MentionPickerMove(VertDir)`, + `MentionPickerTabChange(HorizDir)`, `MentionPickerAccept` — modify the Insert-mode Char('@') branch: insert the char, check word-start, open picker — the modal file picker path is REPLACED entirely: `file_picker_state`, `decode_file_picker_key`, and the FilePicker* Action variants are removed together with their modal key takeover @@ -439,7 +500,9 @@ crates/zeph-tui/src/widgets/mod.rs ### Reused Without Modification -- `crates/zeph-tui/src/file_picker.rs` — FileIndex infrastructure (search, TTL) +- `crates/zeph-tui/src/file_picker.rs` — FileIndex infrastructure (`build()`/`paths_arc()`, + TTL); `FileIndex::search`/`PickerMatch` did not survive — see the amended note in §7 + "File Index Building" - `crates/zeph-tui/src/command.rs` — styling, layout utilities - `zeph-skills` SkillRegistry and `zeph-subagent` SubAgentDef (read-only) - ratatui List, Clear, Paragraph widgets @@ -458,7 +521,7 @@ crates/zeph-tui/src/widgets/mod.rs |----|-----------|--------------| | AC-001 | Typing `@` at start of empty input opens the picker | Unit test: verify `mention_picker.is_some()` after Char('@') on empty input | | AC-002 | Typing `@` mid-word does NOT open picker | Unit test: input = "user", type '@', assert `mention_picker.is_none()` and input = "user@" | -| AC-003 | Tab cycles through All → Files → Skills → Agents → All | Unit test: verify active_tab sequence | +| AC-003 | **Amended (M7)**: `Left`/`Right` cycle through All → Files → Skills → Agents → All (not `Tab`, which is bound to Accept per FR-006/US-003) | Unit test: verify active_tab sequence after `Left`/`Right` key events | | AC-004 | Filtering works with nucleo matching | Unit test: query "fil" matches "src/file.rs", "*.filters", etc. | | AC-005 | Up/Down wraps at boundaries | Unit test | | AC-006 | Tab on a file entry inserts path + space | Unit test: entry = File("src/main.rs"), accept → input has "src/main.rs " | diff --git a/specs/UX/mention-routing.md b/specs/UX/mention-routing.md index 5d79e2b4a..4599e30bf 100644 --- a/specs/UX/mention-routing.md +++ b/specs/UX/mention-routing.md @@ -133,7 +133,7 @@ stabilisation). Label: `enhancement`, `P4`, `tui`, `a2a`. The primary TUI-side blocker for agent mention routing was the lack of an inline `@` picker UI. This blocker is now **resolved** by spec [[084-tui-mention-picker/spec]], which delivers: 1. **Inline non-modal `@` popup** — typed `@` mentions become discoverable and auto-completable in the TUI -2. **Agent category** — agent definitions are now plumbed into the mention picker (via `SubAgentManager::definitions()`) +2. **Agent category** — agent definitions are now plumbed into the mention picker's Agents tab, reading directly from the existing `MetricsSnapshot::agent_definitions: Arc<[AgentDefSummary]>` (already populated from loaded `.zeph/agents/*.md` definitions and refreshed every render frame) — no new plumbing was needed for this category (spec 084 §6 D1) 3. **Mention sigil preservation** — accepted `@agent` mentions retain the sigil, allowing downstream slash-command dispatch to correctly route them The routing invariants in this spec (§ Key Invariants) remain binding and are inherited by spec 081. Full A2A-remote mention routing (agent discovery from remote A2A peers) remains deferred as P4 follow-up work. diff --git a/src/channel.rs b/src/channel.rs index 4340475d7..f7cb03958 100644 --- a/src/channel.rs +++ b/src/channel.rs @@ -16,8 +16,8 @@ use zeph_common::TaskSupervisor; use crate::execution_mode::ExecutionMode; #[cfg(feature = "tui")] use zeph_core::channel::{ - Channel, ChannelError, ChannelMessage, ElicitationRequest, ElicitationResponse, StopHint, - ToolOutputEvent, ToolStartEvent, + Channel, ChannelError, ChannelMessage, ElicitationRequest, ElicitationResponse, + SkillCatalogItem, StopHint, ToolOutputEvent, ToolStartEvent, }; use zeph_core::config::Config; use zeph_core::json_event_sink::JsonEventSink; @@ -92,6 +92,9 @@ impl Channel for AppChannel { async fn send_status(&mut self, text: &str) -> Result<(), ChannelError> { dispatch_app_channel!(self, send_status, text) } + async fn send_skill_catalog(&mut self, items: &[SkillCatalogItem]) -> Result<(), ChannelError> { + dispatch_app_channel!(self, send_skill_catalog, items) + } async fn send_queue_count(&mut self, count: usize) -> Result<(), ChannelError> { dispatch_app_channel!(self, send_queue_count, count) } @@ -369,6 +372,8 @@ mod tests { ch.send_typing().await.unwrap(); // 8. send_status ch.send_status("working").await.unwrap(); + // 8b. send_skill_catalog + ch.send_skill_catalog(&[]).await.unwrap(); // 9. send_thinking_chunk ch.send_thinking_chunk("...").await.unwrap(); // 10. send_queue_count @@ -502,6 +507,45 @@ mod tests { other => panic!("expected AgentEvent::ContextEstimate, got {other:?}"), } } + + /// Regression test for the mention-picker Skills tab (spec 084 S3): the + /// `app_channel_forwards_all_channel_methods` checklist above only ever + /// constructs `AppChannel::Standard(AnyChannel::Cli(..))`, whose `CliChannel` + /// has no `send_skill_catalog` override — that call silently resolves to the + /// trait's no-op default regardless of whether `AppChannel`'s own forwarding + /// arm exists, so that test cannot detect the arm being deleted. This test + /// exercises the `AppChannel::Tui` variant directly and asserts the catalog + /// actually reaches `TuiChannel`'s `AgentEvent` channel. + #[tokio::test] + async fn app_channel_forwards_send_skill_catalog_to_real_implementation() { + use zeph_core::channel::SkillCatalogItem; + use zeph_tui::{AgentEvent, TuiChannel}; + + let (_user_tx, user_rx) = tokio::sync::mpsc::channel(1); + let (agent_tx, mut agent_rx) = tokio::sync::mpsc::channel(4); + let mut ch = AppChannel::Tui(TuiChannel::new(user_rx, agent_tx)); + + let items = vec![SkillCatalogItem { + name: "web_search".to_owned(), + description: "Search the web".to_owned(), + }]; + ch.send_skill_catalog(&items).await.unwrap(); + + let event = tokio::time::timeout(std::time::Duration::from_secs(5), agent_rx.recv()) + .await + .expect( + "send_skill_catalog() must forward an AgentEvent instead of no-op'ing \ + (timed out waiting for it)", + ) + .expect("agent_tx channel closed unexpectedly"); + match event { + AgentEvent::SkillCatalog(got) => { + assert_eq!(got.len(), 1); + assert_eq!(got[0].name, "web_search"); + } + other => panic!("expected AgentEvent::SkillCatalog, got {other:?}"), + } + } } pub(crate) async fn build_cli_history( diff --git a/src/gateway_spawn.rs b/src/gateway_spawn.rs index be806afc5..f4b4f0f6e 100644 --- a/src/gateway_spawn.rs +++ b/src/gateway_spawn.rs @@ -113,6 +113,13 @@ impl zeph_core::channel::Channel for GatewayChan self.inner.send_status(text).await } + async fn send_skill_catalog( + &mut self, + items: &[zeph_core::channel::SkillCatalogItem], + ) -> Result<(), zeph_core::channel::ChannelError> { + self.inner.send_skill_catalog(items).await + } + async fn send_thinking_chunk( &mut self, chunk: &str,