From e75b6b1e0c87b13b36ff552d6201755107553cce Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sun, 27 Sep 2026 17:54:28 +0000 Subject: [PATCH] Show hidden output line counts in compact terminal activity (#48761) ## What changed Replace generic output disclosure labels with counts such as `+ 5 lines (ctrl+t to expand)` for compact command activity, both live and in history. Count retained output lines hidden or clipped by the preview, excluding output discarded by storage limits. Use the configured `open_transcript` shortcut and omit the hint when it does not fit or the action is unbound. Invalidate cached layouts when keybindings change. Keep generic `Show details` labels for other hidden details and keep disclosure controls outside copyable and searchable text. ## Testing Add regression tests for live/history consistency, remapped and disabled shortcuts, and exclusion of hints from source text. Add snapshot coverage for fully visible output, hidden and clipped lines, command-only details, and storage-truncated output. GitOrigin-RevId: c468be9b88b69dc3ca1abdb6b7be023244d32a57 --- .../src/chatwidget/activity_presentation.rs | 7 +- codex-rs/tui/src/exec_cell/compact.rs | 40 +++-- codex-rs/tui/src/exec_cell/render.rs | 10 +- .../tui/src/history_cell/activity_preview.rs | 8 + codex-rs/tui/src/history_cell/mod.rs | 7 +- codex-rs/tui/src/history_cell/plans.rs | 8 +- ...sclosure_counts_only_revealable_lines.snap | 33 +++++ codex-rs/tui/src/transcript_view.rs | 6 +- .../tui/src/transcript_view/footer_tests.rs | 2 +- codex-rs/tui/src/transcript_view/layout.rs | 37 +++-- codex-rs/tui/src/transcript_view/search.rs | 2 + codex-rs/tui/src/transcript_view_tests.rs | 137 ++++++++++++++++++ 12 files changed, 263 insertions(+), 34 deletions(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__transcript_view__tests__terminal_output_disclosure_counts_only_revealable_lines.snap diff --git a/codex-rs/tui/src/chatwidget/activity_presentation.rs b/codex-rs/tui/src/chatwidget/activity_presentation.rs index bbafee3f0b..573a4770f6 100644 --- a/codex-rs/tui/src/chatwidget/activity_presentation.rs +++ b/codex-rs/tui/src/chatwidget/activity_presentation.rs @@ -38,8 +38,9 @@ impl ChatWidget { render_compact(cell, width) } }); - let has_hidden_details = - has_activity && active.is_some_and(|cell| cell.has_hidden_activity_details(width)); + let disclosure = active + .filter(|_| has_activity) + .and_then(|cell| cell.activity_disclosure(width)); let mut auxiliary = self .active_cell_hyperlink_lines_with(width, |cell, width| { if active.is_some_and(|active| std::ptr::eq(active, cell)) { @@ -58,7 +59,7 @@ impl ChatWidget { Some(ActivityTranscriptLines { activity, auxiliary, - has_hidden_details, + disclosure, }) } } diff --git a/codex-rs/tui/src/exec_cell/compact.rs b/codex-rs/tui/src/exec_cell/compact.rs index 87eb6d69a6..f8fe1bb9df 100644 --- a/codex-rs/tui/src/exec_cell/compact.rs +++ b/codex-rs/tui/src/exec_cell/compact.rs @@ -2,6 +2,7 @@ use super::model::ExecCell; use crate::exec_command::strip_bash_lc_and_escape; +use crate::history_cell::ActivityDisclosure; use crate::history_cell::HistoryRenderMode; use crate::history_cell::activity_preview::DETAIL_PREVIEW_LINES; use crate::history_cell::activity_preview::clipped_line; @@ -16,17 +17,33 @@ use ratatui::style::Stylize; use ratatui::text::Line; impl ExecCell { - pub(super) fn command_has_hidden_details(&self, width: u16) -> bool { + pub(super) fn command_disclosure(&self, width: u16) -> Option { let [call] = self.group.calls.as_slice() else { - return false; + return None; }; + // Interaction output is intentionally absent from the expanded transcript. + if !call.is_unified_exec_interaction() + && let Some(output) = &call.output + { + let retained = output.line_counts().1; + let clipped = output + .lines() + .rev() + .take(DETAIL_PREVIEW_LINES) + .filter(|raw| ansi_escape_line(raw.as_ref()).width() + 4 > usize::from(width)) + .count(); + let hidden = retained.saturating_sub(DETAIL_PREVIEW_LINES) + clipped; + if hidden > 0 { + return Some(ActivityDisclosure::OutputLines(hidden)); + } + } if !self .group .details .lines_after(/*after_calls*/ 1, width, HistoryRenderMode::Rich) .is_empty() { - return true; + return Some(ActivityDisclosure::Generic); } let script = strip_bash_lc_and_escape(&call.command); if script.lines().nth(/*n*/ 1).is_some() @@ -35,14 +52,17 @@ impl ExecCell { .first() .is_some_and(|header| header.width() > usize::from(width)) { - return true; + return Some(ActivityDisclosure::Generic); } - call.output.as_ref().is_some_and(|output| { - output.line_counts().1 > DETAIL_PREVIEW_LINES - || output - .transcript_lines() - .any(|line| ansi_escape_line(line.as_ref()).width() + 4 > usize::from(width)) - }) + call.output + .as_ref() + .is_some_and(|output| { + output.line_counts().1 > DETAIL_PREVIEW_LINES + || output.transcript_lines().any(|line| { + ansi_escape_line(line.as_ref()).width() + 4 > usize::from(width) + }) + }) + .then_some(ActivityDisclosure::Generic) } pub(super) fn compact_command_lines(&self, width: u16) -> Vec { diff --git a/codex-rs/tui/src/exec_cell/render.rs b/codex-rs/tui/src/exec_cell/render.rs index 86d66a80d9..d8b326d15b 100644 --- a/codex-rs/tui/src/exec_cell/render.rs +++ b/codex-rs/tui/src/exec_cell/render.rs @@ -6,6 +6,7 @@ use super::model::CommandOutput; use super::model::ExecCall; use super::model::ExecCell; use crate::exec_command::strip_bash_lc_and_escape; +use crate::history_cell::ActivityDisclosure; use crate::history_cell::HistoryCell; use crate::history_cell::HistoryRenderMode; use crate::history_cell::plain_lines; @@ -218,18 +219,19 @@ impl HistoryCell for ExecCell { } } - fn has_hidden_activity_details(&self, width: u16) -> bool { + fn activity_disclosure(&self, width: u16) -> Option { if self.is_exploring_cell() { - return true; + return Some(ActivityDisclosure::Generic); } if self.group.calls.iter().any(ExecCall::is_user_shell_command) { // User-shell previews already retain wrapped commands and up to fifty output rows. // Reuse its actual truncation decision instead of comparing differently styled text. return self .command_display_lines_with_hidden_details(width) - .hidden_details; + .hidden_details + .then_some(ActivityDisclosure::Generic); } - self.command_has_hidden_details(width) + self.command_disclosure(width) } fn compact_hyperlink_lines(&self, width: u16) -> Vec { diff --git a/codex-rs/tui/src/history_cell/activity_preview.rs b/codex-rs/tui/src/history_cell/activity_preview.rs index 96db759b71..0a8ade1e15 100644 --- a/codex-rs/tui/src/history_cell/activity_preview.rs +++ b/codex-rs/tui/src/history_cell/activity_preview.rs @@ -6,6 +6,14 @@ use ratatui::text::Line; pub(crate) const DETAIL_PREVIEW_LINES: usize = 3; +/// Details an activity can reveal without materializing its expanded transcript. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ActivityDisclosure { + Generic, + /// Retained output lines not fully visible in the preview, excluding storage omissions. + OutputLines(usize), +} + /// Clip a preview row without teaching selection to copy text that is currently hidden. pub(crate) fn clipped_line(line: Line<'static>, width: u16) -> HyperlinkLine { truncate_line_with_ellipsis_if_overflow(line, usize::from(width)).into() diff --git a/codex-rs/tui/src/history_cell/mod.rs b/codex-rs/tui/src/history_cell/mod.rs index c177895a1b..bbb05987b4 100644 --- a/codex-rs/tui/src/history_cell/mod.rs +++ b/codex-rs/tui/src/history_cell/mod.rs @@ -123,6 +123,7 @@ mod startup_warnings; mod warnings; pub(crate) use activity_details::ActivityDetails; +pub(crate) use activity_preview::ActivityDisclosure; pub(crate) use approvals::*; pub(crate) use base::*; pub(crate) use dynamic::DynamicToolCallCell; @@ -254,10 +255,10 @@ pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync + Any { self.transcript_hyperlink_lines(width) } - /// Whether an activity offers details beyond its compact presentation. + /// Details an activity offers beyond its compact presentation, if any. /// Prefer source metadata so collapsed rendering does not materialize hidden content. - fn has_hidden_activity_details(&self, _width: u16) -> bool { - true + fn activity_disclosure(&self, _width: u16) -> Option { + Some(ActivityDisclosure::Generic) } fn display_lines_for_mode(&self, width: u16, mode: HistoryRenderMode) -> Vec> { diff --git a/codex-rs/tui/src/history_cell/plans.rs b/codex-rs/tui/src/history_cell/plans.rs index 7f68f5c7b9..f790bff8ed 100644 --- a/codex-rs/tui/src/history_cell/plans.rs +++ b/codex-rs/tui/src/history_cell/plans.rs @@ -207,8 +207,9 @@ impl HistoryCell for PlanUpdateCell { vec![self.activity_id.clone()] } - fn has_hidden_activity_details(&self, width: u16) -> bool { - self.explanation + fn activity_disclosure(&self, width: u16) -> Option { + (self + .explanation .as_ref() .is_some_and(|explanation| !explanation.trim().is_empty()) || self.plan.len() > super::activity_preview::DETAIL_PREVIEW_LINES @@ -216,7 +217,8 @@ impl HistoryCell for PlanUpdateCell { let step = item.step.split_whitespace().collect::>().join(" "); // The compact row reserves four columns for indentation and two for its marker. Line::from(step).width() + 6 > usize::from(width) - }) + })) + .then_some(super::ActivityDisclosure::Generic) } fn compact_hyperlink_lines(&self, width: u16) -> Vec { diff --git a/codex-rs/tui/src/snapshots/codex_tui__transcript_view__tests__terminal_output_disclosure_counts_only_revealable_lines.snap b/codex-rs/tui/src/snapshots/codex_tui__transcript_view__tests__terminal_output_disclosure_counts_only_revealable_lines.snap new file mode 100644 index 0000000000..44e5532a60 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__transcript_view__tests__terminal_output_disclosure_counts_only_revealable_lines.snap @@ -0,0 +1,33 @@ +--- +source: tui/src/transcript_view_tests.rs +expression: "frames.join(\"\\n\\n\")" +--- +three visible +• Ran echo + └ a + b + c + +one hidden +• Ran echo + └ b + c + d + + 1 line (ctrl+t to expand) + +clipped line +• Ran echo + └ abcdefghijklmno… + + 1 line + +command only +• Ran echo a … + └ (no output) + + Show details + +storage truncated +• Running echo + └ x + x + x + + 97 lines (ctrl+t to expand) diff --git a/codex-rs/tui/src/transcript_view.rs b/codex-rs/tui/src/transcript_view.rs index db58338e29..548f63cf11 100644 --- a/codex-rs/tui/src/transcript_view.rs +++ b/codex-rs/tui/src/transcript_view.rs @@ -287,8 +287,12 @@ impl TranscriptView { expanded: bool, lines: impl FnOnce(u16) -> Option, ) -> bool { + let shortcut = self + .disclosure + .keymap + .primary_hint(crate::keymap::KeymapContext::Global, "open_transcript"); self.sync_live_layout(width, key, |width| { - lines(width).map(|lines| layout::activity_layout(lines, width, expanded)) + lines(width).map(|lines| layout::activity_layout(lines, width, expanded, shortcut)) }) } diff --git a/codex-rs/tui/src/transcript_view/footer_tests.rs b/codex-rs/tui/src/transcript_view/footer_tests.rs index 779acaf369..fd42877a39 100644 --- a/codex-rs/tui/src/transcript_view/footer_tests.rs +++ b/codex-rs/tui/src/transcript_view/footer_tests.rs @@ -414,7 +414,7 @@ fn activity_focus_keeps_controls_without_passive_hints() { cell.compact_hyperlink_lines(width) }, auxiliary: Vec::new(), - has_hidden_details: true, + disclosure: cell.activity_disclosure(width), }) }); render(view, &[]) diff --git a/codex-rs/tui/src/transcript_view/layout.rs b/codex-rs/tui/src/transcript_view/layout.rs index e2eb210346..bac11e7a2c 100644 --- a/codex-rs/tui/src/transcript_view/layout.rs +++ b/codex-rs/tui/src/transcript_view/layout.rs @@ -3,6 +3,9 @@ //! Mutable cells refresh each frame, then share that frame's layout across measurement and paint. //! Stable cells also invalidate when animation ticks, syntax themes, or terminal colors change. +use crate::history_cell::ActivityDisclosure; +use crate::key_hint::ShortcutHint; +use crate::keymap::KeymapContext; use std::sync::Weak; use super::*; @@ -14,7 +17,7 @@ const MAX_CACHED_TEXT_BYTES: usize = 8 * 1024 * 1024; pub(crate) struct ActivityTranscriptLines { pub(crate) activity: Vec, pub(crate) auxiliary: Vec, - pub(crate) has_hidden_details: bool, + pub(crate) disclosure: Option, } /// Keep disclosure controls outside selectable source without rendering hidden details. @@ -22,8 +25,9 @@ pub(super) fn activity_layout( mut lines: ActivityTranscriptLines, width: u16, expanded: bool, + shortcut: Option, ) -> TextLayout { - let has_details = !lines.activity.is_empty() && (expanded || lines.has_hidden_details); + let has_details = !lines.activity.is_empty() && (expanded || lines.disclosure.is_some()); let source_offset = (!lines.auxiliary.is_empty()) .then(|| TextLayout::new(lines.activity.clone(), width).text().len()); lines.activity.extend(lines.auxiliary); @@ -31,12 +35,22 @@ pub(super) fn activity_layout( if !has_details { return layout; } - let label = if expanded { - "− Show less" - } else { - "+ Show details" - } - .to_owned(); + let label = match (expanded, lines.disclosure) { + (true, _) => "− Show less".to_owned(), + (false, Some(ActivityDisclosure::OutputLines(count))) => { + let noun = if count == 1 { "line" } else { "lines" }; + let mut label = format!("+ {count} {noun}"); + if let Some(shortcut) = shortcut { + let hint = format!(" ({} to expand)", shortcut.display_label()); + let indent = usize::from(width / 4).min(/*other*/ 4); + if Line::from(format!("{label}{hint}")).width() + indent <= usize::from(width) { + label.push_str(&hint); + } + } + label + } + (false, Some(ActivityDisclosure::Generic) | None) => "+ Show details".to_owned(), + }; match source_offset { Some(offset) => layout.with_disclosure_control_at(label, offset), None => layout.with_disclosure_control(label), @@ -141,6 +155,10 @@ impl TranscriptView { expanded, disclosure, }; + let shortcut = self + .disclosure + .keymap + .primary_hint(KeymapContext::Global, "open_transcript"); Some(self.cache.get(cell, width, presentation, || { if disclosure { activity_layout( @@ -151,10 +169,11 @@ impl TranscriptView { cell.compact_hyperlink_lines(width) }, auxiliary: Vec::new(), - has_hidden_details: cell.has_hidden_activity_details(width), + disclosure: cell.activity_disclosure(width), }, width, expanded, + shortcut, ) } else { TextLayout::new( diff --git a/codex-rs/tui/src/transcript_view/search.rs b/codex-rs/tui/src/transcript_view/search.rs index 047f821923..2e7e456395 100644 --- a/codex-rs/tui/src/transcript_view/search.rs +++ b/codex-rs/tui/src/transcript_view/search.rs @@ -111,6 +111,8 @@ impl TranscriptView { } self.search.editor.set_keymap_bindings(keymap); self.disclosure.keymap = keymap.clone(); + self.cache.clear(); + self.live_key = None; } pub(crate) fn is_search_active(&self) -> bool { diff --git a/codex-rs/tui/src/transcript_view_tests.rs b/codex-rs/tui/src/transcript_view_tests.rs index 9b9a570966..8492e227b0 100644 --- a/codex-rs/tui/src/transcript_view_tests.rs +++ b/codex-rs/tui/src/transcript_view_tests.rs @@ -63,6 +63,143 @@ pub(super) fn text(buffer: &Buffer) -> String { .join("\n") } +#[test] +fn terminal_output_disclosure_follows_live_history_and_keymap() { + use crate::exec_cell::CommandOutput; + use crate::exec_cell::new_active_exec_command; + use crate::keymap::RuntimeKeymap; + use codex_app_server_protocol::CommandExecutionSource; + + let mut cell = new_active_exec_command( + "output-test".into(), + vec!["echo".into()], + Vec::new(), + CommandExecutionSource::Agent, + /*interaction_input*/ None, + /*animations_enabled*/ false, + ); + cell.complete_call( + "output-test", + CommandOutput::new(/*exit_code*/ 0, "1\n2\n3\n4\n5\n6\n7\n8\n".into()), + std::time::Duration::ZERO, + ); + let cell: Arc = Arc::new(cell); + let mut view = TranscriptView::default(); + view.sync_live_activity(&[], cell.activity_ids()); + view.sync_live_activity_tail( + /*width*/ 72, + /*key*/ None, + /*expanded*/ false, + |width| { + Some(ActivityTranscriptLines { + activity: cell.compact_hyperlink_lines(width), + auxiliary: Vec::new(), + disclosure: cell.activity_disclosure(width), + }) + }, + ); + let live = text(&render( + &mut view, + &[], + /*width*/ 72, + /*height*/ 6, + )); + let cells = [cell]; + view.sync_live_activity(&cells, Vec::new()); + view.sync_live_tail(/*width*/ 72, /*key*/ None, |_| None); + let committed = text(&render( + &mut view, &cells, /*width*/ 72, /*height*/ 6, + )); + assert_eq!(live, committed); + let hint = " (ctrl+t to expand)"; + assert!(live.contains(&format!("+ 5 lines{hint}"))); + // Check where the hint is visible, so the copy/search exclusion cannot pass vacuously. + assert!( + !view + .layout(&cells, /*index*/ 0) + .unwrap() + .text() + .contains(hint) + ); + // Warm layouts must follow config changes, including chords and disabling the action. + for (configured, expected) in [ + (serde_json::json!("f12"), " (f12 to expand)"), + (serde_json::json!("ctrl-x t"), " (ctrl+x t to expand)"), + (serde_json::json!([]), ""), + ] { + let config = serde_json::from_value(serde_json::json!({ + "global": {"open_transcript": configured} + })) + .unwrap(); + let keymap = RuntimeKeymap::from_config(&config).unwrap(); + view.set_keymap_bindings(&keymap); + assert_eq!( + text(&render( + &mut view, &cells, /*width*/ 72, /*height*/ 6, + )), + live.replace(hint, expected), + ); + } +} + +#[test] +fn terminal_output_disclosure_counts_only_revealable_lines() { + use crate::exec_cell::CommandOutput; + use crate::exec_cell::new_active_exec_command; + use codex_app_server_protocol::CommandExecutionSource; + + let mut frames = Vec::new(); + for (name, command, output, width, streamed) in [ + ("three visible", "echo", "a\nb\nc\n".to_owned(), 64, false), + ("one hidden", "echo", "a\nb\nc\nd\n".to_owned(), 64, false), + ( + "clipped line", + "echo", + "abcdefghijklmnopqrstuvwxyz\n".to_owned(), + 20, + false, + ), + ("command only", "echo a\necho b", String::new(), 64, false), + ( + "storage truncated", + "echo", + "x\n".repeat(/*n*/ 530_000), + 64, + true, + ), + ] { + let mut cell = new_active_exec_command( + "output-test".into(), + vec!["sh".into(), "-c".into(), command.into()], + Vec::new(), + CommandExecutionSource::Agent, + /*interaction_input*/ None, + /*animations_enabled*/ false, + ); + if streamed { + cell.append_output("output-test", &output); + } else { + cell.complete_call( + "output-test", + CommandOutput::new(/*exit_code*/ 0, output), + std::time::Duration::from_secs(/*secs*/ 1), + ); + } + let cells: [Arc; 1] = [Arc::new(cell)]; + frames.push(format!( + "{name}\n{}", + text(&render( + &mut TranscriptView::default(), + &cells, + width, + /*height*/ 6, + )) + .trim_end() + )); + } + insta::assert_snapshot!(frames.join("\n\n")); +} + #[test] fn startup_warning_keeps_first_visible_header_in_place() { let header: Arc =