fix(diff): skip binary file contents in reviews

[benvinegar]

Summary

• avoid reading full file contents for direct file diffs when either side looks binary
• mark git and file-pair binary diffs as binary so the UI shows a placeholder instead of trying to render hunks
• add regression coverage for binary file loading and placeholder rendering

Testing

• bun run typecheck
• bun run lint
• bun test

This PR description was generated by Pi using OpenAI o3

[greptile-apps]

Greptile Summary

This PR adds binary file detection to skip reading binary file contents in direct file diffs and to mark binary diffs (both file-pair and git-sourced) so the UI renders a placeholder instead of attempting to decode non-textual content. A new src/core/binary.ts module centralises detection heuristics (isProbablyBinaryFile, patchLooksBinary) and the placeholder builder (createSkippedBinaryMetadata), which are consumed by the loaders and both rendering paths.

Confidence Score: 5/5

Safe to merge; the single finding is a naming-only P2 with no functional impact.

All logic is correct — binary detection fires before file reads, the isBinary flag propagates through both the loader and the external HunkDiffView paths, diffMessage ordering is sound, and regression tests cover both code paths. The only open finding is a misleading internal function name that has zero runtime impact.

src/core/binary.ts — minor naming issue only.

Important Files Changed

Filename	Overview
src/core/binary.ts	New binary-detection module; logic is sound but isSuspiciousTextByte is named inversely to what it detects (binary-signal bytes, not text bytes).
src/core/loaders.ts	Binary guard inserted before file reads; buildDiffFile gains an isBinary option and falls back to patchLooksBinary — both paths are correctly wired.
src/core/types.ts	Adds optional isBinary field to DiffFile; backwards-compatible.
src/ui/diff/renderRows.tsx	Adds "Binary file skipped" branch in diffMessage; placed after rename-pure check, which is correct since pure renames carry no binary content.
src/opentui/HunkDiffView.tsx	Correctly derives isBinary from patchLooksBinary for the public HunkDiffFile-to-internal-DiffFile conversion path.
src/core/loaders.test.ts	Adds two new tests covering binary file-pair and git binary diffs; assertions are complete and match the expected shape.
src/ui/components/ui-components.test.tsx	Adds a render test that verifies "Binary file skipped" appears in the frame output; createEmptyDiffFile type union correctly extended to include "change".

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[loadFileDiffChangeset] --> B{isProbablyBinaryFile\nleft or right?}
    B -- yes --> C[buildBinaryFileDiffChangeset]
    C --> D[createSkippedBinaryMetadata\ntype: new/change/deleted]
    D --> E[buildDiffFile\nisBinary: true explicit]
    B -- no --> F[Bun.file.text x2\nread full contents]
    F --> G[parseDiffFromFile\ncreateTwoFilesPatch]
    G --> H[buildDiffFile\nisBinary: patchLooksBinary fallback]

    I[loadGitChangeset] --> J[normalizePatchChangeset]
    J --> K[buildDiffFile\nisBinary: patchLooksBinary fallback]

    E --> L[DiffFile.isBinary = true]
    H --> L
    K --> L

    L --> M{diffMessage}
    M -- rename-pure --> N[rename message]
    M -- isBinary --> O[Binary file skipped]
    M -- new/deleted/other --> P[type-specific message]

    Q[HunkDiffView toInternalDiffFile] --> R[patchLooksBinary on patch]
    R --> L

Loading

_{Reviews (1): Last reviewed commit: "fix(diff): skip binary file contents in ..." | Re-trigger Greptile}

[greptile-apps]

Misleading function name

isSuspiciousTextByte reads as "this byte looks like a suspicious text byte," but the function returns true for control-character ranges that signal binary content — the exact opposite of what the name implies. The JSDoc even says "strong binary signal instead of normal text content," which contradicts the identifier. A name like isBinarySignalByte or looksLikeBinaryByte would match both the doc and the intent.

Suggested change

	function isSuspiciousTextByte(byte: number) {
	return byte < 0x07 || (byte > 0x0d && byte < 0x20) || byte === 0x7f;
	}
	/** Return whether one byte is a strong binary signal instead of normal text content. */
	function isBinarySignalByte(byte: number) {

The call site on line 74 would also need updating to isBinarySignalByte(byte).

[benvinegar]

Good catch. I renamed the helper to isBinarySignalByte, updated the call site, and renamed the counter to binarySignalBytes for consistency. I also re-ran bun run typecheck and bun test src/core/loaders.test.ts.

This comment was generated by Pi using OpenAI o3