diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 4231b669aa76b..9267dc26b5dba 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -418,7 +418,8 @@ export default function DiffPanel({ ? `${routeThreadRef.environmentId}:${routeThreadRef.threadId}` : null; - const selectedGitSource = branchDiffPreview.data?.sources.find( + const preview = branchDiffPreview.isSuccess ? branchDiffPreview.data : null; + const selectedGitSource = preview?.sources.find( (source) => source.kind === (selectedGitScope === "unstaged" @@ -470,7 +471,6 @@ export default function DiffPanel({ const refreshPreviewQuery = branchDiffPreview.refresh; const refreshDiffFromUserAction = refreshPreviewQuery; - const preview = branchDiffPreview.data; const gitDiff = selectedGitSource?.diff; const toolCallPreview = @@ -478,7 +478,9 @@ export default function DiffPanel({ const selectedPatch = toolCallSelection ? toolCallPreview?.patch : selectedTurn - ? activeCheckpointDiff.data?.diff + ? activeCheckpointDiff.error === null + ? activeCheckpointDiff.data?.diff + : undefined : gitDiff; const isSelectedPatchTruncated = toolCallPreview?.truncated === true || @@ -598,6 +600,7 @@ export default function DiffPanel({ ...matchingBaseRefChoices.map(valueForBaseRefChoice), ]; const { + family: filePatchFamily, isPending: areFilePatchesPending, fileStates, retry, @@ -606,6 +609,7 @@ export default function DiffPanel({ remainingFileCount, loadNextFiles, } = useReviewFilePatches({ + reviewKey: collapseScopeKey, environmentId: activeThread?.environmentId, cwd: branchDiffPreview.data?.cwd, source: lazySource, @@ -1450,10 +1454,11 @@ export default function DiffPanel({ projectId={activeThread?.projectId} viewerRef={attachCodeView} onScroll={codeViewScroll.onScroll} - codeViewKey={codeViewMountKey} + codeViewKey={`${codeViewMountKey}:${filePatchFamily}`} className="h-full min-h-0 overflow-auto" files={codeViewFiles} pendingFiles={pendingFiles} + isRefreshingDiff={isRefreshingDiff} renderCodeViewFooter={renderLoadingBoundary} sectionId={reviewSectionId} sectionTitle={reviewSectionTitle} diff --git a/apps/web/src/components/diffs/AnnotatableCodeView.browser.test.tsx b/apps/web/src/components/diffs/AnnotatableCodeView.browser.test.tsx index 3ae6c1d524651..e9e1a25ad958c 100644 --- a/apps/web/src/components/diffs/AnnotatableCodeView.browser.test.tsx +++ b/apps/web/src/components/diffs/AnnotatableCodeView.browser.test.tsx @@ -1,68 +1,28 @@ -import type { CodeViewDiffItem, FileDiffMetadata, SelectedLineRange } from "@pierre/diffs"; +import type { FileDiffMetadata, SelectedLineRange } from "@pierre/diffs"; import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles"; -import { StrictMode, type ComponentProps } from "react"; +import { WorkerPoolManager } from "@pierre/diffs/worker"; +import { createRef, StrictMode } from "react"; import { describe, expect, it, vi } from "vite-plus/test"; import { act, render } from "../../../test/render"; import { DraftId, useComposerDraftStore } from "~/composerDraftStore"; -import { buildFileDiffIdentityKey } from "~/lib/diffRendering"; +import { buildFileDiffContentVersion, buildFileDiffIdentityKey } from "~/lib/diffRendering"; -type ViewerProps = Omit>, "options"> & { - options: { - enableLineSelection: boolean; - enableGutterUtility: boolean; - onGutterUtilityClick: ( - range: SelectedLineRange, - context: { item: CodeViewDiffItem }, - ) => void; - }; -}; - -// Replace only the virtualized viewer; comment editing and submission use the real components. -vi.mock("./StyledDiffCodeView", () => ({ - StyledDiffCodeView: (props: ViewerProps) => - props.items - ?.filter((item) => item.type === "diff") - .map((item) => ( -
- - {props.selectedLines?.id === item.id ? ( - Line {props.selectedLines.range.start} - ) : null} - {item.annotations?.map((annotation) => ( -
- {props.renderAnnotation?.(annotation, item.fileDiff, item)} -
- ))} -
- )), -})); - -import { AnnotatableCodeView } from "./AnnotatableCodeView"; -import type { StyledDiffCodeView } from "./StyledDiffCodeView"; +import { AnnotatableCodeView, type AnnotatableCodeViewHandle } from "./AnnotatableCodeView"; const target = DraftId.make("diff-draft-test"); const range: SelectedLineRange = { start: 1, end: 1, side: "additions" }; -let container: HTMLDivElement; function file( name: string, - fileVersion = 1, type: FileDiffMetadata["type"] = "change", prevName?: string, + contents = "new", ) { const fileDiff = parsePatchFiles( - `diff --git a/${name} b/${name}\n--- a/${name}\n+++ b/${name}\n@@ -1 +1 @@\n-old\n+new\n`, - { cacheKeyPrefix: name }, + `diff --git a/${name} b/${name}\n--- a/${name}\n+++ b/${name}\n@@ -1 +1,${contents.split("\n").length} @@\n-old\n+${contents}\n`, + { cacheKeyPrefix: `${name}:${contents}` }, )[0]!.files[0]!; fileDiff.type = type; if (prevName) fileDiff.prevName = prevName; @@ -70,22 +30,31 @@ function file( fileDiff, filePath: name, fileKey: buildFileDiffIdentityKey(fileDiff), - fileVersion, + fileVersion: buildFileDiffContentVersion(fileDiff), collapsed: false, }; } -function view( - files: ReturnType[], - codeViewKey = "initial", - pendingFiles: FileDiffMetadata[] = [], -) { - return ( +function setup() { + const reset = () => useComposerDraftStore.setState(useComposerDraftStore.getInitialState(), true); + reset(); + const container = document.createElement("div"); + Object.assign(container.style, { width: "800px", height: "400px", display: "grid" }); + const viewerRef = createRef(); + const view = ( + files: ReturnType[], + codeViewKey = "initial", + pendingFiles: FileDiffMetadata[] = [], + isRefreshingDiff = false, + ) => ( ); -} - -function click(label: string) { - const button = Array.from(container.querySelectorAll("button")).find( - (node) => node.textContent === label, - ); - if (!button) throw new Error(`Missing button: ${label}`); - return act(() => button.click()); -} - -function editor() { - const textarea = container.querySelector("textarea"); - if (!textarea) throw new Error("Comment editor is not available"); - return textarea; -} - -function typeComment(text: string) { - return act(() => { - Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")!.set!.call( - editor(), - text, + const renderedFile = () => { + const item = viewerRef.current?.getInstance()?.getRenderedItems()[0]; + if (item?.type !== "diff") throw new Error("Diff is not rendered"); + return item; + }; + const select = () => + act(() => { + // Invoke Pierre's own wrappers, which resolve their context from the latest item. + const { instance } = renderedFile(); + instance.options.onLineSelectionChange?.(range); + instance.options.onGutterUtilityClick?.(range); + }); + const editor = () => { + const textarea = container.querySelector("textarea"); + if (!textarea) throw new Error("Comment editor is not available"); + return textarea; + }; + const click = (label: string) => { + const button = Array.from(container.querySelectorAll("button")).find( + (node) => node.textContent === label, ); - editor().dispatchEvent(new Event("input", { bubbles: true })); - }); -} - -function setup(): Disposable { - const reset = () => useComposerDraftStore.setState(useComposerDraftStore.getInitialState(), true); - reset(); - container = document.createElement("div"); - return { [Symbol.dispose]: reset }; + if (!button) throw new Error(`Missing button: ${label}`); + return act(() => button.click()); + }; + const typeComment = (text: string) => + act(() => { + Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")!.set!.call( + editor(), + text, + ); + editor().dispatchEvent(new Event("input", { bubbles: true })); + }); + return { + container, + viewerRef, + view, + renderedFile, + select, + editor, + click, + typeComment, + [Symbol.dispose]: reset, + }; } describe("diff comment drafts", () => { it("keeps the text and selected line through refresh and a provisional lazy patch", async () => { - using _drafts = setup(); - const original = file("shared.ts", 1, "rename-pure", "old.ts"); - await using screen = await render(view([original]), { container }); - await click("Comment on shared.ts"); - await typeComment("Keep this draft"); - const refreshed = file("shared.ts", 2, "rename-pure", "old.ts"); - await screen.rerender(view([refreshed], "refreshed")); - expect(editor().value).toBe("Keep this draft"); - expect(container.querySelector("mark")?.textContent).toBe("Line 1"); + using test = setup(); + const original = file("shared.ts", "rename-pure", "old.ts"); + await using screen = await render(test.view([original]), { container: test.container }); + await expect.element(screen.getByText("new", { exact: true })).toBeVisible(); + expect(test.renderedFile().instance.getLineIndex(1)).toBeDefined(); + await test.select(); + await vi.waitFor(() => expect(test.editor().value).toBe("")); + await test.typeComment("Keep this draft"); + const refreshed = file("shared.ts", "rename-pure", "old.ts"); + await screen.rerender(test.view([refreshed], "refreshed", [], true)); + await vi.waitFor(() => expect(test.editor().value).toBe("Keep this draft")); + expect(test.viewerRef.current?.getSelectedLines()).toEqual( + expect.objectContaining({ id: original.fileKey, range }), + ); + await test.click("Comment"); + expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments ?? []).toEqual( + [], + ); + await act(() => { + test + .editor() + .dispatchEvent( + new KeyboardEvent("keydown", { key: "Enter", metaKey: true, bubbles: true }), + ); + }); + expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments ?? []).toEqual( + [], + ); + expect(test.editor().value).toBe("Keep this draft"); await screen.rerender( - view([], "loading", [ + test.view([], "loading", [ { ...refreshed.fileDiff, type: "rename-changed", hunks: [], cacheKey: "shared:pending" }, ]), ); - expect(container.querySelector("textarea")).toBeNull(); - await screen.rerender(view([refreshed], "loaded")); - expect(editor().value).toBe("Keep this draft"); - expect(container.querySelector("mark")?.textContent).toBe("Line 1"); - await click("Comment"); + expect(test.container.querySelector("textarea")).toBeNull(); + await screen.rerender(test.view([refreshed], "loaded")); + await vi.waitFor(() => expect(test.editor().value).toBe("Keep this draft")); + expect(test.viewerRef.current?.getSelectedLines()).toEqual( + expect.objectContaining({ id: original.fileKey, range }), + ); + await test.click("Comment"); expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments).toEqual([ expect.objectContaining({ filePath: "shared.ts", @@ -158,6 +161,83 @@ describe("diff comment drafts", () => { selection: { ...range, endSide: "additions" }, }), ]); - expect(container.querySelector("p")?.textContent).toBe("Keep this draft"); + }); + + it("rejects old line ranges and retained submission until Pierre adopts refreshed content", async () => { + using test = setup(); + const original = file("adoption.ts"); + const refreshed = file("adoption.ts", "change", undefined, "inserted\n+new"); + const highlight = WorkerPoolManager.prototype.highlightDiffAST; + let resumeHighlight: (() => void) | undefined; + let pool: WorkerPoolManager | undefined; + using _highlight = vi + .spyOn(WorkerPoolManager.prototype, "highlightDiffAST") + .mockImplementation(function (this: WorkerPoolManager, instance, diff) { + pool = this; + if (diff === refreshed.fileDiff) { + resumeHighlight = () => highlight.call(this, instance, diff); + return; + } + return highlight.call(this, instance, diff); + }); + await using screen = await render(test.view([original]), { container: test.container }); + await expect.element(screen.getByText("new", { exact: true })).toBeVisible(); + const initialPool = pool; + if (!initialPool) throw new Error("Diff worker pool is unavailable"); + await act(() => initialPool.primeDiffHighlightCache(original.fileDiff)); + expect(initialPool.getDiffResultCache(original.fileDiff)).toBeDefined(); + expect(test.renderedFile().instance.getLineIndex(1)).toBeDefined(); + await act(() => test.viewerRef.current?.getInstance()?.render(true)); + await test.select(); + await vi.waitFor(() => expect(test.editor().value).toBe("")); + await test.typeComment("Old coordinates"); + const retainedSubmit = Array.from(test.container.querySelectorAll("button")).find( + (node) => node.textContent === "Comment", + )!; + const previousInstance = test.renderedFile().instance; + + await screen.rerender(test.view([refreshed])); + await vi.waitFor(() => expect(resumeHighlight).toBeDefined()); + const pending = test.renderedFile(); + expect(pending.item.fileDiff).toBe(refreshed.fileDiff); + expect(pending.instance).toBe(previousInstance); + expect(pending.element.shadowRoot?.textContent).not.toContain("inserted"); + expect(retainedSubmit.isConnected).toBe(true); + expect( + pending.instance.__getNextRenderData({ + fileDiff: refreshed.fileDiff, + lineAnnotations: undefined, + }).fileDiff, + ).toBe(original.fileDiff); + await test.typeComment("Still visible old draft"); + expect(retainedSubmit.disabled).toBe(false); + await act(() => retainedSubmit.click()); + expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments ?? []).toEqual( + [], + ); + await test.select(); + expect(test.viewerRef.current?.getSelectedLines()).toBeNull(); + + await act(() => resumeHighlight?.()); + await vi.waitFor(() => { + expect( + test.renderedFile().instance.__getNextRenderData({ + fileDiff: refreshed.fileDiff, + lineAnnotations: undefined, + }).fileDiff, + ).toBe(refreshed.fileDiff); + expect(test.renderedFile().element.shadowRoot?.textContent).toContain("inserted"); + expect(test.container.querySelector("textarea")).toBeNull(); + }); + await test.select(); + await vi.waitFor(() => expect(test.editor().value).toBe("")); + await test.typeComment("Current coordinates"); + await test.click("Comment"); + expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments).toEqual([ + expect.objectContaining({ + text: "Current coordinates", + diff: expect.stringContaining("+inserted"), + }), + ]); }); }); diff --git a/apps/web/src/components/diffs/AnnotatableCodeView.logic.test.ts b/apps/web/src/components/diffs/AnnotatableCodeView.logic.test.ts new file mode 100644 index 0000000000000..1a2739d85a3f6 --- /dev/null +++ b/apps/web/src/components/diffs/AnnotatableCodeView.logic.test.ts @@ -0,0 +1,63 @@ +import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles"; +import { describe, expect, it } from "vite-plus/test"; + +import { buildFileDiffContentVersion, buildFileDiffIdentityKey } from "~/lib/diffRendering"; + +import { isDiffCommentAnchorCurrent } from "./AnnotatableCodeView.logic"; + +function file(contents: string) { + return parsePatchFiles( + `diff --git a/shared.ts b/shared.ts\n--- a/shared.ts\n+++ b/shared.ts\n@@ -1 +1 @@\n-old\n+${contents}\n`, + )[0]!.files[0]!; +} + +describe("diff comment anchors", () => { + it("invalidates old line coordinates when the same file changes", () => { + const original = file("new"); + const refreshed = file("newer"); + const anchor = { + fileKey: buildFileDiffIdentityKey(original), + filePath: "shared.ts", + previousFilePath: "shared.ts", + fileVersion: buildFileDiffContentVersion(original), + }; + expect(buildFileDiffIdentityKey(refreshed)).toBe(anchor.fileKey); + const changedFiles = new Map([ + [ + anchor.fileKey, + { fileVersion: buildFileDiffContentVersion(refreshed), fileDiff: refreshed }, + ], + ]); + expect(isDiffCommentAnchorCurrent(anchor, changedFiles, [])).toBe(false); + expect( + isDiffCommentAnchorCurrent( + anchor, + new Map([[anchor.fileKey, { ...anchor, fileDiff: original }]]), + [], + ), + ).toBe(true); + const pending = { ...original, type: "rename-changed" as const, hunks: [] }; + expect(isDiffCommentAnchorCurrent(anchor, new Map(), [pending])).toBe(true); + expect(isDiffCommentAnchorCurrent(anchor, changedFiles, [pending])).toBe(false); + const sameIdentityPending = { ...original, hunks: [], cacheKey: "shared:pending" }; + expect( + isDiffCommentAnchorCurrent( + anchor, + new Map([ + [ + anchor.fileKey, + { + fileDiff: sameIdentityPending, + fileVersion: buildFileDiffContentVersion(sameIdentityPending), + }, + ], + ]), + [sameIdentityPending], + ), + ).toBe(true); + expect( + isDiffCommentAnchorCurrent(anchor, new Map(), [{ ...pending, prevName: "other.ts" }]), + ).toBe(false); + expect(isDiffCommentAnchorCurrent(anchor, new Map(), [])).toBe(false); + }); +}); diff --git a/apps/web/src/components/diffs/AnnotatableCodeView.logic.ts b/apps/web/src/components/diffs/AnnotatableCodeView.logic.ts new file mode 100644 index 0000000000000..21a144abe27dc --- /dev/null +++ b/apps/web/src/components/diffs/AnnotatableCodeView.logic.ts @@ -0,0 +1,30 @@ +import type { FileDiffMetadata } from "@pierre/diffs"; + +import { resolveFileDiffPath, resolveFileDiffPreviousPath } from "~/lib/diffRendering"; + +export interface DiffCommentAnchor { + readonly fileKey: string; + readonly filePath: string; + readonly previousFilePath: string; + readonly fileVersion: number; +} + +export function isDiffCommentAnchorCurrent( + anchor: DiffCommentAnchor, + filesByKey: ReadonlyMap< + string, + { readonly fileVersion: number; readonly fileDiff: FileDiffMetadata } + >, + pendingFiles: ReadonlyArray, +): boolean { + const file = filesByKey.get(anchor.fileKey); + if (file !== undefined && !file.fileDiff.cacheKey?.endsWith(":pending")) { + return file.fileVersion === anchor.fileVersion; + } + // Lazy placeholders have provisional change types, so match both paths instead. + return pendingFiles.some( + (file) => + resolveFileDiffPath(file) === anchor.filePath && + resolveFileDiffPreviousPath(file) === anchor.previousFilePath, + ); +} diff --git a/apps/web/src/components/diffs/AnnotatableCodeView.tsx b/apps/web/src/components/diffs/AnnotatableCodeView.tsx index f5a0da4b1a903..aaac3f3ec2598 100644 --- a/apps/web/src/components/diffs/AnnotatableCodeView.tsx +++ b/apps/web/src/components/diffs/AnnotatableCodeView.tsx @@ -8,10 +8,15 @@ import type { } from "@pierre/diffs"; import type { CodeViewHandle, CodeViewProps } from "@pierre/diffs/react"; import type { EnvironmentId, ProjectId, ScopedThreadRef } from "@t3tools/contracts"; -import { useState, type ReactNode, type Ref } from "react"; +import { useRef, useState, type ReactNode, type Ref } from "react"; import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; -import { fnv1a32, resolveFileDiffPath, resolveFileDiffPreviousPath } from "~/lib/diffRendering"; +import { + buildFileDiffContentVersion, + fnv1a32, + resolveFileDiffPreviousPath, +} from "~/lib/diffRendering"; +import { useStableCallback } from "~/hooks/useStableCallback"; import { buildDiffReviewComment, restoreDiffReviewCommentRange, @@ -20,6 +25,7 @@ import { import { nextFileCommentId } from "../files/fileCommentAnnotations"; import { DiffCommentAnnotation } from "./DiffCommentAnnotation"; +import { isDiffCommentAnchorCurrent, type DiffCommentAnchor } from "./AnnotatableCodeView.logic"; import { StyledDiffCodeView, type StyledDiffCodeViewOptions } from "./StyledDiffCodeView"; interface DiffCommentAnnotationEntry { @@ -83,6 +89,7 @@ interface AnnotatableCodeViewProps { collapsed: boolean; }>; pendingFiles: ReadonlyArray; + isRefreshingDiff: boolean; sectionId: string; sectionTitle: string; composerDraftTarget: ScopedThreadRef | DraftId; @@ -112,6 +119,7 @@ export function AnnotatableCodeView({ codeViewKey, files, pendingFiles, + isRefreshingDiff, sectionId, sectionTitle, composerDraftTarget, @@ -134,28 +142,36 @@ export function AnnotatableCodeView({ const [selectedLines, setSelectedLines] = useState<{ id: string; range: SelectedLineRange; + anchor: DiffCommentAnchor; } | null>(null); - const [draft, setDraft] = useState<{ - fileKey: string; - filePath: string; - previousFilePath: string; - annotation: DiffCommentLineAnnotation; - } | null>(null); + const [draft, setDraft] = useState< + | (DiffCommentAnchor & { + annotation: DiffCommentLineAnnotation; + }) + | null + >(null); const [draftText, setDraftText] = useState(""); + const renderedFiles = useRef(new Map()); const filesByKey = new Map(files.map((file) => [file.fileKey, file])); - if ( - draft !== null && - !filesByKey.has(draft.fileKey) && - // Lazy placeholders have provisional change types, so match both paths instead. - !pendingFiles.some( - (file) => - resolveFileDiffPath(file) === draft.filePath && - resolveFileDiffPreviousPath(file) === draft.previousFilePath, - ) - ) { + const isCurrentFileRendered = (fileKey: string) => { + const file = filesByKey.get(fileKey); + const rendered = renderedFiles.current.get(fileKey); + return ( + !isRefreshingDiff && + file !== undefined && + rendered !== undefined && + (file.fileDiff === rendered || file.fileVersion === buildFileDiffContentVersion(rendered)) + ); + }; + if (draft !== null && !isDiffCommentAnchorCurrent(draft, filesByKey, pendingFiles)) { setDraft(null); setDraftText(""); + } + if ( + selectedLines !== null && + !isDiffCommentAnchorCurrent(selectedLines.anchor, filesByKey, pendingFiles) + ) { setSelectedLines(null); } const items: CodeViewDiffItem[] = files.map( @@ -208,10 +224,10 @@ export function AnnotatableCodeView({ } }; - const submitEntry = (entryId: string, text: string) => { + const submitEntry = useStableCallback((entryId: string, text: string) => { const entry = draft?.annotation.metadata.entries.find((candidate) => candidate.id === entryId); const file = draft ? filesByKey.get(draft.fileKey) : undefined; - if (!entry || !file) return; + if (isRefreshingDiff || !entry || !file || draft?.fileVersion !== file.fileVersion) return; const comment = buildDiffReviewComment({ id: entry.id, sectionId, @@ -225,38 +241,43 @@ export function AnnotatableCodeView({ setSelectedLines(null); setDraft(null); setDraftText(""); - }; + }); - const beginComment = (range: SelectedLineRange | null, context: DiffSelectionContext) => { - if (!range) return; - const item = context.item; - if (item.type !== "diff") return; - const id = nextFileCommentId(); - const filePath = resolveFileDiffPath(item.fileDiff); - const comment = buildDiffReviewComment({ - id, - sectionId, - sectionTitle, - filePath, - fileDiff: item.fileDiff, - range, - text: "", - }); - if (!comment) return; - setDraftText(""); - setDraft({ - fileKey: item.id, - filePath, - previousFilePath: resolveFileDiffPreviousPath(item.fileDiff), - annotation: { - side: annotationSide(range), - lineNumber: range.end, - metadata: { - entries: [{ id, kind: "draft", range, rangeLabel: comment.rangeLabel, text: "" }], + const beginComment = useStableCallback( + (range: SelectedLineRange | null, context: DiffSelectionContext) => { + if (!range) return; + const item = context.item; + if (item.type !== "diff") return; + const file = filesByKey.get(item.id); + if (!file || !isCurrentFileRendered(item.id)) return; + const id = nextFileCommentId(); + const filePath = file.filePath; + const comment = buildDiffReviewComment({ + id, + sectionId, + sectionTitle, + filePath, + fileDiff: file.fileDiff, + range, + text: "", + }); + if (!comment) return; + setDraftText(""); + setDraft({ + fileKey: item.id, + fileVersion: file.fileVersion, + filePath, + previousFilePath: resolveFileDiffPreviousPath(file.fileDiff), + annotation: { + side: annotationSide(range), + lineNumber: range.end, + metadata: { + entries: [{ id, kind: "draft", range, rangeLabel: comment.rangeLabel, text: "" }], + }, }, - }, - }); - }; + }); + }, + ); return ( @@ -274,13 +295,46 @@ export function AnnotatableCodeView({ {...(renderCodeViewFooter ? { renderCodeViewFooter } : {})} items={items} selectedLines={selectedLines} - onSelectedLinesChange={setSelectedLines} + onSelectedLinesChange={(selection) => { + if (selection !== null && !isCurrentFileRendered(selection.id)) return; + const file = selection === null ? undefined : filesByKey.get(selection.id); + setSelectedLines( + selection !== null && file !== undefined + ? { + ...selection, + anchor: { + fileKey: file.fileKey, + fileVersion: file.fileVersion, + filePath: file.filePath, + previousFilePath: resolveFileDiffPreviousPath(file.fileDiff), + }, + } + : null, + ); + }} options={{ ...options, - enableGutterUtility: draft === null, - enableLineSelection: draft === null, + enableGutterUtility: draft === null && !isRefreshingDiff, + enableLineSelection: draft === null && !isRefreshingDiff, onGutterUtilityClick: beginComment, onRevealSearchMatch: (item) => onRevealSearchMatch(item.id), + onPostRender: (node, _instance, phase, context) => { + if (context.type === "diff") { + if (phase === "unmount") { + renderedFiles.current.delete(context.item.id); + } else { + // At post-render this is the data Pierre just adopted, including retained rows. + const { fileDiff } = context.instance.__getNextRenderData({ + fileDiff: context.item.fileDiff, + lineAnnotations: undefined, + }); + renderedFiles.current.set(context.item.id, fileDiff); + } + options.onPostRender?.(node, context.instance, phase, context); + } else { + options.onPostRender?.(node, context.instance, phase, context); + } + }, }} renderHeaderFilenameSuffix={(item) => item.type === "diff" ? renderHeaderFilenameSuffix(item.fileDiff) : null @@ -313,6 +367,7 @@ export function AnnotatableCodeView({ onTextChange={setDraftText} onCancel={() => removeEntry(entry.id)} onComment={(text) => submitEntry(entry.id, text)} + pending={isRefreshingDiff} onDelete={() => removeEntry(entry.id)} /> ))} diff --git a/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx b/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx index 46edb4186a744..88ae6c23eb49e 100644 --- a/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx +++ b/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx @@ -46,6 +46,7 @@ function source(paths: string[], hash: string): ReviewDiffPreviewSource { function Probe({ source }: { source: ReviewDiffPreviewSource }) { const files = useReviewFilePatches({ + reviewKey: "review", environmentId: EnvironmentId.make("environment"), cwd: "/project", source, @@ -108,14 +109,17 @@ it("retains automatic and explicitly opened files through metadata insertion, re ); await expect.element(screen.getByText("e.txt:loaded")).toBeVisible(); await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); + const refreshedAtom = [...state.patches] + .filter(([key]) => JSON.parse(key)[1] === "f.txt") + .at(-1)![1]; await act(() => registry.set( - atom, + refreshedAtom, AsyncResult.fail(new Error("refresh failed"), { previousSuccess: Option.some(loaded) }), ), ); - await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); - await act(() => registry.set(atom, loaded)); + await expect.element(screen.getByText("f.txt:pending")).toBeVisible(); + await act(() => registry.set(refreshedAtom, loaded)); await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); }); @@ -142,4 +146,11 @@ it("preserves deletion and addition patches for a file replaced by a symlink", a await expect.element(screen.getByText("deleted", { exact: true })).toBeVisible(); await expect.element(screen.getByText("new", { exact: true })).toBeVisible(); await expect.element(screen.getByText("Pending files: 0")).toBeVisible(); + await screen.rerender( + + + , + ); + await expect.element(screen.getByText("deleted", { exact: true })).toBeVisible(); + await expect.element(screen.getByText("new", { exact: true })).toBeVisible(); }); diff --git a/apps/web/src/components/diffs/useReviewFilePatches.ts b/apps/web/src/components/diffs/useReviewFilePatches.ts index 728f43433b010..f6ce8604bf082 100644 --- a/apps/web/src/components/diffs/useReviewFilePatches.ts +++ b/apps/web/src/components/diffs/useReviewFilePatches.ts @@ -5,21 +5,29 @@ import type { ReviewDiffPreviewInput, ReviewDiffPreviewSource, } from "@t3tools/contracts"; -import * as Option from "effect/Option"; import * as AsyncResult from "effect/reactivity/AsyncResult"; import * as Atom from "effect/reactivity/Atom"; import { useContext, useState } from "react"; import { useStableCallback } from "~/hooks/useStableCallback"; -import { getRenderablePatch, resolveFileDiffPath, type RenderablePatch } from "~/lib/diffRendering"; +import { + getRenderablePatch, + resolveFileDiffPath, + retainedReviewFile, + retainLoadedReviewFiles, + type RenderablePatch, +} from "~/lib/diffRendering"; import { reviewEnvironment } from "~/state/review"; +const EMPTY_RETAINED_FILES: ReadonlyMap = new Map(); + const filePathCollator = new Intl.Collator(undefined, { numeric: true, sensitivity: "base", }); -const INITIAL_FILE_COUNT = 4; +const PAGE_SIZE = 4; export function useReviewFilePatches({ + reviewKey, environmentId, cwd, source, @@ -29,6 +37,7 @@ export function useReviewFilePatches({ previewInput, preview, }: { + reviewKey: string | null; environmentId: EnvironmentId | undefined; cwd: string | undefined; source: ReviewDiffPreviewSource | null; @@ -40,22 +49,48 @@ export function useReviewFilePatches({ preview: RenderablePatch | null; }) { const registry = useContext(RegistryContext); - const scope = JSON.stringify([ + const family = JSON.stringify([ + reviewKey, environmentId, cwd, + source?.id, source?.kind, source?.baseRef, - source?.headRef, baseRef, + source?.headRef, ignoreWhitespace, ]); - const files = sortedReviewFiles(source); - const automaticPaths = files.slice(0, INITIAL_FILE_COUNT).map((file) => file.path); - const [requested, setRequested] = useState(() => ({ scope, paths: automaticPaths })); - const previousPaths = requested.scope === scope ? requested.paths : []; - const paths = [...new Set([...previousPaths, ...automaticPaths])]; - if (requested.scope !== scope || paths.length !== previousPaths.length) { - setRequested({ scope, paths }); + const scope = JSON.stringify([family, source?.diffHash]); + const lazy = + (source?.truncated === true || source?.patchesDeferred === true) && source.files !== undefined; + const files = lazy ? sortedReviewFiles(source) : []; + const [requested, setRequested] = useState<{ family: string; paths: readonly string[] }>({ + family, + paths: [], + }); + const [retained, setRetained] = useState(() => ({ + family, + source, + byKey: + !lazy && preview?.kind === "files" + ? retainLoadedReviewFiles(EMPTY_RETAINED_FILES, preview.files) + : EMPTY_RETAINED_FILES, + })); + const previousByKey = retained.family === family ? retained.byKey : EMPTY_RETAINED_FILES; + const previousByPath = new Map(); + for (const file of previousByKey.values()) { + const path = resolveFileDiffPath(file); + const entries = previousByPath.get(path) ?? []; + entries.push(file); + previousByPath.set(path, entries); + } + const requestedPaths = new Set([ + ...files.slice(0, PAGE_SIZE).map((file) => file.path), + ...(requested.family === family ? requested.paths : []), + ...previousByPath.keys(), + ]); + if (requested.family !== family || requested.paths.length !== requestedPaths.size) { + setRequested({ family, paths: [...requestedPaths] }); } const queries = reviewFileQueries({ environmentId, @@ -65,74 +100,97 @@ export function useReviewFilePatches({ ignoreWhitespace, previewInput, scope, - requested, + files, + requestedPaths, }); const queriedPatchesAtom = reviewPatchesAtom( JSON.stringify([theme, queries.map(({ index, target }) => [index, target])]), ); const patches = useAtomValue(queriedPatchesAtom); const retry = useStableCallback((path: string) => { - const files = sortedReviewFiles(source); const query = queries.find(({ index }) => files[index]?.path === path)?.query; if (query) registry.refresh(query); }); - const pendingIndex = files.findIndex((_, index) => { - const patch = patches.get(index); - return !patch || patch._tag === "Initial"; - }); - const settledFileCount = source - ? pendingIndex < 0 - ? files.length - : pendingIndex - : preview?.kind === "files" - ? preview.files.length - : 0; const requestFiles = (indices: number[]) => setRequested((current) => { - const previous = current.scope === scope ? current.paths : []; - const added = indices.flatMap((index) => { - const path = files[index]?.path; - return path !== undefined && !previous.includes(path) ? [path] : []; - }); - return added.length === 0 ? current : { scope, paths: [...previous, ...added] }; + const paths = new Set(current.family === family ? current.paths : []); + for (const index of indices) { + const file = files[index]; + if (file) paths.add(file.path); + } + return current.family === family && paths.size === current.paths.length + ? current + : { family, paths: [...paths] }; }); - // The first unsettled file already requested means a batch is in flight; wait for it. - const loadNextFiles = () => { - if (queries.some(({ index }) => index === settledFileCount)) return; - requestFiles(Array.from({ length: 4 }, (_, index) => settledFileCount + index)); - }; - const requestFile = (path: string) => - requestFiles([files.findIndex((file) => file.path === path)]); - const renderableFiles = source + const renderableFiles = lazy ? files.flatMap((file, index): FileDiffMetadata[] => { const result = patches.get(index); - const value = result && Option.getOrUndefined(AsyncResult.value(result)); - if (value?.patch?.kind === "files") { - const loaded = value.patch.files.filter( - (candidate) => resolveFileDiffPath(candidate) === file.path, - ); - if (loaded.length > 0) return loaded; - } - return [ - { - name: file.path, - ...(file.previousPath ? { prevName: file.previousPath } : {}), - type: file.previousPath ? "rename-changed" : "change", - hunks: [], - additionLines: [], - deletionLines: [], - additionCharacterCount: 0, - deletionCharacterCount: 0, - splitLineCount: 0, - unifiedLineCount: 0, - isPartial: true, - cacheKey: `${scope}:${file.path}:pending`, - }, - ]; + const loaded = + result?._tag === "Success" && result.value.patch?.kind === "files" + ? result.value.patch.files.filter( + (candidate) => resolveFileDiffPath(candidate) === file.path, + ) + : []; + if (loaded.length > 0) return loaded; + const pending = + result === undefined || + result._tag === "Initial" || + (result._tag === "Success" && result.waiting); + const previous = pending ? previousByPath.get(file.path) : undefined; + return (previous ?? [undefined]).map((previous) => + retainedReviewFile({ + loaded: null, + previous, + sameFamily: true, + pending, + placeholder: { + name: file.path, + ...(file.previousPath ? { prevName: file.previousPath } : {}), + type: file.previousPath ? "rename-changed" : "change", + hunks: [], + additionLines: [], + deletionLines: [], + additionCharacterCount: 0, + deletionCharacterCount: 0, + splitLineCount: 0, + unifiedLineCount: 0, + isPartial: true, + cacheKey: `${scope}:${file.path}:pending`, + }, + }), + ); }) : (preview?.kind === "files" ? preview.files : []).toSorted((a, b) => filePathCollator.compare(resolveFileDiffPath(a), resolveFileDiffPath(b)), ); + const nextRetained = !source + ? EMPTY_RETAINED_FILES + : !lazy && retained.family === family && retained.source === source + ? previousByKey + : retainLoadedReviewFiles(previousByKey, renderableFiles); + if (retained.family !== family || retained.source !== source || retained.byKey !== nextRetained) { + setRetained({ family, source, byKey: nextRetained }); + } + const retainedPaths = new Set( + renderableFiles.filter((file) => !file.cacheKey?.endsWith(":pending")).map(resolveFileDiffPath), + ); + const readyFilePaths = new Set( + files + .filter((_, index) => { + const result = patches.get(index); + return ( + (result !== undefined && result._tag !== "Initial") || + retainedPaths.has(files[index]!.path) + ); + }) + .map((file) => file.path), + ); + const pendingIndex = files.findIndex((file) => !readyFilePaths.has(file.path)); + const settledFileCount = lazy + ? pendingIndex < 0 + ? files.length + : pendingIndex + : renderableFiles.length; const fileStates = new Map( files.map((file, index) => { const patch = patches.get(index); @@ -142,6 +200,7 @@ export function useReviewFilePatches({ error: patch?._tag === "Failure" || (patch?._tag === "Success" && + !patch.waiting && (patch.value.patch?.kind !== "files" || !patch.value.patch.files.some( (candidate) => resolveFileDiffPath(candidate) === file.path, @@ -152,16 +211,23 @@ export function useReviewFilePatches({ }), ); return { + scope, + family, fileStates, isPending: [...patches.values()].some((patch) => patch._tag === "Initial" || patch.waiting), retry, - requestFile, + requestFile: (path: string) => requestFiles([files.findIndex((file) => file.path === path)]), + readyFilePaths, renderableFiles, - remainingFileCount: source ? files.length - settledFileCount : 0, - loadNextFiles, + remainingFileCount: lazy ? files.length - settledFileCount : 0, + loadNextFiles: () => { + if (queries.some(({ index }) => index === settledFileCount)) return; + requestFiles(Array.from({ length: PAGE_SIZE }, (_, index) => settledFileCount + index)); + }, }; } +// The refresh effect keys on the identity of these queries. function reviewFileQueries({ environmentId, cwd, @@ -170,7 +236,8 @@ function reviewFileQueries({ ignoreWhitespace, previewInput, scope, - requested, + files, + requestedPaths, }: { environmentId: EnvironmentId | undefined; cwd: string | undefined; @@ -179,18 +246,15 @@ function reviewFileQueries({ ignoreWhitespace: boolean; previewInput: ReviewDiffPreviewInput | undefined; scope: string; - requested: { scope: string; paths: string[] }; + files: ReturnType; + requestedPaths: ReadonlySet; }) { - const files = sortedReviewFiles(source); - const paths = new Set([ - ...files.slice(0, INITIAL_FILE_COUNT).map((file) => file.path), - ...(requested.scope === scope ? requested.paths : []), - ]); const queries = !environmentId || !cwd || !source ? [] : files - .flatMap((file, index) => (paths.has(file.path) ? [{ file, index }] : [])) + .map((file, index) => ({ file, index })) + .filter(({ file }) => requestedPaths.has(file.path)) .map(({ file, index }) => { const target: ReviewPatchTarget = { environmentId, @@ -221,6 +285,7 @@ function sortedReviewFiles(source: ReviewDiffPreviewSource | null) { type ReviewPatchTarget = Parameters[0]; +// Derived atoms parse each query result once, even when another file finishes loading. const parsedPatchAtom = Atom.family((key: string) => { const [theme, target] = JSON.parse(key) as ["light" | "dark", ReviewPatchTarget]; return Atom.map(reviewEnvironment.diffFilePatch(target), (result) => @@ -240,6 +305,7 @@ const parsedPatchAtom = Atom.family((key: string) => { ); }); +// Keyed by theme and query targets so every render with the same files reads the same atom. const reviewPatchesAtom = Atom.family((key: string) => { const [theme, entries] = JSON.parse(key) as [ "light" | "dark", diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.browser.test.tsx index 5b9d0af32a129..33f1b915cb93b 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.browser.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.browser.test.tsx @@ -1,5 +1,7 @@ import { sha256 } from "@noble/hashes/sha2"; import { bytesToHex } from "@noble/hashes/utils"; +import { RegistryContext } from "@effect/atom-react"; +import { AsyncResult, AtomRegistry } from "effect/reactivity"; import { EnvironmentId, ProjectId, @@ -13,7 +15,7 @@ import "~/index.css"; import { __setClientSettingsForTests } from "../../hooks/useSettings"; import { DEFAULT_CLIENT_SETTINGS } from "@t3tools/contracts/settings"; -const { diffAtom, pagedDiffAtom } = await vi.hoisted(async () => { +const { diffAtom, pagedDiffAtom, pageResults, pages } = await vi.hoisted(async () => { const { AsyncResult, Atom } = await import("effect/reactivity"); const filePatch = (path: string) => [ @@ -46,19 +48,23 @@ const { diffAtom, pagedDiffAtom } = await vi.hoisted(async () => { }, "page-2": { patch: filePatch("late.test.ts"), truncated: false, nextCursor: null }, }; + const pageResults = Atom.family((cursor: string) => + Atom.make>(AsyncResult.success(pages[cursor]!)), + ); return { // An environment query whose refetch returns equal content hands back the previous value // itself, so a refresh publishes a new result around the very same page. diffAtom: Atom.make(() => AsyncResult.success(firstPage)), - pagedDiffAtom: Atom.family((cursor: string) => - Atom.make(() => AsyncResult.success(pages[cursor]!)), - ), + pagedDiffAtom: Atom.family((cursor: string) => Atom.make((get) => get(pageResults(cursor)))), + pageResults, + pages, }; }); const mocks = vi.hoisted(() => ({ paged: false, diffCommits: [] as Array, + diffCursors: [] as Array, viewedPaths: null as ReadonlySet | null, viewedListeners: new Set<() => void>(), })); @@ -101,13 +107,14 @@ vi.mock(import("~/state/pullRequests"), async (original) => { ...actual.pullRequestEnvironment, diff: (({ input }: { input: { cursor?: string; commit?: string } }) => { mocks.diffCommits.push(input.commit); + mocks.diffCursors.push(input.cursor); return mocks.paged ? pagedDiffAtom(input.cursor ?? "first") : diffAtom; }) as never, }, }; }); -import { render } from "../../../test/render"; +import { act, render } from "../../../test/render"; import PullRequestCodeTab from "./PullRequestCodeTab"; const detail: PullRequestDetailView = { @@ -167,6 +174,7 @@ function setup({ paged }: { paged: boolean }) { __setClientSettingsForTests(DEFAULT_CLIENT_SETTINGS); mocks.paged = paged; mocks.diffCommits = []; + mocks.diffCursors = []; mocks.viewedPaths = null; mocks.viewedListeners.clear(); } @@ -207,6 +215,37 @@ it("keeps the diff after a refresh that brings back the same page", async () => await expect.element(screen.getByText("1 file", { exact: true })).toBeVisible(); }); +it("restarts a refresh without advancing the cursor of the replaced page walk", async () => { + setup({ paged: true }); + const registry = AtomRegistry.make(); + using _registry = { [Symbol.dispose]: () => registry.dispose() }; + using _firstPage = { [Symbol.dispose]: registry.mount(pageResults("first")) }; + const url = `${detail.url}/files#diff-${bytesToHex(sha256(new TextEncoder().encode("late.test.ts")))}R4-R5`; + const settled = vi.fn(); + const content = (token: number, revealUrl?: string) => ( + + {tab(token, { path: "late.test.ts", nonce: 1 }, revealUrl, settled)} + + ); + await using screen = await render(content(0), { container: pagedContainer() }); + await expect.element(screen.getByText("41 files", { exact: true })).toBeVisible(); + + await act(() => registry.set(pageResults("first"), AsyncResult.initial())); + await screen.rerender(content(1)); + await screen.rerender(content(1, url)); + expect(settled).not.toHaveBeenCalled(); + await act(async () => { + registry.set(pageResults("first"), AsyncResult.success(pages.first!)); + await screen.rerender(content(2, url)); + }); + + await vi.waitFor(() => expect(mocks.diffCursors.at(-1)).toBe("page-2")); + await expect.element(screen.getByText("41 files", { exact: true })).toBeVisible(); + await vi.waitFor(() => expect(settled).toHaveBeenCalledWith(url)); + await screen.rerender(content(3)); + await expect.element(screen.getByText("41 files", { exact: true })).toBeVisible(); +}); + // regression: #13279 a Viewed answer that lands after a link reveals a file must not fold it it("keeps a revealed file open when its Viewed mark loads afterwards", async () => { setup({ paged: false }); diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index febf82441f514..44b9faa926f51 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -7,7 +7,12 @@ import { capitalizeTerm, sourceControlClients, } from "@t3tools/client-runtime/source-control-clients"; -import type { CodeViewItem, DiffLineAnnotation, SelectedLineRange } from "@pierre/diffs"; +import type { + CodeViewItem, + DiffLineAnnotation, + FileDiffMetadata, + SelectedLineRange, +} from "@pierre/diffs"; import type { CodeViewAnnotationArgs, CodeViewDiffItem } from "@pierre/diffs/react"; import type { EnvironmentId, @@ -36,7 +41,7 @@ import { ChevronsDownUpIcon, ChevronsUpDownIcon, } from "lucide-react"; -import { useAtomRefresh } from "@effect/atom-react"; +import { RegistryContext } from "@effect/atom-react"; import * as Schema from "effect/Schema"; import { createContext, @@ -50,6 +55,7 @@ import { import { observeIntersection } from "~/lib/observeIntersection"; import { useLocalStorage } from "~/hooks/useLocalStorage"; +import { useStableCallback } from "~/hooks/useStableCallback"; import { useClientSettings, updateClientSettings } from "~/hooks/useSettings"; import { useRenderedAppearance } from "~/hooks/useTheme"; import { areAllDiffFilesCollapsed } from "~/lib/diffCollapse"; @@ -66,7 +72,8 @@ import { fileDiffRows } from "@t3tools/client-runtime/review-guide-tour"; import { isTestChangedFile } from "@t3tools/shared/changedFileKind"; import { orderDiffFiles } from "./pullRequestFileOrder.logic"; import { - buildFileDiffRenderKey, + buildFileDiffIdentityKey, + buildFileDiffContentVersion, fnv1a32, getDiffLineStat, getRenderablePatch, @@ -96,7 +103,7 @@ import { FileExplorerPane } from "../files/FileExplorerPane"; import { useCodeViewFileReveal } from "../diffs/useCodeViewFileReveal"; import { useHideTestFiles } from "../diffs/useHideTestFiles"; import { diffFileTreeEntries, orderDiffFilesLikeTree } from "../diffs/diffFileTree.logic"; -import { StyledDiffCodeView } from "../diffs/StyledDiffCodeView"; +import { StyledDiffCodeView, type StyledDiffCodeViewOptions } from "../diffs/StyledDiffCodeView"; import { Button } from "../ui/button"; import { Checkbox } from "../ui/checkbox"; import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collapsible"; @@ -126,6 +133,7 @@ import { isFileDiffCollapsed, isLineInFileDiff, withFileDiffFold, + retainReviewDiffSlices, type DiffFoldOverride, type FileDiffFoldChoices, } from "./pullRequestDiff.logic"; @@ -173,6 +181,16 @@ const NO_SLICES: ReadonlyArray = []; // Query results outlive the panel's slices, so their parses survive closing and reopening // without retaining anything beyond the query cache's lifetime. const sliceParses = new WeakMap>(); +const fileContentVersions = new WeakMap(); + +function fileContentVersion(file: FileDiffMetadata) { + let version = fileContentVersions.get(file); + if (version === undefined) { + version = buildFileDiffContentVersion(file); + fileContentVersions.set(file, version); + } + return version; +} function parseSlice(slice: DiffSlice, cacheKey: string, ignoreWhitespace: boolean) { let parses = sliceParses.get(slice.source); @@ -228,7 +246,7 @@ function diffModel( ? parsed.files.map( (file, index) => [ - buildFileDiffRenderKey(file), + buildFileDiffIdentityKey(file), getDiffLineStat([parsed.sourceFiles[index] ?? file]), ] as const, ) @@ -328,6 +346,8 @@ interface MutableAnnotationGroup { interface DraftAnchor { readonly fileKey: string; + readonly contentVersion: number; + readonly scopeKey: string; readonly path: string; readonly oldPath: string | null; readonly position: PullRequestReviewPosition; @@ -406,16 +426,20 @@ function PullRequestCodeTab({ const [selectedLines, setSelectedLines] = useState<{ id: string; range: SelectedLineRange; + contentVersion: number; + scopeKey: string; } | null>(null); const [draft, setDraft] = useState(null); const [threadPending, setThreadPending] = useState(false); + const renderedFiles = useRef(new Map()); const [orphansOpen, setOrphansOpen] = useState(false); // Scope slices before reset to prevent using a previous pull request's data or cursor. const [sliceState, setSliceState] = useState<{ readonly key: string; readonly cursor: string | null; readonly slices: ReadonlyArray; - }>({ key: "", cursor: null, slices: NO_SLICES }); + readonly refreshing: ReadonlyArray | null; + }>({ key: "", cursor: null, slices: NO_SLICES, refreshing: null }); const referenceKey = pullRequestReviewKey(reference); // A commit opened from the timeline or left selected across a force-push may no longer be @@ -437,11 +461,12 @@ function PullRequestCodeTab({ setFoldOverride(null); setVisibleCommitCount(COMMIT_PAGE_SIZE); setOrphansOpen(false); - setSliceState({ key: scopeKey, cursor: null, slices: NO_SLICES }); + setSliceState({ key: scopeKey, cursor: null, slices: NO_SLICES, refreshing: null }); } const loadedSlices = sliceState.key === scopeKey ? sliceState.slices : NO_SLICES; const cursor = sliceState.key === scopeKey ? sliceState.cursor : null; + const registry = use(RegistryContext); const diffQuery = useEnvironmentQuery( pullRequestEnvironment.diff({ environmentId, @@ -457,7 +482,7 @@ function PullRequestCodeTab({ readonly cursor: string | null; readonly scopeKey: string; } | null>(null); - const data = diffQuery.data; + const data = diffQuery.isSuccess && !diffQuery.isPending ? diffQuery.data : null; if ( data !== null && (appliedPage?.data !== data || @@ -475,9 +500,19 @@ function PullRequestCodeTab({ nextCursor: data.nextCursor, omittedFileStats: data.omittedFileStats ?? [], }; + if (previous.key === scopeKey && previous.refreshing !== null) { + const index = previous.refreshing.findIndex((slice) => slice.cursor === cursor); + const refreshing = [ + ...(index < 0 ? previous.refreshing : previous.refreshing.slice(0, index)), + next, + ]; + return refreshing.length >= previous.slices.length || next.nextCursor === null + ? { key: scopeKey, cursor, slices: refreshing, refreshing: null } + : { ...previous, refreshing }; + } const index = slices.findIndex((slice) => slice.cursor === cursor); if (index === -1) { - return { key: scopeKey, cursor, slices: [...slices, next] }; + return { key: scopeKey, cursor, slices: [...slices, next], refreshing: null }; } const existing = slices[index]; if ( @@ -499,15 +534,22 @@ function PullRequestCodeTab({ return previous; } // Drop later slices when replacing a changed page; their cursors target the old diff. - return { key: scopeKey, cursor, slices: [...slices.slice(0, index), next] }; + return { key: scopeKey, cursor, slices: [...slices.slice(0, index), next], refreshing: null }; }); } - const refreshFirstDiffPage = useAtomRefresh( - pullRequestEnvironment.diff({ - environmentId, - input: { ...reference, ...(commit === null ? {} : { commit }) }, - }), - ); + const failedSliceIndex = loadedSlices.findIndex((slice) => slice.cursor === cursor); + if ( + diffQuery.error !== null && + !diffQuery.isPending && + (sliceState.refreshing !== null || failedSliceIndex !== -1) + ) { + setAppliedPage(null); + setSliceState((previous) => ({ + ...previous, + slices: previous.refreshing ?? previous.slices.slice(0, failedSliceIndex), + refreshing: null, + })); + } const reviewKey = referenceKey; const pendingComments = usePendingReviewComments(reference); const addComment = usePullRequestReviewStore((store) => store.addComment); @@ -539,28 +581,70 @@ function PullRequestCodeTab({ isViewed: isFileViewed, isStale: isFileViewedStale, } = filesViewed; - const appliedRefreshToken = useRef(refreshToken); - // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- a new refresh token clears loaded slices and refetches the first diff page and viewed files + const appliedRefresh = useRef({ refreshToken, backgroundRefreshToken }); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- refreshes mounted review pages from the environment query cache useEffect(() => { - if (appliedRefreshToken.current === refreshToken) return; - appliedRefreshToken.current = refreshToken; - setSliceState({ key: scopeKey, cursor: null, slices: NO_SLICES }); - // An equal refetch keeps the answer's identity, so the emptied slices refill only if the - // applied page is forgotten too. + const previous = appliedRefresh.current; + appliedRefresh.current = { refreshToken, backgroundRefreshToken }; + if ( + previous.refreshToken === refreshToken && + previous.backgroundRefreshToken === backgroundRefreshToken + ) + return; + registry.refresh( + pullRequestEnvironment.diff({ + environmentId, + input: { ...reference, ...(commit === null ? {} : { commit }) }, + }), + ); setAppliedPage(null); - refreshFirstDiffPage(); + // Stage a fresh walk while all displayed pages keep the reader's scroll geometry. + setSliceState((current) => ({ + ...retainReviewDiffSlices(current, scopeKey), + refreshing: [], + })); refreshFilesViewed(); - }, [refreshToken, scopeKey, refreshFirstDiffPage, refreshFilesViewed]); - const appliedBackgroundRefreshToken = useRef(backgroundRefreshToken); - // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- a background refresh token refetches the first diff page and viewed files while keeping slices + }, [ + refreshToken, + backgroundRefreshToken, + environmentId, + reference, + commit, + scopeKey, + registry, + refreshFilesViewed, + ]); + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- invalidates the next cached environment diff page before advancing its refresh walk useEffect(() => { - if (appliedBackgroundRefreshToken.current === backgroundRefreshToken) return; - appliedBackgroundRefreshToken.current = backgroundRefreshToken; - // Keeping slices mounted preserves scroll; the page merge invalidates changed pages. - setSliceState((previous) => ({ ...previous, cursor: null })); - refreshFirstDiffPage(); - refreshFilesViewed(); - }, [backgroundRefreshToken, refreshFirstDiffPage, refreshFilesViewed]); + const last = sliceState.refreshing?.at(-1); + if ( + !last || + last.cursor !== cursor || + last.nextCursor === null || + diffQuery.isPending || + diffQuery.error !== null + ) + return; + registry.refresh( + pullRequestEnvironment.diff({ + environmentId, + input: { ...reference, cursor: last.nextCursor, ...(commit === null ? {} : { commit }) }, + }), + ); + // oxlint-disable-next-line react/set-state-in-effect -- Advance only after invalidating the cached next page. + setSliceState((current) => + current.refreshing?.at(-1) === last ? { ...current, cursor: last.nextCursor } : current, + ); + }, [ + sliceState.refreshing, + cursor, + diffQuery.isPending, + diffQuery.error, + registry, + environmentId, + reference, + commit, + ]); const codeViewReveal = useCodeViewFileReveal(); const answeredRevealNonce = useRef(null); @@ -613,6 +697,45 @@ function PullRequestCodeTab({ }) : loadedFiles; const hiddenTestFileCount = loadedFiles.length - files.length; + // A stale mark means the file changed since it was viewed; Fold all and Expand all outrank marks. + const foldsAsViewed = (path: string) => + foldOverride === null && filesViewedEnabled && isFileViewed(path) && !isFileViewedStale(path); + const contentVersions = new Map( + loadedFiles.map((file) => [buildFileDiffIdentityKey(file), fileContentVersion(file)]), + ); + const isCurrentFileRendered = (fileKey: string) => { + const rendered = renderedFiles.current.get(fileKey); + return ( + !diffQuery.isPending && + sliceState.refreshing === null && + rendered !== undefined && + contentVersions.get(fileKey) === fileContentVersion(rendered) + ); + }; + const isCurrentFile = (fileKey: string, version: number, anchorScopeKey: string) => { + const file = loadedFiles.find((candidate) => buildFileDiffIdentityKey(candidate) === fileKey); + return ( + anchorScopeKey === scopeKey && + contentVersions.get(fileKey) === version && + file !== undefined && + !isFileDiffCollapsed( + fileKey, + effectiveFoldOverride, + foldChoices, + foldsAsViewed(resolveFileDiffPath(file)), + ) + ); + }; + // Stable viewer identities preserve scroll, but line coordinates belong to one file version. + if (draft !== null && !isCurrentFile(draft.fileKey, draft.contentVersion, draft.scopeKey)) { + setDraft(null); + } + if ( + selectedLines !== null && + !isCurrentFile(selectedLines.id, selectedLines.contentVersion, selectedLines.scopeKey) + ) { + setSelectedLines(null); + } const filePaths = files.map((file) => resolveFileDiffPath(file)); const omittedFileStats = new Map( loadedSlices.flatMap((slice) => @@ -637,7 +760,7 @@ function PullRequestCodeTab({ })(); const annotatedFiles = files.map((fileDiff) => { - const fileKey = buildFileDiffRenderKey(fileDiff); + const fileKey = buildFileDiffIdentityKey(fileDiff); const path = resolveFileDiffPath(fileDiff); const groups = new Map(); // Stacked view draws a context row once, so its old side shares the new side's annotation. @@ -755,11 +878,9 @@ function PullRequestCodeTab({ ); - // A stale mark means the file changed since it was viewed; Fold all and Expand all outrank marks. - const foldsAsViewed = (path: string) => - foldOverride === null && filesViewedEnabled && isFileViewed(path) && !isFileViewedStale(path); const items: CodeViewDiffItem[] = annotatedFiles.map( ({ fileKey, path, fileDiff, annotations, annotationsVersion }) => { + const contentVersion = fileContentVersion(fileDiff); const collapsed = isFileDiffCollapsed( fileKey, effectiveFoldOverride, @@ -775,7 +896,9 @@ function PullRequestCodeTab({ fileDiff, annotations, collapsed, - version: fnv1a32(`${collapsed ? "1" : "0"}:${viewedMark}:${annotationsVersion}`), + version: fnv1a32( + `${contentVersion}:${collapsed ? "1" : "0"}:${viewedMark}:${annotationsVersion}`, + ), }; }, ); @@ -787,12 +910,19 @@ function PullRequestCodeTab({ // Do not re-arm the visible sentinel after failure; it would retry indefinitely. const canLoadNextSlice = + sliceState.refreshing === null && nextCursor !== null && nextCursor !== cursor && !diffQuery.isPending && diffQuery.error === null; const loadNextSlice = () => { - if (nextCursor === null) return; + if (nextCursor === null || sliceState.refreshing !== null) return; + registry.refresh( + pullRequestEnvironment.diff({ + environmentId, + input: { ...reference, cursor: nextCursor, ...(commit === null ? {} : { commit }) }, + }), + ); setSliceState((previous) => ({ ...previous, cursor: nextCursor })); }; const sentinelRef = (node: HTMLDivElement | null) => { @@ -845,7 +975,13 @@ function PullRequestCodeTab({ if (item === undefined) return; // Record the unfold even when already open, so a Viewed mark that loads later cannot fold it. setFoldChoices((current) => withFileDiffFold(current, item.id, false)); - if (range) setSelectedLines({ id: item.id, range }); + if (range) + setSelectedLines({ + id: item.id, + range, + contentVersion: fileContentVersion(item.fileDiff), + scopeKey, + }); codeViewReveal.request(item.id, range); }; @@ -866,8 +1002,14 @@ function PullRequestCodeTab({ if (!range || !canSelectLines) return; const item = context.item; if (item.type !== "diff") return; - const file = files.find((candidate) => buildFileDiffRenderKey(candidate) === item.id); - if (!file) return; + const file = files.find((candidate) => buildFileDiffIdentityKey(candidate) === item.id); + if ( + !file || + !isCurrentFileRendered(item.id) || + item.collapsed === true || + !isCurrentFile(item.id, fileContentVersion(item.fileDiff), scopeKey) + ) + return; // On a host without ranged comments a range collapses to its last line: one that silently // lost its first line there would be worse than one line. const path = resolveFileDiffPath(file); @@ -881,6 +1023,8 @@ function PullRequestCodeTab({ if (anchor === null) return; setDraft({ fileKey: item.id, + contentVersion: fileContentVersion(file), + scopeKey, path, oldPath: previousPath === path ? null : previousPath, ...anchor, @@ -888,12 +1032,21 @@ function PullRequestCodeTab({ }); }; + // Portals can retain a submit handler from before a refresh invalidated its draft. + const isCurrentDraft = useStableCallback( + (anchor: DraftAnchor) => + draft === anchor && + canSelectLines && + isCurrentFileRendered(anchor.fileKey) && + isCurrentFile(anchor.fileKey, anchor.contentVersion, anchor.scopeKey), + ); const finishSelection = ( anchor: DraftAnchor, text: string, onFinish: (comment: ReviewCommentContext) => void, ) => { - const file = files.find((candidate) => buildFileDiffRenderKey(candidate) === anchor.fileKey); + if (!isCurrentDraft(anchor)) return; + const file = files.find((candidate) => buildFileDiffIdentityKey(candidate) === anchor.fileKey); const comment = file === undefined ? null @@ -986,15 +1139,29 @@ function PullRequestCodeTab({ themeType: resolvedTheme, stickyHeaders: true, loadDiffFiles, - enableGutterUtility: canSelectLines && draft === null, - enableLineSelection: canSelectLines && draft === null, + enableGutterUtility: + canSelectLines && draft === null && !diffQuery.isPending && sliceState.refreshing === null, + enableLineSelection: + canSelectLines && draft === null && !diffQuery.isPending && sliceState.refreshing === null, onGutterUtilityClick: beginComment, onLineSelectionEnd: beginComment, - onPostRender: codeViewReveal.flush, + onPostRender: (_node, _instance, phase, context) => { + if (context.type === "diff") { + if (phase === "unmount") renderedFiles.current.delete(context.item.id); + else { + const { fileDiff } = context.instance.__getNextRenderData({ + fileDiff: context.item.fileDiff, + lineAnnotations: undefined, + }); + renderedFiles.current.set(context.item.id, fileDiff); + } + } + codeViewReveal.flush(); + }, // Find may repeat requests before render; only unfold, never toggle. onRevealSearchMatch: (item: CodeViewItem) => setFoldChoices((current) => withFileDiffFold(current, item.id, false)), - }; + } satisfies StyledDiffCodeViewOptions; const runThreadCommand = async ( label: string, @@ -1118,6 +1285,7 @@ function PullRequestCodeTab({ setSelectedLines(null); }} onComment={(body) => { + if (!isCurrentDraft(draft)) return; if (!canCommentOnLines) { if (onAddToAgentSelection) { finishSelection(draft, body, (comment) => @@ -1487,6 +1655,7 @@ function PullRequestCodeTab({ const revealLink = useEffectEvent(() => { if (revealUrl === undefined) answeredLink.current = null; if (revealUrl === undefined || linkAnchor?.kind !== "diff") return; + if (sliceState.refreshing !== null) return; // The panel selects the link's commit after this tab's effects run, so the files on screen // can still be another commit's. const linkCommit = pullRequestLinkCommit(revealUrl); @@ -1661,12 +1830,22 @@ function PullRequestCodeTab({ {/* The viewer must own its scroll element to position virtual rows within it. */} + key={scopeKey} className="@container h-full overflow-auto [scrollbar-gutter:stable]" containerRef={scrollerRef} viewerRef={codeViewReveal.attach} items={items} selectedLines={selectedLines} - onSelectedLinesChange={setSelectedLines} + onSelectedLinesChange={(selection) => { + if (selection !== null && !isCurrentFileRendered(selection.id)) return; + const contentVersion = + selection === null ? undefined : contentVersions.get(selection.id); + setSelectedLines( + selection !== null && contentVersion !== undefined + ? { ...selection, contentVersion, scopeKey } + : null, + ); + }} options={diffViewOptions} renderCodeViewFooter={renderCodeViewFooter} renderHeaderPrefix={renderHeaderPrefix} diff --git a/apps/web/src/components/pullRequest/pullRequestDiff.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDiff.logic.test.ts index 47f309b3e32c0..494b39f0ef650 100644 --- a/apps/web/src/components/pullRequest/pullRequestDiff.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDiff.logic.test.ts @@ -6,6 +6,7 @@ import { isFileDiffCollapsed, isLineInFileDiff, withFileDiffFold, + retainReviewDiffSlices, } from "./pullRequestDiff.logic"; function fileWithHunks( @@ -19,6 +20,29 @@ function fileWithHunks( return { name: "src/app.ts", hunks } as unknown as FileDiffMetadata; } +describe("retainReviewDiffSlices", () => { + const first = { cursor: null, patch: "page-1" }; + const slices = [first, { cursor: "page-2", patch: "page-2" }]; + + it("keeps every displayed page while the same review refreshes", () => { + expect(retainReviewDiffSlices({ key: "pr-1", cursor: "page-2", slices }, "pr-1")).toEqual({ + key: "pr-1", + cursor: null, + slices, + }); + const open = { key: "pr-1", cursor: null, slices: [first] }; + expect(retainReviewDiffSlices(open, "pr-1")).toBe(open); + }); + + it("starts over when the review itself changes", () => { + expect(retainReviewDiffSlices({ key: "pr-1", cursor: null, slices }, "pr-2")).toEqual({ + key: "pr-2", + cursor: null, + slices: [], + }); + }); +}); + describe("isLineInFileDiff", () => { const file = fileWithHunks([ { deletionStart: 10, deletionCount: 3, additionStart: 10, additionCount: 5 }, diff --git a/apps/web/src/components/pullRequest/pullRequestDiff.logic.ts b/apps/web/src/components/pullRequest/pullRequestDiff.logic.ts index da3844e90e1bb..30c7053ce6fc9 100644 --- a/apps/web/src/components/pullRequest/pullRequestDiff.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDiff.logic.ts @@ -1,6 +1,20 @@ import type { FileDiffMetadata } from "@pierre/diffs"; import type { PullRequestDiffSide } from "@t3tools/contracts"; +export interface ReviewDiffSliceState { + readonly key: string; + readonly cursor: string | null; + readonly slices: ReadonlyArray; +} + +export function retainReviewDiffSlices( + previous: ReviewDiffSliceState, + scopeKey: string, +): ReviewDiffSliceState { + if (previous.key !== scopeKey) return { key: scopeKey, cursor: null, slices: [] }; + return previous.cursor === null ? previous : { ...previous, cursor: null }; +} + /** Only rendered hunks can anchor comments; list others off-diff. */ export function isLineInFileDiff( file: FileDiffMetadata, diff --git a/apps/web/src/hooks/useProactivePanels.ts b/apps/web/src/hooks/useProactivePanels.ts index 60c78a850c25f..5f1a2840abf60 100644 --- a/apps/web/src/hooks/useProactivePanels.ts +++ b/apps/web/src/hooks/useProactivePanels.ts @@ -176,6 +176,7 @@ export function useProactivePanels(input: { diffAction === "defer" || shouldDeferLink ? previousRunningTurnId : activeRunningTurnId, }; if (diffAction !== "open" || newlyCompletedTurnId === null) return; + const alreadyReviewingDiff = openSurface?.kind === "diff"; if ( !panels.openProactive( threadRef, @@ -185,7 +186,9 @@ export function useProactivePanels(input: { ) { return; } - useDiffPanelStore.getState().selectGitScope(threadRef, "branch"); + if (!alreadyReviewingDiff) { + useDiffPanelStore.getState().selectGitScope(threadRef, "branch"); + } onDiffPanelOpen?.(); }, [ activeLatestRun?.runId, diff --git a/apps/web/src/lib/diffRendering.test.ts b/apps/web/src/lib/diffRendering.test.ts index dc8ced6d25738..493ca7f2802c0 100644 --- a/apps/web/src/lib/diffRendering.test.ts +++ b/apps/web/src/lib/diffRendering.test.ts @@ -7,6 +7,8 @@ import { buildFileDiffRenderKey, getDiffLineStat, getRenderablePatch, + retainLoadedReviewFiles, + retainedReviewFile, resolveFileDiffPath, resolveFileDiffPreviousPath, } from "./diffRendering"; @@ -266,6 +268,80 @@ describe("getRenderablePatch", () => { }); describe("diff file reconciliation", () => { + const reviewFile = (line: string) => { + const patch = getRenderablePatch( + [ + "diff --git a/app.ts b/app.ts", + "--- a/app.ts", + "+++ b/app.ts", + "@@ -1 +1 @@", + "-before", + `+${line}`, + ].join("\n"), + "review", + ); + if (patch?.kind !== "files" || !patch.files[0]) throw new Error("Expected a file patch"); + return patch.files[0]; + }; + const loaded = reviewFile("after"); + const placeholder = { + ...loaded, + cacheKey: "refresh:app.ts:pending", + hunks: [], + additionLines: [], + deletionLines: [], + splitLineCount: 0, + unifiedLineCount: 0, + }; + + it("keeps loaded hunks during a same-review refresh until fresh content arrives", () => { + const previous = retainLoadedReviewFiles(new Map(), [loaded]); + const pending = retainedReviewFile({ + loaded: null, + previous: previous.get(buildFileDiffIdentityKey(loaded)), + placeholder, + sameFamily: true, + pending: true, + }); + expect(pending).toBe(loaded); + expect(getDiffLineStat([pending])).toEqual({ additions: 1, deletions: 1 }); + + const refreshed = retainedReviewFile({ + loaded: reviewFile("newer"), + previous: pending, + placeholder, + sameFamily: true, + pending: false, + }); + expect(refreshed.additionLines.join("")).toBe("newer"); + expect(buildFileDiffContentVersion(refreshed)).not.toBe(buildFileDiffContentVersion(pending)); + }); + + it("drops old hunks after a failed refresh or a switch to another review", () => { + const failed = retainedReviewFile({ + loaded: null, + previous: loaded, + placeholder, + sameFamily: true, + pending: false, + }); + expect(failed.hunks).toEqual([]); + const retained = retainLoadedReviewFiles( + new Map([[buildFileDiffIdentityKey(loaded), loaded]]), + [failed], + ); + expect(retained.has(buildFileDiffIdentityKey(loaded))).toBe(false); + + const switched = retainedReviewFile({ + loaded: null, + previous: loaded, + placeholder, + sameFamily: false, + pending: true, + }); + expect(switched.hunks).toEqual([]); + }); + it("keeps Pierre's render key stable when a partial diff hydrates", () => { const patch = [ "diff --git a/example.ts b/example.ts", @@ -313,6 +389,8 @@ describe("diff file reconciliation", () => { expect(buildFileDiffIdentityKey(deleted)).not.toBe(buildFileDiffIdentityKey(added)); expect(new Set(parsed.files.map(buildFileDiffIdentityKey)).size).toBe(parsed.files.length); + const retained = retainLoadedReviewFiles(new Map(), parsed.files); + expect([...retained.values()]).toEqual([deleted, added]); }); it("keeps identities stable and versions local to the changed file", () => { diff --git a/apps/web/src/lib/diffRendering.ts b/apps/web/src/lib/diffRendering.ts index 09b4d792b0e52..f91d9e23bad72 100644 --- a/apps/web/src/lib/diffRendering.ts +++ b/apps/web/src/lib/diffRendering.ts @@ -209,6 +209,40 @@ export function buildFileDiffRenderKey(fileDiff: FileDiffMetadata): string { return cacheKey.endsWith(":hydrated") ? cacheKey.slice(0, -":hydrated".length) : cacheKey; } +export function retainedReviewFile(input: { + readonly loaded: FileDiffMetadata | null; + readonly previous: FileDiffMetadata | undefined; + readonly placeholder: FileDiffMetadata; + readonly sameFamily: boolean; + readonly pending: boolean; +}): FileDiffMetadata { + if (input.loaded) return input.loaded; + if ( + input.pending && + input.sameFamily && + input.previous && + !input.previous.cacheKey?.endsWith(":pending") + ) { + return input.previous; + } + return input.placeholder; +} + +export function retainLoadedReviewFiles( + previous: ReadonlyMap, + files: readonly FileDiffMetadata[], +): ReadonlyMap { + const next = new Map( + files + .filter((file) => !file.cacheKey?.endsWith(":pending")) + .map((file) => [buildFileDiffIdentityKey(file), file]), + ); + return next.size === previous.size && + [...next].every(([path, file]) => previous.get(path) === file) + ? previous + : next; +} + function hashFileDiffPart(hash: number, value: string | number | boolean | undefined): number { const serialized = value === undefined ? "undefined" : String(value); const withLength = fnv1a32(`${typeof value}:${serialized.length}:`, hash);