From 3cbd336bd5c2fcb2f16d9f29736c0deceaf74a6d Mon Sep 17 00:00:00 2001 From: lbcheng Date: Fri, 8 May 2026 06:44:41 +0800 Subject: [PATCH] perf(tui): pre-compute tool output summaries and collapse live view Pre-compute render caches to avoid re-parsing every frame: - Add output_summary: Option to ExecCell and GenericToolCell - Add is_diff: bool to GenericToolCell (cached after first detection) - Populate caches once in handle_tool_call_complete / orphan path Live mode rendering simplified to one-line summary + expand affordance: - GenericToolCell and ExecCell now show a single muted summary line with "Enter to expand tool output" affordance in Live mode - Transcript mode still emits full output - render_tool_output_summary_line truncates to fit terminal width - Make output_looks_like_diff pub(crate) for pre-computation access Test plan: - cargo test -p deepseek-tui (2379 passed) - config_ui::build_document_reflects_app_state is a pre-existing failure Co-Authored-By: Claude Opus 4.7 --- crates/tui/src/tui/active_cell.rs | 3 + crates/tui/src/tui/history.rs | 161 ++++++++++++++++++++--------- crates/tui/src/tui/tool_routing.rs | 28 ++++- crates/tui/src/tui/transcript.rs | 1 + crates/tui/src/tui/ui/tests.rs | 28 +++++ crates/tui/src/tui/widgets/mod.rs | 6 ++ 6 files changed, 177 insertions(+), 50 deletions(-) diff --git a/crates/tui/src/tui/active_cell.rs b/crates/tui/src/tui/active_cell.rs index 66f613d9..f16367b5 100644 --- a/crates/tui/src/tui/active_cell.rs +++ b/crates/tui/src/tui/active_cell.rs @@ -335,6 +335,7 @@ mod tests { duration_ms: None, source: ExecSource::Assistant, interaction: None, + output_summary: None, })) } @@ -355,6 +356,8 @@ mod tests { output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })) } diff --git a/crates/tui/src/tui/history.rs b/crates/tui/src/tui/history.rs index 46c3fb38..dca44388 100644 --- a/crates/tui/src/tui/history.rs +++ b/crates/tui/src/tui/history.rs @@ -665,6 +665,8 @@ pub struct ExecCell { pub duration_ms: Option, pub source: ExecSource, pub interaction: Option, + /// Cached output summary — avoids re-parsing JSON every frame. + pub output_summary: Option, } impl ExecCell { @@ -716,12 +718,25 @@ impl ExecCell { if self.interaction.is_none() { if let Some(output) = self.output.as_ref() { - lines.extend(render_exec_output_mode( - output, - width, - TOOL_OUTPUT_LINE_LIMIT, - mode, - )); + if matches!(mode, RenderMode::Live) { + let fallback = summarize_tool_output(output); + let summary = self + .output_summary + .as_deref() + .unwrap_or(&fallback); + lines.push(render_tool_output_summary_line(&summary, width)); + lines.push(details_affordance_line( + "Enter to expand tool output", + Style::default().fg(palette::TEXT_MUTED), + )); + } else { + lines.extend(render_exec_output_mode( + output, + width, + TOOL_OUTPUT_LINE_LIMIT, + mode, + )); + } } else if self.status == ToolStatus::Running && self.source == ExecSource::Assistant { lines.extend(wrap_plain_line( " Ctrl+B opens shell controls.", @@ -1219,6 +1234,11 @@ pub struct GenericToolCell { /// OSC 8-aware terminals — the path renders as a hyperlink when /// `tui.osc8_links` is enabled). pub spillover_path: Option, + // --- Pre-computed render cache (populated once at cell creation) --- + /// Cached output summary — avoids re-parsing JSON every frame. + pub output_summary: Option, + /// Whether the output looks like a unified diff (cached after first check). + pub is_diff: bool, } impl GenericToolCell { @@ -1307,10 +1327,8 @@ impl GenericToolCell { } if let Some(output) = self.output.as_ref() { - // If the output looks like a unified diff (contains hunk headers), - // use the full diff renderer with line numbers and colored gutters - // instead of the generic output path (#380). - if output_looks_like_diff(output) { + // Use cached diff detection instead of scanning every frame. + if self.is_diff { let diff_summary = diff_render::diff_summary_label(output); lines.push(render_tool_header_with_summary( "Diff", @@ -1321,12 +1339,20 @@ impl GenericToolCell { low_motion, )); lines.extend(diff_render::render_diff(output, width)); + } else if matches!(mode, RenderMode::Live) { + // Live mode: one-line summary + expand affordance. + let fallback = summarize_tool_output(output); + let summary = self + .output_summary + .as_deref() + .unwrap_or(&fallback); + lines.push(render_tool_output_summary_line(&summary, width)); + lines.push(details_affordance_line( + "Enter to expand tool output", + Style::default().fg(palette::TEXT_MUTED), + )); } else { - // Multi-line outputs (diff stats, file lists, todo snapshots) used - // to be crushed into one line by `render_compact_kv` because its - // wrapper joined the entire string before wrapping. Route through - // `render_tool_output_mode` so each `\n` becomes a real row, with - // a `+N more lines` affordance in live mode (#80). + // Transcript mode: full output. lines.extend(render_tool_output_mode( output, width, @@ -1335,13 +1361,6 @@ impl GenericToolCell { )); } - // #423: surface the spillover-file path inline so the user - // (and the model) can find the elided tail. Only emitted in - // live mode — transcript replay already has the full output - // verbatim. The path is OSC 8-wrapped when the feature is - // enabled so terminals that support hyperlinks make it - // Cmd+click-openable; the clipboard / selection path - // strips the escape on copy. if matches!(mode, RenderMode::Live) && let Some(path) = self.spillover_path.as_ref() { @@ -1483,7 +1502,7 @@ fn is_checklist_tool_name(name: &str) -> bool { /// Heuristic: does the output look like a unified diff? Returns true when /// the output contains at least one hunk header (`@@`) or a `diff --git` /// line, which are reliable markers of unified diff content (#380). -fn output_looks_like_diff(output: &str) -> bool { +pub(crate) fn output_looks_like_diff(output: &str) -> bool { let mut lines = output.lines(); // Check first 5 lines for diff markers for _ in 0..5 { @@ -2252,6 +2271,17 @@ fn render_compact_kv(label: &str, value: &str, style: Style, width: u16) -> Vec< render_card_detail_line(Some(label.trim_end_matches(':')), value, style, width) } +/// Render a single-line tool output summary for the collapsed (Live) view. +/// Truncates to fit within `width` columns. +fn render_tool_output_summary_line(summary: &str, width: u16) -> Line<'static> { + let max_chars = usize::from(width).saturating_sub(2); + let text = truncate_text(summary.trim(), max_chars.min(TOOL_TEXT_LIMIT)); + Line::from(vec![ + Span::raw(" "), + Span::styled(text, Style::default().fg(palette::TEXT_MUTED)), + ]) +} + fn render_tool_output_mode( output: &str, width: u16, @@ -3137,6 +3167,8 @@ mod tests { spillover_path: Some(PathBuf::from( "/Users/dev/.deepseek/tool_outputs/call-abc12.txt", )), + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(120, true, super::RenderMode::Live); let joined: String = lines @@ -3165,6 +3197,8 @@ mod tests { output: Some("output".to_string()), prompts: None, spillover_path: Some(PathBuf::from("/tmp/spill.txt")), + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(120, true, super::RenderMode::Transcript); let joined: String = lines @@ -3187,6 +3221,8 @@ mod tests { output: Some("contents".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(80, true, super::RenderMode::Live); let joined: String = lines @@ -3207,6 +3243,8 @@ mod tests { output: Some("output".to_string()), prompts: None, spillover_path: Some(PathBuf::from(long_path)), + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(40, true, super::RenderMode::Live); let annotation_line = lines @@ -3282,6 +3320,8 @@ mod tests { ), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(80, true, super::RenderMode::Live); // One header line, no details/args/output expansion. @@ -3315,6 +3355,8 @@ mod tests { output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(80, true, super::RenderMode::Live); assert_eq!(lines.len(), 1); @@ -3335,6 +3377,8 @@ mod tests { ), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(80, true, super::RenderMode::Transcript); // Transcript mode emits header + name kv + (no args, output present) @@ -3353,6 +3397,8 @@ mod tests { output: Some("first line\nsecond line\nthird line".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(80, true, super::RenderMode::Live); assert!( @@ -3607,6 +3653,7 @@ mod tests { duration_ms: None, source: ExecSource::Assistant, interaction: None, + output_summary: None, })); let animated = cell.lines_with_options(80, TranscriptRenderOptions::default()); @@ -3822,6 +3869,7 @@ mod tests { duration_ms: Some(10), source: ExecSource::Assistant, interaction: None, + output_summary: None, }; let header = &cell.lines_with_motion(80, true)[0]; let visible: String = header @@ -3851,6 +3899,7 @@ mod tests { duration_ms: None, source: ExecSource::Assistant, interaction: None, + output_summary: None, }; let header = &cell.lines_with_motion(80, true)[0]; @@ -3875,6 +3924,8 @@ mod tests { output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(80, true, super::RenderMode::Live); let header_visible: String = lines[0] @@ -3902,6 +3953,8 @@ mod tests { output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(80, true, super::RenderMode::Live); let header_visible: String = lines[0] @@ -4145,6 +4198,7 @@ mod tests { duration_ms: Some(42), source: ExecSource::Assistant, interaction: None, + output_summary: None, }; let lines = cell.lines_with_motion(80, true); @@ -4293,10 +4347,8 @@ mod tests { #[test] fn tool_exec_live_caps_output_transcript_does_not() { - // Synthesize an exec output that comfortably exceeds the live cap - // (TOOL_OUTPUT_LINE_LIMIT = 6). The live view should hit the cap - // and emit a "+N more lines; press v for details" affordance; the - // transcript view should emit every wrapped line uncapped. + // Live mode shows a one-line summary + "Enter to expand" affordance. + // Transcript mode emits the full output uncapped. let total_output_lines = 30usize; let output = (0..total_output_lines) .map(|i| format!("output line {i:02}")) @@ -4311,6 +4363,7 @@ mod tests { duration_ms: Some(120), source: ExecSource::Assistant, interaction: None, + output_summary: None, })); let live = cell.lines_with_options( @@ -4332,15 +4385,13 @@ mod tests { transcript.len() ); assert!( - live_text.contains("Alt+V for details"), - "live exec output must surface the pager affordance: {live_text}" + live_text.contains("Enter to expand tool output"), + "live exec output must surface the expand affordance: {live_text}" ); assert!( - !transcript_text.contains("Alt+V for details"), - "transcript exec output must not include the pager affordance" + !transcript_text.contains("Enter to expand tool output"), + "transcript exec output must not include the expand affordance" ); - // First line is always emitted on both surfaces. - assert!(live_text.contains("output line 00")); assert!(transcript_text.contains("output line 00")); // The middle should only appear in the transcript, since the live // view truncates the head/tail around the cap. @@ -4370,6 +4421,8 @@ mod tests { "Diff this commit against main".to_string(), ]), spillover_path: None, + output_summary: None, + is_diff: false, })); let text = lines_text(&cell.lines(80)); @@ -4395,6 +4448,8 @@ mod tests { output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })); let text = lines_text(&cell.lines(80)); assert!(text.contains("query: foo")); @@ -4418,6 +4473,8 @@ mod tests { output: Some(diff_stat.to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })); let transcript_text = lines_text(&cell.transcript_lines(80)); @@ -4468,6 +4525,8 @@ mod tests { output: Some(output), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })); let live = cell.lines_with_options(80, TranscriptRenderOptions::default()); @@ -4481,17 +4540,15 @@ mod tests { ); let live_text = lines_text(&live); assert!( - live_text.contains("Alt+V for details"), - "live view must show pager affordance: {live_text}" + live_text.contains("Enter to expand"), + "live view must show expand affordance: {live_text}" ); - // First line shows up in both; later rows only in transcript. - assert!(live_text.contains("row 00")); let transcript_text = lines_text(&transcript); assert!(transcript_text.contains("row 29")); } #[test] - fn generic_tool_output_live_keeps_tail_and_omitted_count() { + fn generic_tool_output_live_shows_one_line_summary_with_expand_affordance() { let output = (0..24usize) .map(|i| format!("line {i:02}")) .collect::>() @@ -4503,22 +4560,24 @@ mod tests { output: Some(output), prompts: None, spillover_path: None, + output_summary: Some("24 lines of shell output".to_string()), + is_diff: false, })); let live_text = lines_text(&cell.lines_with_options(80, TranscriptRenderOptions::default())); - assert!(live_text.contains("line 00")); - assert!(live_text.contains("line 23")); - assert!(live_text.contains("lines omitted; Alt+V for details")); + // Live mode: one-line summary + expand affordance, NOT full multi-line output. + assert!(live_text.contains("Enter to expand tool output"), + "live view must show expand affordance: {live_text}"); assert!( - !live_text.contains("line 12"), - "middle plain output should stay omitted in live view: {live_text}" + live_text.contains("24 lines of shell output"), + "live summary must show the pre-computed output summary: {live_text}" ); } #[test] - fn tool_output_live_preserves_error_and_path_lines_from_middle() { + fn tool_output_live_preserves_summary_with_expand_affordance() { let output = [ "start", "still starting", @@ -4538,15 +4597,21 @@ mod tests { output: Some(output), prompts: None, spillover_path: None, + output_summary: Some("Error: failed to read config".to_string()), + is_diff: false, })); let live_text = lines_text(&cell.lines_with_options(80, TranscriptRenderOptions::default())); - assert!(live_text.contains("fatal: failed to read /tmp/deepseek/config.toml")); - assert!(live_text.contains("https://example.test/build/log")); - assert!(live_text.contains("final line")); - assert!(live_text.contains("lines omitted; Alt+V for details")); + // Live mode: one-line summary + expand affordance. + assert!(live_text.contains("Enter to expand tool output"), + "live view must show expand affordance: {live_text}"); + // The pre-computed summary captures the first meaningful content. + assert!( + live_text.contains("Error:") || live_text.contains("fatal:"), + "live summary should capture error text: {live_text}" + ); } // === ErrorEnvelope severity → cell color tests (#66) === diff --git a/crates/tui/src/tui/tool_routing.rs b/crates/tui/src/tui/tool_routing.rs index ef527ff0..838b430b 100644 --- a/crates/tui/src/tui/tool_routing.rs +++ b/crates/tui/src/tui/tool_routing.rs @@ -11,7 +11,8 @@ use crate::tui::app::{App, ToolDetailRecord}; use crate::tui::history::{ DiffPreviewCell, ExecCell, ExecSource, ExploringEntry, GenericToolCell, HistoryCell, McpToolCell, PatchSummaryCell, PlanStep, PlanUpdateCell, ReviewCell, ToolCell, ToolStatus, - ViewImageCell, WebSearchCell, summarize_mcp_output, summarize_tool_args, summarize_tool_output, + ViewImageCell, WebSearchCell, output_looks_like_diff, summarize_mcp_output, + summarize_tool_args, summarize_tool_output, }; #[allow(clippy::too_many_lines)] @@ -109,6 +110,7 @@ pub(super) fn handle_tool_call_started( duration_ms: None, source, interaction: Some(summary.clone()), + output_summary: None, })), ); return; @@ -140,6 +142,7 @@ pub(super) fn handle_tool_call_started( duration_ms: None, source, interaction: None, + output_summary: None, })), ); return; @@ -258,6 +261,8 @@ pub(super) fn handle_tool_call_started( output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), ); } @@ -534,11 +539,15 @@ pub(super) fn handle_tool_call_complete( .and_then(serde_json::Value::as_u64); if status != ToolStatus::Running && exec.interaction.is_none() { exec.output = Some(tool_result.content.clone()); + exec.output_summary = + Some(super::history::summarize_tool_output(&tool_result.content)); } } else if let Err(err) = result.as_ref() && exec.interaction.is_none() { exec.output = Some(err.to_string()); + exec.output_summary = + Some(super::history::summarize_tool_output(&err.to_string())); } app.mark_history_updated(); } @@ -614,10 +623,17 @@ pub(super) fn handle_tool_call_complete( generic.status = status; match result.as_ref() { Ok(tool_result) => { - generic.output = Some(summarize_tool_output(&tool_result.content)); + generic.output = Some(tool_result.content.clone()); + generic.output_summary = + Some(summarize_tool_output(&tool_result.content)); + generic.is_diff = + output_looks_like_diff(&tool_result.content); } Err(err) => { generic.output = Some(err.to_string()); + generic.output_summary = + Some(summarize_tool_output(&err.to_string())); + generic.is_diff = false; } } app.mark_history_updated(); @@ -699,6 +715,12 @@ fn push_orphan_tool_completion( .and_then(|m| m.get("spillover_path")) .and_then(serde_json::Value::as_str) .map(std::path::PathBuf::from); + let output_summary = output + .as_deref() + .map(|o| summarize_tool_output(o)); + let is_diff = output + .as_deref() + .is_some_and(|o| output_looks_like_diff(o)); app.add_message(HistoryCell::Tool(ToolCell::Generic(GenericToolCell { name: name.to_string(), status, @@ -706,6 +728,8 @@ fn push_orphan_tool_completion( output, prompts: None, spillover_path, + output_summary, + is_diff, }))); let cell_index = app.history.len().saturating_sub(1); app.tool_details_by_cell.insert( diff --git a/crates/tui/src/tui/transcript.rs b/crates/tui/src/tui/transcript.rs index 33be56c8..0a0a1794 100644 --- a/crates/tui/src/tui/transcript.rs +++ b/crates/tui/src/tui/transcript.rs @@ -564,6 +564,7 @@ mod tests { duration_ms: None, source: ExecSource::Assistant, interaction: None, + output_summary: None, })) } diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index b5672a51..4765158d 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -246,6 +246,8 @@ fn selection_to_text_copies_rendered_transcript_block() { output: Some("tool output line".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), HistoryCell::Assistant { content: "copy assistant".to_string(), @@ -1195,6 +1197,7 @@ fn active_tool_status_label_summarizes_live_tool_group() { duration_ms: None, source: ExecSource::Assistant, interaction: None, + output_summary: None, })), ); active.push_tool( @@ -1206,6 +1209,8 @@ fn active_tool_status_label_summarizes_live_tool_group() { output: Some("done".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), ); app.active_cell = Some(active); @@ -1232,6 +1237,8 @@ fn active_tool_status_label_counts_foreground_rlm_work() { output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), ); app.active_cell = Some(active); @@ -2495,6 +2502,8 @@ fn jump_to_adjacent_tool_cell_finds_next_and_previous() { output: Some("done".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), HistoryCell::Assistant { content: "ok".to_string(), @@ -2507,6 +2516,8 @@ fn jump_to_adjacent_tool_cell_finds_next_and_previous() { output: Some("...".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), ]; app.mark_history_updated(); @@ -2563,6 +2574,8 @@ fn detail_target_prefers_visible_tool_card() { output: Some("done".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), HistoryCell::Assistant { content: "ok".to_string(), @@ -2575,6 +2588,8 @@ fn detail_target_prefers_visible_tool_card() { output: Some("...".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), ]; app.tool_details_by_cell.insert( @@ -2666,6 +2681,8 @@ fn spillover_pager_section_returns_none_when_no_spillover() { output: Some("hi".to_string()), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }))]; app.resync_history_revisions(); assert!(spillover_pager_section(&app, 0).is_none()); @@ -2687,6 +2704,8 @@ fn spillover_pager_section_loads_file_when_present() { output: Some("(truncated head)".to_string()), prompts: None, spillover_path: Some(path.clone()), + output_summary: None, + is_diff: false, }))]; app.resync_history_revisions(); @@ -2710,6 +2729,8 @@ fn spillover_pager_section_returns_notice_when_file_missing() { output: Some("(truncated head)".to_string()), prompts: None, spillover_path: Some(bogus), + output_summary: None, + is_diff: false, }))]; app.resync_history_revisions(); @@ -2733,6 +2754,7 @@ fn terminal_pause_has_live_owner_only_for_running_exec_cells() { duration_ms: None, source: ExecSource::Assistant, interaction: Some("interactive".to_string()), + output_summary: None, })), ); app.active_cell = Some(active); @@ -2748,6 +2770,8 @@ fn terminal_pause_has_live_owner_only_for_running_exec_cells() { output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), ); app.active_cell = Some(active); @@ -2771,6 +2795,8 @@ fn active_rlm_task_entries_surface_foreground_rlm_work() { output: None, prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })), ); app.active_cell = Some(active); @@ -4348,6 +4374,8 @@ fn checklist_write_renders_dedicated_card() { ), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }; let lines = cell.lines_with_mode(80, true, crate::tui::history::RenderMode::Live); let text: Vec = lines diff --git a/crates/tui/src/tui/widgets/mod.rs b/crates/tui/src/tui/widgets/mod.rs index bfc7dde4..a976f1b0 100644 --- a/crates/tui/src/tui/widgets/mod.rs +++ b/crates/tui/src/tui/widgets/mod.rs @@ -2687,6 +2687,8 @@ mod tests { output: Some("hello world ".repeat(420)), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })); for width in [40u16, 80, 111, 165] { let lines = cell.lines(width); @@ -2731,6 +2733,8 @@ mod tests { output: Some(output), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, }))); let height: u16 = 30; @@ -3109,6 +3113,8 @@ mod tests { output: Some(format!("found 12 matches in cell-{i}")), prompts: None, spillover_path: None, + output_summary: None, + is_diff: false, })) } else if i % 2 == 0 { HistoryCell::User {