From 6396bffcd4efa41c8d426337e9755a4716409a8e Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Tue, 28 Apr 2026 17:58:45 -0500 Subject: [PATCH] Make tool details selected-card aware (#153) --- crates/tui/src/tui/app.rs | 61 +++++++++++++++- crates/tui/src/tui/ui.rs | 101 ++++++++++++++++++++++---- crates/tui/src/tui/ui/tests.rs | 113 ++++++++++++++++++++++++++++++ crates/tui/src/tui/widgets/mod.rs | 26 +++++++ 4 files changed, 287 insertions(+), 14 deletions(-) diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index ba68bed8..5ff70aee 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -27,7 +27,7 @@ use crate::tui::approval::ApprovalMode; use crate::tui::clipboard::{ClipboardContent, ClipboardHandler}; use crate::tui::history::{HistoryCell, TranscriptRenderOptions}; use crate::tui::paste_burst::{FlushResult, PasteBurst}; -use crate::tui::scrolling::{MouseScrollState, TranscriptScroll}; +use crate::tui::scrolling::{MouseScrollState, TranscriptLineMeta, TranscriptScroll}; use crate::tui::selection::TranscriptSelection; use crate::tui::streaming::StreamingState; use crate::tui::transcript::TranscriptViewCache; @@ -1282,6 +1282,65 @@ impl App { } } + /// Resolve the tool-detail record for a committed or still-active virtual + /// transcript cell. + #[must_use] + pub fn tool_detail_record_for_cell(&self, index: usize) -> Option<&ToolDetailRecord> { + if let Some(detail) = self.tool_details_by_cell.get(&index) { + return Some(detail); + } + self.active_tool_details + .values() + .find(|detail| self.tool_cells.get(&detail.tool_id).copied() == Some(index)) + } + + /// Whether a virtual transcript cell can open a meaningful Alt+V detail + /// view. + #[must_use] + pub fn cell_has_detail_target(&self, index: usize) -> bool { + self.tool_detail_record_for_cell(index).is_some() + || matches!( + self.cell_at_virtual_index(index), + Some(HistoryCell::Tool(_) | HistoryCell::SubAgent(_)) + ) + } + + /// Pick the detail target for the current viewport. This is used by the + /// transcript highlight and footer hint so they agree with Alt+V. + #[must_use] + pub fn detail_cell_index_for_viewport( + &self, + top: usize, + visible: usize, + line_meta: &[TranscriptLineMeta], + ) -> Option { + let selected_cell = self + .transcript_selection + .ordered_endpoints() + .and_then(|(start, _)| line_meta.get(start.line_index)) + .and_then(TranscriptLineMeta::cell_line) + .map(|(cell_index, _)| cell_index) + .filter(|&idx| self.cell_has_detail_target(idx)); + if selected_cell.is_some() { + return selected_cell; + } + + let start = top.min(line_meta.len().saturating_sub(1)); + let end = start.saturating_add(visible).min(line_meta.len()); + for meta in line_meta.iter().take(end).skip(start) { + let Some((cell_index, _)) = meta.cell_line() else { + continue; + }; + if self.cell_has_detail_target(cell_index) { + return Some(cell_index); + } + } + + (0..self.virtual_cell_count()) + .rev() + .find(|&idx| self.cell_has_detail_target(idx)) + } + /// Mutable variant of [`Self::cell_at_virtual_index`]. Bumps the /// appropriate revision counter (active-cell revision when targeting an /// in-flight entry, history version otherwise). diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 5212448c..de8b3148 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -1426,8 +1426,8 @@ async fn run_event_loop( { continue; } - KeyCode::Char('v') - if key.modifiers.is_empty() + KeyCode::Char('v') | KeyCode::Char('V') + if details_shortcut_modifiers(key.modifiers) && app.input.is_empty() && open_tool_details_pager(app) => { @@ -3980,6 +3980,11 @@ fn render_footer(f: &mut Frame, area: Rect, app: &mut App) { let strip_frame = now_ms / 150; props.working_strip_frame = Some(strip_frame); } + } else if props.state_label == "ready" + && let Some(label) = selected_detail_footer_label(app) + { + props.state_label = label; + props.state_color = palette::TEXT_MUTED; } let widget = FooterWidget::new(props); @@ -4983,20 +4988,12 @@ fn open_thinking_pager(app: &mut App) -> bool { } fn open_tool_details_pager(app: &mut App) -> bool { - let target_cell = if let Some((start, _)) = app.transcript_selection.ordered_endpoints() { - app.transcript_cache - .line_meta() - .get(start.line_index) - .and_then(|meta| meta.cell_line()) - .map(|(cell_index, _)| cell_index) - } else { - app.history.len().checked_sub(1) - }; + let target_cell = detail_target_cell_index(app); let Some(cell_index) = target_cell else { return false; }; - if let Some(detail) = app.tool_details_by_cell.get(&cell_index) { + if let Some(detail) = app.tool_detail_record_for_cell(cell_index) { let input = serde_json::to_string_pretty(&detail.input) .unwrap_or_else(|_| detail.input.to_string()); let output = detail.output.as_deref().map_or( @@ -5020,7 +5017,7 @@ fn open_tool_details_pager(app: &mut App) -> bool { return true; } - let Some(cell) = app.history.get(cell_index) else { + let Some(cell) = app.cell_at_virtual_index(cell_index) else { app.status_message = Some("No details available for the selected line".to_string()); return false; }; @@ -5046,6 +5043,76 @@ fn open_tool_details_pager(app: &mut App) -> bool { true } +fn detail_target_cell_index(app: &App) -> Option { + if let Some((start, _)) = app.transcript_selection.ordered_endpoints() { + return app + .transcript_cache + .line_meta() + .get(start.line_index) + .and_then(|meta| meta.cell_line()) + .map(|(cell_index, _)| cell_index); + } + + app.detail_cell_index_for_viewport( + app.last_transcript_top, + app.last_transcript_visible.max(1), + app.transcript_cache.line_meta(), + ) + .or_else(|| app.history.len().checked_sub(1)) +} + +fn selected_detail_footer_label(app: &App) -> Option { + if app.transcript_selection.is_active() { + return None; + } + let cell_index = app.detail_cell_index_for_viewport( + app.last_transcript_top, + app.last_transcript_visible.max(1), + app.transcript_cache.line_meta(), + )?; + let label = detail_target_label(app, cell_index)?; + Some(format!( + "Alt+V details: {}", + truncate_line_to_width(&label, 34) + )) +} + +fn detail_target_label(app: &App, cell_index: usize) -> Option { + if let Some(detail) = app.tool_detail_record_for_cell(cell_index) { + return Some(detail.tool_name.clone()); + } + let cell = app.cell_at_virtual_index(cell_index)?; + match cell { + HistoryCell::Tool(ToolCell::Exec(exec)) => { + Some(format!("run {}", one_line_summary(&exec.command, 80))) + } + HistoryCell::Tool(ToolCell::Exploring(explore)) => Some(format!( + "workspace {} item{}", + explore.entries.len(), + if explore.entries.len() == 1 { "" } else { "s" } + )), + HistoryCell::Tool(ToolCell::PlanUpdate(_)) => Some("update plan".to_string()), + HistoryCell::Tool(ToolCell::PatchSummary(patch)) => Some(format!("patch {}", patch.path)), + HistoryCell::Tool(ToolCell::Review(review)) => { + let target = one_line_summary(&review.target, 80); + Some(if target.is_empty() { + "review".to_string() + } else { + format!("review {target}") + }) + } + HistoryCell::Tool(ToolCell::DiffPreview(diff)) => Some(format!("diff {}", diff.title)), + HistoryCell::Tool(ToolCell::Mcp(mcp)) => Some(format!("tool {}", mcp.tool)), + HistoryCell::Tool(ToolCell::ViewImage(image)) => { + Some(format!("image {}", image.path.display())) + } + HistoryCell::Tool(ToolCell::WebSearch(search)) => Some(format!("search {}", search.query)), + HistoryCell::Tool(ToolCell::Generic(generic)) => Some(format!("tool {}", generic.name)), + HistoryCell::SubAgent(_) => Some("sub-agent".to_string()), + _ => None, + } +} + fn is_copy_shortcut(key: &KeyEvent) -> bool { let is_c = matches!(key.code, KeyCode::Char('c') | KeyCode::Char('C')); if !is_c { @@ -5059,6 +5126,14 @@ fn is_copy_shortcut(key: &KeyEvent) -> bool { key.modifiers.contains(KeyModifiers::CONTROL) && key.modifiers.contains(KeyModifiers::SHIFT) } +fn details_shortcut_modifiers(modifiers: KeyModifiers) -> bool { + modifiers.is_empty() + || modifiers == KeyModifiers::SHIFT + || (modifiers.contains(KeyModifiers::ALT) + && !modifiers.contains(KeyModifiers::CONTROL) + && !modifiers.contains(KeyModifiers::SUPER)) +} + fn is_paste_shortcut(key: &KeyEvent) -> bool { let is_v = matches!(key.code, KeyCode::Char('v') | KeyCode::Char('V')); if !is_v { diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index 41f60377..ba55697c 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -1053,6 +1053,119 @@ fn jump_to_adjacent_tool_cell_finds_next_and_previous() { )); } +fn first_line_for_cell(app: &App, cell_index: usize) -> usize { + app.transcript_cache + .line_meta() + .iter() + .position(|meta| meta.cell_line().is_some_and(|(idx, _)| idx == cell_index)) + .expect("cell should have rendered line") +} + +#[test] +fn detail_target_prefers_visible_tool_card() { + let mut app = create_test_app(); + app.history = vec![ + HistoryCell::User { + content: "hello".to_string(), + }, + HistoryCell::Tool(ToolCell::Generic(GenericToolCell { + name: "file_search".to_string(), + status: ToolStatus::Success, + input_summary: Some("query: foo".to_string()), + output: Some("done".to_string()), + prompts: None, + })), + HistoryCell::Assistant { + content: "ok".to_string(), + streaming: false, + }, + HistoryCell::Tool(ToolCell::Generic(GenericToolCell { + name: "exec_shell".to_string(), + status: ToolStatus::Success, + input_summary: Some("command: ls".to_string()), + output: Some("...".to_string()), + prompts: None, + })), + ]; + app.tool_details_by_cell.insert( + 1, + ToolDetailRecord { + tool_id: "search-1".to_string(), + tool_name: "file_search".to_string(), + input: serde_json::json!({"query": "foo"}), + output: Some("done".to_string()), + }, + ); + app.tool_details_by_cell.insert( + 3, + ToolDetailRecord { + tool_id: "exec-1".to_string(), + tool_name: "exec_shell".to_string(), + input: serde_json::json!({"command": "ls"}), + output: Some("...".to_string()), + }, + ); + app.resync_history_revisions(); + let revisions = app.history_revisions.clone(); + app.transcript_cache.ensure( + &app.history, + &revisions, + 100, + app.transcript_render_options(), + ); + app.last_transcript_top = first_line_for_cell(&app, 1); + app.last_transcript_visible = 6; + + assert_eq!(detail_target_cell_index(&app), Some(1)); + assert_eq!( + selected_detail_footer_label(&app).as_deref(), + Some("Alt+V details: file_search") + ); +} + +#[test] +fn open_tool_details_pager_supports_active_virtual_tool_cell() { + let mut app = create_test_app(); + handle_tool_call_started( + &mut app, + "active-1", + "exec_shell", + &serde_json::json!({"command": "echo hi"}), + ); + let active_entries = app + .active_cell + .as_ref() + .expect("active cell") + .entries() + .to_vec(); + app.transcript_cache.ensure_split( + &[&app.history, active_entries.as_slice()], + &[1], + 100, + app.transcript_render_options(), + ); + app.last_transcript_top = 0; + app.last_transcript_visible = 4; + + assert_eq!(detail_target_cell_index(&app), Some(0)); + assert!(open_tool_details_pager(&mut app)); + assert_eq!(app.view_stack.top_kind(), Some(ModalKind::Pager)); +} + +#[test] +fn details_shortcut_modifiers_accept_plain_shift_and_alt_only() { + assert!(details_shortcut_modifiers(KeyModifiers::NONE)); + assert!(details_shortcut_modifiers(KeyModifiers::SHIFT)); + assert!(details_shortcut_modifiers(KeyModifiers::ALT)); + assert!(details_shortcut_modifiers( + KeyModifiers::ALT | KeyModifiers::SHIFT + )); + assert!(!details_shortcut_modifiers(KeyModifiers::CONTROL)); + assert!(!details_shortcut_modifiers( + KeyModifiers::ALT | KeyModifiers::CONTROL + )); +} + #[test] fn partial_file_mention_finds_token_under_cursor() { // Cursor in middle of `@docs/de` should be detected as a partial mention. diff --git a/crates/tui/src/tui/widgets/mod.rs b/crates/tui/src/tui/widgets/mod.rs index 7acdedb9..7be2d6b3 100644 --- a/crates/tui/src/tui/widgets/mod.rs +++ b/crates/tui/src/tui/widgets/mod.rs @@ -159,6 +159,9 @@ impl ChatWidget { app.last_transcript_visible = visible_lines; app.last_transcript_total = total_lines; app.last_transcript_padding_top = 0; + let detail_target_cell = (!app.transcript_selection.is_active()) + .then(|| app.detail_cell_index_for_viewport(top, visible_lines, line_meta)) + .flatten(); let end = (top + visible_lines).min(total_lines); let mut lines = if total_lines == 0 { @@ -178,6 +181,10 @@ impl ChatWidget { } } + if let Some(target_cell) = detail_target_cell { + apply_detail_target_highlight(&mut lines, top, target_cell, line_meta); + } + apply_selection(&mut lines, top, app); if app.transcript_scroll.is_at_tail() { @@ -1067,6 +1074,25 @@ fn apply_selection(lines: &mut [Line<'static>], top: usize, app: &App) { } } +fn apply_detail_target_highlight( + lines: &mut [Line<'static>], + top: usize, + target_cell: usize, + line_meta: &[TranscriptLineMeta], +) { + let highlight_bg = Color::Rgb(18, 29, 39); + for (idx, line) in lines.iter_mut().enumerate() { + let line_index = top + idx; + if let Some(TranscriptLineMeta::CellLine { cell_index, .. }) = line_meta.get(line_index) + && *cell_index == target_cell + { + for span in &mut line.spans { + span.style = span.style.bg(highlight_bg); + } + } + } +} + /// Apply a brief background tint to the last user message's visible lines. fn apply_send_flash( lines: &mut [Line<'static>],