openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-31 19:28:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(tui): repair streaming markdown table scrollback

Browse Source
This commit is contained in:
Felipe Coury
2026-05-05 17:44:09 -03:00
parent dc1d0133ad
commit 49d046e4cb
5 changed files with 151 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/app/event_dispatch.rs
+19 -7
View File
@@ -218,9 +218,10 @@ impl App {
let end = self.transcript_cells.len();
let start =
trailing_run_start::<history_cell::AgentMessageCell>(&self.transcript_cells);
if start < end {
let consolidated: Arc<dyn HistoryCell> =
Arc::new(history_cell::AgentMarkdownCell::new(source, &cwd));
let replaced_stream_cells = start < end;
let consolidated: Arc<dyn HistoryCell> =
Arc::new(history_cell::AgentMarkdownCell::new(source, &cwd));
if replaced_stream_cells {
self.transcript_cells
.splice(start..end, std::iter::once(consolidated.clone()));
@@ -228,11 +229,20 @@ impl App {
t.consolidate_cells(start..end, consolidated.clone());
tui.frame_requester().schedule_frame();
}
self.maybe_finish_stream_reflow(tui)?;
} else {
self.maybe_finish_stream_reflow(tui)?;
self.transcript_cells.push(consolidated.clone());
if let Some(Overlay::Transcript(t)) = &mut self.overlay {
t.insert_cell(consolidated.clone());
tui.frame_requester().schedule_frame();
}
self.insert_history_cell_lines(
tui,
consolidated.as_ref(),
tui.terminal.last_known_screen_size.width,
);
}
let force_source_backed_reflow = tui.uses_newline_history_insert();
self.maybe_finish_stream_reflow(tui, force_source_backed_reflow)?;
}
AppEvent::ConsolidateProposedPlan(source) => {
if !self.terminal_resize_reflow_enabled() {
@@ -268,7 +278,9 @@ impl App {
tui.terminal.last_known_screen_size.width,
);
self.maybe_finish_stream_reflow(tui)?;
self.maybe_finish_stream_reflow(
tui, /*force_source_backed_reflow*/ false,
)?;
}
}
AppEvent::ApplyThreadRollback { num_turns } => {
codex-rs/tui/src/app/resize_reflow.rs
+9 -2
View File
@@ -239,13 +239,20 @@ impl App {
/// was active or while its transient cells were still present, this method runs an immediate
/// source-backed reflow so terminal scrollback reflects the finalized cell instead of the
/// transient stream rows.
pub(super) fn maybe_finish_stream_reflow(&mut self, tui: &mut tui::Tui) -> Result<()> {
pub(super) fn maybe_finish_stream_reflow(
&mut self,
tui: &mut tui::Tui,
force_source_backed_reflow: bool,
) -> Result<()> {
if !self.terminal_resize_reflow_enabled() {
self.transcript_reflow.clear();
return Ok(());
}
if self.transcript_reflow.take_stream_finish_reflow_needed() {
if self
.transcript_reflow
.take_stream_finish_reflow_needed(force_source_backed_reflow)
{
self.schedule_immediate_resize_reflow(tui);
self.maybe_run_resize_reflow(tui)?;
} else if self.transcript_reflow.pending_is_due(Instant::now()) {
codex-rs/tui/src/insert_history.rs
+18 -17
View File
@@ -38,19 +38,20 @@ use ratatui::text::Span;
/// Selects the terminal escape strategy for inserting history lines above the viewport.
///
/// Standard terminals support `DECSTBM` scroll regions and Reverse Index (`ESC M`),
/// which let us slide existing content down without redrawing it. Zellij silently
/// drops or mishandles those sequences, so `Zellij` mode falls back to emitting
/// newlines at the bottom of the screen and writing lines at absolute positions.
/// which let us slide existing content down without redrawing it. Some terminals
/// or terminal-like surfaces mishandle those sequences for normal scrollback, so
/// `Newline` mode falls back to emitting newlines at the bottom of the screen
/// and writing lines at absolute positions.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum InsertHistoryMode {
Standard,
Zellij,
Newline,
}
impl InsertHistoryMode {
pub fn new(is_zellij: bool) -> Self {
if is_zellij {
Self::Zellij
pub fn new(use_newline_insert: bool) -> Self {
if use_newline_insert {
Self::Newline
} else {
Self::Standard
}
@@ -72,9 +73,9 @@ where
/// Insert `lines` above the viewport, using the escape strategy selected by `mode`.
///
/// In `Standard` mode this manipulates DECSTBM scroll regions to slide existing
/// scrollback down and writes new lines into the freed space. In `Zellij` mode it
/// emits newlines at the screen bottom to create space (since Zellij ignores scroll
/// region escapes) and writes lines at computed absolute positions. Both modes
/// scrollback down and writes new lines into the freed space. In `Newline` mode it
/// emits newlines at the screen bottom to create space and writes lines at computed
/// absolute positions. Both modes
/// update `terminal.viewport_area` so subsequent draw passes know where the
/// viewport moved to. Resize reflow uses the same viewport-aware path after
/// clearing old scrollback.
@@ -124,7 +125,7 @@ where
let wrapped_lines = wrapped_rows as u16;
match mode {
InsertHistoryMode::Zellij => {
InsertHistoryMode::Newline => {
let space_below = screen_size.height.saturating_sub(area.bottom());
let shift_down = wrapped_lines.min(space_below);
let scroll_up_amount = wrapped_lines.saturating_sub(shift_down);
@@ -814,7 +815,7 @@ mod tests {
}
#[test]
fn vt100_zellij_mode_inserts_history_and_updates_viewport() {
fn vt100_newline_mode_inserts_history_and_updates_viewport() {
let width: u16 = 32;
let height: u16 = 8;
let backend = VT100Backend::new(width, height);
@@ -822,9 +823,9 @@ mod tests {
let viewport = Rect::new(/*x*/ 0, /*y*/ 4, width, /*height*/ 2);
term.set_viewport_area(viewport);
let line: Line<'static> = Line::from("zellij history");
insert_history_lines_with_mode(&mut term, vec![line], InsertHistoryMode::Zellij)
.expect("insert zellij history");
let line: Line<'static> = Line::from("newline history");
insert_history_lines_with_mode(&mut term, vec![line], InsertHistoryMode::Newline)
.expect("insert newline history");
let start_row = 0;
let rows: Vec<String> = term
@@ -834,8 +835,8 @@ mod tests {
.rows(start_row, width)
.collect();
assert!(
rows.iter().any(|row| row.contains("zellij history")),
"expected zellij history row in screen output, rows: {rows:?}"
rows.iter().any(|row| row.contains("newline history")),
"expected newline history row in screen output, rows: {rows:?}"
);
assert_eq!(term.viewport_area, Rect::new(0, 5, width, 2));
assert_eq!(term.visible_history_rows(), 1);
codex-rs/tui/src/transcript_reflow.rs
+26 -7
View File
@@ -148,8 +148,13 @@ impl TranscriptReflowState {
/// This is a draining read because each resize-during-stream episode should force at most one
/// post-consolidation repair. Calling it before consolidation would drop the repair request and
/// leave finalized scrollback shaped by transient stream rows.
pub(crate) fn take_stream_finish_reflow_needed(&mut self) -> bool {
let needed = self.ran_during_stream || self.resize_requested_during_stream;
pub(crate) fn take_stream_finish_reflow_needed(
&mut self,
force_source_backed_reflow: bool,
) -> bool {
let needed = self.ran_during_stream
|| self.resize_requested_during_stream
|| force_source_backed_reflow;
self.ran_during_stream = false;
self.resize_requested_during_stream = false;
needed
@@ -276,8 +281,8 @@ mod tests {
let mut state = TranscriptReflowState::default();
state.mark_resize_requested_during_stream();
assert!(state.take_stream_finish_reflow_needed());
assert!(!state.take_stream_finish_reflow_needed());
assert!(state.take_stream_finish_reflow_needed(/*force_source_backed_reflow*/ false));
assert!(!state.take_stream_finish_reflow_needed(/*force_source_backed_reflow*/ false));
}
#[test]
@@ -285,8 +290,22 @@ mod tests {
let mut state = TranscriptReflowState::default();
state.mark_ran_during_stream();
assert!(state.take_stream_finish_reflow_needed());
assert!(!state.take_stream_finish_reflow_needed());
assert!(state.take_stream_finish_reflow_needed(/*force_source_backed_reflow*/ false));
assert!(!state.take_stream_finish_reflow_needed(/*force_source_backed_reflow*/ false));
}
#[test]
fn take_stream_finish_reflow_needed_reflows_when_forced() {
let mut state = TranscriptReflowState::default();
assert!(state.take_stream_finish_reflow_needed(/*force_source_backed_reflow*/ true));
}
#[test]
fn take_stream_finish_reflow_needed_ignores_unforced_finalization() {
let mut state = TranscriptReflowState::default();
assert!(!state.take_stream_finish_reflow_needed(/*force_source_backed_reflow*/ false));
}
#[test]
@@ -297,6 +316,6 @@ mod tests {
state.clear();
assert!(!state.take_stream_finish_reflow_needed());
assert!(!state.take_stream_finish_reflow_needed(/*force_source_backed_reflow*/ false));
}
}
codex-rs/tui/src/tui.rs
+79 -10
View File
@@ -48,6 +48,9 @@ use crate::tui::event_stream::TuiEventStream;
use crate::tui::job_control::SuspendContext;
use codex_config::types::NotificationCondition;
use codex_config::types::NotificationMethod;
use codex_terminal_detection::Multiplexer;
use codex_terminal_detection::TerminalInfo;
use codex_terminal_detection::TerminalName;
mod event_stream;
mod frame_rate_limiter;
@@ -73,10 +76,38 @@ fn should_emit_notification(condition: NotificationCondition, terminal_focused:
}
}
fn insert_history_mode_for_terminal(
terminal_info: &TerminalInfo,
) -> crate::insert_history::InsertHistoryMode {
let use_newline_insert = matches!(terminal_info.multiplexer, Some(Multiplexer::Zellij {}))
|| matches!(
terminal_info.name,
TerminalName::GnomeTerminal
| TerminalName::Vte
| TerminalName::VsCode
| TerminalName::WindowsTerminal
);
crate::insert_history::InsertHistoryMode::new(use_newline_insert)
}
#[cfg(test)]
mod tests {
use super::insert_history_mode_for_terminal;
use super::should_emit_notification;
use codex_config::types::NotificationCondition;
use codex_terminal_detection::Multiplexer;
use codex_terminal_detection::TerminalInfo;
use codex_terminal_detection::TerminalName;
fn terminal_info(name: TerminalName, multiplexer: Option<Multiplexer>) -> TerminalInfo {
TerminalInfo {
name,
term_program: None,
version: None,
term: None,
multiplexer,
}
}
#[test]
fn unfocused_notification_condition_is_suppressed_when_focused() {
@@ -101,6 +132,39 @@ mod tests {
/*terminal_focused*/ false
));
}
#[test]
fn newline_history_terminals_use_newline_history_insert() {
for name in [
TerminalName::GnomeTerminal,
TerminalName::Vte,
TerminalName::VsCode,
TerminalName::WindowsTerminal,
] {
assert_eq!(
insert_history_mode_for_terminal(&terminal_info(name, /*multiplexer*/ None)),
crate::insert_history::InsertHistoryMode::Newline
);
}
assert_eq!(
insert_history_mode_for_terminal(&terminal_info(
TerminalName::Unknown,
Some(Multiplexer::Zellij {})
)),
crate::insert_history::InsertHistoryMode::Newline
);
}
#[test]
fn regular_terminals_keep_standard_history_insert() {
assert_eq!(
insert_history_mode_for_terminal(&terminal_info(
TerminalName::Iterm2,
/*multiplexer*/ None
)),
crate::insert_history::InsertHistoryMode::Standard
);
}
}
pub fn set_modes() -> Result<()> {
@@ -381,6 +445,7 @@ pub struct Tui {
notification_backend: Option<DesktopNotificationBackend>,
notification_condition: NotificationCondition,
is_zellij: bool,
history_insert_mode: crate::insert_history::InsertHistoryMode,
// When false, enter_alt_screen() becomes a no-op (for Zellij scrollback support)
alt_screen_enabled: bool,
}
@@ -397,10 +462,9 @@ impl Tui {
// Cache this to avoid contention with the event reader.
supports_color::on_cached(supports_color::Stream::Stdout);
let _ = crate::terminal_palette::default_colors();
let is_zellij = matches!(
codex_terminal_detection::terminal_info().multiplexer,
Some(codex_terminal_detection::Multiplexer::Zellij {})
);
let terminal_info = codex_terminal_detection::terminal_info();
let is_zellij = matches!(terminal_info.multiplexer, Some(Multiplexer::Zellij {}));
let history_insert_mode = insert_history_mode_for_terminal(&terminal_info);
Self {
frame_requester,
@@ -417,10 +481,15 @@ impl Tui {
notification_backend: Some(detect_backend(NotificationMethod::default())),
notification_condition: NotificationCondition::default(),
is_zellij,
history_insert_mode,
alt_screen_enabled: true,
}
}
pub(crate) fn uses_newline_history_insert(&self) -> bool {
self.history_insert_mode == crate::insert_history::InsertHistoryMode::Newline
}
/// Set whether alternate screen is enabled. When false, enter_alt_screen() becomes a no-op.
pub fn set_alt_screen_enabled(&mut self, enabled: bool) {
self.alt_screen_enabled = enabled;
@@ -694,12 +763,12 @@ impl Tui {
}
/// Write any buffered history lines above the viewport and clear the buffer.
/// Returns `true` when Zellij mode was used, signaling that the caller must
/// Returns `true` when newline mode was used, signaling that the caller must
/// invalidate the diff buffer for a full repaint.
fn flush_pending_history_lines(
terminal: &mut Terminal,
pending_history_lines: &mut Vec<Line<'static>>,
is_zellij: bool,
history_insert_mode: crate::insert_history::InsertHistoryMode,
) -> Result<bool> {
if pending_history_lines.is_empty() {
return Ok(false);
@@ -708,10 +777,10 @@ impl Tui {
crate::insert_history::insert_history_lines_with_mode(
terminal,
pending_history_lines.clone(),
crate::insert_history::InsertHistoryMode::new(is_zellij),
history_insert_mode,
)?;
pending_history_lines.clear();
Ok(is_zellij)
Ok(history_insert_mode == crate::insert_history::InsertHistoryMode::Newline)
}
pub fn draw(
@@ -747,7 +816,7 @@ impl Tui {
needs_full_repaint |= Self::flush_pending_history_lines(
terminal,
&mut self.pending_history_lines,
self.is_zellij,
self.history_insert_mode,
)?;
if needs_full_repaint {
@@ -803,7 +872,7 @@ impl Tui {
let flushed_history = Self::flush_pending_history_lines(
terminal,
&mut self.pending_history_lines,
self.is_zellij,
self.history_insert_mode,
)?;
needs_full_repaint |= flushed_history;