From 01806453b03b18388afa90920ba3778f66b02fa6 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 14 Nov 2026 00:01:33 +0000 Subject: [PATCH] Refill the transcript viewport when collapsing tool groups (#61424) ## Why Collapsing a tool group while reading its expanded details can leave the scroll anchor beyond the new bottom limit, leaving available transcript space unfilled. ## What changed Clamp the reading position to the bottom limit during collapse layout and persist it after the final viewport height is known. Keep activity focus and reading mode intact, including when copy confirmation changes the composer layout. ## Testing Add regression coverage for collapsing a scrolled tool group, preserving the resulting view across renders, and rendering a collapsed group with clipboard confirmation. GitOrigin-RevId: f48271720f7b400736bc02b08b6eafefdb60ac63 --- codex-rs/tui/src/app/owned_transcript.rs | 1 + .../src/app/owned_transcript_follow_tests.rs | 36 ++++++++++ ..._tests__running_copy_feedback_spacing.snap | 20 +++++- ...ollapsing_tool_group_refills_viewport.snap | 12 ++++ codex-rs/tui/src/transcript_view.rs | 67 ++++++++++++++++--- .../tui/src/transcript_view/disclosure.rs | 6 +- codex-rs/tui/src/transcript_view/turn_tip.rs | 4 +- codex-rs/tui/src/transcript_view_tests.rs | 36 ++++++++++ 8 files changed, 168 insertions(+), 14 deletions(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__transcript_view__tests__collapsing_tool_group_refills_viewport.snap diff --git a/codex-rs/tui/src/app/owned_transcript.rs b/codex-rs/tui/src/app/owned_transcript.rs index a4c54fb1d6..59ae94c3f6 100644 --- a/codex-rs/tui/src/app/owned_transcript.rs +++ b/codex-rs/tui/src/app/owned_transcript.rs @@ -673,6 +673,7 @@ impl App { ); needs_redraw = false; } + view.finish_collapse_layout(&self.transcript_cells); let blossom_tick = chat_widget .empty_state_animation .borrow_mut() diff --git a/codex-rs/tui/src/app/owned_transcript_follow_tests.rs b/codex-rs/tui/src/app/owned_transcript_follow_tests.rs index 181a8959a1..3a68cfdd78 100644 --- a/codex-rs/tui/src/app/owned_transcript_follow_tests.rs +++ b/codex-rs/tui/src/app/owned_transcript_follow_tests.rs @@ -192,6 +192,42 @@ async fn follow_control_click_preserves_draft_caret_and_composer_geometry() -> R super::tests::buffer_text(buffer) )); } + + app.local_settings.tui.collapse_turn_details = true; + for id in 1..=6 { + app.transcript_cells + .push(Arc::new(crate::history_cell::new_web_search_call( + format!("lookup-{id}"), + format!("field note {id}"), + codex_app_server_protocol::WebSearchAction::Other, + ))); + } + let size = Size::new(/*width*/ 80, /*height*/ 16); + tui.screen_size_for_event(&TuiEvent::Resize(size))?; + app.render_owned_transcript(&mut tui, size)?; + for key in [KeyCode::F(4), KeyCode::Enter] { + app.transcript_view + .handle_key(key.into(), &app.transcript_cells); + } + app.render_owned_transcript(&mut tui, size)?; + app.transcript_view + .scroll(&app.transcript_cells, /*rows*/ 2); + app.render_owned_transcript(&mut tui, size)?; + app.transcript_view + .handle_key(KeyCode::Enter.into(), &app.transcript_cells); + app.transcript_view.show_copy_feedback( + &Ok(crate::clipboard_copy::CopyStatus::Confirmed), + /*characters*/ 24, + ); + app.render_owned_transcript(&mut tui, size)?; + let buffer = crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal); + let collapsed = super::tests::buffer_text(buffer); + assert!(collapsed.contains("6 tool calls"), "{collapsed}"); + assert!(app.transcript_view.is_activity_focused()); + snapshots.push(format!( + "Collapsed tool group with copy feedback\n{collapsed}" + )); + insta::assert_snapshot!( "running_copy_feedback_spacing", crate::chatwidget::tests::helpers::normalize_working_snapshot( diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__owned_transcript__follow_tests__running_copy_feedback_spacing.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__owned_transcript__follow_tests__running_copy_feedback_spacing.snap index bf17823140..9cc75d1a21 100644 --- a/codex-rs/tui/src/app/snapshots/codex_tui__app__owned_transcript__follow_tests__running_copy_feedback_spacing.snap +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__owned_transcript__follow_tests__running_copy_feedback_spacing.snap @@ -1,6 +1,6 @@ --- source: tui/src/app/owned_transcript_follow_tests.rs -expression: "crate::chatwidget::tests::helpers::normalize_snapshot_paths(snapshots.join(\"\\n\\n\"))" +expression: "crate::chatwidget::tests::helpers::normalize_working_snapshot(snapshots.join(\"\\n\\n\"))" --- 80 columns, 12 rows transcript row 37 @@ -59,3 +59,21 @@ row 39 GPT-5.6-Sol default · /tmp/project tab to queue message + +Collapsed tool group with copy feedback +transcript row 34 +transcript row 35 +transcript row 36 +transcript row 37 +transcript row 38 +transcript row 39 + ⌕ 6 tool calls · Searched the web for field note 6 + +• Working (0s • esc to interrupt) + + Copied 24 chars to host clipboard + +› draft stays here + + GPT-5.6-Sol default · /tmp/project + ↑ previous · ↓ next · enter expand · esc back diff --git a/codex-rs/tui/src/snapshots/codex_tui__transcript_view__tests__collapsing_tool_group_refills_viewport.snap b/codex-rs/tui/src/snapshots/codex_tui__transcript_view__tests__collapsing_tool_group_refills_viewport.snap new file mode 100644 index 0000000000..077ca84e06 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__transcript_view__tests__collapsing_tool_group_refills_viewport.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/transcript_view_tests.rs +expression: collapsed +--- + + +Opened the field notes +Checking dates and locations +Checking names and totals + ⌕ 6 tool calls · Searched the web for field note + +Checking the last source diff --git a/codex-rs/tui/src/transcript_view.rs b/codex-rs/tui/src/transcript_view.rs index 691387f9b7..fa7f333132 100644 --- a/codex-rs/tui/src/transcript_view.rs +++ b/codex-rs/tui/src/transcript_view.rs @@ -186,6 +186,11 @@ impl TranscriptView { } pub(crate) fn render(&mut self, area: Rect, buf: &mut Buffer, cells: &[Arc]) { + self.paint_rows(area, buf, cells); + self.finish_collapse_layout(cells); + } + + fn paint_rows(&mut self, area: Rect, buf: &mut Buffer, cells: &[Arc]) { self.composer_tip = None; self.prompt_header_target = None; self.cache.begin_frame(); @@ -218,7 +223,19 @@ impl TranscriptView { anchor.row_bias = layout.row_for_offset(anchor.offset) as isize - row as isize; self.position = Position::Reading(anchor); } - let initial_start = self.start(cells); + // Closing a disclosure can leave its reading anchor below the new scroll limit. + let clamp_after_collapse = self.disclosure.clamp_after_collapse + && self.selection.is_none() + && matches!(self.position, Position::Reading(_)); + let render_start = |view: &mut Self| { + let start = view.start(cells); + if clamp_after_collapse { + start.min(view.bottom_start(cells)) + } else { + start + } + }; + let initial_start = render_start(self); let initial_position = self.position; let mut start = initial_start; let mut body = area; @@ -242,8 +259,8 @@ impl TranscriptView { if resized { self.reveal_copy_context(cells); } - // Following may advance into the next turn after reserving the header row. - start = self.start(cells); + // The bottom limit changes after reserving the header row. + start = render_start(self); if let Some((header, key)) = prompt_header::line(cells, start.0, area.width, self.detailed) { @@ -324,6 +341,26 @@ impl TranscriptView { self.render_search(buf); } + /// Persist the clamped reading position after the last paint at the final viewport height. + pub(crate) fn finish_collapse_layout(&mut self, cells: &[Arc]) { + if self.area.is_empty() + || !std::mem::take(&mut self.disclosure.clamp_after_collapse) + || self.selection.is_some() + || !matches!(self.position, Position::Reading(_)) + { + return; + } + let snapshot = self.snapshot_cells(); + let cells = snapshot.as_deref().unwrap_or(cells); + let start = self.start(cells); + if let Some(first) = self.visible.first() + && (first.index, first.row) < start + { + let (index, row, layout) = (first.index, first.row, Arc::clone(&first.layout)); + self.read_from_row(cells, index, row, &layout); + } + } + pub(crate) fn image_placements(&self) -> Vec { let mut placements = Vec::new(); let mut y = self.area.y; @@ -583,17 +620,27 @@ impl TranscriptView { return; } if let Some(layout) = self.layout(cells, index) { - let offset = layout.position_at(row, /*column*/ 0); - self.position = Position::Reading(Anchor { - key: self.entry_key(cells, index), - index, - offset, - row_bias: layout.row_for_offset(offset) as isize - row as isize, - }); + self.read_from_row(cells, index, row, &layout); self.hold_live_reading(current_cells, layout); } } + fn read_from_row( + &mut self, + cells: &[Arc], + index: usize, + row: usize, + layout: &TextLayout, + ) { + let offset = layout.position_at(row, /*column*/ 0); + self.position = Position::Reading(Anchor { + key: self.entry_key(cells, index), + index, + offset, + row_bias: layout.row_for_offset(offset) as isize - row as isize, + }); + } + pub(crate) fn jump_to_entry(&mut self, cells: &[Arc], index: usize) { self.cancel_beginning(); self.release_live_reading(); diff --git a/codex-rs/tui/src/transcript_view/disclosure.rs b/codex-rs/tui/src/transcript_view/disclosure.rs index c7caeb96c0..ab4fa1c408 100644 --- a/codex-rs/tui/src/transcript_view/disclosure.rs +++ b/codex-rs/tui/src/transcript_view/disclosure.rs @@ -21,6 +21,7 @@ pub(super) struct Disclosure { 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, + pub(super) clamp_after_collapse: bool, pub(super) live_ids: Vec, pub(super) keymap: RuntimeKeymap, } @@ -32,6 +33,7 @@ impl Default for Disclosure { focused: None, resume_after_approval: None, anchor_control: None, + clamp_after_collapse: false, live_ids: Vec::new(), keymap: RuntimeKeymap::defaults(), } @@ -429,11 +431,13 @@ impl TranscriptView { offset: 0, row_bias: 0, }); - if matched || self.disclosure.is_expanded(&ids) { + let collapsing = matched || self.disclosure.is_expanded(&ids); + if collapsing { self.disclosure.expanded.retain(|id| !ids.contains(id)); } else { self.disclosure.expanded.extend(ids.iter().cloned()); } + self.disclosure.clamp_after_collapse = collapsing; self.disclosure.focused = Some((key, ids)); self.position = Position::Reading(anchor); self.cache.clear(); diff --git a/codex-rs/tui/src/transcript_view/turn_tip.rs b/codex-rs/tui/src/transcript_view/turn_tip.rs index 358c9a2ffb..7dcabd609e 100644 --- a/codex-rs/tui/src/transcript_view/turn_tip.rs +++ b/codex-rs/tui/src/transcript_view/turn_tip.rs @@ -25,7 +25,7 @@ impl TranscriptView { }) .map(EntryKey::cell) }); - self.render( + self.paint_rows( Rect { height: area.height - u16::from(tip.is_some() && self.turn_tip_key.is_none()), ..area @@ -39,7 +39,7 @@ impl TranscriptView { && self.visible.first().is_none_or(|row| row.key == key) { self.turn_tip_key = None; - self.render(area, buf, cells); + self.paint_rows(area, buf, cells); return None; } tip?; diff --git a/codex-rs/tui/src/transcript_view_tests.rs b/codex-rs/tui/src/transcript_view_tests.rs index 8199b1ac69..8b73170510 100644 --- a/codex-rs/tui/src/transcript_view_tests.rs +++ b/codex-rs/tui/src/transcript_view_tests.rs @@ -1421,6 +1421,42 @@ fn assistant_message( ) } +#[test] +fn collapsing_tool_group_refills_viewport() { + let mut cells = vec![ + prompt("Check the field notes"), + cell("Opened the field notes\nChecking dates and locations\nChecking names and totals"), + ]; + for id in 1..=6 { + cells.push(Arc::new(crate::history_cell::new_web_search_call( + format!("lookup-{id}"), + format!("field note {id}"), + codex_app_server_protocol::WebSearchAction::Other, + ))); + } + cells.push(cell("Checking the last source")); + let mut view = TranscriptView::default(); + view.sync_turn_details(&cells, /*enabled*/ true, /*active*/ None); + let screen = |view: &mut TranscriptView| { + text(&render(view, &cells, /*width*/ 52, /*height*/ 8)) + }; + let initial = screen(&mut view); + view.handle_key(KeyCode::F(4).into(), &cells); + view.handle_key(KeyCode::Enter.into(), &cells); + screen(&mut view); + // Read down to the expanded group's details, then close it in place. + view.scroll(&cells, /*rows*/ 2); + let expanded = screen(&mut view); + assert!(expanded.contains("▾ 6 tool calls"), "{expanded}"); + view.handle_key(KeyCode::Enter.into(), &cells); + let collapsed = screen(&mut view); + assert_eq!(collapsed, initial); + assert!(view.is_activity_focused()); + assert!(!view.is_following()); + assert_eq!(screen(&mut view), collapsed); + insta::assert_snapshot!(collapsed); +} + #[test] fn tool_groups_expand_independently_inside_a_completed_turn() { use crate::exec_cell::CommandOutput;