Revisit hand-rolled markdown parser: close setext / footnotes / HTML / indented-code gaps

[patrick-chinchill]

Coverage gap — documented non-parity with upstream remark.

Current state

src/chat_sdk/shared/markdown_parser.py is a ~880-line hand-rolled regex parser producing mdast-shaped output. It covers ATX headings, fenced code blocks, thematic breaks, blockquotes, ordered/unordered lists, GFM tables, inline emphasis/strong/delete/code/links/images, hard line breaks.

What it doesn't cover (listed at docs/UPSTREAM_SYNC.md:442 as "by design, won't fix"):

• Setext headings (===== / ----- underlines)
• Footnotes ([^1]: ...)
• Raw HTML nodes (block + inline)
• Indented code blocks (4-space indent)
• Escaped characters (\*, \[, \\, …)
• Backtick spans longer than 1 ( code with ` inside )

The decision is recorded at docs/DECISIONS.md:7-21. Summary of the original rationale: (1) mdast compatibility — no Python parser produces mdast natively; (2) round-trip fidelity; (3) subset-is-sufficient; (4) zero runtime deps; (5) streaming renderer needs to understand the same constructs.

Why reopen

Each "by design, won't fix" row accumulates risk:

• LLM-generated content: models frequently emit setext headings and escaped characters; these currently get rendered as raw text, breaking display on every adapter that pipes through our parser (all of them).
• Upstream drift: upstream remark handles these correctly. Every time upstream tests something that depends on them, we have to explicitly skip; the skip list grows.
• Fragility: the hand-rolled regex parser is already 880 lines and has had several correctness fixes for edge cases (see git history on shared/markdown_parser.py). Each new construct adds risk of subtle inline-span bugs; each fix risks regressing another construct.
• Downstream asks: consumers periodically ask why \*not italic\* doesn't render; the answer "we don't handle escaped characters" doesn't age well.

Options

Option A — close the gaps in the existing parser

Add setext, indented code, raw HTML, footnotes, escaped chars, and multi-backtick spans to shared/markdown_parser.py. Estimate: ~300–400 LOC, new edge cases to test. Keeps zero runtime deps.

Option B — swap the backend

Evaluate mistune (has GFM + footnote plugins) or markdown-it-py (CommonMark + plugin ecosystem) with a thin translation layer to mdast. The original "no mdast from Python libs" objection stands, but the translation layer has shrunk in scope — we only need the node shapes we already emit (paragraph, heading, code, list, listItem, blockquote, table, tableRow, tableCell, thematicBreak, emphasis, strong, delete, inlineCode, link, image, text, break). A translator for those nodes is probably ~250 LOC. The parser itself shrinks to a call into the library.

Key concern with Option B: the StreamingMarkdownRenderer in shared/streaming_markdown.py needs to understand code-fence state and inline-marker state for mid-stream closure. If the parser library is used only for static parses (after-the-fact mdast), and streaming keeps its own state machine, they could diverge. Scope this carefully.

Option C — close specific high-value gaps only

Order of ROI:

1. Escaped characters — common in LLM output; small implementation (~30 LOC).
2. Indented code blocks — common in technical content; small (~40 LOC).
3. Setext headings — common; medium (~60 LOC).
4. Multi-backtick spans — less common; small (~30 LOC).
5. Footnotes — rare in chat; defer.
6. Raw HTML — rare in chat; defer.

Option C is pragmatic: close items 1–3 in one PR, document 4–6 as remaining gaps, come back if consumers ask.

Recommendation

Start with C. Benchmark against mistune in a separate spike to see whether the translator layer lands under 250 LOC; if yes, B becomes attractive for the 0.5 cycle.

Acceptance

• Decide A / B / C (probably C, then reassess)
• If C: escaped chars + indented code + setext land with faithful tests from upstream remark test suite where available
• Remove matching rows from docs/UPSTREAM_SYNC.md non-parity table
• docs/DECISIONS.md:7-21 updated to reflect the narrowed gap list
• Benchmark: parsing a 10KB mixed-content document stays under 5ms on CI hardware

[patrick-chinchill]

Adding new evidence that strengthens the case for Option B (library swap) over Option C, and broadening the scope a bit — we're hitting three concrete streaming bugs in production whose fix lives in the sibling file src/chat_sdk/shared/streaming_markdown.py, not in markdown_parser.py. They share the same root cause (hand-rolled simplifications of upstream TS behavior), so they're naturally part of this conversation.

Three reproducers

Run against the installed SDK (verified on v0.4.26.2):

from chat_sdk.shared.streaming_markdown import StreamingMarkdownRenderer, _remend, _is_clean

# Bug 1: single-line `* item` bullet → `_remend` appends a stray `*`
assert _remend("* item one\n") == "* item one\n*"   # actual
# Expected: unchanged (line-leading `*` is a list marker, not an italic opener)

# Bug 2: odd-count bullets at finish() → corruption
r = StreamingMarkdownRenderer(wrap_tables_for_append=False)
r.push("* item one\n* item two\n* item three\n")
assert r.finish() == "* item one\n* item two\n* item three\n*"  # actual
# Sent to Slack as `markdown_text`, the trailing `*` breaks parsing

# Bug 3: GFM tables fragment across appends
r = StreamingMarkdownRenderer(wrap_tables_for_append=False)
chunks = ["Header:\n\n", "| ID", " | Status |\n", "|---|---|\n",
          "| 1 | Open |\n", "| 2 | Closed |\n"]
last = ""
deltas = []
for c in chunks:
    r.push(c)
    cur = r.get_committable_text()
    deltas.append(cur[len(last):]); last = cur
# deltas: ['Header:\n\n', '', '', '| ID | Status |\n|---|---|\n',
#          '| 1 | Open |\n', '| 2 | Closed |\n']
# Slack's `chat.appendStream` receives header+separator with zero rows in one
# append, then bare rows in subsequent appends — incremental parser doesn't
# reconnect them as a single table, renders as raw `|` pipes.

User-visible impact in Slack streaming today: * bullets show as literal asterisks, odd-count bullet lists end with a dangling *, and GFM tables render as raw | col | text instead of native Slack tables.

Why this is a port issue, not an upstream issue

The TS upstream (vercel/chat) doesn't have these bugs because:

• _remend: TS imports the actual remend npm package — import remend from "remend" — whose package docs explicitly call out handling "list markers, word-internal characters, escaped sequences" as a feature. Our _remend is docstringed "simplified Python equivalent" and the simplification is the regression.
• Tables: TS renderer has the same chunk-boundary behavior, but per Vercel's Slack streaming changelog tables in Slack streaming are an advertised feature. So either the TS renderer emits slightly different deltas, or the npm remend smooths something on the way out that our port doesn't.

Why Option C alone won't close these

Option C in this issue targets markdown_parser.py (setext, escaped chars, indented code). Those are real gaps, but none of them touch the streaming bugs above — those live in streaming_markdown.py::_remend and the chunk-boundary logic. We'd ship Option C and still be stuck on bullets/tables.

Why Option B (library swap) becomes the right answer

A single dependency on mistune / marko (or whichever wins benchmarks) gives us:

• A real CommonMark parser → closes the markdown_parser.py gaps Option C targets
• Real list-marker awareness → closes the _remend bullet bug
• A consistent AST → unblocks rewriting _remend / _close_emphasis against that AST (correct by construction) instead of run-counting characters

It's strictly more work upfront than Option C, but it's the only path that resolves all three of these production bugs in one stroke instead of accreting case-by-case patches across two files. Once the dependency is in, the hand-rolled _remend simplification just disappears.

[patrick-chinchill]

Catalog of _remend / parser divergences from upstream remend (vercel/streamdown's packages/remend/src/), now that PR #99 has closed the three production reproducers from the earlier comment. Posting these here so the Option A/B/C decision has a concrete gap list to weigh against.

Closed in #99

Surgical fix landed for the streaming side:

• _remend("* item one\n") no longer appends a stray * (list-marker awareness)
• Odd-count bullet lists at finish() no longer corrupt the tail
• _get_committable_prefix no longer emits header+separator without a body row — atomic header+separator+row delivery for Slack chat.appendStream
• Helper aligned with remend's shouldSkipAsterisk: any whitespace-flanked single * is excluded per CommonMark flanking rules (also picks up text * more, trailing *\n, bare * at end of buffer)

Remaining _remend gaps (vs packages/remend/src/emphasis-handlers.ts)

These are all in shared/streaming_markdown.py::_remend and _close_emphasis. Each is its own potential source of stray closing markers in LLM streaming output.

1. Word-internal asterisks

remend explicitly excludes * between two word characters (e.g. foo*bar). We don't. Models occasionally emit 5*3=15 or path strings with * glob characters; we'll count those and append a stray *.

2. Math-block contents ($...$, $$...$$)

remend strips math regions before counting markers (countDoubleAsterisksOutsideCodeBlocks etc.). We only strip fenced code blocks and backtick spans. A model emitting $a^* b^*$ mid-stream gets two stray * suffix appends from us.

3. Escaped sequences (\*, \_, \\, \[)

The outer _close_emphasis already handles \ skip-ahead for the marker character it's scanning, but _remend's top-level strikethrough/backtick/link counters (outside_fences.count("~~"), .count("\"), the bracket walk) don't. ~~foo~~will mis-count tildes;text [not link]` will mis-count brackets.

4. Multi-backtick code spans

We count total backticks and pair them; remend (via the CommonMark parser path) handles code with \ inside`` `` correctly. A code span using ``` `` ```-delimiters will be parsed wrong by our_strip_fenced_code` / backtick counter.

5. KaTeX / single-tilde handlers

remend has dedicated katex-handler.ts and single-tilde-handler.ts. We have no equivalent — ~text~ (single-tilde-as-subscript in some flavors) and any math gets counted as plain text or strikethrough.

6. Comparison operators

remend has comparison-operator-handler.ts to disambiguate < and > from incomplete HTML tags. We don't, which doesn't matter today because we don't try to repair HTML — but it's worth knowing the surface area.

Parser-side gaps (vs the broader CommonMark surface)

These are the original "by design, won't fix" items from docs/UPSTREAM_SYNC.md:442, restated for the Option A/B decision:

• Setext headings (===== / ----- underlines)
• Footnotes ([^1]: ...)
• Raw HTML nodes (block + inline)
• Indented code blocks (4-space indent)
• Escaped characters (\*, \[, \\, …)
• Backtick spans longer than 1 (same as _remend gap fix: lazy import cryptography — broken install without extras #4 above)

Implication for the strategy choice

The original Option A/B/C framing was scoped to markdown_parser.py. Now that we have a concrete _remend gap list as well (#1–#6 above), the decision space shifts:

• Option A (close parser gaps in hand-rolled code) doesn't address _remend gaps security: Fix all critical and high findings from security audit #1, fix: correct GitHub Actions commit SHAs #2, fix: resolve all ruff lint errors — zero warnings #5 — those live in a different module.
• Option B (swap to mistune / markdown-it-py + thin mdast translator) consolidates: the parser gaps go away, and _remend can be reimplemented against the same library's tokenizer (or kept as a runtime safety net but with library-aware token boundaries). Gaps security: Fix all critical and high findings from security audit #1–fix: lazy import cryptography — broken install without extras #4 close as a side effect.
• Option C (selective parser fixes only) leaves all of _remend security: Fix all critical and high findings from security audit #1–chore: bump to 0.0.1a3 #6 unaddressed — and as fix(streaming-markdown): list-marker awareness + table chunk-boundary (issue #69) #99 demonstrated, that's where the production bugs have actually surfaced.

The case for Option B has strengthened: it's the only path that closes both gap lists in one motion. The shape of the translator layer is now also clearer — we know exactly which node types we need to emit (the existing parser already documents them) and which streaming-safety guarantees we need to preserve (the get_committable_text contract from #99).

Suggested next step before deciding: a small spike to benchmark mistune parse-and-translate against the current hand-rolled parser on a 10KB mixed-content fixture, and prototype a remend-equivalent over mistune's token stream. If both come in under the original 250-LOC translator estimate and the 5ms parse budget, Option B becomes the clear choice for the 4.28 cycle.

---

Generated by Claude Code

[patrick-chinchill]

Bake-off data archive (PR #100)

Archiving the parser-replacement spike's measurements here so they survive any decision on whether PR #100 itself merges. The spike repo is at claude/markdown-parser-swap-spike — three CommonMark/GFM libraries (mistune 3.x, markdown-it-py 4.x, marko 2.x) evaluated as drop-in replacements for the hand-rolled shared/markdown_parser.py.

Parse-and-translate time (12KB mixed corpus, 50 iterations)

parser	median	p95	5ms budget?
baseline (hand)	2.6ms	2.7ms	✓
mistune	11.9ms	13.0ms	✗ (2.4× over)
markdown-it-py	13.4ms	20.6ms	✗ (2.7× over)
marko	46.6ms	49.6ms	✗ (9.3× over)

Important caveat: part of the baseline's perf win comes from "fewer constructs to parse per byte" — setext, indented code, multi-backtick spans, escaped chars, and raw HTML all skip straight through the inline fast-paths. An apples-to-apples comparison would require extending the baseline first.

Translator LOC (excluding blank lines + comments + docstrings)

library	LOC
mistune	149
markdown-it-py	194
marko	147

All comfortably under the 250-LOC budget. ast-based docstring exclusion via ast.walk so the count is honest.

Completeness gap on hard constructs (gap_cases.md)

"Silent drop" = construct flattened to plain text; "recognised" = correct mdast node type emitted.

construct	baseline	mistune	markdown-it-py	marko
setext heading	silent drop	recognised	recognised	recognised
indented code block	silent drop	recognised	recognised	recognised
task list item	recognised¹	silent drop	recognised	recognised
footnote definition	silent drop	silent drop	silent drop²	silent drop
inline HTML	silent drop	silent drop	silent drop	silent drop
definition list	silent drop	silent drop	silent drop	silent drop
silent-drop count	5	4	3	3

¹ Baseline matches - [x] as a list item but doesn't extract checkbox state. PR #101 adds proper checked extraction.
² markdown-it-py supports footnotes via mdit-py-plugins (not enabled in the spike); would drop the count to 2.

Happy-path mdast fidelity (mixed_content.md)

library	divergences
mistune	26
markdown-it-py	24
marko	27

Most "divergences" are cases where the baseline is less mdast-spec-compliant than the candidates (soft-break splitting, trailing-newline handling on code blocks, link-followed-by-text splitting). A library swap would fix these as a side effect.

Decision (May 2026)

Option A scoped to chat-realistic gaps, not the library swap (Option B). Rationale:

• The 5ms parse-time budget is met only by the baseline (~5× faster than the best candidate).
• mdast fidelity is roughly equivalent across all three candidates.
• The constructs the libraries handle better (setext, indented code, footnotes, raw HTML, definition lists) are almost never in LLM/chat output — they're human-authored-markdown concerns.
• The realistic chat-completeness gaps (escaped chars, task lists, math regions, multi-backtick) are small, scoped fixes to the existing parser/_remend. Shipped as PRs fix(streaming-markdown): list-marker awareness + table chunk-boundary (issue #69) #99 + fix(markdown): chat-scoped completeness — escaped chars, task lists, math/escape in _remend #101.

Triggers to revisit (when to re-run the bake-off)

1. A non-chat input surface lands — RAG, user-authored memory, markdown-file ingestion, GitHub-body parsing. Humans writing markdown means setext / footnotes / HTML / indented code start mattering.
2. A long-form artifact output surface lands — research summaries / reports with citations + footnotes + math. Parse latency is also less critical here (one-shot, not per-stream-chunk).
3. A web rendering surface for chat-sdk-python — upstream's @chat-adapter/web Python port. Browser tolerates richer markdown.
4. A new chat platform demanding richer parsing — unlikely in the near term, but possible.

Re-run playbook

If a trigger materialises:

1. Author a fixture under tests/parser_spike/fixtures/ representing the new surface's actual content (workload-shaped, not generic CommonMark).
2. Re-run pytest tests/parser_spike/test_mdast_parity.py -s and python scripts/parser_spike/benchmark.py.
3. Decision shifts toward Option B when: silent-drop count ≥6 on the new fixture, parse latency is one-shot rather than per-stream-chunk, and the team is OK adding a dependency.
4. If thresholds met, promote markdown-it-py translator (best completeness, only 1.5ms slower than mistune; mdit-py-plugins available for footnotes).

Whether to merge PR #100

The infrastructure (translators + harness + benchmark + fixtures) is ~890 LOC of dead code that would ship to PyPI. The team's call:

• Merge: keeps the infrastructure available for re-runs without re-creating it (~1 day work otherwise).
• Close: cleaner runtime surface area, no PyPI dead weight; this comment preserves the durable findings.

Either way, the spike's data is now archived here.

---

Generated by Claude Code