authorgravatar for me@t3.ggTheo Browne <me@t3.gg> 2026-09-04 11:38:54-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-04 11:38:54-07:00
logd6e29dc9dee943b34d6b0d11441fa944c7bff7c9
tree79ca0817fd9a32d424ef2526c57a89207a99a76a
parentcbe93e8dfba68ff6fbe8c69e43a633fdc79db62d
signature Signed by PGP key B5690EEEBB952194

perf(mobile): bound the parsed review cache (#9749)

Bound cached parsed review sections by eight entries and 4,194,304 full source characters. Evicted inactive parsed and native results can be collected. Raw patches, comments, and view state stay unchanged. Prewarm only nearby sections that fit beside the selected section. Use actual retained source weights for normalized cache hits, and skip oversized prewarms. Created with GPT-6 Astra (preview) in Codex.

4 files changed, 397 insertions(+), 16 deletions(-)

apps/mobile/src/features/review/reviewState.test.ts+105-1
...@@ -1,11 +1,31 @@...@@ -1,11 +1,31 @@
1import { assert, it } from "vite-plus/test";1import { afterEach, assert, it } from "vite-plus/test";
22
3import { appAtomRegistry } from "../../state/atom-registry";
4import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter";
3import {5import {
6 getCachedReviewParsedDiff,
4 getReviewAsyncStateSnapshot,7 getReviewAsyncStateSnapshot,
5 setReviewAsyncError,8 setReviewAsyncError,
6 setReviewTurnDiffLoading,9 setReviewTurnDiffLoading,
7} from "./reviewState";10} from "./reviewState";
811
12const reviewInput = {
13 threadKey: "env-local:thread-review",
14 sectionId: "turn:1",
15 diff: [
16 "diff --git a/src/a.ts b/src/a.ts",
17 "new file mode 100644",
18 "--- /dev/null",
19 "+++ b/src/a.ts",
20 "@@ -0,0 +1 @@",
21 "+export const value = 1;",
22 ].join("\n"),
23};
24
25afterEach(() => {
26 appAtomRegistry.reset();
27});
28
9it("stores review async loading and error state in atoms", () => {29it("stores review async loading and error state in atoms", () => {
10 const threadKey = `env-local:thread-review-state-${Date.now()}`;30 const threadKey = `env-local:thread-review-state-${Date.now()}`;
1131
...@@ -25,3 +45,87 @@ it("stores review async loading and error state in atoms", () => {...@@ -25,3 +45,87 @@ it("stores review async loading and error state in atoms", () => {
25 error: null,45 error: null,
26 });46 });
27});47});
48
49it("reuses unchanged parsed diffs and replaces changed sections", () => {
50 const parsed = getCachedReviewParsedDiff(reviewInput);
51
52 assert.strictEqual(
53 getCachedReviewParsedDiff({ ...reviewInput, diff: `\n${reviewInput.diff}\n` }),
54 parsed,
55 );
56
57 const changed = { ...reviewInput, diff: reviewInput.diff.replace("value = 1", "value = 2") };
58 const updated = getCachedReviewParsedDiff(changed);
59 assert.notStrictEqual(updated, parsed);
60 assert.strictEqual(getCachedReviewParsedDiff(changed), updated);
61});
62
63it("evicts the least recently used section across threads without changing live results or IDs", () => {
64 const recentInput = { ...reviewInput, threadKey: "env-local:thread-recent" };
65 const oldestInput = { ...reviewInput, threadKey: "env-remote:thread-oldest" };
66 const recent = getCachedReviewParsedDiff(recentInput);
67 const oldest = getCachedReviewParsedDiff(oldestInput);
68 const native = getCachedNativeReviewDiffData({ parsedDiff: oldest });
69 for (let index = 0; index < 6; index += 1) {
70 getCachedReviewParsedDiff({ ...reviewInput, threadKey: `env-local:thread-${index}` });
71 }
72
73 assert.strictEqual(getCachedReviewParsedDiff(recentInput), recent);
74 getCachedReviewParsedDiff({ ...reviewInput, threadKey: "env-local:thread-new" });
75
76 assert.strictEqual(getCachedReviewParsedDiff(recentInput), recent);
77 const rebuilt = getCachedReviewParsedDiff(oldestInput);
78 assert.notStrictEqual(rebuilt, oldest);
79 assert.deepStrictEqual(rebuilt, oldest);
80 assert.strictEqual(getCachedNativeReviewDiffData({ parsedDiff: oldest }), native);
81 assert.deepStrictEqual(getCachedNativeReviewDiffData({ parsedDiff: rebuilt }), native);
82});
83
84it("limits cached diffs to 4 Mi source characters before the entry limit", () => {
85 const firstInput = { ...reviewInput, diff: "x".repeat(2 * 1024 * 1024) };
86 const secondInput = { ...firstInput, sectionId: "turn:2" };
87 const first = getCachedReviewParsedDiff(firstInput);
88 const second = getCachedReviewParsedDiff(secondInput);
89
90 getCachedReviewParsedDiff({ ...firstInput, sectionId: "turn:3" });
91
92 assert.strictEqual(getCachedReviewParsedDiff(secondInput), second);
93 assert.notStrictEqual(getCachedReviewParsedDiff(firstInput), first);
94});
95
96it("reclaims the source budget when a cached section changes", () => {
97 const firstInput = { ...reviewInput, diff: "x".repeat(2 * 1024 * 1024) };
98 const secondInput = { ...firstInput, sectionId: "turn:2" };
99 getCachedReviewParsedDiff(firstInput);
100 const second = getCachedReviewParsedDiff(secondInput);
101
102 getCachedReviewParsedDiff({ ...firstInput, diff: null });
103 getCachedReviewParsedDiff({ ...firstInput, sectionId: "turn:3" });
104
105 assert.strictEqual(getCachedReviewParsedDiff(secondInput), second);
106});
107
108it("counts the full source and skips oversized diffs without evicting cached sections", () => {
109 const cached = getCachedReviewParsedDiff(reviewInput);
110 const oversizedInput = {
111 ...reviewInput,
112 sectionId: "turn:oversized",
113 diff: `${reviewInput.diff}${" ".repeat(4 * 1024 * 1024)}`,
114 };
115 const first = getCachedReviewParsedDiff(oversizedInput);
116 const second = getCachedReviewParsedDiff(oversizedInput);
117
118 assert.notStrictEqual(second, first);
119 assert.deepStrictEqual(second, first);
120 assert.strictEqual(getCachedReviewParsedDiff(reviewInput), cached);
121});
122
123it("drops parsed diffs when the app registry resets", () => {
124 const parsed = getCachedReviewParsedDiff(reviewInput);
125
126 appAtomRegistry.reset();
127
128 const rebuilt = getCachedReviewParsedDiff(reviewInput);
129 assert.notStrictEqual(rebuilt, parsed);
130 assert.deepStrictEqual(rebuilt, parsed);
131});
apps/mobile/src/features/review/reviewState.ts+55-9
...@@ -87,12 +87,21 @@ const reviewViewedFileIdsByThreadKeyAtom = Atom.family((threadKey: string) =>...@@ -87,12 +87,21 @@ const reviewViewedFileIdsByThreadKeyAtom = Atom.family((threadKey: string) =>
87 ),87 ),
88);88);
8989
90const reviewParsedDiffBySectionCacheKeyAtom = Atom.family((cacheKey: string) =>90export const MAX_CACHED_REVIEW_DIFFS = 8;
91 Atom.make<{ readonly diff: string | null; readonly parsed: ReviewParsedDiff } | null>(null).pipe(91// This bounds source string length, not the parsed or native heap size.
92 Atom.keepAlive,92export const MAX_CACHED_REVIEW_SOURCE_CHARACTERS = 4 * 1024 * 1024;
93 Atom.withLabel(`mobile:review:parsed-diffs:${cacheKey}`),93
94 ),94interface CachedReviewParsedDiff {
95);95 readonly diff: string | null;
96 readonly parsed: ReviewParsedDiff;
97 readonly sourceCharacterCount: number;
98}
99
100// The factory keeps this mutable cache local to the registry and releases it on reset or disposal.
101const reviewParsedDiffCacheAtom = Atom.make(() => ({
102 entries: new Map<string, CachedReviewParsedDiff>(),
103 sourceCharacterCount: 0,
104})).pipe(Atom.keepAlive, Atom.withLabel("mobile:review:parsed-diffs"));
96105
97export interface ReviewCacheForThread {106export interface ReviewCacheForThread {
98 readonly threadKey: string | null;107 readonly threadKey: string | null;
...@@ -256,6 +265,20 @@ export function updateReviewViewedFileIds(...@@ -256,6 +265,20 @@ export function updateReviewViewedFileIds(
256 });265 });
257}266}
258267
268/** Returns the larger of current input and matching cached source, without changing recency. */
269export function getReviewParsedDiffSourceCharacterCount(input: {
270 readonly threadKey: string;
271 readonly sectionId: string;
272 readonly diff: string | null;
273}): number {
274 const sourceCharacterCount = input.diff?.length ?? 0;
275 const cache = appAtomRegistry.get(reviewParsedDiffCacheAtom);
276 const cached = cache.entries.get(buildSectionCacheKey(input.threadKey, input.sectionId));
277 return cached && cached.diff === (input.diff?.trim() ?? null)
278 ? Math.max(sourceCharacterCount, cached.sourceCharacterCount)
279 : sourceCharacterCount;
280}
281
259export function getCachedReviewParsedDiff(input: {282export function getCachedReviewParsedDiff(input: {
260 readonly threadKey: string | null;283 readonly threadKey: string | null;
261 readonly sectionId: string | null;284 readonly sectionId: string | null;
...@@ -267,16 +290,39 @@ export function getCachedReviewParsedDiff(input: {...@@ -267,16 +290,39 @@ export function getCachedReviewParsedDiff(input: {
267290
268 const cacheKey = buildSectionCacheKey(input.threadKey, input.sectionId);291 const cacheKey = buildSectionCacheKey(input.threadKey, input.sectionId);
269 const normalizedDiff = input.diff?.trim() ?? null;292 const normalizedDiff = input.diff?.trim() ?? null;
270 const atom = reviewParsedDiffBySectionCacheKeyAtom(cacheKey);293 const cache = appAtomRegistry.get(reviewParsedDiffCacheAtom);
271 const cached = appAtomRegistry.get(atom);294 const cached = cache.entries.get(cacheKey);
272 if (cached && cached.diff === normalizedDiff) {295 if (cached && cached.diff === normalizedDiff) {
296 cache.entries.delete(cacheKey);
297 cache.entries.set(cacheKey, cached);
273 return cached.parsed;298 return cached.parsed;
274 }299 }
275300
276 const parsed = buildReviewParsedDiff(input.diff, input.sectionId);301 const parsed = buildReviewParsedDiff(input.diff, input.sectionId);
277 appAtomRegistry.set(atom, {302 if (cached) {
303 cache.entries.delete(cacheKey);
304 cache.sourceCharacterCount -= cached.sourceCharacterCount;
305 }
306 const sourceCharacterCount = input.diff?.length ?? 0;
307 if (sourceCharacterCount > MAX_CACHED_REVIEW_SOURCE_CHARACTERS) {
308 return parsed;
309 }
310
311 for (const [oldestKey, oldest] of cache.entries) {
312 if (
313 cache.entries.size < MAX_CACHED_REVIEW_DIFFS &&
314 cache.sourceCharacterCount + sourceCharacterCount <= MAX_CACHED_REVIEW_SOURCE_CHARACTERS
315 ) {
316 break;
317 }
318 cache.entries.delete(oldestKey);
319 cache.sourceCharacterCount -= oldest.sourceCharacterCount;
320 }
321 cache.entries.set(cacheKey, {
278 diff: normalizedDiff,322 diff: normalizedDiff,
279 parsed,323 parsed,
324 sourceCharacterCount,
280 });325 });
326 cache.sourceCharacterCount += sourceCharacterCount;
281 return parsed;327 return parsed;
282}328}
apps/mobile/src/features/review/useReviewDiffPrewarming.test.ts created+177
...@@ -0,0 +1,177 @@
1import { afterEach, assert, it, vi } from "vite-plus/test";
2
3import { appAtomRegistry } from "../../state/atom-registry";
4import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter";
5import * as ReviewModel from "./reviewModel";
6import { getCachedReviewParsedDiff, MAX_CACHED_REVIEW_SOURCE_CHARACTERS } from "./reviewState";
7import { getReviewDiffPrewarmSections, prewarmReviewDiffSection } from "./useReviewDiffPrewarming";
8
9const threadKey = "env:thread-prewarm";
10const reviewDiff = [
11 "diff --git a/src/a.ts b/src/a.ts",
12 "new file mode 100644",
13 "--- /dev/null",
14 "+++ b/src/a.ts",
15 "@@ -0,0 +1 @@",
16 "+export const value = 1;",
17].join("\n");
18
19function makeSection(index: number, diff: string | null = reviewDiff) {
20 return {
21 id: `turn:${index}`,
22 kind: "turn",
23 title: `Turn ${index}`,
24 subtitle: null,
25 isLoading: false,
26 diff,
27 } satisfies ReviewModel.ReviewSectionItem;
28}
29
30function prepareSelection(
31 sections: ReadonlyArray<ReviewModel.ReviewSectionItem>,
32 selectedSectionId: string,
33) {
34 const section = sections.find((candidate) => candidate.id === selectedSectionId);
35 assert.ok(section);
36 const input = { threadKey, sectionId: section.id, diff: section.diff };
37 const parsed = getCachedReviewParsedDiff(input);
38 const native = getCachedNativeReviewDiffData({ parsedDiff: parsed });
39 for (const pending of getReviewDiffPrewarmSections({ threadKey, sections, selectedSectionId })) {
40 prewarmReviewDiffSection({ threadKey: input.threadKey, section: pending });
41 }
42 assert.strictEqual(getCachedReviewParsedDiff(input), parsed);
43 assert.strictEqual(getCachedNativeReviewDiffData({ parsedDiff: parsed }), native);
44 return { parsed, native };
45}
46
47afterEach(() => {
48 appAtomRegistry.reset();
49 vi.restoreAllMocks();
50});
51
52it("warms the nearest sections within the remaining entry budget", () => {
53 const sections = Array.from({ length: 12 }, (_, index) => makeSection(index));
54
55 const pending = getReviewDiffPrewarmSections({
56 threadKey,
57 sections,
58 selectedSectionId: "turn:5",
59 });
60
61 assert.deepStrictEqual(
62 pending.map((section) => section.id),
63 ["turn:4", "turn:6", "turn:3", "turn:7", "turn:2", "turn:8", "turn:1"],
64 );
65});
66
67it("reserves the selected source budget and skips unloaded or oversized sections", () => {
68 const halfBudget = "x".repeat(2 * 1024 * 1024);
69 const sections = [
70 makeSection(0),
71 makeSection(1, halfBudget),
72 makeSection(2, "x".repeat(MAX_CACHED_REVIEW_SOURCE_CHARACTERS + 1)),
73 makeSection(3, halfBudget),
74 makeSection(4, null),
75 makeSection(5),
76 ];
77
78 const pending = getReviewDiffPrewarmSections({
79 threadKey,
80 sections,
81 selectedSectionId: "turn:3",
82 });
83
84 assert.deepStrictEqual(
85 pending.map((section) => section.id),
86 ["turn:1"],
87 );
88 prepareSelection(sections, "turn:3");
89});
90
91it("reserves a larger retained source when the selected input loses whitespace", () => {
92 const parsed = getCachedReviewParsedDiff({
93 threadKey,
94 sectionId: "turn:0",
95 diff: reviewDiff.padEnd(MAX_CACHED_REVIEW_SOURCE_CHARACTERS, " "),
96 });
97 const native = getCachedNativeReviewDiffData({ parsedDiff: parsed });
98
99 const selected = prepareSelection([makeSection(0), makeSection(1)], "turn:0");
100
101 assert.strictEqual(selected.parsed, parsed);
102 assert.strictEqual(selected.native, native);
103});
104
105it("reserves a larger retained source when a neighboring input loses whitespace", () => {
106 const parsed = getCachedReviewParsedDiff({
107 threadKey,
108 sectionId: "turn:1",
109 diff: reviewDiff.padEnd(MAX_CACHED_REVIEW_SOURCE_CHARACTERS - reviewDiff.length, " "),
110 });
111 const native = getCachedNativeReviewDiffData({ parsedDiff: parsed });
112
113 prepareSelection([makeSection(0), makeSection(1), makeSection(2)], "turn:0");
114
115 assert.strictEqual(
116 getCachedReviewParsedDiff({
117 threadKey,
118 sectionId: "turn:1",
119 diff: reviewDiff,
120 }),
121 parsed,
122 );
123 assert.strictEqual(getCachedNativeReviewDiffData({ parsedDiff: parsed }), native);
124});
125
126it.each([null, "turn:missing"])(
127 "does not prewarm without a selected section: %s",
128 (selectedSectionId) => {
129 assert.deepStrictEqual(
130 getReviewDiffPrewarmSections({ threadKey, sections: [makeSection(0)], selectedSectionId }),
131 [],
132 );
133 },
134);
135
136it("does not prewarm when the selected section exceeds the source budget", () => {
137 const sections = [
138 makeSection(0, "x".repeat(MAX_CACHED_REVIEW_SOURCE_CHARACTERS + 1)),
139 makeSection(1),
140 ];
141
142 assert.deepStrictEqual(
143 getReviewDiffPrewarmSections({ threadKey, sections, selectedSectionId: "turn:0" }),
144 [],
145 );
146});
147
148it("does not parse an oversized direct prewarm that the cache cannot retain", () => {
149 const parse = vi.spyOn(ReviewModel, "buildReviewParsedDiff");
150
151 prewarmReviewDiffSection({
152 threadKey,
153 section: makeSection(0, "x".repeat(MAX_CACHED_REVIEW_SOURCE_CHARACTERS + 1)),
154 });
155
156 assert.strictEqual(parse.mock.calls.length, 0);
157});
158
159it("reuses selected and native results across repeated nearby selections", () => {
160 const parse = vi.spyOn(ReviewModel, "buildReviewParsedDiff");
161 const sections = Array.from({ length: 10 }, (_, index) => makeSection(index));
162 const first = prepareSelection(sections, "turn:0");
163 assert.strictEqual(parse.mock.calls.length, 8);
164 parse.mockClear();
165 const second = prepareSelection(sections, "turn:1");
166
167 for (let pass = 0; pass < 3; pass += 1) {
168 const firstAgain = prepareSelection(sections, "turn:0");
169 assert.strictEqual(firstAgain.parsed, first.parsed);
170 assert.strictEqual(firstAgain.native, first.native);
171 const secondAgain = prepareSelection(sections, "turn:1");
172 assert.strictEqual(secondAgain.parsed, second.parsed);
173 assert.strictEqual(secondAgain.native, second.native);
174 }
175
176 assert.strictEqual(parse.mock.calls.length, 0);
177});
apps/mobile/src/features/review/useReviewDiffPrewarming.ts+60-6
...@@ -2,7 +2,12 @@ import { useEffect } from "react";...@@ -2,7 +2,12 @@ import { useEffect } from "react";
22
3import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter";3import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter";
4import type { ReviewSectionItem } from "./reviewModel";4import type { ReviewSectionItem } from "./reviewModel";
5import { getCachedReviewParsedDiff } from "./reviewState";5import {
6 getCachedReviewParsedDiff,
7 getReviewParsedDiffSourceCharacterCount,
8 MAX_CACHED_REVIEW_DIFFS,
9 MAX_CACHED_REVIEW_SOURCE_CHARACTERS,
10} from "./reviewState";
611
7interface IdleDeadlineLike {12interface IdleDeadlineLike {
8 readonly didTimeout: boolean;13 readonly didTimeout: boolean;
...@@ -35,7 +40,7 @@ export function prewarmReviewDiffSection(input: {...@@ -35,7 +40,7 @@ export function prewarmReviewDiffSection(input: {
35 readonly section: ReviewSectionItem;40 readonly section: ReviewSectionItem;
36}): void {41}): void {
37 const { section, threadKey } = input;42 const { section, threadKey } = input;
38 if (section.diff === null) {43 if (section.diff === null || section.diff.length > MAX_CACHED_REVIEW_SOURCE_CHARACTERS) {
39 return;44 return;
40 }45 }
4146
...@@ -47,7 +52,54 @@ export function prewarmReviewDiffSection(input: {...@@ -47,7 +52,54 @@ export function prewarmReviewDiffSection(input: {
47 getCachedNativeReviewDiffData({ parsedDiff, comments: [] });52 getCachedNativeReviewDiffData({ parsedDiff, comments: [] });
48}53}
4954
50/** Warms one cached section per idle period, after navigation animations finish. */55/** Selects nearby loaded sections that fit in the cache with the selected section. */
56export function getReviewDiffPrewarmSections(input: {
57 readonly threadKey: string;
58 readonly sections: ReadonlyArray<ReviewSectionItem>;
59 readonly selectedSectionId: string | null;
60}): ReadonlyArray<ReviewSectionItem> {
61 const { threadKey, sections, selectedSectionId } = input;
62 const selectedIndex = sections.findIndex((section) => section.id === selectedSectionId);
63 const selectedSection = sections[selectedIndex];
64 if (!selectedSection) {
65 return [];
66 }
67
68 let sourceCharacterCount = getReviewParsedDiffSourceCharacterCount({
69 threadKey,
70 sectionId: selectedSection.id,
71 diff: selectedSection.diff,
72 });
73 if (sourceCharacterCount > MAX_CACHED_REVIEW_SOURCE_CHARACTERS) {
74 return [];
75 }
76
77 const pendingSections: ReviewSectionItem[] = [];
78 for (let distance = 1; distance < sections.length; distance += 1) {
79 for (const index of [selectedIndex - distance, selectedIndex + distance]) {
80 if (pendingSections.length >= MAX_CACHED_REVIEW_DIFFS - 1) {
81 return pendingSections;
82 }
83 const section = sections[index];
84 if (!section || section.diff === null) {
85 continue;
86 }
87 const sectionCharacterCount = getReviewParsedDiffSourceCharacterCount({
88 threadKey,
89 sectionId: section.id,
90 diff: section.diff,
91 });
92 if (sourceCharacterCount + sectionCharacterCount > MAX_CACHED_REVIEW_SOURCE_CHARACTERS) {
93 continue;
94 }
95 pendingSections.push(section);
96 sourceCharacterCount += sectionCharacterCount;
97 }
98 }
99 return pendingSections;
100}
101
102/** Warms one nearby section per idle period, after navigation animations finish. */
51export function useReviewDiffPrewarming(input: {103export function useReviewDiffPrewarming(input: {
52 readonly threadKey: string | null;104 readonly threadKey: string | null;
53 readonly sections: ReadonlyArray<ReviewSectionItem>;105 readonly sections: ReadonlyArray<ReviewSectionItem>;
...@@ -60,9 +112,11 @@ export function useReviewDiffPrewarming(input: {...@@ -60,9 +112,11 @@ export function useReviewDiffPrewarming(input: {
60 return;112 return;
61 }113 }
62114
63 const pendingSections = sections.filter(115 const pendingSections = getReviewDiffPrewarmSections({
64 (section) => section.id !== selectedSectionId && section.diff !== null,116 threadKey,
65 );117 sections,
118 selectedSectionId,
119 });
66 if (pendingSections.length === 0) {120 if (pendingSections.length === 0) {
67 return;121 return;
68 }122 }