From 4cd30cc296371d9d2d3954506239745e92dcc1f0 Mon Sep 17 00:00:00 2001 From: nimrodkor Date: Tue, 14 Apr 2026 17:56:11 +0300 Subject: [PATCH 1/3] Implement diff against main, including everything --- src/flows/LocalReview/LocalDiffPrompt.tsx | 164 ++++++++++-------- .../LocalReview/LocalPullRequestReview.tsx | 8 +- src/flows/Review/Review.tsx | 21 ++- src/lib/git.ts | 100 +++++------ src/pages/PRSelector/PullRequestSelector.tsx | 20 --- .../PullRequestSelectorContainer.tsx | 16 +- 6 files changed, 162 insertions(+), 167 deletions(-) diff --git a/src/flows/LocalReview/LocalDiffPrompt.tsx b/src/flows/LocalReview/LocalDiffPrompt.tsx index 6848313..29bac33 100644 --- a/src/flows/LocalReview/LocalDiffPrompt.tsx +++ b/src/flows/LocalReview/LocalDiffPrompt.tsx @@ -3,21 +3,25 @@ import { Box, Text } from "ink"; import SelectInput from "ink-select-input"; import { ITEM_SELECTION_GAP, ITEM_SELECTOR } from "../../theme/symbols.js"; import { - hasStagedChanges, - getUnstagedFiles, - getStagedDiff, - getAllDiff, + getDiff, + getBranchDiff, + isOnNonDefaultBranch, } from "../../lib/git.js"; -type DiffChoice = "staged" | "all"; +export interface DiffResult { + diffText: string; + currentBranch: string | null; + defaultBranch: string | null; + hasUncommittedChanges: boolean; +} interface SelectItem { label: string; - value: DiffChoice; + value: "yes" | "no"; } interface LocalDiffPromptProps { - onReady: (diffText: string) => void; + onReady: (result: DiffResult) => void; onError: (message: string) => void; } @@ -25,99 +29,115 @@ const LocalDiffPrompt: React.FC = ({ onReady, onError, }) => { - const [unstagedFiles, setUnstagedFiles] = useState([]); - const [hasStaged, setHasStaged] = useState(false); + const [branchInfo, setBranchInfo] = useState<{ + currentBranch: string; + defaultBranch: string; + hasUncommittedChanges: boolean; + } | null>(null); const [ready, setReady] = useState(false); useEffect(() => { try { - const staged = hasStagedChanges(); - const unstaged = getUnstagedFiles(); - - if (!staged && unstaged.length === 0) { - onError( - "No changes detected. Stage some changes with `git add` and try again.", - ); + const info = isOnNonDefaultBranch(); + const hasUncommitted = !!getDiff(); + + if (!info.onFeatureBranch || !info.defaultBranch) { + if (!hasUncommitted) { + onError( + "No changes detected. Stage some changes with `git add` and try again.", + ); + return; + } + onReady({ + diffText: getDiff(), + currentBranch: null, + defaultBranch: null, + hasUncommittedChanges: false, + }); return; } - setHasStaged(staged); - setUnstagedFiles(unstaged); - - // Auto-proceed if no unstaged files - if (unstaged.length === 0) { - onReady(getStagedDiff()); - return; - } - - // If no staged changes but there are unstaged, auto-select all - if (!staged) { - onReady(getAllDiff()); - return; - } - - setReady(true); + setBranchInfo({ + currentBranch: info.currentBranch, + defaultBranch: info.defaultBranch, + hasUncommittedChanges: hasUncommitted, + }); } catch (err) { onError(err instanceof Error ? err.message : "Failed to read git state"); } }, []); - if (!ready) { + // Delay rendering SelectInput by one tick to avoid Enter key leak from previous screen + useEffect(() => { + if (branchInfo) { + const timer = setTimeout(() => setReady(true), 100); + return () => clearTimeout(timer); + } + }, [branchInfo]); + + if (!branchInfo || !ready) { return null; } const items: SelectItem[] = [ - { label: "Review only staged changes", value: "staged" }, - { label: "Review all changes (staged + unstaged)", value: "all" }, + { label: "Yes", value: "yes" }, + { label: "No, back to PR list", value: "no" }, ]; const handleSelect = (item: SelectItem) => { - if (item.value === "staged") { - onReady(getStagedDiff()); - } else { - onReady(getAllDiff()); + if (item.value === "no") { + onError("Review cancelled."); + return; + } + + try { + const diff = getBranchDiff(branchInfo.defaultBranch); + if (!diff) { + onError( + `No changes found between ${branchInfo.currentBranch} and ${branchInfo.defaultBranch}.`, + ); + return; + } + onReady({ + diffText: diff, + currentBranch: branchInfo.currentBranch, + defaultBranch: branchInfo.defaultBranch, + hasUncommittedChanges: branchInfo.hasUncommittedChanges, + }); + } catch (err) { + onError( + err instanceof Error ? err.message : "Failed to compute branch diff", + ); } }; return ( - - - The following files have unstaged changes: + + + You're on branch {branchInfo.currentBranch}. + Review changes vs {branchInfo.defaultBranch}? - {unstagedFiles.map((file) => ( - - {" "}- {file} - - ))} - {hasStaged && ( - <> - - How would you like to proceed? - - - ( - - {isSelected ? ITEM_SELECTOR : ITEM_SELECTION_GAP} - - )} - itemComponent={({ isSelected, label }) => ( - {label} - )} - /> - - - - Use ↑↓ arrows and Enter to select - - - + {branchInfo.hasUncommittedChanges && ( + + ⚠ You have uncommitted changes (not included in branch diff) + )} + + ( + + {isSelected ? ITEM_SELECTOR : ITEM_SELECTION_GAP} + + )} + itemComponent={({ isSelected, label }) => ( + {label} + )} + /> ); }; diff --git a/src/flows/LocalReview/LocalPullRequestReview.tsx b/src/flows/LocalReview/LocalPullRequestReview.tsx index 12db14e..b611793 100644 --- a/src/flows/LocalReview/LocalPullRequestReview.tsx +++ b/src/flows/LocalReview/LocalPullRequestReview.tsx @@ -12,6 +12,8 @@ const LOCAL_WALKTHROUGH_PROMPT = interface LocalPullRequestReviewProps { diffText: string; repoName: string; + currentBranch: string | null; + defaultBranch: string | null; onComplete: () => void; onBack: () => void; } @@ -32,6 +34,8 @@ const LOCAL_PR_NUMBER = 0; const LocalPullRequestReview: React.FC = ({ diffText, repoName, + currentBranch, + defaultBranch, onComplete, onBack, }) => { @@ -118,8 +122,8 @@ const LocalPullRequestReview: React.FC = ({ prId={LOCAL_PR_ID} fullRepoName={repoName} prNumber={LOCAL_PR_NUMBER} - chatTitle="Local Changes Walkthrough" - chatDescription="Walkthrough of local changes. Press ESC to go back." + chatTitle={currentBranch ? `${currentBranch} vs ${defaultBranch}` : "Uncommitted Changes Walkthrough"} + chatDescription={currentBranch ? `Walkthrough of changes on ${currentBranch} vs ${defaultBranch}. Press ESC to go back.` : "Walkthrough of uncommitted changes. Press ESC to go back."} chatInput={LOCAL_WALKTHROUGH_PROMPT} outputInitialMessage={false} buildChatRequestOverride={buildLocalChatRequest} diff --git a/src/flows/Review/Review.tsx b/src/flows/Review/Review.tsx index c692aac..555bf46 100644 --- a/src/flows/Review/Review.tsx +++ b/src/flows/Review/Review.tsx @@ -7,7 +7,7 @@ import IntegrationsCheck from "../Integration/IntegrationsCheck.js"; import PostReviewPrompt, { PostReviewAction } from "./PostReviewPrompt.js"; import { logger } from "../../lib/logger.js"; import PullRequestReview from "../../components/PullRequestReview.js"; -import LocalDiffPrompt from "../LocalReview/LocalDiffPrompt.js"; +import LocalDiffPrompt, { type DiffResult } from "../LocalReview/LocalDiffPrompt.js"; import LocalPullRequestReview from "../LocalReview/LocalPullRequestReview.js"; import { getRepoName } from "../../lib/git.js"; import { MAIN_COLOR } from "../../theme/colors.js"; @@ -54,7 +54,7 @@ type FlowState = } | { step: "localReview"; - diffText: string; + diffResult: DiffResult; } | { step: "localReviewComplete"; @@ -185,8 +185,8 @@ const InternalReviewFlow: React.FC = ({ isLocal }) => { setFlowState({ step: "localDiffPrompt" }); }; - const handleLocalDiffReady = (diffText: string) => { - setFlowState({ step: "localReview", diffText }); + const handleLocalDiffReady = (result: DiffResult) => { + setFlowState({ step: "localReview", diffResult: result }); }; const handleLocalDiffError = (message: string) => { @@ -266,21 +266,28 @@ const InternalReviewFlow: React.FC = ({ isLocal }) => { ); - case "localReview": + case "localReview": { + const { diffResult } = flowState; + const reviewLabel = diffResult.currentBranch + ? `${diffResult.currentBranch} vs ${diffResult.defaultBranch}` + : "uncommitted changes"; return ( - ✓ Reviewing local changes + ✓ Reviewing {reviewLabel} [{getRepoName()}] ); + } case "localReviewComplete": return ( diff --git a/src/lib/git.ts b/src/lib/git.ts index 30dc981..656cbd2 100644 --- a/src/lib/git.ts +++ b/src/lib/git.ts @@ -22,17 +22,51 @@ function gitExec(args: string): string { } } -export function getStagedDiff(): string { - return gitExec("diff --cached"); +export function getDiff(): string { + return gitExec("diff HEAD"); } -export function getUnstagedFiles(): string[] { - const output = gitExec("diff --name-only"); - return output ? output.split("\n") : []; +export function getCurrentBranch(): string { + return gitExec("rev-parse --abbrev-ref HEAD"); } -export function getAllDiff(): string { - return gitExec("diff HEAD"); +export function getDefaultBranch(): string | null { + try { + const ref = gitExec("symbolic-ref refs/remotes/origin/HEAD"); + return ref.split("/").pop() ?? null; + } catch { + // origin/HEAD not set — check for common defaults + for (const branch of ["main", "master"]) { + try { + gitExec(`rev-parse --verify refs/remotes/origin/${branch}`); + return branch; + } catch { + console.trace(`Branch ${branch} not found`); + } + } + return null; + } +} + +export function getBranchDiff(baseBranch: string): string { + return gitExec(`diff origin/${baseBranch}...HEAD`); +} + +export function isOnNonDefaultBranch(): { + onFeatureBranch: boolean; + currentBranch: string; + defaultBranch: string | null; +} { + const currentBranch = getCurrentBranch(); + if (currentBranch === "HEAD") { + return { onFeatureBranch: false, currentBranch, defaultBranch: null }; + } + const defaultBranch = getDefaultBranch(); + return { + onFeatureBranch: defaultBranch !== null && currentBranch !== defaultBranch, + currentBranch, + defaultBranch, + }; } export function getRepoName(): string { @@ -52,55 +86,3 @@ export function getRepoName(): string { return path.basename(process.cwd()); } } - -export function hasStagedChanges(): boolean { - return getStagedDiff().length > 0; -} - -export interface ChangeSummary { - modified: number; - added: number; - deleted: number; -} - -export function getChangeSummary(): ChangeSummary | null { - const staged = gitExec("diff --cached --name-status"); - const unstaged = gitExec("diff --name-status"); - - // Combine and deduplicate by filename (staged takes precedence) - const fileStatuses = new Map(); - for (const line of [...unstaged.split("\n"), ...staged.split("\n")]) { - if (!line.trim()) continue; - const [status, ...fileParts] = line.split("\t"); - const file = fileParts.join("\t"); - if (status && file) { - fileStatuses.set(file, status.charAt(0)); - } - } - - if (fileStatuses.size === 0) return null; - - let modified = 0; - let added = 0; - let deleted = 0; - - for (const status of fileStatuses.values()) { - switch (status) { - case "A": - added++; - break; - case "D": - deleted++; - break; - default: - modified++; - break; - } - } - - return { modified, added, deleted }; -} - -export function hasAnyChanges(): boolean { - return getChangeSummary() !== null; -} diff --git a/src/pages/PRSelector/PullRequestSelector.tsx b/src/pages/PRSelector/PullRequestSelector.tsx index 7a417e5..b37ed27 100644 --- a/src/pages/PRSelector/PullRequestSelector.tsx +++ b/src/pages/PRSelector/PullRequestSelector.tsx @@ -1,7 +1,6 @@ import React, { useEffect, useState, useRef } from "react"; import { Box, Text, useInput } from "ink"; import type { PullRequest } from "../../lib/providers/index.js"; -import type { ChangeSummary } from "../../lib/git.js"; import { updatedTimeAgo } from "../../lib/date.js"; import { PullRequestCard } from "./PullRequestCard.js"; import { useFetchUser } from "../../hooks/useFetchUser.js"; @@ -19,7 +18,6 @@ interface PullRequestSelectorProps { pullRequests: PullRequest[]; onSelect: (pr: PullRequest) => void; onLocalSelect?: () => void; - changeSummary?: ChangeSummary; initialPrId?: string; updateData: (updater: (prev: PullRequest[]) => PullRequest[]) => void; } @@ -30,7 +28,6 @@ const PullRequestSelector: React.FC = ({ pullRequests, onSelect, onLocalSelect, - changeSummary, initialPrId, updateData, }) => { @@ -207,9 +204,6 @@ const PullRequestSelector: React.FC = ({ {selectedIndex === LOCAL_CHANGES_INDEX ? "❯ " : " "} Review local changes - {changeSummary && ( - ({formatChangeSummary(changeSummary)}) - )} )} @@ -319,18 +313,4 @@ function extractSearchKeywords(query: string): PullRequestSearchKeywords { return { author, authorNot, repo, repoNot, freetext: freeText.join(" ") }; } -function formatChangeSummary(summary: ChangeSummary): string { - const parts: string[] = []; - if (summary.modified > 0) { - parts.push(`${summary.modified} modified`); - } - if (summary.added > 0) { - parts.push(`${summary.added} added`); - } - if (summary.deleted > 0) { - parts.push(`${summary.deleted} deleted`); - } - return parts.join(", "); -} - export default PullRequestSelector; diff --git a/src/pages/PRSelector/PullRequestSelectorContainer.tsx b/src/pages/PRSelector/PullRequestSelectorContainer.tsx index caec506..d31c5da 100644 --- a/src/pages/PRSelector/PullRequestSelectorContainer.tsx +++ b/src/pages/PRSelector/PullRequestSelectorContainer.tsx @@ -3,7 +3,7 @@ import { Box, Text, useInput } from "ink"; import Spinner from "ink-spinner"; import type { PullRequest } from "../../lib/providers/index.js"; import { usePullRequests } from "../../hooks/usePullRequests.js"; -import { getChangeSummary, type ChangeSummary } from "../../lib/git.js"; +import { getDiff, isOnNonDefaultBranch } from "../../lib/git.js"; import PullRequestSelector from "./PullRequestSelector.js"; interface PullRequestSelectorContainerProps { @@ -16,13 +16,16 @@ const PullRequestSelectorContainer: React.FC< PullRequestSelectorContainerProps > = ({ onSelect, onLocalSelect, initialPrId }) => { const { data, loading, error, updateData } = usePullRequests(); - const [changeSummary, setChangeSummary] = useState( - null, - ); + const [hasLocalReviewOption, setHasLocalReviewOption] = useState(false); useEffect(() => { try { - setChangeSummary(getChangeSummary()); + const { onFeatureBranch } = isOnNonDefaultBranch(); + if (onFeatureBranch) { + setHasLocalReviewOption(true); + return; + } + setHasLocalReviewOption(!!getDiff()); } catch { // Not a git repo or git not available — don't show local option } @@ -57,8 +60,7 @@ const PullRequestSelectorContainer: React.FC< From 8d4ddfe56b609ffdac1da013e28e42b5c7d35234 Mon Sep 17 00:00:00 2001 From: nimrodkor Date: Tue, 14 Apr 2026 17:57:39 +0300 Subject: [PATCH 2/3] Fix flow --- src/flows/LocalReview/LocalDiffPrompt.tsx | 116 ++++------------------ 1 file changed, 19 insertions(+), 97 deletions(-) diff --git a/src/flows/LocalReview/LocalDiffPrompt.tsx b/src/flows/LocalReview/LocalDiffPrompt.tsx index 29bac33..4692906 100644 --- a/src/flows/LocalReview/LocalDiffPrompt.tsx +++ b/src/flows/LocalReview/LocalDiffPrompt.tsx @@ -1,7 +1,4 @@ -import React, { useState, useEffect } from "react"; -import { Box, Text } from "ink"; -import SelectInput from "ink-select-input"; -import { ITEM_SELECTION_GAP, ITEM_SELECTOR } from "../../theme/symbols.js"; +import React, { useEffect } from "react"; import { getDiff, getBranchDiff, @@ -15,11 +12,6 @@ export interface DiffResult { hasUncommittedChanges: boolean; } -interface SelectItem { - label: string; - value: "yes" | "no"; -} - interface LocalDiffPromptProps { onReady: (result: DiffResult) => void; onError: (message: string) => void; @@ -29,117 +21,47 @@ const LocalDiffPrompt: React.FC = ({ onReady, onError, }) => { - const [branchInfo, setBranchInfo] = useState<{ - currentBranch: string; - defaultBranch: string; - hasUncommittedChanges: boolean; - } | null>(null); - const [ready, setReady] = useState(false); - useEffect(() => { try { const info = isOnNonDefaultBranch(); const hasUncommitted = !!getDiff(); - if (!info.onFeatureBranch || !info.defaultBranch) { - if (!hasUncommitted) { + if (info.onFeatureBranch && info.defaultBranch) { + const diff = getBranchDiff(info.defaultBranch); + if (!diff) { onError( - "No changes detected. Stage some changes with `git add` and try again.", + `No changes found between ${info.currentBranch} and ${info.defaultBranch}.`, ); return; } onReady({ - diffText: getDiff(), - currentBranch: null, - defaultBranch: null, - hasUncommittedChanges: false, + diffText: diff, + currentBranch: info.currentBranch, + defaultBranch: info.defaultBranch, + hasUncommittedChanges: hasUncommitted, }); return; } - setBranchInfo({ - currentBranch: info.currentBranch, - defaultBranch: info.defaultBranch, - hasUncommittedChanges: hasUncommitted, - }); - } catch (err) { - onError(err instanceof Error ? err.message : "Failed to read git state"); - } - }, []); - - // Delay rendering SelectInput by one tick to avoid Enter key leak from previous screen - useEffect(() => { - if (branchInfo) { - const timer = setTimeout(() => setReady(true), 100); - return () => clearTimeout(timer); - } - }, [branchInfo]); - - if (!branchInfo || !ready) { - return null; - } - - const items: SelectItem[] = [ - { label: "Yes", value: "yes" }, - { label: "No, back to PR list", value: "no" }, - ]; - - const handleSelect = (item: SelectItem) => { - if (item.value === "no") { - onError("Review cancelled."); - return; - } - - try { - const diff = getBranchDiff(branchInfo.defaultBranch); - if (!diff) { + // Default branch or detection failed — use uncommitted diff + if (!hasUncommitted) { onError( - `No changes found between ${branchInfo.currentBranch} and ${branchInfo.defaultBranch}.`, + "No changes detected. Stage some changes with `git add` and try again.", ); return; } onReady({ - diffText: diff, - currentBranch: branchInfo.currentBranch, - defaultBranch: branchInfo.defaultBranch, - hasUncommittedChanges: branchInfo.hasUncommittedChanges, + diffText: getDiff(), + currentBranch: null, + defaultBranch: null, + hasUncommittedChanges: false, }); } catch (err) { - onError( - err instanceof Error ? err.message : "Failed to compute branch diff", - ); + onError(err instanceof Error ? err.message : "Failed to read git state"); } - }; - - return ( - - - - You're on branch {branchInfo.currentBranch}. - Review changes vs {branchInfo.defaultBranch}? - - - - {branchInfo.hasUncommittedChanges && ( - - ⚠ You have uncommitted changes (not included in branch diff) - - )} + }, []); - ( - - {isSelected ? ITEM_SELECTOR : ITEM_SELECTION_GAP} - - )} - itemComponent={({ isSelected, label }) => ( - {label} - )} - /> - - ); + return null; }; export default LocalDiffPrompt; From 0705d576913e224af5392aadd021b30bb3c2a0e2 Mon Sep 17 00:00:00 2001 From: nimrodkor Date: Tue, 14 Apr 2026 18:00:57 +0300 Subject: [PATCH 3/3] CI --- src/flows/LocalReview/LocalDiffPrompt.tsx | 6 +----- src/flows/LocalReview/LocalPullRequestReview.tsx | 12 ++++++++++-- src/flows/Review/Review.tsx | 4 +++- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/flows/LocalReview/LocalDiffPrompt.tsx b/src/flows/LocalReview/LocalDiffPrompt.tsx index 4692906..0436450 100644 --- a/src/flows/LocalReview/LocalDiffPrompt.tsx +++ b/src/flows/LocalReview/LocalDiffPrompt.tsx @@ -1,9 +1,5 @@ import React, { useEffect } from "react"; -import { - getDiff, - getBranchDiff, - isOnNonDefaultBranch, -} from "../../lib/git.js"; +import { getDiff, getBranchDiff, isOnNonDefaultBranch } from "../../lib/git.js"; export interface DiffResult { diffText: string; diff --git a/src/flows/LocalReview/LocalPullRequestReview.tsx b/src/flows/LocalReview/LocalPullRequestReview.tsx index b611793..77e9d56 100644 --- a/src/flows/LocalReview/LocalPullRequestReview.tsx +++ b/src/flows/LocalReview/LocalPullRequestReview.tsx @@ -122,8 +122,16 @@ const LocalPullRequestReview: React.FC = ({ prId={LOCAL_PR_ID} fullRepoName={repoName} prNumber={LOCAL_PR_NUMBER} - chatTitle={currentBranch ? `${currentBranch} vs ${defaultBranch}` : "Uncommitted Changes Walkthrough"} - chatDescription={currentBranch ? `Walkthrough of changes on ${currentBranch} vs ${defaultBranch}. Press ESC to go back.` : "Walkthrough of uncommitted changes. Press ESC to go back."} + chatTitle={ + currentBranch + ? `${currentBranch} vs ${defaultBranch}` + : "Uncommitted Changes Walkthrough" + } + chatDescription={ + currentBranch + ? `Walkthrough of changes on ${currentBranch} vs ${defaultBranch}. Press ESC to go back.` + : "Walkthrough of uncommitted changes. Press ESC to go back." + } chatInput={LOCAL_WALKTHROUGH_PROMPT} outputInitialMessage={false} buildChatRequestOverride={buildLocalChatRequest} diff --git a/src/flows/Review/Review.tsx b/src/flows/Review/Review.tsx index 555bf46..8ba5b2c 100644 --- a/src/flows/Review/Review.tsx +++ b/src/flows/Review/Review.tsx @@ -7,7 +7,9 @@ import IntegrationsCheck from "../Integration/IntegrationsCheck.js"; import PostReviewPrompt, { PostReviewAction } from "./PostReviewPrompt.js"; import { logger } from "../../lib/logger.js"; import PullRequestReview from "../../components/PullRequestReview.js"; -import LocalDiffPrompt, { type DiffResult } from "../LocalReview/LocalDiffPrompt.js"; +import LocalDiffPrompt, { + type DiffResult, +} from "../LocalReview/LocalDiffPrompt.js"; import LocalPullRequestReview from "../LocalReview/LocalPullRequestReview.js"; import { getRepoName } from "../../lib/git.js"; import { MAIN_COLOR } from "../../theme/colors.js";