From a7a9d82f15f15e8a89399abb15f4766430b9d5c7 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 10 Nov 2026 08:18:17 +0000 Subject: [PATCH] Collapse completed turn activity beneath transcript prompts (#60557) ## What changed - Fold earlier commentary, reasoning, and tool activity beneath an expandable prompt control in the full-screen transcript, keeping partial and final answers visible. - Label the control with completion timing when available, or a previous-message count otherwise. Keep tool groups independently expandable within the turn. - Preserve assistant message phases during live completion and replay, and retain untimed completion markers to identify completed turns. ## Testing Add transcript coverage for nested expansion, keyboard and mouse controls, narrow viewports, search anchor preservation, detailed mode, and legacy messages without phase or timing metadata. GitOrigin-RevId: b17d9d7c870990178d76d12a13d463ff9c32b96c --- codex-rs/tui/src/app/event_dispatch.rs | 2 + .../tui/src/app/tests/realtime_requests.rs | 10 +- codex-rs/tui/src/app_event.rs | 3 +- codex-rs/tui/src/chatwidget/completion.rs | 1 + .../chatwidget/realtime_tests/transcripts.rs | 6 +- codex-rs/tui/src/chatwidget/replay.rs | 2 +- codex-rs/tui/src/chatwidget/streaming.rs | 27 ++-- .../tui/src/chatwidget/tests/app_server.rs | 1 + .../tui/src/chatwidget/tests/exec_flow.rs | 5 +- .../chatwidget/tests/list_spacing_tests.rs | 2 +- codex-rs/tui/src/chatwidget/turn_lifecycle.rs | 7 +- codex-rs/tui/src/history_cell/messages.rs | 10 ++ codex-rs/tui/src/history_cell/separators.rs | 27 +++- codex-rs/tui/src/thread_transcript.rs | 6 +- .../tui/src/transcript_view/disclosure.rs | 32 +++- codex-rs/tui/src/transcript_view/layout.rs | 18 ++- codex-rs/tui/src/transcript_view/text.rs | 109 ++++++++----- .../tui/src/transcript_view/turn_details.rs | 82 +++++++++- codex-rs/tui/src/transcript_view/turn_tip.rs | 6 +- codex-rs/tui/src/transcript_view_tests.rs | 146 +++++++++++++++++- 20 files changed, 412 insertions(+), 90 deletions(-) diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 70700f6fa9..dfa503d330 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1097,6 +1097,7 @@ impl App { self.finish_initial_history_replay_buffer(tui); } AppEvent::ConsolidateAgentMessage { + phase, interrupted_item, source, copy_source, @@ -1107,6 +1108,7 @@ impl App { } => { let mut cell = history_cell::AgentMarkdownCell::new_with_inline_visualizations(source, &cwd, inline_visualization_context).with_copy_source(copy_source); cell.interrupted_item = interrupted_item; + cell.phase = phase; self.handle_consolidate_agent_message( tui, cell, diff --git a/codex-rs/tui/src/app/tests/realtime_requests.rs b/codex-rs/tui/src/app/tests/realtime_requests.rs index e335e78ade..d4cb7cc43b 100644 --- a/codex-rs/tui/src/app/tests/realtime_requests.rs +++ b/codex-rs/tui/src/app/tests/realtime_requests.rs @@ -708,12 +708,11 @@ async fn replay_reconciles_only_matching_voice_captions_one_for_one() { AppEvent::InsertHistoryCell(cell) => Some( cell.transcript_lines(/*width*/ 80) .into_iter() - .map(|line| line.to_string()) - .collect::>() - .join("\n"), + .map(|line| line.to_string()), ), _ => None, }) + .flatten() .collect::>() .join("\n"); assert_eq!(rendered.matches("show & tests").count(), 2); @@ -1086,12 +1085,11 @@ async fn completed_voice_caption_survives_repeated_thread_replacement() { AppEvent::InsertHistoryCell(cell) => Some( cell.transcript_lines(/*width*/ 80) .into_iter() - .map(|line| line.to_string()) - .collect::>() - .join("\n"), + .map(|line| line.to_string()), ), _ => None, }) + .flatten() .collect::>() .join("\n"); assert_eq!(rendered.matches("spoken complete").count(), 1); diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 113e4a4c0c..ec587b2f23 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -313,7 +313,7 @@ pub(crate) enum CapturedSideStream { /// Presentation retained alongside the transcript while its composer is inactive. #[derive(Debug)] pub(crate) struct CapturedSideDisplay { - pub(crate) rendered_completion_turn_ids: std::collections::HashSet, + pub(crate) rendered_completion_turn_ids: std::collections::HashMap, pub(crate) active: Option>, pub(crate) auxiliary: Vec, pub(crate) draft: Box, @@ -1359,6 +1359,7 @@ pub(crate) enum AppEvent { /// `deferred_history_cell` lets callers add the final stream tail to the /// transcript without first writing its provisional render to scrollback. ConsolidateAgentMessage { + phase: Option, interrupted_item: Option<(String, String)>, source: String, copy_source: Option, diff --git a/codex-rs/tui/src/chatwidget/completion.rs b/codex-rs/tui/src/chatwidget/completion.rs index 7088acf7b6..38213845d0 100644 --- a/codex-rs/tui/src/chatwidget/completion.rs +++ b/codex-rs/tui/src/chatwidget/completion.rs @@ -16,5 +16,6 @@ impl ChatWidget { self.bottom_pane.status_elapsed().filter(|_| live), live.then(Local::now), ) + .filter(|cell| self.local_settings.transcript_mode.is_owned() || cell.has_timing()) } } diff --git a/codex-rs/tui/src/chatwidget/realtime_tests/transcripts.rs b/codex-rs/tui/src/chatwidget/realtime_tests/transcripts.rs index 0984ea3eba..7563398bec 100644 --- a/codex-rs/tui/src/chatwidget/realtime_tests/transcripts.rs +++ b/codex-rs/tui/src/chatwidget/realtime_tests/transcripts.rs @@ -498,7 +498,7 @@ async fn transcript_completion_waits_for_normal_agent_stream_consolidation() { } } - chat.finalize_completed_assistant_message(Some("normal typed output")); + chat.finalize_completed_assistant_message("normal typed output", /*phase*/ None); chat.note_stream_consolidation_completed(); chat.flush_realtime_transcript_history(); @@ -542,7 +542,7 @@ async fn transcript_handoff_moves_deferred_repeats_and_partial_once() { assert_eq!(cells.len(), super::super::MAX_PENDING_TRANSCRIPT_CELLS + 1); assert!(chat.take_realtime_transcript_cells_for_replay().is_empty()); chat.restore_realtime_transcript_cells(cells); - chat.finalize_completed_assistant_message(Some("ordinary stream")); + chat.finalize_completed_assistant_message("ordinary stream", /*phase*/ None); chat.note_stream_consolidation_completed(); chat.flush_realtime_transcript_history(); @@ -849,7 +849,7 @@ async fn completed_user_caption_stays_visible_until_history_commit() { chat.on_agent_message_delta("An earlier answer".into()); assert!(chat.take_realtime_transcript_history().is_empty()); assert!(render_bottom_popup(&chat, /*width*/ 80).contains("Keep these words visible.")); - chat.finalize_completed_assistant_message(Some("An earlier answer")); + chat.finalize_completed_assistant_message("An earlier answer", /*phase*/ None); chat.note_stream_consolidation_completed(); commit_realtime_history_events(&mut chat, &mut events); let history = std::iter::from_fn(|| events.try_recv().ok()) diff --git a/codex-rs/tui/src/chatwidget/replay.rs b/codex-rs/tui/src/chatwidget/replay.rs index 085fdf15b4..a33c326030 100644 --- a/codex-rs/tui/src/chatwidget/replay.rs +++ b/codex-rs/tui/src/chatwidget/replay.rs @@ -228,7 +228,7 @@ impl ChatWidget { if hidden_nested_review_turn { self.turn_lifecycle .rendered_completion_turn_ids - .insert(turn_id.clone()); + .insert(turn_id.clone(), /*v*/ true); } if matches!( status, diff --git a/codex-rs/tui/src/chatwidget/streaming.rs b/codex-rs/tui/src/chatwidget/streaming.rs index fdf5fda1db..6f784435a4 100644 --- a/codex-rs/tui/src/chatwidget/streaming.rs +++ b/codex-rs/tui/src/chatwidget/streaming.rs @@ -86,10 +86,14 @@ impl ChatWidget { } pub(super) fn flush_answer_stream_with_separator(&mut self) { - self.flush_answer_stream(/*completed_message*/ None); + self.flush_answer_stream(/*completed_message*/ None, /*phase*/ None); } - fn flush_answer_stream(&mut self, completed_message: Option<&str>) { + fn flush_answer_stream( + &mut self, + completed_message: Option<&str>, + phase: Option, + ) { let had_stream_controller = self.stream_controller.is_some(); if let Some(mut controller) = self.stream_controller.take() { let needs_scrollback_reflow = controller.needs_scrollback_reflow(); @@ -147,6 +151,7 @@ impl ChatWidget { }); self.note_stream_consolidation_queued(); self.app_event_tx.send(AppEvent::ConsolidateAgentMessage { + phase, interrupted_item, source, copy_source, @@ -281,18 +286,19 @@ impl ChatWidget { (stream, display) } - pub(super) fn finalize_completed_assistant_message(&mut self, message: Option<&str>) { - if self.stream_controller.is_none() - && let Some(message) = message - && !message.is_empty() - { + pub(super) fn finalize_completed_assistant_message( + &mut self, + message: &str, + phase: Option, + ) { + if self.stream_controller.is_none() && !message.is_empty() { let displayed = parse_assistant_markdown(message, self.config.cwd.as_path()).visible_markdown; self.handle_streaming_delta(displayed); } // Item completion is authoritative. Use it for consolidation so any // deltas dropped by a saturated transport cannot truncate the transcript. - self.flush_answer_stream(message); + self.flush_answer_stream(Some(message), phase); self.handle_stream_finished(); self.request_redraw(); } @@ -575,7 +581,8 @@ impl ChatWidget { self.config.cwd.as_path(), context, questions, - ); + ) + .with_phase(item.phase.clone()); if let Some(item) = interrupted { self.app_event_tx .send(AppEvent::CorrectFlushedAgentMessage { item, cell }); @@ -592,7 +599,7 @@ impl ChatWidget { self.handle_stream_finished(); self.request_redraw(); } else { - self.finalize_completed_assistant_message(Some(&message)); + self.finalize_completed_assistant_message(&message, item.phase.clone()); } self.transcript.last_flushed_agent_message = None; if !parsed.visible_markdown.is_empty() { diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index 8107d9a524..745ebe52b2 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -1803,6 +1803,7 @@ async fn live_app_server_turn_completion_repairs_dropped_message_deltas() { .filter_map(|event| match event { AppEvent::ConsolidateAgentMessage { source, + phase: Some(MessagePhase::FinalAnswer), scrollback_reflow, .. } => { diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index c5327a5375..735e01f04c 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -443,14 +443,15 @@ async fn replayed_commands_preserve_individual_output_and_failure_status() { let cells = std::iter::from_fn(|| rx.try_recv().ok()) .filter_map(|event| match event { - AppEvent::InsertHistoryCell(cell) => Some(cell), + AppEvent::InsertHistoryCell(cell) => Some(cell.transcript_lines(/*width*/ 80)), _ => None, }) + .filter(|lines| !lines.is_empty()) .collect::>(); assert_eq!(cells.len(), 4); let transcript = cells .iter() - .map(|cell| lines_to_single_string(&cell.transcript_lines(/*width*/ 80))) + .map(|lines| lines_to_single_string(lines)) .collect::>() .join("\n"); insta::assert_snapshot!(transcript, @r"$ printf first diff --git a/codex-rs/tui/src/chatwidget/tests/list_spacing_tests.rs b/codex-rs/tui/src/chatwidget/tests/list_spacing_tests.rs index e3df19ed23..f0cbaaa56e 100644 --- a/codex-rs/tui/src/chatwidget/tests/list_spacing_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/list_spacing_tests.rs @@ -69,7 +69,7 @@ async fn list_spacing_streams_compact_then_reflows_on_completion_or_interruption } else if plan { chat.on_plan_item_completed(source.into()); } else { - chat.finalize_completed_assistant_message(Some(source)); + chat.finalize_completed_assistant_message(source, /*phase*/ None); } let mut completed = None; while let Ok(event) = rx.try_recv() { diff --git a/codex-rs/tui/src/chatwidget/turn_lifecycle.rs b/codex-rs/tui/src/chatwidget/turn_lifecycle.rs index 9d0215775e..a6ed3332b2 100644 --- a/codex-rs/tui/src/chatwidget/turn_lifecycle.rs +++ b/codex-rs/tui/src/chatwidget/turn_lifecycle.rs @@ -1,5 +1,6 @@ //! Agent-turn lifecycle state for `ChatWidget`. +use std::collections::HashMap; use std::collections::HashSet; use std::time::Instant; @@ -16,8 +17,8 @@ pub(super) struct TurnLifecycleState { /// Cloud confirmed admission to a turn whose live start we have yet to process. pub(super) admitted_turn_id: Option, pub(super) budget_limited_turn_ids: HashSet, - /// Completion labels already inserted into this thread's visible history. - pub(super) rendered_completion_turn_ids: HashSet, + /// False records an untimed turn boundary that may still receive a completion label. + pub(super) rendered_completion_turn_ids: HashMap, pub(super) goal_status_active_turn_started_at: Option, } @@ -30,7 +31,7 @@ impl TurnLifecycleState { has_turn_history: false, admitted_turn_id: None, budget_limited_turn_ids: HashSet::new(), - rendered_completion_turn_ids: HashSet::new(), + rendered_completion_turn_ids: HashMap::new(), goal_status_active_turn_started_at: None, } } diff --git a/codex-rs/tui/src/history_cell/messages.rs b/codex-rs/tui/src/history_cell/messages.rs index 18b51b001a..84f5a86373 100644 --- a/codex-rs/tui/src/history_cell/messages.rs +++ b/codex-rs/tui/src/history_cell/messages.rs @@ -588,6 +588,7 @@ impl HistoryCell for AgentMessageCell { pub(crate) struct AgentMarkdownCell { /// Identifies an interrupted reply that can still receive its authoritative completion. pub(crate) interrupted_item: Option<(String, String)>, + pub(crate) phase: Option, markdown_source: String, copy_source: Option, cwd: PathBuf, @@ -643,6 +644,7 @@ impl AgentMarkdownCell { interrupted_item: None, markdown_source, copy_source: None, + phase: None, cwd: cwd.to_path_buf(), inline_visualization_context, rendered_lines, @@ -651,6 +653,14 @@ impl AgentMarkdownCell { } } + pub(crate) fn with_phase( + mut self, + phase: Option, + ) -> Self { + self.phase = phase; + self + } + pub(crate) fn with_questions( mut self, questions: &[codex_protocol::items::AsyncUserInputQuestion], diff --git a/codex-rs/tui/src/history_cell/separators.rs b/codex-rs/tui/src/history_cell/separators.rs index b00386c78c..14b8c93aa6 100644 --- a/codex-rs/tui/src/history_cell/separators.rs +++ b/codex-rs/tui/src/history_cell/separators.rs @@ -7,7 +7,7 @@ use chrono::Datelike; use chrono::Local; use chrono::NaiveDate; use codex_app_server_protocol::Turn; -use std::collections::HashSet; +use std::collections::HashMap; /// Completion metadata shown after the assistant's final response. /// @@ -26,9 +26,10 @@ pub struct FinalMessageSeparator { clock_format: ClockFormat, } impl FinalMessageSeparator { + /// Full-screen transcripts also use untimed separators to identify completed turns. pub(crate) fn from_turn_once( turn: &Turn, - rendered_turn_ids: &mut HashSet, + rendered_completion_turn_ids: &mut HashMap, fallback_elapsed: Option, fallback_completed_at: Option>, ) -> Option { @@ -42,12 +43,14 @@ impl FinalMessageSeparator { .and_then(|timestamp| DateTime::from_timestamp(timestamp, /*nsecs*/ 0)) .map(|timestamp| timestamp.with_timezone(&Local)) .or(fallback_completed_at); - if completed_at.is_none() && elapsed_seconds.is_none() { - return None; - } - if !rendered_turn_ids.insert(turn.id.clone()) { + let has_label = completed_at.is_some() || elapsed_seconds.is_some(); + if rendered_completion_turn_ids + .get(&turn.id) + .is_some_and(|labeled| *labeled || !has_label) + { return None; } + rendered_completion_turn_ids.insert(turn.id.clone(), has_label); let cell = Self::new(elapsed_seconds, /*runtime_metrics*/ None); Some(match completed_at { Some(completed_at) => cell.with_completed_at(completed_at, ClockFormat::system()), @@ -87,6 +90,14 @@ impl FinalMessageSeparator { self } + pub(crate) fn has_timing(&self) -> bool { + self.completed_at.is_some() || self.elapsed_seconds.is_some() + } + + pub(crate) fn display_label(&self) -> Option { + self.label(self.display_date) + } + fn label(&self, today: NaiveDate) -> Option { let mut label_parts = Vec::new(); if let Some(elapsed_seconds) = self.elapsed_seconds { @@ -129,7 +140,7 @@ impl HistoryCell for FinalMessageSeparator { if width == 0 { return Vec::new(); } - self.label(self.display_date) + self.display_label() .map(|label| { let indent = if width > 2 { " " } else { "" }; let options = textwrap::Options::new(usize::from(width)) @@ -144,7 +155,7 @@ impl HistoryCell for FinalMessageSeparator { } fn raw_lines(&self) -> Vec> { - self.label(self.display_date) + self.display_label() .map(|label| vec![Line::from(label)]) .unwrap_or_default() } diff --git a/codex-rs/tui/src/thread_transcript.rs b/codex-rs/tui/src/thread_transcript.rs index 361513e17f..060d4a9c41 100644 --- a/codex-rs/tui/src/thread_transcript.rs +++ b/codex-rs/tui/src/thread_transcript.rs @@ -275,7 +275,10 @@ fn item_to_cells( cells.push(Arc::new(cell.with_async_replies(replies, cwd.as_path()))); } ThreadItemPayload::AgentMessage { - text, questions, .. + text, + questions, + phase, + .. } => { let parsed = parse_assistant_markdown(&text, cwd.as_path()); if !parsed.visible_markdown.trim().is_empty() { @@ -285,6 +288,7 @@ fn item_to_cells( cwd.as_path(), inline_visualization_context, ) + .with_phase(phase) .with_questions(questions.as_deref().unwrap_or_default()), )); } diff --git a/codex-rs/tui/src/transcript_view/disclosure.rs b/codex-rs/tui/src/transcript_view/disclosure.rs index d161fafcbd..c7caeb96c0 100644 --- a/codex-rs/tui/src/transcript_view/disclosure.rs +++ b/codex-rs/tui/src/transcript_view/disclosure.rs @@ -166,6 +166,13 @@ impl TranscriptView { .bold() .remove_modifier(Modifier::DIM), ); + } else if layout.is_turn_disclosure() { + buf.set_style( + area, + Style::default() + .fg(crate::style::user_message_accent_color()) + .bold(), + ); } else if row == usize::from(layout.separated) { buf.set_style(area, Style::default().underlined()); } @@ -334,11 +341,18 @@ impl TranscriptView { let Some(layout) = self.layout(cells, index) else { return false; }; - if !layout.disclosure { + let Some(control) = layout.disclosure_row() else { return false; - } + }; let ids = self.activity_ids(cells, index); - let row = usize::from(layout.separated); + let row = if matches!( + self.turn_row(cells, index), + Some(turn_details::TurnRow::Prompt { .. }) + ) { + control + } else { + usize::from(layout.separated) + }; let visible = self.visible.iter().any(|visible| { visible.row == row && visible.activity_ids.iter().any(|id| ids.contains(id)) }); @@ -381,6 +395,12 @@ impl TranscriptView { if self.search.has_active_query() && !matched { if let Some(snapshot) = &mut self.held_reading { snapshot.pinned.remove(&key); + if let Some(details) = &self.turn_details { + for child in details.child_keys(index) { + snapshot.pinned.remove(&child); + snapshot.activities.remove(&child); + } + } } } else { self.release_live_reading(); @@ -391,7 +411,11 @@ impl TranscriptView { .and_then(|(key, top_ids, offset, row_bias)| { let unchanged = cells.iter().position(|cell| EntryKey::cell(cell) == key); let top = unchanged.or_else(|| self.canonical_activity_index(cells, &top_ids))?; - let preserve_offset = unchanged.is_some() && top != index; + let prompt = matches!( + self.turn_row(cells, top), + Some(turn_details::TurnRow::Prompt { .. }) + ); + let preserve_offset = unchanged.is_some() && (top != index || prompt); Some(Anchor { key: cells.get(top).map_or(EntryKey::Live, EntryKey::cell), index: top, diff --git a/codex-rs/tui/src/transcript_view/layout.rs b/codex-rs/tui/src/transcript_view/layout.rs index 2eb87a83a0..1edbd6ee45 100644 --- a/codex-rs/tui/src/transcript_view/layout.rs +++ b/codex-rs/tui/src/transcript_view/layout.rs @@ -240,12 +240,17 @@ impl TranscriptView { } let mode = self.mode; let mut presentation = self.content_presentation(cell.as_ref(), full_content, mode); + let mut turn_header = None; if !full_content && let Some(row) = self.turn_row(cells, index) { + let ids = row.ids(); + presentation.expanded = self.disclosure.is_expanded(ids); + presentation.summarized = true; match &row { + turn_details::TurnRow::Prompt { label, .. } => { + let arrow = if presentation.expanded { "▾" } else { "▸" }; + turn_header = Some(format!("{arrow} {label}")); + } turn_details::TurnRow::Tools(_) => { - let ids = row.ids(); - presentation.expanded = self.disclosure.is_expanded(ids); - presentation.summarized = true; if presentation.expanded { self.disclosure.expanded.extend(ids.iter().cloned()); } @@ -253,9 +258,7 @@ impl TranscriptView { row.layout(width, presentation.expanded, Vec::new()) })); } - turn_details::TurnRow::Hidden => { - return Some(Arc::new(TextLayout::new(Vec::new(), width))); - } + _ => return Some(Arc::new(TextLayout::new(Vec::new(), width))), } } let shortcut = self @@ -302,6 +305,9 @@ impl TranscriptView { if shares_prompt_background { layout = layout.without_leading_blank_row(); } + if let Some(label) = turn_header { + layout = layout.with_turn_disclosure(label); + } layout })) } diff --git a/codex-rs/tui/src/transcript_view/text.rs b/codex-rs/tui/src/transcript_view/text.rs index ad255a2e60..93c50c8b6b 100644 --- a/codex-rs/tui/src/transcript_view/text.rs +++ b/codex-rs/tui/src/transcript_view/text.rs @@ -53,9 +53,15 @@ pub(crate) struct TextLayout { disclosure_control: Option, } +#[derive(Clone, Copy, PartialEq)] +enum DisclosureAnchor { + Activity(usize), + Turn, +} + struct DisclosureControl { + anchor: DisclosureAnchor, label: String, - source_offset: usize, row: usize, } @@ -160,8 +166,7 @@ impl TextLayout { pub(super) fn rewrap(&self, width: u16) -> Self { let mut layout = Self::from_logical(self.logical.clone(), width); if let Some(control) = &self.disclosure_control { - layout = - layout.with_disclosure_control_at(control.label.clone(), control.source_offset); + layout = layout.with_control(control.label.clone(), control.anchor); } layout.image = self.image.clone(); if self.leading_blank_hidden { @@ -181,46 +186,63 @@ impl TextLayout { } /// Place a control after an activity, before any following auxiliary source text. - pub(super) fn with_disclosure_control_at( - mut self, - label: String, - source_offset: usize, - ) -> Self { + pub(super) fn with_disclosure_control_at(self, label: String, source_offset: usize) -> Self { + self.with_control(label, DisclosureAnchor::Activity(source_offset)) + } + + pub(super) fn with_turn_disclosure(self, label: String) -> Self { + self.with_control(label, DisclosureAnchor::Turn) + } + + pub(super) fn is_turn_disclosure(&self) -> bool { + self.disclosure_control + .as_ref() + .is_some_and(|control| control.anchor == DisclosureAnchor::Turn) + } + + fn with_control(mut self, label: String, anchor: DisclosureAnchor) -> Self { if label.is_empty() { return self; } if let Some(control) = self.disclosure_control.take() { self.rows.remove(control.row); + if control.anchor == DisclosureAnchor::Turn { + self.rows.remove(control.row - 1); + } } - let source_offset = self - .text - .floor_char_boundary(source_offset.min(self.text.len())); - let control_row = self + let source_offset = match anchor { + DisclosureAnchor::Activity(offset) => { + self.text.floor_char_boundary(offset.min(self.text.len())) + } + DisclosureAnchor::Turn => self.text.len(), + }; + let mut control_row = self .rows .iter() .rposition(|row| row.source.end <= source_offset) .map_or(/*default*/ 0, |row| row + 1); - let indent = usize::from(self.width / 4).min(/*other*/ 4); + let indent = match anchor { + DisclosureAnchor::Activity(_) => usize::from(self.width / 4).min(/*other*/ 4), + DisclosureAnchor::Turn => { + self.insert_presentation_row( + control_row, + source_offset, + /*indent*/ 0, + Line::default(), + ); + control_row += 1; + 0 + } + }; let line = truncate_line_to_width( Line::from(format!("{}{label}", " ".repeat(indent))).dim(), usize::from(self.width), ); - self.rows.insert( - control_row, - TextRow { - line: HyperlinkLine::from(line), - source: source_offset..source_offset, - line_end: None, - content_width: self.width, - first_column: indent, - prefix_columns: indent, - tabs: tabs::TabStops::default(), - }, - ); + self.insert_presentation_row(control_row, source_offset, indent, line); self.disclosure = true; self.disclosure_control = Some(DisclosureControl { + anchor, label, - source_offset, row: control_row, }); self @@ -254,17 +276,11 @@ impl TextLayout { /// Reserve one presentation-only row before existing spacing and source text. pub(super) fn with_leading_spacer(mut self) -> Self { if !self.rows.is_empty() { - self.rows.insert( + self.insert_presentation_row( /*index*/ 0, - TextRow { - line: HyperlinkLine::from(""), - source: 0..0, - line_end: None, - content_width: self.width, - first_column: 0, - prefix_columns: 0, - tabs: tabs::TabStops::default(), - }, + /*offset*/ 0, + /*indent*/ 0, + Line::default(), ); if let Some(control) = &mut self.disclosure_control { control.row += 1; @@ -273,6 +289,27 @@ impl TextLayout { self } + fn insert_presentation_row( + &mut self, + index: usize, + offset: usize, + indent: usize, + line: Line<'static>, + ) { + self.rows.insert( + index, + TextRow { + line: HyperlinkLine::from(line), + source: offset..offset, + line_end: None, + content_width: self.width, + first_column: indent, + prefix_columns: indent, + tabs: tabs::TabStops::default(), + }, + ); + } + /// Source text between adjacent retained fragments; distinct logical lines have a hard break. pub(super) fn separator_after(&self, next: &Self) -> &str { if let Some(previous) = self.logical.last() diff --git a/codex-rs/tui/src/transcript_view/turn_details.rs b/codex-rs/tui/src/transcript_view/turn_details.rs index 39346439b5..a7136378fb 100644 --- a/codex-rs/tui/src/transcript_view/turn_details.rs +++ b/codex-rs/tui/src/transcript_view/turn_details.rs @@ -2,12 +2,18 @@ //! and scrollback retain their original cells, including command output. use super::*; +use crate::history_cell::AgentMarkdownCell; +use crate::history_cell::AgentMessageCell; use crate::history_cell::DynamicToolCallCell; +use crate::history_cell::FinalMessageSeparator; +use crate::history_cell::ReasoningSummaryCell; use crate::history_cell::UserHistoryCell; use crate::history_cell::WebSearchCell; +use codex_protocol::models::MessagePhase; #[derive(Clone)] pub(crate) enum TurnRow { + Prompt { id: String, label: String }, Tools(Arc), Hidden, } @@ -15,6 +21,7 @@ pub(crate) enum TurnRow { impl TurnRow { pub(crate) fn ids(&self) -> &[String] { match self { + Self::Prompt { id, .. } => std::slice::from_ref(id), Self::Tools(group) => &group.ids, Self::Hidden => &[], } @@ -70,6 +77,7 @@ pub(super) struct TurnDetails { struct Entry { key: EntryKey, + parent: Option, row: Option, } @@ -151,6 +159,13 @@ impl ToolGroup { } impl TurnDetails { + pub(super) fn child_keys(&self, parent: usize) -> impl Iterator + '_ { + self.rows + .iter() + .filter(move |entry| entry.parent == Some(parent)) + .map(|entry| entry.key) + } + fn index_turn( &mut self, cells: &[Arc], @@ -159,16 +174,27 @@ impl TurnDetails { ) { let rows = &mut self.rows; let start = rows.len(); + let has_prompt = cells + .first() + .is_some_and(|cell| cell.as_any().is::()); let keys = cells .iter() .map(EntryKey::cell) .chain(latest.then_some(EntryKey::Live)); - rows.extend(keys.map(|key| Entry { key, row: None })); + rows.extend(keys.map(|key| Entry { + key, + parent: None, + row: None, + })); let rows = &mut rows[start..]; let mut group = ToolGroup::default(); if latest && !self.live.0.is_empty() { group.push(rows, cells.len(), self.live.1.clone(), self.live.0.clone()); } + let mut completion = None; + let mut separator = None; + let mut fold = false; + let mut hidden_count = 0; let mut searches = std::collections::HashSet::new(); for (index, cell) in cells.iter().enumerate().rev() { // Keep older calls outside the group whose disclosure is pinned on screen. @@ -187,7 +213,43 @@ impl TurnDetails { rows[index].row = Some(TurnRow::Hidden); continue; } + let markdown = any.downcast_ref::(); + let phase = markdown.and_then(|message| message.phase.as_ref()); let calls = cell.tool_call_labels(); + if let Some(done) = any.downcast_ref::() { + // /review can complete another turn under the same prompt. + // A live timestamp can arrive immediately after an untimed replay marker. + if separator.is_none_or(|(next, _)| next != index + 1) + || done.display_label().is_some() + { + separator = Some((index, done)); + } + fold = false; + } else if phase == Some(&MessagePhase::FinalAnswer) + || (!fold + && separator.is_some() + && ((markdown.is_some() && phase.is_none()) + || any.is::())) + { + // An answer item can finish before its turn succeeds. + fold |= separator.is_some(); + } else if fold + && phase != Some(&MessagePhase::PartialAnswer) + && has_prompt + && (!calls.is_empty() + || markdown.is_some() + || any.is::() + || any + .downcast_ref::() + .is_some_and(|reasoning| !reasoning.is_transcript_only())) + { + rows[index].parent = Some(start); + hidden_count += calls.len().max(1); + if let Some((at, done)) = separator.take() { + rows[at].row = Some(TurnRow::Hidden); + completion.get_or_insert(done); + } + } if let Some(tool) = any.downcast_ref::() && tool.is_active() { @@ -213,6 +275,18 @@ impl TurnDetails { } } group.finish(rows, cells); + if hidden_count > 0 { + let label = completion + .and_then(FinalMessageSeparator::display_label) + .unwrap_or_else(|| match hidden_count { + 1 => "1 previous message".into(), + count => format!("{count} previous messages"), + }); + rows[0].row = Some(TurnRow::Prompt { + id: format!("turn:{:?}", rows[0].key), + label, + }); + } } } @@ -322,6 +396,12 @@ impl TranscriptView { let details = self.turn_details.as_ref()?; let key = cells.get(index).map_or(EntryKey::Live, EntryKey::cell); let entry = details.rows.get(index).filter(|entry| entry.key == key)?; + if let Some(parent) = entry.parent + && let Some(TurnRow::Prompt { id, .. }) = &details.rows[parent].row + && !self.disclosure.expanded.contains(id) + { + return Some(TurnRow::Hidden); + } entry.row.clone() } } diff --git a/codex-rs/tui/src/transcript_view/turn_tip.rs b/codex-rs/tui/src/transcript_view/turn_tip.rs index 854bef6adf..358c9a2ffb 100644 --- a/codex-rs/tui/src/transcript_view/turn_tip.rs +++ b/codex-rs/tui/src/transcript_view/turn_tip.rs @@ -17,8 +17,10 @@ impl TranscriptView { cells .last() .filter(|cell| { - cell.as_any() - .is::() + self.turn_row(cells, cells.len() - 1).is_none() + && cell + .as_any() + .is::() && !cell.display_lines(area.width).is_empty() }) .map(EntryKey::cell) diff --git a/codex-rs/tui/src/transcript_view_tests.rs b/codex-rs/tui/src/transcript_view_tests.rs index 9ad212d3ab..8199b1ac69 100644 --- a/codex-rs/tui/src/transcript_view_tests.rs +++ b/codex-rs/tui/src/transcript_view_tests.rs @@ -1408,8 +1408,130 @@ fn tool_disclosure_hides_output() { "); } +fn assistant_message( + text: &str, + phase: Option, +) -> Arc { + Arc::new( + crate::history_cell::AgentMarkdownCell::new( + text.into(), + &crate::test_support::test_path_buf("/repo"), + ) + .with_phase(phase), + ) +} + #[test] -fn web_search_summary_follows_the_completed_call() { +fn tool_groups_expand_independently_inside_a_completed_turn() { + use crate::exec_cell::CommandOutput; + use crate::exec_cell::new_active_exec_command; + use codex_app_server_protocol::CommandExecutionSource; + use codex_protocol::models::MessagePhase::Commentary; + use codex_protocol::models::MessagePhase::FinalAnswer; + let mut cells = vec![ + prompt("Check why the service writes a diagnostic to stderr each time it starts."), + assistant_message("Checking stderr.", Some(Commentary)), + ]; + for id in ["hello", "world"] { + let mut command = new_active_exec_command( + id.into(), + vec!["echo".into(), id.into()], + Vec::new(), + CommandExecutionSource::Agent, + /*interaction_input*/ None, + /*animations_enabled*/ false, + ); + command.complete_call( + id, + CommandOutput::new(/*exit_code*/ 0, "output sentinel".into()), + std::time::Duration::ZERO, + ); + cells.push(Arc::new(command)); + } + cells.push(assistant_message( + "The path is valid.", + Some(codex_protocol::models::MessagePhase::PartialAnswer), + )); + cells.push(assistant_message( + "The fixture is missing.", + Some(FinalAnswer), + )); + let mut view = TranscriptView::default(); + view.sync_turn_details(&cells, /*enabled*/ true, /*active*/ None); + assert!(turn_screen(&mut view, &cells).contains("Checking stderr.")); + cells.push(Arc::new(crate::history_cell::FinalMessageSeparator::new( + /*elapsed_seconds*/ None, /*runtime_metrics*/ None, + ))); + view.sync_turn_details(&cells, /*enabled*/ true, /*active*/ None); + insta::assert_snapshot!(turn_screen(&mut view, &cells).trim(), @r" + › Check why the service writes a diagnostic to + stderr each time it starts. + + + ▸ 3 previous messages + + • The path is valid. + + • The fixture is missing. + "); + + // The wrapped prompt is taller than this viewport. Keyboard expansion keeps its control visible. + let narrow = |view: &mut TranscriptView| { + text(&render(view, &cells, /*width*/ 25, /*height*/ 3)) + }; + narrow(&mut view); + view.handle_key(KeyCode::F(4).into(), &cells); + narrow(&mut view); + view.handle_key(KeyCode::Enter.into(), &cells); + assert!(narrow(&mut view).contains("▾ 3 previous messages")); + let expanded = turn_screen(&mut view, &cells); + assert!(expanded.contains("Checking stderr.") && expanded.contains("▸ Ran 2 commands")); + assert!(!expanded.contains("Ran echo hello")); + view.handle_key(KeyCode::Down.into(), &cells); + view.handle_key(KeyCode::Enter.into(), &cells); + let expanded = turn_screen(&mut view, &cells); + assert!(expanded.contains("▾ Ran 2 commands")); + assert!(expanded.contains("Ran echo hello") && expanded.contains("Ran echo world")); + assert!(!expanded.contains("output sentinel")); + view.begin_search(); + view.paste_search("fixture is missing"); + while view.advance_search(&cells) {} + view.handle_key(KeyCode::Enter.into(), &cells); + let matched = view.search.match_anchor().expect("final answer match"); + view.scroll(&cells, /*rows*/ -20); + let frame = turn_screen(&mut view, &cells); + let row = |needle: &str| { + frame + .lines() + .position(|line| line.contains(needle)) + .unwrap_or_else(|| panic!("missing {needle} in {frame}")) as u16 + }; + view.begin_selection( + &cells, + /*column*/ 5, + row("Checking stderr."), + /*clicks*/ 2, + ); + assert!(view.toggle_disclosure_at(&cells, /*column*/ 0, row("previous messages"))); + let collapsed = turn_screen(&mut view, &cells); + assert!(!collapsed.contains("Checking stderr.") && !collapsed.contains("Ran 2 commands")); + assert!(collapsed.contains("The fixture is missing.")); + assert_eq!(view.search.match_anchor(), Some(matched)); + view.handle_key(KeyCode::Esc.into(), &cells); + view.set_presentation(/*detailed*/ true, HistoryRenderMode::Rich); + assert!(turn_screen(&mut view, &cells).contains("output sentinel")); + view.set_presentation(/*detailed*/ false, HistoryRenderMode::Rich); + + view.sync_turn_details(&cells, /*enabled*/ false, /*active*/ None); + view.jump_to_latest(); + let disabled = turn_screen(&mut view, &cells); + assert!(disabled.contains("Checking stderr.")); + assert!(disabled.contains("output sentinel")); +} + +#[test] +fn legacy_progress_stays_visible_until_a_final_answer() { + use codex_protocol::models::MessagePhase::FinalAnswer; let completed = |id: &str, query: &str| { Arc::new(crate::history_cell::new_web_search_call( id.into(), @@ -1419,10 +1541,7 @@ fn web_search_summary_follows_the_completed_call() { }; let mut cells = vec![ prompt("Check the fixture"), - Arc::new(crate::history_cell::AgentMarkdownCell::new( - "Intermediate progress".into(), - &crate::test_support::test_path_buf("/repo"), - )), + assistant_message("Intermediate progress", /*phase*/ None), ]; let search = crate::history_cell::new_active_web_search_call( "search".into(), @@ -1484,6 +1603,23 @@ fn web_search_summary_follows_the_completed_call() { let joined = turn_screen(&mut view, &cells); assert!(joined.contains("▾ 3 tool calls")); assert!(joined.contains(" Image generation failed\n Searched the web for fixture\n Searched the web for docs")); + cells.extend([ + assistant_message("Fixture verified", /*phase*/ None), + Arc::new(crate::history_cell::FinalMessageSeparator::new( + /*elapsed_seconds*/ None, /*runtime_metrics*/ None, + )), + Arc::new(crate::history_cell::FinalMessageSeparator::new( + Some(2), + /*runtime_metrics*/ None, + )), + cell("Code review started"), + assistant_message("Review complete", Some(FinalAnswer)), + ]); + view.sync_turn_details(&cells, /*enabled*/ true, /*active*/ None); + let screen = turn_screen(&mut view, &cells); + assert!(screen.contains("▸ Worked for 2s"), "{screen}"); + assert!(screen.contains("Fixture verified") && screen.contains("Review complete")); + assert!(!screen.contains("Intermediate progress")); } #[test]