Repository navigation
fix(native-chat): render math and center standalone equations - #31546
brennanb2025 merged 16 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
0c3bec85-ab20-493c-9754-cb4d968d7b54
📒 Files selected for processing: 6
src/renderer/src/components/native-chat/NativeChatMarkdown.tsxsrc/renderer/src/components/sidebar/CommentMarkdown.math.test.tsxsrc/renderer/src/components/sidebar/CommentMarkdown.tsxsrc/renderer/src/components/sidebar/chat-markdown-math.tssrc/renderer/src/components/sidebar/comment-markdown-word-fade.tssrc/renderer/src/components/sidebar/markdown-top-level-blocks.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
04b055b0-4693-41e8-9f9c-0ad105756d7a
📒 Files selected for processing: 7
src/renderer/src/components/native-chat/native-chat-quote-selection.tssrc/renderer/src/components/sidebar/CommentMarkdown.math.test.tsxsrc/renderer/src/components/sidebar/CommentMarkdown.tsxsrc/renderer/src/components/sidebar/chat-markdown-math-rendering.tssrc/renderer/src/components/sidebar/chat-markdown-math-selection.tssrc/renderer/src/components/sidebar/chat-markdown-math.tssrc/renderer/src/components/sidebar/markdown-top-level-blocks.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
There was a problem hiding this comment.
🧹 Nitpick comments: 2
src/renderer/src/components/sidebar/CommentMarkdown.math.test.tsx: 2
450-454: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the debug
process.stdout.writecalls from the tests.The block at Lines 450-454 is debug output. It runs only for the
\r\randx\r>fixtures. The calls at Lines 553-555 and 685-687 print measurements that the tests already assert on. Each run therefore adds output to the CI log with no assertion value. If you need the measurements for review, put them in the assertion failure message, for example as the second argument toexpect(...).♻️ Proposed removal for the fixture-specific debug block
- if (content === source && (source.endsWith('\r\r') || source.includes('x\r> '))) { - process.stdout.write( - `Authored fence parse ${JSON.stringify({ source, value: chat.value, position: chat.position })}\n` - ) - }Also applies to: 553-555, 685-687
658-667: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the
codeFencedFencespy in afinallyblock.
vi.spyOn(enter, 'codeFencedFence')patches the sharedchatMathFromMarkdown.enterobject.observations.mockRestore()at Line 696 runs only if every earlier assertion passes. If an assertion fails, the spy stays installed. The nextit.eachcase then wraps an already wrapped handler, and later tests in the file see the patched fence handler. A failing case can therefore cause unrelated failures. Wrap the body afterspyOnintry { ... } finally { observations.mockRestore() }. Alternatively, callvi.restoreAllMocks()inafterEach.Also applies to: 696-696
ℹ️ Review info
-
⚙️ Run configuration
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b79714eb-20ac-4b06-a24d-cc0ce1935710
📒 Files selected for processing: 3
-
src/renderer/src/components/sidebar/CommentMarkdown.math.test.tsxsrc/renderer/src/components/sidebar/chat-markdown-math-nodes.tssrc/renderer/src/components/sidebar/chat-markdown-math.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
-
|
Native chat showed formulas as raw TeX. Thank you @xinzhoulvxinda for the desktop formula contribution. Additive review fixes preserve Markdown containers and authored text, keep unfinished/invalid formulas readable, retain source for Copy/quote, and bound repeated failed delimiter searches. The optional typesetter loads only after opted-in, parser-recognized complete math. Ordinary messages never request it. While loading, the original Markdown remains visible; formulas typeset when it resolves, without a placeholder or notice. Six review rounds and the single authorized loading-delta check are complete, with no open in-scope correctness or reference findings. All four review threads are resolved, including courteous replies to the two requested threads. Validation: focused parser/renderer/consumer tests cover math containers, authored code-fence closure, invalid/incomplete input, source copying, held-import resolution and operation bounds. The final quote mock correction also passes all 12 named tests. Earlier-head CI is green on The next blocking conflict was resolved by additive main merge Main’s repository-wide comment cleanup caused another blocking conflict. Additive merge Later on October 25, another blocking main conflict required additive merge Scope remains desktop native chat. Phone and editor preview are unchanged. GitHub backtick-wrapped inline math remains unsupported, tracked in #10910. Long unfinished blocks still reparse while growing; operation tests certify the stated bounds, not constant-time total streaming or browser latency. For Brennan's reviewActual branch app on Windows: seeded saved native-chat transcript, no live agents. Both inline subscripts and the display fraction render with readable surrounding prose. This chat-screen flow uses renderer tests plus one screenshot ( Two code spots for judgment: optional loading trigger and held-import behavior test. |
|
I checked this branch (3a686d0) against Chinese chat text, which is most of what my agents write, by running
The CJK price heuristic keeps |

