fix(markdown): stop CJK punctuation being swallowed into bare URLs - #2171
Conversation
UX Review (Fable 5) — ✅ PASSAdvisory UX-level review of UX-Verdict: PASS Invisible-plumbing fix that only repairs broken renders: CJK chat messages stop corrupting, screenshots prove both themes, and legitimate URLs like [UX-REVIEWED] 5bef225 |
Design Review (Fable 5) — ✅ PASSAdvisory design-level review of Design-Verdict: PASS Real, evidenced rendering corruption fixed at the only layer that can restore code-span pairing, with conservative evidence-gated cuts, measured rejection of the aggressive alternative, and clean reversibility. Suggestions
[DESIGN-REVIEWED] 5bef225 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsI've reviewed the diff thoroughly. Let me verify one correctness-critical aspect: that the module-level The rendered-output tests (lines 861-886) actually render through the pipeline and assert on I examined:
No semantic defect completes a consequence chain to user- or system-visible harm on the changed lines. No findings. [OPUS-REVIEWED] 5bef225 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
4f3404d to
03002a8
Compare
|
Both blocking findings on 1. Valid punctuation inside URL paths is truncated — Replaced with an evidence-based cut (
Accepted residual, locked in by a test rather than left implicit: 2. Source rewriting corrupts inline-comment coordinates — Also fixed — Design Review suggestion (advisory). Local gates on |
03002a8 to
fce708a
Compare
|
Both blocking findings on 1. Valid CJK URLs truncated before later markup —
2. Tab-indented code rewritten — Local gates on |
fce708a to
16d6942
Compare
|
Both blocking findings on Rounds 1-3 each surfaced two real defects, all in one place: the hand-rolled Structural fix: read the protected regions off remark's own parse.
Two bonus effects worth flagging: a math span is now protected (it was not before), and the old mask's conservative "blank any 4-column-indented line" rule no longer skips list continuation paragraphs — both are tests.
Local gates on |
16d6942 to
10e1c3c
Compare
`**中文文本(带括号)。**这句子继续` renders as literal asterisks today. So does every Chinese, Japanese and Korean phrase whose emphasised run ends in ideographic punctuation — which, in CJK prose, is most of them. Root cause is a known CommonMark defect (commonmark/commonmark-spec#650), not our renderer. A closing `**` is only right-flanking when it is NOT preceded by punctuation, or IS followed by whitespace/punctuation. Here it is preceded by `。` and followed by the letter `这`, so it fails both clauses and cannot close. English sidesteps this by writing `**bold.** tail` with a space; CJK cannot, because the space is visibly wrong. Adopts the two upstream plugins that implement the CJK-friendly flanking amendment — the same pair Vercel's Streamdown ships as `@streamdown/cjk`: - `remark-cjk-friendly` runs BEFORE remark-gfm; it changes how emphasis delimiters are classified. - `remark-cjk-friendly-gfm-strikethrough` runs AFTER remark-gfm; it extends gfm's own `~~` construct. Order is load-bearing, so the tests assert rendered DOM rather than plugin presence: they fail if either plugin is dropped OR mis-ordered. Deliberately NOT adopted from the same upstream package: its autolink boundary handling. That splits a URL at the FIRST character in a fixed CJK punctuation set, unconditionally, which was measured to break real links — `…/wiki/苹果(公司)` becomes `…/wiki/苹果`, `…/wiki/モーニング娘。` becomes `…/wiki/モーニング娘`, `…/wiki/我,机器人` becomes `…/wiki/我`. It also cannot repair the code-span pairing that a swallowed backtick shifts, because it runs after tokenization. That problem is handled separately in #2171. Verified against the whole suite: 11356 pass. The one red, `catalogParity ko`, is pre-existing on main (ko.json is missing 101 `_one` plural keys as of #2306) and touches no file in this diff.
0219fbd to
bc2a408
Compare
`**中文文本(带括号)。**这句子继续` renders as literal asterisks today. So does every Chinese, Japanese and Korean phrase whose emphasised run ends in ideographic punctuation — which, in CJK prose, is most of them. Root cause is a known CommonMark defect (commonmark/commonmark-spec#650), not our renderer. A closing `**` is only right-flanking when it is NOT preceded by punctuation, or IS followed by whitespace/punctuation. Here it is preceded by `。` and followed by the letter `这`, so it fails both clauses and cannot close. English sidesteps this by writing `**bold.** tail` with a space; CJK cannot, because the space is visibly wrong. Adopts the two upstream plugins that implement the CJK-friendly flanking amendment — the same pair Vercel's Streamdown ships as `@streamdown/cjk`: - `remark-cjk-friendly` runs BEFORE remark-gfm; it changes how emphasis delimiters are classified. - `remark-cjk-friendly-gfm-strikethrough` runs AFTER remark-gfm; it extends gfm's own `~~` construct. Order is load-bearing, so the tests assert rendered DOM rather than plugin presence: they fail if either plugin is dropped OR mis-ordered. Verified to discriminate: with both plugins removed, exactly the four CJK tests fail and the four ASCII/code-span control tests still pass. Deliberately NOT adopted from the same upstream package: its autolink boundary handling. That splits a URL at the FIRST character in a fixed CJK punctuation set, unconditionally, which was measured to break real links — `…/wiki/苹果(公司)` becomes `…/wiki/苹果`, `…/wiki/モーニング娘。` becomes `…/wiki/モーニング娘`, `…/wiki/我,机器人` becomes `…/wiki/我`. It also cannot repair the code-span pairing that a swallowed backtick shifts, because it runs after tokenization. That problem is handled separately in #2171. Full suite on this base: 853 files, 11357 tests, zero failures. `tsc -b` clean.
`**中文文本(带括号)。**这句子继续` renders as literal asterisks today. So does every Chinese, Japanese and Korean phrase whose emphasised run ends in ideographic punctuation — which, in CJK prose, is most of them. Root cause is a known CommonMark defect (commonmark/commonmark-spec#650), not our renderer. A closing `**` is only right-flanking when it is NOT preceded by punctuation, or IS followed by whitespace/punctuation. Here it is preceded by `。` and followed by the letter `这`, so it fails both clauses and cannot close. English sidesteps this by writing `**bold.** tail` with a space; CJK cannot, because the space is visibly wrong. Adopts the two upstream plugins that implement the CJK-friendly flanking amendment — the same pair Vercel's Streamdown ships as `@streamdown/cjk`: - `remark-cjk-friendly` runs BEFORE remark-gfm; it changes how emphasis delimiters are classified. - `remark-cjk-friendly-gfm-strikethrough` runs AFTER remark-gfm; it extends gfm's own `~~` construct. Order is load-bearing, so the tests assert rendered DOM rather than plugin presence: they fail if either plugin is dropped OR mis-ordered. Verified to discriminate: with both plugins removed, exactly the four CJK tests fail and the four ASCII/code-span control tests still pass. Deliberately NOT adopted from the same upstream package: its autolink boundary handling. That splits a URL at the FIRST character in a fixed CJK punctuation set, unconditionally, which was measured to break real links — `…/wiki/苹果(公司)` becomes `…/wiki/苹果`, `…/wiki/モーニング娘。` becomes `…/wiki/モーニング娘`, `…/wiki/我,机器人` becomes `…/wiki/我`. It also cannot repair the code-span pairing that a swallowed backtick shifts, because it runs after tokenization. That problem is handled separately in #2171. Full suite on this base: 853 files, 11357 tests, zero failures. `tsc -b` clean.
bc2a408 to
fd85640
Compare
|
The blocking finding on Why not the literal suggested fix. The suggestion was to drop backtick adjacency as boundary evidence entirely. That would remove the fix for the case this PR exists to solve: the reported message is What changed. Rule 2 now applies only to separator-class marks. Sentence-enders A mixed run resolves the conservative way with no extra code: the loop skips the sentence-ender and reaches the separator on a later iteration, so Accepted residual, locked in by a test rather than left implicit. A genuine prose Discriminating power verified. Removing the one-line narrowing makes exactly the two new tests fail ( Also relevant to this thread. Rebased onto current main (level, 0 behind). Local gates on |
Reported from the dashboard chat transcript:
**#2137 — review-ready**(https://github.com/kirodotdev/KiroCrew/pull/2137,`96ed647b`):`readiness: passed`
GFM's autolink-literal extension ends a bare `https://…` run only at ASCII
whitespace or `<`. CJK prose writes punctuation directly after a URL with no
space, so `,`96ed647b`):`readiness` all lands inside the href.
The broken href is the smaller half of the damage. The run also eats the
OPENING backtick of the code span that follows, which shifts every later
backtick pairing in the paragraph by one: prose renders as inline code and real
code renders with literal backticks. One missing space corrupts the rest of the
message.
Fixed at the SOURCE level, before micromark tokenizes — re-splitting the link
node on the mdast fixes the href but cannot restore the code-span pairing,
because that pairing is decided while the whole paragraph is tokenized. The URL
head is re-emitted as an angle autolink `<url>`, which has an explicit end and
renders identically.
Which regions are off-limits is read off remark's OWN parse (same plugin set as
the render pipeline), so code, existing links, raw HTML and math are excluded by
construction rather than by a hand-rolled scanner.
The cut is EVIDENCE-BASED, because CJK punctuation reaches real URLs raw:
1. A CJK closing bracket that closes an opener left unclosed in the PROSE before
the URL — GFM's own paren-balancing rule generalised. `(https://x.com/a)`
and `(见 https://x.com/a)` cut; `https://x.com/苹果(公司)` does not, and
neither does `https://x.com/search?q=foo)` (nothing opened a bracket).
2. A SEPARATOR-class CJK mark immediately followed by a BACKTICK — the
destructive case, and the backtick is the one character RFC 3986 excludes so
it cannot be inside a raw-written URL.
Sentence-enders (`。.!?…。`) are excluded from rule 2. Real page titles end in
them and reach the URL raw — `…/wiki/モーニング娘。`, `…/wiki/魔法先生ネギま!` —
so cutting there would point the anchor at the wrong article. Separators like
`,`、`、`、`;`、`:` do not end titles, so they stay eligible, which is what
keeps the reported case fixed. A mixed run cuts at the separator and leaves the
sentence-ender inside the URL: `…/a。,`c`` gives `<…/a。>,`c``.
Accepted trade, locked in by tests rather than left implicit: a genuine prose
`。` directly before a code span keeps today's behaviour, and so does
`…/pull/1,然后回来` (indistinguishable from `…/wiki/我,机器人`).
Not adopted: `@streamdown/cjk`'s autolink handling, measured at 1.0.3. It splits
at the FIRST character in a fixed 20-mark set with no evidence test, which
breaks `…/wiki/苹果(公司)`, `…/wiki/モーニング娘。` and `…/wiki/我,机器人`; it
also runs after tokenization so it cannot repair the code-span pairing, and it
mis-splits explicit `<…>` autolinks.
34 tests. Full suite on this base: 877 files, 11829 tests, zero failures.
Visual evidence under `temp-screenshots/cjk-autolink-boundary/`, captured by
`website/scripts/capture-cjk-autolink.mjs`: the REAL built SPA behind the shared
fixture server, once from this branch and once from a dist built with
origin/main's `MarkdownRenderer.tsx`. The frame carries the reported line, the
bracket rule, and a real article URL containing `(公司)` that must survive
untouched in both builds.
fd85640 to
5bef225
Compare
`**中文文本(带括号)。**这句子继续` renders as literal asterisks today. So does every Chinese, Japanese and Korean phrase whose emphasised run ends in ideographic punctuation — which, in CJK prose, is most of them. Root cause is a known CommonMark defect (commonmark/commonmark-spec#650), not our renderer. A closing `**` is only right-flanking when it is NOT preceded by punctuation, or IS followed by whitespace/punctuation. Here it is preceded by `。` and followed by the letter `这`, so it fails both clauses and cannot close. English sidesteps this by writing `**bold.** tail` with a space; CJK cannot, because the space is visibly wrong. Adopts the two upstream plugins that implement the CJK-friendly flanking amendment — the same pair Vercel's Streamdown ships as `@streamdown/cjk`: - `remark-cjk-friendly` runs BEFORE remark-gfm; it changes how emphasis delimiters are classified. - `remark-cjk-friendly-gfm-strikethrough` runs AFTER remark-gfm; it extends gfm's own `~~` construct. Order is load-bearing, so the tests assert rendered DOM rather than plugin presence: they fail if either plugin is dropped OR mis-ordered. Verified to discriminate: with both plugins removed, exactly the four CJK tests fail and the four ASCII/code-span control tests still pass. Deliberately NOT adopted from the same upstream package: its autolink boundary handling. That splits a URL at the FIRST character in a fixed CJK punctuation set, unconditionally, which was measured to break real links — `…/wiki/苹果(公司)` becomes `…/wiki/苹果`, `…/wiki/モーニング娘。` becomes `…/wiki/モーニング娘`, `…/wiki/我,机器人` becomes `…/wiki/我`. It also cannot repair the code-span pairing that a swallowed backtick shifts, because it runs after tokenization. That problem is handled separately in kirodotdev#2171. Full suite on this base: 853 files, 11357 tests, zero failures. `tsc -b` clean.
What it looks like
Captured by
website/scripts/capture-cjk-autolink.mjs— the REAL built SPAbehind the repo's shared fixture server, so the bubble goes through the actual
remark/rehype pipeline. One run from this branch, one from a dist built with
origin/main'sMarkdownRenderer.tsx. Three lines in one frame: the reportedmessage, the bracket rule, and a real article URL containing
(公司)that must survive untouched in BOTH builds.
Before (
origin/main)After (this branch)
Light theme: before · after
Read the frames line by line:
,and eats the code span's opening backtick;readiness: passedrenders with literal backticks/2137; both code spans render as code(详见 …/pull/2137))and the rest of the sentence land inside the href)后面还有正文。is prose again…/wiki/苹果(公司)Not pictured, deliberately:
…/wiki/モーニング娘。紹介``. A sentence-enderdirectly before a backtick is left alone, so it renders the same in both builds.
That trade is stated below and locked by a unit test rather than photographed.
Problem
Reported from the dashboard chat transcript. This message:
renders as:
GFM's autolink-literal extension ends a bare
https://…run only at ASCIIwhitespace or
<. CJK prose writes punctuation directly after a URL with nospace, so
,96ed647):readiness` all lands inside the href.Why it matters
The broken href is the smaller half of the damage. The run also eats the
opening backtick of the code span that follows, which shifts every later
backtick pairing in the paragraph by one — so prose renders as inline code
(the whole tail of the message goes monospace) and real code renders with
literal backticks. One missing space corrupts the rest of the message, and
every Chinese/Japanese message that cites a URL hits it.
Fix (symptom → root cause → change)
Symptom is a wrong href plus a corrupted paragraph. Root cause is a tokenizer
boundary: the run has no end until whitespace. So the change gives it one —
fixCjkAutolinkBoundariesruns on the markdown source, beside the existingfixCodeFences/stripStrayTagspreprocessors, and re-emits the URL head as anangle autolink
<url>(explicit end, identical rendering).Source level is not a shortcut: re-splitting the
linknode on the mdast fixesthe href but cannot restore the code-span pairing, because that pairing is
decided while micromark tokenizes the whole paragraph. The boundary has to exist
before parsing.
Two questions have to be answered, and they are answered by different means.
Which regions are off-limits — read off remark's own parse
autolinkLiteralSpans()parses the source with the same plugin set the renderpipeline uses (
remark-parse+remark-gfm+remark-mathwithsingleDollarTextMath: false) and returns the source offsets of GFMautolink-literal nodes only.
Everything that must not be rewritten — fenced code, indented code, inline-code
spans including multi-line ones, existing links and images, angle autolinks, raw
HTML tags, math — never becomes such a node, so it is excluded by
construction. An earlier revision hand-rolled a masking scanner for this; three
review rounds found six real defects in it (tab-stop indentation, blockquoted
fences, multi-line code spans, closing-fence validity, …), which is a signal
about the approach rather than the cases. Deleting the scanner in favour of the
parser closes the whole class, and picked up two behaviours the mask never had:
math spans are protected, and list continuation paragraphs are no longer
skipped by a blanket "any 4-column-indented line is code" rule.
Angle autolinks and
[text](url)links can also satisfytext === url, so theliteral test is on the source text at the node's start, not on node shape alone.
A parse failure returns no spans — a malformed message renders unfixed rather
than not at all.
Where the URL ends — evidence, not characters
CJK punctuation is not by itself proof that a URL ended — it reaches real
URLs raw (
…/wiki/苹果(公司),…/wiki/我,机器人,…/wiki/モーニング娘。areall live articles that GFM links correctly today). So a cut needs one of two
pieces of evidence:
A CJK closing bracket that closes an opener SURROUNDING the URL — one
left unclosed in the prose before the URL on its line, and not opened
inside the run. Prose is the operative word: the parse also yields a mask of
every non-prose node (
inlineCode,code, non-literal links, images,html,math, link definitions), and those characters are blanked out of the prefix, so
a
(in a code sample or an HTML attribute cannot supply the opener.Autolink literals are not pre-masked, because a greedy run's span is not
uniformly URL —
(https://a/1)和【https://b/2】is ONE run whose)和【isreal prose. Instead each URL's own characters are masked as the scan consumes
them: a run that yields no cut is masked whole (so a
(in its query stringstill cannot pose as context), while a run that yields a cut masks only the
head, leaving the inter-URL prose visible to the next URL's bracket balance. This
is GFM's own ASCII paren-balancing rule, generalised.
(https://x.com/a)and(见 https://x.com/a)cut.https://x.com/苹果(公司)does not (the openeris inside the URL), and neither does
https://x.com/search?q=foo)— nothingthere opened a bracket, so it is plausibly part of the query and GFM links it
correctly today.
A SEPARATOR-class CJK mark IMMEDIATELY followed by a BACKTICK. The backtick is the
one character RFC 3986 excludes (browsers percent-encode it), so unlike
*,[or]— all legal and common in query strings (?q=foo,*test,?filter[name]=x) — it cannot plausibly be inside a raw-written URL. It isalso the character whose loss does the real damage: the run eats an opening
code-span delimiter and every later backtick pairing in the paragraph shifts.
Directly-after (not anywhere-later) matters:
…/wiki/我,机器人简介has the markup in the same whitespace-delimited run, but the comma is followed by more *title*, so it must not cut. A contiguous punctuation run counts as one boundary and the cut lands at its start, so `…/a、,`cdoes not leave、inside the href.Sentence-enders are excluded from rule 2.
。.!?…。end realpage titles and reach the URL raw —
…/wiki/モーニング娘。,…/wiki/魔法先生ネギま!— so cutting there would point the anchor at thewrong article. Separators (
,、;:·~“) do not end titles,so they stay eligible, which is what keeps the reported case fixed. A mixed run
resolves conservatively:
…/a。,cyields `<…/a。>,`c, leaving thepossibly-title
。inside the href.Vercel's
@streamdown/cjk@1.0.3takes the aggressive side of this same trade —first mark in a fixed 20-character set, no evidence test. Measured, it breaks
…/wiki/苹果(公司),…/wiki/モーニング娘。and…/wiki/我,机器人,cannot repair code-span pairing (it runs after tokenization), and mis-splits
explicit
<…>autolinks. Independent evidence that the unconditional cut is thewrong default.
Documented limitation:
…/pull/1,然后回来— a bare CJK sentence running offa URL with no space and no markup — is left alone, and so is
…/a,**注意**. It is character-for-characterindistinguishable from a legitimate
…/wiki/我,机器人, so it keeps today'sbehaviour rather than risking a correct link. A test locks that in so it reads as
a decision, not an oversight.
Other guards
contains a dot; neither of the last two labels contains
_), sohttp://localhost:5476/a,cand `http://a_b.com/x,`care left alone.?!.,:*_~plus an unbalanced)), which an angle autolink would otherwise keep.fixCodeFences. That preprocessor does not only escapeN.lines — later passes create code blocks the raw source did not have (blank
line before a fence glued to preceding text; splitting a closing fence glued to
trailing text). Rewriting first would judge such a region as prose and leave a
literal
<…>inside what ends up displayed as code. Both directions areasserted in a test.
sourcePosis on. That surface maps a DOM selectionback to source coordinates through
data-sourceposfor inline commenting;inserting two characters shifts every later column on the line and would
anchor a comment to the wrong occurrence. The inline-comment surface keeps the
unfixed but coordinate-accurate render.
(https://a/1)和(https://b/2)is ONE run, so the scan resumes after each head.https://nested inside another URL's path (?u=https://…) is never cutseparately.
Scope:
http(s)://only. Scheme-lesswww.literals have the same flaw butcannot be closed with
<…>, which requires a scheme — noted in the code.Dependencies
unifiedandremark-parsebecome declared dependencies ofwebsite/. Bothwere already resolved in the tree as
react-markdowntransitives, sopackage-lock.jsongains 2 lines and no new package is downloaded.Tests
website/src/test/cjkAutolinkBoundaries.test.tsx— 34 tests in four groups:sits earlier on the line rather than adjacent, a second URL whose opener sits in
the prose INSIDE the same greedy run, every
non-bracket punctuation class with a markdown-active follower, emphasis and
link followers, contiguous punctuation runs, multi-URL runs, GFM tail trimming,
balanced ASCII parens.
the CJK-titled URL with a code span attached in the same run, the
bare-sentence limitation, every protected region (fenced, blockquoted fenced,
fence-content-that-looks-like-a-closer, indented, quoted-indented,
tab-indented, mixed-space-tab-indented, single-line and multi-line inline
code, math, existing links/images/autolinks/definitions), and the two
must-not-linkify hosts, the three URL shapes that carry
*/[/]legitimately, three shapes where the only
(lives in a non-prose region(earlier URL query, inline code, HTML attribute), and three unmatched-closer
shapes (no opener at all,
a different bracket type, and an opener already closed before the URL). Two
positive controls prove prose inside a blockquote and inside an indented list
continuation is still fixed.
text```sh) proves the rewrite mustrun after
fixCodeFences, asserting both that the URL survives and that no<https://is inserted.(
container.textContentcontains no literal backtick), and theモーニング娘。article still resolves to the right percent-encoded page.Manual verification
N/A — the defect is textual (href value plus code-span pairing) and the
rendered-DOM assertions pin both exactly. No UI surface changed.