[codex] Handle binary and video file changes

apps/desktop/src/lib/trpc/routers/changes/utils/apply-numstat.ts

@@ -18,6 +18,7 @@ export async function applyNumstatToFiles(
 			if (fileStat) {
 				file.additions = fileStat.additions;
 				file.deletions = fileStat.deletions;
+				file.isBinary = fileStat.isBinary;
 			}
 		}
 	} catch {}

apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts

@@ -98,8 +98,16 @@ describe("parseDiffNumstat", () => {
 
 		const stats = parseDiffNumstat(numstatOutput);
 
-		expect(stats.get("src/file1.ts")).toEqual({ additions: 10, deletions: 5 });
-		expect(stats.get("src/file2.ts")).toEqual({ additions: 20, deletions: 3 });
+		expect(stats.get("src/file1.ts")).toEqual({
+			additions: 10,
+			deletions: 5,
+			isBinary: false,
+		});
+		expect(stats.get("src/file2.ts")).toEqual({
+			additions: 20,
+			deletions: 3,
+			isBinary: false,
+		});
 	});
 
 	test("handles binary files with dash markers", () => {
@@ -108,8 +116,16 @@ describe("parseDiffNumstat", () => {
 
 		const stats = parseDiffNumstat(numstatOutput);
 
-		expect(stats.get("image.png")).toEqual({ additions: 0, deletions: 0 });
-		expect(stats.get("src/code.ts")).toEqual({ additions: 10, deletions: 5 });
+		expect(stats.get("image.png")).toEqual({
+			additions: 0,
+			deletions: 0,
+			isBinary: true,
+		});
+		expect(stats.get("src/code.ts")).toEqual({
+			additions: 10,
+			deletions: 5,
+			isBinary: false,
+		});
 	});
 
 	test("handles renamed files with arrow format", () => {
@@ -118,26 +134,50 @@ describe("parseDiffNumstat", () => {
 		const stats = parseDiffNumstat(numstatOutput);
 
 		// Should be accessible by both old and new paths
-		expect(stats.get("new/path.ts")).toEqual({ additions: 5, deletions: 2 });
-		expect(stats.get("old/path.ts")).toEqual({ additions: 5, deletions: 2 });
+		expect(stats.get("new/path.ts")).toEqual({
+			additions: 5,
+			deletions: 2,
+			isBinary: false,
+		});
+		expect(stats.get("old/path.ts")).toEqual({
+			additions: 5,
+			deletions: 2,
+			isBinary: false,
+		});
 	});
 
 	test("handles copied files with arrow format", () => {
 		const numstatOutput = `0	0	source.ts => copy.ts`;
 
 		const stats = parseDiffNumstat(numstatOutput);
 
-		expect(stats.get("copy.ts")).toEqual({ additions: 0, deletions: 0 });
-		expect(stats.get("source.ts")).toEqual({ additions: 0, deletions: 0 });
+		expect(stats.get("copy.ts")).toEqual({
+			additions: 0,
+			deletions: 0,
+			isBinary: false,
+		});
+		expect(stats.get("source.ts")).toEqual({
+			additions: 0,
+			deletions: 0,
+			isBinary: false,
+		});
 	});
 
 	test("handles paths with spaces in rename format", () => {
 		const numstatOutput = `3	1	old file.ts => new file.ts`;
 
 		const stats = parseDiffNumstat(numstatOutput);
 
-		expect(stats.get("new file.ts")).toEqual({ additions: 3, deletions: 1 });
-		expect(stats.get("old file.ts")).toEqual({ additions: 3, deletions: 1 });
+		expect(stats.get("new file.ts")).toEqual({
+			additions: 3,
+			deletions: 1,
+			isBinary: false,
+		});
+		expect(stats.get("old file.ts")).toEqual({
+			additions: 3,
+			deletions: 1,
+			isBinary: false,
+		});
 	});
 
 	test("returns empty map for empty input", () => {
@@ -153,15 +193,23 @@ describe("parseDiffNumstat", () => {
 		const stats = parseDiffNumstat(numstatOutput);
 
 		expect(stats.size).toBe(1);
-		expect(stats.get("valid/path.ts")).toEqual({ additions: 20, deletions: 3 });
+		expect(stats.get("valid/path.ts")).toEqual({
+			additions: 20,
+			deletions: 3,
+			isBinary: false,
+		});
 	});
 
 	test("handles non-numeric additions/deletions gracefully", () => {
 		const numstatOutput = `abc	xyz	file.ts`;
 
 		const stats = parseDiffNumstat(numstatOutput);
 
-		expect(stats.get("file.ts")).toEqual({ additions: 0, deletions: 0 });
+		expect(stats.get("file.ts")).toEqual({
+			additions: 0,
+			deletions: 0,
+			isBinary: false,
+		});
 	});
 });
 

apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts

@@ -118,8 +118,11 @@ export function parseGitLog(logOutput: string): CommitInfo[] {
 
 export function parseDiffNumstat(
 	numstatOutput: string,
-): Map<string, { additions: number; deletions: number }> {
-	const stats = new Map<string, { additions: number; deletions: number }>();
+): Map<string, { additions: number; deletions: number; isBinary: boolean }> {
+	const stats = new Map<
+		string,
+		{ additions: number; deletions: number; isBinary: boolean }
+	>();
 
 	for (const line of numstatOutput.trim().split("\n")) {
 		if (!line.trim()) continue;
@@ -132,7 +135,11 @@ export function parseDiffNumstat(
 
 		const additions = addStr === "-" ? 0 : Number.parseInt(addStr, 10) || 0;
 		const deletions = delStr === "-" ? 0 : Number.parseInt(delStr, 10) || 0;
-		const statEntry = { additions, deletions };
+		const statEntry = {
+			additions,
+			deletions,
+			isBinary: addStr === "-" && delStr === "-",
+		};
 
 		const renameMatch = rawPath.match(/^(.+) => (.+)$/);
 		if (renameMatch) {

apps/desktop/src/lib/trpc/routers/changes/workers/git-task-handlers.ts

@@ -1,5 +1,9 @@
-import { readFile, realpath, stat } from "node:fs/promises";
+import { open, realpath, stat } from "node:fs/promises";
 import { isAbsolute, relative, resolve, sep } from "node:path";
+import {
+	BINARY_SNIFF_BYTES,
+	isBinaryMediaFile,
+} from "@superset/shared/media-files";
 import type { ChangedFile, GitChangesStatus } from "shared/changes-types";
 import type { SimpleGit, StatusResult } from "simple-git";
 import { getStatusNoLock } from "../../workspaces/utils/git";
@@ -30,6 +34,9 @@ interface TrackingStatus {
 }
 
 const MAX_LINE_COUNT_SIZE = 1 * 1024 * 1024;
+// Chunk size for streaming untracked files when counting lines, so a file is
+// never fully held in memory. Comfortably covers the binary sniff window.
+const LINE_COUNT_CHUNK_SIZE = 64 * 1024;
 const WORKER_DEBUG = process.env.SUPERSET_WORKER_DEBUG === "1";
 
 function logWorkerWarning(message: string, error: unknown): void {
@@ -89,17 +96,63 @@ async function applyUntrackedLineCount(
 			if (!isPathWithinWorktree(worktreeReal, fileReal)) continue;
 
 			const stats = await stat(fileReal);
-			if (!stats.isFile() || stats.size > MAX_LINE_COUNT_SIZE) continue;
-
-			const content = await readFile(fileReal, "utf-8");
-			const lineCount =
-				content === ""
-					? 0
-					: content.endsWith("\n")
-						? content.split(/\r?\n/).length - 1
-						: content.split(/\r?\n/).length;
-			file.additions = lineCount;
-			file.deletions = 0;
+			if (!stats.isFile()) continue;
+
+			if (isBinaryMediaFile(file.path)) {
+				file.isBinary = true;
+				file.additions = 0;
+				file.deletions = 0;
+				continue;
+			}
+
+			// Stream in fixed chunks rather than slurping: readFile would pin
+			// the whole file in memory and, decoded as utf-8, would turn binary
+			// into U+FFFDs and report a bogus line count. Reuse one buffer,
+			// sniff the first 8KB for NULs (git's binary heuristic), and tally
+			// newlines as we go.
+			const handle = await open(fileReal, "r");
+			try {
+				const buf = Buffer.allocUnsafe(LINE_COUNT_CHUNK_SIZE);
+				let { bytesRead } = await handle.read(buf, 0, buf.length, 0);
+
+				const sniffEnd = Math.min(bytesRead, BINARY_SNIFF_BYTES);
+				let isBinary = false;
+				for (let i = 0; i < sniffEnd; i++) {
+					if (buf[i] === 0) {
+						isBinary = true;
+						break;
+					}
+				}
+				if (isBinary) {
+					file.isBinary = true;
+					file.additions = 0;
+					file.deletions = 0;
+					continue;
+				}
+
+				// Over the budget: skip the LOC signal without reading the rest.
+				if (stats.size > MAX_LINE_COUNT_SIZE) continue;
+
+				let newlines = 0;
+				let lastByte = -1;
+				let offset = 0;
+				while (bytesRead > 0) {
+					for (let i = 0; i < bytesRead; i++) {
+						if (buf[i] === 0x0a) newlines++;
+					}
+					lastByte = buf[bytesRead - 1] ?? lastByte;
+					offset += bytesRead;
+					({ bytesRead } = await handle.read(buf, 0, buf.length, offset));
+				}
+				// Match `content.split(/\r?\n/)`: a trailing newline doesn't add
+				// a line, but a final non-empty line without one does. (\r\n
+				// shares the \n, so counting \n bytes is equivalent.)
+				file.additions =
+					lastByte === -1 ? 0 : lastByte === 0x0a ? newlines : newlines + 1;
+				file.deletions = 0;
+			} finally {
+				await handle.close();
+			}
 		} catch (error) {
 			logWorkerDebug(
 				`failed untracked line count for "${file.path}" in "${worktreePath}"`,

apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/types.ts

@@ -17,5 +17,6 @@ export interface ChangesetFile {
 	status: FileStatus;
 	additions: number;
 	deletions: number;
+	isBinary?: boolean;
 	source: DiffFileSource;
 }

apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useChangeset/useChangeset.ts

@@ -43,6 +43,7 @@ export function useChangeset({
 				status: file.status as FileStatus,
 				additions: file.additions,
 				deletions: file.deletions,
+				isBinary: file.isBinary,
 				source: {
 					kind: "commit",
 					commitHash: ref.commitHash,
@@ -62,6 +63,7 @@ export function useChangeset({
 					status: file.status as FileStatus,
 					additions: file.additions,
 					deletions: file.deletions,
+					isBinary: file.isBinary,
 					source: { kind: "unstaged" },
 				})),
 				...status.staged.map<ChangesetFile>((file) => ({
@@ -70,6 +72,7 @@ export function useChangeset({
 					status: file.status as FileStatus,
 					additions: file.additions,
 					deletions: file.deletions,
+					isBinary: file.isBinary,
 					source: { kind: "staged" },
 				})),
 			];
@@ -86,6 +89,7 @@ export function useChangeset({
 				status: file.status as FileStatus,
 				additions: file.additions,
 				deletions: file.deletions,
+				isBinary: file.isBinary,
 				source: { kind: "unstaged" },
 			});
 		}
@@ -97,6 +101,7 @@ export function useChangeset({
 				status: file.status as FileStatus,
 				additions: file.additions,
 				deletions: file.deletions,
+				isBinary: file.isBinary,
 				source: { kind: "staged" },
 			});
 		}
@@ -108,6 +113,7 @@ export function useChangeset({
 				status: file.status as FileStatus,
 				additions: file.additions,
 				deletions: file.deletions,
+				isBinary: file.isBinary,
 				source: { kind: "against-base", baseBranch: ref.baseBranch },
 			});
 		}

apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsx

@@ -5,7 +5,9 @@ import type {
 } from "@pierre/diffs";
 import { CodeView, type CodeViewHandle } from "@pierre/diffs/react";
 import type { RendererContext } from "@superset/panes";
+import { Button } from "@superset/ui/button";
 import { useCallback, useMemo, useRef } from "react";
+import { LuFileCode } from "react-icons/lu";
 import type { DiffPaneData, PaneViewerData } from "../../../../types";
 import { type ChangesetFile, useChangeset } from "../../../useChangeset";
 import { useOpenInExternalEditor } from "../../../useOpenInExternalEditor";
@@ -181,9 +183,15 @@ export function DiffPane({
 				| DiffLineAnnotation<DiffAnnotationMetadata>,
 			item: CodeViewItem<DiffAnnotationMetadata>,
 		) => {
-			if (item.type !== "diff") return null;
 			const m = annotation.metadata;
+			if (m.kind === "binary-placeholder") {
+				if (item.type !== "file") return null;
+				const file = fileByItemId.get(item.id);
+				if (!file) return null;
+				return <BinaryDiffPlaceholder file={file} onOpenFile={onOpenFile} />;
+			}
 			if (m.kind === "composer") {
+				if (item.type !== "diff") return null;
 				return (
 					<AgentCommentComposer
 						workspaceId={workspaceId}
@@ -194,11 +202,13 @@ export function DiffPane({
 					/>
 				);
 			}
+			if (m.kind !== "thread") return null;
 			const annotationSide = "side" in annotation ? annotation.side : undefined;
+			const focusLine = m.sourceLine ?? annotation.lineNumber;
 			const focused =
 				item.id === targetItemId &&
 				data.focusLine != null &&
-				annotation.lineNumber === data.focusLine &&
+				focusLine === data.focusLine &&
 				(data.focusSide == null || annotationSide === data.focusSide);
 
 			return (
@@ -221,6 +231,8 @@ export function DiffPane({
 			data.focusTick,
 			clearComposer,
 			submitComposer,
+			fileByItemId,
+			onOpenFile,
 		],
 	);
 
@@ -257,3 +269,29 @@ export function DiffPane({
 		/>
 	);
 }
+
+function BinaryDiffPlaceholder({
+	file,
+	onOpenFile,
+}: {
+	file: ChangesetFile;
+	onOpenFile: (path: string, openInNewTab?: boolean) => void;
+}) {
+	const canOpen = file.status !== "deleted";
+
+	return (
+		<div className="flex flex-col items-center justify-center gap-3 bg-muted/30 py-8 text-muted-foreground">
+			<LuFileCode className="size-8" />
+			<p className="cursor-text select-text text-sm">Binary file hidden</p>
+			{canOpen ? (
+				<Button
+					variant="outline"
+					size="sm"
+					onClick={() => onOpenFile(file.path)}
+				>
+					Open file
+				</Button>
+			) : null}
+		</div>
+	);
+}