From dc1d0133ad68c7d52c6a4abd3671b61b5e75d34f Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 5 May 2026 01:01:50 -0300 Subject: [PATCH] fix(tui): limit stream tail holdback to tables Keeps ordinary newline-complete markdown streaming through the existing history path while holding possible table rows as the mutable live tail. This preserves recovery/status behavior for normal text. --- codex-rs/tui/src/streaming/controller.rs | 15 ++- .../tui/src/streaming/source_partition.rs | 110 +++++++++++++++--- 2 files changed, 104 insertions(+), 21 deletions(-) diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 81d1dea35d..594881d391 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -24,6 +24,7 @@ use std::time::Instant; use super::StreamState; use super::source_partition::partition_source; +use super::source_partition::source_has_table_block; /// Shared source-retaining stream state for assistant and plan output. /// @@ -177,6 +178,9 @@ impl StreamCore { if !source.ends_with('\n') { source.push('\n'); } + if !source_has_table_block(&source) { + return Vec::new(); + } let mut rendered = Vec::new(); append_markdown(&source, self.width, Some(self.cwd.as_path()), &mut rendered); @@ -642,7 +646,7 @@ mod tests { #[test] fn controller_set_width_rebuilds_queued_lines() { let mut ctrl = stream_controller(Some(120)); - let delta = "This is a long line that should wrap into multiple rows when resized.\n\nNext block.\n"; + let delta = "This is a long line that should wrap into multiple rows when resized.\n"; assert!(ctrl.push(delta)); assert_eq!(ctrl.queued_lines(), 1); @@ -664,7 +668,8 @@ mod tests { #[test] fn controller_set_width_no_duplicate_after_emit() { let mut ctrl = stream_controller(Some(120)); - let line = "This is a long line that definitely wraps when the terminal shrinks to 24 columns.\n\nNext block.\n"; + let line = + "This is a long line that definitely wraps when the terminal shrinks to 24 columns.\n"; ctrl.push(line); let (cell, _) = ctrl.on_commit_tick_batch(usize::MAX); assert!(cell.is_some(), "expected emitted cell"); @@ -682,7 +687,7 @@ mod tests { #[test] fn controller_tick_batch_zero_is_noop() { let mut ctrl = stream_controller(Some(80)); - assert!(ctrl.push("line one\n\nline two\n")); + assert!(ctrl.push("line one\n")); assert_eq!(ctrl.queued_lines(), 1); let (cell, idle) = ctrl.on_commit_tick_batch(/*max_lines*/ 0); @@ -698,7 +703,7 @@ mod tests { #[test] fn controller_finalize_returns_raw_source_for_consolidation() { let mut ctrl = stream_controller(Some(80)); - assert!(!ctrl.push("hello\n")); + assert!(ctrl.push("hello\n")); let (_cell, source) = ctrl.finalize(); assert_eq!(source, Some("hello\n".to_string())); } @@ -706,7 +711,7 @@ mod tests { #[test] fn plan_controller_finalize_returns_raw_source_for_consolidation() { let mut ctrl = plan_stream_controller(Some(80)); - assert!(!ctrl.push("- step\n")); + assert!(ctrl.push("- step\n")); let (_cell, source) = ctrl.finalize(); assert_eq!(source, Some("- step\n".to_string())); } diff --git a/codex-rs/tui/src/streaming/source_partition.rs b/codex-rs/tui/src/streaming/source_partition.rs index 33be3af8ac..2baf21108d 100644 --- a/codex-rs/tui/src/streaming/source_partition.rs +++ b/codex-rs/tui/src/streaming/source_partition.rs @@ -13,17 +13,35 @@ pub(crate) struct SourcePartition { pub(crate) tail: Range, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum BlockKind { + Table, + Other, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct SourceBlock { + range: Range, + kind: BlockKind, +} + pub(crate) fn partition_source(source: &str) -> SourcePartition { let blocks = top_level_blocks(source); - let stable_end = if blocks.len() >= 2 { - blocks[blocks.len() - 2].end + let stable_end = if blocks.last().is_some_and(|block| { + block.kind == BlockKind::Table || looks_like_table_prefix(&source[block.range.clone()]) + }) { + if blocks.len() >= 2 { + blocks[blocks.len() - 2].range.end + } else { + 0 + } } else { - 0 + source.len() }; let stable_blocks = blocks .iter() - .filter(|range| range.end <= stable_end) - .cloned() + .filter(|block| block.range.end <= stable_end) + .map(|block| block.range.clone()) .collect(); SourcePartition { @@ -33,20 +51,38 @@ pub(crate) fn partition_source(source: &str) -> SourcePartition { } } -fn top_level_blocks(source: &str) -> Vec> { +pub(crate) fn source_has_table_block(source: &str) -> bool { + top_level_blocks(source) + .iter() + .any(|block| block.kind == BlockKind::Table) +} + +fn looks_like_table_prefix(source: &str) -> bool { + let mut lines = source.lines().filter(|line| !line.trim().is_empty()); + let Some(first) = lines.next() else { + return false; + }; + let trimmed = first.trim(); + let pipe_count = trimmed.chars().filter(|ch| *ch == '|').count(); + pipe_count >= 2 || (pipe_count >= 1 && trimmed.starts_with('|')) +} + +fn top_level_blocks(source: &str) -> Vec { let mut options = Options::empty(); options.insert(Options::ENABLE_STRIKETHROUGH); options.insert(Options::ENABLE_TABLES); - let mut blocks: Vec> = Vec::new(); + let mut blocks: Vec = Vec::new(); let mut depth = 0usize; let mut block_start: Option = None; + let mut block_kind = BlockKind::Other; for (event, range) in Parser::new_ext(source, options).into_offset_iter() { match event { Event::Start(tag) if is_block_start(&tag) => { if depth == 0 { block_start = Some(range.start); + block_kind = block_kind_for_start(&tag); } depth += 1; } @@ -56,12 +92,19 @@ fn top_level_blocks(source: &str) -> Vec> { if depth == 0 && let Some(start) = block_start.take() { - blocks.push(start..range.end); + blocks.push(SourceBlock { + range: start..range.end, + kind: block_kind, + }); + block_kind = BlockKind::Other; } } } Event::Rule if depth == 0 => { - blocks.push(range.clone()); + blocks.push(SourceBlock { + range: range.clone(), + kind: BlockKind::Other, + }); } _ => {} } @@ -85,6 +128,14 @@ fn is_block_start(tag: &Tag<'_>) -> bool { ) } +fn block_kind_for_start(tag: &Tag<'_>) -> BlockKind { + if matches!(tag, Tag::Table(_)) { + BlockKind::Table + } else { + BlockKind::Other + } +} + fn is_block_end(tag: TagEnd) -> bool { matches!( tag, @@ -121,21 +172,48 @@ mod tests { let partition = partition_source(source); - assert_eq!( - &source[..partition.stable_end], - "| A | B |\n| --- | --- |\n| 1 | 2 |\n" - ); - assert_eq!(&source[partition.tail], "\nDone.\n"); - assert_eq!(partition.stable_blocks.len(), 1); + assert_eq!(partition.stable_end, source.len()); + assert_eq!(&source[partition.tail], ""); + assert_eq!(partition.stable_blocks.len(), 2); } #[test] - fn fenced_code_with_pipes_is_one_tail_block() { + fn fenced_code_with_pipes_is_stable() { let source = "```\n| A | B |\n| --- | --- |\n```\n"; let partition = partition_source(source); + assert_eq!(partition.stable_end, source.len()); + assert_eq!(&source[partition.tail], ""); + } + + #[test] + fn normal_paragraph_is_stable() { + let source = "hello\n"; + + let partition = partition_source(source); + + assert_eq!(partition.stable_end, source.len()); + assert_eq!(&source[partition.tail], ""); + } + + #[test] + fn table_header_candidate_remains_tail() { + let source = "| A | B |\n"; + + let partition = partition_source(source); + assert_eq!(partition.stable_end, 0); assert_eq!(&source[partition.tail], source); } + + #[test] + fn detects_table_blocks() { + assert!(source_has_table_block( + "| A | B |\n| --- | --- |\n| 1 | 2 |\n" + )); + assert!(!source_has_table_block( + "```\n| A | B |\n| --- | --- |\n```\n" + )); + } }