mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
fix(tui): streamed table resize corruption and history reflow robustness
This commit is contained in:
committed by
Felipe Coury
parent
edc5008dd0
commit
e64f777765
+13
-3
@@ -3277,6 +3277,7 @@ impl App {
|
||||
self.overlay = None;
|
||||
self.transcript_cells.clear();
|
||||
self.deferred_history_lines.clear();
|
||||
tui.clear_pending_history_lines();
|
||||
self.has_emitted_history_lines = false;
|
||||
self.last_transcript_render_width = None;
|
||||
self.resize_reflow_pending_until = None;
|
||||
@@ -3330,9 +3331,10 @@ impl App {
|
||||
size: ratatui::layout::Size,
|
||||
last_known_screen_size: ratatui::layout::Size,
|
||||
frame_requester: &tui::FrameRequester,
|
||||
) {
|
||||
) -> bool {
|
||||
let previous_width = self.last_transcript_render_width.replace(size.width);
|
||||
if previous_width.is_some_and(|width| width != size.width) {
|
||||
let width_changed = previous_width.is_some_and(|width| width != size.width);
|
||||
if width_changed {
|
||||
self.chat_widget.on_terminal_resize(size.width);
|
||||
self.schedule_resize_reflow();
|
||||
frame_requester.schedule_frame_in(RESIZE_REFLOW_DEBOUNCE);
|
||||
@@ -3343,6 +3345,7 @@ impl App {
|
||||
self.refresh_status_line();
|
||||
}
|
||||
self.maybe_clear_resize_reflow_without_terminal();
|
||||
width_changed
|
||||
}
|
||||
|
||||
fn maybe_clear_resize_reflow_without_terminal(&mut self) {
|
||||
@@ -3361,11 +3364,16 @@ impl App {
|
||||
|
||||
fn handle_draw_pre_render(&mut self, tui: &mut tui::Tui) -> Result<()> {
|
||||
let size = tui.terminal.size()?;
|
||||
self.handle_draw_size_change(
|
||||
let width_changed = self.handle_draw_size_change(
|
||||
size,
|
||||
tui.terminal.last_known_screen_size,
|
||||
&tui.frame_requester(),
|
||||
);
|
||||
if width_changed {
|
||||
// Width-sensitive history inserts queued before this frame may be wrapped for the old
|
||||
// viewport. Drop them and let resize reflow rebuild from transcript cells.
|
||||
tui.clear_pending_history_lines();
|
||||
}
|
||||
self.maybe_run_resize_reflow(tui)?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -3379,6 +3387,8 @@ impl App {
|
||||
}
|
||||
|
||||
self.resize_reflow_pending_until = None;
|
||||
// Drop any queued pre-resize inserts before rebuilding at the new width.
|
||||
tui.clear_pending_history_lines();
|
||||
if self.transcript_cells.is_empty() {
|
||||
self.has_emitted_history_lines = false;
|
||||
self.deferred_history_lines.clear();
|
||||
|
||||
@@ -1127,6 +1127,94 @@ async fn interrupt_drops_stream_deltas_until_turn_aborted() {
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn interrupt_remains_responsive_during_resized_table_stream() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.last_rendered_width.set(Some(120));
|
||||
chat.handle_codex_event(Event {
|
||||
id: "turn-resize".into(),
|
||||
msg: EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-resize".to_string(),
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: ModeKind::Default,
|
||||
}),
|
||||
});
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "table-head".into(),
|
||||
msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent {
|
||||
delta:
|
||||
"| Row | Initiative Summary | Extended Notes | URL |\n| --- | --- | --- | --- |\n"
|
||||
.to_string(),
|
||||
}),
|
||||
});
|
||||
|
||||
for idx in 0..40 {
|
||||
chat.handle_codex_event(Event {
|
||||
id: format!("table-row-{idx}"),
|
||||
msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent {
|
||||
delta: format!(
|
||||
"| {idx:03} | Workstream {idx:03} provides a long narrative about scope boundaries, sequencing assumptions, contingency paths, stakeholder dependencies, and quality criteria to keep complex coordination readable under pressure. | Record {idx:03} stores extended execution commentary including risk signals, approvals, rollback conditions, evidence links, and checkpoint outcomes so auditors and new contributors can understand context without reopening old threads. | https://example.com/program/workstream-{idx:03}-detailed-governance-and-delivery-context |\n",
|
||||
),
|
||||
}),
|
||||
});
|
||||
chat.on_terminal_resize(if idx % 2 == 0 { 72 } else { 116 });
|
||||
chat.on_commit_tick();
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
}
|
||||
|
||||
assert!(
|
||||
chat.stream_controller.is_some(),
|
||||
"expected active stream during table tail stress",
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL));
|
||||
let mut saw_interrupt = false;
|
||||
while let Ok(op) = op_rx.try_recv() {
|
||||
if matches!(op, Op::Interrupt) {
|
||||
saw_interrupt = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(saw_interrupt, "expected Ctrl+C to submit Op::Interrupt");
|
||||
|
||||
chat.on_terminal_resize(64);
|
||||
let resized_tail = {
|
||||
let controller = chat
|
||||
.stream_controller
|
||||
.as_ref()
|
||||
.expect("expected stream controller after resize");
|
||||
lines_to_single_string(&controller.current_tail_lines())
|
||||
};
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "table-row-stale-after-interrupt".into(),
|
||||
msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent {
|
||||
delta: "| 999 | INTERRUPT_STALE_SENTINEL | INTERRUPT_STALE_SENTINEL | https://example.com/stale |\n"
|
||||
.to_string(),
|
||||
}),
|
||||
});
|
||||
|
||||
let tail_after_stale_delta = {
|
||||
let controller = chat
|
||||
.stream_controller
|
||||
.as_ref()
|
||||
.expect("expected stream controller after stale delta");
|
||||
lines_to_single_string(&controller.current_tail_lines())
|
||||
};
|
||||
assert_eq!(
|
||||
tail_after_stale_delta, resized_tail,
|
||||
"expected stale table delta to be dropped while interrupt is pending",
|
||||
);
|
||||
assert!(
|
||||
!tail_after_stale_delta.contains("INTERRUPT_STALE_SENTINEL"),
|
||||
"stale sentinel should never reach the active stream tail",
|
||||
);
|
||||
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn interrupt_prepends_queued_messages_before_existing_composer_text() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
@@ -79,6 +79,7 @@ use ratatui::style::Modifier;
|
||||
use ratatui::style::Style;
|
||||
use ratatui::style::Styled;
|
||||
use ratatui::style::Stylize;
|
||||
use ratatui::widgets::Clear;
|
||||
use ratatui::widgets::Paragraph;
|
||||
use ratatui::widgets::Wrap;
|
||||
use std::any::Any;
|
||||
@@ -190,6 +191,9 @@ impl Renderable for Box<dyn HistoryCell> {
|
||||
.saturating_sub(usize::from(area.height));
|
||||
u16::try_from(overflow).unwrap_or(u16::MAX)
|
||||
};
|
||||
// Active-cell content can reflow dramatically during resize/stream updates. Clear the
|
||||
// entire draw area first so stale glyphs from previous frames never linger.
|
||||
Clear.render(area, buf);
|
||||
paragraph.scroll((y, 0)).render(area, buf);
|
||||
}
|
||||
fn desired_height(&self, width: u16) -> u16 {
|
||||
@@ -2875,6 +2879,8 @@ mod tests {
|
||||
use codex_protocol::protocol::SessionConfiguredEvent;
|
||||
use dirs::home_dir;
|
||||
use pretty_assertions::assert_eq;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use serde_json::json;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
@@ -4855,6 +4861,44 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_clears_area_when_cell_content_shrinks() {
|
||||
let area = Rect::new(0, 0, 40, 6);
|
||||
let mut buf = Buffer::empty(area);
|
||||
|
||||
let first: Box<dyn HistoryCell> = Box::new(PlainHistoryCell::new(vec![
|
||||
Line::from("STALE ROW 1"),
|
||||
Line::from("STALE ROW 2"),
|
||||
Line::from("STALE ROW 3"),
|
||||
Line::from("STALE ROW 4"),
|
||||
]));
|
||||
first.render(area, &mut buf);
|
||||
|
||||
let second: Box<dyn HistoryCell> =
|
||||
Box::new(PlainHistoryCell::new(vec![Line::from("fresh")]));
|
||||
second.render(area, &mut buf);
|
||||
|
||||
let mut rendered_rows: Vec<String> = Vec::new();
|
||||
for y in 0..area.height {
|
||||
let mut row = String::new();
|
||||
for x in 0..area.width {
|
||||
row.push_str(buf.cell((x, y)).expect("cell should exist").symbol());
|
||||
}
|
||||
rendered_rows.push(row);
|
||||
}
|
||||
|
||||
assert!(
|
||||
rendered_rows.iter().all(|row| !row.contains("STALE")),
|
||||
"rendered buffer should not retain stale glyphs: {rendered_rows:?}",
|
||||
);
|
||||
assert!(
|
||||
rendered_rows
|
||||
.first()
|
||||
.is_some_and(|row| row.contains("fresh")),
|
||||
"expected fresh content in first row: {rendered_rows:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_markdown_cell_survives_insert_history_rewrap() {
|
||||
let source = "\
|
||||
|
||||
@@ -185,8 +185,15 @@ where
|
||||
// fetch/restore the cursor position. insert_history_lines should be cursor-position-neutral :)
|
||||
queue!(writer, MoveTo(0, cursor_top))?;
|
||||
|
||||
let scroll_bottom = area.top().saturating_sub(1);
|
||||
let mut advance_row = cursor_top;
|
||||
for line in &wrapped {
|
||||
queue!(writer, Print("\r\n"))?;
|
||||
// Explicitly anchor before each line advance to avoid terminal wrap-pending drift when
|
||||
// prior content reached the right edge.
|
||||
queue!(writer, MoveTo(0, advance_row), Print("\n"))?;
|
||||
if advance_row < scroll_bottom {
|
||||
advance_row += 1;
|
||||
}
|
||||
write_history_line(writer, line, wrap_width)?;
|
||||
}
|
||||
|
||||
@@ -821,4 +828,58 @@ mod tests {
|
||||
assert_eq!(term.viewport_area, Rect::new(0, 5, width, 2));
|
||||
assert_eq!(term.visible_history_rows(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn vt100_exact_width_rows_keep_stable_line_progression() {
|
||||
let width: u16 = 10;
|
||||
let height: u16 = 7;
|
||||
let backend = VT100Backend::new(width, height);
|
||||
let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal");
|
||||
let viewport = Rect::new(0, height - 1, width, 1);
|
||||
term.set_viewport_area(viewport);
|
||||
|
||||
let lines = vec![
|
||||
Line::from("1234567890"),
|
||||
Line::from("abcdefghij"),
|
||||
Line::from("KLMNOPQRST"),
|
||||
];
|
||||
insert_history_lines(&mut term, lines).expect("insert_history_lines should succeed");
|
||||
|
||||
let screen = term.backend().vt100().screen();
|
||||
let mut non_empty_rows: Vec<(u16, String)> = Vec::new();
|
||||
for row in 0..height.saturating_sub(1) {
|
||||
let row_text = (0..width)
|
||||
.map(|col| {
|
||||
screen
|
||||
.cell(row, col)
|
||||
.map(|cell| cell.contents().to_string())
|
||||
.unwrap_or_default()
|
||||
})
|
||||
.collect::<String>();
|
||||
if !row_text.trim().is_empty() {
|
||||
non_empty_rows.push((row, row_text));
|
||||
}
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
non_empty_rows.len(),
|
||||
3,
|
||||
"expected exactly three populated rows, got {non_empty_rows:?}",
|
||||
);
|
||||
|
||||
let expected = ["1234567890", "abcdefghij", "KLMNOPQRST"];
|
||||
for (idx, (row, row_text)) in non_empty_rows.iter().enumerate() {
|
||||
assert_eq!(
|
||||
row_text, expected[idx],
|
||||
"unexpected row content at y={row}: {row_text:?}",
|
||||
);
|
||||
}
|
||||
for pair in non_empty_rows.windows(2) {
|
||||
assert_eq!(
|
||||
pair[1].0,
|
||||
pair[0].0 + 1,
|
||||
"expected contiguous row progression, got {non_empty_rows:?}",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -449,6 +449,7 @@ impl Tui {
|
||||
self.frame_requester().schedule_frame();
|
||||
}
|
||||
|
||||
/// Drop any queued history lines that have not yet been flushed to the terminal.
|
||||
pub fn clear_pending_history_lines(&mut self) {
|
||||
self.pending_history_lines.clear();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user