From 5a645f40dddb97409ece44fe2e095f991be0d635 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Wed, 29 Apr 2026 17:57:16 -0300 Subject: [PATCH] fix(tui): reflow scrollback immediately on resize --- codex-rs/tui/src/app/resize_reflow.rs | 18 ++---- codex-rs/tui/src/app/tests.rs | 5 ++ codex-rs/tui/src/transcript_reflow.rs | 88 ++++----------------------- 3 files changed, 22 insertions(+), 89 deletions(-) diff --git a/codex-rs/tui/src/app/resize_reflow.rs b/codex-rs/tui/src/app/resize_reflow.rs index b2702f470f..7bf03c974b 100644 --- a/codex-rs/tui/src/app/resize_reflow.rs +++ b/codex-rs/tui/src/app/resize_reflow.rs @@ -177,11 +177,6 @@ impl App { } } - fn schedule_resize_reflow(&mut self, target_width: Option) -> bool { - debug_assert!(self.terminal_resize_reflow_enabled()); - self.transcript_reflow.schedule_debounced(target_width) - } - fn resize_reflow_max_rows(&self) -> Option { crate::resize_reflow_cap::resize_reflow_max_rows(self.config.terminal_resize_reflow) } @@ -273,12 +268,11 @@ impl App { if reflow_needed && self.should_mark_reflow_as_stream_time() { self.transcript_reflow.mark_resize_requested_during_stream(); } - let target_width = reflow_needed.then_some(size.width); - if self.schedule_resize_reflow(target_width) { - frame_requester.schedule_frame(); - } else { - frame_requester.schedule_frame_in(TRANSCRIPT_REFLOW_DEBOUNCE); - } + // Reflow immediately on the draw that observes the resize. Leaving a debounced + // gap lets the terminal emulator's native scrollback reflow show stale rows under + // Codex's next table repaint, especially in xterm.js-backed split panes. + self.transcript_reflow.schedule_immediate(); + frame_requester.schedule_frame(); } else if !self.terminal_resize_reflow_enabled() && width.changed { self.transcript_reflow.clear(); } @@ -326,7 +320,7 @@ impl App { Ok(()) } - /// Run a pending transcript reflow when its debounce deadline has arrived. + /// Run a pending transcript reflow once it is due. /// /// Reflow is deferred while an overlay is active because the overlay owns the current draw /// surface. Callers must keep using `HistoryCell` source as the rebuild input; attempting to diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index b81e2b4b8b..7eff4dabbb 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -4011,6 +4011,7 @@ async fn initial_replay_buffer_keeps_recent_rows_when_row_cap_present() { async fn height_shrink_schedules_resize_reflow() { let (mut app, _rx, _op_rx) = make_test_app_with_channels().await; enable_terminal_resize_reflow(&mut app); + app.transcript_cells = vec![plain_line_cell("resize source")]; let frame_requester = crate::tui::FrameRequester::test_dummy(); assert!(!app.handle_draw_size_change( @@ -4025,6 +4026,10 @@ async fn height_shrink_schedules_resize_reflow() { &frame_requester, )); assert!(app.transcript_reflow.has_pending_reflow()); + assert!( + app.transcript_reflow + .pending_is_due(std::time::Instant::now()) + ); } fn test_turn(turn_id: &str, status: TurnStatus, items: Vec) -> Turn { diff --git a/codex-rs/tui/src/transcript_reflow.rs b/codex-rs/tui/src/transcript_reflow.rs index 33318a4d0f..ff8984ff0f 100644 --- a/codex-rs/tui/src/transcript_reflow.rs +++ b/codex-rs/tui/src/transcript_reflow.rs @@ -27,7 +27,6 @@ pub(crate) const TRANSCRIPT_REFLOW_DEBOUNCE: Duration = Duration::from_millis(75 pub(crate) struct TranscriptReflowState { last_observed_width: Option, last_reflow_width: Option, - pending_reflow_width: Option, pending_until: Option, ran_during_stream: bool, resize_requested_during_stream: bool, @@ -66,38 +65,17 @@ impl TranscriptReflowState { /// the resize event, so the follow-up draw must be able to request one more reflow even if /// the observed-width tracker already saw that value. pub(crate) fn reflow_needed_for_width(&self, width: u16) -> bool { - self.last_reflow_width != Some(width) && self.pending_reflow_width != Some(width) - } - - /// Schedule a trailing-debounced reflow and return whether it should run immediately. - /// - /// Repeated resize events push the deadline out so dragging a terminal edge rebuilds scrollback - /// at the final observed width rather than at intermediate widths. `target_width` is present - /// only for width-changing rebuilds; height-only exposure still needs a rebuild, but it must not - /// suppress a later width repair for the same draw cycle. - pub(crate) fn schedule_debounced(&mut self, target_width: Option) -> bool { - let now = Instant::now(); - if let Some(target_width) = target_width { - self.pending_reflow_width = Some(target_width); - } - self.pending_until = Some(now + TRANSCRIPT_REFLOW_DEBOUNCE); - false + self.last_reflow_width != Some(width) } /// Schedule an immediate reflow for the next draw opportunity. /// - /// This is used after stream consolidation when waiting for the debounce interval would leave - /// visible terminal-wrapped stream rows in the finalized transcript. + /// This is used for terminal resize and stream consolidation so terminal-owned wrapping is + /// replaced by source-backed transcript rendering without a stale intermediate frame. pub(crate) fn schedule_immediate(&mut self) { - self.pending_reflow_width = None; self.pending_until = Some(Instant::now()); } - #[cfg(test)] - pub(crate) fn set_due_for_test(&mut self) { - self.pending_until = Some(Instant::now() - Duration::from_millis(1)); - } - pub(crate) fn pending_is_due(&self, now: Instant) -> bool { self.pending_until.is_some_and(|deadline| now >= deadline) } @@ -112,14 +90,13 @@ impl TranscriptReflowState { pub(crate) fn clear_pending_reflow(&mut self) { self.pending_until = None; - self.pending_reflow_width = None; } /// Remember the terminal width that actually rebuilt transcript scrollback. /// - /// Resize scheduling is driven by observed widths, but debounced redraws may run before a - /// terminal emulator has settled on its final size. Keeping the rendered width separate avoids - /// confusing "seen during a draw" with "scrollback has been repaired at this width". + /// Resize scheduling is driven by observed widths, but a terminal emulator may settle on its + /// final size after an earlier draw. Keeping the rendered width separate avoids confusing + /// "seen during a draw" with "scrollback has been repaired at this width". pub(crate) fn mark_reflowed_width(&mut self, width: u16) -> bool { self.last_reflow_width.replace(width) != Some(width) } @@ -135,10 +112,9 @@ impl TranscriptReflowState { /// Remember that the terminal width changed while streaming or pre-consolidation cells existed. /// - /// This captures the case where the debounce did not fire before the stream finished. Without + /// This captures the case where stream consolidation finishes after a resize request. Without /// this flag, consolidation could complete without the final source-backed resize repair. - /// Marking the request rather than forcing immediate rendering keeps resize drag behavior - /// debounced while still guaranteeing that finalized stream cells replace transient rows. + /// Marking the request guarantees finalized stream cells replace transient rows. pub(crate) fn mark_resize_requested_during_stream(&mut self) { self.resize_requested_during_stream = true; } @@ -179,32 +155,12 @@ mod tests { use super::*; #[test] - fn schedule_debounced_postpones_existing_reflow() { + fn schedule_immediate_marks_reflow_due_now() { let mut state = TranscriptReflowState::default(); - assert!(!state.schedule_debounced(/*target_width*/ None)); - let first_deadline = state.pending_until().expect("pending reflow"); + state.schedule_immediate(); - std::thread::sleep(Duration::from_millis(1)); - assert!(!state.schedule_debounced(/*target_width*/ None)); - - assert!( - state.pending_until().expect("pending reflow") > first_deadline, - "a later resize should push the debounce deadline out" - ); - } - - #[test] - fn schedule_debounced_postpones_due_existing_reflow() { - let mut state = TranscriptReflowState::default(); - state.set_due_for_test(); - let before_reschedule = Instant::now(); - - assert!(!state.schedule_debounced(/*target_width*/ None)); - assert!( - state.pending_until().expect("pending reflow") > before_reschedule, - "a resize after the old deadline should start a fresh quiet period" - ); + assert!(state.pending_is_due(Instant::now())); } #[test] @@ -240,28 +196,6 @@ mod tests { assert!(state.reflow_needed_for_width(/*width*/ 100)); } - #[test] - fn pending_reflow_target_prevents_repeated_reschedule() { - let mut state = TranscriptReflowState::default(); - state.note_width(/*width*/ 80); - - assert!(state.reflow_needed_for_width(/*width*/ 100)); - state.schedule_debounced(/*target_width*/ Some(100)); - - assert!(!state.reflow_needed_for_width(/*width*/ 100)); - } - - #[test] - fn clear_pending_reflow_allows_same_width_to_be_rescheduled() { - let mut state = TranscriptReflowState::default(); - state.note_width(/*width*/ 80); - state.schedule_debounced(/*target_width*/ Some(100)); - - state.clear_pending_reflow(); - - assert!(state.reflow_needed_for_width(/*width*/ 100)); - } - #[test] fn mark_reflowed_width_reports_unchanged_width() { let mut state = TranscriptReflowState::default();