refactor(tui): simplification pass over md-table rendering pipeline

Remove accumulated duplication and dead code across the md-table
branch: extract shared helpers (trailing_run_start<T>, strip_line_indent,
compute_target_stable_len, reset_history_emission_state,
maybe_finish_stream_reflow, is_blockquote_active, active_cell_is_stream_tail),
collapse redundant conditionals (normalize_row, FenceTracker Option state,
merged match arms), delete duplicate methods and tests, and fix stale
doc comments. No behavioral changes; -86 net lines.
This commit is contained in:
Felipe Coury
2026-04-06 13:42:16 -03:00
committed by Felipe Coury
parent a66e481c36
commit c0db2f0dc1
8 changed files with 120 additions and 205 deletions
+34 -62
View File
@@ -466,9 +466,9 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config)
message.push_str(&format!(" {reason}\n"));
}
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_warning_event(message),
)));
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_warning_event(message),
)));
}
fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &Config) {
@@ -483,48 +483,19 @@ fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &Config) {
)));
}
fn trailing_agent_message_run_start(transcript_cells: &[Arc<dyn HistoryCell>]) -> usize {
fn trailing_run_start<T: 'static>(transcript_cells: &[Arc<dyn HistoryCell>]) -> usize {
let end = transcript_cells.len();
let mut start = end;
while start > 0
&& transcript_cells[start - 1].is_stream_continuation()
&& transcript_cells[start - 1]
.as_any()
.is::<history_cell::AgentMessageCell>()
&& transcript_cells[start - 1].as_any().is::<T>()
{
start -= 1;
}
if start > 0
&& transcript_cells[start - 1]
.as_any()
.is::<history_cell::AgentMessageCell>()
&& !transcript_cells[start - 1].is_stream_continuation()
{
start -= 1;
}
start
}
fn trailing_proposed_plan_stream_run_start(transcript_cells: &[Arc<dyn HistoryCell>]) -> usize {
let end = transcript_cells.len();
let mut start = end;
while start > 0
&& transcript_cells[start - 1].is_stream_continuation()
&& transcript_cells[start - 1]
.as_any()
.is::<history_cell::ProposedPlanStreamCell>()
{
start -= 1;
}
if start > 0
&& transcript_cells[start - 1]
.as_any()
.is::<history_cell::ProposedPlanStreamCell>()
&& transcript_cells[start - 1].as_any().is::<T>()
&& !transcript_cells[start - 1].is_stream_continuation()
{
start -= 1;
@@ -3314,6 +3285,11 @@ impl App {
Ok(())
}
fn reset_history_emission_state(&mut self) {
self.has_emitted_history_lines = false;
self.deferred_history_lines.clear();
}
fn display_lines_for_history_insert(
&mut self,
cell: &dyn HistoryCell,
@@ -3351,6 +3327,16 @@ impl App {
self.resize_reflow_pending_until = Some(Instant::now() + RESIZE_REFLOW_DEBOUNCE);
}
/// After stream consolidation, schedule a follow-up reflow if one ran mid-stream.
fn maybe_finish_stream_reflow(&mut self, tui: &mut tui::Tui) {
if self.reflow_ran_during_stream {
self.schedule_resize_reflow();
tui.frame_requester()
.schedule_frame_in(RESIZE_REFLOW_DEBOUNCE);
}
self.reflow_ran_during_stream = false;
}
fn handle_draw_size_change(
&mut self,
size: ratatui::layout::Size,
@@ -3383,8 +3369,7 @@ impl App {
}
self.resize_reflow_pending_until = None;
self.has_emitted_history_lines = false;
self.deferred_history_lines.clear();
self.reset_history_emission_state();
}
fn handle_draw_pre_render(&mut self, tui: &mut tui::Tui) -> Result<()> {
@@ -3419,16 +3404,14 @@ impl App {
// 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();
self.reset_history_emission_state();
return Ok(());
}
tui.terminal.clear_scrollback()?;
tui.terminal.clear()?;
self.has_emitted_history_lines = false;
self.deferred_history_lines.clear();
self.reset_history_emission_state();
// Track that a reflow happened during an active stream or while trailing
// unconsolidated AgentMessageCells are still pending consolidation so
@@ -3448,9 +3431,9 @@ impl App {
fn should_mark_reflow_as_stream_time(&self) -> bool {
self.chat_widget.has_active_agent_stream()
|| self.chat_widget.has_active_plan_stream()
|| trailing_agent_message_run_start(&self.transcript_cells)
|| trailing_run_start::<history_cell::AgentMessageCell>(&self.transcript_cells)
< self.transcript_cells.len()
|| trailing_proposed_plan_stream_run_start(&self.transcript_cells)
|| trailing_run_start::<history_cell::ProposedPlanStreamCell>(&self.transcript_cells)
< self.transcript_cells.len()
}
@@ -4432,7 +4415,8 @@ impl App {
"ConsolidateAgentMessage: transcript_cells.len()={end}, source_len={}",
source.len()
);
let start = trailing_agent_message_run_start(&self.transcript_cells);
let start =
trailing_run_start::<history_cell::AgentMessageCell>(&self.transcript_cells);
if start < end {
tracing::debug!(
"ConsolidateAgentMessage: replacing cells [{start}..{end}] with AgentMarkdownCell"
@@ -4448,26 +4432,19 @@ impl App {
tui.frame_requester().schedule_frame();
}
// If a resize reflow ran while these cells were still unconsolidated
// AgentMessageCells, the scrollback has stale table rendering. Schedule a
// re-reflow.
if self.reflow_ran_during_stream {
self.schedule_resize_reflow();
tui.frame_requester()
.schedule_frame_in(RESIZE_REFLOW_DEBOUNCE);
}
self.reflow_ran_during_stream = false;
self.maybe_finish_stream_reflow(tui);
} else {
tracing::debug!(
"ConsolidateAgentMessage: no cells to consolidate(start={start}, end={end})",
);
self.reflow_ran_during_stream = false;
self.maybe_finish_stream_reflow(tui);
}
}
AppEvent::ConsolidateProposedPlan(source) => {
let end = self.transcript_cells.len();
let start = trailing_proposed_plan_stream_run_start(&self.transcript_cells);
let start = trailing_run_start::<history_cell::ProposedPlanStreamCell>(
&self.transcript_cells,
);
let consolidated: Arc<dyn HistoryCell> =
Arc::new(history_cell::new_proposed_plan(source, &self.config.cwd));
@@ -4492,12 +4469,7 @@ impl App {
);
}
if self.reflow_ran_during_stream {
self.schedule_resize_reflow();
tui.frame_requester()
.schedule_frame_in(RESIZE_REFLOW_DEBOUNCE);
}
self.reflow_ran_during_stream = false;
self.maybe_finish_stream_reflow(tui);
}
AppEvent::ApplyThreadRollback { num_turns } => {
if self.apply_non_pending_thread_rollback(num_turns) {
+13 -20
View File
@@ -4988,7 +4988,7 @@ impl ChatWidget {
return;
};
let should_submit_now =
self.is_session_configured() && !self.is_plan_streaming_in_tui();
self.is_session_configured() && !self.has_active_plan_stream();
if should_submit_now {
// Submitted is emitted when user submits.
// Reset any reasoning header only when we are actually submitting a turn.
@@ -5639,11 +5639,7 @@ impl ChatWidget {
}
fn flush_active_cell(&mut self) {
if self
.active_cell
.as_ref()
.is_some_and(|cell| cell.as_any().is::<history_cell::StreamingAgentTailCell>())
{
if self.active_cell_is_stream_tail() {
if self.stream_controller.is_some() {
return;
}
@@ -5674,10 +5670,8 @@ impl ChatWidget {
if !keep_placeholder_header_active && !cell.display_lines(u16::MAX).is_empty() {
// Only break exec grouping if the cell renders visible lines.
let keep_stream_tail_active = self.stream_controller.is_some()
&& self.active_cell.as_ref().is_some_and(|active| {
active.as_any().is::<history_cell::StreamingAgentTailCell>()
});
let keep_stream_tail_active =
self.stream_controller.is_some() && self.active_cell_is_stream_tail();
if !keep_stream_tail_active {
self.flush_active_cell();
}
@@ -5711,11 +5705,7 @@ impl ChatWidget {
}
fn clear_active_stream_tail(&mut self) {
if self
.active_cell
.as_ref()
.is_some_and(|cell| cell.as_any().is::<history_cell::StreamingAgentTailCell>())
{
if self.active_cell_is_stream_tail() {
self.active_cell = None;
self.bump_active_cell_revision();
}
@@ -10475,10 +10465,6 @@ impl ChatWidget {
self.bottom_pane.is_task_running() || self.is_review_mode
}
fn is_plan_streaming_in_tui(&self) -> bool {
self.plan_stream_controller.is_some()
}
/// Whether an agent message stream is active (not a plan stream).
pub(crate) fn has_active_agent_stream(&self) -> bool {
self.stream_controller.is_some()
@@ -10488,6 +10474,13 @@ impl ChatWidget {
self.plan_stream_controller.is_some()
}
/// Whether the active cell is a transient streaming tail preview.
fn active_cell_is_stream_tail(&self) -> bool {
self.active_cell
.as_ref()
.is_some_and(|cell| cell.as_any().is::<history_cell::StreamingAgentTailCell>())
}
pub(crate) fn composer_is_empty(&self) -> bool {
self.bottom_pane.composer_is_empty()
}
@@ -10516,7 +10509,7 @@ impl ChatWidget {
return;
}
self.set_collaboration_mask(collaboration_mode);
let should_queue = self.is_plan_streaming_in_tui();
let should_queue = self.has_active_plan_stream();
let user_message = UserMessage {
text,
local_images: Vec::new(),
-1
View File
@@ -38,7 +38,6 @@ use crate::version::CODEX_CLI_VERSION;
use crate::wrapping::RtOptions;
use crate::wrapping::adaptive_wrap_line;
use crate::wrapping::adaptive_wrap_lines;
use crate::wrapping::word_wrap_lines;
use base64::Engine;
use codex_app_server_protocol::McpServerStatus;
use codex_config::types::McpServerTransportConfig;
+22 -19
View File
@@ -91,11 +91,13 @@ fn unwrap_markdown_fences(markdown_source: &str) -> String {
struct Fence {
marker: u8,
len: usize,
is_markdown: bool,
is_blockquoted: bool,
}
fn parse_open_fence(line: &str) -> Option<Fence> {
// Strip a trailing newline and up to 3 leading spaces, returning the
// trimmed slice. Returns `None` when the line has 4+ leading spaces
// (which makes it an indented code line per CommonMark).
fn strip_line_indent(line: &str) -> Option<&str> {
let without_newline = line.strip_suffix('\n').unwrap_or(line);
let leading_ws = without_newline
.as_bytes()
@@ -105,30 +107,31 @@ fn unwrap_markdown_fences(markdown_source: &str) -> String {
if leading_ws > 3 {
return None;
}
let trimmed = &without_newline[leading_ws..];
Some(&without_newline[leading_ws..])
}
// Parse an opening fence line, returning the fence metadata and whether
// the fence info string indicates markdown content.
fn parse_open_fence(line: &str) -> Option<(Fence, bool)> {
let trimmed = strip_line_indent(line)?;
let is_blockquoted = trimmed.trim_start().starts_with('>');
let fence_scan_text = table_detect::strip_blockquote_prefix(trimmed);
let (marker, len) = table_detect::parse_fence_marker(fence_scan_text)?;
let is_markdown = table_detect::is_markdown_fence_info(fence_scan_text, len);
Some(Fence {
marker: marker as u8,
len,
Some((
Fence {
marker: marker as u8,
len,
is_blockquoted,
},
is_markdown,
is_blockquoted,
})
))
}
fn is_close_fence(line: &str, fence: Fence) -> bool {
let without_newline = line.strip_suffix('\n').unwrap_or(line);
let leading_ws = without_newline
.as_bytes()
.iter()
.take_while(|byte| **byte == b' ')
.count();
if leading_ws > 3 {
let Some(trimmed) = strip_line_indent(line) else {
return false;
}
let trimmed = &without_newline[leading_ws..];
};
let fence_scan_text = if fence.is_blockquoted {
if !trimmed.trim_start().starts_with('>') {
return false;
@@ -246,8 +249,8 @@ fn unwrap_markdown_fences(markdown_source: &str) -> String {
continue;
}
if let Some(fence) = parse_open_fence(line) {
if fence.is_markdown {
if let Some((fence, is_markdown)) = parse_open_fence(line) {
if is_markdown {
active_fence = Some(ActiveFence::MarkdownCandidate {
fence,
opening_range: line_range,
+14 -18
View File
@@ -22,7 +22,7 @@
//! alignment count.
//! 3. **Compute column widths** -- allocate widths with Narrative/Structured
//! priority and iterative shrinking.
//! 4. **Render box grid** -- Unicode borders (`+---+`) or fallback to pipe
//! 4. **Render box grid** -- Unicode borders (`┌───┬───┐`) or fallback to pipe
//! format when the minimum cannot fit.
//! 5. **Append spillover** -- extracted spillover rows rendered as plain text
//! after the table.
@@ -1028,12 +1028,8 @@ where
}
fn normalize_row(row: &mut Vec<TableCell>, column_count: usize) {
if row.len() > column_count {
row.truncate(column_count);
}
if row.len() < column_count {
row.resize(column_count, TableCell::default());
}
row.truncate(column_count);
row.resize(column_count, TableCell::default());
}
/// subtracts the space eaten by border characters
@@ -1436,8 +1432,8 @@ where
fn looks_like_html_content(text: &str) -> bool {
let bytes = text.as_bytes();
for idx in 0..bytes.len() {
if bytes[idx] != b'<' {
for (idx, &byte) in bytes.iter().enumerate() {
if byte != b'<' {
continue;
}
@@ -1473,7 +1469,7 @@ where
}
fn line_display_width(line: &Line<'_>) -> usize {
line.spans.iter().map(|span| span.content.width()).sum()
Self::spans_display_width(&line.spans)
}
fn cell_display_width(cell: &TableCell) -> usize {
@@ -1590,12 +1586,15 @@ where
/// borders. Passing them through `word_wrap_line` would break the grid at
/// arbitrary positions. This method prepends the indent/blockquote prefix
/// and pushes directly to `self.text.lines`.
fn is_blockquote_active(&self) -> bool {
self.indent_stack
.iter()
.any(|ctx| ctx.prefix.iter().any(|p| p.content.contains('>')))
}
fn push_prewrapped_line(&mut self, line: Line<'static>, pending_marker_line: bool) {
self.flush_current_line();
let blockquote_active = self
.indent_stack
.iter()
.any(|ctx| ctx.prefix.iter().any(|p| p.content.contains('>')));
let blockquote_active = self.is_blockquote_active();
let style = if blockquote_active {
self.styles.blockquote.patch(line.style)
} else {
@@ -1609,10 +1608,7 @@ where
fn push_line(&mut self, line: Line<'static>) {
self.flush_current_line();
let blockquote_active = self
.indent_stack
.iter()
.any(|ctx| ctx.prefix.iter().any(|s| s.content.contains('>')));
let blockquote_active = self.is_blockquote_active();
let style = if blockquote_active {
self.styles.blockquote
} else {
-17
View File
@@ -1111,23 +1111,6 @@ fn code_block_multiple_lines_inside_unordered_list() {
assert_eq!(lines, vec!["- Item", "", " first", " second"]);
}
#[test]
fn code_block_inside_unordered_list_item_multiple_lines() {
let md = "- Item\n\n ```\n first\n second\n ```\n";
let text = render_markdown_text(md);
let lines: Vec<String> = text
.lines
.iter()
.map(|l| {
l.spans
.iter()
.map(|s| s.content.clone())
.collect::<String>()
})
.collect();
assert_eq!(lines, vec!["- Item", "", " first", " second"]);
}
#[test]
fn markdown_render_complex_snapshot() {
let md = r#"# H1: Markdown Streaming Test
+16 -19
View File
@@ -256,15 +256,19 @@ impl StreamCore {
);
}
/// Compute how many rendered lines should be in the stable region.
fn compute_target_stable_len(&mut self) -> usize {
let tail_budget = self.active_tail_budget_lines();
self.rendered_lines
.len()
.saturating_sub(tail_budget)
.max(self.emitted_stable_len)
}
/// Advance `enqueued_stable_len` toward the target stable boundary and enqueue any
/// newly-stable lines. Returns `true` if new lines were enqueued.
fn sync_stable_queue(&mut self) -> bool {
let tail_budget = self.active_tail_budget_lines();
let target_stable_len = self
.rendered_lines
.len()
.saturating_sub(tail_budget)
.max(self.emitted_stable_len);
let target_stable_len = self.compute_target_stable_len();
// A structural rewrite moved the stable boundary backward into enqueue-but-unemitted
// lines. Rebuild queue from the latest snapshot.
@@ -292,12 +296,7 @@ impl StreamCore {
/// Rebuild the stable queue from the current render snapshot. Used after `set_width()` where
/// the old queue is stale.
fn rebuild_stable_queue_from_render(&mut self) {
let tail_budget = self.active_tail_budget_lines();
let target_stable_len = self
.rendered_lines
.len()
.saturating_sub(tail_budget)
.max(self.emitted_stable_len);
let target_stable_len = self.compute_target_stable_len();
self.state.clear_queue();
if self.emitted_stable_len < target_stable_len {
self.state
@@ -318,11 +317,9 @@ impl StreamCore {
let scan_start = Instant::now();
let holdback_state = self.holdback_scanner.state();
let tail_budget = match holdback_state {
TableHoldbackState::Confirmed { table_start } => {
self.tail_budget_from_source_start(table_start)
}
TableHoldbackState::PendingHeader { header_start } => {
self.tail_budget_from_source_start(header_start)
TableHoldbackState::Confirmed { table_start: start }
| TableHoldbackState::PendingHeader { header_start: start } => {
self.tail_budget_from_source_start(start)
}
TableHoldbackState::None => 0,
};
@@ -632,8 +629,8 @@ fn table_candidate_text(line: &str) -> Option<&str> {
///
/// `StreamCore::active_tail_budget_lines` uses this to decide whether to
/// commit rendered lines to the stable queue or withhold them as mutable tail.
/// The scan is stateless: `table_holdback_state` re-evaluates the full source
/// on every delta rather than maintaining incremental state.
/// The incremental `TableHoldbackScanner` maintains state across deltas;
/// the stateless `table_holdback_state` function is used only in tests.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
enum TableHoldbackState {
/// No table detected -- all rendered lines can flow into the stable queue.
+21 -49
View File
@@ -36,13 +36,8 @@ pub(crate) fn parse_table_segments(line: &str) -> Option<Vec<&str>> {
return None;
}
let mut content = trimmed;
if let Some(without_leading) = content.strip_prefix('|') {
content = without_leading;
}
if let Some(without_trailing) = content.strip_suffix('|') {
content = without_trailing;
}
let content = trimmed.strip_prefix('|').unwrap_or(trimmed);
let content = content.strip_suffix('|').unwrap_or(content);
let segments: Vec<&str> = split_unescaped_pipe(content)
.iter()
@@ -85,7 +80,7 @@ pub(crate) fn is_table_header_line(line: &str) -> bool {
/// Whether a single segment matches the `---`, `:---`, `---:`, or `:---:`
/// alignment-colon syntax used in markdown table delimiter rows.
pub(crate) fn is_table_delimiter_segment(segment: &str) -> bool {
fn is_table_delimiter_segment(segment: &str) -> bool {
let trimmed = segment.trim();
if trimmed.is_empty() {
return false;
@@ -129,20 +124,12 @@ pub(crate) enum FenceKind {
/// limits (>3 spaces → not a fence), blockquote prefix stripping, and
/// backtick/tilde marker matching.
pub(crate) struct FenceTracker {
in_fence: bool,
fence_char: char,
fence_len: usize,
fence_kind: FenceKind,
state: Option<(char, usize, FenceKind)>,
}
impl FenceTracker {
pub(crate) fn new() -> Self {
Self {
in_fence: false,
fence_char: '\0',
fence_len: 0,
fence_kind: FenceKind::Other,
}
Self { state: None }
}
/// Process one raw source line and update fence state.
@@ -162,36 +149,29 @@ impl FenceTracker {
let trimmed = &raw_line[leading_spaces..];
let fence_scan_text = strip_blockquote_prefix(trimmed);
if let Some((marker, len)) = parse_fence_marker(fence_scan_text) {
if !self.in_fence {
if let Some((open_char, open_len, _)) = self.state {
// Close the current fence if the marker matches.
if marker == open_char
&& len >= open_len
&& fence_scan_text[len..].trim().is_empty()
{
self.state = None;
}
} else {
// Opening a new fence.
self.in_fence = true;
self.fence_char = marker;
self.fence_len = len;
self.fence_kind = if is_markdown_fence_info(fence_scan_text, len) {
let kind = if is_markdown_fence_info(fence_scan_text, len) {
FenceKind::Markdown
} else {
FenceKind::Other
};
} else if marker == self.fence_char
&& len >= self.fence_len
&& fence_scan_text[len..].trim().is_empty()
{
// Closing the current fence.
self.in_fence = false;
self.fence_char = '\0';
self.fence_len = 0;
self.fence_kind = FenceKind::Other;
self.state = Some((marker, len, kind));
}
}
}
/// Current fence context for the most-recently-advanced line.
pub(crate) fn kind(&self) -> FenceKind {
if self.in_fence {
self.fence_kind
} else {
FenceKind::Outside
}
self.state.map_or(FenceKind::Outside, |(_, _, k)| k)
}
}
@@ -201,23 +181,15 @@ impl FenceTracker {
/// The input should already have leading whitespace and blockquote prefixes
/// stripped.
pub(crate) fn parse_fence_marker(line: &str) -> Option<(char, usize)> {
let mut chars = line.chars();
let first = chars.next()?;
if first != '`' && first != '~' {
let first = line.as_bytes().first().copied()?;
if first != b'`' && first != b'~' {
return None;
}
let mut len = 1usize;
for ch in chars {
if ch == first {
len += 1;
} else {
break;
}
}
let len = line.bytes().take_while(|&b| b == first).count();
if len < 3 {
return None;
}
Some((first, len))
Some((first as char, len))
}
/// Whether the info string after a fence marker indicates markdown content.