Skip to content

fix(dashboard): render LaTeX-native math delimiters via KaTeX - #7806

Open
jeeshofone wants to merge 1 commit into
kirodotdev:mainfrom
jeeshofone:fix/7803-latex-delimiters
Open

fix(dashboard): render LaTeX-native math delimiters via KaTeX#7806
jeeshofone wants to merge 1 commit into
kirodotdev:mainfrom
jeeshofone:fix/7803-latex-delimiters

Conversation

@jeeshofone

@jeeshofone jeeshofone commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

Assistant responses containing valid LaTeX display math (\[ ... \]) or inline math (\( ... \)) render as raw source text in the dashboard chat instead of formatted equations (#7803).

Why it matters

These are the delimiters LaTeX itself uses, and exactly what models tend to emit. Any technical or mathematical conversation degrades to unreadable markup, even though the dashboard already ships a full KaTeX pipeline.

What changed (motivation → approach → change)

Symptom: valid math shows as raw text. Root cause: the markdown pipeline runs remark-math with singleDollarTextMath: false — a deliberate guard so currency strings ($9.99 … $19.95) don't get parsed as one giant math span — which leaves $$…$$ as the only recognized math form. remark-math never tokenizes \[…\]/\(…\), so they fall through as text. KaTeX itself (rehype-katex + stylesheet) is fully wired; the gap is purely delimiter recognition.

Change: a remark transform, remarkLatexDelimiters (website/src/utils/remarkLatexDelimiters.ts), registered in REMARK_PLUGINS right after remark-math. It emits remark-math's own mdast nodes (inlineMath / math, identical data.hName/hProperties/hChildren) from eligible text nodes only, so rehype-katex renders them exactly like $$ math. Because remark has already consumed CommonMark escapes when it builds a text node, the transform reads the node's RAW source slice through its position (so \[ is still visible) and re-applies escape + character-reference decoding to the prose it hands back. Display math found inside a paragraph splits the paragraph so the math block is valid flow content.

What is protected, and why it is structural rather than enumerated: code, inlineCode, html, link, linkReference, image, imageReference, definition, footnotes and frontmatter are other node types and are never visited. So fenced code (including inside blockquotes), indented code, inline code spans, link destinations, reference-link definitions and raw HTML attributes are untouched by construction — not by a scanner that has to recognise each one in source. Eligibility rules, each with a regression test:

  • The delimiter backslash must be unescaped (\\( is an escaped backslash followed by a plain paren; \\\( is an escaped backslash then a real opener).
  • Inline \( … \) needs its closer in the same text node.
  • Display \[ … \] needs whitespace-shaped delimiters that own their line ends within the node (\[ x \], \[\n…\n\]); a hugging escaped bracket (\[a\], \[REDACTED: …\], see \[ x \] here — the Jira/ADF converter's shapes) stays a literal escape.
  • A closer immediately followed by ( is link/image syntax and never converts.
  • Single forward pass (openers remembered, paired when the closer arrives), so pathological unmatched-opener or ]( junk input stays linear — the timeout-guarded tests are kept.

This is the shape arbitrated in round 6: review rounds 1–6 each surfaced one more non-prose context a source scanner had to enumerate (ADF brackets, link destinations, definitions, escape parity, indented code, and finally raw HTML attributes), a list that can never be shown complete by inspection. The plugin costs no extra parse — it runs inside the parse the renderer already performs — and follows the file's existing "read it off remark's own parse" pattern (AUTOLINK_PARSER, remarkAutolinkRules).

Alternatives considered: enabling singleDollarTextMath (reintroduces the currency-crash class the guard exists for); a standalone pre-parse mask (measured: the parse itself is super-linear on this PR's own ]( junk input, ~6.9s at 100kB — acceptable inside the render parse that already pays it, not as an additional pass).

Tests

Twenty render-level cases in MarkdownRenderer.test.tsx (144/144 in the file; 547/547 across the 35 renderer test files): display and inline math render through KaTeX (.katex-display / .katex); literal \[ survives inside fenced code, blockquoted fenced code, indented code and inline code; unmatched openers are left alone; ADF/Jira hugging and mid-sentence escaped brackets stay literal; whitespace-padded display converts; link destinations and reference-link definitions keep their escaped parens; escape parity (\\( literal, \\\( converts); the two linear-time guards against pathological unmatched-opener and ]( junk input; a raw HTML href with escaped parens survives intact while prose math beside it converts (the round-7 finding — fails on the scanner head); a paragraph is split around display math with no <pre> under a <p>; character references in prose next to converted math decode correctly.

Note: the new describe block is appended at the end of the test file — inserting it mid-file surfaced a pre-existing order-sensitivity in a neighboring test (reproducible at base with only test insertion, no source change); left for a separate report rather than folded into this fix.

Manual verification

N/A — unit coverage exercises the full render path (ReactMarkdown → remark-math → rehype-katex) through the real component in jsdom, including the exact payload from the issue.

Related Issues

Fixes #7803

Pattern harvest

Rule candidate: review-prompt
Pattern: "renderer feature enabled for only one delimiter dialect — check what producers actually emit (LLMs emit LaTeX-native \[ \]/\( \), not $$) before concluding a rendering feature works"

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

Contribution License Agreement

@jeeshofone
jeeshofone requested a review from a team September 2, 2026 05:33
@jeeshofone
jeeshofone requested a review from a team as a code owner September 2, 2026 05:33
@jeeshofone
jeeshofone requested a review from pepmach September 2, 2026 05:33
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 2, 2026
@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from 02225e0 to 3c7ee2a Compare September 2, 2026 05:56
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of 1bcd277073df145d0b1892fd9cc57fcff9da820e via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff adds no new user-visible control, label, or string — it routes already-emitted \[…\]/\(…\) content into the KaTeX rendering that $$ math already uses today (same mdast nodes, same .katex classes, same shipped stylesheet). The user-visible effect is that previously-broken raw LaTeX source in chat now renders as the formatted math the product already displays; the eligibility guards (hugging escaped brackets, code contexts, link syntax, unmatched openers) each carry a regression test, so plain prose and ADF-escaped brackets keep rendering as before. No persistent element changes form or place, so no recording is owed, and with no added control there is no blind-read gap under this lane's rules.

UX-Verdict: PASS

No new controls or copy — existing raw-LaTeX breakage now renders through the already-shipped KaTeX pipeline, with prose/code false-positive shapes each regression-tested.

[UX-REVIEWED] 1bcd277

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 1bcd277073df145d0b1892fd9cc57fcff9da820e via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound structural approach; the verbatim-tag pairing grammar is duplicated, not shared, so the two passes can disagree about what is "shown as source."

Watch

The plugin hand-rolls its own tag grammar (OPEN_TAG_RE/CLOSE_TAG_RE + a second matchingCloseIndex) instead of sharing the renderer's SINGLE_TAG_RE/singleTagName/matchingCloseIndex, and the grammars differ: SINGLE_TAG_RE accepts a quoted attribute containing > (<customBlock title="a>b">), while OPEN_TAG_RE's (?:\s[^>]*)?> rejects it. On that input the verbatim pass diverts the whole span to literal source, but the math plugin never opens a verbatim context, converts \(x\) inside it, and the surviving inlineMath node renders a KaTeX span mid-source — exactly the state the diff's own comment forbids ("a <customBlock> shown verbatim must not carry a rendered KaTeX span in the middle of its source"). Two copies of one load-bearing concept will keep drifting as either pass evolves.
Clears when: the plugin consumes the renderer's exported tag-recognition helpers (or one shared module both passes import), with a test covering a quoted-> attribute inside an unknown paired tag.

Suggestions

  • alignRawToValue's null-fallback (unreconcilable → leave literal) is the right fail-safe; state that invariant in a test name so a future "fix" doesn't flip the error direction toward math.

[DESIGN-REVIEWED] 1bcd277

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 1bcd277073df145d0b1892fd9cc57fcff9da820e via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification is done. The base has no existing \(/\[ handling (the only remark-math pipeline is MarkdownRenderer.tsx, and REMARK_PLUGINS_WITH_BREAKS spreads REMARK_PLUGINS, so both paths are covered; Mochi's ChatPanel has no math pipeline — no unfixed siblings). Two real findings: the plugin re-implements the renderer's existing tag-pairing helper with a weaker regex, and its verbatimTag option ships a default no caller uses.

First-Principles-Verdict: CONCERNS

The plugin re-spells the renderer's existing tag-pairing (matchingCloseIndex/singleTagName) with a weaker regex, and ships a verbatimTag default no caller ever uses.

Not justified as shipped

  • Item 5 — one consumer, generalized: the only caller (MarkdownRenderer.tsx:2274) always passes verbatimTag, so the VERBATIM_HTML default set and fallback (remarkLatexDelimiters.ts:610, 662) have zero consumers.
  • Item 6 — duplicate of website/src/components/MarkdownRenderer.tsx:2208: grep matchingCloseIndex finds 2 implementations, and they already diverge — the renderer's SINGLE_TAG_RE (line 2193) is quote-aware for > inside attribute values; the plugin's OPEN_TAG_RE [^>]* is not, so the two verbatim passes can disagree on the same tag.

What this change ships

Inventory (7 items) — 5 justified

Intent: make LaTeX-native \[…\]/\(…\) math render as equations in dashboard chat instead of raw source (#7803) — a FIX.

  1. \[ … \] display math renders through KaTeX instead of showing raw — justified
  2. \( … \) inline math renders through KaTeX — justified
  3. Display math inside a paragraph becomes its own block (paragraph split) — justified
  4. Escaped brackets in code, links, raw HTML, and Jira/ADF shapes stay literal — justified
  5. New exported plugin option verbatimTag with unused VERBATIM_HTML default — one consumer, generalized
  6. Second tag-pairing scanner inside the plugin — duplicate of website/src/components/MarkdownRenderer.tsx:2208
  7. Small named-entity table + raw/value alignment fallback (errs literal) — justified

Watch

  • The two tag-pairing spellings decide "is this span verbatim?" independently and already differ on quote-aware attributes; the verbatim-unknown-tags pass and this plugin can classify the same <customBlock a="b>c"> differently.
    Clears when: the plugin consumes one shared pairing helper (injected like verbatimTag) and OPEN_TAG_RE/CLOSE_TAG_RE/its matchingCloseIndex (remarkLatexDelimiters.ts:611-659) are deleted.

Subtractions

  • Delete VERBATIM_HTML and the ?? ((tag) => VERBATIM_HTML.has(tag)) fallback (remarkLatexDelimiters.ts:610, 662); make verbatimTag required — 1 caller, 0 consumers of the default.
  • Delete the plugin's OPEN_TAG_RE, CLOSE_TAG_RE and matchingCloseIndex (remarkLatexDelimiters.ts:611-612, 643-659); reuse the renderer's quote-aware singleTagName/matchingCloseIndex (MarkdownRenderer.tsx:2193-2220), injected alongside verbatimTag.

[FIRST-PRINCIPLES-REVIEWED] 1bcd277

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 1bcd277073df145d0b1892fd9cc57fcff9da820e via the fork AI-review pipeline; updated in place on each push.

2 of 2 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- website/src/utils/remarkLatexDelimiters.ts:378 -- Inline math removes surrounding soft breaks

while (text.startsWith('\n')...) / while (text.endsWith('\n')...)
first\n\(x\)\nsecond -> rewriteText strips both breaks -> prose renders glued to the equation.
Anchor: residual/crash-data-loss-corruption
Fix: Trim adjacent newlines only for display-math spans.

BLOCKING -- website/src/utils/remarkLatexDelimiters.ts:292 -- Quoted > bypasses verbatim-tag detection

const OPEN_TAG_RE = /^<([A-Za-z][A-Za-z0-9-]*)(?:\s[^>]*)?>$/
<code title="a>b">f\(x\)</code> -> opener is missed -> code contents are converted to KaTeX instead of remaining literal.
Anchor: residual/crash-data-loss-corruption
Fix: Make tag parsing quote-aware using the existing single-tag grammar.

[BLOCK-MERGE] 1bcd277
[GPT-REVIEWED] 1bcd277

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

I've read the prompt, the findings, and traced both fenced findings against the new file's source in the diff (the file is PR-added, so it exists only in the patch).

F1 (remarkLatexDelimiters.ts:378)pushText's newline-stripping loops run for every span, inline included. For first\n(x)second-shaped input the prose-before slice first\n loses its trailing soft break and \nsecond loses its leading one, gluing prose to inline math. Confirmed conditions: while (text.startsWith('\n')...)/endsWith at lines 378/383 unconditionally strip, and rewriteText calls pushText for inline spans too (line 720, span.kind === 'inline'). Recovery: none — persistent for that content. But the triggering condition (a soft break adjacent to inline math in wrapped prose) is COMMON, not extreme, so no rarity argument exists; harm is cosmetic lost-whitespace. Cannot complete a FLAG record → UPHOLD-FENCED.

F2 (remarkLatexDelimiters.ts:292)OPEN_TAG_RE's [^>]* stops at the first >, so a verbatim tag whose quoted attribute contains > (e.g. <code title="a>b">) fails the >$ anchor, the verbatim context is never opened (line 745), and interior \(x\) converts to KaTeX. Confirmed at line 292 regex and 745 isVerbatimTag. Recovery: none automatic, but source is uncorrupted and the outcome is display-only. Condition combination — raw inline HTML on a verbatim tag, a literal > inside a quoted attribute value, AND LaTeX delimiters inside — is a genuinely extreme confluence a human would plausibly accept; consequence is cosmetic misrender, not loss. Evidence record complete → FLAG.

[ADJUDICATION] 1bcd277 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 1bcd277
[ADJUDICATION-FENCED] 1bcd277 fenced=2 flagged=1
UPHOLD-FENCED F1 website/src/utils/remarkLatexDelimiters.ts:378 -- Inline math adjacent to a soft break is a common wrapped-prose shape, so the whitespace-gluing defect is not rarity-gated and cannot be flagged.
FLAG F2 website/src/utils/remarkLatexDelimiters.ts:292 -- Requires raw inline HTML on a verbatim tag carrying a literal > inside a quoted attribute plus interior LaTeX delimiters, an extreme confluence whose only outcome is a cosmetic misrender with source preserved.
[GPT-ADJUDICATED-FENCED] 1bcd277

🏷️ Fenced finding(s) machine-flagged as likely edge case

The security fence keeps these findings blocking regardless of adjudication; the only clearance path is a human override recorded by a repository writer, who must independently verify a rationale before recording it — it is machine-authored, and a wrong override on a security-class finding ships exactly the class the fence exists to stop. (This lane's comment deliberately carries no override command.)

  • F2 website/src/utils/remarkLatexDelimiters.ts:292 — Requires raw inline HTML on a verbatim tag carrying a literal > inside a quoted attribute plus interior LaTeX delimiters, an extreme confluence whose only outcome is a cosmetic misrender with source preserved.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 1bcd277073df145d0b1892fd9cc57fcff9da820e via the fork AI-review pipeline; updated in place on each push.

Review details

I've traced both candidates against the actual code.

Candidate 1 (**\[ x \]**<pre> nested in <strong> in <p> → removeChild crash): I confirmed the mechanical path — transformChildren recurses into strong, rewriteText sees the node-local raw slice \[ x \] where lineStartBefore/lineEndAfter both return true, so a display math (hName pre) node is spliced inside strong; splitParagraphs only splits on a direct math child and rehypeUnwrapBlocks only hoists direct block children of <p>, so both miss it. But the finding's defining claim is the crash, and (c) is not establishable: client-side React renders host trees via createElement/appendChild, which does not apply HTML-parser end-tag-omission, and the pipeline's rehype-raw (parse5) pass would itself normalize a <pre> descendant out of a <p>. The candidate itself concedes it "could not execute the render to confirm React actually throws rather than silently normalizing." The observable-crash outcome comes out as "might," which is a drop under the falsification bar. No existing test reproduces the actual throw either — the guards are structural.

Candidate 2 (mid-line \[ x \] after an inline sibling wrongly promoted to display): the behavior is real (node-local lineStartBefore returns true across an inline-node boundary), but the "wrong outcome" is contestable — a whitespace-padded \[ x \] after a bold/italic label is plausibly an intended display formula, and the ADF/Jira-safety shapes (\[REDACTED\]) hug their text and are already blocked by the wsAfterOpen/wsBeforeClose checks regardless. The harm is a niche mis-render on an uncommon shape, not a defect I can carry at 80+.

No new grounded findings surfaced while falsifying these.

No findings.

[OPUS-REVIEWED] 1bcd277

@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from 3c7ee2a to 5f51fca Compare September 2, 2026 06:41
@jeeshofone

Copy link
Copy Markdown
Contributor Author

Review round 1 — GPT blockings addressed in 5f51fca32630205c28c66b8879e1aa577d08232e

Blocking #1 (Jira/ADF padded escaped brackets converted to math): FIXED, without dropping display math. The suggested fix (normalize only \(…\)) would remove the issue's headline case — models emit \[…\] display math constantly. Instead, display conversion is now LINE-ANCHORED: \[ must be preceded on its line only by whitespace and \] followed only by whitespace. Mid-sentence Jira/ADF shapes (see \[ x \] here) stay literal — new regression test. Residual, stated honestly: a Jira paragraph consisting of NOTHING but a padded bracketed expression still converts; the full fix is provenance-gating (a renderer prop set only by LLM-output surfaces), which touches 46 call sites and is proposed as a follow-up rather than smuggled into this diff.

Blocking #2 (unmatched opener's closer search enters inline code): FIXED. closerIndex now skips balanced backtick spans while scanning, so a \) inside inline code can never be selected as a math closer; an unterminated span aborts the search. Regression test: a lone \( followed by code containing \) leaves the code span intact.

Blocking #3 (fence-like content line ends code protection): FIXED. A closing fence now requires the marker be followed only by whitespace (CommonMark: closing fences carry no info string), so ~~~not-close inside a tilde fence stays content. Regression test keeps \[ … \] inside such a fence literal.

Verified: 125/125 across MarkdownRenderer.test.tsx + MarkdownRenderer.adfSafety.test.tsx, tsc, eslint — green.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 2, 2026
@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from 5f51fca to b290b4d Compare September 2, 2026 09:19
@jeeshofone

Copy link
Copy Markdown
Contributor Author

Review round 2 — both lanes' blockings addressed in b290b4d51f5a48b90f2ceff292116dff9cef492b

Quadratic scan over untrusted content (GPT + Opus, convergent): FIXED by restructuring, not patching. The scanner is now a SINGLE FORWARD PASS with pending-opener state: an opener is remembered and paired when its closer arrives, so an unmatched opener costs only its own visit — no per-opener suffix rescan. Every position is visited once; the display-shape checks at pair time are bounded by their own line. Semantics preserved: fences kill pending openers (math cannot cross a fence, as before), code spans never contribute closers, and an ineligible pair consumes both ends rather than lying in wait. Regression guard: a new test feeds 50k unmatched openers (100k chars) through the function — at the old complexity that's ~1.25e9 steps and trips vitest's 5s timeout, so a quadratic reintroduction fails the suite by construction; the current pass completes in milliseconds (whole suite: 314ms of test time).

Escaped parens inside link destinations rewritten (GPT): FIXED. The scanner now recognizes ]( and skips the destination to its matching unescaped ) (escape-aware, depth-tracked, aborting at EOL), so \(/\) inside a URL are never rewritten. A pending inline opener survives the skip, preserving prior pairing behavior everywhere outside the destination. Regression test uses a disambiguation-style URL and asserts both the transform output and the rendered href are intact.

Verified: 127/127 across both renderer suites + tsc + eslint, green.

Round ledger: r1 = escape-collision family (three findings, fixed); r2 = complexity + link-destination (new families, fixed). No repeats.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 2, 2026
@github-actions github-actions Bot added the readiness: action required A blocking check or review needs attention label Sep 2, 2026
@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from 63c1a76 to e5777f9 Compare September 3, 2026 00:13
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 3, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator

@jeeshofone Thanks for staying with this one. Here is where the audit leaves it.

Nothing on main covers this yet. Every commit that touched website/src/components/MarkdownRenderer.tsx since your merge base does something else: #9224 the Formatted/Raw content-card toggle, #8996 the Mermaid source view, #8195 note-bubble whitespace, #8004 diff-fence collapse, #7969 Windows path chips, #7569 eslint cleanup. main still configures remarkMath with singleDollarTextMath: false and still has the plain const fenced = fixCodeFences(clean), so \[...\] and \(...\) keep rendering as raw text and #7803 is still open. Your normalizeMathDelimiters is the only implementation of that behavior, so the remaining scope here is the whole PR.

What it needs to land:

  1. The round-6 arbitration you asked for. Pick one and we will review that: enumerate a sixth non-prose rule for raw HTML attributes, derive the mask from the parser, or take the size-thresholded hybrid. State the choice in the PR body.
  2. A rebase. The branch is 679 commits behind main. fix(website): clamp pathological markdown nesting depth before parse #8916 inserts clean = clampNestingDepth(clean) a few lines above your fenced line in the same pre-parse chain, so expect a textual conflict there; the two changes are independent and can both land.
  3. Revert the ride-along in fixCodeFences, where the doc comment is collapsed onto the signature line. It is unrelated to this fix.
  4. Refresh the description. It declares six new tests while the diff ships 18 it() blocks, and it omits guards the diff carries: link destinations, reference-link definitions, indented code, and escaped-backslash parity.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from e5777f9 to 0dbbbb8 Compare September 10, 2026 01:16
@jeeshofone

Copy link
Copy Markdown
Contributor Author

@bolichen97 — apologies for the two-day gap on this one; that was a monitoring failure on my side, now fixed. All four items, on head 0dbbbb81d:

2. Rebased onto current main (f6d38741c, was 904 behind). Clean — the clampNestingDepth insertion you flagged sits above the fenced line and git took both without conflict; the call chain is now clampNestingDepth → fixCodeFences → normalizeMathDelimiters.

3. Ride-along reverted. fixCodeFences's doc comment is back on its own line; the diff on that function is now the call site only.

4. Description refreshed. Tests section states the 18 cases the diff ships; the guards list now includes link destinations, reference-link definitions, indented code, and escaped-backslash parity.

1. Arbitration — picked: the parser-derived route, as a remark plugin (round-6 option 2), with one correction to my own round-6 framing. I objected then that a parser mask adds an ~18s parse on the pathological input. I measured it properly this time: remark's parse IS super-linear on this PR's ](-junk input (~6.9s at 100kB) — but the renderer already performs exactly that parse to display the message. A remark plugin that rewrites \(…\) / \[…\] inside mdast text nodes runs inside that existing parse and adds no second one; only a standalone pre-parse mask would have. Text nodes exclude fenced/indented/inline code, raw HTML (the round-6 sixth context), link destinations and definitions by construction — every context rounds 1–6 enumerated one at a time — and it's the same "read it off remark's own parse, unreachable by construction" pattern this file already uses for AUTOLINK_PARSER. The enumerated scanner on this head is therefore a stopgap; the next push replaces it with the plugin and keeps the 18 tests as the behavioural contract. I've stated the pick in the PR body as requested.

@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from 0dbbbb8 to 993b56c Compare September 11, 2026 02:58
@jeeshofone

Copy link
Copy Markdown
Contributor Author

Review disposition (head 993b56c73) — the arbitrated shape, shipped

F1 (source scanner rewrites raw HTML attributes) — accepted; fixed by replacing the scanner with the remark transform picked in round 6. The scanner is gone (−209 lines). remarkLatexDelimiters (website/src/utils/remarkLatexDelimiters.ts) runs in REMARK_PLUGINS right after remark-math and emits remark-math's own mdast nodes (inlineMath / math, same data.hName/hProperties/hChildren) from eligible text nodes only. code, inlineCode, html, link, linkReference, image, definition, footnotes and frontmatter are other node types and are never visited, so fenced/indented/blockquoted code, link destinations, reference definitions and — the finding — raw HTML attributes are protected structurally rather than by re-parsing the source. Because remark has already consumed CommonMark escapes when it builds a text node, the transform reads the node's RAW source slice through its position (so \[ is still visible), and re-applies escape + character-reference decoding to the prose it hands back.

Eligibility is unchanged and now stated against the tree: unescaped delimiter backslash (escape parity), inline needs its closer in the same text node, display needs whitespace-shaped delimiters that own their line ends within the node, a closer followed by ( is never math. Display math inside a paragraph splits the paragraph so the math block is valid flow content (no <pre> inside <p>). Single forward pass, so the linear-time guards still hold.

Tests (MarkdownRenderer LaTeX-native delimiters (#7803): 17 render-level, all the previous behaviours carried over — the string-level ones were rewritten as render assertions since the string function no longer exists — plus three new: raw-HTML href with escaped parens survives intact while prose math beside it converts (fails on the previous head); a paragraph is split around display math with no <pre>/.katex-display under a <p>; character references in prose next to converted math are decoded). Whole renderer test set: 35 files, 547 passed. tsc clean after refreshing this worktree's stale node_modules; i18n-check pass.

The PR body's What changed will be refreshed to describe the plugin rather than the scanner in the next push (no code difference).

@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from 993b56c to 2ea24f6 Compare September 11, 2026 03:39
@jeeshofone

Copy link
Copy Markdown
Contributor Author

Review disposition (head 2ea24f608)

GPT F1 (per-position backward backslash scan is O(n²) on a long backslash run) — accepted, fixed. unescapedBackslashAt is gone. The forward pass now carries a run counter of consecutive backslashes ending at the current position; a delimiter backslash is one that closes an ODD run (\( yes, \\( no, \\\( yes), and the counter resets after a consumed delimiter. One visit per character, no rescans. Test: a 100,000-backslash run followed by ( x \) end renders with no math (even run → escaped backslash + plain paren) and the 100,001 variant converts; on the previous head this test times out at 15 s, on this head it takes ~0.4 s.

Opus F1 / GPT F2 (innerHTML assignment as an entity decoder) — accepted, removed (GPT flagged it inert-by-construction; Opus blocked it as a policy violation regardless — I agree the sink should not exist). Character references are now decoded with pure string work: numeric references via String.fromCodePoint (0, out-of-range and surrogate code points → U+FFFD per spec), named references via a small table of the entities that occur in prose; an unknown name is left literal, which is what CommonMark does too. No document access remains in the module. Existing test (Tom &amp; Jerry \(x\) &copy; 2026) still passes.

CI: Frontend Tests (1) / Coverage Gate red on the previous head — mine, fixed. The ](-junk linear-time test I had rewritten to go through the full render took 72 s on the CI runner: the transform is linear, but the remark parse that precedes it is super-linear on that input (I measured ~7 s/100 kB two days ago — the same figure that motivated the plugin over a second parse). The test now uses 2,000 junk tokens (well inside the timeout, both maths convert) and says why; transform-level linearity is pinned by the backslash-run test, which does not touch remark's slow path. Coverage Gate failed closed on the missing frontend shard and clears with it.

548/548 across the 35 renderer test files; tsc, eslint clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 11, 2026
@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from 2ea24f6 to 9d203f4 Compare September 11, 2026 04:15
@jeeshofone

Copy link
Copy Markdown
Contributor Author

Review disposition (head 9d203f448)

All three upheld findings accepted. They share one root — I rebuilt prose from a contiguous raw-source slice — so the fix is to that root, not to each symptom.

F1 (raw slice corrupts prose: blockquote continuation markers, entities outside the table) — accepted, fixed structurally. Prose is now always the parser-decoded node.value; raw source is used only to locate delimiters and to carry the math content itself (the one place the backslashes must survive). The two are joined by alignRawToValue, a single forward walk that maps every raw index onto the value index it produced: escapes (\x → one char), character references (&amp;&; a reference the small table cannot name but remark decoded — &Omega;Ω — is consumed onto the single code point remark emitted), and raw-only characters (blockquote > markers, stripped continuation indentation) map to nothing. If the two cannot be reconciled the node is left alone — literal, never math. Multi-line display content inside a blockquote drops the interior markers. Tests: two-line blockquote with math on each line → both convert, the visible text has no > and both lines' prose intact; &Omega; beside math → Ω. Both fail on the previous head.

F2 (text sibling inside paired raw <code> HTML converted to math) — accepted, fixed. The sibling walk tracks paired raw HTML: an opening html node whose tag is verbatim-content (code, pre, kbd, samp, var, tt, script, style, textarea, svg, math) opens a verbatim context that its matching close tag ends; text siblings inside it are skipped. Test: <code>f\(x\)</code> beside \(y\) → one KaTeX node, none inside the <code>. Fails on the previous head.

F3 (synthesized paragraphs and math nodes have no position; rehypeSourcepos mis-anchors) — accepted, fixed. Every node the transform emits carries a real position (line/column/offset, computed from a precomputed line-start table): prose pieces span their raw source; math nodes span their delimiters; a split paragraph spans its children. The single line break on either side of a display block is layout, not prose, and is trimmed so the neighbouring paragraph anchors to its own line. Test (sourcePos mode): Line one / \[ … \] / Line five → paragraphs anchored 1:1-1:9 and 5:1-5:10. Fails on the previous head (the second paragraph had no data-sourcepos). The display block itself carries no data-sourcepos — rehype-katex replaces the positioned element — exactly as remark-math's own $$ blocks behave today (checked on this head); the mdast math node does carry its span.

552/552 across the 35 renderer test files; tsc, eslint clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 11, 2026
Models emit \[ ... \] (display) and \( ... \) (inline) — LaTeX's own
delimiters — but remark-math only tokenizes dollar math, and the renderer
deliberately runs singleDollarTextMath: false (the currency guard), so
valid math displayed as raw source text (kirodotdev#7803).

normalizeMathDelimiters rewrites both forms to $$ before remark-math,
skipping fenced code blocks and inline code spans, leaving unmatched
openers alone, and preserving string length exactly (2 chars -> 2 chars)
so sourcePos coordinates stay valid.

Fixes kirodotdev#7803
@jeeshofone
jeeshofone force-pushed the fix/7803-latex-delimiters branch from 9d203f4 to 1bcd277 Compare September 11, 2026 04:55
@jeeshofone

Copy link
Copy Markdown
Contributor Author

Review disposition (head 1bcd27707)

Both upheld findings accepted (GPT F1/F2; Opus's single blocking finding is the same as GPT F1).

F1 (unbounded indexOf(';', i) before the 33-char cap — quadratic on an &-flood) — accepted, fixed. The reference scan is now a bounded forward loop over at most 33 characters from the &, so each & costs O(1) and alignRawToValue is linear in the node. Honest note on the test: a CI-sized input cannot distinguish the two by timing — V8's vectorised indexOf hides the old quadratic below roughly a million characters (I measured 955 ms vs 325 ms at 250k, both under any sane bound). The added test is therefore a gross-regression guard at 100k with a comment saying exactly that; the bound itself is what the diff shows.

F2 (remarkLatexDelimiters runs before remarkVerbatimUnknownTags, and its verbatim set omitted the containers that pass diverts to literal source) — accepted, fixed. The plugin now takes a verbatimTag predicate; the renderer passes "verbatim-content tags (code, pre, kbd, samp, var, tt, textarea, svg, math) OR any tag not in ALLOWED_TAGS" — i.e. exactly the set the verbatim pass will show as source. A context opens only when the closing sibling exists (same-tag nesting tracked, the same matchingCloseIndex rule the verbatim pass uses); an unclosed tag is a lone tag and what follows it stays prose. The predicate is injected rather than imported because the util importing ALLOWED_TAGS from the component would be a module cycle. Tests: <customBlock>\(x\)</customBlock> then \(y\) → one KaTeX node and the literal <customBlock> text (fails on the previous head); <b>bold \(x\) here</b> still converts; <customBlock> lone tag then \(x\) converts (unclosed = prose).

556/556 across the 35 renderer files; tsc, eslint clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard displays valid LaTeX as raw text instead of rendering equations

2 participants