fix(loaders): restore a/b prefixes on noprefix patch input

[mo]

Summary

hunk pager and hunk patch show "No files match the current filter." for any input produced by a git diff that ran with diff.noprefix=true. The diff itself reaches Hunk fine — the parser just rejects it.

Repro (in any repo, in a TTY):

git config --global diff.noprefix true
git config --global core.pager 'hunk pager'
git diff HEAD~1..HEAD     # → "No files match the current filter."

Same symptom via the patch path:

git -c diff.noprefix=true diff HEAD~1 | hunk patch -

Root cause

@pierre/diffs processFile() runs:

if (line.startsWith("diff --git")) {
  const [, , prevName, , name] = line.trim().match(ALTERNATE_FILE_NAMES_GIT) ?? [];
  currentFile.name = name.trim();   // TypeError when match is null
  ...
}

ALTERNATE_FILE_NAMES_GIT requires literal a/ and b/ prefixes:

/^diff --git (?:"a\/(.+?)"|a\/(.+?)) (?:"b\/(.+?)"|b\/(.+?))$/

With diff.noprefix=true, git emits diff --git src/x.ts src/x.ts (no a//b/). The regex fails, name is undefined, .trim() throws TypeError, and parsePatchFiles rethrows because Hunk passes throwOnError: true. The outer try/catch in normalizePatchChangeset swallows the throw and returns files: [] — hence the empty review.

Why it only affects the patch path

src/core/git.ts already defends git-backed reviews with DIFF_PREFIX_NORMALIZATION_ARGS:

const DIFF_PREFIX_NORMALIZATION_ARGS = [
  "-c", "diff.noprefix=false",
  "-c", "diff.mnemonicPrefix=false",
  "-c", "diff.srcPrefix=a/",
  "-c", "diff.dstPrefix=b/",
];

That covers hunk diff, hunk show, hunk stash show because Hunk runs git itself. It can't cover the patch path, where the patch text was already produced by an outer git process (the user's core.pager = hunk pager, or a literal git diff | hunk patch).

There is also a hint of this in the existing test suite: loads colorized git patch files like the real pager stdin stream shells out to git diff --no-index --color=always and feeds the result to loadAppBootstrap. On any machine with diff.noprefix=true set globally, that test fails today (Expected length: 1, Received length: 0).

Fix

Add normalizeGitPatchPrefixes() to src/core/loaders.ts and run it inside normalizePatchChangeset before parsePatchFiles. The helper:

• Walks the patch line by line, scoped per diff --git block.
• Detects diff --git headers in noprefix form (quoted or unquoted, single- or multi-token paths) and rewrites them to canonical a/X b/Y.
• Tracks whether the current block actually needed rewriting and only rewrites the corresponding ---/+++ lines in that block. That keeps it safe for paths that legitimately live inside an a/-named directory and were already prefixed (diff --git a/a/inner.ts b/a/inner.ts).
• Leaves /dev/null untouched on the unified-diff lines.
• Leaves the one genuinely ambiguous case alone: a rename of unquoted paths that both contain spaces (diff --git old name.txt new name.txt). Git itself can't unambiguously round-trip that with diff.noprefix=true, so we don't try to.

Tests

Three new tests in src/core/loaders.test.ts exercise the patch path directly via kind: "patch":

• noprefix non-rename input parses to one file.
• noprefix rename input parses to one renamed file with previousPath recovered.
• already-prefixed input where the path lives in an a/-named directory is preserved unchanged.

The previously-failing loads colorized git patch files like the real pager stdin stream test passes on machines that have diff.noprefix=true set globally.

Test plan

• bun run typecheck
• bun test src/core/ — 140 pass, 0 fail
• bun test src/core/loaders.test.ts — 37 pass, 0 fail (3 new)
• bun run lint — 0 warnings, 0 errors
• bun run format
• Manual: git config --global diff.noprefix true && git diff HEAD~3..HEAD | hunk pager now renders the diff instead of the empty-state message.
• CI green

[greptile-apps]

Greptile Summary

