Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 27 additions & 10 deletions apps/web/src/components/DiffPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ import { readLocalApi } from "../localApi";
import { resolvePathLinkTarget } from "../terminal-links";
import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch";
import { useTheme } from "../hooks/useTheme";
import { buildPatchCacheKey } from "../lib/diffRendering";
import { resolveDiffThemeName } from "../lib/diffRendering";
import { buildPatchCacheKey, canRenderFileDiff, resolveDiffThemeName } from "../lib/diffRendering";
import { useTurnDiffSummaries } from "../hooks/useTurnDiffSummaries";
import { selectProjectByRef, useStore } from "../store";
import { createThreadSelectorByRef } from "../storeSelectors";
Expand Down Expand Up @@ -328,12 +327,22 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
if (!renderablePatch || renderablePatch.kind !== "files") {
return [];
}
return renderablePatch.files.toSorted((left, right) =>
resolveFileDiffPath(left).localeCompare(resolveFileDiffPath(right), undefined, {
numeric: true,
sensitivity: "base",
}),
);
return renderablePatch.files
.map((fileDiff) => ({
canRender: canRenderFileDiff(fileDiff),
fileDiff,
filePath: resolveFileDiffPath(fileDiff),
}))
.toSorted((left, right) => {
const renderOrder = Number(!left.canRender) - Number(!right.canRender);
if (renderOrder !== 0) {
return renderOrder;
}
Comment on lines +337 to +340
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.

Not sure about this, but everything else looks good

return left.filePath.localeCompare(right.filePath, undefined, {
numeric: true,
sensitivity: "base",
});
});
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.

Wrong key in visibility effect

Medium Severity

After renderableFiles became wrapper objects, renderableFiles.map(buildFileDiffRenderKey) still passes each wrapper into buildFileDiffRenderKey instead of fileDiff. Visible keys collapse to a bogus shared value, so the effect that prunes collapsedDiffFileKeys drops legitimate per-file keys whenever the list is recomputed.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 57d9293. Configure here.

}, [renderablePatch]);

useEffect(() => {
Expand Down Expand Up @@ -659,8 +668,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
intersectionObserverMargin: 1200,
}}
>
{renderableFiles.map((fileDiff) => {
const filePath = resolveFileDiffPath(fileDiff);
{renderableFiles.map(({ canRender, fileDiff, filePath }) => {
const fileKey = buildFileDiffRenderKey(fileDiff);
const themedFileKey = `${fileKey}:${resolvedTheme}`;
const collapsed = collapsedDiffFileKeys.has(fileKey);
Expand Down Expand Up @@ -712,8 +720,17 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
theme: resolveDiffThemeName(resolvedTheme),
themeType: resolvedTheme as DiffThemeType,
unsafeCSS: DIFF_PANEL_UNSAFE_CSS,
collapsed: !canRender,
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.

Duplicate collapsed option key

Medium Severity

The FileDiff options object sets collapsed twice; the later collapsed: !canRender overrides the user-driven collapsed flag. Renderable files always receive collapsed: false, so manual collapse/expand no longer matches the header control or what the diff body shows.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 57d9293. Configure here.

}}
/>

{!canRender && (
<div className="px-3 py-3 text-[11px] leading-relaxed text-muted-foreground">
<p>
This file is too large to display. Open the file to inspect the change.
</p>
</div>
)}
</div>
);
})}
Expand Down
26 changes: 25 additions & 1 deletion apps/web/src/lib/diffRendering.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { describe, expect, it } from "vitest";
import { buildPatchCacheKey } from "./diffRendering";
import {
buildPatchCacheKey,
canRenderFileDiff,
MAX_RENDERABLE_DIFF_LINE_LENGTH,
} from "./diffRendering";

describe("buildPatchCacheKey", () => {
it("returns a stable cache key for identical content", () => {
Expand Down Expand Up @@ -29,3 +33,23 @@ describe("buildPatchCacheKey", () => {
);
});
});

describe("diff render line limits", () => {
it("rejects file diffs with pathological line lengths", () => {
expect(
canRenderFileDiff({
additionLines: ["small"],
deletionLines: ["x".repeat(MAX_RENDERABLE_DIFF_LINE_LENGTH + 1)],
}),
).toBe(false);
});

it("allows file diffs within the line length limit", () => {
expect(
canRenderFileDiff({
additionLines: ["x".repeat(MAX_RENDERABLE_DIFF_LINE_LENGTH)],
deletionLines: ["small"],
}),
).toBe(true);
});
});
13 changes: 13 additions & 0 deletions apps/web/src/lib/diffRendering.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import type { FileDiffMetadata } from "@pierre/diffs";

export const DIFF_THEME_NAMES = {
light: "pierre-light",
dark: "pierre-dark",
} as const;

export type DiffThemeName = (typeof DIFF_THEME_NAMES)[keyof typeof DIFF_THEME_NAMES];

export const MAX_RENDERABLE_DIFF_LINE_LENGTH = 500_000;

export function resolveDiffThemeName(theme: "light" | "dark"): DiffThemeName {
return theme === "dark" ? DIFF_THEME_NAMES.dark : DIFF_THEME_NAMES.light;
}
Expand Down Expand Up @@ -37,3 +41,12 @@ export function buildPatchCacheKey(patch: string, scope = "diff-panel"): string
).toString(36);
return `${scope}:${normalizedPatch.length}:${primary}:${secondary}`;
}

export function canRenderFileDiff(
fileDiff: Pick<FileDiffMetadata, "additionLines" | "deletionLines">,
): boolean {
return (
fileDiff.additionLines.every((line) => line.length <= MAX_RENDERABLE_DIFF_LINE_LENGTH) &&
fileDiff.deletionLines.every((line) => line.length <= MAX_RENDERABLE_DIFF_LINE_LENGTH)
);
}
Loading