perf(mobile): bound the parsed review cache

apps/mobile/src/features/review/reviewState.test.ts

@@ -1,11 +1,31 @@
-import { assert, it } from "vite-plus/test";
+import { afterEach, assert, it } from "vite-plus/test";
 
+import { appAtomRegistry } from "../../state/atom-registry";
+import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter";
 import {
+  getCachedReviewParsedDiff,
   getReviewAsyncStateSnapshot,
   setReviewAsyncError,
   setReviewTurnDiffLoading,
 } from "./reviewState";
 
+const reviewInput = {
+  threadKey: "env-local:thread-review",
+  sectionId: "turn:1",
+  diff: [
+    "diff --git a/src/a.ts b/src/a.ts",
+    "new file mode 100644",
+    "--- /dev/null",
+    "+++ b/src/a.ts",
+    "@@ -0,0 +1 @@",
+    "+export const value = 1;",
+  ].join("\n"),
+};
+
+afterEach(() => {
+  appAtomRegistry.reset();
+});
+
 it("stores review async loading and error state in atoms", () => {
   const threadKey = `env-local:thread-review-state-${Date.now()}`;
 
@@ -25,3 +45,87 @@ it("stores review async loading and error state in atoms", () => {
     error: null,
   });
 });
+
+it("reuses unchanged parsed diffs and replaces changed sections", () => {
+  const parsed = getCachedReviewParsedDiff(reviewInput);
+
+  assert.strictEqual(
+    getCachedReviewParsedDiff({ ...reviewInput, diff: `\n${reviewInput.diff}\n` }),
+    parsed,
+  );
+
+  const changed = { ...reviewInput, diff: reviewInput.diff.replace("value = 1", "value = 2") };
+  const updated = getCachedReviewParsedDiff(changed);
+  assert.notStrictEqual(updated, parsed);
+  assert.strictEqual(getCachedReviewParsedDiff(changed), updated);
+});
+
+it("evicts the least recently used section across threads without changing live results or IDs", () => {
+  const recentInput = { ...reviewInput, threadKey: "env-local:thread-recent" };
+  const oldestInput = { ...reviewInput, threadKey: "env-remote:thread-oldest" };
+  const recent = getCachedReviewParsedDiff(recentInput);
+  const oldest = getCachedReviewParsedDiff(oldestInput);
+  const native = getCachedNativeReviewDiffData({ parsedDiff: oldest });
+  for (let index = 0; index < 6; index += 1) {
+    getCachedReviewParsedDiff({ ...reviewInput, threadKey: `env-local:thread-${index}` });
+  }
+
+  assert.strictEqual(getCachedReviewParsedDiff(recentInput), recent);
+  getCachedReviewParsedDiff({ ...reviewInput, threadKey: "env-local:thread-new" });
+
+  assert.strictEqual(getCachedReviewParsedDiff(recentInput), recent);
+  const rebuilt = getCachedReviewParsedDiff(oldestInput);
+  assert.notStrictEqual(rebuilt, oldest);
+  assert.deepStrictEqual(rebuilt, oldest);
+  assert.strictEqual(getCachedNativeReviewDiffData({ parsedDiff: oldest }), native);
+  assert.deepStrictEqual(getCachedNativeReviewDiffData({ parsedDiff: rebuilt }), native);
+});
+
+it("limits cached diffs to 4 Mi source characters before the entry limit", () => {
+  const firstInput = { ...reviewInput, diff: "x".repeat(2 * 1024 * 1024) };
+  const secondInput = { ...firstInput, sectionId: "turn:2" };
+  const first = getCachedReviewParsedDiff(firstInput);
+  const second = getCachedReviewParsedDiff(secondInput);
+
+  getCachedReviewParsedDiff({ ...firstInput, sectionId: "turn:3" });
+
+  assert.strictEqual(getCachedReviewParsedDiff(secondInput), second);
+  assert.notStrictEqual(getCachedReviewParsedDiff(firstInput), first);
+});
+
+it("reclaims the source budget when a cached section changes", () => {
+  const firstInput = { ...reviewInput, diff: "x".repeat(2 * 1024 * 1024) };
+  const secondInput = { ...firstInput, sectionId: "turn:2" };
+  getCachedReviewParsedDiff(firstInput);
+  const second = getCachedReviewParsedDiff(secondInput);
+
+  getCachedReviewParsedDiff({ ...firstInput, diff: null });
+  getCachedReviewParsedDiff({ ...firstInput, sectionId: "turn:3" });
+
+  assert.strictEqual(getCachedReviewParsedDiff(secondInput), second);
+});
+
+it("counts the full source and skips oversized diffs without evicting cached sections", () => {
+  const cached = getCachedReviewParsedDiff(reviewInput);
+  const oversizedInput = {
+    ...reviewInput,
+    sectionId: "turn:oversized",
+    diff: `${reviewInput.diff}${" ".repeat(4 * 1024 * 1024)}`,
+  };
+  const first = getCachedReviewParsedDiff(oversizedInput);
+  const second = getCachedReviewParsedDiff(oversizedInput);
+
+  assert.notStrictEqual(second, first);
+  assert.deepStrictEqual(second, first);
+  assert.strictEqual(getCachedReviewParsedDiff(reviewInput), cached);
+});
+
+it("drops parsed diffs when the app registry resets", () => {
+  const parsed = getCachedReviewParsedDiff(reviewInput);
+
+  appAtomRegistry.reset();
+
+  const rebuilt = getCachedReviewParsedDiff(reviewInput);
+  assert.notStrictEqual(rebuilt, parsed);
+  assert.deepStrictEqual(rebuilt, parsed);
+});