This PR fixes the long-standing "No files match" blank-screen for hunk pager and hunk patch when the upstream git diff was run with diff.noprefix=true. The fix pre-processes patch text before it reaches @pierre/diffs to restore the expected a//b/ prefixes.

• Adds normalizeGitPatchPrefixes to loaders.ts, which walks the patch line-by-line, detects noprefix diff --git headers (quoted or unquoted, including space-in-path and rename cases), rewrites them to canonical form, and mirrors the rewrite onto the ---/+++ file-header lines in the same block only.
• Adds three targeted tests covering a plain modify, a pure rename, and an already-prefixed path inside an a/-named directory; the previously-flaky colorized-pager test is now reliably green on machines with diff.noprefix=true set globally.

Confidence Score: 3/5

The core noprefix fix works for the common case, but can corrupt patches from files with -- comment lines (SQL, Lua, Haskell, Ada) when those lines are removed in the diff.

The blockNeedsPrefix flag is never cleared after the +++ file header is processed. Any subsequent diff body line that starts with --- — exactly what a removed -- sql comment produces — gets an a/ prefix prepended, corrupting the patch. The parser rejects it, normalizePatchChangeset swallows the error, and the file disappears from the review. This affects SQL, Lua, Haskell, and Ada files and is a real regression on the patch path.

src/core/loaders.ts around normalizeGitPatchPrefixes — specifically the blockNeedsPrefix lifetime after the +++ header. A companion test covering a noprefix diff that removes a -- comment line would prevent regressions here.

Important Files Changed

Filename	Overview
src/core/loaders.ts	Adds normalizeGitPatchPrefixes and helpers to restore a//b/ prefixes stripped by diff.noprefix=true; the blockNeedsPrefix flag is not cleared after the +++ header, causing diff content lines that start with --- (e.g. removed -- SQL/Lua comments) to be incorrectly rewritten
src/core/loaders.test.ts	Adds three new integration tests for the noprefix normalizer; covers plain modify, pure rename, and already-prefixed a/-directory path — missing a test for file content with -- removed lines that would catch the blockNeedsPrefix lifetime bug

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["patchText input"] --> B["stripTerminalControl + replaceAll CRLF"]
    B --> C["normalizeGitPatchPrefixes"]
    C --> D{"line starts with 'diff --git '?"}
    D -- yes --> E["rewriteGitDiffHeader"]
    E --> F{"changed?"}
    F -- yes --> G["blockNeedsPrefix = true"]
    F -- no --> H["blockNeedsPrefix = false"]
    D -- no --> I{"blockNeedsPrefix && starts with '--- '?"}
    I -- yes --> J["rewriteUnifiedFileLine (prepend a/)"]
    I -- no --> K{"blockNeedsPrefix && starts with '+++ '?"}
    K -- yes --> L["rewriteUnifiedFileLine (prepend b/) — blockNeedsPrefix should be cleared here"]
    K -- no --> M["pass through unchanged"]
    L --> D
    D -- end --> N["parsePatchFiles"]
    N --> O["normalizePatchChangeset → Changeset"]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/core/loaders.ts:201-203
**`blockNeedsPrefix` is never cleared after the `+++` header, corrupting removals of `--` comment lines**

Once the `+++ ` file header is rewritten, `blockNeedsPrefix` stays `true` for the rest of the block. In a unified diff, a removed line whose content starts with `-- ` (SQL/Lua/Haskell/Ada comments are exactly this) produces a diff body line that begins with `--- ` (three dashes + space). This line is indistinguishable from the file header marker by the current `startsWith("--- ")` check, so it gets an `a/` prefix prepended — corrupting the diff before it reaches the parser. A minimal repro: any noprefix diff that removes a SQL comment like `-- drop table` will have a content line `--- drop table`, which becomes `--- a/drop table` after this rewrite. The parser will reject the mangled patch, and `normalizePatchChangeset` will swallow the error and return `files: []`.

Setting `blockNeedsPrefix = false` immediately after the `+++ ` line is processed is sufficient; the `---`/`+++` pair is always the last pair of header lines before the first `@@` hunk marker in any valid git diff output.

