clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-06-01 19:02:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(tui): improve multiline markdown list readability (#24351)

Browse Source 
## 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.

<table><tr><td>
<center>Before</center>
<img width="1718" height="836" alt="CleanShot 2026-05-24 at 14 00 49"
src="https://github.com/user-attachments/assets/f1ee0023-50fa-4f81-a641-ae08b17b99bd"
/>
</td></tr>
<tr><td> 
<center>After</center>
<img width="1714" height="906" alt="image"
src="https://github.com/user-attachments/assets/b123a5e0-a232-47bf-96d5-c935295f7c0a"
/>
</td></tr>
</table>

## 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.
This commit is contained in:
Felipe Coury
2026-05-25 15:42:28 -03:00
committed by GitHub
parent 20d1b7674d
commit 14d80e55cd
10 changed files with 123 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
codex-rs/tui/src/history_cell/messages.rs
View File

@@ -283,16 +283,26 @@ impl AgentMessageCell {
impl HistoryCell for AgentMessageCell {
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
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<Line<'static>> {

10
codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__streamed_agent_list_paragraph_preserves_item_indent_when_wrapped.snap Normal file
View File

@@ -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.

24
codex-rs/tui/src/history_cell/tests.rs
View File

@@ -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";

2
codex-rs/tui/src/insert_history.rs
View File

@@ -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

14
codex-rs/tui/src/markdown_render.rs
View File

@@ -336,7 +336,7 @@ where
indent_stack: Vec<IndentContext>,
list_indices: Vec<Option<u64>>,
list_needs_blank_before_next_item: Vec<bool>,
list_item_contains_code_block: Vec<bool>,
list_item_start_line_counts: Vec<usize>,
link: Option<LinkState>,
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<String>, indent: Option<Span<'static>>) {
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();

41
codex-rs/tui/src/markdown_render_tests.rs
View File

@@ -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);

1
codex-rs/tui/src/markdown_stream.rs
View File

@@ -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(),

1
codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap
View File

@@ -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

17
codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__multiline_finding_items_are_separated_snapshot.snap Normal file
View File

@@ -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.

1
codex-rs/tui/src/streaming/controller.rs
View File

@@ -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(),