From 803cfeb8824bc540776db11ad3aa159a33755a65 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 24 Mar 2026 14:29:53 +0100 Subject: [PATCH] fix(site/src/pages/AgentsPage): stabilize remote diff cache keys (#23487) ## Summary - use React Query's `dataUpdatedAt` as the remote diff cache invalidation token instead of a component-local counter - keep the `@pierre/diffs` cache key stable across remounts without a custom hashing implementation - preserve targeted coverage for the cache-key helper used by the /agents remote diff viewer ## Testing - `cd site && pnpm exec biome check src/pages/AgentsPage/components/DiffViewer/RemoteDiffPanel.tsx src/pages/AgentsPage/components/DiffViewer/diffCacheKey.ts src/pages/AgentsPage/components/DiffViewer/diffCacheKey.test.ts` - `cd site && pnpm exec vitest run src/pages/AgentsPage/components/DiffViewer/diffCacheKey.test.ts --project=unit` - `cd site && pnpm exec tsc -p .` --- .../components/DiffViewer/RemoteDiffPanel.tsx | 30 +++++++------------ .../DiffViewer/diffCacheKey.test.ts | 21 +++++++++++++ .../components/DiffViewer/diffCacheKey.ts | 12 ++++++++ 3 files changed, 43 insertions(+), 20 deletions(-) create mode 100644 site/src/pages/AgentsPage/components/DiffViewer/diffCacheKey.test.ts create mode 100644 site/src/pages/AgentsPage/components/DiffViewer/diffCacheKey.ts diff --git a/site/src/pages/AgentsPage/components/DiffViewer/RemoteDiffPanel.tsx b/site/src/pages/AgentsPage/components/DiffViewer/RemoteDiffPanel.tsx index 29d1fcee52..7ba00551f7 100644 --- a/site/src/pages/AgentsPage/components/DiffViewer/RemoteDiffPanel.tsx +++ b/site/src/pages/AgentsPage/components/DiffViewer/RemoteDiffPanel.tsx @@ -19,6 +19,7 @@ import type { ChatMessageInputRef } from "../AgentChatInput"; import { CommentableDiffViewer } from "../DiffViewer/CommentableDiffViewer"; import { DiffStatBadge } from "../DiffViewer/DiffStats"; import type { DiffStyle } from "../DiffViewer/DiffViewer"; +import { getDiffCacheKeyPrefix } from "../DiffViewer/diffCacheKey"; export { InlinePromptInput } from "../DiffViewer/CommentableDiffViewer"; @@ -89,33 +90,22 @@ export const RemoteDiffPanel: FC = ({ }); const diffContent = diffContentsQuery.data?.diff; - const [diffVersion, setDiffVersion] = useState(0); - const [prevDiffContent, setPrevDiffContent] = useState( - undefined, - ); - if (diffContent !== prevDiffContent) { - setPrevDiffContent(diffContent); - setDiffVersion((v) => v + 1); - } - const parsedFiles = (() => { if (!diffContent) { return [] as FileDiffMetadata[]; } try { - // The cacheKeyPrefix enables the worker pool's LRU cache - // so highlighted ASTs are reused across re-renders instead - // of being re-computed on every render cycle. We include a - // version counter derived from the diff content so that when - // the diff changes (e.g. new commits pushed) the old cached - // highlight AST is not reused with mismatched line indices, - // which would cause DiffHunksRenderer.processDiffResult to - // throw. Unlike dataUpdatedAt, this counter only increments - // when the actual diff string changes, avoiding unnecessary - // recomputation on refetches with identical content. + // The @pierre/diffs worker pool only keys cached highlighted + // ASTs by `cacheKey`, so the key must change whenever the diff + // query updates. React Query's `dataUpdatedAt` survives panel + // remounts, which prevents stale cache hits from pairing a new + // FileDiffMetadata with an older highlighted AST. const patches = parsePatchFiles( diffContent, - `chat-${chatId}-v${diffVersion}`, + getDiffCacheKeyPrefix( + `chat-${chatId}`, + diffContentsQuery.dataUpdatedAt, + ), ); return patches.flatMap((p) => p.files); } catch { diff --git a/site/src/pages/AgentsPage/components/DiffViewer/diffCacheKey.test.ts b/site/src/pages/AgentsPage/components/DiffViewer/diffCacheKey.test.ts new file mode 100644 index 0000000000..5797860289 --- /dev/null +++ b/site/src/pages/AgentsPage/components/DiffViewer/diffCacheKey.test.ts @@ -0,0 +1,21 @@ +import { getDiffCacheKeyPrefix } from "./diffCacheKey"; + +describe("getDiffCacheKeyPrefix", () => { + it("returns the same key for the same scope and query update time", () => { + expect(getDiffCacheKeyPrefix("chat-123", 101)).toBe( + getDiffCacheKeyPrefix("chat-123", 101), + ); + }); + + it("changes when the query update time changes", () => { + expect(getDiffCacheKeyPrefix("chat-123", 101)).not.toBe( + getDiffCacheKeyPrefix("chat-123", 202), + ); + }); + + it("changes when the scope changes", () => { + expect(getDiffCacheKeyPrefix("chat-123", 101)).not.toBe( + getDiffCacheKeyPrefix("chat-456", 101), + ); + }); +}); diff --git a/site/src/pages/AgentsPage/components/DiffViewer/diffCacheKey.ts b/site/src/pages/AgentsPage/components/DiffViewer/diffCacheKey.ts new file mode 100644 index 0000000000..cb015da2d9 --- /dev/null +++ b/site/src/pages/AgentsPage/components/DiffViewer/diffCacheKey.ts @@ -0,0 +1,12 @@ +/** + * Build a stable worker-pool cache key prefix for `@pierre/diffs`. + * + * We use React Query's `dataUpdatedAt` as the invalidation token instead of a + * component-local counter. That timestamp survives component remounts, so a + * freshly fetched diff cannot accidentally reuse a highlighted AST cached for + * an older diff body. + */ +export const getDiffCacheKeyPrefix = ( + prefix: string, + dataUpdatedAt: number, +): string => `${prefix}-${dataUpdatedAt}`;