From 4a560b4e4ebb37efb7f57805ba79e37f5500bdca Mon Sep 17 00:00:00 2001 From: Bilal Bakr <62337003+Bil0000@users.noreply.github.com> Date: Sun, 20 Sep 2026 14:49:04 +0300 Subject: [PATCH 1/2] fix(web): honor whitespace settings in pull request diffs (#12438) Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com> --- .../pullRequest/PullRequestCodeTab.tsx | 46 ++++-- apps/web/src/lib/diffRendering.test.ts | 131 ++++++++++++++++++ apps/web/src/lib/diffRendering.ts | 68 ++++++++- 3 files changed, 230 insertions(+), 15 deletions(-) diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index b535c4f45e97..08a46872b697 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -20,6 +20,7 @@ import { InfoIcon, MessageSquareIcon, MessageSquareOffIcon, + PilcrowIcon, Rows3Icon, TextWrapIcon, TriangleAlertIcon, @@ -233,6 +234,7 @@ function PullRequestCodeTab({ const diffLayout = settings.diffLayout; const updateClientSettings = useUpdateClientSettings(); const [wordWrap, setWordWrap] = useState(settings.wordWrap); + const [ignoreWhitespace, setIgnoreWhitespace] = useState(settings.diffIgnoreWhitespace); const [fileTreeOpen, setFileTreeOpen] = useLocalStorage( PULL_REQUEST_FILE_TREE_STORAGE_KEY, false, @@ -390,16 +392,17 @@ function PullRequestCodeTab({ loadedSlices.map((slice) => { // The patch's own hash is part of the key: a refreshed page reuses its cursor, and a // key of position alone would keep handing back the parse of the patch it replaced. - const cacheKey = `pull-request:${scopeKey}:${resolvedTheme}:${slice.cursor ?? "first"}:${fnv1a32(slice.patch)}`; + const cacheKey = `pull-request:${scopeKey}:${resolvedTheme}:${ignoreWhitespace}:${slice.cursor ?? "first"}:${fnv1a32(slice.patch)}`; const cached = parseCache.current.get(cacheKey); if (cached) return cached; const parsed = getRenderablePatch(slice.patch, cacheKey, { compactPartialHunkOffsets: true, + ignoreWhitespace, }); if (parsed) parseCache.current.set(cacheKey, parsed); return parsed; }), - [loadedSlices, resolvedTheme, scopeKey], + [loadedSlices, resolvedTheme, scopeKey, ignoreWhitespace], ); // Ordered within a slice rather than across them: ordering the accumulated set would let a late // slice push a file the reader is part way through further down the page. @@ -702,7 +705,13 @@ function PullRequestCodeTab({ // that silently lost its first line on the other hosts would be worse than one line. const path = resolveFileDiffPath(file); const previousPath = resolveFileDiffPreviousPath(file); - const position = resolveDiffReviewPosition(file, range.end, range.endSide ?? range.side); + const sourceFile = parsedSlices + .flatMap((slice) => (slice?.kind === "files" ? slice.sourceFiles : [])) + .find((candidate) => resolveFileDiffPath(candidate) === path); + const side = range.endSide ?? range.side; + const position = + (sourceFile && resolveDiffReviewPosition(sourceFile, range.end, side)) ?? + resolveDiffReviewPosition(file, range.end, side); if (position === null) return; setDraft({ fileKey: item.id, @@ -712,7 +721,7 @@ function PullRequestCodeTab({ range, }); }, - [canCommentOnLines, files], + [canCommentOnLines, files, parsedSlices], ); // Built here because the parsed diff only lives here, and built by the same function the @@ -1123,11 +1132,6 @@ function PullRequestCodeTab({ } }, [commit, onSelectedCommitChange, selectedCommit]); const scopeLabel = selectedCommit ? selectedCommit.messageHeadline : "All commits"; - /** - * The same controls the thread diff panel carries, in the same order, minus the - * ignore-whitespace toggle: that is `git diff -w` on the server, and no host's pull request - * diff API offers it. - */ const toolbar = (
@@ -1276,6 +1280,30 @@ function PullRequestCodeTab({
+ + { + setIgnoreWhitespace(Boolean(pressed)); + setDraft(null); + setSelectedLines(null); + }} + /> + } + > + + + + {ignoreWhitespace ? "Show whitespace changes" : "Hide whitespace changes"} + + {fileKeys.length > 0 ? ( { }); describe("getRenderablePatch", () => { + it("hides indentation changes around inserted JSX without moving review lines", () => { + const patch = [ + "diff --git a/item.tsx b/item.tsx", + "--- a/item.tsx", + "+++ b/item.tsx", + "@@ -40,5 +40,7 @@", + ' ', + "- ", + '-

{name}

', + "- ", + "+ {showName && (", + "+ ", + '+

{name}

', + "+ ", + "+ )}", + " ", + "@@ -80 +82 @@", + "-const value = 1;", + "+const value = 2;", + ].join("\n"); + const shown = getRenderablePatch(patch, "pr", { compactPartialHunkOffsets: true }); + const hidden = getRenderablePatch(patch, "pr", { + compactPartialHunkOffsets: true, + ignoreWhitespace: true, + }); + expect(shown?.kind).toBe("files"); + expect(hidden?.kind).toBe("files"); + if (shown?.kind !== "files" || hidden?.kind !== "files") return; + expect(getDiffLineStat(shown.files)).toEqual({ additions: 6, deletions: 4 }); + expect(getDiffLineStat(hidden.files)).toEqual({ additions: 3, deletions: 1 }); + const file = hidden.files[0]!; + expect(file.additionLines).toEqual(shown.files[0]!.additionLines); + expect(file.deletionLines).toEqual(shown.files[0]!.deletionLines); + expect(file.cacheKey).not.toBe(shown.files[0]!.cacheKey); + expect(resolveDiffReviewPosition(hidden.sourceFiles[0]!, 43, "additions")).toEqual({ + kind: "added", + newLine: 43, + }); + expect(resolveDiffReviewPosition(hidden.sourceFiles[0]!, 42, "deletions")).toEqual({ + kind: "deleted", + oldLine: 42, + }); + expect(file.hunks[0]?.hunkContent).toContainEqual({ + type: "context", + lines: 3, + additionLineIndex: 2, + deletionLineIndex: 1, + }); + expect(resolveDiffReviewPosition(file, 41, "additions")).toEqual({ + kind: "added", + newLine: 41, + }); + expect(resolveDiffReviewPosition(file, 43, "additions")).toEqual({ + kind: "context", + oldLine: 42, + newLine: 43, + side: "right", + }); + expect(resolveDiffReviewPosition(file, 42, "deletions")).toEqual({ + kind: "context", + oldLine: 42, + newLine: 43, + side: "left", + }); + expect(file.hunks[1]).toMatchObject({ + additionStart: 82, + deletionStart: 80, + splitLineStart: 7, + unifiedLineStart: 7, + }); + const prefix = "unchanged\n".repeat(39); + const gap = "unchanged\n".repeat(35); + const hydrated = hydratePartialDiff("clone", file, { + oldFile: { + name: file.name, + contents: prefix + file.deletionLines.slice(0, 5).join("") + gap + "const value = 1;\n", + }, + newFile: { + name: file.name, + contents: prefix + file.additionLines.slice(0, 7).join("") + gap + "const value = 2;\n", + }, + }); + expect(getDiffLineStat([hydrated])).toEqual({ additions: 3, deletions: 1 }); + expect(resolveDiffReviewPosition(hydrated, 43, "additions")).toEqual( + resolveDiffReviewPosition(file, 43, "additions"), + ); + }); + + it.each([ + [" const x = 1;\t", "\tconst x=1;", 0, 0], + ["const x = 1;", "const x = 2;", 1, 1], + ["const x = 1;", "const x = 1;\n", 1, 0], + ])("filters whitespace in %j to %j", (before, after, additions, deletions) => { + const patch = [ + "diff --git a/example.ts b/example.ts", + "--- a/example.ts", + "+++ b/example.ts", + `@@ -1 +1,${after.split("\n").length} @@`, + `-${before}`, + ...after.split("\n").map((line) => `+${line}`), + "", + ].join("\n"); + const filtered = getRenderablePatch(patch, "pr", { ignoreWhitespace: true }); + expect(filtered?.kind).toBe("files"); + if (filtered?.kind !== "files") return; + expect(getDiffLineStat(filtered.files)).toEqual({ additions, deletions }); + }); + + it.each(["+", "-"])("keeps %s blank lines without a final newline", (sign) => { + const parsed = getRenderablePatch( + [ + "diff --git a/blank.txt b/blank.txt", + "--- a/blank.txt", + "+++ b/blank.txt", + sign === "+" ? "@@ -0,0 +1 @@" : "@@ -1 +0,0 @@", + `${sign} `, + "\\ No newline at end of file", + ].join("\n"), + "pr", + { ignoreWhitespace: true }, + ); + expect(parsed?.kind).toBe("files"); + if (parsed?.kind !== "files") return; + expect(getDiffLineStat(parsed.files)).toEqual({ + additions: sign === "+" ? 1 : 0, + deletions: sign === "-" ? 1 : 0, + }); + }); + it.each([ ["a/example.ts", "a/example.ts", "change"], ["b/example.ts", "b/example.ts", "change"], diff --git a/apps/web/src/lib/diffRendering.ts b/apps/web/src/lib/diffRendering.ts index 3a6c6e20969a..c99349a7dafd 100644 --- a/apps/web/src/lib/diffRendering.ts +++ b/apps/web/src/lib/diffRendering.ts @@ -1,4 +1,5 @@ import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles"; +import { parseDiffFromFile } from "@pierre/diffs"; import type { FileDiffMetadata } from "@pierre/diffs/types"; import { unquoteGitPatchPath } from "@t3tools/shared/gitPatchPath"; @@ -46,6 +47,7 @@ export type RenderablePatch = | { kind: "files"; files: FileDiffMetadata[]; + sourceFiles: FileDiffMetadata[]; } | { kind: "raw"; @@ -73,6 +75,7 @@ export function getDiffLineStat(files: ReadonlyArray): DiffLin } interface RenderablePatchOptions { + ignoreWhitespace?: boolean; /** * Pierre's partial-patch parser keeps hunk render starts in source-file * coordinates. Its virtualizer iterates partial patches as compact rows, so @@ -82,6 +85,59 @@ interface RenderablePatchOptions { compactPartialHunkOffsets?: boolean; } +function hideWhitespaceChanges(file: FileDiffMetadata): FileDiffMetadata { + let splitDelta = 0; + let unifiedDelta = 0; + const hunks = file.hunks.map((hunk) => { + const oldContents = file.deletionLines + .slice(hunk.deletionLineIndex, hunk.deletionLineIndex + hunk.deletionCount) + .map((line) => `${line.replace(/\s/g, "")}\n`) + .join(""); + const newContents = file.additionLines + .slice(hunk.additionLineIndex, hunk.additionLineIndex + hunk.additionCount) + .map((line) => `${line.replace(/\s/g, "")}\n`) + .join(""); + const filtered = parseDiffFromFile( + { name: file.name, contents: oldContents }, + { name: file.name, contents: newContents }, + { context: Infinity }, + ).hunks[0]; + const next = { + ...hunk, + additionLines: filtered?.additionLines ?? 0, + deletionLines: filtered?.deletionLines ?? 0, + hunkContent: filtered + ? filtered.hunkContent.map((content) => ({ + ...content, + additionLineIndex: content.additionLineIndex + hunk.additionLineIndex, + deletionLineIndex: content.deletionLineIndex + hunk.deletionLineIndex, + })) + : [ + { + type: "context" as const, + lines: hunk.additionCount, + additionLineIndex: hunk.additionLineIndex, + deletionLineIndex: hunk.deletionLineIndex, + }, + ], + splitLineStart: hunk.splitLineStart + splitDelta, + unifiedLineStart: hunk.unifiedLineStart + unifiedDelta, + splitLineCount: filtered?.splitLineCount ?? hunk.additionCount, + unifiedLineCount: filtered?.unifiedLineCount ?? hunk.additionCount, + }; + splitDelta += next.splitLineCount - hunk.splitLineCount; + unifiedDelta += next.unifiedLineCount - hunk.unifiedLineCount; + return next; + }); + return { + ...file, + hunks, + splitLineCount: file.splitLineCount + splitDelta, + unifiedLineCount: file.unifiedLineCount + unifiedDelta, + ...(file.cacheKey ? { cacheKey: `${file.cacheKey}:ignore-whitespace` } : {}), + }; +} + function compactPartialHunkOffsets(file: FileDiffMetadata): FileDiffMetadata { if (!file.isPartial) return file; @@ -121,13 +177,13 @@ export function getRenderablePatch( normalizedPatch, buildPatchCacheKey(normalizedPatch, cacheScope), ); - const files = parsedPatches.flatMap((parsedPatch) => - options.compactPartialHunkOffsets - ? parsedPatch.files.map(compactPartialHunkOffsets) - : parsedPatch.files, - ); + const sourceFiles = parsedPatches.flatMap((parsedPatch) => parsedPatch.files); + const files = sourceFiles.map((file) => { + const filtered = options.ignoreWhitespace ? hideWhitespaceChanges(file) : file; + return options.compactPartialHunkOffsets ? compactPartialHunkOffsets(filtered) : filtered; + }); if (files.length > 0) { - return { kind: "files", files }; + return { kind: "files", files, sourceFiles }; } return { From 0ff87f251dafb32703d5f531e7b248ad0a581ee2 Mon Sep 17 00:00:00 2001 From: oliver <97427849+flamboh@users.noreply.github.com> Date: Sun, 20 Sep 2026 05:55:11 -0700 Subject: [PATCH 2/2] fix(web): keep citation comment when popover is dismissed (#10831) Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com> --- .../chat/AssistantCitationChip.test.tsx | 156 ++++++++++++++++++ .../components/chat/AssistantCitationChip.tsx | 61 +++++-- .../chat/AssistantCitationCommentEditor.tsx | 7 +- .../assistantCitationCommentDismissal.test.ts | 80 +++++++++ .../chat/assistantCitationCommentDismissal.ts | 21 +++ 5 files changed, 314 insertions(+), 11 deletions(-) create mode 100644 apps/web/src/components/chat/AssistantCitationChip.test.tsx create mode 100644 apps/web/src/components/chat/assistantCitationCommentDismissal.test.ts create mode 100644 apps/web/src/components/chat/assistantCitationCommentDismissal.ts diff --git a/apps/web/src/components/chat/AssistantCitationChip.test.tsx b/apps/web/src/components/chat/AssistantCitationChip.test.tsx new file mode 100644 index 000000000000..7d664f237983 --- /dev/null +++ b/apps/web/src/components/chat/AssistantCitationChip.test.tsx @@ -0,0 +1,156 @@ +import { + ASSISTANT_CITATION_MAX_COMMENT_LENGTH, + EnvironmentId, + MessageId, + ThreadId, +} from "@t3tools/contracts"; +import { act, useState, type ReactNode } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import type { AssistantCitationSourceAnchor } from "~/lib/assistantTextSelection"; + +const mocks = vi.hoisted(() => ({ observeSource: vi.fn(), dispose: vi.fn() })); +vi.mock("./AssistantCitationSource", () => ({ + observeAssistantCitationCommentSource: mocks.observeSource, +})); +vi.mock("@tanstack/react-router", () => ({ + useNavigate: () => vi.fn(), + Link: ({ children }: { children: ReactNode }) => {children}, +})); +// Keep the real chip/editor lifecycle while replacing DOM positioning and floating layers. +vi.mock("../ui/tooltip", () => ({ + Tooltip: ({ children }: { children: ReactNode }) => <>{children}, + TooltipTrigger: ({ render }: { render: ReactNode }) => render, + TooltipPopup: ({ children }: { children: ReactNode }) => <>{children}, +})); +vi.mock("../ui/popover", () => ({ + Popover: ({ children }: { children: ReactNode }) => <>{children}, + PopoverTrigger: ({ children }: { children: ReactNode }) => , + PopoverPopup: ({ children }: { children: ReactNode }) => <>{children}, +})); +vi.mock("../ui/button", () => ({ + Button: (props: React.ComponentProps<"button">) =>