fix(web): explain binary files in PR code diffs

apps/web/src/components/pullRequest/PullRequestCodeTab.tsx

@@ -36,6 +36,8 @@ import {
   ChevronsDownUpIcon,
   ChevronsUpDownIcon,
 } from "lucide-react";
+import { sha256 } from "@noble/hashes/sha2";
+import * as Hex from "effect/encoding/Hex";
 import { useAtomRefresh } from "@effect/atom-react";
 import * as Schema from "effect/Schema";
 import {
@@ -66,6 +68,7 @@ import { fileDiffRows } from "@t3tools/client-runtime/review-guide-tour";
 import { isTestChangedFile } from "@t3tools/shared/changedFileKind";
 import { orderDiffFiles } from "./pullRequestFileOrder.logic";
 import {
+  buildFileDiffIdentityKey,
   buildFileDiffRenderKey,
   fnv1a32,
   getDiffLineStat,
@@ -192,6 +195,7 @@ function parseSlice(slice: DiffSlice, cacheKey: string, ignoreWhitespace: boolea
 type DiffModel = {
   readonly parsedSlices: ReadonlyArray<RenderablePatch | null>;
   readonly loadedFiles: ReadonlyArray<Extract<RenderablePatch, { kind: "files" }>["files"][number]>;
+  readonly binaryFileKeys: ReadonlySet<string>;
   readonly sourceFileStats: ReadonlyMap<string, ReturnType<typeof getDiffLineStat>>;
 };
 const diffModels = new WeakMap<ReadonlyArray<DiffSlice>, Map<string, DiffModel>>();
@@ -222,6 +226,11 @@ function diffModel(
         parsed?.kind === "files" ? orderDiffFiles(parsed.files) : [],
       ),
     ),
+    binaryFileKeys: new Set(
+      parsedSlices.flatMap((parsed) =>
+        parsed?.kind === "files" ? [...parsed.binaryFileKeys] : [],
+      ),
+    ),
     sourceFileStats: new Map(
       parsedSlices.flatMap((parsed) =>
         parsed?.kind === "files"
@@ -583,7 +592,7 @@ function PullRequestCodeTab({
   // Agent context is local, so it does not require host comment permission.
   const canSelectLines =
     commit === null && (review.inlineComment || onAddToAgentSelection !== undefined);
-  const { parsedSlices, loadedFiles, sourceFileStats } = diffModel(
+  const { parsedSlices, loadedFiles, binaryFileKeys, sourceFileStats } = diffModel(
     loadedSlices,
     `pull-request:${scopeKey}:${resolvedTheme}:${ignoreWhitespace}`,
     ignoreWhitespace,
@@ -760,12 +769,10 @@ function PullRequestCodeTab({
     foldOverride === null && filesViewedEnabled && isFileViewed(path) && !isFileViewedStale(path);
   const items: CodeViewDiffItem<ReviewAnnotationGroup>[] = annotatedFiles.map(
     ({ fileKey, path, fileDiff, annotations, annotationsVersion }) => {
-      const collapsed = isFileDiffCollapsed(
-        fileKey,
-        effectiveFoldOverride,
-        foldChoices,
-        foldsAsViewed(path),
-      );
+      // A binary diff has no body to show, so its header is the whole row.
+      const collapsed =
+        binaryFileKeys.has(buildFileDiffIdentityKey(fileDiff)) ||
+        isFileDiffCollapsed(fileKey, effectiveFoldOverride, foldChoices, foldsAsViewed(path));
       const viewedMark = filesViewedEnabled
         ? `e${isFileViewed(path) ? "v" : ""}${isFileViewedStale(path) ? "s" : ""}`
         : "";
@@ -932,6 +939,9 @@ function PullRequestCodeTab({
     );
 
   const renderHeaderPrefix = (item: CodeViewItem<ReviewAnnotationGroup>) => {
+    if (item.type === "diff" && binaryFileKeys.has(buildFileDiffIdentityKey(item.fileDiff))) {
+      return <span aria-hidden className="mr-1 size-5 shrink-0" />;
+    }
     const collapsed = item.collapsed === true;
     return (
       <Button
@@ -962,16 +972,35 @@ function PullRequestCodeTab({
       const withheld = omittedFileStats.get(path);
       if (withheld) ({ additions, deletions } = withheld);
     }
+    const binary = binaryFileKeys.has(buildFileDiffIdentityKey(item.fileDiff));
+    const host = sourceControlClients.get(detail.provider);
+    const fileUrl =
+      binary && detail.provider === "github"
+        ? `${detail.url}/files#diff-${Hex.encode(sha256(new TextEncoder().encode(path)))}`
+        : detail.url;
     return (
       <FileViewedToggle
         fileKey={item.id}
         path={path}
         stat={
-          <PullRequestDiffStat
-            additions={additions}
-            deletions={deletions}
-            className="font-mono text-2xs"
-          />
+          binary ? (
+            <span className="flex items-center gap-2 text-xs text-muted-foreground">
+              <span>Binary file</span>
+              <Button
+                size="micro"
+                variant="link"
+                render={<a href={fileUrl} target="_blank" rel="noopener noreferrer" />}
+              >
+                View on {host.label}
+              </Button>
+            </span>
+          ) : (
+            <PullRequestDiffStat
+              additions={additions}
+              deletions={deletions}
+              className="font-mono text-2xs"
+            />
+          )
         }
       />
     );

apps/web/src/lib/diffRendering.test.ts

@@ -12,6 +12,64 @@ import {
 } from "./diffRendering";
 
 describe("getRenderablePatch", () => {
+  it("retains explicit binary markers without classifying other hunkless files as binary", () => {
+    const patch = [
+      "diff --git a/added.png b/added.png",
+      "new file mode 100644",
+      "index 0000000..1234567",
+      "Binary files /dev/null and b/added.png differ",
+      'diff --git "a/old image.png" "b/new image.png"',
+      "similarity index 50%",
+      "rename from old image.png",
+      "rename to new image.png",
+      'Binary files "a/old image.png" and "b/new image.png" differ',
+      "diff --git a/deleted.png b/deleted.png",
+      "deleted file mode 100644",
+      "Binary files a/deleted.png and /dev/null differ",
+      "diff --git a/payload.bin b/payload.bin",
+      "GIT binary patch",
+      "literal 3",
+      "KcmZQzWC8#H2mk;8",
+      "diff --git a/empty.txt b/empty.txt",
+      "new file mode 100644",
+      "index 0000000..e69de29",
+      "diff --git a/old.txt b/new.txt",
+      "similarity index 100%",
+      "rename from old.txt",
+      "rename to new.txt",
+      "diff --git a/omitted.txt b/omitted.txt",
+      "index 1111111..2222222 100644",
+      "diff --git a/lfs.png b/lfs.png",
+      "new file mode 100644",
+      "--- /dev/null",
+      "+++ b/lfs.png",
+      "@@ -0,0 +1,3 @@",
+      "+version https://git-lfs.github.com/spec/v1",
+      "+oid sha256:1234567890",
+      "+size 123",
+      "diff --git a/marker.txt b/marker.txt",
+      "--- a/marker.txt",
+      "+++ b/marker.txt",
+      "@@ -1 +1 @@",
+      "-Binary files a/fake.png and b/fake.png differ",
+      "+GIT binary patch",
+      "",
+    ].join("\n");
+    for (const ignoreWhitespace of [false, true]) {
+      const parsed = getRenderablePatch(patch, "pr-page-1", {
+        ignoreWhitespace,
+        compactPartialHunkOffsets: true,
+      });
+      expect(parsed?.kind).toBe("files");
+      if (parsed?.kind !== "files") return;
+      expect(parsed.files).toHaveLength(9);
+      expect(parsed.files.slice(0, 7).every((file) => file.hunks.length === 0)).toBe(true);
+      expect(parsed.binaryFileKeys).toEqual(
+        new Set(parsed.files.slice(0, 4).map(buildFileDiffIdentityKey)),
+      );
+    }
+  });
+
   it("hides indentation changes around inserted JSX without moving review lines", () => {
     const patch = [
       "diff --git a/item.tsx b/item.tsx",

apps/web/src/lib/diffRendering.ts

@@ -1,4 +1,4 @@
-import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles";
+import { parsePatchFiles, processFile } from "@pierre/diffs/utils/parsePatchFiles";
 import { parseDiffFromFile } from "@pierre/diffs";
 import type { FileDiffMetadata } from "@pierre/diffs/types";
 import { READABLE_LIGHT_THEME_NAME } from "./readableSyntaxTheme";
@@ -44,6 +44,7 @@ export type RenderablePatch =
       kind: "files";
       files: FileDiffMetadata[];
       sourceFiles: FileDiffMetadata[];
+      binaryFileKeys: ReadonlySet<string>;
     }
   | {
       kind: "raw";
@@ -171,7 +172,16 @@ export function getRenderablePatch(
       return options.compactPartialHunkOffsets ? compactPartialHunkOffsets(filtered) : filtered;
     });
     if (files.length > 0) {
-      return { kind: "files", files, sourceFiles };
+      const binaryFileKeys = new Set<string>();
+      // Pierre discards binary markers, so retain them before handing its metadata to the viewer.
+      for (const section of patch.split(/(?=^diff --git )/m)) {
+        if (!section.startsWith("diff --git ")) continue;
+        const header = section.split("\n@@ ", 1)[0]!;
+        if (!/^(?:Binary files .+ and .+ differ|GIT binary patch)\r?$/m.test(header)) continue;
+        const file = processFile(header);
+        if (file) binaryFileKeys.add(buildFileDiffIdentityKey(file));
+      }
+      return { kind: "files", files, sourceFiles, binaryFileKeys };
     }
 
     return {

packages/source-control-github/src/server/gitHubPullRequestJson.test.ts

@@ -1644,6 +1644,38 @@ describe("decodePullRequestFilesJson", () => {
     expect(result.truncated).toBe(true);
     expect(result.rawCount).toBe(2);
   });
+
+  it("marks a patchless, countless change as binary unless it is the empty blob", () => {
+    const result = expectSuccess(
+      decodePullRequestFilesJson(
+        JSON.stringify([
+          { filename: "icon.png", status: "added", additions: 0, deletions: 0, sha: "3e26" },
+          {
+            filename: "empty.txt",
+            status: "added",
+            additions: 0,
+            deletions: 0,
+            sha: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
+          },
+        ]),
+      ),
+    );
+
+    expect(result.patch).toBe(
+      [
+        "diff --git a/icon.png b/icon.png",
+        "new file mode 100644",
+        "--- /dev/null",
+        "+++ b/icon.png",
+        "Binary files /dev/null and b/icon.png differ",
+        "diff --git a/empty.txt b/empty.txt",
+        "new file mode 100644",
+        "--- /dev/null",
+        "+++ b/empty.txt",
+        "",
+      ].join("\n"),
+    );
+  });
 });
 
 describe("decodePullRequestFilesViewedJson", () => {

packages/source-control-github/src/server/gitHubPullRequestJson.ts

@@ -229,6 +229,7 @@ const RawPullRequestFileSchema = Schema.Struct({
   patch: Schema.optional(Schema.NullOr(Schema.String)),
   additions: Schema.optional(Schema.NullOr(Schema.Int)),
   deletions: Schema.optional(Schema.NullOr(Schema.Int)),
+  sha: Schema.optional(Schema.NullOr(Schema.String)),
 });
 
 export function toPullRequestPreview(
@@ -1799,6 +1800,8 @@ const decodeCommitFiles = decodeJsonResult(
   Schema.Struct({ files: Schema.optional(Schema.NullOr(Schema.Array(Schema.Unknown))) }),
 );
 
+const EMPTY_BLOB_SHA = "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391";
+
 function toFilesPatch(entries: ReadonlyArray<unknown>): GitHubPullRequestFilesPatch {
   const sections: string[] = [];
   const omittedFileStats: PullRequestOmittedFileStat[] = [];
@@ -1820,6 +1823,15 @@ function toFilesPatch(entries: ReadonlyArray<unknown>): GitHubPullRequestFilesPa
     }
     const oldPath =
       status === "renamed" ? value.previous_filename || value.filename : value.filename;
+    const from = status === "added" ? "/dev/null" : quoteGitPatchPath(`a/${oldPath}`);
+    const to = status === "removed" ? "/dev/null" : quoteGitPatchPath(`b/${value.filename}`);
+    // The files API sends a binary change as a patchless, countless entry; only the empty blob
+    // looks the same, so restore git's marker for the viewer.
+    const binary =
+      hunks.length === 0 &&
+      (value.additions ?? 0) + (value.deletions ?? 0) === 0 &&
+      (status === "added" || status === "removed" || status === "modified") &&
+      value.sha !== EMPTY_BLOB_SHA;
     const header = [
       `diff --git ${quoteGitPatchPath(`a/${oldPath}`)} ${quoteGitPatchPath(`b/${value.filename}`)}`,
       ...(status === "added" ? ["new file mode 100644"] : []),
@@ -1832,8 +1844,9 @@ function toFilesPatch(entries: ReadonlyArray<unknown>): GitHubPullRequestFilesPa
             `rename to ${quoteGitPatchPath(value.filename)}`,
           ]
         : []),
-      `--- ${status === "added" ? "/dev/null" : quoteGitPatchPath(`a/${oldPath}`)}`,
-      `+++ ${status === "removed" ? "/dev/null" : quoteGitPatchPath(`b/${value.filename}`)}`,
+      `--- ${from}`,
+      `+++ ${to}`,
+      ...(binary ? [`Binary files ${from} and ${to} differ`] : []),
     ].join("\n");
     sections.push(hunks.length === 0 ? `${header}\n` : `${header}\n${hunks.replace(/\n?$/, "\n")}`);
   }