From 98072cf5f68a2959961d5aa0ef0c9a78d6db80d1 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 27 Sep 2026 05:35:17 +0000 Subject: [PATCH] Preserve empty Markdown list markers in the TUI (#48623) ## Why Empty list items can lose their markers, and a bare marker received during streaming can still acquire content in a later chunk. ## What changed - Render pending markers when an empty item ends or a nested list starts on a new line, including inside blockquotes. - Keep trailing bare list markers mutable during streaming so later content and terminal resizing preserve the item correctly. ## Testing Add snapshot coverage for empty ordered, nested, and blockquoted list items, plus regression tests for streamed continuations, finalization, and resizing with a held marker. GitOrigin-RevId: d9116bddc49670ad66d71dbddc66b2ab347cb6b7 --- codex-rs/tui/src/markdown_render.rs | 15 +++++- codex-rs/tui/src/markdown_render_tests.rs | 7 +++ ...rkdown_render_tests__empty_list_items.snap | 17 ++++++ codex-rs/tui/src/streaming/controller.rs | 11 ++++ .../streaming/rendering_preferences_tests.rs | 53 +++++++++++++++++++ 5 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__empty_list_items.snap diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index a7109210a2..a85c7a11df 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -620,7 +620,17 @@ impl<'a, 'policy> Writer<'a, 'policy> { }; self.start_codeblock(lang, indent) } - Tag::List(start) => self.start_list(start), + Tag::List(start) => { + if self.pending_marker_line + && self.input[..range.start] + .rsplit('\n') + .next() + .is_some_and(|line| line.trim_matches(['>', ' ', '\t']).is_empty()) + { + self.push_line(Line::default()); + } + self.start_list(start); + } Tag::Item => { self.start_item(); if let Some((next, next_range)) = iter.next() { @@ -685,6 +695,9 @@ impl<'a, 'policy> Writer<'a, 'policy> { TagEnd::CodeBlock => self.end_codeblock(range), TagEnd::List(_) => self.end_list(), TagEnd::Item => { + if self.pending_marker_line { + self.push_line(Line::default()); + } self.flush_current_line(); let start_line_count = self.list_item_start_line_counts.pop().unwrap_or_default(); if let Some(list) = self.uniform_lists.last_mut() { diff --git a/codex-rs/tui/src/markdown_render_tests.rs b/codex-rs/tui/src/markdown_render_tests.rs index d0c8ab0643..680e989062 100644 --- a/codex-rs/tui/src/markdown_render_tests.rs +++ b/codex-rs/tui/src/markdown_render_tests.rs @@ -590,6 +590,13 @@ fn list_ordered() { assert_eq!(text, expected); } +#[test] +fn empty_list_items_keep_their_markers() { + let rendered = + ["8.", "-\n -", "> -\n> -"].map(|source| plain_lines(&render_markdown_text(source))); + assert_debug_snapshot!("empty_list_items", rendered); +} + #[test] fn list_nested() { let text = render_markdown_text("- List item 1\n - Nested list item 1\n"); diff --git a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__empty_list_items.snap b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__empty_list_items.snap new file mode 100644 index 0000000000..4f05d8ca10 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__empty_list_items.snap @@ -0,0 +1,17 @@ +--- +source: tui/src/markdown_render_tests.rs +expression: rendered +--- +[ + [ + "8. ", + ], + [ + "• ", + " • ", + ], + [ + "> • ", + "> • ", + ], +] diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index d66d11e7db..02fd4e6a04 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -421,10 +421,21 @@ impl StreamCore { } => Some(table_start), TableHoldbackState::None => None, }; + let source = self.state.collector.committed_source(); + let source = source.strip_suffix('\n').unwrap_or(source); + let marker_start = source.rfind('\n').map_or(0, |index| index + 1); + let marker = source[marker_start..] + .trim() + .trim_start_matches(['>', ' ', '\t']); + let bare_list_marker = matches!(marker, "-" | "+" | "*") + || marker.strip_suffix(['.', ')']).is_some_and(|number| { + !number.is_empty() && number.bytes().all(|byte| byte.is_ascii_digit()) + }); [ table_start, self.render.mutable_fence_start, self.render.pending_math_start, + bare_list_marker.then_some(marker_start), ] .into_iter() .flatten() diff --git a/codex-rs/tui/src/streaming/rendering_preferences_tests.rs b/codex-rs/tui/src/streaming/rendering_preferences_tests.rs index 378ef214fb..9bd077d048 100644 --- a/codex-rs/tui/src/streaming/rendering_preferences_tests.rs +++ b/codex-rs/tui/src/streaming/rendering_preferences_tests.rs @@ -5,6 +5,59 @@ use crate::markdown_render::preferences; use codex_config::types::TuiRendering; use pretty_assertions::assert_eq; +#[test] +fn empty_list_item_stays_mutable_until_finalization() { + let cwd = std::env::temp_dir(); + for (marker, continuation) in [ + ("8.", ""), + ("8.\n", " answer\n"), + ("> 8.\n", "> answer\n"), + ] { + let mut stream = StreamCore::new( + Some(24), + &cwd, + HistoryRenderMode::Rich, + /*inline_visualization_context*/ None, + ); + stream.push_delta(marker); + let mut emitted = stream.tick_batch(usize::MAX); + stream.push_delta(continuation); + let (remaining, source) = stream.finalize_remaining(); + emitted.extend(remaining); + assert_eq!( + emitted, + render_source( + &source, + Some(24), + &cwd, + HistoryRenderMode::Rich, + /*inline_visualization_context*/ None, + ), + "marker={marker:?}" + ); + } +} + +#[test] +fn resizing_does_not_drop_held_list_marker() { + let cwd = std::env::temp_dir(); + let mut stream = StreamCore::new( + Some(10), + &cwd, + HistoryRenderMode::Rich, + /*inline_visualization_context*/ None, + ); + stream.push_delta("Long prose with enough words to wrap.\n\n"); + assert!(stream.tick_batch(usize::MAX).len() > 1); + stream.push_delta("8.\n"); + stream.set_width(Some(40)); + let (remaining, _) = stream.finalize_remaining(); + assert_eq!( + remaining.last().map(|line| line.line.to_string()), + Some("8. ".to_string()) + ); +} + #[test] fn lists_match_incremental_and_emitted_responses() { let cwd = std::env::temp_dir();