Skip to content

Commit 0938436

Browse files
flex-yj-kimbacknotpropclaude
authored
fix: Fix review showing empty files + propagate diff errors to UI (#200)
* 🐛 fix: force standard a/b diff prefix to handle mnemonic prefix config When diff.mnemonicPrefix is enabled in git config, `git diff` uses context-dependent prefixes (c/ for commit, w/ for worktree, i/ for index) instead of the standard a/b. Both the internal parseDiffToFiles and @pierre/diffs library expect a/b prefixes, causing silent parse failures that result in an empty file list. Add --src-prefix=a/ --dst-prefix=b/ to all git diff invocations to ensure consistent output regardless of user git configuration. * ✨ feat: propagate git diff errors to review UI Previously, git diff errors were silently caught and returned as empty patches. The UI showed "No changes" with no indication of failure. - Add error field to DiffResult and propagate through ReviewServerOptions - Include error in /api/diff and /api/diff/switch responses - Show distinct error state in review UI (red icon + error message) - Clear/set error state on diff type switch * Fix mnemonic prefix handling in pi-extension runGitDiff Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 99c0666 commit 0938436

6 files changed

Lines changed: 55 additions & 24 deletions

File tree

‎apps/hook/server/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ if (args[0] === "review") {
6868
const gitContext = await getGitContext();
6969

7070
// Run git diff HEAD (uncommitted changes - default)
71-
const { patch: rawPatch, label: gitRef } = await runGitDiff(
71+
const { patch: rawPatch, label: gitRef, error: diffError } = await runGitDiff(
7272
"uncommitted",
7373
gitContext.defaultBranch
7474
);
@@ -77,6 +77,7 @@ if (args[0] === "review") {
7777
const server = await startReviewServer({
7878
rawPatch,
7979
gitRef,
80+
error: diffError,
8081
origin: "claude-code",
8182
diffType: "uncommitted",
8283
gitContext,

‎apps/opencode-plugin/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ Do NOT proceed with implementation until your plan is approved.
160160
const gitContext = await getGitContext();
161161

162162
// Run git diff HEAD (uncommitted changes - default)
163-
const { patch: rawPatch, label: gitRef } = await runGitDiff(
163+
const { patch: rawPatch, label: gitRef, error: diffError } = await runGitDiff(
164164
"uncommitted",
165165
gitContext.defaultBranch
166166
);
@@ -169,6 +169,7 @@ Do NOT proceed with implementation until your plan is approved.
169169
const server = await startReviewServer({
170170
rawPatch,
171171
gitRef,
172+
error: diffError,
172173
origin: "opencode",
173174
diffType: "uncommitted",
174175
gitContext,

‎apps/pi-extension/server.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -389,15 +389,15 @@ export function getGitContext(): GitContext {
389389
export function runGitDiff(diffType: DiffType, defaultBranch = "main"): { patch: string; label: string } {
390390
switch (diffType) {
391391
case "uncommitted":
392-
return { patch: git("diff HEAD"), label: "Uncommitted changes" };
392+
return { patch: git("diff HEAD --src-prefix=a/ --dst-prefix=b/"), label: "Uncommitted changes" };
393393
case "staged":
394-
return { patch: git("diff --staged"), label: "Staged changes" };
394+
return { patch: git("diff --staged --src-prefix=a/ --dst-prefix=b/"), label: "Staged changes" };
395395
case "unstaged":
396-
return { patch: git("diff"), label: "Unstaged changes" };
396+
return { patch: git("diff --src-prefix=a/ --dst-prefix=b/"), label: "Unstaged changes" };
397397
case "last-commit":
398-
return { patch: git("diff HEAD~1..HEAD"), label: "Last commit" };
398+
return { patch: git("diff HEAD~1..HEAD --src-prefix=a/ --dst-prefix=b/"), label: "Last commit" };
399399
case "branch":
400-
return { patch: git(`diff ${defaultBranch}..HEAD`), label: `Changes vs ${defaultBranch}` };
400+
return { patch: git(`diff ${defaultBranch}..HEAD --src-prefix=a/ --dst-prefix=b/`), label: `Changes vs ${defaultBranch}` };
401401
default:
402402
return { patch: "", label: "Unknown diff type" };
403403
}

‎packages/review-editor/App.tsx‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ const ReviewApp: React.FC = () => {
149149
const [diffType, setDiffType] = useState<string>('uncommitted');
150150
const [gitContext, setGitContext] = useState<GitContext | null>(null);
151151
const [isLoadingDiff, setIsLoadingDiff] = useState(false);
152+
const [diffError, setDiffError] = useState<string | null>(null);
152153
const [isSendingFeedback, setIsSendingFeedback] = useState(false);
153154
const [isApproving, setIsApproving] = useState(false);
154155
const [submitted, setSubmitted] = useState<'approved' | 'feedback' | false>(false);
@@ -209,6 +210,7 @@ const ReviewApp: React.FC = () => {
209210
gitContext?: GitContext;
210211
sharingEnabled?: boolean;
211212
repoInfo?: { display: string; branch?: string };
213+
error?: string;
212214
}) => {
213215
const apiFiles = parseDiffToFiles(data.rawPatch);
214216
setDiffData({
@@ -226,6 +228,7 @@ const ReviewApp: React.FC = () => {
226228
if (data.gitContext) setGitContext(data.gitContext);
227229
if (data.sharingEnabled !== undefined) setSharingEnabled(data.sharingEnabled);
228230
if (data.repoInfo) setRepoInfo(data.repoInfo);
231+
if (data.error) setDiffError(data.error);
229232
})
230233
.catch(() => {
231234
// Not in API mode - use demo content
@@ -359,10 +362,12 @@ const ReviewApp: React.FC = () => {
359362
setDiffType(data.diffType);
360363
setActiveFileIndex(0);
361364
setPendingSelection(null);
365+
setDiffError((data as { error?: string }).error || null);
362366
// Note: We keep existing annotations - they may still be relevant
363367
// or user can clear them manually
364368
} catch (err) {
365369
console.error('Failed to switch diff:', err);
370+
setDiffError(err instanceof Error ? err.message : 'Failed to switch diff');
366371
} finally {
367372
setIsLoadingDiff(false);
368373
}
@@ -783,20 +788,35 @@ const ReviewApp: React.FC = () => {
783788
) : (
784789
<div className="h-full flex items-center justify-center">
785790
<div className="text-center space-y-3 max-w-md px-8">
786-
<div className="mx-auto w-12 h-12 rounded-full bg-muted/50 flex items-center justify-center">
787-
<svg className="w-6 h-6 text-muted-foreground" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}>
788-
<path strokeLinecap="round" strokeLinejoin="round" d="M19.5 14.25v-2.625a3.375 3.375 0 00-3.375-3.375h-1.5A1.125 1.125 0 0113.5 7.125v-1.5a3.375 3.375 0 00-3.375-3.375H8.25m0 12.75h7.5m-7.5 3H12M10.5 2.25H5.625c-.621 0-1.125.504-1.125 1.125v17.25c0 .621.504 1.125 1.125 1.125h12.75c.621 0 1.125-.504 1.125-1.125V11.25a9 9 0 00-9-9z" />
789-
</svg>
791+
<div className={`mx-auto w-12 h-12 rounded-full flex items-center justify-center ${diffError ? 'bg-destructive/10' : 'bg-muted/50'}`}>
792+
{diffError ? (
793+
<svg className="w-6 h-6 text-destructive" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}>
794+
<path strokeLinecap="round" strokeLinejoin="round" d="M12 9v3.75m-9.303 3.376c-.866 1.5.217 3.374 1.948 3.374h14.71c1.73 0 2.813-1.874 1.948-3.374L13.949 3.378c-.866-1.5-3.032-1.5-3.898 0L2.697 16.126zM12 15.75h.007v.008H12v-.008z" />
795+
</svg>
796+
) : (
797+
<svg className="w-6 h-6 text-muted-foreground" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}>
798+
<path strokeLinecap="round" strokeLinejoin="round" d="M19.5 14.25v-2.625a3.375 3.375 0 00-3.375-3.375h-1.5A1.125 1.125 0 0113.5 7.125v-1.5a3.375 3.375 0 00-3.375-3.375H8.25m0 12.75h7.5m-7.5 3H12M10.5 2.25H5.625c-.621 0-1.125.504-1.125 1.125v17.25c0 .621.504 1.125 1.125 1.125h12.75c.621 0 1.125-.504 1.125-1.125V11.25a9 9 0 00-9-9z" />
799+
</svg>
800+
)}
790801
</div>
791802
<div>
792-
<h3 className="text-sm font-medium text-foreground">No changes</h3>
793-
<p className="text-xs text-muted-foreground mt-1">
794-
{diffType === 'uncommitted' && "No uncommitted changes to review."}
795-
{diffType === 'staged' && "No staged changes. Stage some files with git add."}
796-
{diffType === 'unstaged' && "No unstaged changes. All changes are staged."}
797-
{diffType === 'last-commit' && "No changes in the last commit."}
798-
{diffType === 'branch' && `No changes between this branch and ${gitContext?.defaultBranch || 'main'}.`}
799-
</p>
803+
{diffError ? (
804+
<>
805+
<h3 className="text-sm font-medium text-destructive">Failed to load diff</h3>
806+
<p className="text-xs text-muted-foreground mt-1">{diffError}</p>
807+
</>
808+
) : (
809+
<>
810+
<h3 className="text-sm font-medium text-foreground">No changes</h3>
811+
<p className="text-xs text-muted-foreground mt-1">
812+
{diffType === 'uncommitted' && "No uncommitted changes to review."}
813+
{diffType === 'staged' && "No staged changes. Stage some files with git add."}
814+
{diffType === 'unstaged' && "No unstaged changes. All changes are staged."}
815+
{diffType === 'last-commit' && "No changes in the last commit."}
816+
{diffType === 'branch' && `No changes between this branch and ${gitContext?.defaultBranch || 'main'}.`}
817+
</p>
818+
</>
819+
)}
800820
</div>
801821
{gitContext?.diffOptions && gitContext.diffOptions.length > 1 && (
802822
<p className="text-xs text-muted-foreground/60">

‎packages/server/git.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export interface GitContext {
2828
export interface DiffResult {
2929
patch: string;
3030
label: string;
31+
error?: string;
3132
}
3233

3334
/**
@@ -108,27 +109,27 @@ export async function runGitDiff(
108109
try {
109110
switch (diffType) {
110111
case "uncommitted":
111-
patch = (await $`git diff HEAD`.quiet()).text();
112+
patch = (await $`git diff HEAD --src-prefix=a/ --dst-prefix=b/`.quiet()).text();
112113
label = "Uncommitted changes";
113114
break;
114115

115116
case "staged":
116-
patch = (await $`git diff --staged`.quiet()).text();
117+
patch = (await $`git diff --staged --src-prefix=a/ --dst-prefix=b/`.quiet()).text();
117118
label = "Staged changes";
118119
break;
119120

120121
case "unstaged":
121-
patch = (await $`git diff`.quiet()).text();
122+
patch = (await $`git diff --src-prefix=a/ --dst-prefix=b/`.quiet()).text();
122123
label = "Unstaged changes";
123124
break;
124125

125126
case "last-commit":
126-
patch = (await $`git diff HEAD~1..HEAD`.quiet()).text();
127+
patch = (await $`git diff HEAD~1..HEAD --src-prefix=a/ --dst-prefix=b/`.quiet()).text();
127128
label = "Last commit";
128129
break;
129130

130131
case "branch":
131-
patch = (await $`git diff ${defaultBranch}..HEAD`.quiet()).text();
132+
patch = (await $`git diff ${defaultBranch}..HEAD --src-prefix=a/ --dst-prefix=b/`.quiet()).text();
132133
label = `Changes vs ${defaultBranch}`;
133134
break;
134135

@@ -139,8 +140,10 @@ export async function runGitDiff(
139140
} catch (error) {
140141
// Handle errors gracefully (e.g., no commits yet, invalid ref)
141142
console.error(`Git diff error for ${diffType}:`, error);
143+
const errorMessage = error instanceof Error ? error.message : String(error);
142144
patch = "";
143145
label = `Error: ${diffType}`;
146+
return { patch, label, error: errorMessage };
144147
}
145148

146149
return { patch, label };

‎packages/server/review.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export interface ReviewServerOptions {
2828
rawPatch: string;
2929
/** Git ref used for the diff (e.g., "HEAD", "main..HEAD", "--staged") */
3030
gitRef: string;
31+
/** Error message if git diff failed */
32+
error?: string;
3133
/** HTML content to serve for the UI */
3234
htmlContent: string;
3335
/** Origin identifier for UI customization */
@@ -89,6 +91,7 @@ export async function startReviewServer(
8991
let currentPatch = options.rawPatch;
9092
let currentGitRef = options.gitRef;
9193
let currentDiffType: DiffType = options.diffType || "uncommitted";
94+
let currentError = options.error;
9295

9396
const isRemote = isRemoteSession();
9497
const configuredPort = getServerPort();
@@ -132,6 +135,7 @@ export async function startReviewServer(
132135
sharingEnabled,
133136
shareBaseUrl,
134137
repoInfo,
138+
...(currentError && { error: currentError }),
135139
});
136140
}
137141

@@ -156,11 +160,13 @@ export async function startReviewServer(
156160
currentPatch = result.patch;
157161
currentGitRef = result.label;
158162
currentDiffType = newDiffType;
163+
currentError = result.error;
159164

160165
return Response.json({
161166
rawPatch: currentPatch,
162167
gitRef: currentGitRef,
163168
diffType: currentDiffType,
169+
...(currentError && { error: currentError }),
164170
});
165171
} catch (err) {
166172
const message =

0 commit comments

Comments
 (0)