Skip to content

text: Preserve sparse Markdown table rows - #3673

Merged
huacnlee merged 1 commit into
longbridge:mainfrom
lurenjia534:fix/sparse-markdown-table
Oct 24, 2026
Merged

huacnlee merged 1 commit into
longbridge:mainfrom
lurenjia534:fix/sparse-markdown-table

Conversation

@lurenjia534

@lurenjia534 lurenjia534 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Description

Markdown table conversion currently pads every parsed row to the header width with default TableCell values. A wide header and many one-cell body rows therefore turn O(columns + rows) input into O(columns × rows) retained cells. Each synthesized cell owns a default Paragraph with Arc-backed state. This can exhaust an embedding application's memory when it renders attacker-influenced Markdown.

Keep missing cells implicit and continue truncating excess cells to the GFM header width. The existing renderer iterates the shared column grid and reads optional cell content with row.children.get(ix), so stored placeholder paragraphs are unnecessary. Add a bounded sparse-cell regression and update the ragged-row test to cover short rows, excess cells, and an explicitly empty middle cell.

The code diff is confined to crates/base/src/text/format/markdown.rs: 48 insertions and 8 deletions. There are no public API changes.

Before and after

Measured retained TableCell counts from the actual production Markdown converter, using identical compact inputs:

Header columns Body rows Input bytes Before After
32 64 452 2,080 96
64 128 900 8,256 192
128 256 1,796 32,896 384

The new sparse-cell regression fails with the original padding and passes with the fix. No intentional OOM or large-case crash was attempted.

Following previous contributions, English report, raw logs, the temporary production-parser probe and reproduction script are preserved at an immutable commit on a separate fork evidence branch. Evidence files are not included in this code PR.

Exposure depends on a host rendering untrusted Markdown; this library is not itself a network service. This change removes the introduced retained parser-cell expansion. It does not establish a general document budget or bounded upstream parser, layout or rendering cost. Visual alignment and end-to-end hover interaction were not exercised.

How to Test

Validated on Linux against base 69a2077c plus code commit 0e7a5806:

cargo test --locked --offline -p gpui-base --features test-support --lib \
  text::format::markdown::tests::sparse_table_cell_count_tracks_source_cells -- --exact --nocapture
cargo test --locked --offline -p gpui-base --features test-support --lib \
  text::format::markdown::tests::table_rows_keep_missing_cells_implicit -- --exact --nocapture
cargo clippy --locked --offline -p gpui-base --features test-support --lib --tests -- -D warnings
cargo fmt --all --check
git diff --check

Both regression tests, Clippy and formatting passed. The recorded regression invocations omitted --locked; Clippy used it. Omit --offline when dependencies are not cached. The existing Base tests require test-support in this checkout.

The evidence branch's docs/evidence/sparse-markdown-table/reproduce.py temporarily restores the old sink, runs the same bounded probe before and after, confirms the expected regression failure/pass, and restores the source. Its complete reproduction run passed.

AI Assistance

AI assisted with analysis, patch review, bounded production-parser validation, English evidence, and commit/PR preparation. The submitted fix and regression tests originated in the supplied patch. Human maintainer review remains required.

Checklist

  • Read CONTRIBUTING and followed the repository's coding and design guidance.
  • Reviewed the focused implementation and AI-assisted validation evidence.
  • Targeted regressions, formatting and Base lib/tests Clippy passed.
  • Ran the affected native Story/example and checked visual alignment.
  • Performed macOS/Windows runtime verification.

@huacnlee
huacnlee merged commit f8f5e4a into longbridge:main Oct 24, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants