feat(tui): render responsive Markdown tables in TUI - #20252
Conversation
fcoury-oai
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 916e73853d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c32994513b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cc4173121
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
a2e59a8 to
887947d
Compare
|
Thanks for pushing this forward. This would address a pretty painful readability gap tracked in #8259, especially for agent outputs that naturally use comparison tables or status matrices. Is there anything still blocking this PR besides rebasing/resolving conflicts and final review? If external validation would help, I can test the branch in a real Codex CLI workflow with table-heavy responses and report any resize/streaming/rendering issues. |
|
I took another pass over this branch and found one small concrete GFM behavior gap: column alignment markers are parsed by The existing complex markdown fixture already includes Validation run locally on this branch: Both passed. Primary patch for the alignment issue: diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs
index 87c70d2..2a70159 100644
--- a/codex-rs/tui/src/markdown_render.rs
+++ b/codex-rs/tui/src/markdown_render.rs
@@ -11,6 +11,7 @@ use crate::wrapping::RtOptions;
use crate::wrapping::adaptive_wrap_line;
use codex_utils_string::normalize_markdown_hash_location_suffix;
use dirs::home_dir;
+use pulldown_cmark::Alignment;
use pulldown_cmark::CodeBlockKind;
use pulldown_cmark::CowStr;
use pulldown_cmark::Event;
@@ -289,7 +290,7 @@ where
Tag::Strong => self.push_inline_style(self.styles.strong),
Tag::Strikethrough => self.push_inline_style(self.styles.strikethrough),
Tag::Link { dest_url, .. } => self.push_link(dest_url.to_string()),
- Tag::Table(_) => self.start_table(),
+ Tag::Table(alignments) => self.start_table(alignments),
Tag::TableHead | Tag::TableRow => self.start_table_row(),
Tag::TableCell => self.start_table_cell(),
Tag::HtmlBlock
@@ -389,13 +390,13 @@ where
self.needs_newline = true;
}
- fn start_table(&mut self) {
+ fn start_table(&mut self, alignments: Vec<Alignment>) {
if self.needs_newline {
self.push_blank_line();
self.needs_newline = false;
}
self.flush_current_line();
- self.table = Some(TableState::default());
+ self.table = Some(TableState::new(alignments));
}
fn end_table(&mut self) {
@@ -410,7 +411,7 @@ where
let table_width = self
.wrap_width
.map(|width| width.saturating_sub(prefix_width));
- for line in render_table_lines(&table.rows, table_width) {
+ for line in render_table_lines(&table.rows, &table.alignments, table_width) {
self.push_line(line);
self.flush_current_line();
}
diff --git a/codex-rs/tui/src/markdown_render/table.rs b/codex-rs/tui/src/markdown_render/table.rs
index 8b67e02..cfc13dd 100644
--- a/codex-rs/tui/src/markdown_render/table.rs
+++ b/codex-rs/tui/src/markdown_render/table.rs
@@ -5,6 +5,7 @@ use std::ops::Range;
use crate::render::line_utils::line_to_static;
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_line;
+use pulldown_cmark::Alignment;
use ratatui::text::Line;
use ratatui::text::Span;
use unicode_width::UnicodeWidthStr;
@@ -25,6 +26,7 @@ struct TableMetrics {
pub(super) fn render_table_lines(
rows: &[Vec<TableCell>],
+ alignments: &[Alignment],
width: Option<usize>,
) -> Vec<Line<'static>> {
if rows.is_empty() {
@@ -49,6 +51,7 @@ pub(super) fn render_table_lines(
match choose_table_layout(&normalized_rows, &widths, available_width, column_count) {
Some(candidate) => render_box_table(
&normalized_rows,
+ alignments,
&candidate.column_widths,
candidate.padding,
candidate.hard_wrap,
@@ -465,6 +468,7 @@ fn grow_columns_to_targets(widths: &mut [usize], remaining: &mut usize, targets:
fn render_box_table(
rows: &[Vec<TableCell>],
+ alignments: &[Alignment],
column_widths: &[usize],
padding: usize,
hard_wrap: bool,
@@ -479,7 +483,13 @@ fn render_box_table(
)));
for (index, row) in rows.iter().enumerate() {
- out.extend(render_table_row(row, column_widths, padding, hard_wrap));
+ out.extend(render_table_row(
+ row,
+ alignments,
+ column_widths,
+ padding,
+ hard_wrap,
+ ));
if index == 0 {
out.push(Line::from(border_line(
"├",
@@ -503,6 +513,7 @@ fn render_box_table(
fn render_table_row(
row: &[TableCell],
+ alignments: &[Alignment],
column_widths: &[usize],
padding: usize,
hard_wrap: bool,
@@ -517,12 +528,21 @@ fn render_table_row(
for line_index in 0..row_height {
let mut spans = vec![Span::from("│")];
- for (cell_lines, width) in wrapped_cells.iter().zip(column_widths) {
+ for (column_index, (cell_lines, width)) in
+ wrapped_cells.iter().zip(column_widths).enumerate()
+ {
let content = cell_lines.get(line_index);
push_padding(&mut spans, padding);
if let Some(content) = content {
- spans.extend(content.spans.iter().cloned());
- push_padding(&mut spans, width.saturating_sub(content.width()));
+ push_aligned_content(
+ &mut spans,
+ content,
+ *width,
+ alignments
+ .get(column_index)
+ .copied()
+ .unwrap_or(Alignment::None),
+ );
} else {
push_padding(&mut spans, *width);
}
@@ -535,6 +555,25 @@ fn render_table_row(
out
}
+fn push_aligned_content(
+ spans: &mut Vec<Span<'static>>,
+ content: &Line<'static>,
+ width: usize,
+ alignment: Alignment,
+) {
+ let content_width = content.width();
+ let padding = width.saturating_sub(content_width);
+ let (left_padding, right_padding) = match alignment {
+ Alignment::Center => (padding / 2, padding - padding / 2),
+ Alignment::Right => (padding, 0),
+ Alignment::None | Alignment::Left => (0, padding),
+ };
+
+ push_padding(spans, left_padding);
+ spans.extend(content.spans.iter().cloned());
+ push_padding(spans, right_padding);
+}
+
fn push_padding(spans: &mut Vec<Span<'static>>, width: usize) {
if width > 0 {
spans.push(Span::from(" ".repeat(width)));
diff --git a/codex-rs/tui/src/markdown_render/table_state.rs b/codex-rs/tui/src/markdown_render/table_state.rs
index 5cedcec..1781e23 100644
--- a/codex-rs/tui/src/markdown_render/table_state.rs
+++ b/codex-rs/tui/src/markdown_render/table_state.rs
@@ -6,12 +6,20 @@ use super::table_cell::TableCell;
#[derive(Debug, Default)]
pub(super) struct TableState {
pub(super) rows: Vec<Vec<TableCell>>,
+ pub(super) alignments: Vec<pulldown_cmark::Alignment>,
current_row: Vec<TableCell>,
current_cell: TableCell,
in_cell: bool,
}
impl TableState {
+ pub(super) fn new(alignments: Vec<pulldown_cmark::Alignment>) -> Self {
+ Self {
+ alignments,
+ ..Self::default()
+ }
+ }
+
pub(super) fn start_row(&mut self) {
self.current_row.clear();
}
diff --git a/codex-rs/tui/src/markdown_render_tests.rs b/codex-rs/tui/src/markdown_render_tests.rs
index f084640..e25276b 100644
--- a/codex-rs/tui/src/markdown_render_tests.rs
+++ b/codex-rs/tui/src/markdown_render_tests.rs
@@ -403,6 +403,44 @@ fn table_inline_links_and_html_breaks_stay_inside_table() {
);
}
+#[test]
+fn table_alignment_markers_align_cell_content() {
+ let markdown =
+ "| Left | Center | Right |\n|:-----|:------:|------:|\n| a | b | c |\n| alpha | beta | gamma |\n";
+ let rendered =
+ render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(80), /*cwd*/ None);
+ let lines = plain_lines(&rendered);
+
+ assert!(
+ lines.iter().any(|line| line.contains("│ a │ b │ c │")),
+ "left, center, and right alignment should be reflected in short cells: {lines:?}"
+ );
+ assert!(
+ lines
+ .iter()
+ .any(|line| line.contains("│ alpha │ beta │ gamma │")),
+ "alignment should preserve full-width content without truncation: {lines:?}"
+ );
+}
+
+#[test]
+fn table_alignment_markers_align_multiline_cell_content() {
+ let markdown =
+ "| Left | Centered | Righted |\n|:-----|:--------:|--------:|\n| x | a<br>bb | c<br>dd |\n";
+ let rendered =
+ render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(80), /*cwd*/ None);
+ let lines = plain_lines(&rendered);
+
+ assert!(
+ lines.iter().any(|line| line.contains("│ x │ a │ c │")),
+ "alignment should apply to the first rendered line of multiline cells: {lines:?}"
+ );
+ assert!(
+ lines.iter().any(|line| line.contains("│ │ bb │ dd │")),
+ "alignment should apply to continuation lines of multiline cells: {lines:?}"
+ );
+}
+
#[test]
fn table_raw_pipes_inside_inline_code_stay_inside_cell() {
let markdown = "| Scenario | ✅ Pass Case | ⚠️ Edge Case | ❌ Fail Case |\n|---|---|---|---|\n| Pipes | `a \\| b` | escaped pipe inside code | raw `a | b` can split |\n";
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 9c7d2d4..a838e09 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
@@ -32,7 +32,7 @@ Table below (alignment test):
┌──────┬────────┬───────┐
│ Left │ Center │ Right │
├──────┼────────┼───────┤
-│ a │ b │ c │
+│ a │ b │ c │
└──────┴────────┴───────┘
Inline HTML: <sup>sup</sup> and <sub>sub</sub>.Separately, here is a small optional robustness guard that is independent from the alignment fix. After choosing and rendering the boxed layout, it re-checks the actual rendered diff --git a/codex-rs/tui/src/markdown_render/table.rs b/codex-rs/tui/src/markdown_render/table.rs
--- a/codex-rs/tui/src/markdown_render/table.rs
+++ b/codex-rs/tui/src/markdown_render/table.rs
@@ -49,13 +49,20 @@ pub(super) fn render_table_lines(
let widths = desired_column_widths(&normalized_rows, column_count);
match choose_table_layout(&normalized_rows, &widths, available_width, column_count) {
- Some(candidate) => render_box_table(
- &normalized_rows,
- alignments,
- &candidate.column_widths,
- candidate.padding,
- candidate.hard_wrap,
- ),
+ Some(candidate) => {
+ let boxed = render_box_table(
+ &normalized_rows,
+ alignments,
+ &candidate.column_widths,
+ candidate.padding,
+ candidate.hard_wrap,
+ );
+ if table_lines_fit(&boxed, available_width) {
+ boxed
+ } else {
+ render_vertical_table(&normalized_rows, available_width)
+ }
+ }
None => render_vertical_table(&normalized_rows, available_width),
}
}
@@ -724,6 +731,10 @@ fn table_total_width(column_widths: &[usize], padding: usize) -> usize {
+ padding * 2 * column_widths.len()
}
+fn table_lines_fit(lines: &[Line<'static>], available_width: usize) -> bool {
+ lines.iter().all(|line| line.width() <= available_width)
+}
+
fn is_index_column(rows: &[Vec<TableCell>], index: usize) -> bool {
let header = rows
.first() |
|
Tested this locally on macOS arm64 from RUSTUP_TOOLCHAIN=stable cargo test -p codex-tui table --lib -- --nocaptureResult: This also covers the cases I care about for the table-rendering issue: streaming/reflow behavior, scrollback persistence, narrow-width fallback, inline code containing pipes, nested blockquote/list tables, links, emoji width handling, and local file link display. From a user perspective, this fixes a real readability gap in Codex TUI and looks substantially more complete than the smaller table-rendering alternative. I would strongly support getting this merged once maintainers are satisfied with the review details. |
887947d to
36830a3
Compare
|
Tested locally on I applied and verified the main alignment fix from @Eridanus117's comment. GFM alignment markers ( Validation:
I did not apply the optional post-render width guard, only the primary alignment fix plus CJK coverage. |
36830a3 to
5dcaeda
Compare
4 participants
Summary
Adds responsive Markdown table rendering in the TUI and preserves the raw Markdown source needed to re-render streamed and finalized transcript content after terminal resizes.
The change keeps the final in-progress Markdown block as a live, width-sensitive tail while queuing only stable blocks for scrollback. This lets tables redraw during streaming, after terminal resize, and after turn finalization without losing scrollback ordering.
It also tightens table behavior discovered during validation:
<br>content inside table cells instead of leaking URL text after the tableTesting
cargo test -p codex-tui table_readability_fallback --no-fail-fastcargo test -p codex-tui markdown_render --no-fail-fastcargo test -p codex-tui streaming::controller --no-fail-fastcargo test -p codex-tui table_resize_lifecycle --no-fail-fastjust fix -p codex-tuijust argument-comment-lintgit diff --checkcargo insta pending-snapshots --manifest-path codex-rs/tui/Cargo.tomlFull
cargo test -p codex-tui --no-fail-fastwas also run, but thecodex-tui --libtarget still aborts on the pre-existing stack overflow inapp::tests::attach_live_thread_for_selection_rejects_unmaterialized_fallback_threads; the non-lib test targets and doctests completed successfully.