Skip to content

Commit 3398bc6

Browse files
adboioadboio
committed
feat(code): use remote files for cloud review panel
1 parent 3618421 commit 3398bc6

File tree

9 files changed

+161
-82
lines changed

9 files changed

+161
-82
lines changed

‎apps/code/src/main/services/git/schemas.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export const changedFileSchema = z.object({
2222
linesAdded: z.number().optional(),
2323
linesRemoved: z.number().optional(),
2424
staged: z.boolean().optional(),
25+
patch: z.string().optional(),
2526
});
2627

2728
export type ChangedFile = z.infer<typeof changedFileSchema>;

‎apps/code/src/main/services/git/service.ts‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,23 @@ const log = logger.scope("git-service");
7878
const FETCH_THROTTLE_MS = 5 * 60 * 1000;
7979
const MAX_DIFF_LENGTH = 8000;
8080

81+
/**
82+
* Wraps a GitHub API per-file patch (hunk content only) with
83+
* the `diff --git` / `---` / `+++` header so that unified-diff
84+
* parsers like `@pierre/diffs` can process it correctly.
85+
*/
86+
function toUnifiedDiffPatch(
87+
rawPatch: string,
88+
filename: string,
89+
previousFilename: string | undefined,
90+
status: ChangedFile["status"],
91+
): string {
92+
const oldPath = previousFilename ?? filename;
93+
const fromPath = status === "added" ? "/dev/null" : `a/${oldPath}`;
94+
const toPath = status === "deleted" ? "/dev/null" : `b/${filename}`;
95+
return `diff --git a/${oldPath} b/${filename}\n--- ${fromPath}\n+++ ${toPath}\n${rawPatch}`;
96+
}
97+
8198
@injectable()
8299
export class GitService extends TypedEventEmitter<GitServiceEvents> {
83100
private lastFetchTime = new Map<string, number>();
@@ -843,6 +860,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
843860
previous_filename?: string;
844861
additions: number;
845862
deletions: number;
863+
patch?: string;
846864
}>
847865
>;
848866
const files = pages.flat();
@@ -870,6 +888,14 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
870888
originalPath: f.previous_filename,
871889
linesAdded: f.additions,
872890
linesRemoved: f.deletions,
891+
patch: f.patch
892+
? toUnifiedDiffPatch(
893+
f.patch,
894+
f.filename,
895+
f.previous_filename,
896+
status,
897+
)
898+
: undefined,
873899
};
874900
});
875901
} catch (error) {
@@ -903,8 +929,6 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
903929
const result = await execGh([
904930
"api",
905931
`repos/${owner}/${repoName}/compare/${defaultBranch}...${branch}`,
906-
"--jq",
907-
".files",
908932
]);
909933

910934
if (result.exitCode !== 0) {
@@ -913,13 +937,17 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
913937
);
914938
}
915939

916-
const files = JSON.parse(result.stdout) as Array<{
917-
filename: string;
918-
status: string;
919-
previous_filename?: string;
920-
additions: number;
921-
deletions: number;
922-
}> | null;
940+
const response = JSON.parse(result.stdout) as {
941+
files?: Array<{
942+
filename: string;
943+
status: string;
944+
previous_filename?: string;
945+
additions: number;
946+
deletions: number;
947+
patch?: string;
948+
}>;
949+
};
950+
const files = response.files;
923951

924952
if (!files) return [];
925953

@@ -946,6 +974,14 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
946974
originalPath: f.previous_filename,
947975
linesAdded: f.additions,
948976
linesRemoved: f.deletions,
977+
patch: f.patch
978+
? toUnifiedDiffPatch(
979+
f.patch,
980+
f.filename,
981+
f.previous_filename,
982+
status,
983+
)
984+
: undefined,
949985
};
950986
});
951987
} catch (error) {

‎apps/code/src/renderer/features/code-review/components/CloudReviewPage.tsx‎

Lines changed: 37 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
11
import { useCloudChangedFiles } from "@features/task-detail/hooks/useCloudChangedFiles";
2-
import {
3-
buildCloudEventSummary,
4-
extractCloudFileDiff,
5-
type ParsedToolCall,
6-
} from "@features/task-detail/utils/cloudToolChanges";
2+
import type { FileDiffMetadata } from "@pierre/diffs";
3+
import { processFile } from "@pierre/diffs";
74
import { Flex, Spinner, Text } from "@radix-ui/themes";
85
import type { ChangedFile, Task } from "@shared/types";
9-
import type { AcpMessage } from "@shared/types/session-events";
106
import { useMemo } from "react";
117
import { useReviewComment } from "../hooks/useReviewComment";
128
import type { DiffOptions, OnCommentCallback } from "../types";
@@ -18,30 +14,17 @@ import {
1814
useReviewState,
1915
} from "./ReviewShell";
2016

21-
const EMPTY_EVENTS: AcpMessage[] = [];
22-
2317
interface CloudReviewPageProps {
2418
taskId: string;
2519
task: Task;
2620
}
2721

2822
export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) {
29-
const {
30-
session,
31-
effectiveBranch,
32-
prUrl,
33-
isRunActive,
34-
changedFiles,
35-
isLoading,
36-
} = useCloudChangedFiles(taskId, task);
23+
const { effectiveBranch, prUrl, isRunActive, remoteFiles, isLoading } =
24+
useCloudChangedFiles(taskId, task);
3725
const onComment = useReviewComment(taskId);
38-
const events = session?.events ?? EMPTY_EVENTS;
39-
const summary = useMemo(() => buildCloudEventSummary(events), [events]);
4026

41-
const allPaths = useMemo(
42-
() => changedFiles.map((f) => f.path),
43-
[changedFiles],
44-
);
27+
const allPaths = useMemo(() => remoteFiles.map((f) => f.path), [remoteFiles]);
4528

4629
const {
4730
diffOptions,
@@ -54,9 +37,9 @@ export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) {
5437
uncollapseFile,
5538
revealFile,
5639
getDeferredReason,
57-
} = useReviewState(changedFiles, allPaths);
40+
} = useReviewState(remoteFiles, allPaths);
5841

