Skip to content

fix(web): prevent duplicate diff file ids #11447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Frexxty wants to merge 3 commits into from
Closed

fix(web): prevent duplicate diff file ids #11447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e1d8c5
fix(web): prevent duplicate diff file ids
Frexxty Sep 12, 2026
4ede6cf
fix(web): preserve duplicate diff interactions
Frexxty Sep 12, 2026
46ef41e
Merge branch 'main' into fix/duplicate-diff-file-ids
Frexxty Sep 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 18 additions & 20 deletions apps/web/src/components/DiffPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { useLocalStorage } from "../hooks/useLocalStorage";
import { useTheme } from "../hooks/useTheme";
import {
buildFileDiffContentVersion,
buildFileDiffIdentityKey,
buildFileDiffIdentityKeys,
getDiffCollapseIconClassName,
getDiffLineStat,
getRenderablePatch,
Expand Down Expand Up @@ -416,15 +416,14 @@ export default function DiffPanel({
}),
);
}, [renderablePatch]);
const renderableFileEntries = useMemo(
() =>
renderableFiles.map((fileDiff) => ({
fileDiff,
fileKey: buildFileDiffIdentityKey(fileDiff),
fileVersion: buildFileDiffContentVersion(fileDiff),
})),
[renderableFiles],
);
const renderableFileEntries = useMemo(() => {
const fileKeys = buildFileDiffIdentityKeys(renderableFiles);
return renderableFiles.map((fileDiff, index) => ({
fileDiff,
fileKey: fileKeys[index]!,

@macroscopeapp macroscopeapp Bot Sep 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium components/DiffPanel.tsx:423

Duplicate-path records cannot be independently collapsed or selected: clicking the non-title area of the second header resolves its filePath with codeViewFiles.find(...) and toggles the first matching record, while the file tree has the same path-only ambiguity. Pass the stable fileKey through header and file-tree interactions instead of identifying records by displayed path.

🤖 Copy this AI Prompt to have your agent fix this: 
In file @apps/web/src/components/DiffPanel.tsx around line 423:

Duplicate-path records cannot be independently collapsed or selected: clicking the non-title area of the second header resolves its `filePath` with `codeViewFiles.find(...)` and toggles the first matching record, while the file tree has the same path-only ambiguity. Pass the stable `fileKey` through header and file-tree interactions instead of identifying records by displayed path.

@Frexxty Frexxty Sep 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The header interaction now carries and uses the occurrence-aware fileKey, so repeated records collapse independently. The file tree intentionally keys entries by path and deduplicates identical paths through its tree model; it cannot expose two independently selectable rows for the same displayed path. Changing that would require inventing a new visible tree identity and would be outside this crash fix.

@macroscopeapp macroscopeapp Bot Sep 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

fileVersion: buildFileDiffContentVersion(fileDiff),
}));
}, [renderableFiles]);
const codeViewFiles = useMemo(
() =>
renderableFileEntries.map(({ fileDiff, fileKey, fileVersion }) => {
Expand Down Expand Up @@ -956,14 +955,10 @@ export default function DiffPanel({
(node): node is HTMLElement =>
node instanceof HTMLElement && node.hasAttribute("data-diffs-header"),
);
const headerFilePath = header
?.querySelector("[data-title]")
?.textContent?.trim();
if (!headerFilePath) return;
const file = codeViewFiles.find(
(candidate) => candidate.filePath === headerFilePath,
);
if (file) toggleDiffFileCollapsed(file.fileKey);
const fileKey =
header?.querySelector<HTMLElement>("[data-diff-file-key]")?.dataset
.diffFileKey;
if (fileKey) toggleDiffFileCollapsed(fileKey);
}}
>
<AnnotatableCodeView
Expand All @@ -975,8 +970,11 @@ export default function DiffPanel({
sectionId={reviewSectionId}
sectionTitle={reviewSectionTitle}
composerDraftTarget={composerDraftTarget}
renderHeaderFilenameSuffix={(fileDiff) => (
<DiffFilePathCopyButton filePath={resolveFileDiffPath(fileDiff)} />
renderHeaderFilenameSuffix={(fileDiff, fileKey) => (
<>
<span data-diff-file-key={fileKey} />
<DiffFilePathCopyButton filePath={resolveFileDiffPath(fileDiff)} />
</>
)}
renderHeaderPrefix={(fileDiff, fileKey, collapsed) => {
const filePath = resolveFileDiffPath(fileDiff);
Expand Down
4 changes: 2 additions & 2 deletions apps/web/src/components/diffs/AnnotatableCodeView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ interface AnnotatableCodeViewProps {
options: StyledDiffCodeViewOptions<DiffCommentAnnotationGroup>;
viewerRef?: Ref<AnnotatableCodeViewHandle>;
className?: string;
renderHeaderFilenameSuffix: (fileDiff: FileDiffMetadata) => ReactNode;
renderHeaderFilenameSuffix: (fileDiff: FileDiffMetadata, fileKey: string) => ReactNode;
renderHeaderPrefix: (
fileDiff: FileDiffMetadata,
fileKey: string,
Expand Down Expand Up @@ -255,7 +255,7 @@ export function AnnotatableCodeView({
onGutterUtilityClick: beginComment,
}}
renderHeaderFilenameSuffix={(item) =>
item.type === "diff" ? renderHeaderFilenameSuffix(item.fileDiff) : null
item.type === "diff" ? renderHeaderFilenameSuffix(item.fileDiff, item.id) : null
}
renderHeaderPrefix={(item) =>
item.type === "diff"
Expand Down
20 changes: 20 additions & 0 deletions apps/web/src/lib/diffRendering.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, it } from "vite-plus/test";
import {
buildFileDiffContentVersion,
buildFileDiffIdentityKey,
buildFileDiffIdentityKeys,
buildFileDiffRenderKey,
buildPatchCacheKey,
getDiffLineStat,
Expand Down Expand Up @@ -148,6 +149,25 @@ describe("diff file reconciliation", () => {
buildFileDiffContentVersion(beforeChanged),
);
});

it("disambiguates repeated file identities without dropping either diff", () => {
const filePatch = [
"diff --git a/scripts/push-release.sh b/scripts/push-release.sh",
"--- a/scripts/push-release.sh",
"+++ b/scripts/push-release.sh",
"@@ -1 +1 @@",
"-before",
"+after",
].join("\n");
const parsed = getRenderablePatch(`${filePatch}\n${filePatch}`);
expect(parsed?.kind).toBe("files");
if (parsed?.kind !== "files") return;

const keys = buildFileDiffIdentityKeys(parsed.files);
expect(parsed.files).toHaveLength(2);
expect(new Set(keys).size).toBe(2);
expect(keys[0]).toBe(buildFileDiffIdentityKey(parsed.files[0]!));
});
});

describe("getDiffLineStat", () => {
Expand Down
10 changes: 10 additions & 0 deletions apps/web/src/lib/diffRendering.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,16 @@ export function buildFileDiffIdentityKey(fileDiff: FileDiffMetadata): string {
return `${resolveFileDiffPreviousPath(fileDiff)}\u0000${resolveFileDiffPath(fileDiff)}`;
}

export function buildFileDiffIdentityKeys(files: ReadonlyArray<FileDiffMetadata>): string[] {
const occurrences = new Map<string, number>();
return files.map((file) => {
const identity = buildFileDiffIdentityKey(file);
const occurrence = occurrences.get(identity) ?? 0;
occurrences.set(identity, occurrence + 1);
return occurrence === 0 ? identity : `${identity}\u0000${occurrence}`;
});
}

export function buildFileDiffRenderKey(fileDiff: FileDiffMetadata): string {
const cacheKey = fileDiff.cacheKey;
if (!cacheKey) return `${fileDiff.prevName ?? "none"}:${fileDiff.name}`;
Expand Down
Loading