```suggestion
      if (blockNeedsPrefix && line.startsWith("+++ ")) {
        blockNeedsPrefix = false;
        return rewriteUnifiedFileLine(line, "+++ ", "b/");
      }
```

_{Reviews (1): Last reviewed commit: "fix(loaders): restore a/b prefixes on no..." | Re-trigger Greptile}

[greptile-apps]

blockNeedsPrefix is never cleared after the +++ header, corrupting removals of -- comment lines

Once the +++ file header is rewritten, blockNeedsPrefix stays true for the rest of the block. In a unified diff, a removed line whose content starts with -- (SQL/Lua/Haskell/Ada comments are exactly this) produces a diff body line that begins with --- (three dashes + space). This line is indistinguishable from the file header marker by the current startsWith("--- ") check, so it gets an a/ prefix prepended — corrupting the diff before it reaches the parser. A minimal repro: any noprefix diff that removes a SQL comment like -- drop table will have a content line --- drop table, which becomes --- a/drop table after this rewrite. The parser will reject the mangled patch, and normalizePatchChangeset will swallow the error and return files: [].

Setting blockNeedsPrefix = false immediately after the +++ line is processed is sufficient; the ---/+++ pair is always the last pair of header lines before the first @@ hunk marker in any valid git diff output.

Suggested change

	if (blockNeedsPrefix && line.startsWith("+++ ")) {
	return rewriteUnifiedFileLine(line, "+++ ", "b/");
	}
	if (blockNeedsPrefix && line.startsWith("+++ ")) {
	blockNeedsPrefix = false;
	return rewriteUnifiedFileLine(line, "+++ ", "b/");
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/core/loaders.ts
Line: 201-203

Comment:
**`blockNeedsPrefix` is never cleared after the `+++` header, corrupting removals of `--` comment lines**

Once the `+++ ` file header is rewritten, `blockNeedsPrefix` stays `true` for the rest of the block. In a unified diff, a removed line whose content starts with `-- ` (SQL/Lua/Haskell/Ada comments are exactly this) produces a diff body line that begins with `--- ` (three dashes + space). This line is indistinguishable from the file header marker by the current `startsWith("--- ")` check, so it gets an `a/` prefix prepended — corrupting the diff before it reaches the parser. A minimal repro: any noprefix diff that removes a SQL comment like `-- drop table` will have a content line `--- drop table`, which becomes `--- a/drop table` after this rewrite. The parser will reject the mangled patch, and `normalizePatchChangeset` will swallow the error and return `files: []`.

Setting `blockNeedsPrefix = false` immediately after the `+++ ` line is processed is sufficient; the `---`/`+++` pair is always the last pair of header lines before the first `@@` hunk marker in any valid git diff output.

```suggestion
      if (blockNeedsPrefix && line.startsWith("+++ ")) {
        blockNeedsPrefix = false;
        return rewriteUnifiedFileLine(line, "+++ ", "b/");
      }
```

How can I resolve this? If you propose a fix, please make it concise.

[mo]

Thanks @greptile-apps — confirmed real bug, fixed in bf21291.

For anyone tracking, the corruption I could reproduce is slightly different from "the file disappears": with the original commit a noprefix patch that deletes a SQL -- comment line still rendered the file, but the deletion content showed as -- a/drop table users; instead of -- drop table users;. The line counts still added up so the parser didn't reject the patch — it just silently displayed the wrong text on the left side of the diff. Either way, the suggested fix (clearing blockNeedsPrefix after +++) is the right call, and I've added a regression test that reproduces the corruption against the previous commit.

[greptile-apps]

The corruption you described is a subtler failure mode than the outright files: [] case — the parser accepted the mangled patch because line counts still balanced, so there was no signal that anything went wrong. That's the more dangerous class of bug.

The fix in bf21291 is exactly right: clearing blockNeedsPrefix after +++ closes the rewrite window precisely at the boundary where the ---/+++ header pair ends and hunk body lines begin. And having a regression test that fails on the previous commit is the right way to lock this in — good call adding that.

Thanks for the thorough follow-up and for including the credit in the commit message.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

[mo]

@benvinegar let me know if you have any questions about this PR, I'd be happy to explain the problem it fixes more in depth if necessary