authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-06-20 15:15:00-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-06-20 15:15:00-07:00
log42ff43915a2ed5c0f190019c0c1a6a1cb68658c6
tree306638903074bd725f0efe31fb10ae3d2d034760
parent834e5db45843d8b5da4fbfe1999fbaa4c58e117b
signature Signed by PGP key B5690EEEBB952194

[codex] Preserve mobile review highlighter failures (#3337)

Co-authored-by: codex <codex@users.noreply.github.com>

5 files changed, 182 insertions(+), 48 deletions(-)

apps/mobile/src/features/diffs/nativeReviewDiffHighlighter.ts+71-10
...@@ -8,6 +8,7 @@ import jsxLanguage from "@shikijs/langs/jsx";...@@ -8,6 +8,7 @@ import jsxLanguage from "@shikijs/langs/jsx";
8import tsxLanguage from "@shikijs/langs/tsx";8import tsxLanguage from "@shikijs/langs/tsx";
9import typescriptLanguage from "@shikijs/langs/typescript";9import typescriptLanguage from "@shikijs/langs/typescript";
10import yamlLanguage from "@shikijs/langs/yaml";10import yamlLanguage from "@shikijs/langs/yaml";
11import * as Schema from "effect/Schema";
1112
12import type { NativeReviewDiffFile, NativeReviewDiffLanguage } from "./nativeReviewDiffTypes";13import type { NativeReviewDiffFile, NativeReviewDiffLanguage } from "./nativeReviewDiffTypes";
13import type { NativeReviewDiffRow, NativeReviewDiffToken } from "./nativeReviewDiffSurface";14import type { NativeReviewDiffRow, NativeReviewDiffToken } from "./nativeReviewDiffSurface";
...@@ -15,6 +16,32 @@ import type { NativeReviewDiffRow, NativeReviewDiffToken } from "./nativeReviewD...@@ -15,6 +16,32 @@ import type { NativeReviewDiffRow, NativeReviewDiffToken } from "./nativeReviewD
15export type NativeReviewDiffHighlightScheme = "light" | "dark";16export type NativeReviewDiffHighlightScheme = "light" | "dark";
16export type NativeReviewDiffHighlightEngine = "native" | "javascript";17export type NativeReviewDiffHighlightEngine = "native" | "javascript";
1718
19export class NativeReviewDiffHighlighterUnavailableError extends Schema.TaggedErrorClass<NativeReviewDiffHighlighterUnavailableError>()(
20 "NativeReviewDiffHighlighterUnavailableError",
21 {},
22) {
23 override get message(): string {
24 return "The native review diff highlighter is unavailable in this build.";
25 }
26}
27
28export const isNativeReviewDiffHighlighterUnavailableError = Schema.is(
29 NativeReviewDiffHighlighterUnavailableError,
30);
31
32export class NativeReviewDiffHighlighterInitializationError extends Schema.TaggedErrorClass<NativeReviewDiffHighlighterInitializationError>()(
33 "NativeReviewDiffHighlighterInitializationError",
34 {
35 requestedEngine: Schema.Literals(["native", "javascript"]),
36 attemptedEngine: Schema.Literals(["native", "javascript"]),
37 cause: Schema.Defect(),
38 },
39) {
40 override get message(): string {
41 return `Failed to initialize the ${this.attemptedEngine} review diff highlighter requested as ${this.requestedEngine}.`;
42 }
43}
44
18export interface NativeReviewDiffHighlighterHandle {45export interface NativeReviewDiffHighlighterHandle {
19 readonly engine: NativeReviewDiffHighlightEngine;46 readonly engine: NativeReviewDiffHighlightEngine;
20 readonly tokenize: (47 readonly tokenize: (
...@@ -197,7 +224,7 @@ function normalizeTokens(...@@ -197,7 +224,7 @@ function normalizeTokens(
197async function createNativeReviewDiffHighlighter(): Promise<NativeReviewDiffHighlighterHandle> {224async function createNativeReviewDiffHighlighter(): Promise<NativeReviewDiffHighlighterHandle> {
198 const nativeEngineModule = await import("react-native-shiki-engine");225 const nativeEngineModule = await import("react-native-shiki-engine");
199 if (!nativeEngineModule.isNativeEngineAvailable()) {226 if (!nativeEngineModule.isNativeEngineAvailable()) {
200 throw new Error("Native Shiki engine is not available in this build.");227 throw new NativeReviewDiffHighlighterUnavailableError();
201 }228 }
202229
203 const highlighter = await createHighlighterCore({230 const highlighter = await createHighlighterCore({
...@@ -229,18 +256,52 @@ export async function getNativeReviewDiffHighlighter(...@@ -229,18 +256,52 @@ export async function getNativeReviewDiffHighlighter(
229 engine: NativeReviewDiffHighlightEngine = "native",256 engine: NativeReviewDiffHighlightEngine = "native",
230): Promise<NativeReviewDiffHighlighterHandle> {257): Promise<NativeReviewDiffHighlighterHandle> {
231 if (engine === "javascript") {258 if (engine === "javascript") {
232 javascriptHighlighterPromise ??= createJavascriptReviewDiffHighlighter();259 try {
233 return javascriptHighlighterPromise;260 javascriptHighlighterPromise ??= createJavascriptReviewDiffHighlighter();
261 return await javascriptHighlighterPromise;
262 } catch (cause) {
263 javascriptHighlighterPromise = null;
264 throw new NativeReviewDiffHighlighterInitializationError({
265 requestedEngine: engine,
266 attemptedEngine: "javascript",
267 cause,
268 });
269 }
234 }270 }
235271
236 nativeHighlighterPromise ??= createNativeReviewDiffHighlighter().catch((error: unknown) => {272 nativeHighlighterPromise ??= createNativeReviewDiffHighlighter()
237 console.warn("[debug-native-diff] native highlighter unavailable", {273 .catch(async (cause: unknown) => {
238 error: error instanceof Error ? error.message : String(error),274 const nativeError = isNativeReviewDiffHighlighterUnavailableError(cause)
275 ? cause
276 : new NativeReviewDiffHighlighterInitializationError({
277 requestedEngine: engine,
278 attemptedEngine: "native",
279 cause,
280 });
281 console.warn("[debug-native-diff] native highlighter unavailable", {
282 error: nativeError,
283 });
284 try {
285 javascriptHighlighterPromise ??= createJavascriptReviewDiffHighlighter();
286 return await javascriptHighlighterPromise;
287 } catch (fallbackCause) {
288 javascriptHighlighterPromise = null;
289 throw new NativeReviewDiffHighlighterInitializationError({
290 requestedEngine: engine,
291 attemptedEngine: "javascript",
292 cause: new AggregateError(
293 [nativeError, fallbackCause],
294 "Native and JavaScript review diff highlighter initialization failed.",
295 { cause: nativeError },
296 ),
297 });
298 }
299 })
300 .catch((error) => {
301 nativeHighlighterPromise = null;
302 throw error;
239 });303 });
240 javascriptHighlighterPromise ??= createJavascriptReviewDiffHighlighter();304 return await nativeHighlighterPromise;
241 return javascriptHighlighterPromise;
242 });
243 return nativeHighlighterPromise;
244}305}
245306
246function isHighlightableLineRow(row: NativeReviewDiffRow): row is NativeReviewDiffLineRow {307function isHighlightableLineRow(row: NativeReviewDiffRow): row is NativeReviewDiffLineRow {
apps/mobile/src/features/review/reviewHighlighterState.test.ts+21-6
...@@ -53,11 +53,12 @@ it("initializes review highlighter state once", async () => {...@@ -53,11 +53,12 @@ it("initializes review highlighter state once", async () => {
53});53});
5454
55it("stores initialization failures in atom state", async () => {55it("stores initialization failures in atom state", async () => {
56 const cause = new Error("load failed");
56 const manager = createReviewHighlighterManager({57 const manager = createReviewHighlighterManager({
57 getRegistry: () => registry,58 getRegistry: () => registry,
58 loader: {59 loader: {
59 prepare: async () => {60 prepare: async () => {
60 throw new Error("load failed");61 throw cause;
61 },62 },
62 prepareLanguages: async () => undefined,63 prepareLanguages: async () => undefined,
63 getEngine: async () => "javascript",64 getEngine: async () => "javascript",
...@@ -67,9 +68,23 @@ it("stores initialization failures in atom state", async () => {...@@ -67,9 +68,23 @@ it("stores initialization failures in atom state", async () => {
67 void manager.initialize();68 void manager.initialize();
68 await flushAsyncWork();69 await flushAsyncWork();
6970
70 assert.deepStrictEqual(manager.getSnapshot(), {71 const snapshot = manager.getSnapshot();
71 engine: null,72 assert.strictEqual(snapshot.engine, null);
72 error: "load failed",73 assert.strictEqual(snapshot.status, "error");
73 status: "error",74 assert.strictEqual(snapshot.error?._tag, "ReviewHighlighterManagerError");
74 });75 assert.strictEqual(snapshot.error?.operation, "prepare");
76 assert.deepStrictEqual(snapshot.error?.languages, [
77 "typescript",
78 "tsx",
79 "javascript",
80 "jsx",
81 "json",
82 "yaml",
83 "bash",
84 ]);
85 assert.strictEqual(snapshot.error?.cause, cause);
86 assert.strictEqual(
87 snapshot.error?.message,
88 "Review highlighter operation prepare failed for languages typescript, tsx, javascript, jsx, json, yaml, bash.",
89 );
75});90});
apps/mobile/src/features/review/reviewHighlighterState.ts+40-15
...@@ -1,4 +1,5 @@...@@ -1,4 +1,5 @@
1import { useAtomValue } from "@effect/atom-react";1import { useAtomValue } from "@effect/atom-react";
2import * as Schema from "effect/Schema";
2import { Atom, type AtomRegistry } from "effect/unstable/reactivity";3import { Atom, type AtomRegistry } from "effect/unstable/reactivity";
3import { useEffect } from "react";4import { useEffect } from "react";
45
...@@ -12,9 +13,22 @@ import {...@@ -12,9 +13,22 @@ import {
1213
13export type ReviewHighlighterStatus = "idle" | "initializing" | "ready" | "error";14export type ReviewHighlighterStatus = "idle" | "initializing" | "ready" | "error";
1415
16export class ReviewHighlighterManagerError extends Schema.TaggedErrorClass<ReviewHighlighterManagerError>()(
17 "ReviewHighlighterManagerError",
18 {
19 operation: Schema.Literals(["prepare", "prepare-languages", "resolve-engine"]),
20 languages: Schema.Array(Schema.String),
21 cause: Schema.Defect(),
22 },
23) {
24 override get message(): string {
25 return `Review highlighter operation ${this.operation} failed for languages ${this.languages.join(", ")}.`;
26 }
27}
28
15export interface ReviewHighlighterState {29export interface ReviewHighlighterState {
16 readonly engine: ReviewHighlighterEngine | null;30 readonly engine: ReviewHighlighterEngine | null;
17 readonly error: string | null;31 readonly error: ReviewHighlighterManagerError | null;
18 readonly status: ReviewHighlighterStatus;32 readonly status: ReviewHighlighterStatus;
19}33}
2034
...@@ -101,24 +115,35 @@ export function createReviewHighlighterManager(config: {...@@ -101,24 +115,35 @@ export function createReviewHighlighterManager(config: {
101115
102 inFlight = (async () => {116 inFlight = (async () => {
103 const startedAt = performance.now();117 const startedAt = performance.now();
118 const languages = config.languages ?? REVIEW_INITIAL_LANGUAGES;
119 let operation: ReviewHighlighterManagerError["operation"] = "prepare";
120 let engine: ReviewHighlighterEngine;
104 try {121 try {
105 await config.loader.prepare();122 await config.loader.prepare();
106 await config.loader.prepareLanguages(config.languages ?? REVIEW_INITIAL_LANGUAGES);123 operation = "prepare-languages";
107 const engine = await config.loader.getEngine();124 await config.loader.prepareLanguages(languages);
108 const durationMs = Math.round(performance.now() - startedAt);125 operation = "resolve-engine";
109 logReviewHighlighterProviderDiagnostic("initialized", {126 engine = await config.loader.getEngine();
110 durationMs,127 } catch (cause) {
111 engine,128 const error = new ReviewHighlighterManagerError({
129 operation,
130 languages,
131 cause,
112 });132 });
113 setState({ engine, error: null, status: "ready" });133 logReviewHighlighterProviderDiagnostic("initialization failed", { error });
114 } catch (error) {134 setState({ engine: null, error, status: "error" });
115 const message = error instanceof Error ? error.message : String(error);135 return;
116 logReviewHighlighterProviderDiagnostic("initialization failed", { error: message });
117 setState({ engine: null, error: message, status: "error" });
118 } finally {
119 inFlight = null;
120 }136 }
121 })();137
138 const durationMs = Math.round(performance.now() - startedAt);
139 logReviewHighlighterProviderDiagnostic("initialized", {
140 durationMs,
141 engine,
142 });
143 setState({ engine, error: null, status: "ready" });
144 })().finally(() => {
145 inFlight = null;
146 });
122147
123 return inFlight;148 return inFlight;
124 }149 }
apps/mobile/src/features/review/shikiReviewHighlighter.ts+45-16
...@@ -10,6 +10,7 @@ import yamlLanguage from "@shikijs/langs/yaml";...@@ -10,6 +10,7 @@ import yamlLanguage from "@shikijs/langs/yaml";
10import githubDarkDefault from "@shikijs/themes/github-dark-default";10import githubDarkDefault from "@shikijs/themes/github-dark-default";
11import githubLightDefault from "@shikijs/themes/github-light-default";11import githubLightDefault from "@shikijs/themes/github-light-default";
12import { getFiletypeFromFileName } from "@pierre/diffs/utils/getFiletypeFromFileName";12import { getFiletypeFromFileName } from "@pierre/diffs/utils/getFiletypeFromFileName";
13import * as Schema from "effect/Schema";
1314
14import {15import {
15 resolveReviewHighlighterEngine,16 resolveReviewHighlighterEngine,
...@@ -22,6 +23,19 @@ import { applyDiffRangesToTokens, computeWordAltDiffRanges } from "./reviewWordD...@@ -22,6 +23,19 @@ import { applyDiffRangesToTokens, computeWordAltDiffRanges } from "./reviewWordD
22export type ReviewDiffTheme = "light" | "dark";23export type ReviewDiffTheme = "light" | "dark";
23export type { ReviewHighlighterEngine };24export type { ReviewHighlighterEngine };
2425
26export class ReviewHighlighterEngineInitializationError extends Schema.TaggedErrorClass<ReviewHighlighterEngineInitializationError>()(
27 "ReviewHighlighterEngineInitializationError",
28 {
29 preferredEngine: Schema.Literals(["native", "javascript"]),
30 attemptedEngine: Schema.Literals(["native", "javascript"]),
31 cause: Schema.Defect(),
32 },
33) {
34 override get message(): string {
35 return `Failed to initialize the ${this.attemptedEngine} review highlighter with ${this.preferredEngine} preferred.`;
36 }
37}
38
25export interface ReviewHighlightedToken {39export interface ReviewHighlightedToken {
26 content: string;40 content: string;
27 readonly color: string | null;41 readonly color: string | null;
...@@ -227,16 +241,6 @@ function logReviewHighlighterDiagnosticError(message: string, error: unknown): v...@@ -227,16 +241,6 @@ function logReviewHighlighterDiagnosticError(message: string, error: unknown): v
227 if (!isReviewHighlighterDebugLoggingEnabled()) {241 if (!isReviewHighlighterDebugLoggingEnabled()) {
228 return;242 return;
229 }243 }
230
231 if (error instanceof Error) {
232 console.error(`[review-highlighter] ${message}`, {
233 name: error.name,
234 message: error.message,
235 stack: error.stack,
236 });
237 return;
238 }
239
240 console.error(`[review-highlighter] ${message}`, error);244 console.error(`[review-highlighter] ${message}`, error);
241}245}
242246
...@@ -258,6 +262,7 @@ async function getHighlighter(): Promise<HighlighterCore> {...@@ -258,6 +262,7 @@ async function getHighlighter(): Promise<HighlighterCore> {
258 if (!highlighterPromise) {262 if (!highlighterPromise) {
259 const configuredHighlighterPromise = (async () => {263 const configuredHighlighterPromise = (async () => {
260 let nativeEngineAvailable = false;264 let nativeEngineAvailable = false;
265 let nativeInitializationError: ReviewHighlighterEngineInitializationError | undefined;
261266
262 logReviewHighlighterDiagnostic("initializing", {267 logReviewHighlighterDiagnostic("initializing", {
263 configuredPreference: REVIEW_HIGHLIGHTER_ENGINE_ENV_VALUE,268 configuredPreference: REVIEW_HIGHLIGHTER_ENGINE_ENV_VALUE,
...@@ -289,9 +294,14 @@ async function getHighlighter(): Promise<HighlighterCore> {...@@ -289,9 +294,14 @@ async function getHighlighter(): Promise<HighlighterCore> {
289 };294 };
290 }295 }
291 } catch (error) {296 } catch (error) {
297 nativeInitializationError = new ReviewHighlighterEngineInitializationError({
298 preferredEngine: REVIEW_HIGHLIGHTER_ENGINE_PREFERENCE,
299 attemptedEngine: "native",
300 cause: error,
301 });
292 logReviewHighlighterDiagnosticError(302 logReviewHighlighterDiagnosticError(
293 "native engine initialization failed; falling back to javascript",303 "native engine initialization failed; falling back to javascript",
294 error,304 nativeInitializationError,
295 );305 );
296 nativeEngineAvailable = false;306 nativeEngineAvailable = false;
297 }307 }
...@@ -305,11 +315,30 @@ async function getHighlighter(): Promise<HighlighterCore> {...@@ -305,11 +315,30 @@ async function getHighlighter(): Promise<HighlighterCore> {
305 REVIEW_HIGHLIGHTER_ENGINE_PREFERENCE,315 REVIEW_HIGHLIGHTER_ENGINE_PREFERENCE,
306 nativeEngineAvailable,316 nativeEngineAvailable,
307 );317 );
308 const highlighter = await createHighlighterCore({318 let highlighter: HighlighterCore;
309 themes,319 try {
310 langs: REVIEW_INITIAL_LANGUAGE_MODULES,320 highlighter = await createHighlighterCore({
311 engine: createJavaScriptRegexEngine(),321 themes,
312 });322 langs: REVIEW_INITIAL_LANGUAGE_MODULES,
323 engine: createJavaScriptRegexEngine(),
324 });
325 } catch (cause) {
326 const javascriptError = new ReviewHighlighterEngineInitializationError({
327 preferredEngine: REVIEW_HIGHLIGHTER_ENGINE_PREFERENCE,
328 attemptedEngine: "javascript",
329 cause,
330 });
331 if (!nativeInitializationError) throw javascriptError;
332 throw new ReviewHighlighterEngineInitializationError({
333 preferredEngine: REVIEW_HIGHLIGHTER_ENGINE_PREFERENCE,
334 attemptedEngine: "javascript",
335 cause: new AggregateError(
336 [nativeInitializationError, javascriptError],
337 "Native and JavaScript review highlighter initialization failed.",
338 { cause: nativeInitializationError },
339 ),
340 });
341 }
313 logReviewHighlighterDiagnostic("using javascript engine", {342 logReviewHighlighterDiagnostic("using javascript engine", {
314 resolvedEngine: engine,343 resolvedEngine: engine,
315 });344 });
apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts+5-1
...@@ -108,7 +108,11 @@ export function useNativeReviewDiffHighlighting(input: {...@@ -108,7 +108,11 @@ export function useNativeReviewDiffHighlighting(input: {
108 } catch (error) {108 } catch (error) {
109 if (!abortController.signal.aborted) {109 if (!abortController.signal.aborted) {
110 logReviewDiffDiagnostic("native visible highlight failed", {110 logReviewDiffDiagnostic("native visible highlight failed", {
111 error: error instanceof Error ? error.message : String(error),111 error,
112 resetKey,
113 scheme,
114 firstRowIndex: requestRange.firstRowIndex,
115 lastRowIndex: requestRange.lastRowIndex,
112 });116 });
113 }117 }
114 }118 }