Uh oh!
There was an error while loading. Please reload this page.
fix(web): stop highlighter freezes and worker spin by using the Oniguruma WASM engine - #8360
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
One finding: the new "every creation site must pass this value" invariant is not applied to the remaining highlighter creation site in apps/web/src/components/settings/SettingsFontPreviews.tsx. Details inline.
Posted via Macroscope — UI Consistency
| /** | ||
| * Always highlight with the Oniguruma WASM engine. The JavaScript regex engine | ||
| * can backtrack catastrophically on ordinary source lines (a Go comment | ||
| * containing `{}` and a non-ASCII dash pinned the renderer main thread for 12+ | ||
| * minutes; the same input tokenizes in under 10ms on WASM). The shared | ||
| * highlighter is a first-caller-wins singleton, so every creation site must | ||
| * pass this value. | ||
| */ | ||
| export const PREFERRED_HIGHLIGHTER: HighlighterTypes = "shiki-wasm"; |
There was a problem hiding this comment.
The invariant documented here ("first-caller-wins singleton, so every creation site must pass this value") is not applied everywhere: apps/web/src/components/settings/SettingsFontPreviews.tsx:80-83 still calls preloadPatchFile({ patch: DIFF_PREVIEW_PATCH, options: { diffStyle: "unified", theme } }), and its own comment above states that pipeline "always awaits the shared highlighter". Appearance settings can easily be opened before any diff/file surface mounts, in which case that call wins the race and pins the JS regex engine for the whole session — exactly the hang this PR fixes.
Suggested smallest fix: add preferredHighlighter: PREFERRED_HIGHLIGHTER to the options passed to preloadPatchFile. If the SSR options type does not accept it, please note in this comment that the SSR path builds its own highlighter rather than the shared singleton, so the invariant stated here is accurate.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Confirmed and fixed in 5e5651c — preloadPatchFile reaches DiffHunksRenderer.initializeHighlighter(), which does create the shared singleton with the shiki-js default, so this was a real missed site. preferredHighlighter: PREFERRED_HIGHLIGHTER is now passed there too (the SSR options type accepts it). Typecheck and the 2,859 apps/web unit tests pass.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The patch changes the highlighter implementation for every existing file, diff, preview, and worker path, adding a lazy WASM runtime dependency and altering application-wide tokenization behavior. The code is small and consistently wired, but the production blast radius warrants human review. You can add or adjust custom eligibility rules. Learn more. |
Dismissing prior approval to re-evaluate 5e5651c
…engine The JavaScript regex engine backtracks catastrophically on ordinary source lines. A 66-character Go comment line (containing braces and a non-ASCII dash) pinned the renderer main thread for 12+ minutes when the file preview panel restored a 476 KB / 10,915-line file: the editor's synchronous EditorTokenizer #buildStateStack loop calls tokenizeLine per line, and its 500 ms per-line limit cannot fire because vscode-textmate only checks the clock between regex matches — a single catastrophic match is uninterruptible. The whole window froze until force-quit (pingdotgg#8356). The same content tokenizes in 377 ms end-to-end on the Oniguruma WASM engine (worst single line 9.1 ms, measured via @pierre/diffs getSharedHighlighter on both engines). The shared highlighter is a first-caller-wins singleton, so every creation site must name the engine: the app's getSharedHighlighter call, the diff worker pool options, and each component options object whose FileRenderer can lazily initialize the singleton with the library's shiki-js default. PREFERRED_HIGHLIGHTER in syntaxHighlighting.ts names the decision once. The workers in DiffWorkerPoolProvider previously ran shiki-js as well, which plausibly explains the runnable-forever DedicatedWorkers in pingdotgg#3884. Verified: tsgo --noEmit clean; 2859 apps/web unit tests pass; production build emits the WASM engine as a lazy ~600 KB chunk; in the dev app the full repro file tokenizes in 385 ms in-browser where the JS engine previously hung indefinitely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e comment Macroscope review: SettingsFontPreviews' preloadPatchFile call reaches DiffHunksRenderer.initializeHighlighter, which creates the shared first-caller-wins singleton with the library's shiki-js default. Opening Appearance settings before any diff/file surface would pin the JS regex engine for the whole session. Pass PREFERRED_HIGHLIGHTER there too, and trim the incident detail out of the constant's doc comment.
5e5651c to
6384d96CompareUh oh!
There was an error while loading. Please reload this page.
Opening a large file in the file preview panel could freeze the whole window until force-quit: Shiki's JavaScript regex engine backtracks catastrophically on some ordinary source lines (repro in #8356 is a 66-char Go comment), and one catastrophic match is uninterruptible — the editor's synchronous tokenizer then pins the renderer main thread indefinitely (12+ min at ~200% CPU observed). In the read-only path the same input instead leaves the diff workers spinning forever and highlighting never renders — the symptom in #3884.
Fix: select
shiki-wasm(Oniguruma) at every site that can create a highlighter — the shared singleton (syntaxHighlighting.ts, now aPREFERRED_HIGHLIGHTERconstant), the diff worker pool, and each component options object. All must agree because the singleton is first-caller-wins andFileRenderer.initializeHighlighter()falls back to the library'sshiki-jsdefault. Same grammars and themes, so no visual change; the WASM engine loads as a lazy ~600 KB chunk (CSP already allows'wasm-unsafe-eval'). The repro file (476 KB, 10,915-line Go) tokenizes in 377 ms on WASM vs a >120 s hang on the JS engine. Superset of #3885, which switches only the worker pool; credit to it for the approach.Fixes#8356. Fixes#3884 (worker spin reproduced on macOS: four DedicatedWorkers at 100% CPU each, cleared by this change).
Before (
main): file renders with no highlighting, worker pool at ~400% CPU; the editable path freezes the tab ("Page Unresponsive"):After: same file, highlighted, UI interactive:
Tests
pnpm -C apps/web typecheck(tsgo) — cleanpnpm -C apps/web test— 281 files, 2,859 tests passpnpm lint— no errors, none in changed filespnpm -C apps/web build— succeeds; WASM engine emitted as lazy chunkBuilt with Claude Fable 5 in Claude Code.
Note
Medium Risk
Touches every syntax-highlighting and diff render path; wrong or partial rollout could leave mixed engines on the singleton, but the change is narrowly scoped to
preferredHighlighterwith no auth or data-handling impact.Overview
Fixes main-thread freezes and diff-worker CPU spin by standardizing on the Oniguruma WASM highlighter (
shiki-wasm) instead of Shiki’s JS regex engine, which could hang indefinitely on some source lines.Introduces
PREFERRED_HIGHLIGHTERinsyntaxHighlighting.ts(replacing"shiki-js"ingetSharedHighlighter) and threadspreferredHighlighter: PREFERRED_HIGHLIGHTERthrough every Pierre diffs entry point: the diff worker pool, diff panel, file preview (read-only and editable), chat turn/review diffs, PR code tab, and settings font diff preview SSR.No intended visual change (same themes/grammars); the WASM bundle loads lazily. All sites must agree because the shared highlighter is first-caller-wins.
Reviewed by Cursor Bugbot for commit 6384d96. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Switch syntax highlighter to
shiki-wasmto stop UI freezesPREFERRED_HIGHLIGHTER = "shiki-wasm"constant to syntaxHighlighting.ts to switch to the Oniguruma WASM engine.preferredHighlighter: PREFERRED_HIGHLIGHTERinto file, diff, and PR code viewer options across the web app.shiki-wasm.Macroscope summarized 6384d96.