diff --git a/apps/code/src/renderer/features/code-editor/stores/diffViewerStore.ts b/apps/code/src/renderer/features/code-editor/stores/diffViewerStore.ts index d5beaf142..7b962fca2 100644 --- a/apps/code/src/renderer/features/code-editor/stores/diffViewerStore.ts +++ b/apps/code/src/renderer/features/code-editor/stores/diffViewerStore.ts @@ -4,6 +4,7 @@ import { create } from "zustand"; import { persist } from "zustand/middleware"; export type ViewMode = "split" | "unified"; +export type DiffSource = "local" | "branch"; interface DiffViewerStoreState { viewMode: ViewMode; @@ -12,6 +13,7 @@ interface DiffViewerStoreState { wordDiffs: boolean; hideWhitespaceChanges: boolean; showReviewComments: boolean; + diffSource: Record; } interface DiffViewerStoreActions { @@ -22,6 +24,7 @@ interface DiffViewerStoreActions { toggleWordDiffs: () => void; toggleHideWhitespaceChanges: () => void; toggleShowReviewComments: () => void; + setDiffSource: (taskId: string, source: DiffSource) => void; } type DiffViewerStore = DiffViewerStoreState & DiffViewerStoreActions; @@ -35,6 +38,7 @@ export const useDiffViewerStore = create()( wordDiffs: true, hideWhitespaceChanges: false, showReviewComments: true, + diffSource: {}, setViewMode: (mode) => set((state) => { if (state.viewMode === mode) { @@ -69,6 +73,10 @@ export const useDiffViewerStore = create()( set((s) => ({ hideWhitespaceChanges: !s.hideWhitespaceChanges })), toggleShowReviewComments: () => set((s) => ({ showReviewComments: !s.showReviewComments })), + setDiffSource: (taskId, source) => + set((s) => ({ + diffSource: { ...s.diffSource, [taskId]: source }, + })), }), { name: "diff-viewer-storage", 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 38ff0b7f1..c333f4082 100644 --- a/apps/code/src/renderer/features/code-review/components/CloudReviewPage.tsx +++ b/apps/code/src/renderer/features/code-review/components/CloudReviewPage.tsx @@ -2,19 +2,14 @@ import { useDiffViewerStore } from "@features/code-editor/stores/diffViewerStore import { usePrDetails } from "@features/git-interaction/hooks/usePrDetails"; import { useCloudChangedFiles } from "@features/task-detail/hooks/useCloudChangedFiles"; import { extractCloudFileDiff } 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 { useReviewNavigationStore } from "@renderer/features/code-review/stores/reviewNavigationStore"; -import type { ChangedFile, Task } from "@shared/types"; +import type { Task } from "@shared/types"; import { useMemo } from "react"; -import type { DiffOptions } from "../types"; -import type { PrCommentThread } from "../utils/prCommentAnnotations"; -import { InteractiveFileDiff } from "./InteractiveFileDiff"; import { LazyDiff } from "./LazyDiff"; +import { PatchedFileDiff } from "./PatchedFileDiff"; import { DeferredDiffPlaceholder, - DiffFileHeader, ReviewShell, useReviewState, } from "./ReviewShell"; @@ -122,10 +117,14 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) { ); } + const githubFileUrl = prUrl + ? `${prUrl}/files#diff-${file.path.replaceAll("/", "-")}` + : undefined; + return (
- toggleFile(file.path)} commentThreads={showReviewComments ? commentThreads : undefined} - toolCallDiff={toolCallDiffs?.get(file.path) ?? null} + fallback={toolCallDiffs?.get(file.path) ?? null} + externalUrl={githubFileUrl} />
@@ -142,76 +142,3 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) { ); } - -function CloudFileDiff({ - file, - taskId, - prUrl, - options, - collapsed, - onToggle, - commentThreads, - toolCallDiff, -}: { - file: ChangedFile; - taskId: string; - prUrl: string | null; - options: DiffOptions; - collapsed: boolean; - onToggle: () => void; - commentThreads?: Map; - toolCallDiff: { oldText: string | null; newText: string | null } | null; -}) { - const fileDiff = useMemo((): FileDiffMetadata | undefined => { - if (!file.patch) return undefined; - return processFile(file.patch, { isGitDiff: true }); - }, [file.patch]); - - const diffSourceProps = useMemo(() => { - if (fileDiff) return { fileDiff }; - if (toolCallDiff) { - const name = file.path.split("/").pop() || file.path; - return { - oldFile: { name, contents: toolCallDiff.oldText ?? "" }, - newFile: { name, contents: toolCallDiff.newText ?? "" }, - }; - } - return null; - }, [fileDiff, toolCallDiff, file.path]); - - if (!diffSourceProps) { - 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/DiffSourceSelector.tsx b/apps/code/src/renderer/features/code-review/components/DiffSourceSelector.tsx new file mode 100644 index 000000000..8ee2f5f3d --- /dev/null +++ b/apps/code/src/renderer/features/code-review/components/DiffSourceSelector.tsx @@ -0,0 +1,66 @@ +import { CaretDown, GitBranch, HardDrives } from "@phosphor-icons/react"; +import { + Button, + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "@posthog/quill"; +import { useDiffViewerStore } from "@renderer/features/code-editor/stores/diffViewerStore"; +import type { ResolvedDiffSource } from "../utils/resolveDiffSource"; + +interface DiffSourceSelectorProps { + taskId: string; + effectiveSource: ResolvedDiffSource; + branchAvailable: boolean; + defaultBranch: string | null; +} + +export function DiffSourceSelector({ + taskId, + effectiveSource, + branchAvailable, + defaultBranch, +}: DiffSourceSelectorProps) { + const setDiffSource = useDiffViewerStore((s) => s.setDiffSource); + + if (!branchAvailable || !defaultBranch) return null; + + const Icon = effectiveSource === "branch" ? GitBranch : HardDrives; + const branchLabel = `Branch vs. ${defaultBranch}`; + const triggerLabel = effectiveSource === "branch" ? branchLabel : "Local"; + + return ( + + + + {triggerLabel} + + + } + /> + + setDiffSource(taskId, "local")}> + + Local changes + + setDiffSource(taskId, "branch")}> + + {branchLabel} + + + + ); +} diff --git a/apps/code/src/renderer/features/code-review/components/PatchedFileDiff.tsx b/apps/code/src/renderer/features/code-review/components/PatchedFileDiff.tsx new file mode 100644 index 000000000..066477a0f --- /dev/null +++ b/apps/code/src/renderer/features/code-review/components/PatchedFileDiff.tsx @@ -0,0 +1,80 @@ +import { type FileDiffMetadata, processFile } from "@pierre/diffs"; +import type { ChangedFile } from "@shared/types"; +import { useMemo } from "react"; +import type { DiffOptions } from "../types"; +import type { PrCommentThread } from "../utils/prCommentAnnotations"; +import { InteractiveFileDiff } from "./InteractiveFileDiff"; +import { DeferredDiffPlaceholder, DiffFileHeader } from "./ReviewShell"; + +interface PatchedFileDiffProps { + file: ChangedFile; + taskId: string; + options: DiffOptions; + collapsed: boolean; + onToggle: () => void; + fallback?: { oldText: string | null; newText: string | null } | null; + externalUrl?: string; + prUrl?: string | null; + commentThreads?: Map; +} + +export function PatchedFileDiff({ + file, + taskId, + options, + collapsed, + onToggle, + fallback, + externalUrl, + prUrl, + commentThreads, +}: PatchedFileDiffProps) { + const fileDiff = useMemo((): FileDiffMetadata | undefined => { + if (!file.patch) return undefined; + return processFile(file.patch, { isGitDiff: true }); + }, [file.patch]); + + const diffSourceProps = useMemo(() => { + if (fileDiff) return { fileDiff }; + if (fallback) { + const name = file.path.split("/").pop() || file.path; + return { + oldFile: { name, contents: fallback.oldText ?? "" }, + newFile: { name, contents: fallback.newText ?? "" }, + }; + } + return null; + }, [fileDiff, fallback, file.path]); + + if (!diffSourceProps) { + const hasChanges = (file.linesAdded ?? 0) + (file.linesRemoved ?? 0) > 0; + return ( + + ); + } + + return ( + ( + + )} + /> + ); +} diff --git a/apps/code/src/renderer/features/code-review/components/ReviewPage.tsx b/apps/code/src/renderer/features/code-review/components/ReviewPage.tsx index 8ed535515..25798936b 100644 --- a/apps/code/src/renderer/features/code-review/components/ReviewPage.tsx +++ b/apps/code/src/renderer/features/code-review/components/ReviewPage.tsx @@ -1,6 +1,9 @@ +import { useDiffViewerStore } from "@features/code-editor/stores/diffViewerStore"; +import { useGitQueries } from "@features/git-interaction/hooks/useGitQueries"; import { makeFileKey } from "@features/git-interaction/utils/fileKey"; import { usePanelLayoutStore } from "@features/panels/store/panelLayoutStore"; import { useCwd } from "@features/sidebar/hooks/useCwd"; +import { useWorkspace } from "@features/workspace/hooks/useWorkspace"; import type { parsePatchFiles } from "@pierre/diffs"; import { Flex, Text } from "@radix-ui/themes"; import { useReviewNavigationStore } from "@renderer/features/code-review/stores/reviewNavigationStore"; @@ -10,8 +13,13 @@ import { useQuery } from "@tanstack/react-query"; import { useMemo } from "react"; import { useReviewDiffs } from "../hooks/useReviewDiffs"; import type { DiffOptions } from "../types"; +import { + type ResolvedDiffSource, + resolveDiffSource, +} from "../utils/resolveDiffSource"; import { InteractiveFileDiff } from "./InteractiveFileDiff"; import { LazyDiff } from "./LazyDiff"; +import { PatchedFileDiff } from "./PatchedFileDiff"; import { DeferredDiffPlaceholder, type DeferredReason, @@ -21,6 +29,8 @@ import { useReviewState, } from "./ReviewShell"; +const EMPTY_BRANCH_FILES: ChangedFile[] = []; + interface ReviewPageProps { task: Task; } @@ -28,10 +38,36 @@ interface ReviewPageProps { export function ReviewPage({ task }: ReviewPageProps) { const taskId = task.id; const repoPath = useCwd(taskId); + const workspace = useWorkspace(taskId); + const linkedBranch = workspace?.linkedBranch ?? null; const openFile = usePanelLayoutStore((s) => s.openFile); + const isReviewOpen = useReviewNavigationStore( (s) => (s.reviewModes[taskId] ?? "closed") !== "closed", ); + + const configuredSource = useDiffViewerStore( + (s) => s.diffSource[taskId] ?? null, + ); + + const { + repoInfo, + aheadOfDefault, + defaultBranch, + changedFiles: workspaceFiles, + } = useGitQueries(repoPath); + const hasLocalChanges = workspaceFiles.length > 0; + const branchSourceAvailable = !!linkedBranch && aheadOfDefault > 0; + + const effectiveSource = resolveDiffSource({ + configured: configuredSource, + hasLocalChanges, + linkedBranch, + aheadOfDefault, + }); + + const isLocalActive = isReviewOpen && effectiveSource === "local"; + const { changedFiles, changesLoading, @@ -43,7 +79,7 @@ export function ReviewPage({ task }: ReviewPageProps) { allPaths, diffLoading, refetch, - } = useReviewDiffs(repoPath, isReviewOpen); + } = useReviewDiffs(repoPath, isLocalActive); const { diffOptions, @@ -68,6 +104,20 @@ export function ReviewPage({ task }: ReviewPageProps) { ); } + if (effectiveSource === "branch") { + return ( + + ); + } + const sharedDiffProps = { repoPath, taskId, @@ -92,6 +142,9 @@ export function ReviewPage({ task }: ReviewPageProps) { onCollapseAll={collapseAll} onUncollapseFile={uncollapseFile} onRefresh={refetch} + effectiveSource={effectiveSource} + branchSourceAvailable={branchSourceAvailable} + defaultBranch={defaultBranch} > {hasStagedFiles && stagedParsedFiles.length > 0 && ( <> @@ -126,6 +179,113 @@ export function ReviewPage({ task }: ReviewPageProps) { ); } +function BranchReviewPage({ + task, + branch, + repoInfo, + defaultBranch, + isReviewOpen, + effectiveSource, + branchSourceAvailable, +}: { + task: Task; + branch: string; + repoInfo: { organization: string; repository: string } | undefined; + defaultBranch: string | null; + isReviewOpen: boolean; + effectiveSource: ResolvedDiffSource; + branchSourceAvailable: boolean; +}) { + const taskId = task.id; + const trpc = useTRPC(); + + const repoSlug = repoInfo + ? `${repoInfo.organization}/${repoInfo.repository}` + : null; + + const { data: files = EMPTY_BRANCH_FILES, isLoading } = useQuery( + trpc.git.getBranchChangedFiles.queryOptions( + { repo: repoSlug as string, branch }, + { + enabled: isReviewOpen && !!repoSlug, + staleTime: 30_000, + refetchInterval: 30_000, + retry: 1, + }, + ), + ); + + const allPaths = useMemo(() => files.map((f) => f.path), [files]); + + const { + diffOptions, + linesAdded, + linesRemoved, + collapsedFiles, + toggleFile, + expandAll, + collapseAll, + uncollapseFile, + revealFile, + getDeferredReason, + } = useReviewState(files, allPaths); + + return ( + + {files.map((file) => { + const isCollapsed = collapsedFiles.has(file.path); + const deferredReason = getDeferredReason(file.path); + + if (deferredReason) { + return ( +
+ toggleFile(file.path)} + onShow={() => revealFile(file.path)} + /> +
+ ); + } + + return ( +
+ + toggleFile(file.path)} + /> + +
+ ); + })} +
+ ); +} + function SectionLabel({ label }: { label: string }) { return ( 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 10e43b65d..47730a7e7 100644 --- a/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx +++ b/apps/code/src/renderer/features/code-review/components/ReviewShell.tsx @@ -18,6 +18,7 @@ import { useRef, useState, } from "react"; +import type { ResolvedDiffSource } from "../utils/resolveDiffSource"; import { ReviewToolbar } from "./ReviewToolbar"; function splitFilePath(fullPath: string): { @@ -306,6 +307,9 @@ export interface ReviewShellProps { onExpandAll: () => void; onCollapseAll: () => void; onRefresh?: () => void; + effectiveSource?: ResolvedDiffSource; + branchSourceAvailable?: boolean; + defaultBranch?: string | null; } export function ReviewShell({ @@ -321,6 +325,9 @@ export function ReviewShell({ onExpandAll, onCollapseAll, onRefresh, + effectiveSource, + branchSourceAvailable, + defaultBranch, }: ReviewShellProps) { const taskId = task.id; const scrollContainerRef = useRef(null); @@ -409,24 +416,6 @@ export function ReviewShell({ return () => observer.disconnect(); }, [taskId, setActiveFilePath]); - if (isLoading) { - return ( - - - - ); - } - - if (isEmpty) { - return ( - - - No file changes to review - - - ); - } - return (
- {children} + {isLoading ? ( + + + + ) : isEmpty ? ( + + + No file changes to review + + + ) : ( + children + )}
{isExpanded && } diff --git a/apps/code/src/renderer/features/code-review/components/ReviewToolbar.tsx b/apps/code/src/renderer/features/code-review/components/ReviewToolbar.tsx index 94758cbcf..9a390afdf 100644 --- a/apps/code/src/renderer/features/code-review/components/ReviewToolbar.tsx +++ b/apps/code/src/renderer/features/code-review/components/ReviewToolbar.tsx @@ -4,10 +4,12 @@ import { ArrowsClockwise, Columns, Rows, X } from "@phosphor-icons/react"; import { Button } from "@posthog/quill"; import { Flex, Separator, Text } from "@radix-ui/themes"; import { DiffSettingsMenu } from "@renderer/features/code-review/components/DiffSettingsMenu"; +import { DiffSourceSelector } from "@renderer/features/code-review/components/DiffSourceSelector"; import { type ReviewMode, useReviewNavigationStore, } from "@renderer/features/code-review/stores/reviewNavigationStore"; +import type { ResolvedDiffSource } from "@renderer/features/code-review/utils/resolveDiffSource"; import { FoldVertical, Maximize, Minimize, UnfoldVertical } from "lucide-react"; import { memo } from "react"; @@ -20,6 +22,9 @@ interface ReviewToolbarProps { onExpandAll: () => void; onCollapseAll: () => void; onRefresh?: () => void; + effectiveSource?: ResolvedDiffSource; + branchSourceAvailable?: boolean; + defaultBranch?: string | null; } export const ReviewToolbar = memo(function ReviewToolbar({ @@ -29,6 +34,9 @@ export const ReviewToolbar = memo(function ReviewToolbar({ onExpandAll, onCollapseAll, onRefresh, + effectiveSource, + branchSourceAvailable, + defaultBranch, }: ReviewToolbarProps) { const viewMode = useDiffViewerStore((s) => s.viewMode); const toggleViewMode = useDiffViewerStore((s) => s.toggleViewMode); @@ -62,9 +70,19 @@ export const ReviewToolbar = memo(function ReviewToolbar({ flexShrink: 0, }} > - - {fileCount} file{fileCount !== 1 ? "s" : ""} changed - + + + {fileCount} file{fileCount !== 1 ? "s" : ""} changed + + {effectiveSource && ( + + )} + {onRefresh && ( diff --git a/apps/code/src/renderer/features/code-review/utils/resolveDiffSource.test.ts b/apps/code/src/renderer/features/code-review/utils/resolveDiffSource.test.ts new file mode 100644 index 000000000..14f92ac88 --- /dev/null +++ b/apps/code/src/renderer/features/code-review/utils/resolveDiffSource.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "vitest"; +import { + type ResolveDiffSourceInput, + type ResolvedDiffSource, + resolveDiffSource, +} from "./resolveDiffSource"; + +describe("resolveDiffSource", () => { + it.each< + ResolveDiffSourceInput & { expected: ResolvedDiffSource; desc: string } + >([ + { + desc: "heuristic: uncommitted changes → local", + configured: null, + hasLocalChanges: true, + linkedBranch: "feat/x", + aheadOfDefault: 3, + expected: "local", + }, + { + desc: "heuristic: clean tree with commits ahead → branch", + configured: null, + hasLocalChanges: false, + linkedBranch: "feat/x", + aheadOfDefault: 2, + expected: "branch", + }, + { + desc: "heuristic: no linked branch → local", + configured: null, + hasLocalChanges: false, + linkedBranch: null, + aheadOfDefault: 0, + expected: "local", + }, + { + desc: "heuristic: linked branch but no commits ahead → local", + configured: null, + hasLocalChanges: false, + linkedBranch: "feat/x", + aheadOfDefault: 0, + expected: "local", + }, + { + desc: "explicit local respected even when branch is available", + configured: "local", + hasLocalChanges: false, + linkedBranch: "feat/x", + aheadOfDefault: 5, + expected: "local", + }, + { + desc: "explicit branch respected when available", + configured: "branch", + hasLocalChanges: true, + linkedBranch: "feat/x", + aheadOfDefault: 1, + expected: "branch", + }, + { + desc: "explicit branch falls back to local when no linked branch", + configured: "branch", + hasLocalChanges: false, + linkedBranch: null, + aheadOfDefault: 0, + expected: "local", + }, + { + desc: "explicit branch falls back to local when no commits ahead", + configured: "branch", + hasLocalChanges: false, + linkedBranch: "feat/x", + aheadOfDefault: 0, + expected: "local", + }, + ])("$desc", ({ expected, ...input }) => { + expect(resolveDiffSource(input)).toBe(expected); + }); +}); diff --git a/apps/code/src/renderer/features/code-review/utils/resolveDiffSource.ts b/apps/code/src/renderer/features/code-review/utils/resolveDiffSource.ts new file mode 100644 index 000000000..307351da3 --- /dev/null +++ b/apps/code/src/renderer/features/code-review/utils/resolveDiffSource.ts @@ -0,0 +1,30 @@ +import type { DiffSource } from "@features/code-editor/stores/diffViewerStore"; + +export type ResolvedDiffSource = DiffSource; + +export interface ResolveDiffSourceInput { + configured: DiffSource | null; + hasLocalChanges: boolean; + linkedBranch: string | null; + aheadOfDefault: number; +} + +export function resolveDiffSource({ + configured, + hasLocalChanges, + linkedBranch, + aheadOfDefault, +}: ResolveDiffSourceInput): ResolvedDiffSource { + const branchAvailable = !!linkedBranch && aheadOfDefault > 0; + + if (configured === "branch") { + return branchAvailable ? "branch" : "local"; + } + if (configured === "local") { + return "local"; + } + + if (hasLocalChanges) return "local"; + if (branchAvailable) return "branch"; + return "local"; +}