apps/mobile/src/features/review/reviewState.ts

@@ -87,12 +87,21 @@ const reviewViewedFileIdsByThreadKeyAtom = Atom.family((threadKey: string) =>
   ),
 );
 
-const reviewParsedDiffBySectionCacheKeyAtom = Atom.family((cacheKey: string) =>
-  Atom.make<{ readonly diff: string | null; readonly parsed: ReviewParsedDiff } | null>(null).pipe(
-    Atom.keepAlive,
-    Atom.withLabel(`mobile:review:parsed-diffs:${cacheKey}`),
-  ),
-);
+export const MAX_CACHED_REVIEW_DIFFS = 8;
+// This bounds source string length, not the parsed or native heap size.
+export const MAX_CACHED_REVIEW_SOURCE_CHARACTERS = 4 * 1024 * 1024;
+
+interface CachedReviewParsedDiff {
+  readonly diff: string | null;
+  readonly parsed: ReviewParsedDiff;
+  readonly sourceCharacterCount: number;
+}
+
+// The factory keeps this mutable cache local to the registry and releases it on reset or disposal.
+const reviewParsedDiffCacheAtom = Atom.make(() => ({
+  entries: new Map<string, CachedReviewParsedDiff>(),
+  sourceCharacterCount: 0,
+})).pipe(Atom.keepAlive, Atom.withLabel("mobile:review:parsed-diffs"));
 
 export interface ReviewCacheForThread {
   readonly threadKey: string | null;
@@ -256,6 +265,20 @@ export function updateReviewViewedFileIds(
   });
 }
 
