From 7539b0a856e6dcd16459b7a8dac6c5e2a7c77a9f Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 10 Nov 2026 08:18:16 +0000 Subject: [PATCH] Extend output-free transcript summaries to more tools (#60555) ## What changed - Add summary labels for MCP and dynamic tools, computer activity, terminal interactions, file edits, images, plan updates, agent calls, and historical tool fallbacks. - Keep tool output out of summaries and give summaries disclosure identities even when their cells have no activity IDs. - Bring focused disclosure headers into view and show `expand` or `collapse` in the footer, including for search-expanded entries. ## Testing Add assertions for tool labels, a transcript snapshot verifying that image-generation output stays hidden, and coverage for expanded and collapsed footer hints. GitOrigin-RevId: 076b8d74c17f26b10aa8221c65b8b38dbb5ad5ea --- codex-rs/tui/src/chatwidget/tool_lifecycle.rs | 2 +- codex-rs/tui/src/history_cell/base.rs | 31 +++++++++++++ .../tui/src/history_cell/computer_activity.rs | 8 ++++ codex-rs/tui/src/history_cell/dynamic.rs | 13 ++++++ .../tui/src/history_cell/dynamic_tests.rs | 3 ++ codex-rs/tui/src/history_cell/exec.rs | 13 ++++++ codex-rs/tui/src/history_cell/mcp.rs | 9 ++++ codex-rs/tui/src/history_cell/mcp_preview.rs | 24 +++++----- codex-rs/tui/src/history_cell/mcp_tests.rs | 4 ++ codex-rs/tui/src/history_cell/patches.rs | 38 +++++++++++++++- .../tui/src/history_cell/patches_tests.rs | 1 + codex-rs/tui/src/history_cell/plans.rs | 4 ++ codex-rs/tui/src/multi_agents.rs | 33 ++++++++++---- codex-rs/tui/src/thread_transcript/tools.rs | 15 ++++++- .../tui/src/thread_transcript/tools_tests.rs | 3 ++ .../tui/src/transcript_view/disclosure.rs | 44 +++++++++++-------- .../tui/src/transcript_view/footer_tests.rs | 2 + codex-rs/tui/src/transcript_view/layout.rs | 6 ++- .../tui/src/transcript_view/search_tests.rs | 9 +++- ..._keeps_controls_without_passive_hints.snap | 6 +-- .../tui/src/transcript_view/turn_details.rs | 13 ++++-- codex-rs/tui/src/transcript_view_tests.rs | 21 +++++++++ 22 files changed, 250 insertions(+), 52 deletions(-) diff --git a/codex-rs/tui/src/chatwidget/tool_lifecycle.rs b/codex-rs/tui/src/chatwidget/tool_lifecycle.rs index 9f13cc572b..04fbbc9d6f 100644 --- a/codex-rs/tui/src/chatwidget/tool_lifecycle.rs +++ b/codex-rs/tui/src/chatwidget/tool_lifecycle.rs @@ -105,7 +105,7 @@ impl ChatWidget { } } - pub(super) fn on_collab_event(&mut self, cell: PlainHistoryCell) { + pub(super) fn on_collab_event(&mut self, cell: impl HistoryCell + 'static) { self.flush_answer_stream_with_separator(); self.add_to_history(cell); self.request_redraw(); diff --git a/codex-rs/tui/src/history_cell/base.rs b/codex-rs/tui/src/history_cell/base.rs index 16d4845188..bccc4112e9 100644 --- a/codex-rs/tui/src/history_cell/base.rs +++ b/codex-rs/tui/src/history_cell/base.rs @@ -2,6 +2,7 @@ //! Wrapped prefixes remain display-only while annotated rows retain their logical source. use super::*; +use itertools::Itertools; #[derive(Debug)] pub(crate) struct PlainHistoryCell { @@ -12,6 +13,16 @@ impl PlainHistoryCell { pub(crate) fn new(lines: Vec>) -> Self { Self { lines } } + + /// Only invocation lines belong in a tool summary. + pub(crate) fn with_tool_header(self, count: usize) -> LabeledToolCell { + let label = self.lines.iter().take(count).join(" · "); + self.with_tool_label(label) + } + + pub(crate) fn with_tool_label(self, label: String) -> LabeledToolCell { + LabeledToolCell { cell: self, label } + } } impl HistoryCell for PlainHistoryCell { @@ -24,6 +35,26 @@ impl HistoryCell for PlainHistoryCell { } } +#[derive(Debug)] +pub(crate) struct LabeledToolCell { + cell: PlainHistoryCell, + label: String, +} + +impl HistoryCell for LabeledToolCell { + fn tool_call_labels(&self) -> Vec { + vec![self.label.clone()] + } + + fn display_lines(&self, width: u16) -> Vec> { + self.cell.display_lines(width) + } + + fn raw_lines(&self) -> Vec> { + self.cell.raw_lines() + } +} + #[derive(Debug)] pub(crate) struct WebHyperlinkHistoryCell { lines: Vec, diff --git a/codex-rs/tui/src/history_cell/computer_activity.rs b/codex-rs/tui/src/history_cell/computer_activity.rs index 7ef3342ecf..fd51d9fb08 100644 --- a/codex-rs/tui/src/history_cell/computer_activity.rs +++ b/codex-rs/tui/src/history_cell/computer_activity.rs @@ -119,6 +119,14 @@ fn error_preview(call: &McpToolCallCell) -> Option<&str> { } impl HistoryCell for ComputerActivityCell { + fn tool_call_labels(&self) -> Vec { + self.group + .calls + .iter() + .flat_map(HistoryCell::tool_call_labels) + .collect() + } + fn append_reasoning(&mut self, cell: Box) -> Result<(), Box> { if self.group.calls.is_empty() { Err(cell) diff --git a/codex-rs/tui/src/history_cell/dynamic.rs b/codex-rs/tui/src/history_cell/dynamic.rs index 1ac5eb7c37..f6ca544516 100644 --- a/codex-rs/tui/src/history_cell/dynamic.rs +++ b/codex-rs/tui/src/history_cell/dynamic.rs @@ -173,6 +173,19 @@ impl DynamicToolCallData { } impl HistoryCell for DynamicToolCallCell { + fn tool_call_labels(&self) -> Vec { + let data = self + .data + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner); + vec![ + data.header() + .to_string() + .trim_start_matches("• ") + .to_owned(), + ] + } + fn has_stable_transcript_height(&self) -> bool { false } diff --git a/codex-rs/tui/src/history_cell/dynamic_tests.rs b/codex-rs/tui/src/history_cell/dynamic_tests.rs index e01278849c..260a8c84a4 100644 --- a/codex-rs/tui/src/history_cell/dynamic_tests.rs +++ b/codex-rs/tui/src/history_cell/dynamic_tests.rs @@ -52,6 +52,9 @@ fn dynamic_status_and_output_match_persisted_presentations() { ); assert_eq!(replayed.len(), 1); let cell = &replayed[0]; + if label == "success" { + assert_eq!(cell.tool_call_labels(), ["Called example.inspect"]); + } for (mode, lines) in [ ("compact", cell.display_lines(/*width*/ 80)), ( diff --git a/codex-rs/tui/src/history_cell/exec.rs b/codex-rs/tui/src/history_cell/exec.rs index 57c2f166c0..e5abcf2db5 100644 --- a/codex-rs/tui/src/history_cell/exec.rs +++ b/codex-rs/tui/src/history_cell/exec.rs @@ -24,6 +24,19 @@ impl UnifiedExecInteractionCell { } impl HistoryCell for UnifiedExecInteractionCell { + fn tool_call_labels(&self) -> Vec { + let action = if self.stdin.is_empty() { + "Waited for terminal" + } else { + "Sent terminal input" + }; + vec![ + self.command_display + .as_ref() + .map_or_else(|| action.to_string(), |cmd| format!("{action} · {cmd}")), + ] + } + fn display_lines(&self, _width: u16) -> Vec> { Vec::new() } diff --git a/codex-rs/tui/src/history_cell/mcp.rs b/codex-rs/tui/src/history_cell/mcp.rs index 9f80e5e5d7..6aa4e712e6 100644 --- a/codex-rs/tui/src/history_cell/mcp.rs +++ b/codex-rs/tui/src/history_cell/mcp.rs @@ -356,6 +356,15 @@ impl McpToolCallCell { } impl HistoryCell for McpToolCallCell { + fn tool_call_labels(&self) -> Vec { + let verb = match self.success() { + Some(true) => "Called", + Some(false) => "Failed", + None => "Calling", + }; + vec![format!("{verb} {}", self.compact_tool_name())] + } + fn activity_ids(&self) -> Vec { vec![format!("mcp:{}", self.call_id)] } diff --git a/codex-rs/tui/src/history_cell/mcp_preview.rs b/codex-rs/tui/src/history_cell/mcp_preview.rs index cffaf2700a..dbdfe85a34 100644 --- a/codex-rs/tui/src/history_cell/mcp_preview.rs +++ b/codex-rs/tui/src/history_cell/mcp_preview.rs @@ -17,6 +17,18 @@ use std::borrow::Cow; use std::collections::VecDeque; impl McpToolCallCell { + pub(super) fn compact_tool_name(&self) -> String { + self.invocation + .arguments + .as_ref() + .filter(|_| self.result_kind() == McpResultKind::NodeRepl) + .and_then(|arguments| arguments.get("title")) + .and_then(serde_json::Value::as_str) + .map(|title| title.split_whitespace().collect::>().join(" ")) + .filter(|title| !title.is_empty()) + .unwrap_or_else(|| format!("{}.{}", self.invocation.server, self.invocation.tool)) + } + pub(super) fn compact_mcp_lines(&self, width: u16) -> Vec { let status = self.success(); let (marker, verb) = match status { @@ -32,23 +44,13 @@ impl McpToolCallCell { "Calling", ), }; - let title = self - .invocation - .arguments - .as_ref() - .filter(|_| self.result_kind() == McpResultKind::NodeRepl) - .and_then(|arguments| arguments.get("title")) - .and_then(serde_json::Value::as_str) - .map(|title| title.split_whitespace().collect::>().join(" ")) - .filter(|title| !title.is_empty()) - .unwrap_or_else(|| format!("{}.{}", self.invocation.server, self.invocation.tool)); let mut lines = vec![clipped_line( Line::from(vec![ marker, " ".into(), verb.bold(), " ".into(), - title.fg(accent_color()), + self.compact_tool_name().fg(accent_color()), ]), width, )]; diff --git a/codex-rs/tui/src/history_cell/mcp_tests.rs b/codex-rs/tui/src/history_cell/mcp_tests.rs index 97d1a5b10a..7bb789edd8 100644 --- a/codex-rs/tui/src/history_cell/mcp_tests.rs +++ b/codex-rs/tui/src/history_cell/mcp_tests.rs @@ -154,6 +154,7 @@ fn mcp_preview_shares_one_limit_across_blocks_and_preserves_transcript() { json!({"type": "text", "text": "third\nfourth\nfifth"}), ])), ); + assert_eq!(cell.tool_call_labels(), ["Called search.lookup"]); let display = cell.display_lines(/*width*/ 80); let transcript = cell.transcript_lines(/*width*/ 80); insta::assert_snapshot!(format!( @@ -541,6 +542,7 @@ fn titled_image_call_keeps_error_and_full_title_when_narrow() { }, /*animations_enabled*/ false, ); + assert_eq!(cell.tool_call_labels(), [format!("Calling {title}")]); assert_eq!( cell.display_lines(/*width*/ 80)[0].to_string(), format!("• {title}") @@ -571,6 +573,8 @@ fn titled_image_call_keeps_error_and_full_title_when_narrow() { .to_string() .contains(title) ); + cell.invocation.arguments = Some(json!({"title": " "})); + assert_eq!(cell.tool_call_labels(), ["Failed node_repl.js"]); } /// Reconstruct only from explicit provenance, checking every displayed source slice on the way. diff --git a/codex-rs/tui/src/history_cell/patches.rs b/codex-rs/tui/src/history_cell/patches.rs index c0258c1a24..ca4313b137 100644 --- a/codex-rs/tui/src/history_cell/patches.rs +++ b/codex-rs/tui/src/history_cell/patches.rs @@ -26,6 +26,20 @@ impl PatchHistoryCell { } impl HistoryCell for PatchHistoryCell { + fn tool_call_labels(&self) -> Vec { + let mut paths = self.changes.keys(); + let label = match (paths.next(), paths.next()) { + (Some(path), None) => format!( + "Edited {}", + path.file_name() + .unwrap_or(path.as_os_str()) + .to_string_lossy() + ), + _ => "Edited files".into(), + }; + vec![label] + } + fn activity_ids(&self) -> Vec { vec![self.activity_id.clone()] } @@ -92,6 +106,10 @@ pub(crate) struct PatchFailureCell { } impl HistoryCell for PatchFailureCell { + fn tool_call_labels(&self) -> Vec { + vec!["Failed to apply patch".into()] + } + fn activity_ids(&self) -> Vec { vec![self.activity_id.clone()] } @@ -178,6 +196,10 @@ impl ViewImageHistoryCell { } impl HistoryCell for ViewImageHistoryCell { + fn tool_call_labels(&self) -> Vec { + vec![format!("Viewed image {}", self.path_label)] + } + fn activity_ids(&self) -> Vec { vec![self.activity_id.clone()] } @@ -259,7 +281,8 @@ pub(crate) fn new_image_generation_call( saved_path: Option, ) -> ImageGenerationHistoryCell { let detail = revised_prompt.unwrap_or(call_id); - let heading = if status == "failed" { + let failed = status == "failed"; + let heading = if failed { vec!["✗ ".red().bold(), "Image generation failed".bold()].into() } else { vec!["• ".dim(), "Generated Image:".bold()].into() @@ -274,17 +297,28 @@ pub(crate) fn new_image_generation_call( ImageGenerationHistoryCell { lines, - saved_path: saved_path.filter(|_| status != "failed"), + failed, + saved_path: saved_path.filter(|_| !failed), } } #[derive(Debug)] pub(crate) struct ImageGenerationHistoryCell { + failed: bool, lines: Vec>, saved_path: Option, } impl HistoryCell for ImageGenerationHistoryCell { + fn tool_call_labels(&self) -> Vec { + let label = if self.failed { + "Image generation failed" + } else { + "Generated image" + }; + vec![label.into()] + } + fn display_lines(&self, _width: u16) -> Vec> { self.lines.clone() } diff --git a/codex-rs/tui/src/history_cell/patches_tests.rs b/codex-rs/tui/src/history_cell/patches_tests.rs index 0ea5b8c04d..fa1d483510 100644 --- a/codex-rs/tui/src/history_cell/patches_tests.rs +++ b/codex-rs/tui/src/history_cell/patches_tests.rs @@ -21,6 +21,7 @@ fn compact_patches_retain_full_changes_and_failure_details() { ]), Path::new("."), ); + assert_eq!(patch.tool_call_labels(), ["Edited files"]); let failure = new_patch_apply_failure("first\nsecond\nthird\nfourth diagnostic".into()); let missing = new_patch_apply_failure(String::new()); let mut snapshots = Vec::new(); diff --git a/codex-rs/tui/src/history_cell/plans.rs b/codex-rs/tui/src/history_cell/plans.rs index 3d4ab4b49a..830fb5d991 100644 --- a/codex-rs/tui/src/history_cell/plans.rs +++ b/codex-rs/tui/src/history_cell/plans.rs @@ -239,6 +239,10 @@ pub(crate) struct PlanUpdateCell { } impl HistoryCell for PlanUpdateCell { + fn tool_call_labels(&self) -> Vec { + vec!["Updated plan".into()] + } + fn activity_ids(&self) -> Vec { vec![self.activity_id.clone()] } diff --git a/codex-rs/tui/src/multi_agents.rs b/codex-rs/tui/src/multi_agents.rs index e7f3d38951..dce8a9685a 100644 --- a/codex-rs/tui/src/multi_agents.rs +++ b/codex-rs/tui/src/multi_agents.rs @@ -4,6 +4,7 @@ //! picker entries, and the fast-switch keyboard shortcuts. Higher-level coordination, such as //! deciding which thread becomes active or when a thread closes, stays in [`crate::app::App`]. +use crate::history_cell::LabeledToolCell; use crate::history_cell::PlainHistoryCell; use crate::render::line_utils::prefix_lines; use crate::style::accent_color; @@ -205,7 +206,7 @@ pub(crate) fn tool_call_history_cell( item: &ThreadItemPayload, cached_spawn_request: Option<&SpawnRequestSummary>, mut agent_metadata: impl FnMut(ThreadId) -> AgentMetadata, -) -> Option { +) -> Option { let ThreadItemPayload::CollabAgentToolCall { tool, status, @@ -282,6 +283,12 @@ pub(crate) fn tool_call_history_cell( .map(|receiver_thread_id| close_end(receiver_thread_id, &mut agent_metadata)) } } + .map(|cell| match (tool, status) { + (CollabAgentTool::ResumeAgent, CollabAgentToolCallStatus::Failed) => { + cell.with_tool_label("Failed to resume agent".into()) + } + _ => cell.with_tool_header(/*count*/ 1), + }) } pub(crate) fn sub_agent_activity_display( @@ -928,16 +935,28 @@ mod tests { #[test] fn collab_resume_interrupted_snapshot() { + use CollabAgentToolCallStatus::Completed; + use CollabAgentToolCallStatus::Failed; + let cell = resume_cell(Completed, CollabAgentStatus::Interrupted); + assert_snapshot!("collab_resume_interrupted", cell_to_text(&cell)); + let missing = resume_cell(Failed, CollabAgentStatus::NotFound); + assert_eq!(missing.tool_call_labels(), ["Failed to resume agent"]); + } + + fn resume_cell( + status: CollabAgentToolCallStatus, + agent_status: CollabAgentStatus, + ) -> LabeledToolCell { let sender_thread_id = ThreadId::from_string("00000000-0000-0000-0000-000000000001") .expect("valid sender thread id"); let robie_id = ThreadId::from_string("00000000-0000-0000-0000-000000000002") .expect("valid robie thread id"); - let cell = tool_call_history_cell( + tool_call_history_cell( &ThreadItemPayload::CollabAgentToolCall { id: "call-resume".to_string(), tool: CollabAgentTool::ResumeAgent, - status: CollabAgentToolCallStatus::Completed, + status, sender_thread_id: sender_thread_id.to_string(), receiver_thread_ids: vec![robie_id.to_string()], prompt: None, @@ -945,15 +964,13 @@ mod tests { reasoning_effort: None, agents_states: HashMap::from([( robie_id.to_string(), - agent_state(CollabAgentStatus::Interrupted, /*message*/ None), + agent_state(agent_status, /*message*/ None), )]), }, /*cached_spawn_request*/ None, |thread_id| metadata_for(thread_id, robie_id, ThreadId::new()), ) - .expect("resume item renders"); - - assert_snapshot!("collab_resume_interrupted", cell_to_text(&cell)); + .expect("resume item renders") } fn agent_state(status: CollabAgentStatus, message: Option<&str>) -> CollabAgentState { @@ -979,7 +996,7 @@ mod tests { } } - fn cell_to_text(cell: &PlainHistoryCell) -> String { + fn cell_to_text(cell: &impl HistoryCell) -> String { cell.display_lines(/*width*/ 200) .iter() .map(line_to_text) diff --git a/codex-rs/tui/src/thread_transcript/tools.rs b/codex-rs/tui/src/thread_transcript/tools.rs index 14fe7b1f02..44c012063e 100644 --- a/codex-rs/tui/src/thread_transcript/tools.rs +++ b/codex-rs/tui/src/thread_transcript/tools.rs @@ -27,7 +27,9 @@ use ratatui::text::Line; /// Preserve status and output when replay cannot reconstruct a rich completed tool cell. /// Declined commands never ran, and historical terminal interactions do not carry stdin details. /// Pending items retain their last known state without starting a running clock. -pub(crate) fn historical_tool_fallback(item: &ThreadItemPayload) -> Option { +pub(crate) fn historical_tool_fallback( + item: &ThreadItemPayload, +) -> Option { let lines = match item { ThreadItemPayload::CommandExecution { command, @@ -108,7 +110,16 @@ pub(crate) fn historical_tool_fallback(item: &ThreadItemPayload) -> Option return None, }; - Some(PlainHistoryCell::new(lines)) + let cell = PlainHistoryCell::new(lines); + Some(match item { + ThreadItemPayload::CommandExecution { + status: CommandExecutionStatus::Declined, + command, + .. + } => cell.with_tool_label(format!("Declined · {command}")), + ThreadItemPayload::CommandExecution { .. } => cell.with_tool_header(/*count*/ 2), + _ => cell.with_tool_header(/*count*/ 1), + }) } // The protocol joins argv with shlex. Keep noncanonical legacy commands verbatim. diff --git a/codex-rs/tui/src/thread_transcript/tools_tests.rs b/codex-rs/tui/src/thread_transcript/tools_tests.rs index c3249b1614..8e9038067d 100644 --- a/codex-rs/tui/src/thread_transcript/tools_tests.rs +++ b/codex-rs/tui/src/thread_transcript/tools_tests.rs @@ -229,6 +229,9 @@ fn historical_command_fallbacks_preserve_status_output_and_group_boundaries() { /*config*/ None, ); assert_eq!(cells.len(), 3); + if label == "declined" { + assert_eq!(cells[1].tool_call_labels(), ["Declined · cargo check"]); + } for cell in [&cells[0], &cells[2]] { assert!( cell.as_any() diff --git a/codex-rs/tui/src/transcript_view/disclosure.rs b/codex-rs/tui/src/transcript_view/disclosure.rs index 2d3713182e..2a41aa571f 100644 --- a/codex-rs/tui/src/transcript_view/disclosure.rs +++ b/codex-rs/tui/src/transcript_view/disclosure.rs @@ -17,7 +17,7 @@ use super::*; pub(super) struct Disclosure { pub(super) expanded: HashSet, - pub(super) focused: Option>, + pub(super) focused: Option<(EntryKey, Vec)>, pub(super) resume_after_approval: Option<(ThreadId, String)>, // Resolve against the next layout, once a toggled live row has also been rebuilt. pub(super) anchor_control: Option, @@ -95,7 +95,7 @@ impl TranscriptView { pub(super) fn activity_ids(&self, cells: &[Arc], index: usize) -> Vec { cells.get(index).map_or_else( || self.disclosure.live_ids.clone(), - |cell| match self.tool_summary(cell.as_ref()) { + |cell| match self.tool_summary(cell.as_ref(), EntryKey::cell(cell)) { Some(row) if !self.is_search_expanded(EntryKey::cell(cell)) => row.ids, _ => cell.activity_ids(), }, @@ -130,7 +130,7 @@ impl TranscriptView { .disclosure .focused .as_ref() - .is_some_and(|focused| ids.iter().any(|id| focused.contains(id))); + .is_some_and(|(_, focused)| ids.iter().any(|id| focused.contains(id))); if !focused || !layout.disclosure { return; } @@ -214,7 +214,7 @@ impl TranscriptView { .disclosure .focused .as_ref() - .and_then(|ids| self.canonical_activity_index(cells, ids)); + .and_then(|(_, ids)| self.canonical_activity_index(cells, ids)); let Some(focused) = focused else { self.disclosure.focused = None; return None; @@ -313,21 +313,24 @@ impl TranscriptView { } fn focus_activity(&mut self, cells: &[Arc], index: usize) -> bool { - if self - .current_layout(cells, index) - .is_none_or(|layout| !layout.disclosure) - { + let Some(layout) = self.layout(cells, index) else { + return false; + }; + if !layout.disclosure { return false; } let ids = self.activity_ids(cells, index); - let header_visible = self.visible.iter().any(|row| { - row.layout.disclosure - && row.activity_ids.iter().any(|id| ids.contains(id)) - && row.row == usize::from(row.layout.separated) + let row = usize::from(layout.separated); + let visible = self.visible.iter().any(|visible| { + visible.row == row && visible.activity_ids.iter().any(|id| ids.contains(id)) }); - self.disclosure.focused = Some(ids); - if !header_visible { + self.disclosure.focused = Some((self.entry_key(cells, index), ids)); + if !visible { self.jump_to_entry(cells, index); + if let Position::Reading(anchor) = &mut self.position { + anchor.offset = layout.position_at(row, /*column*/ 0); + anchor.row_bias = layout.row_for_offset(anchor.offset) as isize - row as isize; + } } true } @@ -389,7 +392,7 @@ impl TranscriptView { } else { self.disclosure.expanded.extend(ids.iter().cloned()); } - self.disclosure.focused = Some(ids); + self.disclosure.focused = Some((key, ids)); self.position = Position::Reading(anchor); self.cache.clear(); self.live_key = None; @@ -400,14 +403,17 @@ impl TranscriptView { } pub(super) fn disclosure_footer(&self, width: u16) -> Option { - if !self.is_activity_focused() { - return None; - } + let (key, ids) = self.disclosure.focused.as_ref()?; + let toggle = if self.disclosure.is_expanded(ids) || self.is_search_expanded(*key) { + "collapse" + } else { + "expand" + }; let keymap = &self.disclosure.keymap; let hints: Vec<_> = [ (ListAction::MoveUp, "previous"), (ListAction::MoveDown, "next"), - (ListAction::Accept, "details"), + (ListAction::Accept, toggle), (ListAction::Cancel, "back"), ] .into_iter() diff --git a/codex-rs/tui/src/transcript_view/footer_tests.rs b/codex-rs/tui/src/transcript_view/footer_tests.rs index 60167612cd..ed5605c3a5 100644 --- a/codex-rs/tui/src/transcript_view/footer_tests.rs +++ b/codex-rs/tui/src/transcript_view/footer_tests.rs @@ -522,6 +522,8 @@ fn activity_focus_keeps_controls_without_passive_hints() { assert!(view.disclosure.is_expanded(&["sample-activity".to_owned()])); let expanded = render_live(&mut view); hints.push(format!("expanded\n{expanded:?}")); + let footer = view.footer(/*width*/ 80, MotionMode::Reduced).unwrap(); + hints.push(format!("expanded, 80 columns: {}", footer.text)); let hidden = Rect::new( /*x*/ 0, /*y*/ 0, /*width*/ 32, /*height*/ 0, ); diff --git a/codex-rs/tui/src/transcript_view/layout.rs b/codex-rs/tui/src/transcript_view/layout.rs index e5e07fec2d..65d6f64c60 100644 --- a/codex-rs/tui/src/transcript_view/layout.rs +++ b/codex-rs/tui/src/transcript_view/layout.rs @@ -238,7 +238,9 @@ impl TranscriptView { } let mode = self.mode; let mut presentation = self.content_presentation(cell.as_ref(), full_content, mode); - if !full_content && let Some(summary) = self.tool_summary(cell.as_ref()) { + if !full_content + && let Some(summary) = self.tool_summary(cell.as_ref(), EntryKey::cell(cell)) + { let expanded = self.disclosure.is_expanded(&summary.ids); if expanded { self.disclosure.expanded.extend(summary.ids.iter().cloned()); @@ -262,7 +264,7 @@ impl TranscriptView { }) { return layout.row_count() > 0; } - if !full_content && self.tool_summary(previous.as_ref()).is_some() { + if !full_content && self.tool_summary(previous.as_ref(), key).is_some() { return true; } let previous_presentation = diff --git a/codex-rs/tui/src/transcript_view/search_tests.rs b/codex-rs/tui/src/transcript_view/search_tests.rs index 079581c690..7e031bcd3b 100644 --- a/codex-rs/tui/src/transcript_view/search_tests.rs +++ b/codex-rs/tui/src/transcript_view/search_tests.rs @@ -1149,7 +1149,7 @@ fn find_expands_only_the_match_and_preserves_manual_disclosures() { insta::assert_snapshot!(hints.join("\n"), @" ↑ Loading earlier messages… · esc latest Retry history: ⌥ ["ctrl-x t","f4"]: []: diff --git a/codex-rs/tui/src/transcript_view/turn_details.rs b/codex-rs/tui/src/transcript_view/turn_details.rs index eba5dcbefd..19d3d24396 100644 --- a/codex-rs/tui/src/transcript_view/turn_details.rs +++ b/codex-rs/tui/src/transcript_view/turn_details.rs @@ -58,7 +58,11 @@ impl ToolSummary { } impl TranscriptView { - pub(super) fn tool_summary(&self, cell: &dyn HistoryCell) -> Option { + pub(super) fn tool_summary( + &self, + cell: &dyn HistoryCell, + key: EntryKey, + ) -> Option { if !self.collapse_turn_details || self.detailed || self.mode != HistoryRenderMode::Rich { return None; } @@ -66,11 +70,14 @@ impl TranscriptView { if calls.is_empty() { return None; } - let ids = cell + let mut ids: Vec<_> = cell .activity_ids() .into_iter() .map(|id| format!("summary:{id}")) .collect(); + if key != EntryKey::Live || ids.is_empty() { + ids.push(format!("work:{key:?}")); + } Some(ToolSummary { ids, calls }) } @@ -90,7 +97,7 @@ impl TranscriptView { self.cache.clear(); self.release_live_reading(); } - let summary = active.and_then(|cell| self.tool_summary(cell)); + let summary = active.and_then(|cell| self.tool_summary(cell, EntryKey::Live)); let ids = summary.as_ref().map_or_else( || active.map_or_else(Vec::new, HistoryCell::activity_ids), |summary| summary.ids.clone(), diff --git a/codex-rs/tui/src/transcript_view_tests.rs b/codex-rs/tui/src/transcript_view_tests.rs index d4e134dff4..c08f79304d 100644 --- a/codex-rs/tui/src/transcript_view_tests.rs +++ b/codex-rs/tui/src/transcript_view_tests.rs @@ -1383,6 +1383,27 @@ fn parked_screen( text(&buf).trim_end().to_owned() } +#[test] +fn tool_disclosure_hides_output() { + let image = crate::history_cell::new_image_generation_call( + "image".into(), + "failed", + Some("private prompt".into()), + /*saved_path*/ None, + ); + let cells: &[Arc] = &[Arc::new(image)]; + let mut view = TranscriptView::default(); + let collapsed = parked_screen(&mut view, cells, /*active*/ None); + view.handle_key(KeyCode::F(4).into(), cells); + view.handle_key(KeyCode::Enter.into(), cells); + let expanded = parked_screen(&mut view, cells, /*active*/ None); + insta::assert_snapshot!(format!("{collapsed}\n{expanded}"), @r" + ▸ Image generation failed + ▾ Image generation failed + Image generation failed + "); +} + #[test] fn live_search_group_keeps_its_summary_in_the_parked_pane() { use codex_app_server_protocol::CommandExecutionSource;