authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-07-16 09:24:41+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-07-16 09:24:41+02:00
log5e8b2c800ecd98e23adbdbfc8970f4687ac8254b
tree0de8eccda0b6bb6ba085bc862addd786ebd31f1c
parentb98233702c88fd7a4312b231e68b089d5f3b0e80
signature Signed by PGP key B5690EEEBB952194

Isolate native diff highlight grammar state (#4029)

2 files changed, 261 insertions(+), 8 deletions(-)

apps/mobile/src/features/diffs/nativeReviewDiffHighlighter.test.ts created+189
......@@ -0,0 +1,189 @@
1import { describe, expect, it } from "vite-plus/test";
2
3import type { NativeReviewDiffRow } from "./nativeReviewDiffSurface";
4import type { NativeReviewDiffFile } from "./nativeReviewDiffTypes";
5import { highlightNativeReviewDiffVisibleRows } from "./nativeReviewDiffHighlighter";
6
7const TYPESCRIPT_FILE: NativeReviewDiffFile = {
8 id: "file-1",
9 path: "example.ts",
10 language: "typescript",
11 additions: 0,
12 deletions: 0,
13};
14
15function makeLine(
16 input: Pick<NativeReviewDiffRow, "id" | "content" | "change" | "oldLineNumber" | "newLineNumber">,
17): NativeReviewDiffRow {
18 return {
19 kind: "line",
20 fileId: TYPESCRIPT_FILE.id,
21 ...input,
22 };
23}
24
25function makeHunk(id: string): NativeReviewDiffRow {
26 return {
27 kind: "hunk",
28 id,
29 fileId: TYPESCRIPT_FILE.id,
30 text: "@@",
31 };
32}
33
34function highlight(
35 rows: ReadonlyArray<NativeReviewDiffRow>,
36 alreadyHighlightedRowIds?: ReadonlySet<string>,
37) {
38 return highlightNativeReviewDiffVisibleRows({
39 rows,
40 files: [TYPESCRIPT_FILE],
41 scheme: "dark",
42 engine: "javascript",
43 firstRowIndex: 0,
44 lastRowIndex: rows.length - 1,
45 overscanRows: 0,
46 maxRows: 100,
47 alreadyHighlightedRowIds,
48 });
49}
50
51describe("highlightNativeReviewDiffVisibleRows", () => {
52 it("does not carry grammar state across hunk boundaries", async () => {
53 const exportRow = makeLine({
54 id: "export-row",
55 content: "export async function run() {}",
56 change: "add",
57 oldLineNumber: null,
58 newLineNumber: 100,
59 });
60 const rows = [
61 makeHunk("hunk-1"),
62 makeLine({
63 id: "import-open",
64 content: "import {",
65 change: "context",
66 oldLineNumber: 1,
67 newLineNumber: 1,
68 }),
69 makeLine({
70 id: "import-entry",
71 content: " Model,",
72 change: "context",
73 oldLineNumber: 2,
74 newLineNumber: 2,
75 }),
76 makeHunk("hunk-2"),
77 exportRow,
78 ];
79
80 const [highlighted, standalone] = await Promise.all([
81 highlight(rows),
82 highlight([makeHunk("standalone-hunk"), exportRow]),
83 ]);
84
85 expect(highlighted.tokensByRowId[exportRow.id]).toEqual(standalone.tokensByRowId[exportRow.id]);
86 });
87
88 it("keeps grammar state across inline comment rows", async () => {
89 const openingRow = makeLine({
90 id: "template-open",
91 content: "const message = `open",
92 change: "add",
93 oldLineNumber: null,
94 newLineNumber: 1,
95 });
96 const closingRow = makeLine({
97 id: "template-close",
98 content: "closed`;",
99 change: "add",
100 oldLineNumber: null,
101 newLineNumber: 2,
102 });
103 const trailingRow = makeLine({
104 id: "trailing-row",
105 content: "export const answer = 42;",
106 change: "add",
107 oldLineNumber: null,
108 newLineNumber: 3,
109 });
110 const commentRow: NativeReviewDiffRow = {
111 kind: "comment",
112 id: "comment-1",
113 fileId: TYPESCRIPT_FILE.id,
114 commentText: "Review note",
115 };
116
117 const [withComment, contiguous] = await Promise.all([
118 highlight([openingRow, commentRow, closingRow, trailingRow]),
119 highlight([openingRow, closingRow, trailingRow]),
120 ]);
121
122 expect(withComment.tokensByRowId).toEqual(contiguous.tokensByRowId);
123 });
124
125 it("does not join unhighlighted rows across cached gaps", async () => {
126 const trailingRow = makeLine({
127 id: "trailing-row",
128 content: "export const answer = 42;",
129 change: "add",
130 oldLineNumber: null,
131 newLineNumber: 3,
132 });
133 const rows = [
134 makeLine({
135 id: "template-open",
136 content: "const message = `open",
137 change: "add",
138 oldLineNumber: null,
139 newLineNumber: 1,
140 }),
141 makeLine({
142 id: "template-close",
143 content: "closed`;",
144 change: "add",
145 oldLineNumber: null,
146 newLineNumber: 2,
147 }),
148 trailingRow,
149 ];
150
151 const [highlighted, standalone] = await Promise.all([
152 highlight(rows, new Set(["template-close"])),
153 highlight([trailingRow]),
154 ]);
155
156 expect(highlighted.tokensByRowId[trailingRow.id]).toEqual(
157 standalone.tokensByRowId[trailingRow.id],
158 );
159 });
160
161 it("keeps deletion grammar state out of addition rows", async () => {
162 const additionRow = makeLine({
163 id: "addition-row",
164 content: "export const answer = 42;",
165 change: "add",
166 oldLineNumber: null,
167 newLineNumber: 1,
168 });
169 const rows = [
170 makeLine({
171 id: "deletion-row",
172 content: "const removed = `open",
173 change: "delete",
174 oldLineNumber: 1,
175 newLineNumber: null,
176 }),
177 additionRow,
178 ];
179
180 const [highlighted, standalone] = await Promise.all([
181 highlight(rows),
182 highlight([additionRow]),
183 ]);
184
185 expect(highlighted.tokensByRowId[additionRow.id]).toEqual(
186 standalone.tokensByRowId[additionRow.id],
187 );
188 });
189});
apps/mobile/src/features/diffs/nativeReviewDiffHighlighter.ts+72-8
......@@ -56,6 +56,11 @@ interface NativeReviewDiffLineRow extends NativeReviewDiffRow {
5656 readonly content: string;
5757}
5858
59interface IndexedNativeReviewDiffLineRow {
60 readonly row: NativeReviewDiffLineRow;
61 readonly rowIndex: number;
62}
63
5964export interface NativeReviewDiffTokenChunk {
6065 readonly chunkIndex: number;
6166 readonly fileId: string;
......@@ -308,6 +313,58 @@ function isHighlightableLineRow(row: NativeReviewDiffRow): row is NativeReviewDi
308313 return row.kind === "line" && typeof row.fileId === "string" && typeof row.content === "string";
309314}
310315
316function hasConsecutiveLineNumbers(
317 previous: number | null | undefined,
318 next: number | null | undefined,
319): boolean {
320 return typeof previous === "number" && typeof next === "number" && next === previous + 1;
321}
322
323function hasOnlyCommentRowsBetween(
324 rows: ReadonlyArray<NativeReviewDiffRow>,
325 previousRowIndex: number,
326 nextRowIndex: number,
327): boolean {
328 for (let rowIndex = previousRowIndex + 1; rowIndex < nextRowIndex; rowIndex += 1) {
329 if (rows[rowIndex]?.kind !== "comment") {
330 return false;
331 }
332 }
333 return true;
334}
335
336function canShareGrammarContext(
337 previous: IndexedNativeReviewDiffLineRow,
338 next: IndexedNativeReviewDiffLineRow,
339 rows: ReadonlyArray<NativeReviewDiffRow>,
340): boolean {
341 if (
342 next.row.fileId !== previous.row.fileId ||
343 !hasOnlyCommentRowsBetween(rows, previous.rowIndex, next.rowIndex)
344 ) {
345 return false;
346 }
347
348 if (previous.row.change === "delete" || next.row.change === "delete") {
349 return (
350 previous.row.change !== "add" &&
351 next.row.change !== "add" &&
352 hasConsecutiveLineNumbers(previous.row.oldLineNumber, next.row.oldLineNumber)
353 );
354 }
355
356 if (previous.row.change === "add" || next.row.change === "add") {
357 return hasConsecutiveLineNumbers(previous.row.newLineNumber, next.row.newLineNumber);
358 }
359
360 return (
361 previous.row.change === "context" &&
362 next.row.change === "context" &&
363 hasConsecutiveLineNumbers(previous.row.oldLineNumber, next.row.oldLineNumber) &&
364 hasConsecutiveLineNumbers(previous.row.newLineNumber, next.row.newLineNumber)
365 );
366}
367
311368function groupLineRowsByFileId(rows: ReadonlyArray<NativeReviewDiffRow>) {
312369 const rowsByFileId = new Map<string, NativeReviewDiffLineRow[]>();
313370 for (const row of rows) {
......@@ -360,7 +417,7 @@ export async function highlightNativeReviewDiffVisibleRows(
360417 const maxRows = input.maxRows ?? NATIVE_REVIEW_DIFF_VISIBLE_MAX_ROWS;
361418 const startIndex = clampRowIndex(input.firstRowIndex - overscanRows, input.rows);
362419 const endIndex = clampRowIndex(input.lastRowIndex + overscanRows, input.rows);
363 const selectedRows: NativeReviewDiffLineRow[] = [];
420 const selectedRows: IndexedNativeReviewDiffLineRow[] = [];
364421
365422 for (
366423 let rowIndex = startIndex;
......@@ -374,12 +431,12 @@ export async function highlightNativeReviewDiffVisibleRows(
374431 !input.alreadyHighlightedRowIds?.has(row.id) &&
375432 fileMap.has(row.fileId)
376433 ) {
377 selectedRows.push(row);
434 selectedRows.push({ row, rowIndex });
378435 }
379436 }
380437
381438 const tokensByRowId: Record<string, ReadonlyArray<NativeReviewDiffToken>> = {};
382 let segmentRows: NativeReviewDiffLineRow[] = [];
439 let segmentRows: IndexedNativeReviewDiffLineRow[] = [];
383440 let segmentFile: NativeReviewDiffFile | undefined;
384441
385442 const flushSegment = () => {
......@@ -389,27 +446,34 @@ export async function highlightNativeReviewDiffVisibleRows(
389446 return;
390447 }
391448
392 const code = segmentRows.map((row) => row.content).join("\n");
449 const code = segmentRows.map(({ row }) => row.content).join("\n");
393450 const tokenLines = highlighter.tokenize(code, { lang: segmentFile.language, theme });
394 segmentRows.forEach((row, rowIndex) => {
451 segmentRows.forEach(({ row }, rowIndex) => {
395452 tokensByRowId[row.id] = tokenLines[rowIndex] ?? makePlainTokenFallback(row);
396453 });
397454 segmentRows = [];
398455 segmentFile = undefined;
399456 };
400457
401 for (const row of selectedRows) {
458 for (const selectedRow of selectedRows) {
459 const { row } = selectedRow;
402460 const file = fileMap.get(row.fileId);
403461 if (!file) {
404462 continue;
405463 }
406464
407 if (segmentFile && segmentFile.id !== file.id) {
465 const previousRow = segmentRows.at(-1);
466 if (
467 segmentFile &&
468 (segmentFile.id !== file.id ||
469 (previousRow !== undefined &&
470 !canShareGrammarContext(previousRow, selectedRow, input.rows)))
471 ) {
408472 flushSegment();
409473 }
410474
411475 segmentFile = file;
412 segmentRows.push(row);
476 segmentRows.push(selectedRow);
413477 }
414478 flushSegment();
415479