diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index 23d7ca1959..1854203faa 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -1192,10 +1192,11 @@ impl VimMode { } } -/// Cached @-mention completion results to avoid re-walking the filesystem when -/// the cursor moves inside the same mention token. -#[derive(Debug, Clone)] -pub struct MentionCompletionCache { +/// Everything an @-mention completion walk depends on. Two walks with equal +/// keys produce identical results, so the key doubles as the cache/invalidation +/// identity for both the completed cache and the in-flight background walk. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MentionCompletionKey { /// Workspace root used for this completion walk. pub workspace: PathBuf, /// Process cwd captured for cwd-relative completion entries. @@ -1213,16 +1214,49 @@ pub struct MentionCompletionCache { /// Whether symlink following was enabled for this completion walk. /// Included so live config changes invalidate cached popup results. pub follow_links: bool, +} + +impl MentionCompletionKey { + /// Whether `other` differs from `self` only in the typed partial — + /// i.e. the same session/config context at a different keystroke. + #[must_use] + pub fn same_context(&self, other: &Self) -> bool { + self.workspace == other.workspace + && self.cwd == other.cwd + && self.limit == other.limit + && self.walk_depth == other.walk_depth + && self.behavior == other.behavior + && self.follow_links == other.follow_links + } +} + +/// Cached @-mention completion results to avoid re-walking the filesystem when +/// the cursor moves inside the same mention token. +#[derive(Debug, Clone)] +pub struct MentionCompletionCache { + /// The walk inputs that produced `entries`. + pub key: MentionCompletionKey, /// Cached completion entries. pub entries: Vec, } +/// An @-mention completion walk running on a background thread (#3899). +/// The result lands in `cell` and is promoted (or discarded, if the needle +/// moved on) by `visible_mention_menu_entries` / `poll_mention_completion`. +#[derive(Debug, Clone)] +pub struct MentionCompletionPending { + /// The walk inputs the background thread was started with. + pub key: MentionCompletionKey, + /// Shared cell the background walk writes its entries into. + pub cell: std::sync::Arc>>>, +} + /// Cached full candidate walk for @-mention completions. One workspace walk /// serves every subsequent keystroke of the same mention token — the /// per-keystroke synchronous re-walk was the dominant composer latency on /// large repos (#3757). Path-like partials (containing `/` or starting with /// `.`) bypass this cache because local path-reference completions are -/// needle-dependent. +/// needle-dependent; they take the background walk instead (#3899). #[derive(Debug, Clone)] pub struct MentionCandidateCache { pub workspace: PathBuf, @@ -1264,6 +1298,8 @@ pub struct ComposerState { /// Cached @-mention completions to avoid re-walking the filesystem when /// the cursor moves inside the same mention token. pub mention_completion_cache: Option, + /// In-flight background @-mention completion walk, if any (#3899). + pub mention_completion_pending: Option, /// Cached full candidate list so successive keystrokes inside one mention /// token filter in memory instead of re-walking the workspace (#3757). pub mention_candidate_cache: Option, @@ -1303,6 +1339,7 @@ impl Default for ComposerState { mention_menu_selected: 0, mention_menu_hidden: false, mention_completion_cache: None, + mention_completion_pending: None, mention_candidate_cache: None, vim_enabled: false, vim_mode: VimMode::Normal, @@ -2640,6 +2677,7 @@ impl App { mention_menu_selected: 0, mention_menu_hidden: false, mention_completion_cache: None, + mention_completion_pending: None, mention_candidate_cache: None, vim_enabled: composer_vim_enabled, vim_mode: VimMode::Normal, diff --git a/crates/tui/src/tui/file_mention.rs b/crates/tui/src/tui/file_mention.rs index 68dcd548e8..09db6d3c6e 100644 --- a/crates/tui/src/tui/file_mention.rs +++ b/crates/tui/src/tui/file_mention.rs @@ -27,7 +27,9 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; -use crate::tui::app::{App, MentionCompletionCache}; +use crate::tui::app::{ + App, MentionCompletionCache, MentionCompletionKey, MentionCompletionPending, +}; use crate::working_set::Workspace; /// Maximum number of `@`-mentions whose contents are inlined into one user @@ -219,102 +221,251 @@ pub fn visible_mention_menu_entries(app: &mut App, limit: usize) -> Vec return Vec::new(); } - let workspace = app.workspace.clone(); - let cwd = std::env::current_dir().ok(); - let walk_depth = app.mention_walk_depth; - let behavior = app.mention_menu_behavior.clone(); - let follow_links = app.workspace_follow_symlinks; + let key = MentionCompletionKey { + workspace: app.workspace.clone(), + cwd: std::env::current_dir().ok(), + partial, + limit, + walk_depth: app.mention_walk_depth, + behavior: app.mention_menu_behavior.clone(), + follow_links: app.workspace_follow_symlinks, + }; + if let Some(ref cache) = app.composer.mention_completion_cache - && cache.workspace == workspace - && cache.cwd == cwd - && cache.partial == partial - && cache.limit == limit - && cache.walk_depth == walk_depth - && cache.behavior == behavior - && cache.follow_links == follow_links + && cache.key == key { return cache.entries.clone(); } // Fast path (#3757): for non-path-like partials the candidate set is // needle-independent, so one cached walk serves every keystroke of the - // mention token and ranking happens in memory. Path-like partials fall - // through to the live walk because local path-reference completions are - // needle-gated (see `should_try_local_reference_completion`). - let path_like = partial.starts_with('.') || partial.contains('/') || partial.contains('\\'); - if behavior != "browser" && !path_like { + // mention token and ranking happens in memory. Path-like and browser + // partials fall through to the background walk (#3899) because local + // path-reference completions are needle-gated + // (see `should_try_local_reference_completion`). + let path_like = + key.partial.starts_with('.') || key.partial.contains('/') || key.partial.contains('\\'); + if key.behavior != "browser" && !path_like { const CANDIDATE_TTL: std::time::Duration = std::time::Duration::from_secs(4); let fresh = app .composer .mention_candidate_cache .as_ref() .is_some_and(|c| { - c.workspace == workspace - && c.cwd == cwd - && c.walk_depth == walk_depth - && c.follow_links == follow_links + c.workspace == key.workspace + && c.cwd == key.cwd + && c.walk_depth == key.walk_depth + && c.follow_links == key.follow_links && c.collected_at.elapsed() < CANDIDATE_TTL }); if !fresh { let ws = Workspace::with_cwd_depth_and_follow_links( - workspace.clone(), - cwd.clone(), - walk_depth, - follow_links, + key.workspace.clone(), + key.cwd.clone(), + key.walk_depth, + key.follow_links, ); app.composer.mention_candidate_cache = Some(crate::tui::app::MentionCandidateCache { - workspace: workspace.clone(), - cwd: cwd.clone(), - walk_depth, - follow_links, + workspace: key.workspace.clone(), + cwd: key.cwd.clone(), + walk_depth: key.walk_depth, + follow_links: key.follow_links, collected_at: std::time::Instant::now(), candidates: ws.completion_candidates(), }); } let ranked = match app.composer.mention_candidate_cache.as_ref() { - Some(cache) => { - crate::working_set::rank_completion_candidates(&cache.candidates, &partial, limit) - } + Some(cache) => crate::working_set::rank_completion_candidates( + &cache.candidates, + &key.partial, + key.limit, + ), None => Vec::new(), }; let entries = super::file_frecency::rerank_by_frecency(ranked); app.composer.mention_completion_cache = Some(MentionCompletionCache { - workspace, - cwd, - partial, - limit, - walk_depth, - behavior, - follow_links, + key, entries: entries.clone(), }); return entries; } + // Drain a completed background walk: promote it when it still matches + // the live key, discard it when the needle has moved on (#3899). + if let Some(pending) = app.composer.mention_completion_pending.take() { + let ready = pending.cell.lock().ok().and_then(|mut cell| cell.take()); + match ready { + Some(entries) => { + let promoted = pending.key == key; + app.composer.mention_completion_cache = Some(MentionCompletionCache { + key: pending.key, + entries: entries.clone(), + }); + if promoted { + return entries; + } + } + None => { + // Still walking. Keep it even when the needle has moved on: + // dropping the handle cannot stop the blocking walk, so + // respawning per keystroke would stack concurrent + // full-workspace walks on the blocking pool. Keeping the + // old walk serializes them — its result is discarded on + // landing (key mismatch above) and the very next call + // spawns the walk for the live needle. + app.composer.mention_completion_pending = Some(pending); + return stale_mention_fallback_entries(app, &key); + } + } + } + + // Cache miss with no matching walk in flight: start one. Without a tokio + // runtime (plain unit tests) fall back to the synchronous walk so + // results and test semantics are identical to the pre-async behavior. + if tokio::runtime::Handle::try_current().is_ok() { + let cell = std::sync::Arc::new(std::sync::Mutex::new(None)); + let completion = MentionWalkCompletion { cell: cell.clone() }; + let walker_key = key.clone(); + crate::utils::spawn_blocking_supervised("mention-completion", move || { + completion.fill(run_mention_completion_walk(&walker_key)); + }); + app.composer.mention_completion_pending = Some(MentionCompletionPending { + key: key.clone(), + cell, + }); + stale_mention_fallback_entries(app, &key) + } else { + let entries = run_mention_completion_walk(&key); + app.composer.mention_completion_cache = Some(MentionCompletionCache { + key, + entries: entries.clone(), + }); + entries + } +} + +/// Completion guarantee for a background mention walk: the shared cell is +/// ALWAYS filled, even if the walk panics. Without this, a panicking walk +/// would leave `mention_completion_pending` set forever and the Enter-hold +/// for the pending token would never release (until Esc). On unwind the +/// `Drop` impl writes an empty result, which drains normally. +struct MentionWalkCompletion { + cell: std::sync::Arc>>>, +} + +impl MentionWalkCompletion { + fn fill(self, entries: Vec) { + if let Ok(mut guard) = self.cell.lock() { + *guard = Some(entries); + } + // Normal path: `self` drops here with the cell already filled, so + // the Drop impl is a no-op. + } +} + +impl Drop for MentionWalkCompletion { + fn drop(&mut self) { + if let Ok(mut guard) = self.cell.lock() + && guard.is_none() + { + *guard = Some(Vec::new()); + } + } +} + +/// Run the completion walk described by `key`. Called on a background thread +/// for live keystrokes and synchronously when no runtime is available. +fn run_mention_completion_walk(key: &MentionCompletionKey) -> Vec { let ws = Workspace::with_cwd_depth_and_follow_links( - workspace.clone(), - cwd.clone(), - walk_depth, - app.workspace_follow_symlinks, + key.workspace.clone(), + key.cwd.clone(), + key.walk_depth, + key.follow_links, ); - let entries = if behavior == "browser" { - find_file_mention_browser_completions(&ws, &partial, limit) + if key.behavior == "browser" { + find_file_mention_browser_completions(&ws, &key.partial, key.limit) } else { - find_file_mention_completions(&ws, &partial, limit) - }; + find_file_mention_completions(&ws, &key.partial, key.limit) + } +} - app.composer.mention_completion_cache = Some(MentionCompletionCache { - workspace, - cwd, - partial, - limit, - walk_depth, - behavior, - follow_links, - entries: entries.clone(), - }); +/// While a walk is in flight, keep showing the previous keystroke's entries +/// so the popup stays open and Enter/Up/Down keep routing to it — but only +/// when the cached results come from the same session context AND the same +/// token lineage (the live partial extends the cached one, or vice versa +/// for backspacing), and only the entries that still match the LIVE +/// needle. Enter/Tab apply the shown list against the live token, so a +/// fast Enter after a keystroke must never splice an entry the fresh walk +/// would not have returned (e.g. `docs/apple.md` against `@docs/ab`). +/// The filter mirrors the walk's case-insensitive substring match, so the +/// surviving entries are a subset of the fresh walk's results; when it +/// empties the list, the popup reads as pending and Enter is held instead. +fn stale_mention_fallback_entries(app: &App, key: &MentionCompletionKey) -> Vec { + let needle = key.partial.to_lowercase(); + app.composer + .mention_completion_cache + .as_ref() + .filter(|cache| cache.key.same_context(key)) + .filter(|cache| { + key.partial.starts_with(&cache.key.partial) + || cache.key.partial.starts_with(&key.partial) + }) + .map(|cache| { + cache + .entries + .iter() + .filter(|entry| entry.to_lowercase().contains(&needle)) + .cloned() + .collect() + }) + .unwrap_or_default() +} - entries +/// Whether a completion walk is in flight for the mention token under the +/// cursor while the popup has nothing to show yet. The key router treats +/// this as popup-open for Enter/Esc: before #3899 the walk ran +/// synchronously inside the key event, so Enter could never submit a +/// half-typed mention — swallowing Enter for the walk's duration preserves +/// that guarantee without blocking the UI thread. +#[must_use] +pub fn mention_walk_pending(app: &App) -> bool { + !app.mention_menu_hidden + && app.composer.mention_completion_pending.is_some() + && partial_file_mention_at_cursor(&app.input, app.cursor_position).is_some() +} + +/// Drain a completed background completion walk outside the render path. +/// Called once per event-loop iteration (next to the workspace-context +/// refresh) so a finished walk repaints the popup without waiting for an +/// input event. Sets `needs_redraw` only when the drained result changes +/// what the popup would show — never while merely waiting (#3899). +pub fn poll_mention_completion(app: &mut App) { + if app.composer.mention_completion_pending.is_none() { + return; + } + // Cursor left the mention token: the walk result can never be shown. + if app.mention_menu_hidden + || partial_file_mention_at_cursor(&app.input, app.cursor_position).is_none() + { + app.composer.mention_completion_pending = None; + return; + } + let Some(pending) = app.composer.mention_completion_pending.take() else { + return; + }; + let ready = pending.cell.lock().ok().and_then(|mut cell| cell.take()); + match ready { + Some(entries) => { + app.composer.mention_completion_cache = Some(MentionCompletionCache { + key: pending.key, + entries, + }); + app.needs_redraw = true; + } + None => { + app.composer.mention_completion_pending = Some(pending); + } + } } /// Apply the currently selected `@`-mention popup entry to the composer @@ -947,6 +1098,28 @@ mod tests { use super::*; use tempfile::TempDir; + /// A background walk must ALWAYS complete its cell — a panicking walk + /// otherwise leaves `mention_completion_pending` set forever and the + /// Enter-hold for that token never releases. + #[test] + fn mention_walk_completion_fills_cell_even_on_unwind() { + // Normal path: the filled result is preserved. + let cell = std::sync::Arc::new(std::sync::Mutex::new(None)); + let completion = MentionWalkCompletion { cell: cell.clone() }; + completion.fill(vec!["docs/a.md".to_string()]); + assert_eq!( + cell.lock().unwrap().as_deref(), + Some(&["docs/a.md".to_string()][..]) + ); + + // Unwind path: dropping without filling writes an empty result so + // the pending walk drains instead of soft-locking. + let cell = std::sync::Arc::new(std::sync::Mutex::new(None)); + let completion = MentionWalkCompletion { cell: cell.clone() }; + drop(completion); + assert_eq!(cell.lock().unwrap().as_deref(), Some(&[][..])); + } + /// #101 regression — workspace-vs-cwd divergence: `@bar.txt` typed from /// the cwd `/sub` MUST resolve to `/sub/bar.txt`, never to /// `/bar.txt` (which doesn't exist). diff --git a/crates/tui/src/tui/file_tree.rs b/crates/tui/src/tui/file_tree.rs index 34cd1486a8..ed431d1c49 100644 --- a/crates/tui/src/tui/file_tree.rs +++ b/crates/tui/src/tui/file_tree.rs @@ -4,7 +4,7 @@ //! navigate, Enter expands/collapses directories or inserts `@path` for files, //! Esc closes the pane. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -34,6 +34,15 @@ pub struct FileTreeEntry { pub expanded: bool, } +/// An in-flight background expand walk (#3900). The sequence number +/// distinguishes the latest walk for a directory from superseded ones so a +/// stale result can never be spliced in after a re-toggle. +#[derive(Debug, Clone)] +struct PendingExpand { + seq: u64, + cell: Arc>>>, +} + /// Mutable state for the file-tree pane. #[derive(Debug, Clone)] pub struct FileTreeState { @@ -51,13 +60,32 @@ pub struct FileTreeState { pub is_loading: bool, /// Shared cell for async tree-building results (#399 S3). loading_cell: Option>>>>, + /// In-flight expand walks keyed by normalised directory path (#3900). + pending_expands: HashMap, + /// Monotonic counter identifying the latest expand walk per directory. + expand_seq: u64, } impl FileTreeState { /// Build a fresh tree state by walking `workspace`. - /// Spawns the initial walk on a background thread (#399 S3). + /// Spawns the initial walk on a background thread (#399 S3); without a + /// tokio runtime (plain unit tests) the walk runs synchronously. pub fn new(workspace: &Path) -> Self { let expanded_dirs = HashSet::new(); + if tokio::runtime::Handle::try_current().is_err() { + let entries = build_file_tree_inner(workspace, &expanded_dirs, None); + return Self { + entries, + cursor: 0, + scroll_offset: 0, + expanded_dirs, + workspace: workspace.to_path_buf(), + is_loading: false, + loading_cell: None, + pending_expands: HashMap::new(), + expand_seq: 0, + }; + } let loading_cell = Arc::new(Mutex::new(None)); let cell = loading_cell.clone(); let ws = workspace.to_path_buf(); @@ -75,6 +103,8 @@ impl FileTreeState { workspace: workspace.to_path_buf(), is_loading: true, loading_cell: Some(loading_cell), + pending_expands: HashMap::new(), + expand_seq: 0, } } @@ -103,15 +133,37 @@ impl FileTreeState { } } - /// Rebuild the flat entry list from the current `expanded_dirs` set. - /// When loading is in progress, the rebuild is deferred. - pub fn rebuild(&mut self) { - if self.is_loading { - // Defer rebuild until async load completes + /// Drain any completed background walks (initial build or expands). + /// Returns `true` when new results were applied so the event loop can + /// schedule a repaint — without this, a walk finishing while the loop + /// is idle would leave the expanded directory looking empty until the + /// next unrelated input event (#3900). + pub fn poll_background(&mut self) -> bool { + let was_loading = self.is_loading; + self.poll_loading(); + let finished_loading = was_loading && !self.is_loading; + let pending_before = self.pending_expands.len(); + self.poll_pending_expands(); + finished_loading || self.pending_expands.len() != pending_before + } + + /// Poll for background expand-walk results and splice them in. + /// Call from the render loop, after [`Self::poll_loading`] (#3900). + pub fn poll_pending_expands(&mut self) { + if self.pending_expands.is_empty() { return; } - self.entries = build_file_tree_inner(&self.workspace, &self.expanded_dirs, None); - self.clamp_cursor(); + let mut ready: Vec<(PathBuf, u64, Vec)> = Vec::new(); + for (dir, pending) in &self.pending_expands { + if let Ok(mut guard) = pending.cell.lock() + && let Some(children) = guard.take() + { + ready.push((dir.clone(), pending.seq, children)); + } + } + for (dir, seq, children) in ready { + self.apply_expand_result(&dir, seq, children); + } } /// Move the cursor up by one. @@ -140,11 +192,10 @@ impl FileTreeState { if entry.is_dir { let norm = normalize_path(&entry.path); if self.expanded_dirs.contains(&norm) { - self.expanded_dirs.remove(&norm); + self.collapse_dir_at(self.cursor); } else { - self.expanded_dirs.insert(norm); + self.expand_dir_at(self.cursor); } - self.rebuild(); None } else { // Return the path relative to workspace. @@ -158,6 +209,125 @@ impl FileTreeState { } } + /// Collapse the directory at `idx` by splicing its visible descendants + /// out of the flat entry list — no filesystem I/O at all (#3900). + /// + /// Descendant directories stay in `expanded_dirs` so re-expanding the + /// parent restores their expanded state, matching the previous + /// full-rebuild behavior. + fn collapse_dir_at(&mut self, idx: usize) { + let Some(entry) = self.entries.get_mut(idx) else { + return; + }; + if !entry.is_dir { + return; + } + let depth = entry.depth; + entry.expanded = false; + let norm = normalize_path(&entry.path); + self.expanded_dirs.remove(&norm); + // Drop any in-flight expand walk for this directory; its result must + // not splice into a collapsed node. + self.pending_expands.remove(&norm); + + let end = self.entries[idx + 1..] + .iter() + .position(|e| e.depth <= depth) + .map_or(self.entries.len(), |offset| idx + 1 + offset); + let removed = end - (idx + 1); + self.entries.drain(idx + 1..end); + if self.cursor > idx { + self.cursor = if self.cursor < end { + idx + } else { + self.cursor - removed + }; + } + self.clamp_cursor(); + self.clamp_scroll(); + } + + /// Expand the directory at `idx`. The subtree walk runs on a background + /// thread and is spliced in by [`Self::poll_pending_expands`] (#3900); + /// without a tokio runtime (plain unit tests) it runs synchronously. + /// + /// The entry is marked expanded immediately (▼) so the keypress is + /// acknowledged; children appear when the walk completes. + fn expand_dir_at(&mut self, idx: usize) { + let Some(entry) = self.entries.get_mut(idx) else { + return; + }; + if !entry.is_dir { + return; + } + entry.expanded = true; + let dir = entry.path.clone(); + let norm = normalize_path(&dir); + self.expanded_dirs.insert(norm.clone()); + self.expand_seq = self.expand_seq.wrapping_add(1); + let seq = self.expand_seq; + let ws = self.workspace.clone(); + let expanded_snapshot = self.expanded_dirs.clone(); + + let cell = Arc::new(Mutex::new(None)); + self.pending_expands.insert( + norm.clone(), + PendingExpand { + seq, + cell: cell.clone(), + }, + ); + if tokio::runtime::Handle::try_current().is_ok() { + crate::utils::spawn_blocking_supervised("file-tree-expand", move || { + let children = build_file_tree_inner(&ws, &expanded_snapshot, Some(&dir)); + if let Ok(mut guard) = cell.lock() { + *guard = Some(children); + } + }); + } else { + let children = build_file_tree_inner(&ws, &expanded_snapshot, Some(&dir)); + self.apply_expand_result(&norm, seq, children); + } + } + + /// Splice a completed expand walk into the entry list, unless it has + /// been superseded (newer walk for the same directory), the directory + /// was collapsed while the walk was in flight, or the directory is no + /// longer visible (an ancestor collapsed). + fn apply_expand_result(&mut self, dir: &Path, seq: u64, children: Vec) { + let is_current = self + .pending_expands + .get(dir) + .is_some_and(|pending| pending.seq == seq); + if !is_current { + return; + } + self.pending_expands.remove(dir); + if !self.expanded_dirs.contains(dir) { + return; + } + let Some(idx) = self + .entries + .iter() + .position(|e| e.is_dir && normalize_path(&e.path) == *dir) + else { + return; + }; + let depth = self.entries[idx].depth; + // Defensive: never splice a subtree in twice. + if self.entries.get(idx + 1).is_some_and(|e| e.depth > depth) { + return; + } + self.entries[idx].expanded = true; + let inserted = children.len(); + self.entries.splice(idx + 1..idx + 1, children); + if self.cursor > idx { + self.cursor += inserted; + } + self.clamp_cursor(); + self.clamp_scroll(); + } + /// Ensure the cursor is within bounds. fn clamp_cursor(&mut self) { if !self.entries.is_empty() && self.cursor >= self.entries.len() { @@ -294,6 +464,7 @@ pub fn render_file_tree( mode: palette::PaletteMode, ) { state.poll_loading(); + state.poll_pending_expands(); if area.width < FILE_TREE_MIN_WIDTH || area.height < 3 { return; } @@ -370,3 +541,253 @@ pub fn render_file_tree( f.render_widget(section, area); } + +#[cfg(test)] +mod tests { + use super::*; + use std::time::Duration; + + fn fixture_workspace() -> tempfile::TempDir { + let dir = tempfile::TempDir::new().expect("temp dir"); + let root = dir.path(); + std::fs::create_dir_all(root.join("src/nested")).expect("mkdir src/nested"); + std::fs::create_dir_all(root.join("docs")).expect("mkdir docs"); + std::fs::create_dir_all(root.join("node_modules/pkg")).expect("mkdir node_modules"); + std::fs::write(root.join("README.md"), "readme").expect("write README.md"); + std::fs::write(root.join("src/main.rs"), "fn main() {}").expect("write main.rs"); + std::fs::write(root.join("src/Lib.rs"), "").expect("write Lib.rs"); + std::fs::write(root.join("src/nested/mod.rs"), "").expect("write mod.rs"); + std::fs::write(root.join("docs/guide.md"), "").expect("write guide.md"); + dir + } + + fn index_of(state: &FileTreeState, name: &str) -> usize { + state + .entries + .iter() + .position(|e| e.name == name) + .unwrap_or_else(|| panic!("entry {name} missing: {:?}", entry_names(state))) + } + + fn entry_names(state: &FileTreeState) -> Vec { + state.entries.iter().map(|e| e.name.clone()).collect() + } + + fn expand_by_name(state: &mut FileTreeState, name: &str) { + state.cursor = index_of(state, name); + assert!(state.activate().is_none(), "expanding {name} returns None"); + } + + /// The incremental expand path (splice) must produce exactly the flat + /// list the old full rebuild produced, for several expansion orders. + #[test] + fn incremental_expand_matches_full_rebuild() { + let ws = fixture_workspace(); + for order in [["src", "nested", "docs"], ["docs", "src", "nested"]] { + // Plain test: no tokio runtime, so expand walks run synchronously. + let mut state = FileTreeState::new(ws.path()); + assert!(!state.is_loading, "sync fallback builds immediately"); + for name in order { + expand_by_name(&mut state, name); + } + + let oracle = build_file_tree_inner(ws.path(), &state.expanded_dirs, None); + assert_eq!( + state.entries.len(), + oracle.len(), + "entry count parity for order {order:?}: {:?}", + entry_names(&state) + ); + for (spliced, rebuilt) in state.entries.iter().zip(oracle.iter()) { + assert_eq!(spliced.name, rebuilt.name); + assert_eq!(spliced.path, rebuilt.path); + assert_eq!(spliced.is_dir, rebuilt.is_dir); + assert_eq!(spliced.depth, rebuilt.depth); + assert_eq!(spliced.expanded, rebuilt.expanded); + } + } + } + + #[test] + fn collapse_splices_out_subtree_without_io() { + let ws = fixture_workspace(); + let mut state = FileTreeState::new(ws.path()); + expand_by_name(&mut state, "src"); + expand_by_name(&mut state, "nested"); + assert!(state.entries.iter().any(|e| e.name == "mod.rs")); + + // Collapse src: descendants leave the list, nested stays remembered. + let src_idx = index_of(&state, "src"); + state.cursor = src_idx; + assert!(state.activate().is_none()); + + assert!(!state.entries[src_idx].expanded); + assert!(!state.entries.iter().any(|e| e.name == "main.rs")); + assert!(!state.entries.iter().any(|e| e.name == "mod.rs")); + assert_eq!(state.cursor, src_idx, "cursor stays on the collapsed dir"); + let nested_norm = normalize_path(&ws.path().join("src/nested")); + assert!( + state.expanded_dirs.contains(&nested_norm), + "collapsing a parent keeps descendant expansion state" + ); + } + + #[test] + fn re_expand_restores_descendant_expansion() { + let ws = fixture_workspace(); + let mut state = FileTreeState::new(ws.path()); + expand_by_name(&mut state, "src"); + expand_by_name(&mut state, "nested"); + state.cursor = index_of(&state, "src"); + assert!(state.activate().is_none()); // collapse + assert!(state.activate().is_none()); // re-expand + + let nested_idx = index_of(&state, "nested"); + assert!(state.entries[nested_idx].expanded); + assert!( + state.entries.iter().any(|e| e.name == "mod.rs"), + "re-expanding the parent restores the expanded child subtree: {:?}", + entry_names(&state) + ); + } + + #[test] + fn stale_expand_results_are_discarded() { + let ws = fixture_workspace(); + let mut state = FileTreeState::new(ws.path()); + expand_by_name(&mut state, "src"); + let src_norm = normalize_path(&ws.path().join("src")); + + // Collapse removes the pending walk; a result landing afterwards is + // dropped instead of splicing into the collapsed node. + state.cursor = index_of(&state, "src"); + assert!(state.activate().is_none()); + let ghost = vec![FileTreeEntry { + name: "ghost.rs".to_string(), + path: ws.path().join("src/ghost.rs"), + is_dir: false, + depth: 1, + expanded: false, + }]; + state.apply_expand_result(&src_norm, state.expand_seq, ghost.clone()); + assert!(!state.entries.iter().any(|e| e.name == "ghost.rs")); + + // A superseded sequence number is also dropped, and the newer + // pending walk stays registered. + state.pending_expands.insert( + src_norm.clone(), + PendingExpand { + seq: 7, + cell: Arc::new(Mutex::new(None)), + }, + ); + state.expanded_dirs.insert(src_norm.clone()); + state.apply_expand_result(&src_norm, 6, ghost); + assert!(!state.entries.iter().any(|e| e.name == "ghost.rs")); + assert!( + state.pending_expands.contains_key(&src_norm), + "a stale result must not clear the newer pending walk" + ); + } + + #[test] + fn splice_shifts_cursor_positioned_after_the_expanded_dir() { + let ws = fixture_workspace(); + let mut state = FileTreeState::new(ws.path()); + let src_norm = normalize_path(&ws.path().join("src")); + let src_idx = index_of(&state, "src"); + let readme_idx = index_of(&state, "README.md"); + assert!(readme_idx > src_idx); + + state.expanded_dirs.insert(src_norm.clone()); + state.entries[src_idx].expanded = true; + state.pending_expands.insert( + src_norm.clone(), + PendingExpand { + seq: 1, + cell: Arc::new(Mutex::new(None)), + }, + ); + state.cursor = readme_idx; + let children = build_file_tree_inner( + ws.path(), + &state.expanded_dirs, + Some(&ws.path().join("src")), + ); + let inserted = children.len(); + assert!(inserted > 0); + + state.apply_expand_result(&src_norm, 1, children); + + assert_eq!(state.cursor, readme_idx + inserted); + assert_eq!(state.entries[state.cursor].name, "README.md"); + } + + #[test] + fn poll_background_reports_applied_expand_results() { + let ws = fixture_workspace(); + let mut state = FileTreeState::new(ws.path()); + let src_norm = normalize_path(&ws.path().join("src")); + let src_idx = index_of(&state, "src"); + + state.expanded_dirs.insert(src_norm.clone()); + state.entries[src_idx].expanded = true; + let children = build_file_tree_inner( + ws.path(), + &state.expanded_dirs, + Some(&ws.path().join("src")), + ); + state.pending_expands.insert( + src_norm.clone(), + PendingExpand { + seq: 1, + cell: Arc::new(Mutex::new(Some(children))), + }, + ); + + assert!( + state.poll_background(), + "a drained expand result must request a repaint" + ); + assert!(state.entries.iter().any(|e| e.name == "main.rs")); + assert!( + !state.poll_background(), + "nothing pending: no repaint requested" + ); + } + + #[tokio::test] + async fn async_expand_splices_children_after_poll() { + let ws = fixture_workspace(); + let mut state = FileTreeState::new(ws.path()); + for _ in 0..500 { + state.poll_loading(); + if !state.is_loading { + break; + } + tokio::time::sleep(Duration::from_millis(5)).await; + } + assert!(!state.is_loading, "initial async build completes"); + + state.cursor = index_of(&state, "src"); + assert!(state.activate().is_none()); + assert!( + state.entries[state.cursor].expanded, + "expand acknowledges the keypress immediately" + ); + + for _ in 0..500 { + state.poll_pending_expands(); + if state.entries.iter().any(|e| e.name == "main.rs") { + break; + } + tokio::time::sleep(Duration::from_millis(5)).await; + } + assert!( + state.entries.iter().any(|e| e.name == "main.rs"), + "background walk results are spliced in on poll: {:?}", + entry_names(&state) + ); + assert!(state.pending_expands.is_empty()); + } +} diff --git a/crates/tui/src/tui/history.rs b/crates/tui/src/tui/history.rs index 5e48982960..3099dc1009 100644 --- a/crates/tui/src/tui/history.rs +++ b/crates/tui/src/tui/history.rs @@ -42,7 +42,8 @@ use constants::{ use constants::{TOOL_RUNNING_SYMBOLS, TOOL_STATUS_SYMBOL_MS}; use message::{ RenderedTranscriptLine, assistant_label_style_for, hard_break_copy_lines, message_body_style, - render_message, render_message_with_copy_metadata, render_plain_message, render_user_message, + render_assistant_message, render_assistant_message_with_copy_metadata, render_message, + render_message_with_copy_metadata, render_plain_message, render_user_message, system_body_style, system_label_style, user_body_style, user_label_style, }; use thinking::{render_hidden_thinking_activity, render_thinking}; @@ -184,13 +185,9 @@ impl HistoryCell { pub fn lines(&self, width: u16) -> Vec> { match self { HistoryCell::User { content } => render_user_message(content, width), - HistoryCell::Assistant { content, streaming } => render_message( - ASSISTANT_GLYPH, - assistant_label_style_for(*streaming, /*low_motion*/ false), - message_body_style(), - content, - width, - ), + HistoryCell::Assistant { content, streaming } => { + render_assistant_message(content, width, *streaming, /*low_motion*/ false) + } HistoryCell::System { content } => { if is_cycle_boundary(content) { render_cycle_boundary(content, width) @@ -307,13 +304,9 @@ impl HistoryCell { } HistoryCell::Tool(cell) => cell.lines_with_motion(width, options.low_motion), HistoryCell::User { content } => render_user_message(content, width), - HistoryCell::Assistant { content, streaming } => render_message( - ASSISTANT_GLYPH, - assistant_label_style_for(*streaming, options.low_motion), - message_body_style(), - content, - width, - ), + HistoryCell::Assistant { content, streaming } => { + render_assistant_message(content, width, *streaming, options.low_motion) + } HistoryCell::System { .. } | HistoryCell::Error { .. } => self.lines(width), HistoryCell::SubAgent(cell) => cell.lines(width), HistoryCell::ArchivedContext { .. } => { @@ -341,13 +334,14 @@ impl HistoryCell { HistoryCell::User { content } => { hard_break_copy_lines(render_user_message(content, width)) } - HistoryCell::Assistant { content, streaming } => render_message_with_copy_metadata( - ASSISTANT_GLYPH, - assistant_label_style_for(*streaming, options.low_motion), - message_body_style(), - content, - width, - ), + HistoryCell::Assistant { content, streaming } => { + render_assistant_message_with_copy_metadata( + content, + width, + *streaming, + options.low_motion, + ) + } HistoryCell::System { content } if !is_cycle_boundary(content) => { render_message_with_copy_metadata( "Note", diff --git a/crates/tui/src/tui/history/message.rs b/crates/tui/src/tui/history/message.rs index 5089a33ede..002c29bfff 100644 --- a/crates/tui/src/tui/history/message.rs +++ b/crates/tui/src/tui/history/message.rs @@ -29,12 +29,57 @@ pub(super) fn render_message( .collect() } +/// Render an assistant cell for the live transcript. While the cell is +/// streaming, the markdown parse/wrap goes through the incremental streaming +/// cache so each committed chunk costs O(appended source), not O(whole +/// message) (#3897); the final render after streaming ends takes the plain +/// full-parse path, so a finished message is by construction byte-identical +/// to a full re-parse. +pub(super) fn render_assistant_message( + content: &str, + width: u16, + streaming: bool, + low_motion: bool, +) -> Vec> { + render_assistant_message_with_copy_metadata(content, width, streaming, low_motion) + .into_iter() + .map(|rendered| rendered.line) + .collect() +} + +pub(super) fn render_assistant_message_with_copy_metadata( + content: &str, + width: u16, + streaming: bool, + low_motion: bool, +) -> Vec { + render_message_with_copy_metadata_impl( + ASSISTANT_GLYPH, + assistant_label_style_for(streaming, low_motion), + message_body_style(), + content, + width, + streaming, + ) +} + pub(super) fn render_message_with_copy_metadata( prefix: &str, label_style: Style, body_style: Style, content: &str, width: u16, +) -> Vec { + render_message_with_copy_metadata_impl(prefix, label_style, body_style, content, width, false) +} + +fn render_message_with_copy_metadata_impl( + prefix: &str, + label_style: Style, + body_style: Style, + content: &str, + width: u16, + streaming: bool, ) -> Vec { // An assistant cell whose content is entirely whitespace (e.g. a stray // newline streamed between reasoning and a tool call) would otherwise @@ -49,8 +94,11 @@ pub(super) fn render_message_with_copy_metadata( let prefix_width_u16 = u16::try_from(prefix_width.saturating_add(2)).unwrap_or(u16::MAX); let content_width = usize::from(width.saturating_sub(prefix_width_u16).max(1)); let mut lines = Vec::new(); - let rendered = - markdown_render::render_markdown_tagged(content, content_width as u16, body_style); + let rendered = if streaming { + markdown_render::render_markdown_tagged_streaming(content, content_width as u16, body_style) + } else { + markdown_render::render_markdown_tagged(content, content_width as u16, body_style) + }; for (idx, rendered_line) in rendered.into_iter().enumerate() { let line = if idx == 0 { let mut spans = Vec::new(); diff --git a/crates/tui/src/tui/markdown_render.rs b/crates/tui/src/tui/markdown_render.rs index 916d3d8c7f..4d5f532d47 100644 --- a/crates/tui/src/tui/markdown_render.rs +++ b/crates/tui/src/tui/markdown_render.rs @@ -26,6 +26,7 @@ #[cfg(test)] use std::cell::Cell; +use std::cell::RefCell; use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; @@ -55,6 +56,25 @@ pub fn reset_parse_invocation_count() { PARSE_INVOCATIONS.with(|c| c.set(0)); } +// Thread-local counter of source lines classified by `parse_line_into`. Used +// by tests to prove the streaming path parses O(delta) lines per chunk, not +// the whole growing message (#3897). +#[cfg(test)] +thread_local! { + static PARSED_LINES: Cell = const { Cell::new(0) }; +} + +#[cfg(test)] +#[must_use] +pub fn parsed_line_count() -> u64 { + PARSED_LINES.with(|c| c.get()) +} + +#[cfg(test)] +pub fn reset_parsed_line_count() { + PARSED_LINES.with(|c| c.set(0)); +} + /// One classified line of markdown source, width-independent. /// /// All decisions that depend only on the source text (heading level, bullet @@ -122,67 +142,78 @@ pub fn parse(content: &str) -> ParsedMarkdown { let mut in_code_block = false; for raw_line in content.lines() { - let trimmed = raw_line.trim_start(); - if trimmed.starts_with("```") { - in_code_block = !in_code_block; - continue; - } + parse_line_into(raw_line, &mut in_code_block, &mut blocks); + } - if in_code_block { - blocks.push(Block::Code { - line: raw_line.to_string(), - }); - continue; - } + ParsedMarkdown { blocks } +} - if let Some((level, text)) = parse_heading(trimmed) { - blocks.push(Block::Heading { - level, - text: text.to_string(), - }); - if level == 1 { - blocks.push(Block::HeadingRule); - } - continue; - } +/// Classify one source line into blocks. `in_code_block` is the only +/// cross-line parser state; it is advanced in place. Shared by [`parse`] and +/// the incremental streaming path (#3897), so both classify identically by +/// construction. +fn parse_line_into(raw_line: &str, in_code_block: &mut bool, blocks: &mut Vec) { + #[cfg(test)] + PARSED_LINES.with(|c| c.set(c.get() + 1)); - if let Some((bullet, text)) = parse_list_item(trimmed) { - blocks.push(Block::ListItem { - bullet, - text: text.to_string(), - }); - continue; - } + let trimmed = raw_line.trim_start(); + if trimmed.starts_with("```") { + *in_code_block = !*in_code_block; + return; + } - if is_horizontal_rule(trimmed) { - blocks.push(Block::HorizontalRule); - continue; - } + if *in_code_block { + blocks.push(Block::Code { + line: raw_line.to_string(), + }); + return; + } - match parse_table_row(trimmed) { - Some(cells) => { - blocks.push(Block::TableRow(cells)); - continue; - } - None if trimmed.starts_with('|') => { - blocks.push(Block::TableSeparator); - continue; - } - None => {} + if let Some((level, text)) = parse_heading(trimmed) { + blocks.push(Block::Heading { + level, + text: text.to_string(), + }); + if level == 1 { + blocks.push(Block::HeadingRule); } + return; + } - if trimmed.is_empty() { - // Whitespace-only lines are blank paragraphs. - blocks.push(Block::Blank); - continue; + if let Some((bullet, text)) = parse_list_item(trimmed) { + blocks.push(Block::ListItem { + bullet, + text: text.to_string(), + }); + return; + } + + if is_horizontal_rule(trimmed) { + blocks.push(Block::HorizontalRule); + return; + } + + match parse_table_row(trimmed) { + Some(cells) => { + blocks.push(Block::TableRow(cells)); + return; + } + None if trimmed.starts_with('|') => { + blocks.push(Block::TableSeparator); + return; } + None => {} + } - blocks.push(Block::Paragraph { - text: raw_line.to_string(), - }); + if trimmed.is_empty() { + // Whitespace-only lines are blank paragraphs. + blocks.push(Block::Blank); + return; } - ParsedMarkdown { blocks } + blocks.push(Block::Paragraph { + text: raw_line.to_string(), + }); } /// Render a parsed-markdown AST at the given terminal width. @@ -208,24 +239,48 @@ pub fn render_parsed_tagged( ) -> Vec { let width = width.max(1) as usize; let mut out: Vec = Vec::with_capacity(parsed.blocks.len()); + render_blocks_tagged(&parsed.blocks, width, base_style, &mut out); + + if out.is_empty() { + out.push(empty_rendered_line()); + } + + out +} + +fn empty_rendered_line() -> RenderedMarkdownLine { + RenderedMarkdownLine { + line: Line::from(""), + is_code: false, + copy_prefix_width: 0, + copy_separator_after: CopyLineSeparator::Newline, + } +} +fn is_table_block(block: &Block) -> bool { + matches!(block, Block::TableRow(_) | Block::TableSeparator) +} + +/// Render a run of blocks, grouping consecutive table rows/separators so +/// column widths derive from the whole group. Shared by the full render and +/// the incremental streaming path (#3897), so both produce identical lines +/// by construction. No empty-output fallback here — callers add it over the +/// COMBINED output. +fn render_blocks_tagged( + blocks: &[Block], + width: usize, + base_style: Style, + out: &mut Vec, +) { let mut i = 0; - while i < parsed.blocks.len() { - if matches!( - &parsed.blocks[i], - Block::TableRow(_) | Block::TableSeparator - ) { + while i < blocks.len() { + if is_table_block(&blocks[i]) { let start = i; - while i < parsed.blocks.len() - && matches!( - &parsed.blocks[i], - Block::TableRow(_) | Block::TableSeparator - ) - { + while i < blocks.len() && is_table_block(&blocks[i]) { i += 1; } out.extend( - render_table_group(&parsed.blocks[start..i], width, base_style) + render_table_group(&blocks[start..i], width, base_style) .into_iter() .map(|line| RenderedMarkdownLine { line, @@ -237,84 +292,80 @@ pub fn render_parsed_tagged( continue; } - match &parsed.blocks[i] { - Block::Heading { text, .. } => { - let style = Style::default() - .fg(palette::DEEPSEEK_SKY) - .add_modifier(Modifier::BOLD); - out.extend(render_wrapped_line_tagged(text, width, style, false, false)); - } - Block::HeadingRule => { - out.push(RenderedMarkdownLine { - line: Line::from(Span::styled( - "─".repeat(width.min(40)), - Style::default().fg(palette::TEXT_DIM), - )), - is_code: false, - copy_prefix_width: 0, - copy_separator_after: CopyLineSeparator::Newline, - }); - } - Block::HorizontalRule => { - out.push(RenderedMarkdownLine { - line: Line::from(Span::styled( - "─".repeat(width.min(60)), - Style::default().fg(palette::TEXT_DIM), - )), - is_code: false, - copy_prefix_width: 0, - copy_separator_after: CopyLineSeparator::Newline, - }); - } - Block::ListItem { bullet, text } => { - let bullet_style = Style::default().fg(palette::DEEPSEEK_SKY); - out.extend(render_list_line_tagged( - bullet, - text, - width, - bullet_style, - base_style, - )); - } - Block::Code { line } => { - let code_style = Style::default() - .fg(palette::DEEPSEEK_SKY) - .add_modifier(Modifier::ITALIC); - out.extend(render_wrapped_line_tagged( - line, width, code_style, true, true, - )); - } - Block::Paragraph { text } => { - let link_style = Style::default() - .fg(palette::WHALE_ACCENT_PRIMARY) - .add_modifier(Modifier::UNDERLINED); - out.extend(render_line_with_links_tagged( - text, width, base_style, link_style, - )); - } - Block::Blank => { - out.push(RenderedMarkdownLine { - line: Line::from(""), - is_code: false, - copy_prefix_width: 0, - copy_separator_after: CopyLineSeparator::Newline, - }); - } - Block::TableRow(_) | Block::TableSeparator => unreachable!(), - } + render_block_tagged(&blocks[i], width, base_style, out); i += 1; } +} - if out.is_empty() { - out.push(RenderedMarkdownLine { - line: Line::from(""), - is_code: false, - copy_prefix_width: 0, - copy_separator_after: CopyLineSeparator::Newline, - }); +/// Render one non-table block. +fn render_block_tagged( + block: &Block, + width: usize, + base_style: Style, + out: &mut Vec, +) { + match block { + Block::Heading { text, .. } => { + let style = Style::default() + .fg(palette::DEEPSEEK_SKY) + .add_modifier(Modifier::BOLD); + out.extend(render_wrapped_line_tagged(text, width, style, false, false)); + } + Block::HeadingRule => { + out.push(RenderedMarkdownLine { + line: Line::from(Span::styled( + "─".repeat(width.min(40)), + Style::default().fg(palette::TEXT_DIM), + )), + is_code: false, + copy_prefix_width: 0, + copy_separator_after: CopyLineSeparator::Newline, + }); + } + Block::HorizontalRule => { + out.push(RenderedMarkdownLine { + line: Line::from(Span::styled( + "─".repeat(width.min(60)), + Style::default().fg(palette::TEXT_DIM), + )), + is_code: false, + copy_prefix_width: 0, + copy_separator_after: CopyLineSeparator::Newline, + }); + } + Block::ListItem { bullet, text } => { + let bullet_style = Style::default().fg(palette::DEEPSEEK_SKY); + out.extend(render_list_line_tagged( + bullet, + text, + width, + bullet_style, + base_style, + )); + } + Block::Code { line } => { + let code_style = Style::default() + .fg(palette::DEEPSEEK_SKY) + .add_modifier(Modifier::ITALIC); + out.extend(render_wrapped_line_tagged( + line, width, code_style, true, true, + )); + } + Block::Paragraph { text } => { + let link_style = Style::default() + .fg(palette::WHALE_ACCENT_PRIMARY) + .add_modifier(Modifier::UNDERLINED); + out.extend(render_line_with_links_tagged( + text, width, base_style, link_style, + )); + } + Block::Blank => { + out.push(empty_rendered_line()); + } + Block::TableRow(_) | Block::TableSeparator => { + unreachable!("table blocks are grouped by render_blocks_tagged") + } } - - out } /// Convenience wrapper: parse + render in one call. @@ -339,6 +390,197 @@ pub fn render_markdown_tagged( render_parsed_tagged(&parsed, width, base_style) } +// --------------------------------------------------------------------------- +// Incremental rendering for streaming cells (#3897) +// --------------------------------------------------------------------------- + +/// How many concurrent streaming render states to keep. The main transcript +/// and the Ctrl+T overlay render the same growing message at different +/// widths within one frame, and a streaming thinking body can interleave — +/// a single slot would thrash between them. +const STREAM_RENDER_SLOTS: usize = 4; + +thread_local! { + // Thread-local, like PARSE_INVOCATIONS: the TUI renders on one thread, + // and per-thread state keeps parallel tests isolated. + static STREAM_RENDER_CACHE: RefCell> = const { RefCell::new(Vec::new()) }; +} + +/// Incremental parse+render state for one growing markdown source. +/// +/// Prefix stability is provable for this parser: classification is per line +/// with a single carried bool (`in_code_block`), so appending bytes can never +/// re-classify an earlier COMPLETE line — only the trailing partial line +/// (after the last `\n`) is volatile. Rendering is per block except +/// consecutive table rows, whose column widths derive from the whole group; +/// a trailing table run therefore stays volatile until a non-table block +/// lands after it. +struct StreamRenderState { + /// The committed source prefix (everything up to `cutoff`). Stored so a + /// candidate content can be verified as an extension via one memcmp — + /// orders of magnitude cheaper than the parse+wrap it replaces. + committed_source: String, + /// Rendered-cell width the frozen lines were produced at. + width: usize, + base_style: Style, + /// OSC8 hyperlink support is a process-global flag that changes rendered + /// spans; captured so a flip invalidates the slot. + osc8: bool, + /// Parser fence state after the committed prefix. + in_code_block: bool, + /// Blocks parsed from the committed prefix. + stable_blocks: Vec, + /// Rendered output of `stable_blocks[..frozen_block_count]` — every + /// stable block except a trailing table-group run. + frozen_lines: Vec, + frozen_block_count: usize, +} + +impl StreamRenderState { + fn new(width: usize, base_style: Style, osc8: bool) -> Self { + Self { + committed_source: String::new(), + width, + base_style, + osc8, + in_code_block: false, + stable_blocks: Vec::new(), + frozen_lines: Vec::new(), + frozen_block_count: 0, + } + } + + fn matches(&self, content: &str, width: usize, base_style: Style, osc8: bool) -> bool { + self.width == width + && self.base_style == base_style + && self.osc8 == osc8 + && content.len() >= self.committed_source.len() + && content.as_bytes()[..self.committed_source.len()] + == *self.committed_source.as_bytes() + } + + fn render(&mut self, content: &str) -> Vec { + // Absorb newly completed lines (everything up to the last '\n'). + // `lines()` strips a trailing '\r' itself, so a lone '\r' at the + // very end belongs to the volatile tail along with the partial line. + let cutoff = self.committed_source.len(); + let new_cutoff = content.rfind('\n').map_or(0, |pos| pos + 1); + if new_cutoff > cutoff { + for raw_line in content[cutoff..new_cutoff].lines() { + parse_line_into(raw_line, &mut self.in_code_block, &mut self.stable_blocks); + } + self.committed_source.push_str(&content[cutoff..new_cutoff]); + } + + // Freeze forward: render every stable block except a trailing + // table-group run (its column layout can still change when the tail + // appends more rows). + let mut stable_end = self.stable_blocks.len(); + while stable_end > self.frozen_block_count + && is_table_block(&self.stable_blocks[stable_end - 1]) + { + stable_end -= 1; + } + if stable_end > self.frozen_block_count { + render_blocks_tagged( + &self.stable_blocks[self.frozen_block_count..stable_end], + self.width, + self.base_style, + &mut self.frozen_lines, + ); + self.frozen_block_count = stable_end; + } + + // Volatile region: the stable trailing table run (if any) plus the + // partial last line, re-parsed with a scratch copy of the fence + // state so a "```" sitting in the partial line can't poison the + // committed parser state. + let mut volatile_blocks: Vec = + self.stable_blocks[self.frozen_block_count..].to_vec(); + let tail = &content[self.committed_source.len()..]; + if !tail.is_empty() { + let mut tail_fence_state = self.in_code_block; + for raw_line in tail.lines() { + parse_line_into(raw_line, &mut tail_fence_state, &mut volatile_blocks); + } + } + + let mut out = self.frozen_lines.clone(); + render_blocks_tagged(&volatile_blocks, self.width, self.base_style, &mut out); + if out.is_empty() { + // Match render_parsed_tagged's fallback over the COMBINED output. + out.push(empty_rendered_line()); + } + out + } +} + +/// Incremental variant of [`render_markdown_tagged`] for append-only +/// streaming sources. Per call it parses only the source appended since the +/// last call (plus the volatile trailing partial line / open table group) +/// and re-renders only unfrozen blocks, instead of re-parsing and +/// re-wrapping the whole growing message on every committed chunk (#3897). +/// +/// Output is byte-identical to [`render_markdown_tagged`] on the same +/// input; a non-extension of the cached source (backtrack, different cell) +/// simply misses the cache and rebuilds from scratch. Callers should route +/// the FINAL render of a finished message through [`render_markdown_tagged`]. +#[must_use] +pub fn render_markdown_tagged_streaming( + content: &str, + width: u16, + base_style: Style, +) -> Vec { + let width_cells = width.max(1) as usize; + let osc8_enabled = osc8::enabled(); + let out = STREAM_RENDER_CACHE.with(|cache| { + let mut slots = cache.borrow_mut(); + let mut state = match slots + .iter() + .position(|slot| slot.matches(content, width_cells, base_style, osc8_enabled)) + { + Some(idx) => slots.remove(idx), + None => StreamRenderState::new(width_cells, base_style, osc8_enabled), + }; + let rendered = state.render(content); + // Move-to-front LRU keeps the active stream(s) resident. + slots.insert(0, state); + slots.truncate(STREAM_RENDER_SLOTS); + rendered + }); + + // Differential guard: any future parser/renderer change that breaks + // prefix stability fails loudly in debug builds and under `cargo test`. + #[cfg(debug_assertions)] + { + // Keep the test-only parsed-line counter measuring the streaming + // path alone — the guard's full re-parse is not part of the cost + // under test. + #[cfg(test)] + let parsed_lines_before = parsed_line_count(); + let full = render_markdown_tagged(content, width, base_style); + #[cfg(test)] + PARSED_LINES.with(|c| c.set(parsed_lines_before)); + debug_assert!( + rendered_lines_eq(&out, &full), + "streaming markdown render diverged from the full re-parse", + ); + } + + out +} + +#[cfg(debug_assertions)] +fn rendered_lines_eq(a: &[RenderedMarkdownLine], b: &[RenderedMarkdownLine]) -> bool { + a.len() == b.len() + && a.iter().zip(b.iter()).all(|(x, y)| { + x.line == y.line + && x.is_code == y.is_code + && x.copy_prefix_width == y.copy_prefix_width + && x.copy_separator_after == y.copy_separator_after + }) +} + /// Render plain text: split on newlines, word-wrap each line independently, /// preserve leading whitespace and blank lines. No markdown interpretation. #[must_use] @@ -1925,4 +2167,117 @@ mod tests { // Any output is acceptable (the path is degenerate); assert no panic. let _ = rendered; } + + // ---- #3897: incremental rendering for streaming cells ---- + + fn assert_streaming_matches_full(content: &str, width: u16) { + // The streaming entry point carries its own debug_assert differential + // guard, but assert explicitly here too so a failure names the input. + let streaming = render_markdown_tagged_streaming(content, width, Style::default()); + let full = render_markdown_tagged(content, width, Style::default()); + assert_eq!( + streaming.len(), + full.len(), + "line count diverged at width {width} for {content:?}" + ); + for (idx, (s, f)) in streaming.iter().zip(full.iter()).enumerate() { + assert_eq!(s.line, f.line, "line {idx} diverged for {content:?}"); + assert_eq!(s.is_code, f.is_code, "is_code {idx} diverged"); + assert_eq!(s.copy_prefix_width, f.copy_prefix_width); + assert_eq!(s.copy_separator_after, f.copy_separator_after); + } + } + + const STREAMING_FIXTURE: &str = "# Title\n\nIntro paragraph with a [link](https://example.com) inside.\n\n## Sub-heading\n\n- first bullet\n- second bullet with 中文宽字符 text\n\n```rust\nfn main() {\n println!(\"hi\");\n}\n```\n\n| col a | col b |\n|-------|-------|\n| 1 | 2 |\n| 3 | 4 | 5 |\n\n---\n\nClosing paragraph after the table.\n"; + + #[test] + fn streaming_render_matches_full_render_at_every_chunk() { + for width in [24u16, 40, 80] { + for chunk_bytes in [1usize, 3, 7] { + let mut fed = String::new(); + let mut remaining = STREAMING_FIXTURE; + while !remaining.is_empty() { + let mut take = chunk_bytes.min(remaining.len()); + while !remaining.is_char_boundary(take) { + take += 1; + } + fed.push_str(&remaining[..take]); + remaining = &remaining[take..]; + assert_streaming_matches_full(&fed, width); + } + } + } + } + + #[test] + fn streaming_render_parses_only_the_tail() { + let lines: Vec = (0..120).map(|i| format!("paragraph line {i}\n")).collect(); + let mut fed = String::new(); + + reset_parsed_line_count(); + for line in &lines { + fed.push_str(line); + let _ = render_markdown_tagged_streaming(&fed, 80, Style::default()); + } + let parsed = parsed_line_count(); + let n = lines.len() as u64; + // Each committed line is parsed once by the incremental path (plus + // the volatile tail, which is empty here because every chunk ends in + // '\n'). A full re-parse per chunk would cost N*(N+1)/2 ≈ 7260 lines. + assert!( + parsed <= 2 * n + 4, + "streaming path parsed {parsed} lines for {n} single-line chunks — not O(delta)" + ); + } + + #[test] + fn streaming_slot_falls_back_on_non_extension() { + let first = "# Message A\n\nbody of a\n"; + let _ = render_markdown_tagged_streaming(first, 60, Style::default()); + // Shorter, unrelated content: not an extension of the cached prefix. + assert_streaming_matches_full("totally different", 60); + // Backtrack of the original content (shrunk mid-stream). + assert_streaming_matches_full("# Message A\n", 60); + } + + #[test] + fn streaming_two_widths_interleaved_do_not_thrash() { + let mut fed = String::new(); + for i in 0..20 { + fed.push_str(&format!("interleaved line {i} with some words\n")); + // Main transcript and overlay render the same stream at two + // widths in the same frame; both must stay byte-identical. + assert_streaming_matches_full(&fed, 60); + assert_streaming_matches_full(&fed, 80); + } + } + + #[test] + fn trailing_table_group_stays_volatile() { + let mut fed = String::from("intro\n\n"); + let rows = [ + "| a | b |\n", + "|---|---|\n", + "| 1 | 2 |\n", + "| wide-cell-that-changes-layout | 4 | extra |\n", + ]; + for row in rows { + fed.push_str(row); + // A later row can widen the whole group's column layout; earlier + // rows must re-render exactly as the full parse does. + assert_streaming_matches_full(&fed, 60); + } + fed.push_str("\nafter the table\n"); + assert_streaming_matches_full(&fed, 60); + } + + #[test] + fn unclosed_code_fence_streams_byte_identical() { + let source = "before\n```rust\nfn x() {\n```\nafter\n"; + let mut fed = String::new(); + for ch in source.chars() { + fed.push(ch); + assert_streaming_matches_full(&fed, 40); + } + } } diff --git a/crates/tui/src/tui/sidebar.rs b/crates/tui/src/tui/sidebar.rs index c0a7968552..bbb086d35e 100644 --- a/crates/tui/src/tui/sidebar.rs +++ b/crates/tui/src/tui/sidebar.rs @@ -1221,9 +1221,11 @@ fn render_sidebar_tasks(f: &mut Frame, area: Rect, app: &mut App) { let content_width = area.width.saturating_sub(4) as usize; let usable_rows = area.height.saturating_sub(3) as usize; - let (lines, row_actions) = task_panel_rows(app, content_width.max(1), usable_rows.max(1)); + let row_sets = task_panel_row_sets(app); + let (lines, row_actions) = + task_panel_rows(app, &row_sets, content_width.max(1), usable_rows.max(1)); - let full_texts = task_panel_hover_texts(app, usable_rows.max(1)); + let full_texts = task_panel_hover_texts(app, &row_sets, usable_rows.max(1)); render_sidebar_section(f, area, "Tasks", lines, full_texts, row_actions, app); } @@ -1235,9 +1237,43 @@ struct SidebarToolRow { duration_ms: Option, } +/// Row sets shared by the Tasks panel line renderer and hover-text builder. +/// +/// Computed once per frame in `render_sidebar_tasks` (#3898): both consumers +/// previously recomputed `active_tool_rows` / `reasoning_task_rows` / +/// `background_task_rows` (a clone+sort of `app.task_panel`) independently, +/// doubling the work and risking the two passes disagreeing across an +/// `Instant::elapsed` TTL boundary within the same frame. +struct TaskPanelRowSets { + active: Vec, + reasoning: Vec, + background: Vec, + recent: Vec, +} + +fn task_panel_row_sets(app: &App) -> TaskPanelRowSets { + let explicit_tasks_focus = app.sidebar_focus == SidebarFocus::Tasks; + let active = active_tool_rows(app); + let reasoning = reasoning_task_rows(app); + // Auto/Pinned mode deliberately skips live-tool dedup (passes an empty + // slice), matching the previous per-consumer call sites. + let background = background_task_rows(app, if explicit_tasks_focus { &active } else { &[] }); + let recent = if explicit_tasks_focus { + recent_tool_rows(app, 4) + } else { + Vec::new() + }; + TaskPanelRowSets { + active, + reasoning, + background, + recent, + } +} + #[cfg(test)] fn task_panel_lines(app: &App, content_width: usize, max_rows: usize) -> Vec> { - task_panel_rows(app, content_width, max_rows).0 + task_panel_rows(app, &task_panel_row_sets(app), content_width, max_rows).0 } /// Build the Tasks panel lines together with a parallel per-line click-action @@ -1245,6 +1281,7 @@ fn task_panel_lines(app: &App, content_width: usize, max_rows: usize) -> Vec (Vec>, Vec>) { @@ -1273,26 +1310,19 @@ fn task_panel_rows( ))); } - let active_rows = active_tool_rows(app); + let active_rows = &row_sets.active; if explicit_tasks_focus && !active_rows.is_empty() && lines.len() < max_rows { push_sidebar_label_theme(&mut lines, "Live tools", theme); - push_tool_rows(&mut lines, &active_rows, content_width, max_rows, theme); + push_tool_rows(&mut lines, active_rows, content_width, max_rows, theme); } - let reasoning_rows = reasoning_task_rows(app); + let reasoning_rows = &row_sets.reasoning; if explicit_tasks_focus && !reasoning_rows.is_empty() && lines.len() < max_rows { push_sidebar_label_theme(&mut lines, "Model reasoning", theme); - push_reasoning_rows(&mut lines, &reasoning_rows, content_width, max_rows, theme); + push_reasoning_rows(&mut lines, reasoning_rows, content_width, max_rows, theme); } - let background_rows = background_task_rows( - app, - if explicit_tasks_focus { - &active_rows - } else { - &[] - }, - ); + let background_rows = &row_sets.background; // Lines pushed so far (turn label, Live tools header, live tool rows) // are not clickable — backfill their action slots. actions.resize(lines.len(), None); @@ -1391,10 +1421,10 @@ fn task_panel_rows( } if explicit_tasks_focus && lines.len() < max_rows { - let recent_rows = recent_tool_rows(app, 4); + let recent_rows = &row_sets.recent; if !recent_rows.is_empty() { push_sidebar_label_theme(&mut lines, "Recent tools", theme); - push_tool_rows(&mut lines, &recent_rows, content_width, max_rows, theme); + push_tool_rows(&mut lines, recent_rows, content_width, max_rows, theme); } } @@ -1430,7 +1460,7 @@ fn task_panel_rows( (lines, actions) } -fn task_panel_hover_texts(app: &App, max_rows: usize) -> Vec { +fn task_panel_hover_texts(app: &App, row_sets: &TaskPanelRowSets, max_rows: usize) -> Vec { let mut texts = Vec::with_capacity(max_rows.max(4)); let explicit_tasks_focus = app.sidebar_focus == SidebarFocus::Tasks; @@ -1439,26 +1469,19 @@ fn task_panel_hover_texts(app: &App, max_rows: usize) -> Vec { texts.push(format!("turn {turn_id} ({status})")); } - let active_rows = active_tool_rows(app); + let active_rows = &row_sets.active; if explicit_tasks_focus && !active_rows.is_empty() && texts.len() < max_rows { texts.push("Live tools".to_string()); - push_tool_row_hover_texts(&mut texts, &active_rows, max_rows); + push_tool_row_hover_texts(&mut texts, active_rows, max_rows); } - let reasoning_rows = reasoning_task_rows(app); + let reasoning_rows = &row_sets.reasoning; if explicit_tasks_focus && !reasoning_rows.is_empty() && texts.len() < max_rows { texts.push("Model reasoning".to_string()); - push_reasoning_row_hover_texts(&mut texts, &reasoning_rows, max_rows); + push_reasoning_row_hover_texts(&mut texts, reasoning_rows, max_rows); } - let background_rows = background_task_rows( - app, - if explicit_tasks_focus { - &active_rows - } else { - &[] - }, - ); + let background_rows = &row_sets.background; if !background_rows.is_empty() && texts.len() < max_rows { let running = background_rows .iter() @@ -1510,10 +1533,10 @@ fn task_panel_hover_texts(app: &App, max_rows: usize) -> Vec { } if explicit_tasks_focus && texts.len() < max_rows { - let recent_rows = recent_tool_rows(app, 4); + let recent_rows = &row_sets.recent; if !recent_rows.is_empty() { texts.push("Recent tools".to_string()); - push_tool_row_hover_texts(&mut texts, &recent_rows, max_rows); + push_tool_row_hover_texts(&mut texts, recent_rows, max_rows); } } @@ -2616,28 +2639,36 @@ fn sort_sidebar_agent_rows_as_tree(rows: Vec) -> Vec>, seen: &mut std::collections::HashSet, - out: &mut Vec, + order: &mut Vec, ) { if !seen.insert(idx) { return; } - out.push(rows[idx].clone()); + order.push(idx); if let Some(child_indices) = children.get(&rows[idx].id) { for child_idx in child_indices { - push_tree(*child_idx, rows, children, seen, out); + push_tree(*child_idx, rows, children, seen, order); } } } - let mut out = Vec::with_capacity(rows.len()); + let mut order = Vec::with_capacity(rows.len()); let mut seen = std::collections::HashSet::new(); for idx in roots { - push_tree(idx, &rows, &children, &mut seen, &mut out); + push_tree(idx, &rows, &children, &mut seen, &mut order); } for idx in 0..rows.len() { - push_tree(idx, &rows, &children, &mut seen, &mut out); + push_tree(idx, &rows, &children, &mut seen, &mut order); } - out + + // Materialize by move instead of cloning each row a second time (#3898): + // `seen` guarantees every index lands in `order` exactly once, so each + // slot is taken exactly once and no row is dropped. + let mut slots: Vec> = rows.into_iter().map(Some).collect(); + order + .into_iter() + .map(|idx| slots[idx].take().expect("each row emitted exactly once")) + .collect() } fn subagent_status_text(status: &SubAgentStatus) -> &'static str { @@ -3284,7 +3315,8 @@ mod tests { normalize_activity_text, render_sidebar, sidebar_agent_rows, sidebar_hover_rows, sidebar_work_summary, sort_sidebar_agent_rows_as_tree, subagent_panel_hover_texts, subagent_panel_lines, subagent_panel_rows, task_panel_hover_texts, task_panel_lines, - task_panel_rows, work_panel_empty_hint, work_panel_hover_texts, work_panel_lines, + task_panel_row_sets, task_panel_rows, work_panel_empty_hint, work_panel_hover_texts, + work_panel_lines, }; use crate::config::Config; use crate::palette; @@ -4349,6 +4381,98 @@ mod tests { ); } + #[test] + fn task_panel_row_sets_dedup_background_only_when_tasks_focused() { + let mut app = create_test_app(); + let mut active = ActiveCell::new(); + active.push_tool( + "tool-1", + HistoryCell::Tool(ToolCell::Generic(GenericToolCell { + name: "rlm_search".to_string(), + status: ToolStatus::Running, + input_summary: Some("scanning workspace".to_string()), + output: None, + prompts: None, + spillover_path: None, + output_summary: None, + is_diff: false, + })), + ); + app.active_cell = Some(active); + app.task_panel.push(TaskPanelEntry { + id: "rlm-123".to_string(), + status: "running".to_string(), + prompt_summary: "RLM: scanning workspace".to_string(), + duration_ms: Some(1_000), + kind: TaskPanelEntryKind::Background, + stale: false, + elapsed_since_output_ms: None, + owner_agent_id: None, + owner_agent_name: None, + }); + + app.sidebar_focus = SidebarFocus::Tasks; + let focused = task_panel_row_sets(&app); + assert!( + focused.background.is_empty(), + "Tasks focus dedups background jobs against live tools: {:?}", + focused.background + ); + + app.sidebar_focus = SidebarFocus::Auto; + let auto = task_panel_row_sets(&app); + assert_eq!( + auto.background.len(), + 1, + "Auto mode keeps background jobs even when a live tool matches" + ); + } + + #[test] + fn task_panel_rows_and_hover_share_one_snapshot() { + let mut app = create_test_app(); + app.sidebar_focus = SidebarFocus::Tasks; + app.runtime_turn_id = Some("turn_abcdef123456".to_string()); + app.runtime_turn_status = Some("in_progress".to_string()); + app.turn_counter = 3; + app.task_panel.push(TaskPanelEntry { + id: "job_123".to_string(), + status: "running".to_string(), + prompt_summary: "shell: cargo test --workspace".to_string(), + duration_ms: Some(12_000), + kind: TaskPanelEntryKind::Background, + stale: false, + elapsed_since_output_ms: None, + owner_agent_id: None, + owner_agent_name: None, + }); + + let row_sets = task_panel_row_sets(&app); + let (lines, actions) = task_panel_rows(&app, &row_sets, 80, 12); + let hover = task_panel_hover_texts(&app, &row_sets, 12); + let text = lines_to_text(&lines); + + assert_eq!(lines.len(), actions.len(), "actions align with lines"); + let job_idx = text + .iter() + .position(|line| line.contains("Bash running")) + .unwrap_or_else(|| panic!("bash job row missing: {text:?}")); + assert!( + hover[job_idx].contains("cargo test --workspace"), + "hover text at the job row index carries the full command: {hover:?}" + ); + assert!( + text[0].starts_with("Turn 3"), + "line shows stable turn label: {:?}", + text[0] + ); + assert!( + hover[0].contains("turn_abcdef123456"), + "hover carries the full turn id: {:?}", + hover[0] + ); + } + #[test] fn tasks_panel_collapses_stale_running_tool_rows() { let mut app = create_test_app(); @@ -4655,7 +4779,7 @@ mod tests { owner_agent_name: None, }); - let (lines, actions) = task_panel_rows(&app, 80, 12); + let (lines, actions) = task_panel_rows(&app, &task_panel_row_sets(&app), 80, 12); let text = lines_to_text(&lines); assert_eq!(lines.len(), actions.len()); @@ -4695,7 +4819,7 @@ mod tests { owner_agent_name: None, }); - let (lines, actions) = task_panel_rows(&app, 80, 12); + let (lines, actions) = task_panel_rows(&app, &task_panel_row_sets(&app), 80, 12); let text = lines_to_text(&lines); assert!( @@ -4749,7 +4873,7 @@ mod tests { owner_agent_name: None, }); - let (lines, actions) = task_panel_rows(&app, 96, 16); + let (lines, actions) = task_panel_rows(&app, &task_panel_row_sets(&app), 96, 16); let text = lines_to_text(&lines); assert_eq!(lines.len(), actions.len()); @@ -4816,7 +4940,7 @@ mod tests { owner_agent_name: None, }); - let (lines, actions) = task_panel_rows(&app, 80, 12); + let (lines, actions) = task_panel_rows(&app, &task_panel_row_sets(&app), 80, 12); let text = lines_to_text(&lines); let label_idx = text @@ -4867,7 +4991,7 @@ mod tests { owner_agent_name: None, }); - let (lines, actions) = task_panel_rows(&app, 96, 16); + let (lines, actions) = task_panel_rows(&app, &task_panel_row_sets(&app), 96, 16); let text = lines_to_text(&lines); assert_eq!( lines.len(), @@ -4970,6 +5094,50 @@ mod tests { assert_eq!(actions[agent_idx].as_deref(), Some("/fleet status")); } + #[test] + fn sort_sidebar_agent_rows_as_tree_emits_each_row_exactly_once() { + let agent_row = |id: &str, parent: Option<&str>| SidebarAgentRow { + id: id.to_string(), + parent_run_id: parent.map(str::to_string), + spawn_depth: 1, + name: id.to_string(), + role: "explore".to_string(), + status: "running".to_string(), + objective: None, + git_branch: None, + progress: None, + steps_taken: 1, + duration_ms: None, + }; + // Parent + child + a two-node parent cycle: the cycle has no root, so + // only the orphan sweep reaches it. Every row must still come out + // exactly once (the move-based materialization panics on a double + // take and this pins the drop case too). + let rows = vec![ + agent_row("agent_parent", None), + agent_row("agent_child", Some("agent_parent")), + agent_row("agent_cycle_a", Some("agent_cycle_b")), + agent_row("agent_cycle_b", Some("agent_cycle_a")), + ]; + + let sorted = sort_sidebar_agent_rows_as_tree(rows); + + assert_eq!(sorted.len(), 4, "no rows dropped or duplicated"); + let mut ids: Vec<&str> = sorted.iter().map(|row| row.id.as_str()).collect(); + ids.sort_unstable(); + assert_eq!( + ids, + vec![ + "agent_child", + "agent_cycle_a", + "agent_cycle_b", + "agent_parent" + ] + ); + assert_eq!(sorted[0].id, "agent_parent"); + assert_eq!(sorted[1].id, "agent_child"); + } + #[test] fn subagent_panel_collapses_terminal_non_done_rows() { let summary = SidebarSubagentSummary { @@ -5054,6 +5222,7 @@ mod tests { let sorted = sort_sidebar_agent_rows_as_tree(rows); assert_eq!(sorted[0].id, "agent_parent"); assert_eq!(sorted[1].id, "agent_grandchild"); + assert_eq!(sorted.len(), 2, "tree sort must not drop or duplicate rows"); let summary = SidebarSubagentSummary { cached_total: 2, @@ -5849,7 +6018,7 @@ mod tests { "raw turn UUID must stay out of the compact row: {text:?}" ); - let hover = task_panel_hover_texts(&app, 8); + let hover = task_panel_hover_texts(&app, &task_panel_row_sets(&app), 8); assert!( hover[0].contains("0196f0a3-1111-2222-3333-444455556666"), "full turn UUID must remain available in hover text: {hover:?}" diff --git a/crates/tui/src/tui/transcript.rs b/crates/tui/src/tui/transcript.rs index 68fa259ec2..2c21eb75e2 100644 --- a/crates/tui/src/tui/transcript.rs +++ b/crates/tui/src/tui/transcript.rs @@ -159,7 +159,56 @@ impl TranscriptViewCache { original_index_map: Option<&[usize]>, ) { let total_cells: usize = cell_shards.iter().map(|s| s.len()).sum(); + self.ensure_iter( + total_cells, + cell_shards.iter().flat_map(|shard| shard.iter()), + cell_revisions, + width, + options, + folded_cells, + original_index_map, + ); + } + + /// `ensure_split` over an already-filtered list of borrowed cells. + /// + /// The collapse path substitutes synthetic tool-run summary cells and + /// skips collapsed cells, so it cannot hand over contiguous shard + /// slices. Accepting `&[&HistoryCell]` lets it pass borrows instead of + /// deep-cloning every visible cell into a fresh `Vec` each + /// frame (#3896). + #[allow(clippy::too_many_arguments)] + pub fn ensure_filtered( + &mut self, + cells: &[&HistoryCell], + cell_revisions: &[u64], + width: u16, + options: TranscriptRenderOptions, + folded_cells: &HashSet, + original_index_map: Option<&[usize]>, + ) { + self.ensure_iter( + cells.len(), + cells.iter().copied(), + cell_revisions, + width, + options, + folded_cells, + original_index_map, + ); + } + #[allow(clippy::too_many_arguments)] + fn ensure_iter<'a>( + &mut self, + total_cells: usize, + cells: impl Iterator, + cell_revisions: &[u64], + width: u16, + options: TranscriptRenderOptions, + folded_cells: &HashSet, + original_index_map: Option<&[usize]>, + ) { let layout_changed = self.width != width || self.options != options; let folded_changed = self.folded_cells != *folded_cells; if layout_changed || folded_changed { @@ -183,71 +232,69 @@ impl TranscriptViewCache { let revisions_match = cell_revisions.len() == total_cells; let mut idx: usize = 0; - for shard in cell_shards { - for cell in *shard { - let current_rev = if revisions_match { - cell_revisions[idx] - } else { - // No matching revisions — force a re-render this cycle. - u64::MAX - }; - - // Reuse cached entry if the revision matches AND it's at the - // same index (cells can shift on insert/remove, so we only - // reuse when the index is identical — a stricter invariant - // codex also uses for its active-cell tail). - if let Some(prev) = self.per_cell.get(idx) - && !layout_changed - && prev.revision == current_rev - && revisions_match - { - new_per_cell.push(prev.clone()); - idx += 1; - continue; - } - - any_dirty = true; - first_dirty = Some(first_dirty.map_or(idx, |current| current.min(idx))); - let is_tool_groupable = matches!(cell, HistoryCell::Tool(_)); - let render_width = if is_tool_groupable { - width.saturating_sub(2).max(1) - } else { - width - }; - let original_idx = original_index_map - .map(|m| *m.get(idx).unwrap_or(&idx)) - .unwrap_or(idx); - let folded = folded_cells.contains(&original_idx); - let rendered = cell.lines_with_copy_metadata_folded(render_width, options, folded); - let mut lines = Vec::with_capacity(rendered.len()); - let mut copy_separators = Vec::with_capacity(rendered.len()); - let mut copy_prefix_widths = Vec::with_capacity(rendered.len()); - for rendered_line in rendered { - lines.push(rendered_line.line); - copy_prefix_widths.push(rendered_line.copy_prefix_width); - copy_separators.push(rendered_line.copy_separator_after); - } - let is_empty = lines.is_empty(); - new_per_cell.push(CachedCell { - revision: current_rev, - lines: Arc::new(lines), - copy_separators: Arc::new(copy_separators), - copy_prefix_widths: Arc::new(copy_prefix_widths), - is_empty, - is_stream_continuation: cell.is_stream_continuation(), - is_conversational: cell.is_conversational(), - is_system_or_tool: matches!( - cell, - HistoryCell::System { .. } - | HistoryCell::Error { .. } - | HistoryCell::Tool(_) - | HistoryCell::SubAgent(_) - | HistoryCell::ArchivedContext { .. } - ), - is_tool_groupable, - }); + for cell in cells { + let current_rev = if revisions_match { + cell_revisions[idx] + } else { + // No matching revisions — force a re-render this cycle. + u64::MAX + }; + + // Reuse cached entry if the revision matches AND it's at the + // same index (cells can shift on insert/remove, so we only + // reuse when the index is identical — a stricter invariant + // codex also uses for its active-cell tail). + if let Some(prev) = self.per_cell.get(idx) + && !layout_changed + && prev.revision == current_rev + && revisions_match + { + new_per_cell.push(prev.clone()); idx += 1; + continue; + } + + any_dirty = true; + first_dirty = Some(first_dirty.map_or(idx, |current| current.min(idx))); + let is_tool_groupable = matches!(cell, HistoryCell::Tool(_)); + let render_width = if is_tool_groupable { + width.saturating_sub(2).max(1) + } else { + width + }; + let original_idx = original_index_map + .map(|m| *m.get(idx).unwrap_or(&idx)) + .unwrap_or(idx); + let folded = folded_cells.contains(&original_idx); + let rendered = cell.lines_with_copy_metadata_folded(render_width, options, folded); + let mut lines = Vec::with_capacity(rendered.len()); + let mut copy_separators = Vec::with_capacity(rendered.len()); + let mut copy_prefix_widths = Vec::with_capacity(rendered.len()); + for rendered_line in rendered { + lines.push(rendered_line.line); + copy_prefix_widths.push(rendered_line.copy_prefix_width); + copy_separators.push(rendered_line.copy_separator_after); } + let is_empty = lines.is_empty(); + new_per_cell.push(CachedCell { + revision: current_rev, + lines: Arc::new(lines), + copy_separators: Arc::new(copy_separators), + copy_prefix_widths: Arc::new(copy_prefix_widths), + is_empty, + is_stream_continuation: cell.is_stream_continuation(), + is_conversational: cell.is_conversational(), + is_system_or_tool: matches!( + cell, + HistoryCell::System { .. } + | HistoryCell::Error { .. } + | HistoryCell::Tool(_) + | HistoryCell::SubAgent(_) + | HistoryCell::ArchivedContext { .. } + ), + is_tool_groupable, + }); + idx += 1; } self.per_cell = new_per_cell; @@ -1054,6 +1101,97 @@ mod tests { eprintln!(" ✓ well under 1 MB even for very long sessions"); } + #[test] + fn ensure_filtered_matches_ensure_split_output() { + let cells = vec![ + user_cell("hello"), + assistant_cell("some **markdown** body", false), + exec_tool_cell("cargo test"), + user_cell("again"), + ]; + let revisions = vec![1u64, 2, 3, 4]; + let index_map: Vec = vec![0, 1, 2, 3]; + + let mut split_cache = TranscriptViewCache::new(); + split_cache.ensure_split( + &[&cells], + &revisions, + 40, + TranscriptRenderOptions::default(), + &HashSet::new(), + Some(&index_map), + ); + + let refs: Vec<&HistoryCell> = cells.iter().collect(); + let mut filtered_cache = TranscriptViewCache::new(); + filtered_cache.ensure_filtered( + &refs, + &revisions, + 40, + TranscriptRenderOptions::default(), + &HashSet::new(), + Some(&index_map), + ); + + assert_eq!(plain_lines(&split_cache), plain_lines(&filtered_cache)); + assert_eq!( + split_cache.line_meta().len(), + filtered_cache.line_meta().len() + ); + } + + #[test] + fn ensure_filtered_reuses_unchanged_cells() { + let cells = vec![ + user_cell("hello"), + assistant_cell("streaming", true), + user_cell("again"), + ]; + let mut revisions = vec![1u64, 1, 1]; + let refs: Vec<&HistoryCell> = cells.iter().collect(); + + let mut cache = TranscriptViewCache::new(); + cache.ensure_filtered( + &refs, + &revisions, + 80, + TranscriptRenderOptions::default(), + &HashSet::new(), + None, + ); + let first = plain_lines(&cache); + + cache.ensure_filtered( + &refs, + &revisions, + 80, + TranscriptRenderOptions::default(), + &HashSet::new(), + None, + ); + assert_eq!(first, plain_lines(&cache)); + for (idx, cached) in cache.per_cell.iter().enumerate() { + assert_eq!( + cached.revision, 1, + "cell {idx} must be reused, not re-rendered" + ); + } + + // Bump one revision: only that entry re-renders. + revisions[1] = 2; + cache.ensure_filtered( + &refs, + &revisions, + 80, + TranscriptRenderOptions::default(), + &HashSet::new(), + None, + ); + assert_eq!(cache.per_cell[0].revision, 1); + assert_eq!(cache.per_cell[1].revision, 2); + assert_eq!(cache.per_cell[2].revision, 1); + } + #[test] fn folded_thinking_cache_invalidation() { let long_content = "reasoning line\n".repeat(50); diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index aa4b4124a0..1ed8eab352 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -3430,6 +3430,16 @@ async fn run_event_loop( let allow_workspace_context_refresh = !app.is_loading && !has_running_agents && !app.is_compacting && !app.is_purging; workspace_context::refresh_if_needed(app, now, allow_workspace_context_refresh); + // Drain any completed background @mention completion walk so the + // popup repaints as soon as results are ready (#3899). + crate::tui::file_mention::poll_mention_completion(app); + // Drain completed file-tree walks (initial build / expands) so the + // spliced children repaint without waiting for an input event (#3900). + if let Some(tree) = app.file_tree.as_mut() + && tree.poll_background() + { + app.needs_redraw = true; + } // Draw is gated by the frame-rate limiter (120 FPS cap). When a // redraw is needed but the limiter says we're inside the cooldown @@ -4230,6 +4240,12 @@ async fn run_event_loop( if mention_menu_open && app.mention_menu_selected >= mention_menu_entries.len() { app.mention_menu_selected = mention_menu_entries.len().saturating_sub(1); } + // A background completion walk for the token under the cursor + // with nothing to show yet (#3899): Enter must wait rather than + // submit a half-typed @mention, matching the pre-async behavior + // where the walk finished inside the key event. + let mention_walk_pending = + !mention_menu_open && crate::tui::file_mention::mention_walk_pending(app); // Cancel a pending Esc-Esc prime as soon as any non-Esc key // arrives. Without this the prime would hang around for the @@ -4489,7 +4505,7 @@ async fn run_event_loop( KeyCode::Esc if app.clear_composer_attachment_selection() => { continue; } - KeyCode::Esc if mention_menu_open => { + KeyCode::Esc if mention_menu_open || mention_walk_pending => { app.mention_menu_hidden = true; app.mention_menu_selected = 0; } @@ -4886,6 +4902,22 @@ async fn run_event_loop( } } } + KeyCode::Enter if mention_walk_pending => { + // Completion results for the mention under the cursor + // haven't landed yet — hold the submit so a half-typed + // @mention isn't sent (#3899). The walk finishing sets + // needs_redraw, so the popup opens (or, with no + // matches, a following Enter submits) moments later. + if let Some((_, partial)) = + crate::tui::file_mention::partial_file_mention_at_cursor( + &app.input, + app.cursor_position, + ) + { + app.status_message = Some(format!("Scanning files for @{partial}…")); + } + continue; + } KeyCode::Enter => { // #573: when the user typed a slash-command prefix that // the popup is matching (e.g. `/mo` → `/model`), Enter diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index f7a0c9aa82..8ee3b160f2 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -6,10 +6,14 @@ use crate::config::{ use crate::config_ui::{self, WebConfigSession, WebConfigSessionEvent}; use crate::core::engine::mock_engine_handle; use crate::tui::active_cell::ActiveCell; -use crate::tui::app::{ReasoningEffort, SidebarHoverRow, SidebarHoverSection, ToolDetailRecord}; +use crate::tui::app::{ + MentionCompletionKey, MentionCompletionPending, ReasoningEffort, SidebarHoverRow, + SidebarHoverSection, ToolDetailRecord, +}; use crate::tui::file_mention::{ apply_mention_menu_selection, find_file_mention_completions, partial_file_mention_at_cursor, - try_autocomplete_file_mention, user_request_with_file_mentions, visible_mention_menu_entries, + poll_mention_completion, try_autocomplete_file_mention, user_request_with_file_mentions, + visible_mention_menu_entries, }; use crate::tui::footer_ui::{ active_tool_status_label, footer_auxiliary_spans, footer_balance_spans, footer_cache_spans, @@ -8143,6 +8147,309 @@ fn apply_mention_menu_selection_is_noop_outside_a_mention() { assert_eq!(app.input, "no @ here"); } +// ---- #3899: background @-mention completion walk ---- + +fn mention_key_for( + app: &crate::tui::app::App, + partial: &str, + limit: usize, +) -> MentionCompletionKey { + MentionCompletionKey { + workspace: app.workspace.clone(), + cwd: std::env::current_dir().ok(), + partial: partial.to_string(), + limit, + walk_depth: app.mention_walk_depth, + behavior: app.mention_menu_behavior.clone(), + follow_links: app.workspace_follow_symlinks, + } +} + +#[test] +fn mention_completion_sync_fallback_fills_cache_without_runtime() { + let tmpdir = TempDir::new().expect("tempdir"); + std::fs::create_dir_all(tmpdir.path().join("docs")).unwrap(); + std::fs::write(tmpdir.path().join("docs/alpha.md"), "x").unwrap(); + + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/".to_string(); + app.cursor_position = app.input.chars().count(); + + // Plain test — no tokio runtime, so the walk runs synchronously and the + // result is cached immediately with nothing left in flight. + let entries = visible_mention_menu_entries(&mut app, 6); + assert!(entries.iter().any(|e| e == "docs/alpha.md"), "{entries:?}"); + assert!(app.composer.mention_completion_pending.is_none()); + assert!(app.composer.mention_completion_cache.is_some()); +} + +#[tokio::test] +async fn mention_completion_walk_runs_off_thread_and_promotes() { + let tmpdir = TempDir::new().expect("tempdir"); + std::fs::create_dir_all(tmpdir.path().join("docs")).unwrap(); + std::fs::write(tmpdir.path().join("docs/alpha.md"), "x").unwrap(); + + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/".to_string(); + app.cursor_position = app.input.chars().count(); + + // First call starts a background walk; with a cold cache the stale + // fallback is empty, but the pending walk must be registered. + let first = visible_mention_menu_entries(&mut app, 6); + assert!(first.is_empty(), "cold cache returns the empty fallback"); + assert!(app.composer.mention_completion_pending.is_some()); + + let mut entries = Vec::new(); + for _ in 0..400 { + entries = visible_mention_menu_entries(&mut app, 6); + if !entries.is_empty() { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + assert!( + entries.iter().any(|e| e == "docs/alpha.md"), + "background walk result is promoted for the live key: {entries:?}" + ); + assert!(app.composer.mention_completion_pending.is_none()); +} + +#[tokio::test] +async fn mention_completion_stale_needle_walk_is_replaced() { + let tmpdir = TempDir::new().expect("tempdir"); + std::fs::create_dir_all(tmpdir.path().join("docs")).unwrap(); + std::fs::write(tmpdir.path().join("docs/alpha.md"), "x").unwrap(); + + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/".to_string(); + app.cursor_position = app.input.chars().count(); + + // Seed a completed walk for an older needle. + let stale_key = mention_key_for(&app, "old", 6); + app.composer.mention_completion_pending = Some(MentionCompletionPending { + key: stale_key, + cell: std::sync::Arc::new(std::sync::Mutex::new(Some(vec![ + "stale-entry.md".to_string(), + ]))), + }); + + let entries = visible_mention_menu_entries(&mut app, 6); + // The completed old-needle walk is stored under its true key — never + // promoted as the live needle's result — and because "old" shares no + // token lineage with "docs/", it must not surface as the fallback + // either (Enter/Tab apply the shown list against the live token). + assert!( + entries.is_empty(), + "unrelated-token results must not surface: {entries:?}" + ); + let cached = app + .composer + .mention_completion_cache + .as_ref() + .expect("stale result cached under its own key"); + assert_eq!(cached.key.partial, "old"); + let pending = app + .composer + .mention_completion_pending + .as_ref() + .expect("a fresh walk is spawned for the live key"); + assert_eq!(pending.key.partial, "docs/"); + + // Once the fresh walk lands, the live needle's results replace the + // fallback. + let mut entries = Vec::new(); + for _ in 0..400 { + entries = visible_mention_menu_entries(&mut app, 6); + if entries.iter().any(|e| e == "docs/alpha.md") { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + assert!( + entries.iter().any(|e| e == "docs/alpha.md"), + "fresh walk replaces the stale fallback: {entries:?}" + ); +} + +#[tokio::test] +async fn mention_fallback_limited_to_same_token_lineage() { + let tmpdir = TempDir::new().expect("tempdir"); + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/".to_string(); + app.cursor_position = app.input.chars().count(); + + // Cached results from an earlier keystroke of the SAME token ("doc" is + // a prefix of "docs/") serve as the anti-flicker fallback while the + // fresh walk runs. + app.composer.mention_completion_cache = Some(crate::tui::app::MentionCompletionCache { + key: mention_key_for(&app, "doc", 6), + entries: vec!["docs/alpha.md".to_string()], + }); + let entries = visible_mention_menu_entries(&mut app, 6); + assert_eq!(entries, vec!["docs/alpha.md".to_string()]); + + // Cached results from an unrelated token never surface as fallback. + let mut other = create_test_app(); + other.workspace = tmpdir.path().to_path_buf(); + other.input = "look at @docs/".to_string(); + other.cursor_position = other.input.chars().count(); + other.composer.mention_completion_cache = Some(crate::tui::app::MentionCompletionCache { + key: mention_key_for(&other, "src/mai", 6), + entries: vec!["src/main.rs".to_string()], + }); + let entries = visible_mention_menu_entries(&mut other, 6); + assert!( + entries.is_empty(), + "results for another token must not be applied against this one: {entries:?}" + ); +} + +#[test] +fn mention_fallback_drops_entries_that_no_longer_match_the_live_needle() { + // Reviewer repro on PR #3902: `@docs/a` shows docs/apple.md, the user + // types `b`, and a fast Enter before the fresh walk lands must NOT be + // able to apply docs/apple.md against `@docs/ab`. + let tmpdir = TempDir::new().expect("tempdir"); + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/ab".to_string(); + app.cursor_position = app.input.chars().count(); + + app.composer.mention_completion_cache = Some(crate::tui::app::MentionCompletionCache { + key: mention_key_for(&app, "docs/a", 6), + entries: vec!["docs/apple.md".to_string(), "docs/abc.md".to_string()], + }); + // An in-flight walk keeps the call on the fallback path in plain tests. + app.composer.mention_completion_pending = Some(MentionCompletionPending { + key: mention_key_for(&app, "docs/ab", 6), + cell: std::sync::Arc::new(std::sync::Mutex::new(None)), + }); + + let entries = visible_mention_menu_entries(&mut app, 6); + assert_eq!( + entries, + vec!["docs/abc.md".to_string()], + "only entries matching the live needle may be shown or applied" + ); +} + +#[test] +fn mention_inflight_walk_is_not_respawned_per_keystroke() { + let tmpdir = TempDir::new().expect("tempdir"); + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/".to_string(); + app.cursor_position = app.input.chars().count(); + + // A still-running walk for an older needle: dropping it cannot stop + // the blocking work, so it must be kept (serialized) rather than + // replaced with a concurrent walk for the new needle. + let old_key = mention_key_for(&app, "old", 6); + app.composer.mention_completion_pending = Some(MentionCompletionPending { + key: old_key, + cell: std::sync::Arc::new(std::sync::Mutex::new(None)), + }); + + let entries = visible_mention_menu_entries(&mut app, 6); + assert!(entries.is_empty()); + let pending = app + .composer + .mention_completion_pending + .as_ref() + .expect("in-flight walk is retained"); + assert_eq!( + pending.key.partial, "old", + "no concurrent walk is spawned while one is in flight" + ); +} + +#[test] +fn mention_walk_pending_only_for_live_token() { + let tmpdir = TempDir::new().expect("tempdir"); + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/".to_string(); + app.cursor_position = app.input.chars().count(); + let key = mention_key_for(&app, "docs/", 6); + app.composer.mention_completion_pending = Some(MentionCompletionPending { + key, + cell: std::sync::Arc::new(std::sync::Mutex::new(None)), + }); + + assert!(crate::tui::file_mention::mention_walk_pending(&app)); + + app.mention_menu_hidden = true; + assert!( + !crate::tui::file_mention::mention_walk_pending(&app), + "Esc-hidden popup must not swallow Enter" + ); + app.mention_menu_hidden = false; + + app.input = "no mention".to_string(); + app.cursor_position = app.input.chars().count(); + assert!( + !crate::tui::file_mention::mention_walk_pending(&app), + "cursor outside a mention must not swallow Enter" + ); +} + +#[test] +fn poll_mention_completion_promotes_once_and_sets_needs_redraw() { + let tmpdir = TempDir::new().expect("tempdir"); + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/".to_string(); + app.cursor_position = app.input.chars().count(); + + let key = mention_key_for(&app, "docs/", 6); + app.composer.mention_completion_pending = Some(MentionCompletionPending { + key, + cell: std::sync::Arc::new(std::sync::Mutex::new(Some(vec![ + "docs/alpha.md".to_string(), + ]))), + }); + + app.needs_redraw = false; + poll_mention_completion(&mut app); + assert!(app.needs_redraw, "a drained result schedules a repaint"); + assert!(app.composer.mention_completion_pending.is_none()); + assert!(app.composer.mention_completion_cache.is_some()); + + // Nothing left to drain: the poller must not busy-repaint. + app.needs_redraw = false; + poll_mention_completion(&mut app); + assert!(!app.needs_redraw); +} + +#[test] +fn poll_mention_completion_clears_pending_when_cursor_leaves_mention() { + let tmpdir = TempDir::new().expect("tempdir"); + let mut app = create_test_app(); + app.workspace = tmpdir.path().to_path_buf(); + app.input = "look at @docs/".to_string(); + app.cursor_position = app.input.chars().count(); + let key = mention_key_for(&app, "docs/", 6); + app.composer.mention_completion_pending = Some(MentionCompletionPending { + key, + cell: std::sync::Arc::new(std::sync::Mutex::new(None)), + }); + + app.input = "no mention anymore".to_string(); + app.cursor_position = app.input.chars().count(); + + app.needs_redraw = false; + poll_mention_completion(&mut app); + assert!( + app.composer.mention_completion_pending.is_none(), + "an orphaned walk must not keep the loop polling forever" + ); + assert!(!app.needs_redraw); +} + #[test] fn apply_mention_menu_selection_with_no_entries_is_noop() { let mut app = create_test_app(); diff --git a/crates/tui/src/tui/widgets/mod.rs b/crates/tui/src/tui/widgets/mod.rs index 946da244b2..59fe902f02 100644 --- a/crates/tui/src/tui/widgets/mod.rs +++ b/crates/tui/src/tui/widgets/mod.rs @@ -185,10 +185,26 @@ impl ChatWidget { None, ); } else { - // Slow path: clone non-collapsed cells into filtered vecs so - // collapsed cells are excluded from rendering. Build the - // filtered→original index mapping. - let mut filtered_cells: Vec = + // Slow path: borrow non-collapsed cells into a filtered ref list + // so collapsed cells are excluded from rendering, and build the + // filtered→original index mapping. Collapsed run starts render a + // synthetic summary cell; those few summaries are materialized + // up front so the ref list can borrow from a stable Vec — + // avoiding the per-frame deep clone of every visible cell that + // this path used to pay (#3896). + let summary_cells: Vec<(usize, HistoryCell)> = tool_runs + .iter() + .filter(|run| collapsed_run_starts.contains(&run.start)) + .map(|run| (run.start, tool_run_summary_cell(run))) + .collect(); + let summary_cell_for = |idx: usize| -> Option<&HistoryCell> { + summary_cells + .iter() + .find(|(start, _)| *start == idx) + .map(|(_, cell)| cell) + }; + + let mut filtered_cells: Vec<&HistoryCell> = Vec::with_capacity(history_len + active_entries.len()); let mut filtered_revs: Vec = Vec::with_capacity(history_len + active_entries.len()); @@ -206,7 +222,7 @@ impl ChatWidget { .iter() .find(|run| run.start == idx && collapsed_run_starts.contains(&idx)) { - filtered_cells.push(tool_run_summary_cell(run)); + filtered_cells.push(summary_cell_for(idx).expect("summary cell materialized")); filtered_revs.push(tool_run_summary_revision( run, &app.history_revisions, @@ -216,7 +232,7 @@ impl ChatWidget { filtered_to_original.push(idx); continue; } - filtered_cells.push(cell.clone()); + filtered_cells.push(cell); filtered_revs.push(app.history_revisions[idx]); filtered_to_original.push(idx); } @@ -234,7 +250,8 @@ impl ChatWidget { if let Some(run) = tool_runs.iter().find(|run| { run.start == original_idx && collapsed_run_starts.contains(&original_idx) }) { - filtered_cells.push(tool_run_summary_cell(run)); + filtered_cells + .push(summary_cell_for(original_idx).expect("summary materialized")); filtered_revs.push(tool_run_summary_revision( run, &app.history_revisions, @@ -244,7 +261,7 @@ impl ChatWidget { filtered_to_original.push(original_idx); continue; } - filtered_cells.push(cell.clone()); + filtered_cells.push(cell); let salt = (i as u64).wrapping_add(1); filtered_revs.push(active_entry_revision(active_rev, salt)); filtered_to_original.push(original_idx); @@ -253,9 +270,8 @@ impl ChatWidget { app.collapsed_cell_map = filtered_to_original; - let shards: [&[HistoryCell]; 1] = [&filtered_cells]; - app.viewport.transcript_cache.ensure_split( - &shards, + app.viewport.transcript_cache.ensure_filtered( + &filtered_cells, &filtered_revs, content_area.width.max(1), render_options, @@ -3566,6 +3582,80 @@ mod tests { assert_eq!(app.collapsed_cell_map, vec![0, 1, 2]); } + #[test] + fn chat_widget_collapse_path_stable_across_frames() { + let mut app = create_test_app(); + app.tool_collapse_mode = ToolCollapseMode::Compact; + app.tool_collapse_threshold = 3; + add_dense_tool_run(&mut app); + app.add_message(HistoryCell::User { + content: "trailing prompt".to_string(), + }); + + let area = Rect { + x: 0, + y: 0, + width: 80, + height: 10, + }; + + let mut first_buf = Buffer::empty(area); + ChatWidget::new(&mut app, area).render(area, &mut first_buf); + let first = buffer_text(&first_buf, area); + let first_map = app.collapsed_cell_map.clone(); + let first_total = app.viewport.last_transcript_total; + + // Second frame without any app mutation: the borrowed filtered path + // must reproduce the identical output and index map. + let mut second_buf = Buffer::empty(area); + ChatWidget::new(&mut app, area).render(area, &mut second_buf); + let second = buffer_text(&second_buf, area); + + assert_eq!(first, second, "collapse path is frame-stable"); + assert_eq!(first_map, app.collapsed_cell_map); + assert_eq!(first_total, app.viewport.last_transcript_total); + assert!(first.contains("Explored 2 files, 1 search"), "{first}"); + assert!(first.contains("trailing prompt"), "{first}"); + } + + #[test] + fn chat_widget_collapses_run_spanning_history_and_active_entries() { + let mut app = create_test_app(); + app.tool_collapse_mode = ToolCollapseMode::Compact; + app.tool_collapse_threshold = 3; + app.add_message(success_tool_cell("read_file")); + app.add_message(success_tool_cell("list_dir")); + let active = app.active_cell.get_or_insert_with(ActiveCell::new); + active.push_untracked(success_tool_cell("web_search")); + app.bump_active_cell_revision(); + + let area = Rect { + x: 0, + y: 0, + width: 80, + height: 8, + }; + let mut buf = Buffer::empty(area); + ChatWidget::new(&mut app, area).render(area, &mut buf); + let rendered = buffer_text(&buf, area); + + assert_eq!(app.collapsed_cell_map, vec![0]); + assert!( + rendered.contains("Explored 2 files, 1 search"), + "run spanning the history/active boundary renders one summary: {rendered}" + ); + + // Mutating the active tail must re-render the summary (its revision + // folds in the covered active entries). + let rev_before = app.active_cell_revision; + app.bump_active_cell_revision(); + assert_ne!(rev_before, app.active_cell_revision); + let mut second_buf = Buffer::empty(area); + ChatWidget::new(&mut app, area).render(area, &mut second_buf); + let second = buffer_text(&second_buf, area); + assert!(second.contains("Explored 2 files, 1 search"), "{second}"); + } + #[test] fn pad_lines_to_bottom_noop_when_already_filled() { let mut lines = vec![Line::from("one"), Line::from("two")];