mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Fully repaint inline viewports after history overlap (#37335)
## Why During inline-mode resize reflow, replayed history can overlap a zero- or one-row history region and leave stale terminal characters visible in the composer. Resetting the previous render buffer is insufficient because default-style spaces may still compare equal and be skipped. ## What changed - Mark every cell in an invalidated viewport for unconditional repainting. - Invalidate the viewport after flushing pending history when history can overlap it. ## Testing Add regression coverage for repainting spaces over stale cells and a manual `tmux` resize smoke test for preserving visible multiline draft text. GitOrigin-RevId: 5a8a480e2fc50c801561de6a4598a3ab8c12b8d6
This commit is contained in:
@@ -498,11 +498,16 @@ where
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Force the next draw pass to repaint the entire viewport by resetting the
|
||||
/// diff buffer. Call this after raw terminal operations that move screen
|
||||
/// content outside ratatui's knowledge.
|
||||
/// Force the next draw pass to repaint the entire viewport after raw terminal
|
||||
/// operations move screen content outside ratatui's knowledge. Resetting the
|
||||
/// diff buffer alone would leave default-style spaces equal to their previous
|
||||
/// cells, allowing stale terminal content to show through those spaces.
|
||||
pub fn invalidate_viewport(&mut self) {
|
||||
self.previous_buffer_mut().reset();
|
||||
let previous_buffer = self.previous_buffer_mut();
|
||||
previous_buffer.reset();
|
||||
for cell in &mut previous_buffer.content {
|
||||
cell.set_diff_option(CellDiffOption::AlwaysUpdate);
|
||||
}
|
||||
}
|
||||
|
||||
/// Clear the entire visible screen (not just the viewport) and force a full redraw.
|
||||
@@ -809,6 +814,7 @@ mod tests {
|
||||
use super::*;
|
||||
use std::num::NonZeroU16;
|
||||
|
||||
use crate::test_backend::VT100Backend;
|
||||
use insta::assert_snapshot;
|
||||
use pretty_assertions::assert_eq;
|
||||
use ratatui::backend::WindowSize;
|
||||
@@ -926,6 +932,48 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalidate_viewport_repaints_default_style_spaces_over_stale_terminal_cells() {
|
||||
let width = 32;
|
||||
let height = 2;
|
||||
let area = Rect::new(/*x*/ 0, /*y*/ 0, width, height);
|
||||
let mut terminal =
|
||||
Terminal::with_options(VT100Backend::new(width, height)).expect("terminal");
|
||||
terminal.set_viewport_area(area);
|
||||
|
||||
// History insertion writes directly to the terminal, leaving cells that are absent from
|
||||
// the diff buffers. Interior default-style spaces must still overwrite those stale cells.
|
||||
write!(
|
||||
terminal.backend_mut(),
|
||||
"probe-08tcleantwords stale\r\nprobe-09xcleanxwords stale"
|
||||
)
|
||||
.expect("prefill terminal");
|
||||
assert!(
|
||||
terminal
|
||||
.backend()
|
||||
.vt100()
|
||||
.screen()
|
||||
.contents()
|
||||
.contains("probe-08tcleantwords")
|
||||
);
|
||||
|
||||
terminal.invalidate_viewport();
|
||||
terminal
|
||||
.draw(|frame| {
|
||||
Paragraph::new(vec![
|
||||
Line::from("probe-08 clean words"),
|
||||
Line::from("probe-09 clean words"),
|
||||
])
|
||||
.render(area, frame.buffer_mut());
|
||||
})
|
||||
.expect("redraw invalidated viewport");
|
||||
|
||||
assert_snapshot!(terminal.backend().vt100().screen().contents(), @r"
|
||||
probe-08 clean words
|
||||
probe-09 clean words
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ordinary_redraws_with_known_size_do_not_query_backend_size() {
|
||||
let mut terminal =
|
||||
|
||||
@@ -1081,13 +1081,17 @@ impl Tui {
|
||||
let terminal = &mut self.terminal;
|
||||
let needs_full_repaint =
|
||||
Self::update_inline_viewport_for_resize_reflow(terminal, height, screen_size)?;
|
||||
// A zero- or one-row history region cannot isolate raw history writes from the
|
||||
// viewport, so replayed rows can leave stale cells inside the composer.
|
||||
let history_can_overlap_viewport =
|
||||
!self.pending_history_lines.is_empty() && terminal.viewport_area.top() <= 1;
|
||||
Self::flush_pending_history_lines(
|
||||
terminal,
|
||||
&mut self.pending_history_lines,
|
||||
self.is_zellij,
|
||||
)?;
|
||||
|
||||
if needs_full_repaint {
|
||||
if needs_full_repaint || history_can_overlap_viewport {
|
||||
terminal.invalidate_viewport();
|
||||
}
|
||||
|
||||
|
||||
@@ -311,6 +311,143 @@ async fn tmux_width_resize_restore_keeps_visible_content_anchored() -> Result<()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[ignore = "requires tmux and a locally built codex binary; run with --ignored for manual resize smoke"]
|
||||
async fn tmux_scrolled_composer_resize_preserves_visible_draft_text() -> Result<()> {
|
||||
if cfg!(windows) {
|
||||
return Ok(());
|
||||
}
|
||||
skip_if_no_network!(Ok(()));
|
||||
if Command::new("tmux").arg("-V").output().is_err() {
|
||||
eprintln!("skipping resize smoke because tmux is unavailable");
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let repo_root = codex_utils_cargo_bin::repo_root()?;
|
||||
let codex = codex_binary(&repo_root)?;
|
||||
let codex_home = tempdir()?;
|
||||
let server = MockServer::start().await;
|
||||
let openai_base_url_config = format!("openai_base_url=\"{}/v1\"", server.uri());
|
||||
write_config(codex_home.path(), &repo_root)?;
|
||||
write_auth(codex_home.path())?;
|
||||
|
||||
let session_name = format!("codex-resize-scrolled-composer-{}", std::process::id());
|
||||
let _session = TmuxSession {
|
||||
name: session_name.clone(),
|
||||
};
|
||||
|
||||
let start_output = checked_output(
|
||||
Command::new("tmux")
|
||||
.arg("new-session")
|
||||
.arg("-d")
|
||||
.arg("-P")
|
||||
.arg("-F")
|
||||
.arg("#{pane_id}")
|
||||
.arg("-x")
|
||||
.arg("44")
|
||||
.arg("-y")
|
||||
.arg("14")
|
||||
.arg("-s")
|
||||
.arg(&session_name)
|
||||
.arg("--")
|
||||
.arg("env")
|
||||
.arg(format!("CODEX_HOME={}", codex_home.path().display()))
|
||||
.arg("OPENAI_API_KEY=dummy")
|
||||
.arg(codex)
|
||||
.arg("--model")
|
||||
.arg("gpt-5.6-terra")
|
||||
.arg("-c")
|
||||
.arg("analytics.enabled=false")
|
||||
.arg("-c")
|
||||
.arg(&openai_base_url_config)
|
||||
.arg("--no-alt-screen")
|
||||
.arg("-C")
|
||||
.arg(&repo_root),
|
||||
)?;
|
||||
let codex_pane = stdout_text(&start_output).trim().to_string();
|
||||
anyhow::ensure!(!codex_pane.is_empty(), "tmux did not report a pane id");
|
||||
wait_for_capture_contains(
|
||||
&codex_pane,
|
||||
"gpt-5.6-terra",
|
||||
Duration::from_secs(/*secs*/ 15),
|
||||
)?;
|
||||
|
||||
for index in 1..=9 {
|
||||
let line = format!("probe-{index:02} clean words");
|
||||
check(
|
||||
Command::new("tmux")
|
||||
.arg("send-keys")
|
||||
.arg("-t")
|
||||
.arg(&codex_pane)
|
||||
.arg("-l")
|
||||
.arg(line),
|
||||
)?;
|
||||
check(
|
||||
Command::new("tmux")
|
||||
.arg("send-keys")
|
||||
.arg("-t")
|
||||
.arg(&codex_pane)
|
||||
.arg("C-j"),
|
||||
)?;
|
||||
}
|
||||
|
||||
let final_line = "q".repeat(/*n*/ 41);
|
||||
check(
|
||||
Command::new("tmux")
|
||||
.arg("send-keys")
|
||||
.arg("-t")
|
||||
.arg(&codex_pane)
|
||||
.arg("-l")
|
||||
.arg(&final_line),
|
||||
)?;
|
||||
let baseline =
|
||||
wait_for_capture_contains(&codex_pane, &final_line, Duration::from_secs(/*secs*/ 15))?;
|
||||
|
||||
for (phase, width, height, minimum_visible_rows) in
|
||||
[("narrowed", "28", "9", 2), ("restored", "44", "14", 6)]
|
||||
{
|
||||
check(
|
||||
Command::new("tmux")
|
||||
.arg("resize-window")
|
||||
.arg("-t")
|
||||
.arg(&session_name)
|
||||
.arg("-x")
|
||||
.arg(width)
|
||||
.arg("-y")
|
||||
.arg(height),
|
||||
)?;
|
||||
sleep(Duration::from_millis(/*millis*/ 350));
|
||||
let capture = capture_pane(&codex_pane)?;
|
||||
let visible_rows = capture
|
||||
.lines()
|
||||
.filter_map(|line| line.find("probe-").map(|start| (line, start)))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
anyhow::ensure!(
|
||||
visible_rows.len() >= minimum_visible_rows,
|
||||
"{phase} composer exposes too few draft rows: expected at least \
|
||||
{minimum_visible_rows}, found {}\nbaseline:\n{baseline}\ncapture:\n{capture}",
|
||||
visible_rows.len()
|
||||
);
|
||||
|
||||
for (line, start) in visible_rows {
|
||||
let draft = &line[start..];
|
||||
let row_number = draft
|
||||
.get(6..8)
|
||||
.with_context(|| format!("malformed draft row after {phase}: {draft}"))?;
|
||||
let expected = format!("probe-{row_number} clean words");
|
||||
let prefix = line[..start].trim();
|
||||
anyhow::ensure!(
|
||||
draft.starts_with(&expected) && (prefix.is_empty() || prefix == "›"),
|
||||
"{phase} resize left stale terminal content in the composer: \
|
||||
expected {expected:?}, found {line:?}\nbaseline:\n{baseline}\ncapture:\n{capture}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn run_repeated_resize_smoke() -> Result<()> {
|
||||
let repo_root = codex_utils_cargo_bin::repo_root()?;
|
||||
let codex = codex_binary(&repo_root)?;
|
||||
|
||||
Reference in New Issue
Block a user