+/** Returns the larger of current input and matching cached source, without changing recency. */
+export function getReviewParsedDiffSourceCharacterCount(input: {
+  readonly threadKey: string;
+  readonly sectionId: string;
+  readonly diff: string | null;
+}): number {
+  const sourceCharacterCount = input.diff?.length ?? 0;
+  const cache = appAtomRegistry.get(reviewParsedDiffCacheAtom);
+  const cached = cache.entries.get(buildSectionCacheKey(input.threadKey, input.sectionId));
+  return cached && cached.diff === (input.diff?.trim() ?? null)
+    ? Math.max(sourceCharacterCount, cached.sourceCharacterCount)
+    : sourceCharacterCount;
+}
+
 export function getCachedReviewParsedDiff(input: {
   readonly threadKey: string | null;
   readonly sectionId: string | null;
@@ -267,16 +290,39 @@ export function getCachedReviewParsedDiff(input: {
 
   const cacheKey = buildSectionCacheKey(input.threadKey, input.sectionId);
   const normalizedDiff = input.diff?.trim() ?? null;
-  const atom = reviewParsedDiffBySectionCacheKeyAtom(cacheKey);
-  const cached = appAtomRegistry.get(atom);
+  const cache = appAtomRegistry.get(reviewParsedDiffCacheAtom);
+  const cached = cache.entries.get(cacheKey);
   if (cached && cached.diff === normalizedDiff) {
+    cache.entries.delete(cacheKey);
+    cache.entries.set(cacheKey, cached);
     return cached.parsed;
   }
 
   const parsed = buildReviewParsedDiff(input.diff, input.sectionId);
-  appAtomRegistry.set(atom, {
+  if (cached) {
+    cache.entries.delete(cacheKey);
+    cache.sourceCharacterCount -= cached.sourceCharacterCount;
+  }
+  const sourceCharacterCount = input.diff?.length ?? 0;
+  if (sourceCharacterCount > MAX_CACHED_REVIEW_SOURCE_CHARACTERS) {
+    return parsed;
+  }
+
+  for (const [oldestKey, oldest] of cache.entries) {
+    if (
+      cache.entries.size < MAX_CACHED_REVIEW_DIFFS &&
+      cache.sourceCharacterCount + sourceCharacterCount <= MAX_CACHED_REVIEW_SOURCE_CHARACTERS
+    ) {
+      break;
+    }
+    cache.entries.delete(oldestKey);
+    cache.sourceCharacterCount -= oldest.sourceCharacterCount;
+  }
+  cache.entries.set(cacheKey, {
     diff: normalizedDiff,
     parsed,
+    sourceCharacterCount,
   });
+  cache.sourceCharacterCount += sourceCharacterCount;
   return parsed;
 }

apps/mobile/src/features/review/useReviewDiffPrewarming.test.ts

@@ -0,0 +1,177 @@
+import { afterEach, assert, it, vi } from "vite-plus/test";
+
+import { appAtomRegistry } from "../../state/atom-registry";
+import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter";
+import * as ReviewModel from "./reviewModel";
+import { getCachedReviewParsedDiff, MAX_CACHED_REVIEW_SOURCE_CHARACTERS } from "./reviewState";
+import { getReviewDiffPrewarmSections, prewarmReviewDiffSection } from "./useReviewDiffPrewarming";
+
+const threadKey = "env:thread-prewarm";
+const reviewDiff = [
+  "diff --git a/src/a.ts b/src/a.ts",
+  "new file mode 100644",
+  "--- /dev/null",
+  "+++ b/src/a.ts",
+  "@@ -0,0 +1 @@",
+  "+export const value = 1;",
+].join("\n");
+
+function makeSection(index: number, diff: string | null = reviewDiff) {
+  return {
+    id: `turn:${index}`,
+    kind: "turn",
+    title: `Turn ${index}`,
+    subtitle: null,
+    isLoading: false,
+    diff,
+  } satisfies ReviewModel.ReviewSectionItem;
+}
+
+function prepareSelection(
+  sections: ReadonlyArray<ReviewModel.ReviewSectionItem>,
+  selectedSectionId: string,
+) {
+  const section = sections.find((candidate) => candidate.id === selectedSectionId);
+  assert.ok(section);
+  const input = { threadKey, sectionId: section.id, diff: section.diff };
+  const parsed = getCachedReviewParsedDiff(input);
+  const native = getCachedNativeReviewDiffData({ parsedDiff: parsed });
+  for (const pending of getReviewDiffPrewarmSections({ threadKey, sections, selectedSectionId })) {
+    prewarmReviewDiffSection({ threadKey: input.threadKey, section: pending });
+  }
+  assert.strictEqual(getCachedReviewParsedDiff(input), parsed);
+  assert.strictEqual(getCachedNativeReviewDiffData({ parsedDiff: parsed }), native);
+  return { parsed, native };
+}
+
+afterEach(() => {
+  appAtomRegistry.reset();
+  vi.restoreAllMocks();
+});
+
+it("warms the nearest sections within the remaining entry budget", () => {
+  const sections = Array.from({ length: 12 }, (_, index) => makeSection(index));
+
+  const pending = getReviewDiffPrewarmSections({
+    threadKey,
+    sections,
+    selectedSectionId: "turn:5",
+  });
+
+  assert.deepStrictEqual(
+    pending.map((section) => section.id),
+    ["turn:4", "turn:6", "turn:3", "turn:7", "turn:2", "turn:8", "turn:1"],
+  );
+});
+
+it("reserves the selected source budget and skips unloaded or oversized sections", () => {
+  const halfBudget = "x".repeat(2 * 1024 * 1024);
+  const sections = [
+    makeSection(0),
+    makeSection(1, halfBudget),
+    makeSection(2, "x".repeat(MAX_CACHED_REVIEW_SOURCE_CHARACTERS + 1)),
+    makeSection(3, halfBudget),
+    makeSection(4, null),
+    makeSection(5),
+  ];
+
+  const pending = getReviewDiffPrewarmSections({
+    threadKey,
+    sections,
+    selectedSectionId: "turn:3",
+  });
+
+  assert.deepStrictEqual(
+    pending.map((section) => section.id),
+    ["turn:1"],
+  );
+  prepareSelection(sections, "turn:3");
+});
+
+it("reserves a larger retained source when the selected input loses whitespace", () => {
+  const parsed = getCachedReviewParsedDiff({
+    threadKey,
+    sectionId: "turn:0",
+    diff: reviewDiff.padEnd(MAX_CACHED_REVIEW_SOURCE_CHARACTERS, " "),
+  });
+  const native = getCachedNativeReviewDiffData({ parsedDiff: parsed });
+
+  const selected = prepareSelection([makeSection(0), makeSection(1)], "turn:0");
+
+  assert.strictEqual(selected.parsed, parsed);
+  assert.strictEqual(selected.native, native);
+});
+
+it("reserves a larger retained source when a neighboring input loses whitespace", () => {
+  const parsed = getCachedReviewParsedDiff({
+    threadKey,
+    sectionId: "turn:1",
+    diff: reviewDiff.padEnd(MAX_CACHED_REVIEW_SOURCE_CHARACTERS - reviewDiff.length, " "),
+  });
+  const native = getCachedNativeReviewDiffData({ parsedDiff: parsed });
+
+  prepareSelection([makeSection(0), makeSection(1), makeSection(2)], "turn:0");
+
+  assert.strictEqual(
+    getCachedReviewParsedDiff({
+      threadKey,
+      sectionId: "turn:1",
+      diff: reviewDiff,
+    }),
+    parsed,
+  );
+  assert.strictEqual(getCachedNativeReviewDiffData({ parsedDiff: parsed }), native);
+});
+
+it.each([null, "turn:missing"])(
+  "does not prewarm without a selected section: %s",
+  (selectedSectionId) => {
+    assert.deepStrictEqual(
+      getReviewDiffPrewarmSections({ threadKey, sections: [makeSection(0)], selectedSectionId }),
+      [],
+    );
+  },
+);
+
+it("does not prewarm when the selected section exceeds the source budget", () => {
+  const sections = [
+    makeSection(0, "x".repeat(MAX_CACHED_REVIEW_SOURCE_CHARACTERS + 1)),
+    makeSection(1),
+  ];
+
+  assert.deepStrictEqual(
+    getReviewDiffPrewarmSections({ threadKey, sections, selectedSectionId: "turn:0" }),
+    [],
+  );
+});
+
+it("does not parse an oversized direct prewarm that the cache cannot retain", () => {
+  const parse = vi.spyOn(ReviewModel, "buildReviewParsedDiff");
+
+  prewarmReviewDiffSection({
+    threadKey,
+    section: makeSection(0, "x".repeat(MAX_CACHED_REVIEW_SOURCE_CHARACTERS + 1)),
+  });
+
+  assert.strictEqual(parse.mock.calls.length, 0);
+});
+
+it("reuses selected and native results across repeated nearby selections", () => {
+  const parse = vi.spyOn(ReviewModel, "buildReviewParsedDiff");
+  const sections = Array.from({ length: 10 }, (_, index) => makeSection(index));
+  const first = prepareSelection(sections, "turn:0");
+  assert.strictEqual(parse.mock.calls.length, 8);
+  parse.mockClear();
+  const second = prepareSelection(sections, "turn:1");
+
+  for (let pass = 0; pass < 3; pass += 1) {
+    const firstAgain = prepareSelection(sections, "turn:0");
+    assert.strictEqual(firstAgain.parsed, first.parsed);
+    assert.strictEqual(firstAgain.native, first.native);
+    const secondAgain = prepareSelection(sections, "turn:1");
+    assert.strictEqual(secondAgain.parsed, second.parsed);
+    assert.strictEqual(secondAgain.native, second.native);
+  }
+
+  assert.strictEqual(parse.mock.calls.length, 0);
+});