Skip to content

fix(desktop): correct incremental markdown split boundary (setext underline merge) - #67176

Merged
OutThisLife merged 2 commits into
mainfrom
bb/incremental-markdown-lex-fix
Jul 18, 2026
Merged

fix(desktop): correct incremental markdown split boundary (setext underline merge)#67176
OutThisLife merged 2 commits into
mainfrom
bb/incremental-markdown-lex-fix

Conversation

@OutThisLife

Copy link
Copy Markdown
Collaborator

Summary

Follow-up correctness fix to #67154 (incremental block lexing for streaming markdown). The streaming splitter reused the previous parse's blocks up to a boundary that dropped only the last content block before re-lexing the appended suffix. That boundary is unsound for one markdown construct: a trailing Setext underline (-/=) underlines the paragraph above it, so appending to it can retroactively merge the previous parse's last two blocks into one.

  • Minimal repro: cached "…#e\n5\n-" lexes to [ …, "#e\n", "5\n-" ], but grown to "…#e\n5\n-p2=kj:c" collapses #e/5\n- into a single block. Dropping only the last content block reused a stale "#e\n" block.
  • The blocks.join('') === text safety check can't catch this: the wrong split reconstructs the exact same source string, so there was no fallback — it rendered as mis-split blocks.

Fix

Drop the last two content blocks (skipping any whitespace-only blocks around them) before re-lexing the suffix. A Setext underline consumes exactly one preceding block, so the block before the last is the deepest an append can reach; earlier blocks stay fenced off by settled blank lines, making two sufficient and safe. Full-lex fallback behavior is unchanged.

Tests

  • Deterministic regression for the exact prev → grown pair (with a settled prefix past the append-cache threshold so the incremental path actually engages).
  • Character-level streaming property fuzz (12 seeds × 500 growing prefixes over the markdown control alphabet), asserting deep-equality with a fresh full lex at every step.

Both new tests fail on the current main boundary and pass with this fix (verified by swapping the source), so they are real guards, not tautological.

Verification

  • npx vitest run src/lib/markdown-blocks.test.ts — 6 passed (2 new)
  • npx vitest run src/components/assistant-ui/markdown-text — 35 passed
  • tsc --noEmit, eslint, prettier — clean

The streaming block splitter added in #67154 dropped only the previous
parse's trailing whitespace blocks plus its LAST content block before
re-lexing the appended suffix. That boundary is unsound: a trailing
Setext underline (`-`/`=`) underlines the paragraph ABOVE it, so
appending to it can retroactively merge the previous parse's last TWO
blocks into one.

Minimal repro: cached "…#e\n5\n-" lexes to [ …, "#e\n", "5\n-" ], but
grown to "…#e\n5\n-p2=kj:c" collapses "#e"/"5\n-" into a single block.
The reused settled prefix still contained a stale "#e\n" block. The
`blocks.join('') === text` guard can't detect this because the wrong
split reconstructs the same source string, so the divergence rendered
as mis-split blocks with no fallback.

Fix: drop the last TWO content blocks (skipping whitespace-only blocks
around them) before re-lexing the suffix. The block before the last is
the deepest an append can reach — a Setext underline consumes exactly
one preceding block — and earlier blocks stay fenced off by settled
blank lines, so re-lexing two is sufficient and safe.

Tests: a deterministic regression for the exact prev→grown pair, and a
character-level streaming property fuzz (12 seeds × 500 growing prefixes
over the markdown control alphabet). Both fail on the pre-fix boundary
and pass after. tsc/eslint/prettier clean; markdown-text suite green.
@OutThisLife
OutThisLife enabled auto-merge July 18, 2026 22:27
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Jul 18, 2026
The char-level streaming fuzz runs 12 seeds × 500 growing prefixes
(~6000 full+cached lexes) and first trips the pre-fix boundary at
seed 11 / step 257, so the workload can't shrink without gutting the
guard. The work is bounded but exceeds Vitest's 5s per-test default on
CI workers under parallelism, so give this one test an explicit 30s
timeout instead of weakening coverage.
@OutThisLife
OutThisLife merged commit bf41123 into main Jul 18, 2026
23 checks passed
@OutThisLife
OutThisLife deleted the bb/incremental-markdown-lex-fix branch July 18, 2026 22:58
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…l-markdown-lex-fix

fix(desktop): correct incremental markdown split boundary (setext underline merge)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants