mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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 .`
This commit is contained in:
@@ -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<RemoteDiffPanelProps> = ({
|
||||
});
|
||||
|
||||
const diffContent = diffContentsQuery.data?.diff;
|
||||
const [diffVersion, setDiffVersion] = useState(0);
|
||||
const [prevDiffContent, setPrevDiffContent] = useState<string | undefined>(
|
||||
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 {
|
||||
|
||||
@@ -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),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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}`;
|
||||
Reference in New Issue
Block a user