From 14d80e55cd8e83b38b673db5cd3d1401773fe49a Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Mon, 25 May 2026 15:42:28 -0300 Subject: [PATCH] fix(tui): improve multiline markdown list readability (#24351) ## Why Numbered Markdown findings become hard to scan when long items visually run together or when wrapped explanatory paragraphs lose their list indentation. This is especially visible in review output: the next number can look attached to the previous finding, and paragraph continuation rows can jump back toward the left margin instead of staying grouped beneath their item.
Before
CleanShot 2026-05-24 at 14 00 49
After
image
## What Changed - Insert a blank separator before a sibling list item when the previous item occupies more than one rendered line. - Preserve compact rendering for lists whose sibling items each render on one line. - Preserve list-body leading whitespace when transient streamed assistant rows require another wrapping pass for history display, so wrapped paragraphs stay aligned beneath their item. - Share the existing leading-whitespace prefix logic used by history insertion instead of introducing a second indentation rule. - Keep streamed Markdown output aligned with completed rendering and add snapshots for findings-style spacing and streamed paragraph indentation. ## How to Test 1. Start Codex from this branch and open the recorded repro session `019e563f-7d58-7ff2-8ec7-828f20fa61ca`. 2. Inspect the numbered `Findings` list whose items contain explanatory paragraphs. 3. Confirm each multiline finding is separated from the next numbered finding by one blank line. 4. Confirm wrapped rows of each indented paragraph remain aligned beneath the finding body, rather than returning to the left edge. 5. Render a short one-line numbered or unordered list and confirm its items remain compact without added blank rows. Targeted tests: - `just test -p codex-tui history_cell insert_history markdown_render markdown_stream streaming::controller` - `just argument-comment-lint-from-source -p codex-tui` ## Related Work PR #24346 changes Markdown table column allocation in parallel. This PR is intentionally limited to list-item readability and history wrapping; both branches touch `codex-rs/tui/src/markdown_render.rs`, so a small merge conflict may need resolution depending on merge order. --- codex-rs/tui/src/history_cell/messages.rs | 30 +++++++++----- ...ph_preserves_item_indent_when_wrapped.snap | 10 +++++ codex-rs/tui/src/history_cell/tests.rs | 24 +++++++++++ codex-rs/tui/src/insert_history.rs | 2 +- codex-rs/tui/src/markdown_render.rs | 14 +++---- codex-rs/tui/src/markdown_render_tests.rs | 41 +++++++++++++++++++ codex-rs/tui/src/markdown_stream.rs | 1 + ...sts__markdown_render_complex_snapshot.snap | 1 + ..._finding_items_are_separated_snapshot.snap | 17 ++++++++ codex-rs/tui/src/streaming/controller.rs | 1 + 10 files changed, 123 insertions(+), 18 deletions(-) create mode 100644 codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__streamed_agent_list_paragraph_preserves_item_indent_when_wrapped.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__multiline_finding_items_are_separated_snapshot.snap diff --git a/codex-rs/tui/src/history_cell/messages.rs b/codex-rs/tui/src/history_cell/messages.rs index 046d19043..19a6ec290 100644 --- a/codex-rs/tui/src/history_cell/messages.rs +++ b/codex-rs/tui/src/history_cell/messages.rs @@ -283,16 +283,26 @@ impl AgentMessageCell { impl HistoryCell for AgentMessageCell { fn display_lines(&self, width: u16) -> Vec> { - adaptive_wrap_lines( - &self.lines, - RtOptions::new(width as usize) - .initial_indent(if self.is_first_line { - "• ".dim().into() - } else { - " ".into() - }) - .subsequent_indent(" ".into()), - ) + let mut wrapped = Vec::new(); + for (index, line) in self.lines.iter().enumerate() { + let initial_indent = if index == 0 && self.is_first_line { + "• ".dim().into() + } else { + " ".into() + }; + let mut subsequent_indent = Line::from(" "); + subsequent_indent + .spans + .extend(crate::insert_history::leading_whitespace_prefix(line).spans); + let line_wrapped = adaptive_wrap_line( + line, + RtOptions::new(width as usize) + .initial_indent(initial_indent) + .subsequent_indent(subsequent_indent), + ); + push_owned_lines(&line_wrapped, &mut wrapped); + } + wrapped } fn raw_lines(&self) -> Vec> { diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__streamed_agent_list_paragraph_preserves_item_indent_when_wrapped.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__streamed_agent_list_paragraph_preserves_item_indent_when_wrapped.snap new file mode 100644 index 000000000..2fefd57ef --- /dev/null +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__streamed_agent_list_paragraph_preserves_item_indent_when_wrapped.snap @@ -0,0 +1,10 @@ +--- +source: tui/src/history_cell/tests.rs +expression: "lines.join(\"\\n\")" +--- +• 1. Correctness issue: server tool-search completions are + rejected. + + In next_prompt_suggestion.rs, ToolSearchCall records its + call id, but a paired output is ignored and suppresses + suggestions. diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index ca1bae863..7f846d39f 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -2294,6 +2294,30 @@ fn agent_markdown_cell_does_not_split_words_after_inline_markdown() { ); } +#[test] +fn streamed_agent_list_paragraph_preserves_item_indent_when_wrapped() { + let cell = AgentMessageCell::new( + vec![ + Line::from("1. Correctness issue: server tool-search completions are rejected."), + Line::default(), + Line::from( + " In next_prompt_suggestion.rs, ToolSearchCall records its call id, but a paired output is ignored and suppresses suggestions.", + ), + ], + /*is_first_line*/ true, + ); + + let lines = render_lines(&cell.display_lines(/*width*/ 64)); + assert!( + lines + .iter() + .filter(|line| line.contains("paired output") || line.contains("suggestions.")) + .all(|line| line.starts_with(" ")), + "expected all wrapped paragraph rows to retain the assistant gutter and list indent: {lines:?}", + ); + insta::assert_snapshot!(lines.join("\n")); +} + #[test] fn agent_markdown_cell_narrow_width_shows_prefix_only() { let source = "narrow width coverage\n"; diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index f2c64a109..ede7d03c7 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -166,7 +166,7 @@ where Ok(()) } -fn leading_whitespace_prefix(line: &Line<'_>) -> Line<'static> { +pub(crate) fn leading_whitespace_prefix(line: &Line<'_>) -> Line<'static> { let mut spans = Vec::new(); for span in &line.spans { let prefix_end = span diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index d0c2b87d5..75125a1ac 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -336,7 +336,7 @@ where indent_stack: Vec, list_indices: Vec>, list_needs_blank_before_next_item: Vec, - list_item_contains_code_block: Vec, + list_item_start_line_counts: Vec, link: Option, needs_newline: bool, pending_marker_line: bool, @@ -370,7 +370,7 @@ where indent_stack: Vec::new(), list_indices: Vec::new(), list_needs_blank_before_next_item: Vec::new(), - list_item_contains_code_block: Vec::new(), + list_item_start_line_counts: Vec::new(), link: None, needs_newline: false, pending_marker_line: false, @@ -480,7 +480,9 @@ where TagEnd::CodeBlock => self.end_codeblock(), TagEnd::List(_) => self.end_list(), TagEnd::Item => { - if self.list_item_contains_code_block.pop().unwrap_or(false) + self.flush_current_line(); + let start_line_count = self.list_item_start_line_counts.pop().unwrap_or_default(); + if self.text.lines.len().saturating_sub(start_line_count) > 1 && let Some(needs_blank) = self.list_needs_blank_before_next_item.last_mut() { *needs_blank = true; @@ -737,8 +739,9 @@ where { self.push_blank_line(); } + self.flush_current_line(); + self.list_item_start_line_counts.push(self.text.lines.len()); self.pending_marker_line = true; - self.list_item_contains_code_block.push(false); let depth = self.list_indices.len(); let is_ordered = self .list_indices @@ -778,9 +781,6 @@ where } fn start_codeblock(&mut self, lang: Option, indent: Option>) { - for item_contains_code_block in &mut self.list_item_contains_code_block { - *item_contains_code_block = true; - } self.flush_current_line(); if !self.text.lines.is_empty() { self.push_blank_line(); diff --git a/codex-rs/tui/src/markdown_render_tests.rs b/codex-rs/tui/src/markdown_render_tests.rs index 2c6d41e33..d02fef76d 100644 --- a/codex-rs/tui/src/markdown_render_tests.rs +++ b/codex-rs/tui/src/markdown_render_tests.rs @@ -531,6 +531,7 @@ fn nested_unordered_in_ordered() { Line::from_iter(["1. ".light_blue(), "Outer".into()]), Line::from_iter([" - ", "Inner A"]), Line::from_iter([" - ", "Inner B"]), + Line::default(), Line::from_iter(["2. ".light_blue(), "Next".into()]), ]); assert_eq!(text, expected); @@ -544,6 +545,7 @@ fn nested_ordered_in_unordered() { Line::from_iter(["- ", "Outer"]), Line::from_iter([" 1. ".light_blue(), "One".into()]), Line::from_iter([" 2. ".light_blue(), "Two".into()]), + Line::default(), Line::from_iter(["- ", "Last"]), ]); assert_eq!(text, expected); @@ -557,6 +559,7 @@ fn loose_list_item_multiple_paragraphs() { Line::from_iter(["1. ".light_blue(), "First paragraph".into()]), Line::default(), Line::from_iter([" ", "Second paragraph of same item"]), + Line::default(), Line::from_iter(["2. ".light_blue(), "Next item".into()]), ]); assert_eq!(text, expected); @@ -581,6 +584,7 @@ fn deeply_nested_mixed_three_levels() { Line::from_iter(["1. ".light_blue(), "A".into()]), Line::from_iter([" - ", "B"]), Line::from_iter([" 1. ".light_blue(), "C".into()]), + Line::default(), Line::from_iter(["2. ".light_blue(), "D".into()]), ]); assert_eq!(text, expected); @@ -1181,6 +1185,42 @@ fn list_item_after_simple_item_stays_compact() { assert_eq!(plain_lines(&text), vec!["1. First", "2. Second"]); } +#[test] +fn multiline_finding_items_are_separated_snapshot() { + let md = r#"**Findings** + +1. **Correctness issue: server tool-search completions are always rejected.** + + In `next_prompt_suggestion.rs`, the output is ignored, suppressing suggestions after completed searches. + + Minimal correction: count matching outputs and suppress only missing ones. + +2. **High-confidence simplification: remove the unused error channel.** + + The implementation resolves failures to `None`, so its contract can be narrower. + +3. **High-confidence churn reduction: consolidate table-driven filter tests.** +"#; + let text = render_markdown_text(md); + assert_snapshot!(plain_lines(&text).join("\n")); +} + +#[test] +fn wrapped_list_item_is_separated_from_next_sibling() { + let md = "1. This item wraps onto another visible rendered line\n2. Next item\n"; + let text = render_markdown_text_with_width(md, Some(/*width*/ 24)); + assert_eq!( + plain_lines(&text), + vec![ + "1. This item wraps onto", + " another visible", + " rendered line", + "", + "2. Next item", + ] + ); +} + #[test] fn mixed_url_markdown_wraps_prose_without_splitting_words_snapshot() { let md = "This paragraph keeps **strikethrough** intact near a [link](https://example.com/path) while enough surrounding prose forces wrapping."; @@ -1391,6 +1431,7 @@ fn nested_item_continuation_paragraph_is_indented() { Line::from_iter([" - ", "B"]), Line::default(), Line::from_iter([" ", "Continuation for B"]), + Line::default(), Line::from_iter(["2. ".light_blue(), "C".into()]), ]); assert_eq!(text, expected); diff --git a/codex-rs/tui/src/markdown_stream.rs b/codex-rs/tui/src/markdown_stream.rs index 66b5cab12..ad8cc68e7 100644 --- a/codex-rs/tui/src/markdown_stream.rs +++ b/codex-rs/tui/src/markdown_stream.rs @@ -789,6 +789,7 @@ mod tests { "3. Loose item with its own paragraph.".to_string(), "".to_string(), " This paragraph belongs to the same list item.".to_string(), + "".to_string(), "4. Second loose item with a nested list after a blank line.".to_string(), " - Nested bullet under a loose item".to_string(), " - Another nested bullet".to_string(), diff --git a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap index 34cbd1c63..1e32d7c61 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap @@ -16,6 +16,7 @@ Image: alt text - Unordered list item 1 - Nested bullet with italics inner + - Unordered list item 2 with strikethrough 1. Ordered item one diff --git a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__multiline_finding_items_are_separated_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__multiline_finding_items_are_separated_snapshot.snap new file mode 100644 index 000000000..a74cf9f0d --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__multiline_finding_items_are_separated_snapshot.snap @@ -0,0 +1,17 @@ +--- +source: tui/src/markdown_render_tests.rs +expression: "plain_lines(&text).join(\"\\n\")" +--- +Findings + +1. Correctness issue: server tool-search completions are always rejected. + + In next_prompt_suggestion.rs, the output is ignored, suppressing suggestions after completed searches. + + Minimal correction: count matching outputs and suppress only missing ones. + +2. High-confidence simplification: remove the unused error channel. + + The implementation resolves failures to None, so its contract can be narrower. + +3. High-confidence churn reduction: consolidate table-driven filter tests. diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 0427af24c..a653f1b21 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -1189,6 +1189,7 @@ mod tests { "3. Loose item with its own paragraph.".to_string(), "".to_string(), " This paragraph belongs to the same list item.".to_string(), + "".to_string(), "4. Second loose item with a nested list after a blank line.".to_string(), " - Nested bullet under a loose item".to_string(), " - Another nested bullet".to_string(),