From 3398bc632b41438ee782073dd0201a554bcb7ea3 Mon Sep 17 00:00:00 2001 From: Adam Bowker Date: Fri, 3 Apr 2026 09:53:25 -0700 Subject: [PATCH] feat(code): use remote files for cloud review panel --- apps/code/src/main/services/git/schemas.ts | 1 + apps/code/src/main/services/git/service.ts | 54 ++++++++++-- .../components/CloudReviewPage.tsx | 83 +++++++++---------- .../components/InteractiveFileDiff.tsx | 6 +- .../code-review/components/ReviewShell.tsx | 59 +++++++++---- .../renderer/features/code-review/types.ts | 2 +- .../git-interaction/hooks/useGitQueries.ts | 17 +++- .../task-detail/hooks/useCloudChangedFiles.ts | 20 +++-- apps/code/src/shared/types.ts | 1 + 9 files changed, 161 insertions(+), 82 deletions(-) diff --git a/apps/code/src/main/services/git/schemas.ts b/apps/code/src/main/services/git/schemas.ts index fa783fbfc..24effd0e8 100644 --- a/apps/code/src/main/services/git/schemas.ts +++ b/apps/code/src/main/services/git/schemas.ts @@ -22,6 +22,7 @@ export const changedFileSchema = z.object({ linesAdded: z.number().optional(), linesRemoved: z.number().optional(), staged: z.boolean().optional(), + patch: z.string().optional(), }); export type ChangedFile = z.infer; diff --git a/apps/code/src/main/services/git/service.ts b/apps/code/src/main/services/git/service.ts index e02debeb6..1a7219266 100644 --- a/apps/code/src/main/services/git/service.ts +++ b/apps/code/src/main/services/git/service.ts @@ -78,6 +78,23 @@ const log = logger.scope("git-service"); const FETCH_THROTTLE_MS = 5 * 60 * 1000; const MAX_DIFF_LENGTH = 8000; +/** + * Wraps a GitHub API per-file patch (hunk content only) with + * the `diff --git` / `---` / `+++` header so that unified-diff + * parsers like `@pierre/diffs` can process it correctly. + */ +function toUnifiedDiffPatch( + rawPatch: string, + filename: string, + previousFilename: string | undefined, + status: ChangedFile["status"], +): string { + const oldPath = previousFilename ?? filename; + const fromPath = status === "added" ? "/dev/null" : `a/${oldPath}`; + const toPath = status === "deleted" ? "/dev/null" : `b/${filename}`; + return `diff --git a/${oldPath} b/${filename}\n--- ${fromPath}\n+++ ${toPath}\n${rawPatch}`; +} + @injectable() export class GitService extends TypedEventEmitter { private lastFetchTime = new Map(); @@ -843,6 +860,7 @@ export class GitService extends TypedEventEmitter { previous_filename?: string; additions: number; deletions: number; + patch?: string; }> >; const files = pages.flat(); @@ -870,6 +888,14 @@ export class GitService extends TypedEventEmitter { originalPath: f.previous_filename, linesAdded: f.additions, linesRemoved: f.deletions, + patch: f.patch + ? toUnifiedDiffPatch( + f.patch, + f.filename, + f.previous_filename, + status, + ) + : undefined, }; }); } catch (error) { @@ -903,8 +929,6 @@ export class GitService extends TypedEventEmitter { const result = await execGh([ "api", `repos/${owner}/${repoName}/compare/${defaultBranch}...${branch}`, - "--jq", - ".files", ]); if (result.exitCode !== 0) { @@ -913,13 +937,17 @@ export class GitService extends TypedEventEmitter { ); } - const files = JSON.parse(result.stdout) as Array<{ - filename: string; - status: string; - previous_filename?: string; - additions: number; - deletions: number; - }> | null; + const response = JSON.parse(result.stdout) as { + files?: Array<{ + filename: string; + status: string; + previous_filename?: string; + additions: number; + deletions: number; + patch?: string; + }>; + }; + const files = response.files; if (!files) return []; @@ -946,6 +974,14 @@ export class GitService extends TypedEventEmitter { originalPath: f.previous_filename, linesAdded: f.additions, linesRemoved: f.deletions, + patch: f.patch + ? toUnifiedDiffPatch( + f.patch, + f.filename, + f.previous_filename, + status, + ) + : undefined, }; }); } catch (error) { diff --git a/apps/code/src/renderer/features/code-review/components/CloudReviewPage.tsx b/apps/code/src/renderer/features/code-review/components/CloudReviewPage.tsx index 8cae2449e..7cb0ac7a8 100644 --- a/apps/code/src/renderer/features/code-review/components/CloudReviewPage.tsx +++ b/apps/code/src/renderer/features/code-review/components/CloudReviewPage.tsx @@ -1,12 +1,8 @@ import { useCloudChangedFiles } from "@features/task-detail/hooks/useCloudChangedFiles"; -import { - buildCloudEventSummary, - extractCloudFileDiff, - type ParsedToolCall, -} from "@features/task-detail/utils/cloudToolChanges"; +import type { FileDiffMetadata } from "@pierre/diffs"; +import { processFile } from "@pierre/diffs"; import { Flex, Spinner, Text } from "@radix-ui/themes"; import type { ChangedFile, Task } from "@shared/types"; -import type { AcpMessage } from "@shared/types/session-events"; import { useMemo } from "react"; import { useReviewComment } from "../hooks/useReviewComment"; import type { DiffOptions, OnCommentCallback } from "../types"; @@ -18,30 +14,17 @@ import { useReviewState, } from "./ReviewShell"; -const EMPTY_EVENTS: AcpMessage[] = []; - interface CloudReviewPageProps { taskId: string; task: Task; } export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) { - const { - session, - effectiveBranch, - prUrl, - isRunActive, - changedFiles, - isLoading, - } = useCloudChangedFiles(taskId, task); + const { effectiveBranch, prUrl, isRunActive, remoteFiles, isLoading } = + useCloudChangedFiles(taskId, task); const onComment = useReviewComment(taskId); - const events = session?.events ?? EMPTY_EVENTS; - const summary = useMemo(() => buildCloudEventSummary(events), [events]); - const allPaths = useMemo( - () => changedFiles.map((f) => f.path), - [changedFiles], - ); + const allPaths = useMemo(() => remoteFiles.map((f) => f.path), [remoteFiles]); const { diffOptions, @@ -54,9 +37,9 @@ export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) { uncollapseFile, revealFile, getDeferredReason, - } = useReviewState(changedFiles, allPaths); + } = useReviewState(remoteFiles, allPaths); - if (!prUrl && !effectiveBranch && changedFiles.length === 0) { + if (!prUrl && !effectiveBranch && remoteFiles.length === 0) { if (isRunActive) { return ( @@ -81,17 +64,17 @@ export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) { return ( - {changedFiles.map((file) => { + {remoteFiles.map((file) => { const isCollapsed = collapsedFiles.has(file.path); const deferredReason = getDeferredReason(file.path); @@ -115,7 +98,7 @@ export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) {
toggleFile(file.path)} @@ -130,38 +113,46 @@ export function CloudReviewPage({ taskId, task }: CloudReviewPageProps) { function CloudFileDiff({ file, - toolCalls, + prUrl, options, collapsed, onToggle, onComment, }: { file: ChangedFile; - toolCalls: Map; + prUrl: string | null; options: DiffOptions; collapsed: boolean; onToggle: () => void; onComment: OnCommentCallback; }) { - const diff = useMemo( - () => extractCloudFileDiff(toolCalls, file.path), - [toolCalls, file.path], - ); + const fileDiff = useMemo((): FileDiffMetadata | undefined => { + if (!file.patch) return undefined; + return processFile(file.patch, { isGitDiff: true }); + }, [file.patch]); - const fileName = file.path.split("/").pop() || file.path; - const oldFile = useMemo( - () => ({ name: fileName, contents: diff?.oldText ?? "" }), - [fileName, diff], - ); - const newFile = useMemo( - () => ({ name: fileName, contents: diff?.newText ?? "" }), - [fileName, diff], - ); + if (!fileDiff) { + const hasChanges = (file.linesAdded ?? 0) + (file.linesRemoved ?? 0) > 0; + const reason = hasChanges ? "large" : "unavailable"; + const githubFileUrl = prUrl + ? `${prUrl}/files#diff-${file.path.replaceAll("/", "-")}` + : undefined; + return ( + + ); + } return ( ( diff --git a/apps/code/src/renderer/features/code-review/components/InteractiveFileDiff.tsx b/apps/code/src/renderer/features/code-review/components/InteractiveFileDiff.tsx index 63fa8622b..cdd58b877 100644 --- a/apps/code/src/renderer/features/code-review/components/InteractiveFileDiff.tsx +++ b/apps/code/src/renderer/features/code-review/components/InteractiveFileDiff.tsx @@ -67,8 +67,8 @@ function PatchDiffView({ filePathRef.current = currentFilePath; const hunkAnnotations = useMemo( - () => buildHunkAnnotations(fileDiff), - [fileDiff], + () => (repoPath ? buildHunkAnnotations(fileDiff) : []), + [fileDiff, repoPath], ); const annotations = useMemo( () => @@ -97,7 +97,7 @@ function PatchDiffView({ const handleRevert = useCallback( async (hunkIndex: number) => { const filePath = filePathRef.current; - if (!filePath) return; + if (!filePath || !repoPath) return; setRevertingHunks((prev) => new Set(prev).add(hunkIndex)); setFileDiff((prev) => diffAcceptRejectHunk(prev, hunkIndex, "reject")); diff --git a/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx b/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx index fe068deed..d68fc9eff 100644 --- a/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx +++ b/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx @@ -75,7 +75,7 @@ const AUTO_COLLAPSE_PATTERNS = [ /\.pbxproj$/, ]; -export type DeferredReason = "deleted" | "large" | "generated"; +export type DeferredReason = "deleted" | "large" | "generated" | "unavailable"; export function computeAutoDeferred( files: { @@ -495,6 +495,8 @@ function getDeferredMessage( return `Generated file not rendered — ${totalLines} lines changed.`; case "large": return `Large diff not rendered — ${totalLines} lines changed.`; + case "unavailable": + return "Unable to load diff."; } } @@ -506,6 +508,7 @@ export function DeferredDiffPlaceholder({ collapsed, onToggle, onShow, + externalUrl, }: { filePath: string; linesAdded: number; @@ -513,7 +516,8 @@ export function DeferredDiffPlaceholder({ reason: DeferredReason; collapsed: boolean; onToggle: () => void; - onShow: () => void; + onShow?: () => void; + externalUrl?: string; }) { const { dirPath, fileName } = splitFilePath(filePath); @@ -528,28 +532,55 @@ export function DeferredDiffPlaceholder({ onToggle={onToggle} /> {!collapsed && ( - + {getDeferredMessage(reason, linesAdded + linesRemoved)} + {onShow ? ( + <> + {" "} + + + ) : externalUrl ? ( + <> + {" "} + + View on GitHub + + + ) : null} +
)} ); diff --git a/apps/code/src/renderer/features/code-review/types.ts b/apps/code/src/renderer/features/code-review/types.ts index 704f2e4a9..521800a9a 100644 --- a/apps/code/src/renderer/features/code-review/types.ts +++ b/apps/code/src/renderer/features/code-review/types.ts @@ -26,7 +26,7 @@ export type OnCommentCallback = ( ) => void; export type PatchDiffProps = FileDiffProps & { - repoPath: string; + repoPath?: string; onComment?: OnCommentCallback; }; diff --git a/apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts b/apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts index caf61d6e4..824e2c352 100644 --- a/apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts +++ b/apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts @@ -154,12 +154,20 @@ export function useGitQueries(repoPath?: string) { }; } -export function useCloudPrChangedFiles(prUrl: string | null) { +export function useCloudPrChangedFiles( + prUrl: string | null, + isRunActive?: boolean, +) { const trpc = useTRPC(); return useQuery( trpc.git.getPrChangedFiles.queryOptions( { prUrl: prUrl as string }, - { enabled: !!prUrl, staleTime: 5 * 60_000, retry: 1 }, + { + enabled: !!prUrl, + staleTime: isRunActive ? 10_000 : 5 * 60_000, + refetchInterval: isRunActive ? 10_000 : false, + retry: 1, + }, ), ); } @@ -167,6 +175,7 @@ export function useCloudPrChangedFiles(prUrl: string | null) { export function useCloudBranchChangedFiles( repo: string | null, branch: string | null, + isRunActive?: boolean, ) { const trpc = useTRPC(); return useQuery( @@ -174,8 +183,8 @@ export function useCloudBranchChangedFiles( { repo: repo as string, branch: branch as string }, { enabled: !!repo && !!branch, - staleTime: 30_000, - refetchInterval: 30_000, + staleTime: isRunActive ? 10_000 : 30_000, + refetchInterval: isRunActive ? 10_000 : 30_000, retry: 1, }, ), diff --git a/apps/code/src/renderer/features/task-detail/hooks/useCloudChangedFiles.ts b/apps/code/src/renderer/features/task-detail/hooks/useCloudChangedFiles.ts index bf4355e6f..671a6d20e 100644 --- a/apps/code/src/renderer/features/task-detail/hooks/useCloudChangedFiles.ts +++ b/apps/code/src/renderer/features/task-detail/hooks/useCloudChangedFiles.ts @@ -4,16 +4,20 @@ import { } from "@features/git-interaction/hooks/useGitQueries"; import { useCloudRunState } from "@features/task-detail/hooks/useCloudRunState"; import type { ChangedFile, Task } from "@shared/types"; +import { useMemo } from "react"; + +const EMPTY_FILES: ChangedFile[] = []; export function useCloudChangedFiles(taskId: string, task: Task) { const cloudRunState = useCloudRunState(taskId, task); - const { prUrl, effectiveBranch, repo, fallbackFiles } = cloudRunState; + const { prUrl, effectiveBranch, repo, fallbackFiles, isRunActive } = + cloudRunState; const { data: prFiles, isPending: prPending, isError: prError, - } = useCloudPrChangedFiles(prUrl); + } = useCloudPrChangedFiles(prUrl, isRunActive); const { data: branchFiles, @@ -22,19 +26,25 @@ export function useCloudChangedFiles(taskId: string, task: Task) { } = useCloudBranchChangedFiles( !prUrl ? repo : null, !prUrl ? effectiveBranch : null, + isRunActive, ); - const remoteFiles: ChangedFile[] = prUrl - ? (prFiles ?? []) - : (branchFiles ?? []); + const remoteFiles = useMemo((): ChangedFile[] => { + const files = prUrl ? prFiles : branchFiles; + return files ?? EMPTY_FILES; + }, [prUrl, prFiles, branchFiles]); + const isLoading = prUrl ? prPending : effectiveBranch ? branchPending : false; const hasError = prUrl ? prError : effectiveBranch ? branchError : false; + // changedFiles: sidebar list, built from remote with agent output fallback + // remoteFiles: review panel, always uses PR changes with remote branch fallback const changedFiles = remoteFiles.length > 0 ? remoteFiles : fallbackFiles; return { ...cloudRunState, changedFiles, + remoteFiles, isLoading, hasError, }; diff --git a/apps/code/src/shared/types.ts b/apps/code/src/shared/types.ts index 87a9f3619..4441a89b9 100644 --- a/apps/code/src/shared/types.ts +++ b/apps/code/src/shared/types.ts @@ -139,6 +139,7 @@ export interface ChangedFile { linesAdded?: number; linesRemoved?: number; staged?: boolean; + patch?: string; // Unified diff patch from GitHub API } // External apps detection types