| author | |
| committer | |
| log | f2a914b8588bf3038472c14b129fbc5ddb3a2992 |
| tree | 9835880f76c73efa0c76d78e659dd531d8d152f0 |
| parent | 8401f4d85887786fbb773d3fd41b8f07a77624b8 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>7 files changed, 202 insertions(+), 12 deletions(-)
apps/web/src/components/DiffPanel.tsx+15-5| ... | ... | @@ -30,7 +30,8 @@ import { cn } from "~/lib/utils"; |
| 30 | 30 | import { selectThreadDiffPanelSelection, useDiffPanelStore } from "../diffPanelStore"; |
| 31 | 31 | import { useTheme } from "../hooks/useTheme"; |
| 32 | 32 | import { |
| 33 | buildFileDiffRenderKey, | |
| 33 | buildFileDiffContentVersion, | |
| 34 | buildFileDiffIdentityKey, | |
| 34 | 35 | getDiffCollapseIconClassName, |
| 35 | 36 | getDiffLineStat, |
| 36 | 37 | getRenderablePatch, |
| ... | ... | @@ -300,7 +301,7 @@ export default function DiffPanel({ |
| 300 | 301 | const selectedGitSource = branchDiffPreview.data?.sources.find( |
| 301 | 302 | (source) => source.kind === (selectedGitScope === "unstaged" ? "working-tree" : "branch-range"), |
| 302 | 303 | ); |
| 303 | const loadDiffFiles = useMemo<FileDiffContentsLoader | undefined>(() => { | |
| 304 | const currentLoadDiffFiles = useMemo<FileDiffContentsLoader | undefined>(() => { | |
| 304 | 305 | const preview = branchDiffPreview.data; |
| 305 | 306 | if (selectedTurnId !== null || !activeThread || !preview || !selectedGitSource) { |
| 306 | 307 | return undefined; |
| ... | ... | @@ -321,6 +322,13 @@ export default function DiffPanel({ |
| 321 | 322 | selectedGitSource, |
| 322 | 323 | selectedTurnId, |
| 323 | 324 | ]); |
| 325 | const loadDiffFilesRef = useRef(currentLoadDiffFiles); | |
| 326 | loadDiffFilesRef.current = currentLoadDiffFiles; | |
| 327 | const loadDiffFiles = useCallback<FileDiffContentsLoader>(async (fileDiff) => { | |
| 328 | const loader = loadDiffFilesRef.current; | |
| 329 | if (!loader) throw new Error("Diff file contents are unavailable for this selection."); | |
| 330 | return loader(fileDiff); | |
| 331 | }, []); | |
| 324 | 332 | const localBranchRefs = useEnvironmentQuery( |
| 325 | 333 | selectedTurnId === null && |
| 326 | 334 | selectedGitScope === "branch" && |
| ... | ... | @@ -401,17 +409,19 @@ export default function DiffPanel({ |
| 401 | 409 | () => |
| 402 | 410 | renderableFiles.map((fileDiff) => ({ |
| 403 | 411 | fileDiff, |
| 404 | fileKey: buildFileDiffRenderKey(fileDiff), | |
| 412 | fileKey: buildFileDiffIdentityKey(fileDiff), | |
| 413 | fileVersion: buildFileDiffContentVersion(fileDiff), | |
| 405 | 414 | })), |
| 406 | 415 | [renderableFiles], |
| 407 | 416 | ); |
| 408 | 417 | const codeViewFiles = useMemo( |
| 409 | 418 | () => |
| 410 | renderableFileEntries.map(({ fileDiff, fileKey }) => { | |
| 419 | renderableFileEntries.map(({ fileDiff, fileKey, fileVersion }) => { | |
| 411 | 420 | return { |
| 412 | 421 | fileDiff, |
| 413 | 422 | filePath: resolveFileDiffPath(fileDiff), |
| 414 | 423 | fileKey, |
| 424 | fileVersion, | |
| 415 | 425 | collapsed: collapsedDiffFileKeys.has(fileKey), |
| 416 | 426 | }; |
| 417 | 427 | }), |
| ... | ... | @@ -952,7 +962,7 @@ export default function DiffPanel({ |
| 952 | 962 | preferredHighlighter: PREFERRED_HIGHLIGHTER, |
| 953 | 963 | themeType: resolvedTheme as DiffThemeType, |
| 954 | 964 | stickyHeaders: true, |
| 955 | ...(loadDiffFiles ? { loadDiffFiles } : {}), | |
| 965 | ...(currentLoadDiffFiles ? { loadDiffFiles } : {}), | |
| 956 | 966 | }} |
| 957 | 967 | /> |
| 958 | 968 | </div> |
apps/web/src/components/diffs/AnnotatableCodeView.tsx+3-2| ... | ... | @@ -77,6 +77,7 @@ interface AnnotatableCodeViewProps { |
| 77 | 77 | fileDiff: FileDiffMetadata; |
| 78 | 78 | filePath: string; |
| 79 | 79 | fileKey: string; |
| 80 | fileVersion: number; | |
| 80 | 81 | collapsed: boolean; |
| 81 | 82 | }>; |
| 82 | 83 | sectionId: string; |
| ... | ... | @@ -125,7 +126,7 @@ export function AnnotatableCodeView({ |
| 125 | 126 | const filesByKey = useMemo(() => new Map(files.map((file) => [file.fileKey, file])), [files]); |
| 126 | 127 | const items = useMemo<CodeViewDiffItem<DiffCommentAnnotationGroup>[]>( |
| 127 | 128 | () => |
| 128 | files.map(({ fileDiff, filePath, fileKey, collapsed }) => { | |
| 129 | files.map(({ fileDiff, filePath, fileKey, fileVersion, collapsed }) => { | |
| 129 | 130 | const persisted = reviewComments |
| 130 | 131 | .filter( |
| 131 | 132 | (comment) => |
| ... | ... | @@ -153,7 +154,7 @@ export function AnnotatableCodeView({ |
| 153 | 154 | annotations, |
| 154 | 155 | collapsed, |
| 155 | 156 | version: fnv1a32( |
| 156 | `${collapsed ? "1" : "0"}:${annotations | |
| 157 | `${fileVersion}:${collapsed ? "1" : "0"}:${annotations | |
| 157 | 158 | .flatMap((annotation) => |
| 158 | 159 | annotation.metadata.entries.map( |
| 159 | 160 | (entry) => `${entry.id}:${entry.rangeLabel}:${entry.text}`, |
apps/web/src/components/files/FileBrowserPanel.tsx+11-3| ... | ... | @@ -22,6 +22,7 @@ import { T3_PIERRE_ICONS } from "~/pierre-icons"; |
| 22 | 22 | |
| 23 | 23 | import { createFileTreeDragMentionController } from "./fileTreeDragMention"; |
| 24 | 24 | import { areAllDirectoriesExpanded, setAllDirectoriesExpanded } from "./fileTreeExpansion"; |
| 25 | import { buildFileTreePathUpdates } from "./fileTreePathReconciliation"; | |
| 25 | 26 | import { useProjectEntriesQuery } from "./projectFilesQueryState"; |
| 26 | 27 | |
| 27 | 28 | interface FileBrowserPanelProps { |
| ... | ... | @@ -126,7 +127,7 @@ export default function FileBrowserPanel({ |
| 126 | 127 | () => entries.filter((entry) => entry.kind === "directory").map(treePath), |
| 127 | 128 | [entries], |
| 128 | 129 | ); |
| 129 | const previousTreePathsRef = useRef<readonly string[]>([]); | |
| 130 | const previousTreePathsRef = useRef<readonly string[] | null>(null); | |
| 130 | 131 | const syncingSelectionRef = useRef(false); |
| 131 | 132 | const treeSelectionPathRef = useRef<string | null>(null); |
| 132 | 133 | const handledRevealRef = useRef<{ path: string; revealId: number } | null>(null); |
| ... | ... | @@ -281,11 +282,18 @@ export default function FileBrowserPanel({ |
| 281 | 282 | }); |
| 282 | 283 | |
| 283 | 284 | useEffect(() => { |
| 285 | if (entriesQuery.data === null) return; | |
| 284 | 286 | if (previousTreePathsRef.current === treePaths) return; |
| 285 | 287 | entryKindsRef.current = entryKinds; |
| 288 | const previousTreePaths = previousTreePathsRef.current; | |
| 286 | 289 | previousTreePathsRef.current = treePaths; |
| 287 | model.resetPaths(treePaths); | |
| 288 | }, [entryKinds, model, treePaths]); | |
| 290 | if (previousTreePaths === null) { | |
| 291 | model.resetPaths(treePaths); | |
| 292 | return; | |
| 293 | } | |
| 294 | const updates = buildFileTreePathUpdates(previousTreePaths, treePaths); | |
| 295 | if (updates.length > 0) model.batch(updates); | |
| 296 | }, [entriesQuery.data, entryKinds, model, treePaths]); | |
| 289 | 297 | |
| 290 | 298 | useEffect(() => { |
| 291 | 299 | if (!selectedPath) { |
apps/web/src/components/files/fileTreePathReconciliation.test.ts created+31| ... | ... | @@ -0,0 +1,31 @@ |
| 1 | import { describe, expect, it } from "vite-plus/test"; | |
| 2 | ||
| 3 | import { buildFileTreePathUpdates } from "./fileTreePathReconciliation"; | |
| 4 | ||
| 5 | describe("buildFileTreePathUpdates", () => { | |
| 6 | it("updates only paths that changed", () => { | |
| 7 | expect( | |
| 8 | buildFileTreePathUpdates( | |
| 9 | ["src/", "src/kept.ts", "src/removed.ts"], | |
| 10 | ["src/", "src/kept.ts", "src/added.ts"], | |
| 11 | ), | |
| 12 | ).toEqual([ | |
| 13 | { type: "remove", path: "src/removed.ts" }, | |
| 14 | { type: "add", path: "src/added.ts" }, | |
| 15 | ]); | |
| 16 | }); | |
| 17 | ||
| 18 | it("removes a missing subtree with one recursive update", () => { | |
| 19 | expect( | |
| 20 | buildFileTreePathUpdates( | |
| 21 | ["src/", "src/feature/", "src/feature/index.ts", "src/kept.ts"], | |
| 22 | ["src/", "src/kept.ts"], | |
| 23 | ), | |
| 24 | ).toEqual([{ type: "remove", path: "src/feature/", recursive: true }]); | |
| 25 | }); | |
| 26 | ||
| 27 | it("does nothing when a refresh returns the same tree", () => { | |
| 28 | const paths = ["src/", "src/index.ts"]; | |
| 29 | expect(buildFileTreePathUpdates(paths, [...paths])).toEqual([]); | |
| 30 | }); | |
| 31 | }); |
apps/web/src/components/files/fileTreePathReconciliation.ts created+32| ... | ... | @@ -0,0 +1,32 @@ |
| 1 | import type { FileTreeBatchOperation } from "@pierre/trees"; | |
| 2 | ||
| 3 | function pathDepth(path: string): number { | |
| 4 | return path.split("/").filter(Boolean).length; | |
| 5 | } | |
| 6 | ||
| 7 | export function buildFileTreePathUpdates( | |
| 8 | previousPaths: readonly string[], | |
| 9 | nextPaths: readonly string[], | |
| 10 | ): FileTreeBatchOperation[] { | |
| 11 | const previous = new Set(previousPaths); | |
| 12 | const next = new Set(nextPaths); | |
| 13 | const removedDirectoryRoots: string[] = []; | |
| 14 | const updates: FileTreeBatchOperation[] = []; | |
| 15 | ||
| 16 | const removedPaths = previousPaths | |
| 17 | .filter((path) => !next.has(path)) | |
| 18 | .toSorted((left, right) => pathDepth(left) - pathDepth(right)); | |
| 19 | for (const path of removedPaths) { | |
| 20 | if (removedDirectoryRoots.some((directory) => path.startsWith(directory))) continue; | |
| 21 | const recursive = path.endsWith("/"); | |
| 22 | updates.push({ type: "remove", path, ...(recursive ? { recursive: true } : {}) }); | |
| 23 | if (recursive) removedDirectoryRoots.push(path); | |
| 24 | } | |
| 25 | ||
| 26 | const addedPaths = nextPaths | |
| 27 | .filter((path) => !previous.has(path)) | |
| 28 | .toSorted((left, right) => pathDepth(left) - pathDepth(right)); | |
| 29 | for (const path of addedPaths) updates.push({ type: "add", path }); | |
| 30 | ||
| 31 | return updates; | |
| 32 | } |
apps/web/src/lib/diffRendering.test.ts+46-2| ... | ... | @@ -1,5 +1,7 @@ |
| 1 | 1 | import { describe, expect, it } from "vite-plus/test"; |
| 2 | 2 | import { |
| 3 | buildFileDiffContentVersion, | |
| 4 | buildFileDiffIdentityKey, | |
| 3 | 5 | buildFileDiffRenderKey, |
| 4 | 6 | buildPatchCacheKey, |
| 5 | 7 | getDiffLineStat, |
| ... | ... | @@ -82,8 +84,8 @@ describe("getRenderablePatch", () => { |
| 82 | 84 | }); |
| 83 | 85 | }); |
| 84 | 86 | |
| 85 | describe("buildFileDiffRenderKey", () => { | |
| 86 | it("keeps file identity stable when Pierre hydrates a partial diff", () => { | |
| 87 | describe("diff file reconciliation", () => { | |
| 88 | it("keeps Pierre's render key stable when a partial diff hydrates", () => { | |
| 87 | 89 | const patch = [ |
| 88 | 90 | "diff --git a/example.ts b/example.ts", |
| 89 | 91 | "--- a/example.ts", |
| ... | ... | @@ -104,6 +106,48 @@ describe("buildFileDiffRenderKey", () => { |
| 104 | 106 | |
| 105 | 107 | expect(buildFileDiffRenderKey(file)).toBe(key); |
| 106 | 108 | }); |
| 109 | ||
| 110 | it("keeps identities stable and versions local to the changed file", () => { | |
| 111 | const patch = (secondLine: string) => | |
| 112 | [ | |
| 113 | "diff --git a/unchanged.ts b/unchanged.ts", | |
| 114 | "--- a/unchanged.ts", | |
| 115 | "+++ b/unchanged.ts", | |
| 116 | "@@ -1 +1 @@", | |
| 117 | "-before", | |
| 118 | "+after", | |
| 119 | "diff --git a/changed.ts b/changed.ts", | |
| 120 | "--- a/changed.ts", | |
| 121 | "+++ b/changed.ts", | |
| 122 | "@@ -1 +1 @@", | |
| 123 | "-old", | |
| 124 | `+${secondLine}`, | |
| 125 | ].join("\n"); | |
| 126 | const before = getRenderablePatch(patch("new"), "before"); | |
| 127 | const after = getRenderablePatch(patch("newer"), "after"); | |
| 128 | expect(before?.kind).toBe("files"); | |
| 129 | expect(after?.kind).toBe("files"); | |
| 130 | if (before?.kind !== "files" || after?.kind !== "files") return; | |
| 131 | ||
| 132 | const [beforeUnchanged, beforeChanged] = before.files; | |
| 133 | const [afterUnchanged, afterChanged] = after.files; | |
| 134 | expect(beforeUnchanged).toBeDefined(); | |
| 135 | expect(beforeChanged).toBeDefined(); | |
| 136 | expect(afterUnchanged).toBeDefined(); | |
| 137 | expect(afterChanged).toBeDefined(); | |
| 138 | if (!beforeUnchanged || !beforeChanged || !afterUnchanged || !afterChanged) return; | |
| 139 | ||
| 140 | expect(buildFileDiffIdentityKey(afterUnchanged)).toBe( | |
| 141 | buildFileDiffIdentityKey(beforeUnchanged), | |
| 142 | ); | |
| 143 | expect(buildFileDiffIdentityKey(afterChanged)).toBe(buildFileDiffIdentityKey(beforeChanged)); | |
| 144 | expect(buildFileDiffContentVersion(afterUnchanged)).toBe( | |
| 145 | buildFileDiffContentVersion(beforeUnchanged), | |
| 146 | ); | |
| 147 | expect(buildFileDiffContentVersion(afterChanged)).not.toBe( | |
| 148 | buildFileDiffContentVersion(beforeChanged), | |
| 149 | ); | |
| 150 | }); | |
| 107 | 151 | }); |
| 108 | 152 | |
| 109 | 153 | describe("getDiffLineStat", () => { |
apps/web/src/lib/diffRendering.ts+64| ... | ... | @@ -163,6 +163,10 @@ export function resolveFileDiffPreviousPath(fileDiff: FileDiffMetadata): string |
| 163 | 163 | return raw; |
| 164 | 164 | } |
| 165 | 165 | |
| 166 | export function buildFileDiffIdentityKey(fileDiff: FileDiffMetadata): string { | |
| 167 | return `${resolveFileDiffPreviousPath(fileDiff)}\u0000${resolveFileDiffPath(fileDiff)}`; | |
| 168 | } | |
| 169 | ||
| 166 | 170 | export function buildFileDiffRenderKey(fileDiff: FileDiffMetadata): string { |
| 167 | 171 | const cacheKey = fileDiff.cacheKey; |
| 168 | 172 | if (!cacheKey) return `${fileDiff.prevName ?? "none"}:${fileDiff.name}`; |
| ... | ... | @@ -170,6 +174,66 @@ export function buildFileDiffRenderKey(fileDiff: FileDiffMetadata): string { |
| 170 | 174 | return cacheKey.endsWith(":hydrated") ? cacheKey.slice(0, -":hydrated".length) : cacheKey; |
| 171 | 175 | } |
| 172 | 176 | |
| 177 | function hashFileDiffPart(hash: number, value: string | number | boolean | undefined): number { | |
| 178 | const serialized = value === undefined ? "undefined" : String(value); | |
| 179 | const withLength = fnv1a32(`${typeof value}:${serialized.length}:`, hash); | |
| 180 | return fnv1a32(serialized, withLength); | |
| 181 | } | |
| 182 | ||
| 183 | /** | |
| 184 | * Content-only version for CodeView reconciliation. Pierre's cache key includes | |
| 185 | * the whole patch, so using it here would repaint every file when one changes. | |
| 186 | */ | |
| 187 | export function buildFileDiffContentVersion(fileDiff: FileDiffMetadata): number { | |
| 188 | let hash = FNV_OFFSET_BASIS_32; | |
| 189 | const append = (value: string | number | boolean | undefined) => { | |
| 190 | hash = hashFileDiffPart(hash, value); | |
| 191 | }; | |
| 192 | ||
| 193 | append(fileDiff.name); | |
| 194 | append(fileDiff.prevName); | |
| 195 | append(fileDiff.lang); | |
| 196 | append(fileDiff.newObjectId); | |
| 197 | append(fileDiff.prevObjectId); | |
| 198 | append(fileDiff.mode); | |
| 199 | append(fileDiff.prevMode); | |
| 200 | append(fileDiff.type); | |
| 201 | append(fileDiff.isPartial); | |
| 202 | append(fileDiff.splitLineCount); | |
| 203 | append(fileDiff.unifiedLineCount); | |
| 204 | ||
| 205 | for (const line of fileDiff.additionLines) append(line); | |
| 206 | for (const line of fileDiff.deletionLines) append(line); | |
| 207 | for (const hunk of fileDiff.hunks) { | |
| 208 | append(hunk.collapsedBefore); | |
| 209 | append(hunk.additionStart); | |
| 210 | append(hunk.additionCount); | |
| 211 | append(hunk.additionLines); | |
| 212 | append(hunk.additionLineIndex); | |
| 213 | append(hunk.deletionStart); | |
| 214 | append(hunk.deletionCount); | |
| 215 | append(hunk.deletionLines); | |
| 216 | append(hunk.deletionLineIndex); | |
| 217 | append(hunk.hunkContext); | |
| 218 | append(hunk.hunkSpecs); | |
| 219 | append(hunk.splitLineStart); | |
| 220 | append(hunk.splitLineCount); | |
| 221 | append(hunk.unifiedLineStart); | |
| 222 | append(hunk.unifiedLineCount); | |
| 223 | append(hunk.noEOFCRAdditions); | |
| 224 | append(hunk.noEOFCRDeletions); | |
| 225 | for (const content of hunk.hunkContent) { | |
| 226 | append(content.type); | |
| 227 | append(content.additionLineIndex); | |
| 228 | append(content.deletionLineIndex); | |
| 229 | append(content.type === "change" ? content.additions : content.lines); | |
| 230 | append(content.type === "change" ? content.deletions : undefined); | |
| 231 | } | |
| 232 | } | |
| 233 | ||
| 234 | return hash; | |
| 235 | } | |
| 236 | ||
| 173 | 237 | export function getDiffCollapseIconClassName(fileDiff: FileDiffMetadata): string { |
| 174 | 238 | switch (fileDiff.type) { |
| 175 | 239 | case "new": |