Clear inline image previews before history insertion can scroll them (#61196)

## Why

Inline image previews can leave stale pixels in terminal history when streamed output scrolls their rows.

## What changed

Clear retained previews before inserting history that could move them, then let the next frame redraw them while preserving the draft text. Keep direct Kitty previews when standard history insertion scrolls only above an unchanged, bottom-aligned viewport.

## Testing

Add regression coverage for clearing previews before scrolling, preserving unsent draft text, redrawing file-backed previews, and retaining or repainting direct Kitty previews as appropriate.

GitOrigin-RevId: 683d1313349b1ac707499b021092fc390e21530d

codex-rs/tui/src/tui.rs

@@ -48,6 +48,7 @@ use self::input_boundary::terminal_input_is_readable;
 use crate::custom_terminal;
 use crate::custom_terminal::Terminal as CustomTerminal;
 use crate::insert_history::HistoryLineWrapPolicy;
+use crate::insert_history::InsertHistoryMode;
 use crate::notifications::DesktopNotificationBackend;
 use crate::notifications::detect_backend;
 use crate::terminal_hyperlinks::HyperlinkLine;
@@ -73,6 +74,9 @@ mod job_control;
 mod keyboard_modes;
 mod link_pointer;
 pub(crate) use link_pointer::PointerShape;
+#[cfg(test)]
+#[path = "tui/inline_image_history_tests.rs"]
+mod inline_image_history_tests;
 mod notification_preview;
 #[cfg(test)]
 #[path = "tui/owned_screen_tests.rs"]
@@ -1143,6 +1147,7 @@ impl Tui {
         Self::flush_pending_history_lines(
             &mut self.terminal,
             &mut self.pending_history_lines,
+            &mut self.retained_images,
             self.scrollback,
             screen_size,
         )?;
@@ -1325,22 +1330,38 @@ impl Tui {
     }
 
     /// Write any buffered history lines above the viewport and clear the buffer.
-    fn flush_pending_history_lines(
-        terminal: &mut Terminal,
+    fn flush_pending_history_lines<B>(
+        terminal: &mut CustomTerminal<B>,
         pending_history_lines: &mut Vec<PendingHistoryLines>,
+        retained_images: &mut Vec<crate::transcript_image::ImagePlacement>,
         scrollback: ScrollbackStrategy,
         screen_size: Size,
-    ) -> Result<()> {
-        if pending_history_lines.is_empty() {
-            return Ok(());
-        }
-
-        // A failed write can already have emitted part of this batch. Never retry that batch
-        // automatically: replaying it would duplicate the successful prefix. Remaining batches
-        // stay queued, and the caller reports the I/O error.
-        while !pending_history_lines.is_empty() {
-            let batch = pending_history_lines.remove(/*index*/ 0);
+    ) -> Result<()>
+    where
+        B: Backend<Error = io::Error> + Write,
+    {
+        while let Some(batch) = pending_history_lines.first() {
             let mode = scrollback.history_insertion_mode(batch.wrap_policy);
+            let viewport = terminal.viewport_area;
+            // Standard insertion scrolls only above a bottom-aligned viewport when there
+            // are at least two history rows (the minimum for a DEC scroll region).
+            let preserves_direct_kitty = mode == InsertHistoryMode::Standard
+                && viewport.top() > 1
+                && viewport.bottom() == screen_size.height
+                && screen_size == terminal.last_known_screen_size
+                && terminal.tmux_kitty_image_base.is_none();
+            if retained_images.iter().any(|placement| {
+                !preserves_direct_kitty
+                    || placement.image.protocol != crate::pets::ImageProtocol::Kitty
+                    || placement.area.intersection(viewport) != placement.area
+            }) {
+                // iTerm2's file-backed images and Sixel pixels can scroll with history.
+                terminal.clear_frame_images(retained_images)?;
+            }
+
+            // Once insertion starts, an I/O error can leave a prefix on screen. Dequeue
+            // this batch first so retrying the remaining batches cannot duplicate that prefix.
+            let batch = pending_history_lines.remove(/*index*/ 0);
             crate::insert_history::insert_history_hyperlink_lines_with_mode_and_wrap_policy(
                 terminal,
                 &batch.lines,
@@ -1469,6 +1490,7 @@ impl Tui {
             Self::flush_pending_history_lines(
                 terminal,
                 &mut self.pending_history_lines,
+                &mut self.retained_images,
                 self.scrollback,
                 screen_size,
             )?;
@@ -1607,6 +1629,7 @@ impl Tui {
             Self::flush_pending_history_lines(
                 terminal,
                 &mut self.pending_history_lines,
+                &mut self.retained_images,
                 self.scrollback,
                 screen_size,
             )?;

codex-rs/tui/src/tui/history_tail.rs

@@ -32,6 +32,7 @@ impl Tui {
         Self::flush_pending_history_lines(
             &mut self.terminal,
             &mut self.pending_history_lines,
+            &mut self.retained_images,
             self.scrollback,
             screen_size,
         )?;

codex-rs/tui/src/tui/inline_image_history_tests.rs

@@ -0,0 +1,241 @@
+//! Inline previews must be erased before their rows can enter terminal history.
+#![cfg(test)]
+
+use super::PendingHistoryLines;
+use super::ScrollbackStrategy;
+use super::Tui;
+use crate::custom_terminal::Terminal;
+use crate::insert_history::HistoryLineWrapPolicy;
+use crate::pets::ImageProtocol;
+use crate::terminal_hyperlinks::plain_hyperlink_lines;
+use crate::transcript_image::ImagePlacement;
+use crate::transcript_image::TranscriptImage;
+use crossterm::cursor::MoveTo;
+use crossterm::queue;
+use crossterm::style::Print;
+use pretty_assertions::assert_eq;
+use ratatui::backend::CrosstermBackend;
+use ratatui::layout::Position;
+use ratatui::layout::Rect;
+use ratatui::layout::Size;
+use ratatui::text::Line;
+use ratatui::widgets::Widget;
+use std::sync::Arc;
+
+fn inline_terminal(viewport: Rect) -> Terminal<CrosstermBackend<Vec<u8>>> {
+    let mut terminal = Terminal::with_screen_size_and_cursor_position_for_test(
+        CrosstermBackend::new(Vec::new()),
+        Size::new(/*width*/ 32, /*height*/ 8),
+        Position::new(/*x*/ 0, /*y*/ 0),
+    );
+    terminal.set_viewport_area(viewport);
+    terminal
+}
+
+fn preview_at(protocol: ImageProtocol, y: u16) -> ImagePlacement {
+    ImagePlacement {
+        image: Arc::new(TranscriptImage {
+            protocol,
+            columns: 2,
+            rows: 1,
+            pixels: image::RgbaImage::from_pixel(
+                /*width*/ 16,
+                /*height*/ 16,
+                image::Rgba([0, 100, 200, 255]),
+            ),
+            cell_width: 8,
+            cell_height: 16,
+        }),
+        area: Rect::new(/*x*/ 0, y, /*width*/ 2, /*height*/ 1),
+        first_row: 0,
+        file: None,
+    }
+}
+
+#[test]
+fn inline_history_erases_preview_before_scrolling_and_preserves_draft() {
+    let viewport = Rect::new(
+        /*x*/ 0, /*y*/ 4, /*width*/ 32, /*height*/ 4,
+    );
+    let mut terminal = inline_terminal(viewport);
+    let size = terminal.last_known_screen_size;
+    terminal
+        .draw_with_size(size, &mut Vec::new(), |frame| {
+            Line::from("unsent [Image #1]").render(
+                Rect::new(
+                    /*x*/ 0, /*y*/ 7, /*width*/ 32, /*height*/ 1,
+                ),
+                frame.buffer_mut(),
+            );
+        })
+        .unwrap();
+    // vt100 ignores Sixel. Model its out-of-band pixels with a marker written directly
+    // to the terminal, without changing the renderer's text buffers.
+    queue!(
+        terminal.backend_mut(),
+        MoveTo(/*x*/ 0, /*y*/ 4),
+        Print("stale preview pixels")
+    )
+    .unwrap();
+    let mut retained = vec![preview_at(ImageProtocol::Sixel, viewport.top())];
+    // Unrelated frames must not evict an otherwise unchanged preview.
+    Tui::flush_pending_history_lines(
+        &mut terminal,
+        &mut Vec::new(),
+        &mut retained,
+        ScrollbackStrategy::Standard,
+        size,
+    )
+    .unwrap();
+    assert_eq!(retained.len(), 1);
+    let preview = retained[0].registration();
+    let before_flush = terminal.backend().writer().len();
+    let mut history = vec![PendingHistoryLines {
+        lines: plain_hyperlink_lines(
+            (1..=12)
+                .map(|index| Line::from(format!("streamed response {index}")))
+                .collect(),
+        ),
+        wrap_policy: HistoryLineWrapPolicy::PreWrap,
+    }];
+    Tui::flush_pending_history_lines(
+        &mut terminal,
+        &mut history,
+        &mut retained,
+        ScrollbackStrategy::Standard,
+        size,
+    )
+    .unwrap();
+    assert!(retained.is_empty());
+    assert!(history.is_empty());
+    let output = terminal.backend().writer();
+    let flush = String::from_utf8_lossy(&output[before_flush..]);
+    let erase = flush.find("\x1b[K").expect("erase preview row");
+    let history_write = flush.find("\x1b[1;4r").expect("history scroll region");
+    assert!(
+        erase < history_write,
+        "preview must be erased before history: {flush:?}"
+    );
+    let mut parser = vt100::Parser::new(
+        /*rows*/ 8, /*cols*/ 32, /*scrollback_len*/ 32,
+    );
+    parser.process(output);
+    let contents = parser.screen().contents();
+    assert!(!contents.contains("stale preview"), "{contents}");
+    assert!(contents.contains("streamed response"), "{contents}");
+    assert!(contents.contains("unsent [Image #1]"), "{contents}");
+    insta::assert_snapshot!(contents);
+    terminal
+        .draw_with_size(size, &mut retained, |frame| {
+            Line::from("unsent [Image #1]").render(
+                Rect::new(
+                    /*x*/ 0, /*y*/ 7, /*width*/ 32, /*height*/ 1,
+                ),
+                frame.buffer_mut(),
+            );
+            frame.image_placements.push(preview);
+        })
+        .unwrap();
+    assert_eq!(retained.len(), 1);
+    assert!(String::from_utf8_lossy(terminal.backend().writer()).contains("\x1bP"));
+}
+
+/// Capture history output and the following frame separately from the initial Kitty upload.
+fn kitty_history_output(
+    protocol: ImageProtocol,
+    viewport: Rect,
+    scrollback: ScrollbackStrategy,
+) -> (String, String) {
+    let mut terminal = inline_terminal(viewport);
+    let size = terminal.last_known_screen_size;
+    let preview = preview_at(protocol, viewport.top());
+    let draw = |frame: &mut crate::custom_terminal::Frame<'_>| {
+        let mut placement = preview.registration();
+        placement.area.y = frame.area().top();
+        frame.image_placements.push(placement);
+    };
+    let mut retained = Vec::new();
+    terminal.draw_with_size(size, &mut retained, draw).unwrap();
+    let output = terminal.backend().writer();
+    assert!(String::from_utf8_lossy(output).contains("_Ga=T,"));
+    let before_flush = output.len();
+    let mut history = vec![PendingHistoryLines {
+        lines: plain_hyperlink_lines(vec![Line::from("streamed response")]),
+        wrap_policy: HistoryLineWrapPolicy::PreWrap,
+    }];
+    Tui::flush_pending_history_lines(&mut terminal, &mut history, &mut retained, scrollback, size)
+        .unwrap();
+    let before_draw = terminal.backend().writer().len();
+    terminal.draw_with_size(size, &mut retained, draw).unwrap();
+    let output = terminal.backend().writer();
+    (
+        String::from_utf8_lossy(&output[before_flush..before_draw]).into_owned(),
+        String::from_utf8_lossy(&output[before_draw..]).into_owned(),
+    )
+}
+
+#[test]
+fn direct_kitty_preview_survives_history_above_the_viewport() {
+    let viewport = Rect::new(
+        /*x*/ 0, /*y*/ 4, /*width*/ 32, /*height*/ 4,
+    );
+    let (flush, redraw) =
+        kitty_history_output(ImageProtocol::Kitty, viewport, ScrollbackStrategy::Standard);
+    assert!(flush.contains("streamed response"));
+    assert!(!flush.contains("_G"), "{flush:?}");
+    assert!(!redraw.contains("_G"), "{redraw:?}");
+}
+
+#[test]
+fn iterm2_file_preview_is_deleted_before_history_and_uploaded_on_the_next_draw() {
+    let viewport = Rect::new(
+        /*x*/ 0, /*y*/ 4, /*width*/ 32, /*height*/ 4,
+    );
+    let (flush, redraw) = kitty_history_output(
+        ImageProtocol::KittyLocalFile,
+        viewport,
+        ScrollbackStrategy::Standard,
+    );
+    let (before_history, _) = flush
+        .split_once("\x1b[1;4r")
+        .expect("history scroll region");
+    let delete = crate::pets::kitty_delete_image(crate::transcript_image::FIRST_IMAGE_ID);
+    assert!(before_history.contains(&delete), "{flush:?}");
+    assert!(!flush.contains("_Ga=T,"), "{flush:?}");
+    assert!(redraw.contains("_Ga=T,t=f,"), "{redraw:?}");
+}
+
+#[test]
+fn direct_kitty_preview_is_repainted_when_history_can_move_the_viewport() {
+    for (viewport, scrollback) in [
+        (
+            Rect::new(
+                /*x*/ 0, /*y*/ 2, /*width*/ 32, /*height*/ 4,
+            ),
+            ScrollbackStrategy::Standard,
+        ),
+        (
+            Rect::new(
+                /*x*/ 0, /*y*/ 1, /*width*/ 32, /*height*/ 7,
+            ),
+            ScrollbackStrategy::Standard,
+        ),
+        (
+            Rect::new(
+                /*x*/ 0, /*y*/ 4, /*width*/ 32, /*height*/ 4,
+            ),
+            ScrollbackStrategy::FullScreen,
+        ),
+    ] {
+        let (flush, redraw) = kitty_history_output(ImageProtocol::Kitty, viewport, scrollback);
+        let (before_history, _) = flush.split_once("streamed response").expect("history line");
+        assert!(
+            before_history.contains("_Ga=d,"),
+            "{viewport:?} {scrollback:?}: {flush:?}"
+        );
+        assert!(
+            redraw.contains("_Ga=T,t=d,"),
+            "{viewport:?} {scrollback:?}: {redraw:?}"
+        );
+    }
+}

codex-rs/tui/src/tui/snapshots/codex_tui__tui__inline_image_history_tests__inline_history_erases_preview_before_scrolling_and_preserves_draft.snap

@@ -0,0 +1,12 @@
+---
+source: tui/src/tui/inline_image_history_tests.rs
+expression: contents
+---
+streamed response 9
+streamed response 10
+streamed response 11
+streamed response 12
+ 
+ 
+ 
+unsent [Image #1]