From 050aa077b58da7d7e7e6f92d60ce1a6ca3ad5528 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 17 Aug 2026 22:51:09 +0000 Subject: [PATCH] Avoid redundant terminal size queries during history insertion (#39100) ## What changed - Pass the screen size already available to TUI draw and history-tail paths into history insertion. - Use the terminal's cached screen size for direct history insertion calls instead of querying the backend again. - Extend the terminal size-query regression test to cover history insertion. GitOrigin-RevId: 44c7a0f0bc5bc365d8d0c72d7e26587ed17a4821 --- codex-rs/tui/src/custom_terminal.rs | 6 ++++++ codex-rs/tui/src/insert_history.rs | 7 +++++-- codex-rs/tui/src/tui.rs | 4 ++++ codex-rs/tui/src/tui/history_tail.rs | 4 ++++ codex-rs/tui/src/tui/history_tail_tests.rs | 2 ++ 5 files changed, 21 insertions(+), 2 deletions(-) diff --git a/codex-rs/tui/src/custom_terminal.rs b/codex-rs/tui/src/custom_terminal.rs index 089ec9ff16..c7a91e78d7 100644 --- a/codex-rs/tui/src/custom_terminal.rs +++ b/codex-rs/tui/src/custom_terminal.rs @@ -1011,6 +1011,12 @@ mod tests { terminal.draw_with_size(screen_size, |_| {}).expect("draw"); } + terminal.set_viewport_area(Rect::new( + /*x*/ 0, /*y*/ 23, /*width*/ 80, /*height*/ 1, + )); + crate::insert_history::insert_history_lines(&mut terminal, vec![Line::from("history")]) + .expect("insert history"); + assert_eq!(terminal.backend().size_call_count.get(), 1); } diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 5d04ed063d..a16993a48d 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -94,11 +94,13 @@ pub(crate) fn insert_history_lines_with_mode_and_wrap_policy( where B: Backend + Write, { + let screen_size = terminal.last_known_screen_size; insert_history_hyperlink_lines_with_mode_and_wrap_policy( terminal, &plain_hyperlink_lines(lines.iter().map(line_to_static).collect()), mode, wrap_policy, + screen_size, ) } @@ -107,12 +109,11 @@ pub(crate) fn insert_history_hyperlink_lines_with_mode_and_wrap_policy( lines: &[HyperlinkLine], mode: InsertHistoryMode, wrap_policy: HistoryLineWrapPolicy, + screen_size: Size, ) -> io::Result<()> where B: Backend + Write, { - let screen_size = terminal.backend().size().unwrap_or(Size::new(0, 0)); - let mut area = terminal.viewport_area; let mut should_update_area = false; let last_cursor_pos = terminal.last_known_cursor_pos; @@ -832,11 +833,13 @@ mod tests { .map(|line| line.style(ratatui::style::Style::default().bg(Color::Blue))) .collect::>(); + let screen_size = term.last_known_screen_size; insert_history_hyperlink_lines_with_mode_and_wrap_policy( &mut term, &lines, InsertHistoryMode::Standard, HistoryLineWrapPolicy::PreWrap, + screen_size, ) .expect("insert wrapped user message"); diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 639f88fa9b..ad762ab7d1 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -932,6 +932,7 @@ impl Tui { terminal: &mut Terminal, pending_history_lines: &mut Vec, is_zellij: bool, + screen_size: Size, ) -> Result<()> { if pending_history_lines.is_empty() { return Ok(()); @@ -948,6 +949,7 @@ impl Tui { &batch.lines, mode, batch.wrap_policy, + screen_size, )?; } pending_history_lines.clear(); @@ -1006,6 +1008,7 @@ impl Tui { terminal, &mut self.pending_history_lines, self.is_zellij, + screen_size, )?; // Update the y position for suspending so Ctrl-Z can place the cursor correctly. @@ -1121,6 +1124,7 @@ impl Tui { terminal, &mut self.pending_history_lines, self.is_zellij, + screen_size, )?; if needs_full_repaint || history_can_overlap_viewport { diff --git a/codex-rs/tui/src/tui/history_tail.rs b/codex-rs/tui/src/tui/history_tail.rs index 6b018eec84..f97ba5159b 100644 --- a/codex-rs/tui/src/tui/history_tail.rs +++ b/codex-rs/tui/src/tui/history_tail.rs @@ -27,10 +27,12 @@ impl Tui { replacement: &[HyperlinkLine], wrap_policy: HistoryLineWrapPolicy, ) -> io::Result { + let screen_size = self.terminal.last_known_screen_size; Self::flush_pending_history_lines( &mut self.terminal, &mut self.pending_history_lines, self.is_zellij, + screen_size, )?; let mode = if self.is_zellij && wrap_policy == HistoryLineWrapPolicy::Terminal { InsertHistoryMode::ZellijRaw @@ -61,6 +63,7 @@ fn replace_visible_terminal_history_tail( where B: Backend + Write, { + let screen_size = terminal.last_known_screen_size; let mut viewport = terminal.viewport_area; let wrap_width = usize::from(viewport.width.max(/*other*/ 1)); let (_, previous_rows) = wrap_history_hyperlink_lines(previous_lines, wrap_width, wrap_policy); @@ -80,6 +83,7 @@ where replacement, mode, wrap_policy, + screen_size, )?; Ok(true) } diff --git a/codex-rs/tui/src/tui/history_tail_tests.rs b/codex-rs/tui/src/tui/history_tail_tests.rs index 4288a880c0..a02621ca2d 100644 --- a/codex-rs/tui/src/tui/history_tail_tests.rs +++ b/codex-rs/tui/src/tui/history_tail_tests.rs @@ -118,11 +118,13 @@ fn replacing_soft_wrapped_history_counts_physical_terminal_rows() { /*height*/ 2, )); let previous_lines = plain_hyperlink_lines(vec![Line::from("old-long-line-spanning-two-rows")]); + let screen_size = terminal.last_known_screen_size; crate::insert_history::insert_history_hyperlink_lines_with_mode_and_wrap_policy( &mut terminal, &previous_lines, InsertHistoryMode::Standard, HistoryLineWrapPolicy::Terminal, + screen_size, ) .expect("insert soft-wrapped history"); let replacement = plain_hyperlink_lines(vec![Line::from("new billing")]);