From d311797653a3b6ec7f767f663bf889fd98fe1dbc Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 9 Sep 2026 16:05:44 +0200 Subject: [PATCH] Reduce stalls when opening large diffs (#4574) * perf(app): reduce avoidable work when opening large diffs Large snapshots were recursively reconstructed for reference stability, while native scroll worklets captured complete file models. Preserve identity at the data producers and prepare text for the visible window without rebuilding unchanged files. * test(app): await commit diff query completion without polling * chore(app): remove profiling artifacts from the source tree --- .../app/src/data/providers-snapshot.test.ts | 39 ++++ packages/app/src/data/providers-snapshot.ts | 2 + packages/app/src/data/push-router.test.ts | 71 ++++++++ packages/app/src/data/push-router.ts | 3 + packages/app/src/data/query.ts | 19 +- .../src/git/commit-file-diff-query.test.ts | 76 ++++++++ .../app/src/git/commit-file-diff-query.ts | 28 +++ .../horizontal-scroll.native.tsx | 3 +- .../app/src/git/diff-document/model.test.ts | 49 +++++ packages/app/src/git/diff-document/model.ts | 169 ++++++++++-------- .../src/git/diff-document/surface.native.tsx | 17 +- .../diff-document/text-measurement.test.ts | 1 + .../src/git/diff-document/text-measurement.ts | 37 ++-- .../app/src/git/diff-document/text.native.ts | 135 +++++++++----- .../git/diff-document/workspace-cache.test.ts | 38 +++- .../src/git/diff-document/workspace-cache.tsx | 17 +- packages/app/src/git/diff-sharing.test.ts | 148 +++++++++++++++ packages/app/src/git/diff-sharing.ts | 98 ++++++++++ packages/app/src/git/use-diff-files.ts | 24 ++- packages/app/src/git/use-diff-query.ts | 2 + .../app/src/hooks/use-providers-snapshot.ts | 3 + 21 files changed, 816 insertions(+), 163 deletions(-) create mode 100644 packages/app/src/git/commit-file-diff-query.test.ts create mode 100644 packages/app/src/git/commit-file-diff-query.ts create mode 100644 packages/app/src/git/diff-sharing.test.ts create mode 100644 packages/app/src/git/diff-sharing.ts diff --git a/packages/app/src/data/providers-snapshot.test.ts b/packages/app/src/data/providers-snapshot.test.ts index c1a8ca1ec2..56cf80653a 100644 --- a/packages/app/src/data/providers-snapshot.test.ts +++ b/packages/app/src/data/providers-snapshot.test.ts @@ -3,6 +3,7 @@ import { QueryClient, CancelledError } from "@tanstack/react-query"; import type { ProviderSnapshotEntry } from "@getpaseo/protocol/agent-types"; import type { GetProvidersSnapshotResponseMessage } from "@getpaseo/protocol/messages"; import { compactProviderSnapshot } from "@getpaseo/protocol/provider-snapshot-codec"; +import { applyProvidersSnapshotUpdate } from "./push-router"; import { createProviderSnapshotCache } from "./provider-snapshot-cache"; import { resolveProviderIconName, @@ -296,3 +297,41 @@ it("keeps a replacement query when the cancelled native fetch finishes late", as replaceProviderSnapshotIcons(serverId, []); } }); + +describe("provider snapshot publication identity", () => { + it("keeps the hash cache's canonical objects across a catalog change and later announcements", async () => { + const cache = createProviderSnapshotCache(createStorage()); + const queryClient = new QueryClient(); + const serverId = "canonical-provider-snapshot"; + const client = { + async getProvidersSnapshot() { + throw new Error("Unexpected content fetch"); + }, + }; + const changed = { + ...snapshot, + snapshotHash: "next-hash", + compactSnapshot: compactProviderSnapshot([{ ...entries[0]!, label: "Updated provider" }]), + }; + try { + for (const body of [snapshot, changed, changed]) { + await applyProvidersSnapshotUpdate({ + serverId, + queryClient, + cache, + client, + message: { type: "providers_snapshot_update", payload: body }, + }); + const canonical = await cache.materialize(serverId, body); + const published = queryClient.getQueryData( + providersSnapshotQueryKey(serverId, body.cwd), + )!; + expect(published.entries).toBe(canonical.entries); + expect(published.compactSnapshot).toBe(canonical.compactSnapshot); + expect(published.entries[0]!.models).toBe(canonical.entries[0]!.models); + } + } finally { + queryClient.clear(); + } + }); +}); diff --git a/packages/app/src/data/providers-snapshot.ts b/packages/app/src/data/providers-snapshot.ts index 0e9cc26495..fadaf5ee77 100644 --- a/packages/app/src/data/providers-snapshot.ts +++ b/packages/app/src/data/providers-snapshot.ts @@ -78,6 +78,7 @@ export async function fetchProvidersSnapshot(input: { const body = await queryClient.fetchQuery({ queryKey: ["providerSnapshotContent", input.serverId, hash], gcTime: 0, + structuralSharing: false, staleTime: 0, retry: false, queryFn: async () => { @@ -134,6 +135,7 @@ export async function refreshAndApplyProvidersSnapshot(input: { await input.queryClient.fetchQuery({ queryKey, staleTime: 0, + structuralSharing: false, queryFn: ({ signal }) => fetchProvidersSnapshot({ ...input, signal }), }); void input.queryClient.invalidateQueries({ diff --git a/packages/app/src/data/push-router.test.ts b/packages/app/src/data/push-router.test.ts index 7bc9ea2b51..ae99c5dfbc 100644 --- a/packages/app/src/data/push-router.test.ts +++ b/packages/app/src/data/push-router.test.ts @@ -146,6 +146,77 @@ function providerUpdate(generatedAt: string): ProvidersSnapshotUpdateMessage { } describe("server data push router", () => { + it("retains equal diff files without reconstructing changed file bodies", () => { + const queryClient = new QueryClient(); + const fake = createFakeClient(); + const serverId = "diff-sharing"; + const cwd = "/repo"; + const queryKey = checkoutDiffQueryKey(serverId, cwd, "uncommitted", undefined, false); + const subscriptionId = "diff-sharing"; + queryClient.getQueryCache().build(queryClient, { + queryKey, + queryFn: skipToken, + meta: checkoutDiffPushRoute({ + enabled: false, + serverId, + cwd, + subscriptionId, + compare: { mode: "uncommitted" }, + }), + }); + const unmount = mountServerDataPushRouter({ queryClient, client: fake.client, serverId }); + type Payload = SubscribeCheckoutDiffResponseMessage["payload"]; + const files: Payload["files"] = ["a.ts", "b.ts"].map((path) => ({ + path, + isNew: true, + isDeleted: false, + additions: 2, + deletions: 0, + hunks: [ + { + oldStart: 0, + oldCount: 0, + newStart: 1, + newCount: 2, + lines: [ + { + type: "add", + content: "const answer = 42", + tokens: [ + { text: "const", style: "keyword" }, + { text: " answer = 42", style: null }, + ], + }, + { type: "add", content: "answer", tokens: [{ text: "answer", style: null }] }, + ], + }, + ], + })); + const publish = (incomingFiles: Payload["files"], requestId: string) => { + fake.emit({ + type: "subscribe_checkout_diff_response", + payload: { subscriptionId, cwd, files: incomingFiles, requestId, error: null }, + }); + return queryClient.getQueryData(queryKey)!; + }; + try { + const first = publish(files, "first"); + const same = publish(structuredClone(files), "reopen"); + expect(same.files).toBe(first.files); + expect(same.requestId).toBe("reopen"); + const changed = structuredClone(files); + changed[0]!.hunks[0]!.lines[0]!.tokens![0]!.style = "variable"; + const next = publish(changed, "edit"); + expect(next.files[1]).toBe(first.files[1]); + expect(next.files[0]).toBe(changed[0]); + expect(next.files[0]!.hunks[0]!.lines[0]!.tokens![0]!.style).toBe("variable"); + expect(publish([], "deleted").files).toEqual([]); + } finally { + unmount(); + queryClient.clear(); + } + }); + it("routes provider snapshot and daemon config payloads until detached", async () => { const queryClient = new QueryClient(); const fake = createFakeClient(); diff --git a/packages/app/src/data/push-router.ts b/packages/app/src/data/push-router.ts index 1cdf555874..72c8ef20b2 100644 --- a/packages/app/src/data/push-router.ts +++ b/packages/app/src/data/push-router.ts @@ -5,6 +5,7 @@ import type { SessionOutboundMessage, } from "@getpaseo/protocol/messages"; import { agentCommandsQueryRoot } from "@/hooks/agent-commands-query"; +import { shareCheckoutDiff } from "@/git/diff-sharing"; import { orderCheckoutDiffFiles } from "@/git/diff-order"; import { daemonConfigQueryKey } from "@/data/daemon-config"; import { daemonPairingOfferQueryKey } from "@/data/daemon-pairing"; @@ -203,6 +204,7 @@ export async function applyProvidersSnapshotUpdate(input: { await input.queryClient.fetchQuery({ queryKey, staleTime: 0, + structuralSharing: false, retry: false, queryFn: ({ signal }) => { const incoming = announcement; @@ -510,6 +512,7 @@ function setCheckoutDiffPayload(input: { ) { continue; } + query.setOptions({ ...query.options, structuralSharing: shareCheckoutDiff }); input.queryClient.setQueryData(query.queryKey, input.payload); } } diff --git a/packages/app/src/data/query.ts b/packages/app/src/data/query.ts index e1e28ee3b7..2a645f8666 100644 --- a/packages/app/src/data/query.ts +++ b/packages/app/src/data/query.ts @@ -31,8 +31,10 @@ type FetchQueryInput = > & { dataShape: "list" | "value"; queryFn: QueryFnOption; - staleTimeMs: number; -}; +} & ( + | { staleTimeMs: number; immutableWhen?: never } + | { staleTimeMs?: never; immutableWhen: (data: TQueryFnData) => boolean } + ); export function useReplicaQuery< TQueryFnData, @@ -88,7 +90,7 @@ function replicaQueryOptions< }; } -function fetchQueryOptions< +export function fetchQueryOptions< TQueryFnData, TError = Error, TData = TQueryFnData, @@ -96,11 +98,11 @@ function fetchQueryOptions< >( input: FetchQueryInput, ): UseQueryOptions { - if (!Number.isFinite(input.staleTimeMs)) { + if (!input.immutableWhen && !Number.isFinite(input.staleTimeMs)) { throw new Error("Fetch queries must declare a finite staleTimeMs."); } - const { dataShape, meta, staleTimeMs, ...options } = input; + const { dataShape, meta, staleTimeMs, immutableWhen, ...options } = input; return { ...options, ...(dataShape === "list" ? { placeholderData: keepPreviousData } : {}), @@ -111,7 +113,10 @@ function fetchQueryOptions< dataShape, }, }, - refetchOnMount: "always", - staleTime: staleTimeMs, + refetchOnMount: immutableWhen ? true : "always", + staleTime: immutableWhen + ? (query) => + query.state.data !== undefined && immutableWhen(query.state.data) ? Infinity : 0 + : staleTimeMs, }; } diff --git a/packages/app/src/git/commit-file-diff-query.test.ts b/packages/app/src/git/commit-file-diff-query.test.ts new file mode 100644 index 0000000000..173592639d --- /dev/null +++ b/packages/app/src/git/commit-file-diff-query.test.ts @@ -0,0 +1,76 @@ +import { QueryClient, QueryObserver } from "@tanstack/react-query"; +import { describe, expect, it } from "vitest"; +import { fetchQueryOptions } from "@/data/query"; +import { commitFileDiffQueryOptions } from "./commit-file-diff-query"; + +const file = { path: "a.ts", isNew: true, isDeleted: false, additions: 0, deletions: 0, hunks: [] }; + +describe("commit file diff fetching", () => { + it("reuses a cached commit body on remount and fetches on explicit refresh", async () => { + const queryClient = new QueryClient(); + let calls = 0; + const options = fetchQueryOptions( + commitFileDiffQueryOptions({ + serverId: "host", + cwd: "/repo", + sha: "abc", + path: "a.ts", + enabled: true, + client: { + async getCommitFileDiff() { + calls++; + return { file }; + }, + }, + }), + ); + queryClient.setQueryData( + options.queryKey, + { file }, + { updatedAt: Date.now() - 24 * 60 * 60 * 1000 }, + ); + const observer = new QueryObserver(queryClient, options); + const detach = observer.subscribe(() => {}); + try { + await Promise.resolve(); + expect(calls).toBe(0); + expect(observer.getCurrentResult().data?.file).toBe(file); + await observer.refetch(); + expect(calls).toBe(1); + } finally { + detach(); + queryClient.clear(); + } + }); + + it("retries a cached null result when reopened", async () => { + const queryClient = new QueryClient(); + const options = fetchQueryOptions( + commitFileDiffQueryOptions({ + serverId: "host", + cwd: "/repo", + sha: "abc", + path: "a.ts", + enabled: true, + client: { + async getCommitFileDiff() { + return { file }; + }, + }, + }), + ); + queryClient.setQueryData(options.queryKey, { file: null }); + const observer = new QueryObserver(queryClient, options); + const settled = Promise.withResolvers(); + const detach = observer.subscribe((result) => { + if (result.fetchStatus === "idle") settled.resolve(); + }); + try { + await settled.promise; + expect(observer.getCurrentResult().data?.file).toEqual(file); + } finally { + detach(); + queryClient.clear(); + } + }); +}); diff --git a/packages/app/src/git/commit-file-diff-query.ts b/packages/app/src/git/commit-file-diff-query.ts new file mode 100644 index 0000000000..5277551234 --- /dev/null +++ b/packages/app/src/git/commit-file-diff-query.ts @@ -0,0 +1,28 @@ +import type { DaemonClient } from "@getpaseo/client/internal/daemon-client"; +import { checkoutCommitFileDiffQueryKey, COMMIT_FILE_DIFF_STALE_TIME } from "./query-keys"; +import { shareCommitFileDiff } from "./diff-sharing"; + +export function commitFileDiffQueryOptions(input: { + serverId: string; + cwd: string; + sha: string; + path: string; + client: Pick | null; + enabled: boolean; +}) { + return { + queryKey: checkoutCommitFileDiffQueryKey(input.serverId, input.cwd, input.sha, input.path), + queryFn: () => { + if (!input.client) throw new Error("Host disconnected"); + return input.client.getCommitFileDiff(input.cwd, input.sha, input.path); + }, + enabled: input.enabled, + gcTime: COMMIT_FILE_DIFF_STALE_TIME, + // Only successful file bodies are immutable. Null/error results can recover. + immutableWhen: (data: Awaited>) => + data.file !== null, + dataShape: "value" as const, + refetchOnWindowFocus: false, + structuralSharing: shareCommitFileDiff, + }; +} diff --git a/packages/app/src/git/diff-document/horizontal-scroll.native.tsx b/packages/app/src/git/diff-document/horizontal-scroll.native.tsx index 993b2048bb..0aa42e01c6 100644 --- a/packages/app/src/git/diff-document/horizontal-scroll.native.tsx +++ b/packages/app/src/git/diff-document/horizontal-scroll.native.tsx @@ -26,9 +26,10 @@ export function HorizontalScroll({ const scrollId = useId(); const [isAtLeftEdge, setIsAtLeftEdge] = useState(true); const edgeRef = useRef(true); + const path = file.path; const handler = useAnimatedScrollHandler({ onScroll(event) { - offsets.value = { ...offsets.value, [file.path]: event.contentOffset.x }; + offsets.value = { ...offsets.value, [path]: event.contentOffset.x }; }, }); useEffect(() => { diff --git a/packages/app/src/git/diff-document/model.test.ts b/packages/app/src/git/diff-document/model.test.ts index 77d6ae391d..1aa78ab3c6 100644 --- a/packages/app/src/git/diff-document/model.test.ts +++ b/packages/app/src/git/diff-document/model.test.ts @@ -71,6 +71,55 @@ function input(overrides: Partial = {}): BuildDiffD } describe("diff document model", () => { + it("materializes only visible rows of a large file while preserving full horizontal extent", () => { + const source = file("large.ts"); + source.hunks[0]!.lines = Array.from({ length: 1000 }, (_, index) => ({ + type: "add", + content: index === 900 ? "a very wide offscreen line ".repeat(20) : `line ${index}`, + tokens: [{ text: `line ${index}`, style: null }], + })); + let advancedCharacters = 0; + const measureText: TextMeasurer = { + measure: (text) => text.length * 10, + measureAdvances(graphemes) { + advancedCharacters += graphemes.join("").length; + return graphemes.map((_, index) => (index + 1) * 10); + }, + }; + const base = input({ files: [source], wrapLines: false, viewportWidth: 300, measureText }); + const eager = buildDiffDocumentModel(base); + advancedCharacters = 0; + const first = buildDiffDocumentModel({ + ...base, + materializationWindow: { top: FILE_HEADER_HEIGHT, height: 54 }, + }); + expect(measuredFragments(first, source.path)).toBe(3); + expect(advancedCharacters).toBeLessThan(100); + expect(first.height).toBe(eager.height); + expect(first.files[0]!.contentWidth).toBe(eager.files[0]!.contentWidth); + const priorCells = firstLineCells(first, source.path); + advancedCharacters = 0; + const next = buildDiffDocumentModel({ + ...base, + reuseFrom: [first], + materializationWindow: { top: FILE_HEADER_HEIGHT + 900 * 18, height: 54 }, + }); + expect(firstLineCells(next, source.path)).toBe(priorCells); + expect(next.rows[900]).toMatchObject({ + kind: "line", + cells: [{ fragments: [{ text: source.hunks[0]!.lines[900]!.content }] }], + }); + expect(advancedCharacters).toBeLessThan(600); + expect(next.height).toBe(eager.height); + advancedCharacters = 0; + buildDiffDocumentModel({ + ...base, + reuseFrom: [next], + materializationWindow: { top: FILE_HEADER_HEIGHT, height: 54 }, + }); + expect(advancedCharacters).toBe(0); + }); + it("does not imperatively scroll for a no-op relayout", () => { expect(shouldApplyRelayoutScroll(320, 320)).toBe(false); expect(shouldApplyRelayoutScroll(320, 320.4)).toBe(false); diff --git a/packages/app/src/git/diff-document/model.ts b/packages/app/src/git/diff-document/model.ts index 6267519796..a8b2baed90 100644 --- a/packages/app/src/git/diff-document/model.ts +++ b/packages/app/src/git/diff-document/model.ts @@ -157,21 +157,26 @@ function appendReusableFileRows(candidate: { !candidate.input.wrapLines && candidate.model.reviewGeometryKey === candidate.reviewGeometryKey && candidate.file.top === candidate.fileTop && + candidate.file.fileIndex === candidate.fileIndex && candidate.file.rowStart === candidate.rows.length; - if (retainsRowGeometry) { - candidate.rows.push(...reusedRows); - return { - bottom: candidate.bodyTop + candidate.file.bodyHeight, - maximumHorizontalOverflow: Math.max( - 0, - candidate.file.contentWidth - candidate.input.viewportWidth, - ), - }; - } - let top = candidate.bodyTop; for (const reusedRow of reusedRows) { - const nextRow = repositionReusedRow(reusedRow, candidate, top); + let nextRow = retainsRowGeometry ? reusedRow : repositionReusedRow(reusedRow, candidate, top); + if ( + nextRow.kind === "line" && + intersectsMaterializationWindow(candidate.input, top, top + nextRow.height) && + nextRow.cells.some((cell) => cell && cell.fragments.length === 0) + ) { + nextRow = { + ...nextRow, + cells: materializeCells( + nextRow.cells, + candidate.file.file, + candidate.file.gutterWidth, + candidate.input, + ), + }; + } candidate.rows.push(nextRow); top += nextRow.height; } @@ -232,71 +237,45 @@ function appendNewFileRows(candidate: { }; } - const deferredLines = - candidate.input.materializationWindow && !candidate.input.wrapLines - ? geometryLines(lineSources(candidate.file, candidate.input.layout, false), candidate.input) - : null; - const deferredBodyHeight = deferredLines?.reduce((height, line) => height + line.height, 0) ?? 0; - const shouldMaterialize = - deferredLines === null || - intersectsMaterializationWindow( - candidate.input, - candidate.fileTop, - candidate.bodyTop + deferredBodyHeight + DIFF_BODY_BORDER_HEIGHT, - ); - return shouldMaterialize - ? appendMeasuredFileRows(candidate) - : appendGeometryFileRows(candidate, deferredLines); -} - -function appendGeometryFileRows( - candidate: Parameters[0], - lines: readonly GeometryLine[], -): FileRowsResult { - let top = candidate.bodyTop; - for (const line of lines) { - candidate.rows.push({ - kind: "line", - index: candidate.rows.length, - fileIndex: candidate.fileIndex, - path: candidate.file.path, - top, - height: line.height, - cells: line.cells, - reviewHeight: line.reviewHeight, - }); - top += line.height; - } - return { bottom: top + DIFF_BODY_BORDER_HEIGHT, maximumHorizontalOverflow: 0 }; -} - -function appendMeasuredFileRows( - candidate: Parameters[0], -): FileRowsResult { + const lines = geometryLines( + lineSources(candidate.file, candidate.input.layout, false), + candidate.input, + ); + const fileBottom = + candidate.bodyTop + + lines.reduce((height, line) => height + line.height, 0) + + DIFF_BODY_BORDER_HEIGHT; + const measureWidth = + !candidate.input.wrapLines && + intersectsMaterializationWindow(candidate.input, candidate.fileTop, fileBottom); let top = candidate.bodyTop; let maximumHorizontalOverflow = 0; - for (const sourceCells of lineSources(candidate.file, candidate.input.layout, true)) { - const columnCount = sourceCells.length; - const columnWidth = candidate.input.viewportWidth / columnCount; - const availableWidth = Math.max( - candidate.input.measureText.measure("M"), - columnWidth - candidate.gutterWidth - CODE_HORIZONTAL_PADDING, - ); - const cells = sourceCells.map((source) => { - if (!source) return null; - const cell = measureCell({ source, availableWidth, input: candidate.input }); - maximumHorizontalOverflow = Math.max( - maximumHorizontalOverflow, - candidate.gutterWidth + CODE_HORIZONTAL_PADDING + measureCellWidth(cell) - columnWidth, - ); - return cell; - }) as DiffLineRow["cells"]; + for (const line of lines) { + const shouldMaterialize = + candidate.input.wrapLines || + intersectsMaterializationWindow(candidate.input, top, top + line.height); + const cells = shouldMaterialize + ? materializeCells(line.cells, candidate.file, candidate.gutterWidth, candidate.input) + : line.cells; + const columnWidth = candidate.input.viewportWidth / cells.length; + if (measureWidth) { + for (const cell of cells) { + if (!cell) continue; + const width = + cell.fragments.length > 0 + ? measureCellWidth(cell) + : candidate.input.measureText.measure(displayLineText(cell.content)); + maximumHorizontalOverflow = Math.max( + maximumHorizontalOverflow, + candidate.gutterWidth + CODE_HORIZONTAL_PADDING + width - columnWidth, + ); + } + } const textHeight = Math.max( candidate.input.typography.lineHeight, ...cells.map((cell) => (cell?.fragments.length ?? 1) * candidate.input.typography.lineHeight), ); - const reviewHeight = reviewHeightForCells(cells, candidate.input); - const height = textHeight + reviewHeight; + const height = textHeight + line.reviewHeight; candidate.rows.push({ kind: "line", index: candidate.rows.length, @@ -305,13 +284,46 @@ function appendMeasuredFileRows( top, height, cells, - reviewHeight, + reviewHeight: line.reviewHeight, }); top += height; } return { bottom: top + DIFF_BODY_BORDER_HEIGHT, maximumHorizontalOverflow }; } +function materializeCells( + cells: DiffLineRow["cells"], + file: BuildDiffDocumentModelInput["files"][number], + gutterWidth: number, + input: BuildDiffDocumentModelInput, +): DiffLineRow["cells"] { + const availableWidth = Math.max( + input.measureText.measure("M"), + input.viewportWidth / cells.length - gutterWidth - CODE_HORIZONTAL_PADDING, + ); + return cells.map((cell) => { + if (!cell || cell.fragments.length > 0) return cell; + const sourceLine = + file.hunks[cell.sourceIdentity.hunkIndex]!.lines[cell.sourceIdentity.lineIndex]!; + return measureCell({ + source: { + ...cell, + tokenText: cell.type === "header" ? [] : compactHighlightTokens(sourceLine.tokens ?? []), + }, + availableWidth, + input, + }); + }) as DiffLineRow["cells"]; +} + +function displayLineText(text: string): string { + return text.includes("\t") + ? segmentGraphemes(text) + .map((segment) => segment.text) + .join("") + : text; +} + export function reviewGeometryKey( reviewActions: BuildDiffDocumentModelInput["reviewActions"], ): string { @@ -334,7 +346,11 @@ export function retainReusableModels( previous: readonly DiffDocumentModel[] | undefined, next: DiffDocumentModel, ): DiffDocumentModel[] { - const fullest = [...(previous ?? []), next].reduce((best, candidate) => + const sources = new Set(next.files.map((file) => file.file)); + const compatible = previous?.filter((model) => + model.files.every((file) => sources.has(file.file)), + ); + const fullest = [...(compatible ?? []), next].reduce((best, candidate) => measuredFileCount(candidate) > measuredFileCount(best) ? candidate : best, ); return fullest === next ? [next] : [fullest, next]; @@ -437,16 +453,17 @@ function intersectsMaterializationWindow( function hasMeasuredFileRows(model: DiffDocumentModel, file: DiffFileSection): boolean { if (file.isCollapsed) return false; - return model.rows.slice(file.rowStart, file.rowEnd).every((row) => { + return model.rows.slice(file.rowStart, file.rowEnd).some((row) => { if (row.kind !== "line") return true; - return row.cells.every((cell) => !cell || cell.fragments.length > 0); + return row.cells.some((cell) => cell && cell.fragments.length > 0); }); } function indexReusableModels( models: readonly DiffDocumentModel[] | undefined, ): ReusableModelIndex[] { - return (models ?? []).map((model) => { + // The latest model includes text prepared by later scroll windows. + return (models ?? []).toReversed().map((model) => { const filesBySource = new Map(); const measuredFiles = new Set(); for (const file of model.files) { diff --git a/packages/app/src/git/diff-document/surface.native.tsx b/packages/app/src/git/diff-document/surface.native.tsx index 49c2697169..a6813685e9 100644 --- a/packages/app/src/git/diff-document/surface.native.tsx +++ b/packages/app/src/git/diff-document/surface.native.tsx @@ -110,7 +110,6 @@ export function DiffSurface(props: DiffSurfaceProps) { const reviewActions = props.mode.kind === "working" ? props.mode.reviewActions : undefined; const model = useMemo(() => { const dependencies = [ - props.files, props.displayPreferences.layout, props.displayPreferences.wrapLines, viewport.width, @@ -172,8 +171,13 @@ export function DiffSurface(props: DiffSurfaceProps) { [family, props.palette, typography.lineHeight, typography.size], ); const textLayout = useMemo( - () => prepareNativeTextLayout(textLayoutStore, model), - [model, textLayoutStore], + () => + prepareNativeTextLayout( + textLayoutStore, + model, + diffMaterializationWindow(fileWindowTop, viewport.height), + ), + [model, textLayoutStore, fileWindowTop, viewport.height], ); useEffect(() => () => disposeNativeTextLayout(textLayoutStore), [textLayoutStore]); const headerTextLayout = useMemo( @@ -386,11 +390,12 @@ function NativeCanvasFileHeader({ () => recordNativeHeaderPicture({ file, viewportWidth, textLayout, paints }), [file, paints, textLayout, viewportWidth], ); + const { top, bottom, headerHeight } = file; const stickyStyle = useAnimatedStyle(() => { - const pinOffset = Math.max(0, scrollTop.value - file.top); - const maximumPinOffset = Math.max(0, file.bottom - file.headerHeight - file.top); + const pinOffset = Math.max(0, scrollTop.value - top); + const maximumPinOffset = Math.max(0, bottom - headerHeight - top); return { transform: [{ translateY: Math.min(pinOffset, maximumPinOffset) }] }; - }, [file.bottom, file.headerHeight, file.top, scrollTop]); + }, [bottom, headerHeight, top, scrollTop]); const style = useMemo( () => ({ position: "absolute", diff --git a/packages/app/src/git/diff-document/text-measurement.test.ts b/packages/app/src/git/diff-document/text-measurement.test.ts index 476ef3e45d..144676e550 100644 --- a/packages/app/src/git/diff-document/text-measurement.test.ts +++ b/packages/app/src/git/diff-document/text-measurement.test.ts @@ -55,6 +55,7 @@ describe("cached ASCII text metrics", () => { expect(metrics.measureAdvances(["a", "b", "a"])).toEqual([10, 20, 30]); expect(metrics.measureAdvances(["b", "a"])).toEqual([10, 20]); + expect(metrics.measure("abba")).toBe(40); expect(measured).toEqual(["a", "b"]); }); diff --git a/packages/app/src/git/diff-document/text-measurement.ts b/packages/app/src/git/diff-document/text-measurement.ts index 92b245ad73..a141d170a7 100644 --- a/packages/app/src/git/diff-document/text-measurement.ts +++ b/packages/app/src/git/diff-document/text-measurement.ts @@ -6,6 +6,8 @@ export interface PrimaryTextFace { } export interface CachedAsciiTextMetrics { + /** Whole-line width after checking glyph coverage and excluding shaping. */ + measure(text: string): number; hasEveryGlyph(text: string): boolean; measureAdvances(graphemes: readonly string[]): number[]; } @@ -97,24 +99,39 @@ export function advancesFor(measurer: TextMeasurer): (graphemes: readonly string export function createAdditiveAdvances( measure: (text: string) => number, ): (graphemes: readonly string[]) => number[] { + return additiveAdvances(cachedWidths(measure)); +} + +function cachedWidths(measure: (text: string) => number): (text: string) => number { const widths = new Map(); + return (text) => { + let width = widths.get(text); + if (width === undefined) { + width = measure(text); + widths.set(text, width); + } + return width; + }; +} + +function additiveAdvances( + measure: (text: string) => number, +): (graphemes: readonly string[]) => number[] { return (graphemes) => { let advance = 0; - return graphemes.map((grapheme) => { - let width = widths.get(grapheme); - if (width === undefined) { - width = measure(grapheme); - widths.set(grapheme, width); - } - advance += width; - return advance; - }); + return graphemes.map((grapheme) => (advance += measure(grapheme))); }; } export function createCachedAsciiTextMetrics(primary: PrimaryTextFace): CachedAsciiTextMetrics { const glyphCoverage = new Map(); + const measure = cachedWidths((text) => primary.measure(text)); return { + measure(text) { + let width = 0; + for (const character of text) width += measure(character); + return width; + }, hasEveryGlyph(text) { for (const character of text) { let hasGlyph = glyphCoverage.get(character); @@ -126,7 +143,7 @@ export function createCachedAsciiTextMetrics(primary: PrimaryTextFace): CachedAs } return true; }, - measureAdvances: createAdditiveAdvances((text) => primary.measure(text)), + measureAdvances: additiveAdvances(measure), }; } diff --git a/packages/app/src/git/diff-document/text.native.ts b/packages/app/src/git/diff-document/text.native.ts index 2f0252b384..0c6f16bb5d 100644 --- a/packages/app/src/git/diff-document/text.native.ts +++ b/packages/app/src/git/diff-document/text.native.ts @@ -5,12 +5,11 @@ import { type SkFont, type SkParagraph, } from "@shopify/react-native-skia"; -import { fragmentTextForRange } from "./model"; +import { fragmentTextForRange, visibleRowRange } from "./model"; import { codeTextColor } from "./palette"; import { createCachedAsciiTextMetrics, createChunkedAdvanceMeasurer, - createFallbackAwareTextMeasurer, requiresShaping, type CachedAsciiTextMetrics, } from "./text-measurement"; @@ -86,7 +85,8 @@ export function shapeNativeHeaderText(input: { export interface NativeTextLayoutStore { font: SkFont; asciiMetrics: CachedAsciiTextMetrics; - paragraphsByCell: WeakMap>; + paragraphsByCell: Map>; + textStyles: Map>; ownedParagraphs: Set; families: string[]; fontSize: number; @@ -97,6 +97,8 @@ export interface NativeTextLayoutStore { export function disposeNativeTextLayout(layout: NativeTextLayoutStore): void { for (const paragraph of layout.ownedParagraphs) paragraph.dispose(); layout.ownedParagraphs.clear(); + layout.paragraphsByCell.clear(); + layout.textStyles.clear(); layout.font.dispose(); } @@ -114,7 +116,8 @@ export function createNativeTextLayoutStore(input: { glyphIds: (text) => font.getGlyphIDs(text), measure: (text) => font.getTextWidth(text), }), - paragraphsByCell: new WeakMap(), + paragraphsByCell: new Map(), + textStyles: new Map(), ownedParagraphs: new Set(), families, fontSize: input.fontSize, @@ -126,30 +129,64 @@ export function createNativeTextLayoutStore(input: { export function prepareNativeTextLayout( store: NativeTextLayoutStore, model: DiffDocumentModel, + window?: { top: number; height: number }, ): NativeTextLayout { - const paragraphs = model.rows.map((row) => { - if (row.kind !== "line") return []; - return row.cells.map((cell) => { + const range = window + ? visibleRowRange(model.rows, window.top, window.height) + : { start: 0, end: model.rows.length }; + const retainedCells = new Set(); + const retainedParagraphs = new Set(); + const paragraphs: NativeTextLayout["paragraphs"] = []; + paragraphs.length = model.rows.length; + for (let index = range.start; index < range.end; index++) { + const row = model.rows[index]!; + if (row.kind !== "line") continue; + paragraphs[index] = row.cells.map((cell) => { if (!cell) return []; + retainedCells.add(cell); const cached = store.paragraphsByCell.get(cell); - if (cached) return cached; - const cellParagraphs = cell.fragments.map((fragment) => { - if (!requiresRetainedParagraph(fragment.text, store.asciiMetrics)) return null; - const paragraph = createFragmentParagraph({ - cell, - fragment, - families: store.families, - fontSize: store.fontSize, - lineHeight: store.lineHeight, - palette: store.palette, - }); + const start = window ? Math.max(0, Math.floor((window.top - row.top) / model.lineHeight)) : 0; + const end = window + ? Math.min( + cell.fragments.length, + Math.ceil((window.top + window.height - row.top) / model.lineHeight), + ) + : cell.fragments.length; + const cellParagraphs: Array = []; + cellParagraphs.length = cell.fragments.length; + for (let fragmentIndex = start; fragmentIndex < end; fragmentIndex++) { + const fragment = cell.fragments[fragmentIndex]!; + if (!requiresRetainedParagraph(fragment.text, store.asciiMetrics)) { + cellParagraphs[fragmentIndex] = null; + continue; + } + const paragraph = + cached?.[fragmentIndex] ?? + createFragmentParagraph({ + cell, + fragment, + families: store.families, + fontSize: store.fontSize, + lineHeight: store.lineHeight, + palette: store.palette, + textStyles: store.textStyles, + }); store.ownedParagraphs.add(paragraph); - return paragraph; - }); + retainedParagraphs.add(paragraph); + cellParagraphs[fragmentIndex] = paragraph; + } store.paragraphsByCell.set(cell, cellParagraphs); return cellParagraphs; }); - }); + } + for (const paragraph of store.ownedParagraphs) { + if (retainedParagraphs.has(paragraph)) continue; + paragraph.dispose(); + store.ownedParagraphs.delete(paragraph); + } + for (const cell of store.paragraphsByCell.keys()) { + if (!retainedCells.has(cell)) store.paragraphsByCell.delete(cell); + } return { font: store.font, paragraphs }; } @@ -164,23 +201,25 @@ export function createNativeTextMeasurer(input: { measure: (text: string) => font.getTextWidth(text), }; const asciiMetrics = createCachedAsciiTextMetrics(primary); - const fallbackAware = createFallbackAwareTextMeasurer({ - primary, - measureWithSystemFallback(text) { - const paragraph = createParagraph({ - text, - families, - fontSize: input.fontSize, - lineHeight: Math.round(input.fontSize * 1.5), - color: Skia.Color("black"), - }); - const width = paragraph.getLongestLine(); - paragraph.dispose(); - return width; - }, - }); + const black = Skia.Color("black"); + const measureParagraph = (text: string) => { + const paragraph = createParagraph({ + text, + families, + fontSize: input.fontSize, + lineHeight: Math.round(input.fontSize * 1.5), + color: black, + }); + const width = paragraph.getLongestLine(); + paragraph.dispose(); + return width; + }; return { - ...fallbackAware, + measure(text) { + return requiresRetainedParagraph(text, asciiMetrics) + ? measureParagraph(text) + : asciiMetrics.measure(text); + }, measureAdvances: createChunkedAdvanceMeasurer({ requiresShaping: (text) => requiresRetainedParagraph(text, asciiMetrics), measureAdditive: (graphemes) => asciiMetrics.measureAdvances(graphemes), @@ -190,7 +229,7 @@ export function createNativeTextMeasurer(input: { families, fontSize: input.fontSize, lineHeight: Math.round(input.fontSize * 1.5), - color: Skia.Color("black"), + color: black, }); let end = 0; const advances = graphemes.map((grapheme) => { @@ -219,23 +258,21 @@ function createFragmentParagraph(input: { fontSize: number; lineHeight: number; palette: DiffPalette; + textStyles: NativeTextLayoutStore["textStyles"]; }): SkParagraph { const builder = Skia.ParagraphBuilder.Make( paragraphStyle(input.families, input.fontSize, input.lineHeight), ); if (input.cell.tokens.length === 0 || input.cell.type === "header") { const color = codeTextColor(input.cell, input.palette); - builder - .pushStyle(textStyle(input.families, input.fontSize, color)) - .addText(input.fragment.text) - .pop(); + builder.pushStyle(retainedTextStyle(input, color)).addText(input.fragment.text).pop(); } else { for (const run of input.cell.tokens) { const start = Math.max(input.fragment.start, run.start); const end = Math.min(input.fragment.end, run.end); if (end <= start) continue; builder - .pushStyle(textStyle(input.families, input.fontSize, run.color)) + .pushStyle(retainedTextStyle(input, run.color)) .addText(fragmentTextForRange(input.fragment, start, end)) .pop(); } @@ -281,6 +318,18 @@ function paragraphStyle(families: string[], fontSize: number, lineHeight: number }; } +function retainedTextStyle( + input: { families: string[]; fontSize: number; textStyles: NativeTextLayoutStore["textStyles"] }, + color: string, +) { + let style = input.textStyles.get(color); + if (!style) { + style = textStyle(input.families, input.fontSize, color); + input.textStyles.set(color, style); + } + return style; +} + function textStyle(families: string[], fontSize: number, color: string) { return { fontFamilies: families, fontSize, color: Skia.Color(color) }; } diff --git a/packages/app/src/git/diff-document/workspace-cache.test.ts b/packages/app/src/git/diff-document/workspace-cache.test.ts index 4a27cc3d44..ddf9972a46 100644 --- a/packages/app/src/git/diff-document/workspace-cache.test.ts +++ b/packages/app/src/git/diff-document/workspace-cache.test.ts @@ -78,6 +78,40 @@ function modelInput( } describe("diff document workspace cache", () => { + it("keeps text prepared in each visited window of the same file", () => { + const cache = createDiffDocumentWorkspaceCache(); + const source = diffFile(); + source.hunks[0]!.lines = Array.from({ length: 100 }, (_, index) => ({ + type: "add", + content: `const value${index} = ${index};`, + })); + const { measureText, stats } = countingMeasurer(); + const input = modelInput([source], measureText); + for (const top of [30, 300, 600]) { + cache.buildModel({ ...input, materializationWindow: { top, height: 54 } }); + } + stats.calls = 0; + cache.buildModel({ ...input, materializationWindow: { top: 300, height: 54 } }); + expect(stats.calls).toBe(0); + }); + + it("retains text cells for unchanged files when another file changes", () => { + const cache = createDiffDocumentWorkspaceCache(); + const { measureText } = countingMeasurer(); + const files = [diffFile(), { ...diffFile(), path: "b.ts" }]; + const first = cache.buildModel(modelInput(files, measureText)); + const replacement = structuredClone(files[0]!); + replacement.hunks[0]!.lines[0]!.content = "changed text"; + const next = cache.buildModel(modelInput([replacement, files[1]!], measureText)); + const oldRow = first.rows[first.files[1]!.rowStart]!; + const nextRow = next.rows[next.files[1]!.rowStart]!; + expect(oldRow.kind).toBe("line"); + expect(nextRow.kind).toBe("line"); + if (oldRow.kind !== "line" || nextRow.kind !== "line") throw new Error("Missing file text"); + expect(nextRow.cells).toBe(oldRow.cells); + expect(next.files[0]!.file).toBe(replacement); + }); + it("reuses measured rows until a model-building input changes", () => { const cache = createDiffDocumentWorkspaceCache(); const files = [diffFile()]; @@ -115,7 +149,7 @@ describe("diff document workspace cache", () => { stats.calls = 0; cache.buildModel({ ...input, files: [...files] }); - expect(stats.calls).toBeGreaterThan(0); + expect(stats.calls).toBe(0); }); it("bounds retained geometry variants for one diff payload", () => { @@ -152,7 +186,7 @@ describe("diff document workspace cache", () => { const files = [diffFile(), { ...diffFile(), path: "src/b.ts" }]; const { measureText, stats } = countingMeasurer(); const first = cache.buildModel( - modelInput(files, measureText, { materializationWindow: { top: 0, height: 20 } }), + modelInput(files, measureText, { materializationWindow: { top: 30, height: 20 } }), ); const firstCalls = stats.calls; const secondFile = first.files[1]!; diff --git a/packages/app/src/git/diff-document/workspace-cache.tsx b/packages/app/src/git/diff-document/workspace-cache.tsx index 270c7392ca..bd52000af3 100644 --- a/packages/app/src/git/diff-document/workspace-cache.tsx +++ b/packages/app/src/git/diff-document/workspace-cache.tsx @@ -13,6 +13,7 @@ const MAX_MODEL_VARIANTS = 4; const MAX_TYPOGRAPHY_RESOURCES = 4; interface ModelVariant { + measureText: TextMeasurer; key: string; exactKey: string; model: DiffDocumentModel; @@ -38,23 +39,29 @@ export interface DiffDocumentWorkspaceCache { } export function createDiffDocumentWorkspaceCache(): DiffDocumentWorkspaceCache { - const variantsByFiles = new WeakMap(); + let variants: ModelVariant[] = []; const typographyResources = new Map(); return { buildModel(input) { const key = modelVariantKey(input); const exactKey = exactModelKey(input); - const variants = variantsByFiles.get(input.files) ?? []; - const variantIndex = variants.findIndex((candidate) => candidate.key === key); + const variantIndex = variants.findIndex( + (candidate) => candidate.key === key && candidate.measureText === input.measureText, + ); const variant = variantIndex === -1 ? undefined : variants[variantIndex]; - if (variant?.exactKey === exactKey) { + if ( + variant?.exactKey === exactKey && + variant.model.files.length === input.files.length && + variant.model.files.every((file, index) => file.file === input.files[index]) + ) { variants.splice(variantIndex, 1); variants.unshift(variant); return variant.model; } const model = buildDiffDocumentModel({ ...input, reuseFrom: variant?.models }); const nextVariant = { + measureText: input.measureText, key, exactKey, model, @@ -62,7 +69,7 @@ export function createDiffDocumentWorkspaceCache(): DiffDocumentWorkspaceCache { }; if (variantIndex !== -1) variants.splice(variantIndex, 1); variants.unshift(nextVariant); - variantsByFiles.set(input.files, variants.slice(0, MAX_MODEL_VARIANTS)); + variants = variants.slice(0, MAX_MODEL_VARIANTS); return model; }, typography(input) { diff --git a/packages/app/src/git/diff-sharing.test.ts b/packages/app/src/git/diff-sharing.test.ts new file mode 100644 index 0000000000..210c0f1a4b --- /dev/null +++ b/packages/app/src/git/diff-sharing.test.ts @@ -0,0 +1,148 @@ +import type { ParsedDiffFile } from "@getpaseo/protocol/messages"; +import { describe, expect, it } from "vitest"; +import { retainDiffFiles, shareCheckoutDiff, shareCommitFileDiff } from "./diff-sharing"; + +const file: ParsedDiffFile = { + path: "a.ts", + oldPath: "old-a.ts", + status: "ok", + isNew: false, + isDeleted: false, + additions: 1, + deletions: 1, + hunks: [ + { + oldStart: 1, + oldCount: 1, + newStart: 1, + newCount: 1, + lines: [ + { + type: "add", + content: "const a", + tokens: [ + { text: "const", style: "keyword" }, + { text: " a", style: null }, + ], + }, + ], + }, + ], +}; + +// Exhaustive keys make a protocol-field addition require a sharing-policy test. +const fileChanges = { + path: (value) => { + value.path = "b.ts"; + }, + oldPath: (value) => { + delete value.oldPath; + }, + status: (value) => { + value.status = "binary"; + }, + isNew: (value) => { + value.isNew = true; + }, + isDeleted: (value) => { + value.isDeleted = true; + }, + additions: (value) => { + value.additions = 2; + }, + deletions: (value) => { + value.deletions = 2; + }, + hunks: (value) => { + value.hunks = []; + }, +} satisfies Record void>; +type Hunk = ParsedDiffFile["hunks"][number]; +const hunkChanges = { + oldStart: (value) => { + value.oldStart = 2; + }, + oldCount: (value) => { + value.oldCount = 2; + }, + newStart: (value) => { + value.newStart = 2; + }, + newCount: (value) => { + value.newCount = 2; + }, + lines: (value) => { + value.lines = []; + }, +} satisfies Record void>; +type Line = Hunk["lines"][number]; +const lineChanges = { + type: (value) => { + value.type = "remove"; + }, + content: (value) => { + value.content = "const b"; + }, + tokens: (value) => { + delete value.tokens; + }, +} satisfies Record void>; +type Token = NonNullable[number]; +const tokenChanges = { + text: (value) => { + value.text = "class"; + }, + style: (value) => { + value.style = null; + }, +} satisfies Record void>; + +describe("diff sharing", () => { + it.each(Object.entries(fileChanges))("replaces a file when %s changes", (_, change) => { + const next = structuredClone(file); + change(next); + expect(retainDiffFiles([file], [next])[0]).toBe(next); + }); + it.each(Object.entries(hunkChanges))("replaces a file when hunk %s changes", (_, change) => { + const next = structuredClone(file); + change(next.hunks[0]!); + expect(retainDiffFiles([file], [next])[0]).toBe(next); + }); + it.each(Object.entries(lineChanges))("replaces a file when line %s changes", (_, change) => { + const next = structuredClone(file); + change(next.hunks[0]!.lines[0]!); + expect(retainDiffFiles([file], [next])[0]).toBe(next); + }); + it.each(Object.entries(tokenChanges))("replaces a file when token %s changes", (_, change) => { + const next = structuredClone(file); + change(next.hunks[0]!.lines[0]!.tokens![0]!); + expect(retainDiffFiles([file], [next])[0]).toBe(next); + }); + it("retains moved files and removes missing files", () => { + const second = { ...file, path: "b.ts" }; + const reordered = retainDiffFiles([file, second], structuredClone([second, file])); + expect(reordered[0]).toBe(second); + expect(reordered[1]).toBe(file); + expect(retainDiffFiles(reordered, [structuredClone(file)])[0]).toBe(file); + }); + it("updates response metadata without invalidating unchanged text", () => { + const previous = { cwd: "/repo", files: [file], error: null, requestId: "a" }; + expect(shareCheckoutDiff(previous, structuredClone(previous))).toBe(previous); + const next = shareCheckoutDiff(previous, { ...structuredClone(previous), requestId: "b" }); + expect(next.requestId).toBe("b"); + expect(next.files).toBe(previous.files); + const rejected = { + ...previous, + files: [], + error: { code: "UNKNOWN", message: "Too large" }, + diffTooLarge: true, + }; + expect(shareCheckoutDiff(next, rejected)).toEqual(rejected); + }); + it("preserves cached commit bodies and publishes null transitions", () => { + const previous = { file }; + expect(shareCommitFileDiff(previous, structuredClone(previous))).toBe(previous); + expect(shareCommitFileDiff(previous, { file: null })).toEqual({ file: null }); + expect(shareCommitFileDiff({ file: null }, previous)).toBe(previous); + }); +}); diff --git a/packages/app/src/git/diff-sharing.ts b/packages/app/src/git/diff-sharing.ts new file mode 100644 index 0000000000..6bdac10ddf --- /dev/null +++ b/packages/app/src/git/diff-sharing.ts @@ -0,0 +1,98 @@ +import { replaceEqualDeep } from "@tanstack/react-query"; +import type { ParsedDiffFile, SubscribeCheckoutDiffResponse } from "@getpaseo/protocol/messages"; + +type DiffPayload = Omit; + +/** File identity owns text-model reuse. Never reconstruct the hunk/token graph. */ +export function retainDiffFiles( + previous: ParsedDiffFile[], + incoming: ParsedDiffFile[], +): ParsedDiffFile[] { + if (previous === incoming) return previous; + const byPath = new Map(previous.map((file) => [file.path, file])); + let unchanged = previous.length === incoming.length; + const files = incoming.map((file, index) => { + const old = byPath.get(file.path); + const retained = old && equalFile(old, file) ? old : file; + unchanged &&= retained === previous[index]; + return retained; + }); + return unchanged ? previous : files; +} + +export function shareCheckoutDiff(previous: unknown, incoming: unknown): DiffPayload { + const next = incoming as DiffPayload; + const old = previous as DiffPayload | undefined; + if (!old || old === next) return next; + const files = retainDiffFiles(old.files, next.files); + const error = replaceEqualDeep(old.error, next.error); + return files === old.files && + error === old.error && + old.cwd === next.cwd && + old.requestId === next.requestId && + old.diffTooLarge === next.diffTooLarge + ? old + : { ...next, files, error }; +} + +export function shareCommitFileDiff( + previous: unknown, + incoming: unknown, +): { file: ParsedDiffFile | null } { + const next = incoming as { file: ParsedDiffFile | null }; + const old = previous as typeof next | undefined; + return old && + (old.file === next.file || (old.file && next.file && equalFile(old.file, next.file))) + ? old + : next; +} + +function equalFile(left: ParsedDiffFile, right: ParsedDiffFile): boolean { + if (left === right) return true; + if ( + left.path !== right.path || + left.oldPath !== right.oldPath || + left.status !== right.status || + left.isNew !== right.isNew || + left.isDeleted !== right.isDeleted || + left.additions !== right.additions || + left.deletions !== right.deletions || + left.hunks.length !== right.hunks.length + ) + return false; + for (let h = 0; h < left.hunks.length; h++) { + const a = left.hunks[h]!; + const b = right.hunks[h]!; + if (a === b) continue; + if ( + a.oldStart !== b.oldStart || + a.oldCount !== b.oldCount || + a.newStart !== b.newStart || + a.newCount !== b.newCount || + a.lines.length !== b.lines.length + ) + return false; + if (!equalLines(a.lines, b.lines)) return false; + } + return true; +} + +function equalLines( + left: ParsedDiffFile["hunks"][number]["lines"], + right: ParsedDiffFile["hunks"][number]["lines"], +): boolean { + for (let l = 0; l < left.length; l++) { + const x = left[l]!; + const y = right[l]!; + if (x === y) continue; + if (x.type !== y.type || x.content !== y.content) return false; + if (x.tokens === y.tokens) continue; + if (!x.tokens || !y.tokens || x.tokens.length !== y.tokens.length) return false; + for (let t = 0; t < x.tokens.length; t++) { + const p = x.tokens[t]!; + const q = y.tokens[t]!; + if (p.text !== q.text || p.style !== q.style) return false; + } + } + return true; +} diff --git a/packages/app/src/git/use-diff-files.ts b/packages/app/src/git/use-diff-files.ts index 14a17edcb1..9623a31171 100644 --- a/packages/app/src/git/use-diff-files.ts +++ b/packages/app/src/git/use-diff-files.ts @@ -2,7 +2,7 @@ import { useMemo } from "react"; import type { CheckoutCommitFile, ParsedDiffFile } from "@getpaseo/protocol/messages"; import { useRetainedPanelActive } from "@/components/retained-panel"; import { useFetchQueries } from "@/data/query"; -import { checkoutCommitFileDiffQueryKey, COMMIT_FILE_DIFF_STALE_TIME } from "@/git/query-keys"; +import { commitFileDiffQueryOptions } from "./commit-file-diff-query"; import { useCheckoutCommitsQuery } from "@/git/use-commits-query"; import { useHostRuntimeClient, useHostRuntimeIsConnected } from "@/runtime/host-runtime"; @@ -79,18 +79,16 @@ export function useCommitDiffFiles(ctx: CommitDiffFilesContext): CommitDiffFiles Boolean(client) && isConnected; const fileDiffResults = useFetchQueries( - commitFiles.map((file) => ({ - queryKey: checkoutCommitFileDiffQueryKey(serverId, cwd, sha, file.path), - queryFn: async (): Promise<{ file: ParsedDiffFile | null }> => { - if (!client) { - throw new Error("Host disconnected"); - } - return client.getCommitFileDiff(cwd, sha, file.path); - }, - enabled: fileDiffsEnabled, - staleTimeMs: COMMIT_FILE_DIFF_STALE_TIME, - dataShape: "value" as const, - })), + commitFiles.map((file) => + commitFileDiffQueryOptions({ + serverId, + cwd, + sha, + path: file.path, + client, + enabled: fileDiffsEnabled, + }), + ), ); const commitsLoading = commitsQuery.status === "connecting" || commitsQuery.status === "loading"; const commitsError = commitsQuery.status === "error" ? commitsQuery.error : null; diff --git a/packages/app/src/git/use-diff-query.ts b/packages/app/src/git/use-diff-query.ts index 7002bbfc55..35ef268dac 100644 --- a/packages/app/src/git/use-diff-query.ts +++ b/packages/app/src/git/use-diff-query.ts @@ -1,3 +1,4 @@ +import { shareCheckoutDiff } from "./diff-sharing"; import { useMemo } from "react"; import { useRetainedPanelActive } from "@/components/retained-panel"; import { useReplicaQuery } from "@/data/query"; @@ -74,6 +75,7 @@ export function useCheckoutDiffQuery({ const query = useReplicaQuery({ queryKey, + structuralSharing: shareCheckoutDiff, enabled: routeEnabled, pushEvent: "checkout_diff_update", meta: checkoutDiffPushRoute({ diff --git a/packages/app/src/hooks/use-providers-snapshot.ts b/packages/app/src/hooks/use-providers-snapshot.ts index f79bcbc065..dc90c18f06 100644 --- a/packages/app/src/hooks/use-providers-snapshot.ts +++ b/packages/app/src/hooks/use-providers-snapshot.ts @@ -79,6 +79,8 @@ export function useProvidersSnapshot( const snapshotQuery = useReplicaQuery({ queryKey, + // The content-hash cache already owns catalog and model identity. + structuralSharing: false, enabled: Boolean(enabled && supportsSnapshot && serverId && client && isConnected), pushEvent: "providers_snapshot_update", queryFn: async ({ signal }) => { @@ -149,6 +151,7 @@ export function prefetchProvidersSnapshot( void singletonQueryClient.prefetchQuery({ queryKey, staleTime: Infinity, + structuralSharing: false, queryFn: ({ signal }) => fetchProvidersSnapshot({ client, serverId, cwd, queryClient: singletonQueryClient, signal }), });