Native chat currently shows formulas as TeX source. This contributor change adds desktop rendering while retaining authored messages for copying. The initial implementation rewrote source before Markdown parsing, which could move equations out of quotes and lists, drop text on equation fence lines, and repeatedly parse settled streaming content.
Contribution by @xinzhoulvxinda, with additive maintainer review fixes. This keeps the contributor's opt-in integration and replaces source rewriting with chat-scoped parser grammar shared by incremental splitting and rendering. Inline and display formulas keep their Markdown containers and original positions. Unfinished and invalid formulas remain readable source; complete equations use sanitized, untrusted KaTeX with bounded expansion and size. Display equations scroll horizontally, and selecting or quoting an equation returns its TeX source once. Whole-message copy continues to use authored Markdown.
Tracked in #10910; this delivers the narrower STA-11362 desktop formula integration. GitHub's backtick-wrapped inline form remains unsupported, so this does not complete the broader issue. Phone and editor preview remain unchanged. Ordinary code, links and price forms stay literal; explicit
mathcode fences typeset. Unquoted shell-variable prose such as$HOME/$USERis ambiguous under the common math grammar; use code formatting for literal shell text. Sole display formulas inside quote/list paragraphs center intentionally; equations mixed with prose or inside headings/tables stay inline.Review and validation
Full review path: 759 changed source lines, plus the English label catalog entry, above the 150-line light-path limit. This is local desktop presentation with no persisted format, process, settings, transport, server capability or mobile-code changes.
The deferred-loading correction is
e863afda12972a4685cee2485d830a951bc2ebb1. Its focused runs pass 129 tests across six explicitly named renderer/parser and consumer suites through the test queue. One held-import behavior test verifies that ordinary prose and opt-out never request the typesetter, while parser-recognized math retains source until the module resolves and then rerenders typeset. Earlier review runs cover the unchanged context-menu, cross-message Copy, quote and copy-on-select consumers. Coverage includes container exits, original source positions, invalid/incomplete formulas, authored code-fence closure, code/prices/security, and final/growing agreement.The growing-renderer contract counts 2,106 versus 23,433 parsed characters at 10 versus 100 appended blocks. Malformed dollar-run contracts retain 66/606 display and 149/1,409 inline consumes at 20/200 markers. New exhausted-search contracts cover both alias families, mixed families, barriers and distinct dollar lengths, including actual rollback and map operations. Repeated parenthesis aliases use 404/3,824 total counted operations on 82/802 characters; each tested population remains within its declared character-based bound. Long unfinished blocks still reparse as they grow; these checks do not claim constant-time total streaming or measured browser latency.
The independent review accepts the parser, container, fence and source-copy corrections. The settled loading disposition is now implemented: the math opt-in and parser recognition precede the first dynamic typesetter import. Plain Markdown remains visible until resolution, with no loading notice. The same reviewer accepts that delta and the carried whole-mechanism reference comparison. Main was merged once at
67333479a4c07d38b24b328a3ba2088ccff7e734; 148 focused integration tests across five named files and the direct merge hook pass. Main’s CJK/autolink and native image/visual/host-link composition are preserved. Both review threads are resolved. CI reported declaration and localization-boundary errors; additive correctionsf5040c47e1d4andec841203df72preserve runtime behavior, pass the normal hook, and register the English label. The final test-only correction38e17a0812543e2e68866326f3b7e5d0780016c6preserves real i18n exports in the quote-suite mock; all 12 named tests pass. Earlier CI is green: all five Node shards, static/type/localization checks, Linux and Windows packages and selection evidence pass. All four review threads are resolved. The branch-only actual-app screenshot is complete on38e17a0812543e2e68866326f3b7e5d0780016c6. It shows inline subscripts and a display fraction in the real native-chat transcript. This is a seeded saved transcript with no agents, matching the chat-screen row: renderer tests plus one screenshot. Owned processes stopped gracefully, no recorded process or configured-port listeners survived, and real config hashes stayed unchanged. A newer-main conflict required additive merge0f33e47c8c3b250dcac6547fad7c74203d68c54a. It preserved main’s pane menus and the reviewed math-selection behavior; 30 targeted tests and the direct merge check pass. The parser, typesetting and native-chat rendering files remain byte-identical to the screenshot head. Test-only commit980b5b46bba90e23c227ea83278a2a071d0e51dacorrects two inherited-main scaling expectations to require cached display reuse; both named tests and the normal hook pass. The screenshot evidence carries across this main merge and test-only commit. Earlier-head CI ond8383f578d6251ec722ed673162ce1dba9fdd216is green: all five test shards, static/type/localization checks, Linux and Windows packaging, selection evidence and the aggregate gate pass. All 33 checks are successful or skipped, the PR is out of draft, and all review threads are resolved.The next blocking conflict was resolved by additive main merge
3a686d0f39412846b39203bba9f27f772386a3be(main4c2a2ac3d93f3c542fb339a03db82d3cf9499f52). It retains main’s unknown-HTML rendering and unresolved-row handling alongside the accepted math and source-copy behavior. Both-parent composition checks, 82 targeted tests andnode_modules/.bin/lint-staged --diff=MERGE_HEAD --concurrent falsepassed before the merge commit. The core math modules remain unchanged; no flow regression was found, so the existing screenshot evidence carries without another QA run.Main’s repository-wide comment cleanup caused another blocking conflict. Additive merge
d8383f578d6251ec722ed673162ce1dba9fdd216brings in main7d18e19907adcc2fe039f2eafb4aa40dca0c707c, retaining the cleanup and the accepted math-source serializer. Executable syntax across all 18 prior TypeScript files matches the reviewed head, and all upstream comment edits are retained. The required direct merge check passed before committing with--no-verify. CI on this merged head passed; no tests or QA flows were rerun for the comment-only resolution.Later on October 25, another blocking main conflict required additive merge
90bc56a8a28ee3b3d7a5504c9c4a691807492cd4(main47ed6e1154755985d8671dd2625eed41d0a483da). The only conflict joined two English catalog groups: main’s browser-agent label and the accepted math scroll label are both retained. All 18 previously accepted source and test files remain byte-identical, preserving deferred KaTeX loading and the exhausted-search bound. The required direct merge check passed before the merge commit with--no-verify; both-parent checks found no flow regression. Existing renderer tests and the actual-app screenshot carry across this merge without a QA rerun. Merged-head CI is green: all five Node shards, static analysis and typecheck, Linux and Windows packaging, selection evidence and the aggregate gate pass. All 34 checks are successful or skipped; no retry was needed.Actual-app screenshot
Windows branch build at
38e17a081254, seeded saved native-chat transcript; no live-agent run. The full window shows inline math and a display fraction together.Contributor visual evidence
The contributor supplied Linux source-component previews, separate from the actual-app branch screenshot above:
AI disclosure
The contributor prepared this with OpenAI Codex (GPT-6), including implementation and independent review assistance. Maintainer review and fixes also use coding agents; the concrete validation is listed above.