From 3f67dd1204bf41c3d551032d8f11f3a695270c1b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 20:36:33 +0000 Subject: [PATCH 1/2] docs: investigate windowed syntax highlighting in Pierre Large added files freeze the terminal because Pierre highlights a whole file in one uninterruptible call. Confirm why: renderDiffWithHighlighter already carries a full row-window walk, but discards startingLine and totalLines unless the caller also asked for plain text, because nothing carried TextMate grammar state across a window boundary. Shiki 3.x already exposes that state, and Pierre already uses it for streaming. Add a patch against Pierre's TypeScript source that honors an explicitly requested window and threads grammar state per side, verified with their own toolchain, plus a benchmark that checks windowed output byte-for-byte across three languages, five diff shapes and a partial patch. The longest uninterruptible call drops from 661ms to 28ms on an 8000-line added file at 250-row windows. A companion investigation into worker offload does better, so this lands as an upstream contribution rather than as Hunk's own fix. Nothing in src changes and the patch is not activated. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JgbAM65ZsLbsjgqZS8Dtav --- ...erre-chunked-highlighting-investigation.md | 4 + benchmarks/README.md | 2 + benchmarks/pierre-windowed-highlight.ts | 345 ++++++++++ docs/pierre-chunked-highlighting.md | 218 ++++++ docs/pierre-windowed-highlight-pr.md | 174 +++++ package.json | 1 + patches/@pierre%2Fdiffs@1.2.2.patch | 132 ++++ .../pierre-upstream-windowed-highlight.patch | 632 ++++++++++++++++++ 8 files changed, 1508 insertions(+) create mode 100644 .changeset/pierre-chunked-highlighting-investigation.md create mode 100644 benchmarks/pierre-windowed-highlight.ts create mode 100644 docs/pierre-chunked-highlighting.md create mode 100644 docs/pierre-windowed-highlight-pr.md create mode 100644 patches/@pierre%2Fdiffs@1.2.2.patch create mode 100644 patches/pierre-upstream-windowed-highlight.patch diff --git a/.changeset/pierre-chunked-highlighting-investigation.md b/.changeset/pierre-chunked-highlighting-investigation.md new file mode 100644 index 000000000..48558c3c9 --- /dev/null +++ b/.changeset/pierre-chunked-highlighting-investigation.md @@ -0,0 +1,4 @@ +--- +--- + +Investigate chunked syntax highlighting in `@pierre/diffs`: docs, a proof-of-concept patch, and a benchmark. No user-visible change. diff --git a/benchmarks/README.md b/benchmarks/README.md index 69201ed33..a97226c56 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -37,6 +37,7 @@ bun run bench:working-tree-load bun run bench:changeset-parse bun run bench:render-layout bun run bench:highlight-prefetch +bun run bench:pierre-windowed-highlight bun run bench:large-stream bun run bench:interaction-latency bun run bench:non-ascii-stream @@ -58,6 +59,7 @@ bun run bench:competitors - `changeset-parse.ts` — measures patch normalization, Pierre parsing, patch chunking, and normalized `DiffFile` construction for many-small-files, balanced, and large-single-file patches. - `render-layout.ts` — measures pure split/stack row building, section geometry, and review-plan construction for many-small-files, balanced, and large-single-file streams. - `highlight-prefetch.ts` — measures selected-file highlight startup and adjacent prefetch readiness. +- `pierre-windowed-highlight.ts` — compares Pierre's whole-file highlight call against the same diff rendered as row windows, reporting the longest uninterruptible call, then sweeps TypeScript/Python/CSS across five diff shapes and a partial patch to check windowed output byte-for-byte. Windowing needs the proof-of-concept patch in `patches/`; see `docs/pierre-chunked-highlighting.md`. Without it the script reports whole-file cost only. - `large-stream.ts` — measures large split-stream first-frame and scroll cost. - `interaction-latency.ts` — measures per-press `]` hunk-navigation latency and per-scroll-tick latency (median + p95) on the large stream, plus RSS/heap ceilings after first frame and after navigation (the default-suite slice of `memory.ts`). - `non-ascii-stream.ts` — measures first-frame and per-scroll-tick latency on a stream whose diff content embeds CJK, emoji, and box-drawing characters, exercising the string-width path on content rather than chrome glyphs. diff --git a/benchmarks/pierre-windowed-highlight.ts b/benchmarks/pierre-windowed-highlight.ts new file mode 100644 index 000000000..6b1bc15c9 --- /dev/null +++ b/benchmarks/pierre-windowed-highlight.ts @@ -0,0 +1,345 @@ +/** + * Measures whole-file versus windowed syntax highlighting for large contiguous diffs. + * + * Pierre's `renderDiffWithHighlighter` highlights a file in one uninterruptible call, so a large + * added file freezes the terminal for as long as that call runs. This benchmark reports the cost of + * that call next to the cost of rendering the same diff as a sequence of row windows, and checks + * that the windowed output is byte-identical to the whole-file output. + * + * Windowed highlighting needs the change in `patches/@pierre%2Fdiffs@1.2.2.patch`; see + * `docs/pierre-chunked-highlighting.md`. Without that patch the benchmark still runs and reports + * the whole-file cost, plus the fact that windowing was ignored. + */ +import { readdirSync, readFileSync, statSync } from "node:fs"; +import { join } from "node:path"; +import { + getHighlighterOptions, + getSharedHighlighter, + parseDiffFromFile, + parsePatchFiles, + renderDiffWithHighlighter, + type FileDiffMetadata, +} from "@pierre/diffs"; + +const WINDOW_SIZES = [250, 500, 1000]; +const SAMPLE_ROOTS = ["src/ui/diff", "src/ui/components", "src/core/review"]; +const SAMPLE_COUNT = 4; +const SYNTHETIC_LINES = 8_000; + +const renderOptions = { + theme: "pierre-dark" as const, + useTokenTransformer: false, + tokenizeMaxLineLength: 1_000, + lineDiffType: "word-alt" as const, + maxLineDiffLength: 10_000, +}; + +interface WindowedResult { + code: { deletionLines: unknown[]; additionLines: unknown[] }; + worst: number; + total: number; + windows: number; +} + +/** Collect the largest TypeScript sources in the repo so the benchmark uses real code. */ +function collectSamples() { + const files: Array<{ path: string; text: string; lines: number }> = []; + + for (const root of SAMPLE_ROOTS) { + let entries: string[]; + try { + entries = readdirSync(root); + } catch { + continue; + } + + for (const entry of entries) { + const path = join(root, entry); + if (!statSync(path).isFile()) continue; + if (!/\.tsx?$/.test(entry) || entry.includes(".test.")) continue; + const text = readFileSync(path, "utf8"); + files.push({ path, text, lines: text.split("\n").length }); + } + } + + const largest = files.sort((a, b) => b.lines - a.lines).slice(0, SAMPLE_COUNT); + // Repo sources top out around 2.5k lines; a generated file covers the size where the whole-file + // call stops being a stutter and starts being a visible freeze. + const generated = syntheticSource(SYNTHETIC_LINES); + // The name drives Pierre's language detection, so it has to stay a plausible source path. + return [{ path: "synthetic-generated.ts", text: generated, lines: SYNTHETIC_LINES }, ...largest]; +} + +/** Generate TypeScript whose comments and template literals span many lines. */ +function syntheticSource(lines: number, seed = 0) { + const out: string[] = []; + let index = seed; + + while (out.length < lines) { + out.push(`/** Handler ${index}`); + out.push(` * continues across lines`); + out.push(` */`); + out.push(`export function handler${index}(input: { id: string; count: number }): string {`); + out.push(` const label = \`item-\${input.id}`); + out.push(` -\${input.count}\`;`); + out.push(` if (input.count > ${index % 97}) {`); + out.push(` return label.toUpperCase();`); + out.push(` }`); + out.push(` return label;`); + out.push(`}`); + out.push(""); + index += 1; + } + + return out.slice(0, lines).join("\n") + "\n"; +} + +/** Render one diff as a sequence of row windows, threading grammar state between them. */ +function renderWindowed( + metadata: FileDiffMetadata, + highlighter: Awaited>, + windowSize: number, +): WindowedResult { + const code = { deletionLines: [] as unknown[], additionLines: [] as unknown[] }; + const rows = Math.max(metadata.unifiedLineCount, metadata.splitLineCount); + // A partial diff's emitted lines are not contiguous in the real file, so its windows start cold. + const chainState = !metadata.isPartial; + let grammarState: unknown; + let worst = 0; + let total = 0; + let windows = 0; + + for (let start = 0; start < rows; start += windowSize) { + const started = performance.now(); + const chunk = renderDiffWithHighlighter(metadata, highlighter, renderOptions, { + forcePlainText: false, + windowedHighlight: true, + startingLine: start, + totalLines: windowSize, + expandedHunks: true, + grammarState: chainState ? grammarState : undefined, + } as never) as ReturnType & { grammarState?: unknown }; + const elapsed = performance.now() - started; + + total += elapsed; + worst = Math.max(worst, elapsed); + windows += 1; + grammarState = chunk.grammarState; + + for (let index = 0; index < chunk.code.deletionLines.length; index += 1) { + const node = chunk.code.deletionLines[index]; + if (node != null) code.deletionLines[index] = node; + } + for (let index = 0; index < chunk.code.additionLines.length; index += 1) { + const node = chunk.code.additionLines[index]; + if (node != null) code.additionLines[index] = node; + } + } + + return { code, worst, total, windows }; +} + +/** Report whether the installed Pierre honors a highlighted window at all. */ +function detectWindowSupport( + metadata: FileDiffMetadata, + highlighter: Awaited>, +) { + const windowed = renderDiffWithHighlighter(metadata, highlighter, renderOptions, { + forcePlainText: false, + windowedHighlight: true, + startingLine: 0, + totalLines: 10, + expandedHunks: true, + } as never); + const rendered = windowed.code.additionLines.filter((node) => node != null).length; + return rendered > 0 && rendered <= 10; +} + +const options = getHighlighterOptions("typescript", { theme: "pierre-dark" }); +const highlighter = await getSharedHighlighter({ + themes: options.themes, + langs: [...options.langs, "tsx"], + preferredHighlighter: "shiki-wasm", +}); +const samples = collectSamples(); + +if (samples.length === 0) { + console.error("no sample sources found; run from the repo root"); + process.exit(1); +} + +const probeMetadata = parseDiffFromFile( + { name: "probe.ts", contents: "", cacheKey: "probe-old" }, + { name: "probe.ts", contents: samples[0]!.text, cacheKey: "probe-new" }, +); +const windowSupported = detectWindowSupport(probeMetadata, highlighter); + +console.log( + windowSupported + ? "windowed highlighting: supported by the installed @pierre/diffs\n" + : "windowed highlighting: NOT supported by the installed @pierre/diffs " + + "(apply patches/@pierre%2Fdiffs@1.2.2.patch to measure it)\n", +); + +for (const sample of samples) { + // A newly added file is the worst case: one contiguous run of added lines, no context to skip. + const metadata = parseDiffFromFile( + { name: sample.path, contents: "", cacheKey: `${sample.path}-old` }, + { name: sample.path, contents: sample.text, cacheKey: `${sample.path}-new` }, + ); + + // Shiki lazily resolves grammars and themes on first use, so an unwarmed first call charges that + // one-time cost to whichever path runs first. Warm both paths before timing either. + renderDiffWithHighlighter(metadata, highlighter, renderOptions); + if (windowSupported) { + renderWindowed(metadata, highlighter, WINDOW_SIZES[0]!); + } + + const started = performance.now(); + const baseline = renderDiffWithHighlighter(metadata, highlighter, renderOptions); + const baselineMs = performance.now() - started; + + console.log(`${sample.path} (${sample.lines} lines added)`); + console.log(` whole file : ${baselineMs.toFixed(1)}ms in one uninterruptible call`); + + if (!windowSupported) { + console.log(""); + continue; + } + + for (const windowSize of WINDOW_SIZES) { + const windowed = renderWindowed(metadata, highlighter, windowSize); + const identical = + JSON.stringify(baseline.code.additionLines) === JSON.stringify(windowed.code.additionLines) && + JSON.stringify(baseline.code.deletionLines) === JSON.stringify(windowed.code.deletionLines); + + console.log( + ` window ${String(windowSize).padStart(4)}: ${windowed.total.toFixed(1)}ms across ` + + `${windowed.windows} windows, longest ${windowed.worst.toFixed(1)}ms, ` + + `identical=${identical}`, + ); + } + + console.log(""); +} + +// Correctness sweep. The timings above only cover added TypeScript files, but the claim that +// windowed output is byte-identical needs to hold across languages whose multi-line constructs +// differ, across diff shapes, and across window sizes that land mid-construct. +if (windowSupported) { + const SWEEP_LANGUAGES = ["typescript", "python", "css"] as const; + const SWEEP_WINDOWS = [64, 250, 1000]; + + const sweepHighlighter = await getSharedHighlighter({ + themes: options.themes, + langs: [...options.langs, ...SWEEP_LANGUAGES], + preferredHighlighter: "shiki-wasm", + }); + + /** Python with triple-quoted strings that straddle window boundaries. */ + const pythonSource = (lines: number, seed: number) => { + const out: string[] = []; + let index = seed; + while (out.length < lines) { + out.push(`def handler_${index}(value):`); + out.push(` """Docstring for ${index}`); + out.push(` continues past the opening line`); + out.push(` """`); + out.push(` return value + ${index % 13}`); + out.push(""); + index += 1; + } + return out.slice(0, lines).join("\n") + "\n"; + }; + + /** CSS with block comments spanning lines. */ + const cssSource = (lines: number, seed: number) => { + const out: string[] = []; + let index = seed; + while (out.length < lines) { + out.push(`/* rule ${index}`); + out.push(` keeps going */`); + out.push(`.cls-${index} {`); + out.push(` color: #${(index % 999).toString(16).padStart(3, "0")};`); + out.push(`}`); + out.push(""); + index += 1; + } + return out.slice(0, lines).join("\n") + "\n"; + }; + + const generators = { + typescript: { make: (lines: number, seed: number) => syntheticSource(lines, seed), ext: "ts" }, + python: { make: pythonSource, ext: "py" }, + css: { make: cssSource, ext: "css" }, + } as const; + + console.log("correctness sweep (windowed output vs whole-file render)\n"); + let mismatches = 0; + + for (const language of SWEEP_LANGUAGES) { + const { make, ext } = generators[language]; + const shapes: Array<[string, string, string]> = [ + ["new file", "", make(900, 0)], + ["deleted file", make(900, 0), ""], + ["full rewrite", make(700, 0), make(700, 400)], + ["scattered edits", make(900, 0), make(900, 0).replace(/9/g, "8")], + ["no trailing newline", make(300, 0), make(300, 5).trimEnd()], + ]; + + for (const [shape, oldText, newText] of shapes) { + const metadata = parseDiffFromFile( + { name: `sweep.${ext}`, contents: oldText, cacheKey: `${language}-${shape}-old` }, + { name: `sweep.${ext}`, contents: newText, cacheKey: `${language}-${shape}-new` }, + ); + const whole = renderDiffWithHighlighter(metadata, sweepHighlighter, renderOptions); + const results = SWEEP_WINDOWS.map((windowSize) => { + const windowed = renderWindowed(metadata, sweepHighlighter, windowSize); + const identical = + JSON.stringify(whole.code.additionLines) === + JSON.stringify(windowed.code.additionLines) && + JSON.stringify(whole.code.deletionLines) === JSON.stringify(windowed.code.deletionLines); + if (!identical) mismatches += 1; + return `w=${windowSize} ${identical ? "ok" : "MISMATCH"}`; + }); + + console.log(` ${`${language} / ${shape}`.padEnd(34)} ${results.join(" ")}`); + } + } + + // A patch-only diff is not contiguous in the real file, so its windows must start cold. Pierre + // already buckets these per hunk; renderWindowed skips chaining when metadata.isPartial. + const partialPatch = `diff --git a/sweep.ts b/sweep.ts +--- a/sweep.ts ++++ b/sweep.ts +@@ -10,4 +10,4 @@ context + const a = 1; + const b = 2; +-const c = 3; ++const c = 33; + const d = 4; +@@ -40,4 +40,4 @@ context + const e = 5; + const f = 6; +-const g = 7; ++const g = 77; + const h = 8; +`; + const parsedPartial = parsePatchFiles(partialPatch, "sweep", true)[0]?.files[0]; + if (parsedPartial) { + const whole = renderDiffWithHighlighter(parsedPartial, sweepHighlighter, renderOptions); + const results = SWEEP_WINDOWS.map((windowSize) => { + const windowed = renderWindowed(parsedPartial, sweepHighlighter, windowSize); + const identical = + JSON.stringify(whole.code.additionLines) === JSON.stringify(windowed.code.additionLines) && + JSON.stringify(whole.code.deletionLines) === JSON.stringify(windowed.code.deletionLines); + if (!identical) mismatches += 1; + return `w=${windowSize} ${identical ? "ok" : "MISMATCH"}`; + }); + console.log(` ${"typescript / partial patch".padEnd(34)} ${results.join(" ")}`); + } + + console.log( + `\n${mismatches === 0 ? "sweep: all cases identical" : `sweep: ${mismatches} MISMATCHES`}`, + ); +} diff --git a/docs/pierre-chunked-highlighting.md b/docs/pierre-chunked-highlighting.md new file mode 100644 index 000000000..ad7a04826 --- /dev/null +++ b/docs/pierre-chunked-highlighting.md @@ -0,0 +1,218 @@ +# Chunked syntax highlighting in Pierre + +Status: investigation plus a working proof of concept. Nothing here is wired into the shipped +build, and no upstream pull request has been opened. + +This is not the approach Hunk should take. A companion investigation moved the same work to a Bun +worker instead, and its main-thread stall is flat at a few milliseconds where windowing's stays +proportional to window size. What survives here is the upstream contribution: the inability to +highlight less than a whole file is a real gap in `@pierre/diffs`, their own `Virtualizer` runs into +it, and the patch below closes it. + +## The question + +Large contiguous diffs — a newly added file is the clearest case — stall the terminal while Hunk +highlights them. The suspicion was that `@pierre/diffs` can only highlight a whole file at once, so +Hunk has no way to break the work into pieces and yield between them. + +That is correct as of `@pierre/diffs` 1.2.2, and still correct in 1.3.5. + +## Why the whole file is highlighted at once + +`renderDiffWithHighlighter` already accepts `startingLine` and `totalLines`, and +`iterateOverDiff` already implements a complete row-window walk behind them. The very first thing +the renderer does, though, is throw the window away unless the caller also asked for plain text: + +```js +if (forcePlainText) { + startingLine ??= 0; + totalLines ??= Infinity; +} else { + startingLine = 0; // <- any window the caller passed is discarded + totalLines = Infinity; +} +``` + +So today there are exactly two modes: a highlighted whole file, or a windowed slice with no +highlighting at all. Hunk uses the first and never passes a window. + +The gate is not arbitrary. Below it, `shouldGroupAll` concatenates every visible line of each side +into one string and hands it to Shiki in a single `codeToHast` call. That is what makes highlighting +correct: a TextMate grammar is stateful, so a window tokenized on its own starts with an empty rule +stack and mis-colors anything inside a block comment, template literal, or heredoc that opened +earlier in the file. Windowing and highlighting were mutually exclusive because nothing carried the +lexical state across a window boundary. + +Two details soften the problem: + +- Pierre already buckets **per hunk** when `diff.isPartial` is true, so patch-only diffs are already + chunked. The single-giant-call path is specifically the complete-file case — which is exactly the + added-file case Hunk cares about. +- Hunk makes that worse on purpose. `sourceBackedHighlight.ts` grafts full file text onto a partial + diff so grammar state is right, which converts a cheap per-hunk render into one whole-file render. + +## Shiki can already do this + +Shiki 3.x exposes exactly the missing piece: + +- `codeToHast(code, { grammarState })` starts tokenizing from a saved rule stack. +- `highlighter.getLastGrammarState(hast)` reads the state a render ended in. + +Pierre already depends on both — `dist/shiki-stream/tokenizer.js` threads `grammarState` line by +line for streaming. Nothing new needs to be built in Shiki, and nothing needs to change in Pierre's +window walk. The change is to stop discarding the window, and to thread grammar state through the +existing bucket loop. + +## The proof of concept + +There are two patches, carrying the same change against different targets: + +- `patches/pierre-upstream-windowed-highlight.patch` is the real one — TypeScript source plus tests, + against `pierrecomputer/pierre@d9eb0ab`, verified with Pierre's own typecheck and test suite. Its + draft PR description and open questions are in `docs/pierre-windowed-highlight-pr.md`. +- `patches/@pierre%2Fdiffs@1.2.2.patch` is the same change hand-applied to the published `dist` of + the version this repo pins, so the benchmark below runs without building Pierre from source. It + exposes the identical API. + +Apply the `dist` one with: + +```bash +bun patch @pierre/diffs +# copy the patch body over node_modules/@pierre/diffs/dist/utils/renderDiffWithHighlighter.js +bun patch --commit node_modules/@pierre/diffs +``` + +or, to try it without editing `package.json`: + +```bash +patch -p1 -d node_modules/@pierre/diffs < "patches/@pierre%2Fdiffs@1.2.2.patch" +``` + +It does four things: + +1. **Honors a window while highlighting, behind an opt-in flag.** `windowedHighlight: true` makes + `startingLine`/`totalLines` take effect with highlighting on. It is a flag rather than being + inferred from a window being present, so a caller that passes a range today keeps getting the + whole diff instead of silently switching behavior. +2. **Threads grammar state in and out.** A new `grammarState` input, and a `grammarState` field on + the result, carrying one state per side. State flows bucket-to-bucket within a call and + window-to-window across calls. A side that contributed no lines to a bucket keeps its prior + state rather than resetting. +3. **Disables the whole-array shortcut for windows.** `shouldGroupAll` would otherwise overwrite the + output array with one bucket's lines; a window has to go through the existing sparse + segment-fill path. +4. **Walks a single row axis when windowed.** This one is not obvious and is the part that took + longest to find — see below. + +### The `diffStyle: "both"` trap + +`renderDiffWithHighlighter` iterates with `diffStyle: "both"`, which advances a unified row counter +and a split row counter independently and emits a line if it falls in _either_ window. Unified and +split row counts diverge whenever a change block has unequal deletion and addition counts, so +consecutive row windows overlap heavily. Measured on a 4000-line rewrite with 250-row windows: + +``` +additionLines: total=4000 never=0 once=744 duplicated=3256 +deletionLines: total=4000 never=0 once=924 duplicated=3076 +``` + +Coverage is complete, so the rendered output is fine — but 81% of lines get emitted more than once, +which makes grammar-state chaining nonsense, because each window re-consumes lines the previous +window already advanced past. The result was correct-looking output that drifted into comment +coloring a few hundred lines in. + +The fix is to walk one axis. Windowed highlighting uses `diffStyle: "split"`, which keeps each side +line to exactly one row and keeps change pairs on a single row — which matters because Pierre's +word-level diff decorations are computed from a paired change callback. Under `"unified"` the pair +is split across two callbacks and intra-line highlighting would be lost entirely. Every branch of +`getChangeLineData` treats `"both"` and `"split"` identically, so `data-line-index` and the emitted +line metadata are unchanged. + +### Where chaining is not valid + +Grammar state may only be carried between windows while the emitted lines are contiguous in the +underlying file. Two cases break that: + +- **Collapsed context.** If `expandedHunks` leaves gaps, the emitted lines skip source, and the + carried state is stale. The proof of concept passes `expandedHunks: true`. +- **Partial diffs.** A patch-only diff's lines are not contiguous in the real file at all. Pierre + already buckets these per hunk and starts each cold; the benchmark keeps that behavior by not + chaining when `metadata.isPartial`. + +A real upstream API should make this explicit rather than leaving it to the caller — either by +refusing to chain across a gap, or by returning a marker saying the state is only valid if the next +window starts at a given line. + +## Results + +`benchmarks/pierre-windowed-highlight.ts` reproduces these. It warms both paths before timing, +because Shiki resolves grammars lazily and an unwarmed first call otherwise absorbs a large one-time +cost. It detects whether the patch is applied and says so rather than failing. + +Largest real source in the repo, rendered as a newly added file: + +``` +src/ui/diff/renderRows.tsx (2373 lines added) + whole file : 255.6ms in one uninterruptible call + window 250: 267.5ms across 10 windows, longest 36.7ms, identical=true + window 500: 269.4ms across 5 windows, longest 66.3ms, identical=true + window 1000: 236.3ms across 3 windows, longest 114.2ms, identical=true +``` + +A generated 8000-line file: + +``` +synthetic-generated.ts (8000 lines added) + whole file : 660.6ms in one uninterruptible call + window 250: 702.9ms across 32 windows, longest 27.7ms, identical=true + window 500: 721.3ms across 16 windows, longest 58.2ms, identical=true + window 1000: 673.8ms across 8 windows, longest 87.4ms, identical=true +``` + +Total CPU is essentially unchanged — windowing costs a few percent, it does not save work. The whole +win is granularity: the longest uninterruptible call drops from 661ms to 28ms, roughly 24x, which is +the difference between a frozen terminal and a responsive one. Hunk already serializes highlight +jobs through `setTimeout` in `queueHighlightedWork`, so smaller units drop straight into that queue +and let input and frame timers run between them. + +The same benchmark then runs a correctness sweep: TypeScript, Python, and CSS, each in new-file, +deleted-file, full-rewrite, scattered-edit, and no-trailing-newline shapes, plus a partial patch, at +window sizes 64, 250, and 1000. Every case is compared byte-for-byte against the stock whole-file +render, and all 48 are identical. The languages are chosen for the shape of their multi-line +constructs — block comments, template literals, triple-quoted strings — since those are what a window +boundary can cut through. + +## Is an upstream pull request plausible + +Yes, and it is written: see `docs/pierre-windowed-highlight-pr.md`. It is three files against +`pierrecomputer/pierre@d9eb0ab`, and Pierre's own `packages/diffs` suite goes from 1504 to 1515 +passing with zero failures. + +What makes it a comfortable change to propose: + +- It reuses machinery Pierre already ships — the window walk, the segment sparse-fill, the per-hunk + buckets, and its own Shiki grammar-state usage in `shiki-stream`. +- It is opt-in behind a flag. Passing no window preserves today's behavior exactly, which is what the + byte-identity checks demonstrate. +- Pierre has an obvious use for it too: its DOM `Virtualizer` already models `RenderRange` with + `startingLine`, `totalLines`, `bufferBefore`, and `bufferAfter`, and currently has to choose + between a whole-file highlight and an unhighlighted window for the same reason Hunk does. +- The existing code already flags this as a known gap: the branch that discards the range is + commented "Maybe one day we warn about this?". + +Open questions for the maintainers are carried in the PR draft: row space versus per-side line index +space, whether Pierre should police contiguity itself rather than trusting the caller, dual-theme +coverage, and whether `windowedHighlight` belongs on `ForceDiffPlainTextOptions` at all. + +## What Hunk would do with it + +Nothing in `src/` has been changed. The integration would be local to +`renderHighlightedDiff` in `src/ui/diff/diffRows.ts`: replace the single +`renderDiffWithHighlighter` call with a loop over windows, each scheduled through the existing +`queueHighlightedWork` timer, merging into the same `HighlightedDiffCode` shape the UI already +consumes. Publishing each window as it lands would additionally let the top of a large file paint +highlighted while the rest is still being tokenized, but that needs `useHighlightedDiff` to accept +progressive updates and is a separate change. + +That work should wait until the upstream API is settled, so Hunk does not end up carrying a patched +dependency or a second highlighting path. diff --git a/docs/pierre-windowed-highlight-pr.md b/docs/pierre-windowed-highlight-pr.md new file mode 100644 index 000000000..c60da31a5 --- /dev/null +++ b/docs/pierre-windowed-highlight-pr.md @@ -0,0 +1,174 @@ +# Draft upstream PR: windowed syntax highlighting in `@pierre/diffs` + +Not submitted. This is the branch and description we would open against +[`pierrecomputer/pierre`](https://github.com/pierrecomputer/pierre) once the open questions at the +bottom are settled with its maintainers. Background and Hunk-side measurements are in +`docs/pierre-chunked-highlighting.md`. + +The patch is `patches/pierre-upstream-windowed-highlight.patch`, in `git format-patch` form against +`pierrecomputer/pierre@d9eb0ab` (the v1.3.5 development head). Apply it with: + +```bash +git clone https://github.com/pierrecomputer/pierre +git -C pierre am < patches/pierre-upstream-windowed-highlight.patch +``` + +## Shape of the change + +Three files, +435 / -21: + +| File | Change | +| --------------------------------------------------------------- | --------------------------------------------------------------- | +| `packages/diffs/src/types.ts` | `DiffGrammarState`, two new option fields, one new result field | +| `packages/diffs/src/utils/renderDiffWithHighlighter.ts` | Honor the window, thread grammar state, walk a single row axis | +| `packages/diffs/test/renderDiffWithHighlighterWindowed.test.ts` | 11 tests | + +No other module changes. No internal caller passes a range with `forcePlainText: false` today, so +nothing else in that repo is affected. + +## Verification actually run + +Against the real upstream toolchain, in a clone at `d9eb0ab`: + +- `tsc --noEmit` on `packages/diffs` — clean. +- `bun test` on `packages/diffs` — **1515 pass, 0 fail** (1504 before the new tests). +- `oxfmt --check` on the three files — clean. +- `oxlint --type-aware` on the three files — one warning, `no-useless-default-assignment` on the + pre-existing `theme: themeOrThemes = DEFAULT_THEMES` destructure. Confirmed identical on the + unmodified `HEAD`, so it does not come from this change. +- Negative control: reverting only the `diffStyle` line fails 4 of the 11 new tests, so the suite + covers the subtle half of the change rather than just the obvious half. + +`moon run root:format root:lint` was not run — moon and proto are not installed here, so the +underlying `oxfmt` and `oxlint` binaries were invoked directly with the arguments the moon tasks use. + +--- + +## PR description + +### Title + +`feat(diffs): support windowed syntax highlighting` + +### Body + +`renderDiffWithHighlighter` already accepts `startingLine` / `totalLines`, and `iterateOverDiff` +already implements the whole windowed walk behind them — but the range is discarded unless +`forcePlainText` is set: + +```ts +} else { + // If we aren't forcing plain text, then we intentionally do not support + // ranges for highlighting as that could break the syntax highlighting, we + // we override any values that may have been passed in. Maybe one day we + // warn about this? + startingLine = 0; + totalLines = Infinity; +} +``` + +So a caller can have a highlighted whole file, or an unhighlighted window, but not a highlighted +window. On a large added file that means one uninterruptible tokenize pass — precisely the work a +virtualizing caller most wants to break up. `Virtualizer` already models a `RenderRange` with +`startingLine`, `totalLines`, `bufferBefore`, and `bufferAfter`, and hits the same wall. + +The comment's reasoning is right: a window tokenized on its own starts from an empty rule stack and +mis-colors anything inside a block comment, template literal, or heredoc that opened earlier. But +Shiki can continue from a saved state via `codeToHast(code, { grammarState })` and +`getLastGrammarState(hast)`, and this package already relies on that in `shiki-stream`. The window +only needs somewhere to carry it. + +**What this adds** + +```ts +let grammarState: DiffGrammarState | undefined; + +for (let startingLine = 0; startingLine < rows; startingLine += 250) { + const result = renderDiffWithHighlighter(diff, highlighter, options, { + forcePlainText: false, + windowedHighlight: true, + startingLine, + totalLines: 250, + expandedHunks: true, + grammarState, + }); + grammarState = result.grammarState; + // merge result.code into the sparse output the same way the plain-text path does +} +``` + +- `windowedHighlight?: boolean` opts in. It is a flag rather than being inferred from a range being + present, so an existing caller that passes a range with highlighting on keeps today's behavior + instead of silently switching to windowed output. +- `grammarState?: DiffGrammarState` carries lexical state in, and `ThemedDiffResult.grammarState` + carries it out. It is per side, because the deletion and addition sides are tokenized as two + separate documents. State also flows bucket to bucket inside a single call, since a window buckets + per hunk. A side that contributed no lines to a bucket keeps its prior state rather than resetting. +- `shouldGroupAll` is disabled for a window, since it would otherwise replace the whole output array + with one bucket's lines instead of going through the existing sparse segment fill. + +**The `diffStyle` change is the subtle part** + +Windowed highlighting iterates with `'split'` instead of `'both'`. `'both'` advances the unified and +split row counters independently and emits a line landing in _either_ window, so consecutive windows +overlap wherever the two counts diverge. On a 4000-line rewrite with 250-row windows, 81% of side +lines were emitted more than once. Coverage is still complete so the rendered output looks fine, but +grammar-state chaining becomes meaningless, because each window re-consumes lines the previous one +already advanced past — the symptom was correct-looking output drifting into comment coloring a few +hundred lines in. + +`'split'` emits each side line exactly once and keeps a change's deletion and addition on the same +row, which is what `computeLineDiffDecorations` needs — under `'unified'` the pair arrives as two +separate callbacks and intra-line highlighting would be lost. Every branch of `getChangeLineData` +already treats `'both'` and `'split'` identically, so the emitted line metadata is unchanged. + +**Constraints, documented on the option** + +Chaining is only valid while consecutive windows cover the file contiguously. Callers must pass +`expandedHunks: true` so no context is skipped, and must not chain across a partial diff, whose lines +are not contiguous in the real file. The tests cover a partial diff rendering correctly with chaining +off. + +**Compatibility** + +Passing no window is byte-for-byte unchanged, and no `grammarState` is returned. The new tests assert +that a range passed without `windowedHighlight` still renders the whole diff. + +**Tests** — `packages/diffs/test/renderDiffWithHighlighterWindowed.test.ts`, 11 cases: + +- a range is ignored unless `windowedHighlight` is set, and no grammar state is returned +- `windowedHighlight` renders exactly the requested window and does return grammar state +- windows write each side line exactly once, on a diff whose unified and split counts diverge +- windowed output equals the whole-diff render, for a new file and for a rewrite, at window sizes 13, + 25, and 100 +- dropping the grammar state between windows _does_ change the output, so the threading is shown to + be load-bearing rather than incidental +- a partial diff still matches the whole-diff render + +**Measurements** (8000-line added file, `pierre-dark`, `shiki-wasm`): + +| Render | Total | Longest single call | +| --------------- | ------- | ------------------- | +| whole file | 660.6ms | 660.6ms | +| 250-row windows | 702.9ms | 27.7ms | +| 500-row windows | 721.3ms | 58.2ms | + +Windowing costs a few percent of total CPU and saves no work. The point is that the longest +uninterruptible call drops roughly 24x, which is the difference between a frozen UI and a responsive +one. + +### Questions for maintainers + +1. **Row space or side space?** The window is expressed in row space to match `RenderRange`, which is + what forced the `'split'` axis choice. Expressing it in per-side line-index space would sidestep + that entirely and map more directly onto how buckets are built, at the cost of no longer lining up + with `RenderRange`. Happy to switch. +2. **Should Pierre police contiguity itself?** Right now the "no gaps, no partial diffs" rule is + documented on the option and left to the caller. Pierre could instead detect a gap during + iteration and drop the incoming state, which would make the option harder to misuse. +3. **Dual themes.** Only the single-theme path is tested here. `codeToTokensWithThemes` does merge + and propagate a per-theme grammar state, so it should work as-is, but I have not exercised it and + would rather hear what coverage you would want. +4. **Naming.** `windowedHighlight` sits on `ForceDiffPlainTextOptions`, which is now doing more than + its name suggests. Renaming the interface is breaking; adding a sibling options bag is not. No + strong opinion. diff --git a/package.json b/package.json index 2615d69fb..c55eabd45 100644 --- a/package.json +++ b/package.json @@ -94,6 +94,7 @@ "bench:changeset-parse": "bun run benchmarks/changeset-parse.ts", "bench:render-layout": "bun run benchmarks/render-layout.ts", "bench:highlight-prefetch": "bun run benchmarks/highlight-prefetch.ts", + "bench:pierre-windowed-highlight": "bun run benchmarks/pierre-windowed-highlight.ts", "bench:large-stream": "bun run benchmarks/large-stream.ts", "bench:interaction-latency": "bun run benchmarks/interaction-latency.ts", "bench:non-ascii-stream": "bun run benchmarks/non-ascii-stream.ts", diff --git a/patches/@pierre%2Fdiffs@1.2.2.patch b/patches/@pierre%2Fdiffs@1.2.2.patch new file mode 100644 index 000000000..80552cb49 --- /dev/null +++ b/patches/@pierre%2Fdiffs@1.2.2.patch @@ -0,0 +1,132 @@ +--- a/dist/utils/renderDiffWithHighlighter.js ++++ b/dist/utils/renderDiffWithHighlighter.js +@@ -11,8 +11,12 @@ + + //#region src/utils/renderDiffWithHighlighter.ts + const DEFAULT_PLAIN_TEXT_OPTIONS = { forcePlainText: false }; +-function renderDiffWithHighlighter(diff, highlighter, options, { forcePlainText, startingLine, totalLines, expandedHunks, collapsedContextThreshold = DEFAULT_COLLAPSED_CONTEXT_THRESHOLD } = DEFAULT_PLAIN_TEXT_OPTIONS) { +- if (forcePlainText) { ++function renderDiffWithHighlighter(diff, highlighter, options, { forcePlainText, startingLine, totalLines, expandedHunks, collapsedContextThreshold = DEFAULT_COLLAPSED_CONTEXT_THRESHOLD, windowedHighlight = false, grammarState } = DEFAULT_PLAIN_TEXT_OPTIONS) { ++ // A highlighted window is only correct if the caller carries grammar state ++ // from one window to the next, so it stays opt-in rather than being inferred ++ // from a range being present. ++ const isHighlightedWindow = !forcePlainText && windowedHighlight; ++ if (forcePlainText || isHighlightedWindow) { + startingLine ??= 0; + totalLines ??= Infinity; + } else { +@@ -31,8 +35,8 @@ + additionLines: [] + }; + const { maxLineDiffLength } = options; +- const shouldGroupAll = !forcePlainText && !diff.isPartial; +- const expandedHunksForIteration = forcePlainText ? expandedHunks : void 0; ++ const shouldGroupAll = !forcePlainText && !isWindowedHighlight && !diff.isPartial; ++ const expandedHunksForIteration = forcePlainText || isHighlightedWindow ? expandedHunks : void 0; + const buckets = /* @__PURE__ */ new Map(); + function getBucketForHunk(hunkIndex) { + const index = shouldGroupAll ? 0 : hunkIndex; +@@ -57,7 +61,7 @@ + } + iterateOverDiff({ + diff, +- diffStyle: "both", ++ diffStyle: isHighlightedWindow ? "split" : "both", + startingLine, + totalLines, + expandedHunks: isWindowedHighlight ? expandedHunksForIteration : true, +@@ -95,6 +99,7 @@ + } + } + }); ++ let nextGrammarState = grammarState; + for (const bucket of buckets.values()) { + if (bucket.deletionContent.length === 0 && bucket.additionContent.length === 0) continue; + const deletionFile = { +@@ -105,7 +110,11 @@ + name: diff.name, + contents: bucket.additionContent.value + }; +- const { deletionLines, additionLines } = renderTwoFiles({ ++ const { ++ deletionLines, ++ additionLines, ++ grammarState: bucketGrammarState ++ } = renderTwoFiles({ + deletionFile, + deletionInfo: bucket.deletionInfo, + deletionDecorations: bucket.deletionDecorations, +@@ -114,8 +123,11 @@ + additionDecorations: bucket.additionDecorations, + highlighter, + options, +- languageOverride: forcePlainText ? "text" : diff.lang ++ languageOverride: forcePlainText ? "text" : diff.lang, ++ grammarState: isHighlightedWindow ? nextGrammarState : void 0, ++ trackGrammarState: isHighlightedWindow + }); ++ nextGrammarState = bucketGrammarState; + if (shouldGroupAll) { + code.deletionLines = deletionLines; + code.additionLines = additionLines; +@@ -126,7 +138,12 @@ + if (bucket.additionSegments.length > 0) for (const seg of bucket.additionSegments) for (let i = 0; i < seg.count; i++) code.additionLines[seg.targetIndex + i] = additionLines[seg.originalOffset + i]; + else code.additionLines.push(...additionLines); + } +- return { ++ return isHighlightedWindow ? { ++ code, ++ themeStyles, ++ baseThemeType, ++ grammarState: nextGrammarState ++ } : { + code, + themeStyles, + baseThemeType +@@ -217,7 +234,7 @@ + additionSegments: [] + }; + } +-function renderTwoFiles({ deletionFile, additionFile, deletionInfo, additionInfo, highlighter, deletionDecorations, additionDecorations, languageOverride, options: { theme: themeOrThemes = DEFAULT_THEMES,...options } }) { ++function renderTwoFiles({ deletionFile, additionFile, deletionInfo, additionInfo, highlighter, deletionDecorations, additionDecorations, languageOverride, grammarState, trackGrammarState, options: { theme: themeOrThemes = DEFAULT_THEMES,...options } }) { + const deletionLang = languageOverride ?? getFiletypeFromFileName(deletionFile.name); + const additionLang = languageOverride ?? getFiletypeFromFileName(additionFile.name); + const { state, transformers } = createTransformerWithState(options.useTokenTransformer); +@@ -240,21 +257,34 @@ + cssVariablePrefix: formatCSSVariablePrefix("token") + }; + })(); ++ // A side that contributed no lines here has not advanced through the file, ++ // so it keeps the state it came in with rather than resetting to undefined. ++ const nextGrammarState = { ++ deletion: grammarState?.deletion, ++ addition: grammarState?.addition ++ }; + return { + deletionLines: (() => { + if (deletionFile.contents === "") return []; + hastConfig.lang = deletionLang; + state.lineInfo = deletionInfo; + hastConfig.decorations = deletionDecorations; +- return getLineNodes(highlighter.codeToHast(cleanLastNewline(deletionFile.contents), hastConfig)); ++ hastConfig.grammarState = grammarState?.deletion; ++ const hast = highlighter.codeToHast(cleanLastNewline(deletionFile.contents), hastConfig); ++ if (trackGrammarState) nextGrammarState.deletion = highlighter.getLastGrammarState(hast); ++ return getLineNodes(hast); + })(), + additionLines: (() => { + if (additionFile.contents === "") return []; + hastConfig.lang = additionLang; + hastConfig.decorations = additionDecorations; + state.lineInfo = additionInfo; +- return getLineNodes(highlighter.codeToHast(cleanLastNewline(additionFile.contents), hastConfig)); +- })() ++ hastConfig.grammarState = grammarState?.addition; ++ const hast = highlighter.codeToHast(cleanLastNewline(additionFile.contents), hastConfig); ++ if (trackGrammarState) nextGrammarState.addition = highlighter.getLastGrammarState(hast); ++ return getLineNodes(hast); ++ })(), ++ grammarState: trackGrammarState ? nextGrammarState : void 0 + }; + } + diff --git a/patches/pierre-upstream-windowed-highlight.patch b/patches/pierre-upstream-windowed-highlight.patch new file mode 100644 index 000000000..f293589f2 --- /dev/null +++ b/patches/pierre-upstream-windowed-highlight.patch @@ -0,0 +1,632 @@ +From 0713de6adfec5c747b77a5edb322d7db997305fe Mon Sep 17 00:00:00 2001 +From: Claude +Date: Sat, 15 Aug 2026 18:04:54 +0000 +Subject: [PATCH] feat(diffs): support windowed syntax highlighting + +renderDiffWithHighlighter accepts startingLine/totalLines but discards +them unless forcePlainText is set, so a caller that wants highlighting +has to render the whole diff in one call. On a large added file that is +a single uninterruptible tokenize pass, which is what a virtualizing +caller most wants to break up. + +The range was discarded because a window tokenized on its own starts +from an empty rule stack and mis-colors anything inside a construct that +opened earlier in the file. Shiki can already continue from a saved +state, and this package already relies on that in shiki-stream, so the +window only needs somewhere to carry it. + +Add an opt-in windowedHighlight flag that honors the range while +highlighting, and a per-side grammarState that the caller threads from +one window into the next. State also flows bucket to bucket inside a +call, since a window buckets per hunk. + +Windowed highlighting walks the split axis rather than 'both'. 'both' +advances the unified and split row counters independently and emits a +line landing in either window, so windows overlap wherever the counts +diverge and each side line is rendered several times. Split keeps a +change's two sides on one row, which the word-level diff decorations are +computed from. + +Passing no window is unchanged, and windowed output is asserted equal to +the whole-diff render across window sizes. +--- + packages/diffs/src/types.ts | 30 ++ + .../src/utils/renderDiffWithHighlighter.ts | 102 ++++-- + .../renderDiffWithHighlighterWindowed.test.ts | 324 ++++++++++++++++++ + 3 files changed, 435 insertions(+), 21 deletions(-) + create mode 100644 packages/diffs/test/renderDiffWithHighlighterWindowed.test.ts + +diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts +index 67c013c..d2a60af 100644 +--- a/packages/diffs/src/types.ts ++++ b/packages/diffs/src/types.ts +@@ -5,6 +5,7 @@ import type { + BundledTheme, + CodeToHastOptions, + DecorationItem, ++ GrammarState, + HighlighterGeneric, + LanguageRegistration, + ShikiTransformer, +@@ -772,6 +773,20 @@ export interface ThemedDiffResult { + code: RenderDiffFilesResult; + themeStyles: string; + baseThemeType: 'light' | 'dark' | undefined; ++ /** Grammar state each side finished in, produced only for windowed ++ * highlights. Pass it back as `grammarState` on the next window so that ++ * window continues tokenizing from here instead of from an empty rule ++ * stack. */ ++ grammarState?: DiffGrammarState; ++} ++ ++/** ++ * TextMate grammar state for one diff, tracked per side because the deletion ++ * and addition sides are tokenized as two separate documents. ++ */ ++export interface DiffGrammarState { ++ deletion: GrammarState | undefined; ++ addition: GrammarState | undefined; + } + + export interface HunkExpansionRegion { +@@ -785,6 +800,21 @@ export interface ForceDiffPlainTextOptions { + totalLines?: number; + expandedHunks?: Map | true; + collapsedContextThreshold?: number; ++ /** ++ * Honor `startingLine`/`totalLines` while still syntax highlighting, instead ++ * of ignoring them and rendering the whole diff. Rendering a window in ++ * isolation would normally mis-color anything inside a construct that opened ++ * earlier in the file, so a caller that sets this must feed each window's ++ * returned `grammarState` into the next one. ++ * ++ * Only valid when consecutive windows cover the file contiguously: pass ++ * `expandedHunks: true` so no context is skipped, and do not use this on a ++ * partial diff, whose lines are not contiguous in the real file. ++ */ ++ windowedHighlight?: boolean; ++ /** Grammar state returned by the previous window's render. Omit for the ++ * first window. Ignored unless `windowedHighlight` is set. */ ++ grammarState?: DiffGrammarState; + } + + export interface ForceFilePlainTextOptions { +diff --git a/packages/diffs/src/utils/renderDiffWithHighlighter.ts b/packages/diffs/src/utils/renderDiffWithHighlighter.ts +index 709c659..568412e 100644 +--- a/packages/diffs/src/utils/renderDiffWithHighlighter.ts ++++ b/packages/diffs/src/utils/renderDiffWithHighlighter.ts +@@ -7,6 +7,7 @@ import { + import type { + CodeToHastOptions, + DecorationItem, ++ DiffGrammarState, + DiffsHighlighter, + DiffsThemeNames, + FileContents, +@@ -45,15 +46,21 @@ export function renderDiffWithHighlighter( + totalLines, + expandedHunks, + collapsedContextThreshold = DEFAULT_COLLAPSED_CONTEXT_THRESHOLD, ++ windowedHighlight = false, ++ grammarState, + }: ForceDiffPlainTextOptions = DEFAULT_PLAIN_TEXT_OPTIONS + ): ThemedDiffResult { +- if (forcePlainText) { ++ // A highlighted window is only correct if the caller carries grammar state ++ // from one window to the next, so it stays opt-in rather than being inferred ++ // from a range being present. ++ const isHighlightedWindow = !forcePlainText && windowedHighlight; ++ if (forcePlainText || isHighlightedWindow) { + startingLine ??= 0; + totalLines ??= Infinity; + } else { +- // If we aren't forcing plain text, then we intentionally do not support +- // ranges for highlighting as that could break the syntax highlighting, we +- // we override any values that may have been passed in. Maybe one day we ++ // Without plain text or an explicit windowed highlight we do not support ++ // ranges, as rendering a range in isolation breaks syntax highlighting, so ++ // we override any values that may have been passed in. Maybe one day we + // warn about this? + startingLine = 0; + totalLines = Infinity; +@@ -85,8 +92,13 @@ export function renderDiffWithHighlighter( + }; + + const { maxLineDiffLength } = options; +- const shouldGroupAll = !forcePlainText && !diff.isPartial; +- const expandedHunksForIteration = forcePlainText ? expandedHunks : undefined; ++ // A window writes its lines into sparse target indexes through the segment ++ // path below, so it can never take the shortcut of replacing the whole array ++ // with a single bucket's lines. ++ const shouldGroupAll = ++ !forcePlainText && !isWindowedHighlight && !diff.isPartial; ++ const expandedHunksForIteration = ++ forcePlainText || isHighlightedWindow ? expandedHunks : undefined; + const buckets = new Map(); + function getBucketForHunk(hunkIndex: number) { + const index = shouldGroupAll ? 0 : hunkIndex; +@@ -121,7 +133,14 @@ export function renderDiffWithHighlighter( + + iterateOverDiff({ + diff, +- diffStyle: 'both', ++ // 'both' advances the unified and split row counters independently and ++ // emits a line that lands in either window, so consecutive windows overlap ++ // wherever the two counts diverge. A highlighted window has to see each ++ // side line exactly once for its grammar state to be meaningful, so it ++ // walks one axis. Split is the one to walk: it keeps a change's deletion ++ // and addition on the same row, which is what the word-level diff ++ // decorations below are computed from. ++ diffStyle: isHighlightedWindow ? 'split' : 'both', + startingLine, + totalLines, + expandedHunks: isWindowedHighlight ? expandedHunksForIteration : true, +@@ -184,6 +203,10 @@ export function renderDiffWithHighlighter( + }, + }); + ++ // A window buckets per hunk, so grammar state has to flow from bucket to ++ // bucket within this call as well as from window to window across calls. ++ let nextGrammarState = grammarState; ++ + for (const bucket of buckets.values()) { + if ( + bucket.deletionContent.length === 0 && +@@ -200,7 +223,11 @@ export function renderDiffWithHighlighter( + name: diff.name, + contents: bucket.additionContent.value, + }; +- const { deletionLines, additionLines } = renderTwoFiles({ ++ const { ++ deletionLines, ++ additionLines, ++ grammarState: bucketGrammarState, ++ } = renderTwoFiles({ + deletionFile, + deletionInfo: bucket.deletionInfo, + deletionDecorations: bucket.deletionDecorations, +@@ -212,7 +239,10 @@ export function renderDiffWithHighlighter( + highlighter, + options, + languageOverride: forcePlainText ? 'text' : diff.lang, ++ grammarState: isHighlightedWindow ? nextGrammarState : undefined, ++ trackGrammarState: isHighlightedWindow, + }); ++ nextGrammarState = bucketGrammarState; + + if (shouldGroupAll) { + code.deletionLines = deletionLines; +@@ -242,7 +272,9 @@ export function renderDiffWithHighlighter( + } + } + +- return { code, themeStyles, baseThemeType }; ++ return isHighlightedWindow ++ ? { code, themeStyles, baseThemeType, grammarState: nextGrammarState } ++ : { code, themeStyles, baseThemeType }; + } + + interface ProcessLineDiffProps { +@@ -403,6 +435,15 @@ interface RenderTwoFilesProps { + options: RenderDiffOptions; + highlighter: DiffsHighlighter; + languageOverride: SupportedLanguages | undefined; ++ /** Grammar state each side should continue tokenizing from. */ ++ grammarState: DiffGrammarState | undefined; ++ /** Whether to read back the state each side finished in. Reading it costs a ++ * map lookup per side, so only windowed highlights pay for it. */ ++ trackGrammarState: boolean; ++} ++ ++interface RenderTwoFilesResult extends RenderDiffFilesResult { ++ grammarState: DiffGrammarState | undefined; + } + + function renderTwoFiles({ +@@ -414,8 +455,10 @@ function renderTwoFiles({ + deletionDecorations, + additionDecorations, + languageOverride, ++ grammarState, ++ trackGrammarState, + options: { theme: themeOrThemes = DEFAULT_THEMES, ...options }, +-}: RenderTwoFilesProps): RenderDiffFilesResult { ++}: RenderTwoFilesProps): RenderTwoFilesResult { + const deletionLang = + languageOverride ?? getFiletypeFromFileName(deletionFile.name); + const additionLang = +@@ -451,6 +494,13 @@ function renderTwoFiles({ + }; + })(); + ++ // A side that contributed no lines here has not advanced through the file, ++ // so it keeps the state it came in with rather than resetting to undefined. ++ const nextGrammarState: DiffGrammarState = { ++ deletion: grammarState?.deletion, ++ addition: grammarState?.addition, ++ }; ++ + const deletionLines = (() => { + if (deletionFile.contents === '') { + return []; +@@ -458,12 +508,15 @@ function renderTwoFiles({ + hastConfig.lang = deletionLang; + state.lineInfo = deletionInfo; + hastConfig.decorations = deletionDecorations; +- return getLineNodes( +- highlighter.codeToHast( +- cleanLastNewline(deletionFile.contents), +- hastConfig +- ) ++ hastConfig.grammarState = grammarState?.deletion; ++ const hast = highlighter.codeToHast( ++ cleanLastNewline(deletionFile.contents), ++ hastConfig + ); ++ if (trackGrammarState) { ++ nextGrammarState.deletion = highlighter.getLastGrammarState(hast); ++ } ++ return getLineNodes(hast); + })(); + const additionLines = (() => { + if (additionFile.contents === '') { +@@ -472,13 +525,20 @@ function renderTwoFiles({ + hastConfig.lang = additionLang; + hastConfig.decorations = additionDecorations; + state.lineInfo = additionInfo; +- return getLineNodes( +- highlighter.codeToHast( +- cleanLastNewline(additionFile.contents), +- hastConfig +- ) ++ hastConfig.grammarState = grammarState?.addition; ++ const hast = highlighter.codeToHast( ++ cleanLastNewline(additionFile.contents), ++ hastConfig + ); ++ if (trackGrammarState) { ++ nextGrammarState.addition = highlighter.getLastGrammarState(hast); ++ } ++ return getLineNodes(hast); + })(); + +- return { deletionLines, additionLines }; ++ return { ++ deletionLines, ++ additionLines, ++ grammarState: trackGrammarState ? nextGrammarState : undefined, ++ }; + } +diff --git a/packages/diffs/test/renderDiffWithHighlighterWindowed.test.ts b/packages/diffs/test/renderDiffWithHighlighterWindowed.test.ts +new file mode 100644 +index 0000000..da7f3c6 +--- /dev/null ++++ b/packages/diffs/test/renderDiffWithHighlighterWindowed.test.ts +@@ -0,0 +1,324 @@ ++import { afterEach, describe, expect, test } from 'bun:test'; ++ ++import { ++ disposeHighlighter, ++ getSharedHighlighter, ++} from '../src/highlighter/shared_highlighter'; ++import type { ++ DiffGrammarState, ++ DiffsHighlighter, ++ FileDiffMetadata, ++ RenderDiffFilesResult, ++ RenderDiffOptions, ++} from '../src/types'; ++import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; ++import { parsePatchFiles } from '../src/utils/parsePatchFiles'; ++import { renderDiffWithHighlighter } from '../src/utils/renderDiffWithHighlighter'; ++import { assertDefined } from './testUtils'; ++ ++const RENDER_OPTIONS: RenderDiffOptions = { ++ theme: 'pierre-dark', ++ useTokenTransformer: false, ++ tokenizeMaxLineLength: 1000, ++ lineDiffType: 'word-alt', ++ maxLineDiffLength: 10000, ++}; ++ ++afterEach(async () => { ++ await disposeHighlighter(); ++}); ++ ++function createHighlighter(): Promise { ++ return getSharedHighlighter({ ++ themes: ['pierre-dark'], ++ langs: ['typescript'], ++ preferredHighlighter: 'shiki-js', ++ }); ++} ++ ++/** ++ * Builds TypeScript whose lexical state deliberately outlives a single window: ++ * every block opens a multi-line comment and a multi-line template literal, so ++ * a window that started from an empty rule stack would colour the lines after a ++ * boundary as ordinary code instead of as comment or string content. ++ */ ++function sourceWithSpanningConstructs( ++ blocks: number, ++ seed: number, ++ // Adds a line to every other block so change blocks end up with unequal ++ // deletion and addition counts, which is what drives unified and split row ++ // counts apart. ++ padEveryOtherBlock = false ++): string { ++ const lines: string[] = []; ++ for (let index = seed; index < seed + blocks; index++) { ++ lines.push(`/* comment ${index} opens here`); ++ lines.push(` and closes several lines later */`); ++ lines.push(`export const value${index} = \`template ${index}`); ++ lines.push(` second line of the template\`;`); ++ lines.push(`export function fn${index}(input: number): number {`); ++ if (padEveryOtherBlock && index % 2 === 0) { ++ lines.push(` const doubled = input * 2;`); ++ lines.push(` return doubled + ${index};`); ++ } else { ++ lines.push(` return input + ${index};`); ++ } ++ lines.push(`}`); ++ lines.push(''); ++ } ++ return `${lines.join('\n')}\n`; ++} ++ ++/** ++ * Renders a diff as a sequence of windows, feeding each window's grammar state ++ * into the next, and merges the sparse per-window output into one result the ++ * way a virtualizing caller would. ++ */ ++function renderInWindows({ ++ diff, ++ highlighter, ++ windowSize, ++ chainGrammarState = true, ++}: { ++ diff: FileDiffMetadata; ++ highlighter: DiffsHighlighter; ++ windowSize: number; ++ chainGrammarState?: boolean; ++}): RenderDiffFilesResult { ++ const code: RenderDiffFilesResult = { deletionLines: [], additionLines: [] }; ++ const rows = Math.max(diff.unifiedLineCount, diff.splitLineCount); ++ let grammarState: DiffGrammarState | undefined; ++ ++ for (let startingLine = 0; startingLine < rows; startingLine += windowSize) { ++ const result = renderDiffWithHighlighter( ++ diff, ++ highlighter, ++ RENDER_OPTIONS, ++ { ++ forcePlainText: false, ++ windowedHighlight: true, ++ startingLine, ++ totalLines: windowSize, ++ expandedHunks: true, ++ grammarState: chainGrammarState ? grammarState : undefined, ++ } ++ ); ++ grammarState = result.grammarState; ++ ++ result.code.deletionLines.forEach((line, index) => { ++ if (line != null) code.deletionLines[index] = line; ++ }); ++ result.code.additionLines.forEach((line, index) => { ++ if (line != null) code.additionLines[index] = line; ++ }); ++ } ++ ++ return code; ++} ++ ++/** Counts how many windows each side line is emitted into. */ ++function countEmissionsPerLine({ ++ diff, ++ highlighter, ++ windowSize, ++}: { ++ diff: FileDiffMetadata; ++ highlighter: DiffsHighlighter; ++ windowSize: number; ++}) { ++ const deletionCounts = new Array(diff.deletionLines.length).fill(0); ++ const additionCounts = new Array(diff.additionLines.length).fill(0); ++ const rows = Math.max(diff.unifiedLineCount, diff.splitLineCount); ++ ++ for (let startingLine = 0; startingLine < rows; startingLine += windowSize) { ++ const result = renderDiffWithHighlighter( ++ diff, ++ highlighter, ++ RENDER_OPTIONS, ++ { ++ forcePlainText: false, ++ windowedHighlight: true, ++ startingLine, ++ totalLines: windowSize, ++ expandedHunks: true, ++ } ++ ); ++ result.code.deletionLines.forEach((line, index) => { ++ if (line != null) deletionCounts[index]++; ++ }); ++ result.code.additionLines.forEach((line, index) => { ++ if (line != null) additionCounts[index]++; ++ }); ++ } ++ ++ return { deletionCounts, additionCounts }; ++} ++ ++describe('renderDiffWithHighlighter windowed highlighting', () => { ++ const newFile = parseDiffFromFile( ++ { name: 'sample.ts', contents: '' }, ++ { name: 'sample.ts', contents: sourceWithSpanningConstructs(60, 0) } ++ ); ++ const rewrite = parseDiffFromFile( ++ { name: 'sample.ts', contents: sourceWithSpanningConstructs(60, 0) }, ++ { name: 'sample.ts', contents: sourceWithSpanningConstructs(60, 20, true) } ++ ); ++ ++ test('ignores a range unless windowedHighlight is set', async () => { ++ const highlighter = await createHighlighter(); ++ ++ const result = renderDiffWithHighlighter( ++ newFile, ++ highlighter, ++ RENDER_OPTIONS, ++ { forcePlainText: false, startingLine: 0, totalLines: 10 } ++ ); ++ ++ // Existing callers that pass a range with highlighting on still get the ++ // whole diff back, and no grammar state to mistake for a usable one. ++ expect(result.code.additionLines.length).toBe(newFile.additionLines.length); ++ expect(result.grammarState).toBeUndefined(); ++ }); ++ ++ test('renders only the requested window when windowedHighlight is set', async () => { ++ const highlighter = await createHighlighter(); ++ ++ const result = renderDiffWithHighlighter( ++ newFile, ++ highlighter, ++ RENDER_OPTIONS, ++ { ++ forcePlainText: false, ++ windowedHighlight: true, ++ startingLine: 0, ++ totalLines: 10, ++ expandedHunks: true, ++ } ++ ); ++ ++ const rendered = result.code.additionLines.filter((line) => line != null); ++ expect(rendered.length).toBe(10); ++ expect(result.grammarState).toBeDefined(); ++ }); ++ ++ test('windows write each side line exactly once', async () => { ++ const highlighter = await createHighlighter(); ++ ++ // Unified and split row counts diverge on this diff, which is what makes ++ // the single-axis walk load-bearing: a two-axis walk admits a line that ++ // falls in either window and emits boundary lines into both neighbours. ++ expect(rewrite.unifiedLineCount).not.toBe(rewrite.splitLineCount); ++ ++ const { deletionCounts, additionCounts } = countEmissionsPerLine({ ++ diff: rewrite, ++ highlighter, ++ windowSize: 25, ++ }); ++ ++ expect(deletionCounts.every((count) => count === 1)).toBe(true); ++ expect(additionCounts.every((count) => count === 1)).toBe(true); ++ }); ++ ++ test.each([13, 25, 100])( ++ 'windows of %i rows match the whole-diff render for a new file', ++ async (windowSize) => { ++ const highlighter = await createHighlighter(); ++ ++ const whole = renderDiffWithHighlighter( ++ newFile, ++ highlighter, ++ RENDER_OPTIONS ++ ); ++ const code = renderInWindows({ ++ diff: newFile, ++ highlighter, ++ windowSize, ++ }); ++ ++ expect(code.additionLines).toEqual(whole.code.additionLines); ++ expect(code.deletionLines).toEqual(whole.code.deletionLines); ++ } ++ ); ++ ++ test.each([13, 25, 100])( ++ 'windows of %i rows match the whole-diff render for a rewrite', ++ async (windowSize) => { ++ const highlighter = await createHighlighter(); ++ ++ const whole = renderDiffWithHighlighter( ++ rewrite, ++ highlighter, ++ RENDER_OPTIONS ++ ); ++ const code = renderInWindows({ ++ diff: rewrite, ++ highlighter, ++ windowSize, ++ }); ++ ++ expect(code.additionLines).toEqual(whole.code.additionLines); ++ expect(code.deletionLines).toEqual(whole.code.deletionLines); ++ } ++ ); ++ ++ test('dropping the grammar state mis-colours lines after the first window', async () => { ++ const highlighter = await createHighlighter(); ++ ++ const whole = renderDiffWithHighlighter( ++ newFile, ++ highlighter, ++ RENDER_OPTIONS ++ ); ++ const code = renderInWindows({ ++ diff: newFile, ++ highlighter, ++ windowSize: 25, ++ chainGrammarState: false, ++ }); ++ ++ // This is the reason the option carries state rather than just a range: ++ // without it each window restarts from an empty rule stack and lines ++ // inside a construct that opened earlier come out as ordinary code. ++ expect(code.additionLines).not.toEqual(whole.code.additionLines); ++ }); ++ ++ test('a partial diff still renders its window correctly', async () => { ++ const highlighter = await createHighlighter(); ++ const patch = `diff --git a/sample.ts b/sample.ts ++--- a/sample.ts +++++ b/sample.ts ++@@ -10,4 +10,4 @@ context ++ const a = 1; ++ const b = 2; ++-const c = 3; +++const c = 33; ++ const d = 4; ++@@ -40,4 +40,4 @@ context ++ const e = 5; ++ const f = 6; ++-const g = 7; +++const g = 77; ++ const h = 8; ++`; ++ const parsed = parsePatchFiles(patch, 'test', true)[0]?.files[0]; ++ assertDefined(parsed, 'expected the patch to parse into one file'); ++ expect(parsed.isPartial).toBe(true); ++ ++ const whole = renderDiffWithHighlighter( ++ parsed, ++ highlighter, ++ RENDER_OPTIONS ++ ); ++ // A partial diff's lines are not contiguous in the real file, so its ++ // windows must start cold; Pierre already buckets these per hunk. ++ const code = renderInWindows({ ++ diff: parsed, ++ highlighter, ++ windowSize: 4, ++ chainGrammarState: false, ++ }); ++ ++ expect(code.additionLines).toEqual(whole.code.additionLines); ++ expect(code.deletionLines).toEqual(whole.code.deletionLines); ++ }); ++}); +-- +2.43.0 + From 8c9e52362efb2bb92fe45fe0faed121a7b33095b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 22:53:56 +0000 Subject: [PATCH 2/2] docs: record that isHighlightedWindow needs renaming upstream The patch adds isHighlightedWindow next to Pierre's pre-existing isWindowedHighlight. The two are near-anagrams meaning different things, and two separate reviewers have now read the patch and concluded the new flag was a typo for the old one. Note it as an open question so the rename happens before this is sent upstream. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JgbAM65ZsLbsjgqZS8Dtav --- docs/pierre-windowed-highlight-pr.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/pierre-windowed-highlight-pr.md b/docs/pierre-windowed-highlight-pr.md index c60da31a5..cdff8d1ea 100644 --- a/docs/pierre-windowed-highlight-pr.md +++ b/docs/pierre-windowed-highlight-pr.md @@ -172,3 +172,9 @@ one. 4. **Naming.** `windowedHighlight` sits on `ForceDiffPlainTextOptions`, which is now doing more than its name suggests. Renaming the interface is breaking; adding a sibling options bag is not. No strong opinion. +5. **`isHighlightedWindow` needs a different name.** It sits beside the pre-existing + `isWindowedHighlight`, and the two are near-anagrams meaning different things: the existing one + asks whether a range is in effect at all, the new one whether the caller opted into highlighting + that range. Two separate reviewers have read the patch and concluded the new flag was a typo for + the old one, which is a strong signal to rename before this goes upstream. The new flag is the + one that should move — something like `honorHighlightWindow`.