59-
if (!prUrl && !effectiveBranch && changedFiles.length === 0) {
42+
if (!prUrl && !effectiveBranch && remoteFiles.length === 0) {
6043
if (isRunActive) {
6144
return (
6245
<Flex align="center" justify="center" height="100%">
@@ -81,17 +64,17 @@ export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) {
8164
return (
8265
<ReviewShell
8366
taskId={taskId}
84-
fileCount={changedFiles.length}
67+
fileCount={remoteFiles.length}
8568
linesAdded={linesAdded}
8669
linesRemoved={linesRemoved}
87-
isLoading={isLoading && changedFiles.length === 0}
88-
isEmpty={changedFiles.length === 0}
70+
isLoading={isLoading && remoteFiles.length === 0}
71+
isEmpty={remoteFiles.length === 0}
8972
allExpanded={collapsedFiles.size === 0}
9073
onExpandAll={expandAll}
9174
onCollapseAll={collapseAll}
9275
onUncollapseFile={uncollapseFile}
9376
>
94-
{changedFiles.map((file) => {
77+
{remoteFiles.map((file) => {
9578
const isCollapsed = collapsedFiles.has(file.path);
9679
const deferredReason = getDeferredReason(file.path);
9780

@@ -115,7 +98,7 @@ export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) {
11598
<div key={file.path} data-file-path={file.path}>
11699
<CloudFileDiff
117100
file={file}
118-
toolCalls={summary.toolCalls}
101+
prUrl={prUrl}
119102
options={diffOptions}
120103
collapsed={isCollapsed}
121104
onToggle={() => toggleFile(file.path)}
@@ -130,38 +113,46 @@ export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) {
130113

131114
function CloudFileDiff({
132115
file,
133-
toolCalls,
116+
prUrl,
134117
options,
135118
collapsed,
136119
onToggle,
137120
onComment,
138121
}: {
139122
file: ChangedFile;
140-
toolCalls: Map<string, ParsedToolCall>;
123+
prUrl: string | null;
141124
options: DiffOptions;
142125
collapsed: boolean;
143126
onToggle: () => void;
144127
onComment: OnCommentCallback;
145128
}) {
146-
const diff = useMemo(
147-
() => extractCloudFileDiff(toolCalls, file.path),
148-
[toolCalls, file.path],
149-
);
129+
const fileDiff = useMemo((): FileDiffMetadata | undefined => {
130+
if (!file.patch) return undefined;
131+
return processFile(file.patch, { isGitDiff: true });
132+
}, [file.patch]);
150133

151-
const fileName = file.path.split("/").pop() || file.path;
152-
const oldFile = useMemo(
153-
() => ({ name: fileName, contents: diff?.oldText ?? "" }),
154-
[fileName, diff],
155-
);
156-
const newFile = useMemo(
157-
() => ({ name: fileName, contents: diff?.newText ?? "" }),
158-
[fileName, diff],
159-
);
134+
if (!fileDiff) {
135+
const hasChanges = (file.linesAdded ?? 0) + (file.linesRemoved ?? 0) > 0;
136+
const reason = hasChanges ? "large" : "unavailable";
137+
const githubFileUrl = prUrl
138+
? `${prUrl}/files#diff-${file.path.replaceAll("/", "-")}`
139+
: undefined;
140+
return (
141+
<DeferredDiffPlaceholder
142+
filePath={file.path}
143+
linesAdded={file.linesAdded ?? 0}
144+
linesRemoved={file.linesRemoved ?? 0}
145+
reason={reason}
146+
collapsed={collapsed}
147+
onToggle={onToggle}
148+
externalUrl={githubFileUrl}
149+
/>
150+
);
151+
}
160152

161153
return (
162154
<InteractiveFileDiff
163-
oldFile={oldFile}
164-
newFile={newFile}
155+
fileDiff={fileDiff}
165156
options={{ ...options, collapsed }}
166157
onComment={onComment}
167158
renderCustomHeader={(fd) => (

‎apps/code/src/renderer/features/code-review/components/InteractiveFileDiff.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ function PatchDiffView({
6767
filePathRef.current = currentFilePath;
6868

6969
const hunkAnnotations = useMemo(
70-
() => buildHunkAnnotations(fileDiff),
71-
[fileDiff],
70+
() => (repoPath ? buildHunkAnnotations(fileDiff) : []),
71+
[fileDiff, repoPath],
7272
);
7373
const annotations = useMemo(
7474
() =>
@@ -97,7 +97,7 @@ function PatchDiffView({
9797
const handleRevert = useCallback(
9898
async (hunkIndex: number) => {
9999
const filePath = filePathRef.current;
100-
if (!filePath) return;
100+
if (!filePath || !repoPath) return;
101101

102102
setRevertingHunks((prev) => new Set(prev).add(hunkIndex));
103103
setFileDiff((prev) => diffAcceptRejectHunk(prev, hunkIndex, "reject"));

‎apps/code/src/renderer/features/code-review/components/ReviewShell.tsx‎

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ const AUTO_COLLAPSE_PATTERNS = [
7575
/\.pbxproj$/,
7676
];
7777

78-
export type DeferredReason = "deleted" | "large" | "generated";
78+
export type DeferredReason = "deleted" | "large" | "generated" | "unavailable";
7979

8080
export function computeAutoDeferred(
8181
files: {
@@ -495,6 +495,8 @@ function getDeferredMessage(
495495
return `Generated file not rendered — ${totalLines} lines changed.`;
496496
case "large":
497497
return `Large diff not rendered — ${totalLines} lines changed.`;
498+
case "unavailable":
499+
return "Unable to load diff.";
498500
}
499501
}
500502

@@ -506,14 +508,16 @@ export function DeferredDiffPlaceholder({
506508
collapsed,
507509
onToggle,
508510
onShow,
511+
externalUrl,
509512
}: {
510513
filePath: string;
511514
linesAdded: number;
512515
linesRemoved: number;
513516
reason: DeferredReason;
514517
collapsed: boolean;
515518
onToggle: () => void;
516-
onShow: () => void;
519+
onShow?: () => void;
520+
externalUrl?: string;
517521
}) {
518522
const { dirPath, fileName } = splitFilePath(filePath);
519523

@@ -528,28 +532,55 @@ export function DeferredDiffPlaceholder({
528532
onToggle={onToggle}
529533
/>
530534
{!collapsed && (
531-
<button
532-
type="button"
533-
onClick={onShow}
535+
<div
534536
style={{
535537
padding: "16px",
536538
textAlign: "center",
537539
color: "var(--gray-9)",
538540
fontSize: "12px",
539-
cursor: "pointer",
540541
background: "var(--gray-2)",
541542
borderBottom: "1px solid var(--gray-5)",
542543
width: "100%",
543-
border: "none",
544544
}}
545545
>
546-
{getDeferredMessage(reason, linesAdded + linesRemoved)}{" "}
547-
<span
548-
style={{ color: "var(--accent-9)", textDecoration: "underline" }}
549-
>
550-
Load diff
551-
</span>
552-
</button>
546+
{getDeferredMessage(reason, linesAdded + linesRemoved)}
547+
{onShow ? (
548+
<>
549+
{" "}
550+
<button
551+
type="button"
552+
onClick={onShow}
553+
style={{
554+
color: "var(--accent-9)",
555+
textDecoration: "underline",
556+
background: "none",
557+
border: "none",
558+
cursor: "pointer",
559+
fontSize: "inherit",
560+
padding: 0,
561+
}}
562+
>
563+
Load diff
564+
</button>
565+
</>
566+
) : externalUrl ? (
567+
<>
568+
{" "}
569+
<a
570+
href={externalUrl}
571+
target="_blank"
572+
rel="noopener noreferrer"
573+
style={{
574+
color: "var(--accent-9)",
575+
textDecoration: "underline",
576+
fontSize: "inherit",
577+
}}
578+
>
579+
View on GitHub
580+
</a>
581+
</>
582+
) : null}
583+
</div>
553584
)}
554585
</div>
555586
);

‎apps/code/src/renderer/features/code-review/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export type OnCommentCallback = (
2626
) => void;
2727

2828
export type PatchDiffProps = FileDiffProps<AnnotationMetadata> & {
29-
repoPath: string;
29+
repoPath?: string;
3030
onComment?: OnCommentCallback;
3131
};
3232

0 commit comments

Comments
 (0)