Skip to content

Commit c3e09d5

Browse files
authored
🤖 perf: cache review panel diff/tree to speed up workspace switching (#1153)
## Summary Cache git diff and file tree results in a module-scope LRU cache so switching back to a previously visited workspace is near-instant. ## Changes - Add single LRU cache (20MB limit, 20 entries max) for both diff hunks and file tree - Cache key derived from git command string (auto-updates when filters change) - Ctrl/Cmd+R bypasses cache and re-fetches - Simplified implementation: cache check happens in effect only (no sync init) ## Testing - Switch workspace A → B → A: second visit to A should be instant - Ctrl/Cmd+R in review tab: should re-run git and update cache - `make static-check` passes --- _Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking: `high`_
1 parent 7ff598c commit c3e09d5

File tree

1 file changed

+190
-68
lines changed

1 file changed

+190
-68
lines changed

‎src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx‎

Lines changed: 190 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
* - Frontend filtering is fast even for 1000+ hunks (<5ms)
2323
*/
2424

25+
import { LRUCache } from "lru-cache";
2526
import React, { useState, useEffect, useMemo, useCallback, useRef } from "react";
2627
import { HunkViewer } from "./HunkViewer";
2728
import { ReviewControls } from "./ReviewControls";
@@ -41,7 +42,7 @@ import type { FileTreeNode } from "@/common/utils/git/numstatParser";
4142
import { matchesKeybind, KEYBINDS, formatKeybind } from "@/browser/utils/ui/keybinds";
4243
import { applyFrontendFilters } from "@/browser/utils/review/filterHunks";
4344
import { cn } from "@/common/lib/utils";
44-
import { useAPI } from "@/browser/contexts/API";
45+
import { useAPI, type APIClient } from "@/browser/contexts/API";
4546

4647
/** Stats reported to parent for tab display */
4748
interface ReviewPanelStats {
@@ -87,6 +88,68 @@ type DiffState =
8788
| { status: "loaded"; hunks: DiffHunk[]; truncationWarning: string | null }
8889
| { status: "error"; message: string };
8990

91+
const REVIEW_PANEL_CACHE_MAX_ENTRIES = 20;
92+
const REVIEW_PANEL_CACHE_MAX_SIZE_BYTES = 20 * 1024 * 1024; // 20MB
93+
94+
interface ReviewPanelDiffCacheValue {
95+
hunks: DiffHunk[];
96+
truncationWarning: string | null;
97+
diagnosticInfo: DiagnosticInfo | null;
98+
}
99+
100+
type ReviewPanelCacheValue = ReviewPanelDiffCacheValue | FileTreeNode;
101+
102+
function estimateJsonSizeBytes(value: unknown): number {
103+
// Rough bytes for JS strings (UTF-16). Used only for LRU sizing.
104+
try {
105+
return JSON.stringify(value).length * 2;
106+
} catch {
107+
// If we ever hit an unserializable structure, treat it as huge so it won't stick in cache.
108+
return Number.MAX_SAFE_INTEGER;
109+
}
110+
}
111+
112+
const reviewPanelCache = new LRUCache<string, ReviewPanelCacheValue>({
113+
max: REVIEW_PANEL_CACHE_MAX_ENTRIES,
114+
maxSize: REVIEW_PANEL_CACHE_MAX_SIZE_BYTES,
115+
sizeCalculation: (value) => estimateJsonSizeBytes(value),
116+
});
117+
118+
function makeReviewPanelCacheKey(params: {
119+
workspaceId: string;
120+
workspacePath: string;
121+
gitCommand: string;
122+
}): string {
123+
// Key off the actual git command to avoid forgetting to include new inputs.
124+
return [params.workspaceId, params.workspacePath, params.gitCommand].join("\u0000");
125+
}
126+
127+
type ExecuteBashResult = Awaited<ReturnType<APIClient["workspace"]["executeBash"]>>;
128+
type ExecuteBashSuccess = Extract<ExecuteBashResult, { success: true }>;
129+
130+
async function executeWorkspaceBashAndCache<T extends ReviewPanelCacheValue>(params: {
131+
api: APIClient;
132+
workspaceId: string;
133+
script: string;
134+
cacheKey: string;
135+
timeoutSecs: number;
136+
parse: (result: ExecuteBashSuccess) => T;
137+
}): Promise<T> {
138+
const result = await params.api.workspace.executeBash({
139+
workspaceId: params.workspaceId,
140+
script: params.script,
141+
options: { timeout_secs: params.timeoutSecs },
142+
});
143+
144+
if (!result.success) {
145+
throw new Error(result.error ?? "Unknown error");
146+
}
147+
148+
const value = params.parse(result);
149+
reviewPanelCache.set(params.cacheKey, value);
150+
return value;
151+
}
152+
90153
export const ReviewPanel: React.FC<ReviewPanelProps> = ({
91154
workspaceId,
92155
workspacePath,
@@ -100,7 +163,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
100163
const searchInputRef = useRef<HTMLInputElement>(null);
101164

102165
// Unified diff state - discriminated union makes invalid states unrepresentable
103-
// Note: Parent renders with key={workspaceId}, so component remounts on workspace change
166+
// Note: Parent renders with key={workspaceId}, so component remounts on workspace change.
104167
const [diffState, setDiffState] = useState<DiffState>({ status: "loading" });
105168

106169
const [selectedHunkId, setSelectedHunkId] = useState<string | null>(null);
@@ -109,9 +172,15 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
109172
const [isPanelFocused, setIsPanelFocused] = useState(false);
110173
const [refreshTrigger, setRefreshTrigger] = useState(0);
111174
const [fileTree, setFileTree] = useState<FileTreeNode | null>(null);
175+
112176
// Map of hunkId -> toggle function for expand/collapse
113177
const toggleExpandFnsRef = useRef<Map<string, () => void>>(new Map());
114178

179+
// Track refresh trigger changes so we can distinguish initial mount vs manual refresh.
180+
// Each effect gets its own ref to avoid cross-effect interference.
181+
const lastDiffRefreshTriggerRef = useRef<number | null>(null);
182+
const lastFileTreeRefreshTriggerRef = useRef<number | null>(null);
183+
115184
// Unified search state (per-workspace persistence)
116185
const [searchState, setSearchState] = usePersistedState<ReviewSearchState>(
117186
getReviewSearchStateKey(workspaceId),
@@ -177,36 +246,61 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
177246
if (!api || isCreating) return;
178247
let cancelled = false;
179248

249+
const prevRefreshTrigger = lastFileTreeRefreshTriggerRef.current;
250+
lastFileTreeRefreshTriggerRef.current = refreshTrigger;
251+
const isManualRefresh = prevRefreshTrigger !== null && prevRefreshTrigger !== refreshTrigger;
252+
253+
const numstatCommand = buildGitDiffCommand(
254+
filters.diffBase,
255+
filters.includeUncommitted,
256+
"", // No path filter for file tree
257+
"numstat"
258+
);
259+
260+
const cacheKey = makeReviewPanelCacheKey({
261+
workspaceId,
262+
workspacePath,
263+
gitCommand: numstatCommand,
264+
});
265+
266+
// Fast path: use cached tree when switching workspaces (unless user explicitly refreshed).
267+
if (!isManualRefresh) {
268+
const cachedTree = reviewPanelCache.get(cacheKey) as FileTreeNode | undefined;
269+
if (cachedTree) {
270+
setFileTree(cachedTree);
271+
setIsLoadingTree(false);
272+
return () => {
273+
cancelled = true;
274+
};
275+
}
276+
}
277+
180278
const loadFileTree = async () => {
181279
setIsLoadingTree(true);
182280
try {
183-
const numstatCommand = buildGitDiffCommand(
184-
filters.diffBase,
185-
filters.includeUncommitted,
186-
"", // No path filter for file tree
187-
"numstat"
188-
);
189-
190-
const numstatResult = await api.workspace.executeBash({
281+
const tree = await executeWorkspaceBashAndCache({
282+
api,
191283
workspaceId,
192284
script: numstatCommand,
193-
options: { timeout_secs: 30 },
285+
cacheKey,
286+
timeoutSecs: 30,
287+
parse: (result) => {
288+
const numstatOutput = result.data.output ?? "";
289+
const fileStats = parseNumstat(numstatOutput);
290+
291+
// Build tree with original paths (needed for git commands)
292+
return buildFileTree(fileStats);
293+
},
194294
});
195295

196296
if (cancelled) return;
197-
198-
if (numstatResult.success) {
199-
const numstatOutput = numstatResult.data.output ?? "";
200-
const fileStats = parseNumstat(numstatOutput);
201-
202-
// Build tree with original paths (needed for git commands)
203-
const tree = buildFileTree(fileStats);
204-
setFileTree(tree);
205-
}
297+
setFileTree(tree);
206298
} catch (err) {
207299
console.error("Failed to load file tree:", err);
208300
} finally {
209-
setIsLoadingTree(false);
301+
if (!cancelled) {
302+
setIsLoadingTree(false);
303+
}
210304
}
211305
};
212306

@@ -231,6 +325,46 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
231325
if (!api || isCreating) return;
232326
let cancelled = false;
233327

328+
const prevRefreshTrigger = lastDiffRefreshTriggerRef.current;
329+
lastDiffRefreshTriggerRef.current = refreshTrigger;
330+
const isManualRefresh = prevRefreshTrigger !== null && prevRefreshTrigger !== refreshTrigger;
331+
332+
const pathFilter = selectedFilePath ? ` -- "${extractNewPath(selectedFilePath)}"` : "";
333+
334+
const diffCommand = buildGitDiffCommand(
335+
filters.diffBase,
336+
filters.includeUncommitted,
337+
pathFilter,
338+
"diff"
339+
);
340+
341+
const cacheKey = makeReviewPanelCacheKey({
342+
workspaceId,
343+
workspacePath,
344+
gitCommand: diffCommand,
345+
});
346+
347+
// Fast path: use cached diff when switching workspaces (unless user explicitly refreshed).
348+
if (!isManualRefresh) {
349+
const cached = reviewPanelCache.get(cacheKey) as ReviewPanelDiffCacheValue | undefined;
350+
if (cached) {
351+
setDiagnosticInfo(cached.diagnosticInfo);
352+
setDiffState({
353+
status: "loaded",
354+
hunks: cached.hunks,
355+
truncationWarning: cached.truncationWarning,
356+
});
357+
358+
if (cached.hunks.length > 0) {
359+
setSelectedHunkId((prev) => prev ?? cached.hunks[0].id);
360+
}
361+
362+
return () => {
363+
cancelled = true;
364+
};
365+
}
366+
}
367+
234368
// Transition to appropriate loading state:
235369
// - "refreshing" if we have data (keeps UI stable during refresh)
236370
// - "loading" if no data yet
@@ -252,65 +386,54 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
252386
// - includeUncommitted: include working directory changes
253387
// - selectedFilePath: ESSENTIAL for truncation - if full diff is cut off,
254388
// path filter lets us retrieve specific file's hunks
255-
const pathFilter = selectedFilePath ? ` -- "${extractNewPath(selectedFilePath)}"` : "";
256-
257-
const diffCommand = buildGitDiffCommand(
258-
filters.diffBase,
259-
filters.includeUncommitted,
260-
pathFilter,
261-
"diff"
262-
);
263-
264-
// Fetch diff
265-
const diffResult = await api.workspace.executeBash({
389+
const data = await executeWorkspaceBashAndCache({
390+
api,
266391
workspaceId,
267392
script: diffCommand,
268-
options: { timeout_secs: 30 },
393+
cacheKey,
394+
timeoutSecs: 30,
395+
parse: (result) => {
396+
const diffOutput = result.data.output ?? "";
397+
const truncationInfo = "truncated" in result.data ? result.data.truncated : undefined;
398+
399+
const fileDiffs = parseDiff(diffOutput);
400+
const allHunks = extractAllHunks(fileDiffs);
401+
402+
const diagnosticInfo: DiagnosticInfo = {
403+
command: diffCommand,
404+
outputLength: diffOutput.length,
405+
fileDiffCount: fileDiffs.length,
406+
hunkCount: allHunks.length,
407+
};
408+
409+
// Build truncation warning (only when not filtering by path)
410+
const truncationWarning =
411+
truncationInfo && !selectedFilePath
412+
? `Diff truncated (${truncationInfo.reason}). Filter by file to see more.`
413+
: null;
414+
415+
return { hunks: allHunks, truncationWarning, diagnosticInfo };
416+
},
269417
});
270418

271419
if (cancelled) return;
272420

273-
if (!diffResult.success) {
274-
// Real error (not truncation-related)
275-
console.error("Git diff failed:", diffResult.error);
276-
setDiffState({ status: "error", message: diffResult.error ?? "Unknown error" });
277-
setDiagnosticInfo(null);
278-
return;
279-
}
280-
281-
const diffOutput = diffResult.data.output ?? "";
282-
const truncationInfo =
283-
"truncated" in diffResult.data ? diffResult.data.truncated : undefined;
284-
285-
const fileDiffs = parseDiff(diffOutput);
286-
const allHunks = extractAllHunks(fileDiffs);
287-
288-
// Store diagnostic info
289-
setDiagnosticInfo({
290-
command: diffCommand,
291-
outputLength: diffOutput.length,
292-
fileDiffCount: fileDiffs.length,
293-
hunkCount: allHunks.length,
421+
setDiagnosticInfo(data.diagnosticInfo);
422+
setDiffState({
423+
status: "loaded",
424+
hunks: data.hunks,
425+
truncationWarning: data.truncationWarning,
294426
});
295427

296-
// Build truncation warning (only when not filtering by path)
297-
const truncationWarning =
298-
truncationInfo && !selectedFilePath
299-
? `Diff truncated (${truncationInfo.reason}). Filter by file to see more.`
300-
: null;
301-
302-
// Single atomic state update with all data
303-
setDiffState({ status: "loaded", hunks: allHunks, truncationWarning });
304-
305-
// Auto-select first hunk if none selected
306-
if (allHunks.length > 0 && !selectedHunkId) {
307-
setSelectedHunkId(allHunks[0].id);
428+
if (data.hunks.length > 0) {
429+
setSelectedHunkId((prev) => prev ?? data.hunks[0].id);
308430
}
309431
} catch (err) {
310432
if (cancelled) return;
311433
const errorMsg = `Failed to load diff: ${err instanceof Error ? err.message : String(err)}`;
312434
console.error(errorMsg);
313435
setDiffState({ status: "error", message: errorMsg });
436+
setDiagnosticInfo(null);
314437
}
315438
};
316439

@@ -319,9 +442,8 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
319442
return () => {
320443
cancelled = true;
321444
};
322-
// selectedHunkId intentionally omitted - only auto-select on initial load, not on every selection change
323-
// eslint-disable-next-line react-hooks/exhaustive-deps
324445
}, [
446+
api,
325447
workspaceId,
326448
workspacePath,
327449
filters.diffBase,

0 commit comments

Comments
 (0)