fix(gui): emphasis mis-pairing let ** into linkified URLs - #359
Merged
Conversation
…rget (#197) The originally filed `**http://localhost:3003**` repro was fixed in the matcher (#210, bb623af), but the follow-up report was real — the defect lives one level up, in the Markdown inline parser. parseInline paired `**` runs with lazy regexes, so the FIRST two runs it saw won. A literal `**` earlier in the paragraph — Python `**kwargs`, an exponent like `2**32`, a `**Note:**` label followed by `x**2` — stole the `**` that actually opened a later bold span. The bold URL's closing `**` then never became markup: it stayed inside the text node handed to ChatInlineChildren, so it rendered literally (the "** doesn't render as bold" half of the report) and, having no mirrored `**` in front of the URL, sailed past markdownClosingDelimiterBefore straight into the clickable target — `http://localhost:3003**`. Every streaming prefix of such a line shows the same bad target. parser.ts: replace both star regexes with findStarEmphasis, a CommonMark-style delimiter-run scan — a run only opens when a non-space follows, only closes when a non-space precedes, and a closer binds to the NEAREST still-open run. `Pass **kwargs to the server at **http://localhost:3003**` now renders the stray `**` as literal text and bolds only the URL. Also fixes `a * b **x**`, which the old lazy em regex swallowed whole. chatLinks.ts: defense in depth, independent of caller — a URL target may never begin or end on a paired emphasis delimiter (`**`, `__`, `~~`), and the mirrored-delimiter rule now recognises `_`/`__` as well as `*`/`~~`. Only PAIRS are stripped unconditionally, so bb623af's legitimate trailing `*`/`~` (`?q=*`, `/~`) survive untouched. Regression coverage: nearest-opener pairing and flanking in parser.test.ts; static + every streaming prefix of four shifted-pairing lines in MarkdownRenderer.test.tsx; unpaired `**`/`__`/`~~` tails, `_`/`__` wrapping and underscore-bearing URLs in chatLinks.test.ts; the raw (non-Markdown) ChatInlineText surface — user messages, status/error rows, tool labels — in a new ChatInlineText.test.tsx. gui: 179/179 bun tests pass, tsc -b and eslint clean. No Zig changed; `zig build` verified green on the CI-pinned 0.17.0-dev.813+2153f8143. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbsV84fdmf39Bh2RF8LdPs
… stays in the target (#197) Two blocking defects in the #197 fix itself. parser.ts — findStarEmphasis returned the *first* pair that closed, which is the innermost one. `**bold *nested* end**` popped the `*nested*` opener at the inner closer and returned immediately, so the enclosing `**` run was never revisited and both bold markers fell back to literal text: `<p>**Note: see <em>this</em> file at <button …>…</button>**</p>`. That is the "`**…**` does not render as bold" half of the original report, reintroduced for a very common LLM output shape, plus a trailing literal `**` glued to the following text — the exact precondition for the link-target bug. The scan now keeps going and returns the *earliest-starting* pair, which is what findInline wants (parseInline emits the text before the node, the node, then recurses over both the content and the rest, so nesting is handled by the recursion). Openers after the matched run are dropped with it. Pairing also applies CommonMark's rule of three, so the interior `*` of `**x*y**` can no longer steal an asterisk from the opening run: `<strong>x*y</strong>`, matching the pre-#197 behaviour. chatLinks.ts — stripEmphasisBoundaries dropped a trailing `**`/`__`/`~~` from *every* URL, truncating targets where those characters are genuinely part of the URL: `…/datamodel.html#object.__init__` opened as `…#object.__init`, `…/glob/**` as `…/glob/`, `…/a~~` as `…/a`. Cmd-click landed on a 404. That undid bb623af, which had deliberately preserved legitimate trailing markers. The strip is now conditional on the surrounding text having actually opened that pair (a run of the same delimiters, followed by a non-space, somewhere in front of the URL). `**Note: see http://localhost:3003**` — where the opener is too far away for the mirrored-delimiter pass — still yields a clean target, while a URL that merely ends in those characters is left whole. The dead startsWith branch went with it: URL_PATTERN candidates always start with `http`. Coverage: nested/rule-of-three pairing in parser.test.ts; the rendered bold + <em> + clean-link proof and an unpaired `**`-tailed URL in MarkdownRenderer.test.tsx; opener-present vs opener-absent tails, the Python docs anchors and the full-span assertion in chatLinks.test.ts; the raw ChatInlineText surface gains the delimiter-tailed URLs it must keep. The two branch-added tests that asserted an unconditional strip ("strips an unpaired trailing ** with no leading delimiter", "strips unpaired trailing __ and ~~") encoded the regression and are replaced. gui: 185/185 bun tests pass, tsc -b and eslint clean. No Zig, Rust or iOS sources touched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbsV84fdmf39Bh2RF8LdPs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #197.
What changed
Root cause found (the follow-up was real):
**kwargsstyle unpaired openers mis-pair with a later URL's**delimiters —Pass **kwargs to the server at **http://localhost:3003**reproduces on the previous release. Fixed the emphasis pairing inparser.ts(earliest-starting pair wins; nested emphasis no longer abandons the enclosing**— review fixdd66fab), plus defense-in-depth: the linkifier never lets a URL match begin/end with paired emphasis delimiters while preserving legitimate trailing*/~(nobb623afregression). Regression tests for the mis-pairing, nesting, and streaming cases.Why
The originally filed case was fixed but the reporter kept seeing it; the missed path was delimiter pairing, not the linkifier boundary.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QbsV84fdmf39Bh2RF8LdPs