feat(review): pick a commit as the diff base (#709) (#723)

* feat(review): pick a commit as the diff base (#709)

Widens the base picker for `branch` and `merge-base` diff modes from
"branch only" to any commit-ish. `getGitContext` now returns the last
20 commits on HEAD; the picker shows them in a new "Recent commits"
group, and SHA-like or HEAD~N queries get a "Use … as base" affordance
for manual entry. Backend changes are minimal because git diff already
accepts any commit-ish on the left side of the range.

* feat(review): tabbed Branches/Commits in base picker (#709)

Replaces the three-section list (Local / Remote / Recent commits) with
a Branches | Commits tab strip — commits were below a scroll wall in
repos with many branches. SHA-like and HEAD~N queries auto-focus the
Commits tab so manual entry stays one keystroke away.
This commit is contained in:
Michael Ramos
2026-05-13 07:09:17 -07:00
committed by GitHub
parent b04c9e1f30
commit 6db7ca9184
6 changed files with 314 additions and 37 deletions
+3
View File
@@ -1167,6 +1167,8 @@ const ReviewApp: React.FC = () => {
diffOptions: data.gitContext!.diffOptions,
compareTarget: data.gitContext!.compareTarget,
jjEvologs: data.gitContext!.jjEvologs,
// HEAD differs per worktree, so refresh the commit-baseline picker.
recentCommits: data.gitContext!.recentCommits,
};
});
}
@@ -2094,6 +2096,7 @@ const ReviewApp: React.FC = () => {
detectedBase={prMetadata ? undefined : gitContext?.defaultBranch || gitContext?.compareTarget?.fallback}
onSelectBase={prMetadata ? undefined : handleBaseSelect}
compareTarget={gitContext?.compareTarget}
recentCommits={prMetadata ? undefined : gitContext?.recentCommits}
jjEvologs={prMetadata ? undefined : gitContext?.jjEvologs}
detectedEvoBase={prMetadata ? undefined : gitContext?.jjEvologs?.[1]?.commitId}
stagedFiles={stagedFiles}
@@ -1,6 +1,6 @@
import React, { useMemo, useRef, useState } from 'react';
import React, { useEffect, useMemo, useRef, useState } from 'react';
import * as Popover from '@radix-ui/react-popover';
import type { AvailableBranches, CompareTargetPickerCopy } from '@plannotator/shared/types';
import type { AvailableBranches, CompareTargetPickerCopy, RecentCommit } from '@plannotator/shared/types';
interface BaseBranchPickerProps {
availableBranches: AvailableBranches;
@@ -9,6 +9,29 @@ interface BaseBranchPickerProps {
onSelectBase: (branch: string) => void;
disabled?: boolean;
copy: CompareTargetPickerCopy;
/** HEAD ancestry from GitContext.recentCommits — enables picking a commit as the baseline (#709). */
recentCommits?: RecentCommit[];
}
type Tab = 'branches' | 'commits';
// SHA or `HEAD~N` / `HEAD^N` patterns — the picker treats any matching query as
// a usable commit-ish even if it isn't in `recentCommits`. We require ≥ 4 hex
// chars for SHAs to avoid offering "abc" (which is more likely a branch name).
const SHA_PATTERN = /^[0-9a-f]{4,40}$/i;
const HEAD_REL_PATTERN = /^HEAD(?:[~^]\d+)?$/i;
function isCommitishQuery(q: string): boolean {
return SHA_PATTERN.test(q) || HEAD_REL_PATTERN.test(q);
}
function looksLikeSha(ref: string): boolean {
return /^[0-9a-f]{7,}$/i.test(ref);
}
/** Short, human-friendly label for the trigger chip. */
function chipLabel(base: string): string {
return looksLikeSha(base) ? base.slice(0, 7) : base;
}
export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
@@ -18,41 +41,115 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
onSelectBase,
disabled,
copy,
recentCommits,
}) => {
const [open, setOpen] = useState(false);
const [query, setQuery] = useState('');
const [tab, setTab] = useState<Tab>('branches');
const searchRef = useRef<HTMLInputElement>(null);
const { local, remote } = availableBranches;
const commits = recentCommits ?? [];
const hasCommits = commits.length > 0;
const filtered = useMemo(() => {
const q = query.trim().toLowerCase();
if (!q) return { local, remote };
if (!q) return { local, remote, commits };
return {
local: local.filter((b) => b.toLowerCase().includes(q)),
remote: remote.filter((b) => b.toLowerCase().includes(q)),
commits: commits.filter(
(c) =>
c.shortSha.toLowerCase().includes(q) ||
c.sha.toLowerCase().startsWith(q) ||
c.subject.toLowerCase().includes(q),
),
};
}, [local, remote, query]);
}, [local, remote, commits, query]);
const handleSelect = (branch: string) => {
onSelectBase(branch);
// Smart-search: if the typed query looks like a SHA or HEAD~N and isn't
// already an exact match in any group, offer to use it verbatim. Powers the
// "manual commit hash entry" leg of #709 without adding a separate input.
const trimmedQuery = query.trim();
const showUseAsBase =
trimmedQuery.length > 0 &&
isCommitishQuery(trimmedQuery) &&
!filtered.local.includes(trimmedQuery) &&
!filtered.remote.includes(trimmedQuery) &&
!filtered.commits.some((c) => c.sha === trimmedQuery || c.shortSha === trimmedQuery);
// Auto-focus the Commits tab when the user types a SHA-like query — otherwise
// they'd land on Branches (empty for hex queries) and miss the commit list.
useEffect(() => {
if (hasCommits && trimmedQuery && isCommitishQuery(trimmedQuery)) {
setTab('commits');
}
}, [trimmedQuery, hasCommits]);
const handleSelect = (ref: string) => {
onSelectBase(ref);
setOpen(false);
setQuery('');
setTab('branches');
};
const handleReset = () => {
onSelectBase(detectedBase);
setOpen(false);
setQuery('');
setTab('branches');
};
const isCustom = selectedBase !== detectedBase;
const branchesContent = (
<>
{filtered.local.length === 0 && filtered.remote.length === 0 ? (
<div className="px-3 py-2 text-xs text-muted-foreground">{copy.emptyText}</div>
) : (
<>
{filtered.local.length > 0 && (
<BranchGroup
title={copy.localGroupLabel}
branches={filtered.local}
selectedBase={selectedBase}
detectedBase={detectedBase}
onSelect={handleSelect}
/>
)}
{filtered.remote.length > 0 && (
<BranchGroup
title={copy.remoteGroupLabel}
branches={filtered.remote}
selectedBase={selectedBase}
detectedBase={detectedBase}
onSelect={handleSelect}
/>
)}
</>
)}
</>
);
const commitsContent = (
<>
{filtered.commits.length === 0 ? (
<div className="px-3 py-2 text-xs text-muted-foreground">No matching commits.</div>
) : (
<CommitList commits={filtered.commits} selectedBase={selectedBase} onSelect={handleSelect} />
)}
</>
);
return (
<Popover.Root
open={open}
onOpenChange={(v) => {
setOpen(v);
if (!v) setQuery('');
if (!v) {
setQuery('');
setTab('branches');
}
}}
>
<Popover.Trigger asChild>
@@ -69,7 +166,7 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
<span className="text-[10px] uppercase tracking-wide opacity-60 flex-shrink-0">
{copy.triggerLabel}
</span>
<span className="truncate flex-1 text-left">{selectedBase}</span>
<span className="truncate flex-1 text-left">{chipLabel(selectedBase)}</span>
<svg
className="w-3.5 h-3.5 text-muted-foreground flex-shrink-0"
fill="none"
@@ -86,7 +183,7 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
side="bottom"
align="start"
sideOffset={4}
className="z-50 w-72 bg-popover text-popover-foreground border border-border rounded shadow-lg overflow-hidden origin-[var(--radix-popover-content-transform-origin)] data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0"
className="z-50 w-80 bg-popover text-popover-foreground border border-border rounded shadow-lg overflow-hidden origin-[var(--radix-popover-content-transform-origin)] data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0"
onOpenAutoFocus={(e) => {
e.preventDefault();
searchRef.current?.focus();
@@ -98,34 +195,46 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
type="text"
value={query}
onChange={(e) => setQuery(e.target.value)}
placeholder={copy.searchPlaceholder}
placeholder={hasCommits ? `${copy.searchPlaceholder} or SHA / HEAD~N` : copy.searchPlaceholder}
onKeyDown={(e) => {
// Enter on a SHA-like query commits the manual entry —
// matches the "Use … as base" affordance below.
if (e.key === 'Enter' && showUseAsBase) {
e.preventDefault();
handleSelect(trimmedQuery);
}
}}
className="w-full px-2 py-1.5 bg-muted rounded text-xs text-foreground placeholder:text-muted-foreground focus:outline-none focus:ring-1 focus:ring-primary/50"
/>
</div>
{showUseAsBase && (
<div className="border-b border-border/50">
<button
type="button"
onClick={() => handleSelect(trimmedQuery)}
className="w-full flex items-center gap-2 px-3 py-1.5 text-xs text-left hover:bg-muted focus:outline-none focus:bg-muted text-foreground"
>
<span className="flex-1 truncate">
Use <span className="font-mono">{trimmedQuery}</span> as base
</span>
<span className="text-[10px] uppercase tracking-wide text-muted-foreground px-1 py-0.5 rounded bg-muted">
commit
</span>
</button>
</div>
)}
{hasCommits && (
<div className="flex border-b border-border/50 bg-muted/30">
<TabButton active={tab === 'branches'} onClick={() => setTab('branches')}>
Branches
</TabButton>
<TabButton active={tab === 'commits'} onClick={() => setTab('commits')}>
Commits
</TabButton>
</div>
)}
<div className="max-h-72 overflow-y-auto py-1">
{filtered.local.length === 0 && filtered.remote.length === 0 && (
<div className="px-3 py-2 text-xs text-muted-foreground">
{copy.emptyText}
</div>
)}
{filtered.local.length > 0 && (
<BranchGroup
title={copy.localGroupLabel}
branches={filtered.local}
selectedBase={selectedBase}
detectedBase={detectedBase}
onSelect={handleSelect}
/>
)}
{filtered.remote.length > 0 && (
<BranchGroup
title={copy.remoteGroupLabel}
branches={filtered.remote}
selectedBase={selectedBase}
detectedBase={detectedBase}
onSelect={handleSelect}
/>
)}
{hasCommits && tab === 'commits' ? commitsContent : branchesContent}
</div>
{isCustom && (
<div className="border-t border-border/50 p-1">
@@ -144,6 +253,26 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
);
};
interface TabButtonProps {
active: boolean;
onClick: () => void;
children: React.ReactNode;
}
const TabButton: React.FC<TabButtonProps> = ({ active, onClick, children }) => (
<button
type="button"
onClick={onClick}
className={`flex-1 px-3 py-1.5 text-xs transition-colors focus:outline-none ${
active
? 'bg-popover text-foreground font-medium border-b-2 border-primary -mb-px'
: 'text-muted-foreground hover:text-foreground hover:bg-muted/50'
}`}
>
{children}
</button>
);
interface BranchGroupProps {
title: string;
branches: string[];
@@ -193,3 +322,39 @@ const BranchGroup: React.FC<BranchGroupProps> = ({
})}
</div>
);
interface CommitListProps {
commits: RecentCommit[];
selectedBase: string;
onSelect: (sha: string) => void;
}
const CommitList: React.FC<CommitListProps> = ({ commits, selectedBase, onSelect }) => (
<div className="py-1">
{commits.map((c) => {
const isSelected = c.sha === selectedBase || c.shortSha === selectedBase;
return (
<button
key={c.sha}
type="button"
onClick={() => onSelect(c.sha)}
title={`${c.sha}\n${c.subject}\n${c.relativeDate} · ${c.author}`}
className={`w-full flex items-center gap-2 px-3 py-1.5 text-xs text-left hover:bg-muted focus:outline-none focus:bg-muted ${
isSelected ? 'text-foreground font-medium' : 'text-foreground/80'
}`}
>
<span className="w-3 flex-shrink-0">
{isSelected && (
<svg className="w-3 h-3" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={3}>
<path strokeLinecap="round" strokeLinejoin="round" d="M5 13l4 4L19 7" />
</svg>
)}
</span>
<span className="font-mono text-muted-foreground flex-shrink-0">{c.shortSha}</span>
<span className="truncate flex-1">{c.subject}</span>
<span className="text-[10px] text-muted-foreground flex-shrink-0">{c.relativeDate}</span>
</button>
);
})}
</div>
);
@@ -1,6 +1,6 @@
import React, { useEffect, useCallback, useState, useMemo } from 'react';
import { CodeAnnotation } from '@plannotator/ui/types';
import type { AvailableBranches, CompareTargetConfig, DiffOption, JjEvoLogEntry, WorktreeInfo } from '@plannotator/shared/types';
import type { AvailableBranches, CompareTargetConfig, DiffOption, JjEvoLogEntry, RecentCommit, WorktreeInfo } from '@plannotator/shared/types';
import { buildFileTree, getAncestorPaths, getAllFolderPaths, getVisualFileOrder } from '../utils/buildFileTree';
import { FileTreeNodeItem } from './FileTreeNode';
import { BaseBranchPicker } from './BaseBranchPicker';
@@ -37,6 +37,8 @@ interface FileTreeProps {
detectedBase?: string;
onSelectBase?: (branch: string) => void;
compareTarget?: CompareTargetConfig;
/** HEAD ancestry for the commit-baseline picker (git only, #709). */
recentCommits?: RecentCommit[];
/** Evolution log entries for the current jj change (jj-evolog mode only). */
jjEvologs?: JjEvoLogEntry[];
/** Default evolog commit ID to compare against (second evolog entry). */
@@ -90,6 +92,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
detectedBase,
onSelectBase,
compareTarget,
recentCommits,
jjEvologs,
detectedEvoBase,
stagedFiles,
@@ -418,6 +421,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
onSelectBase={onSelectBase}
disabled={isLoadingDiff}
copy={compareTarget.picker}
recentCommits={recentCommits}
/>
</div>
</div>
+40
View File
@@ -7,6 +7,7 @@ import {
getDefaultBranch,
getFileContentsForDiff,
getGitContext,
listRecentCommits,
parseWorktreeDiffType,
runGitDiff,
type DiffType,
@@ -224,6 +225,45 @@ describe("review-core", () => {
});
});
test("listRecentCommits returns HEAD ancestry with shortSha and subject", async () => {
const repoDir = initRepo();
writeFileSync(join(repoDir, "tracked.txt"), "second\n", "utf-8");
git(repoDir, ["add", "tracked.txt"]);
git(repoDir, ["commit", "-m", "second commit"]);
writeFileSync(join(repoDir, "tracked.txt"), "third\n", "utf-8");
git(repoDir, ["add", "tracked.txt"]);
git(repoDir, ["commit", "-m", "third commit"]);
const runtime = makeRuntime(repoDir);
const commits = await listRecentCommits(runtime, repoDir, 10);
expect(commits.length).toBe(3);
expect(commits[0].subject).toBe("third commit");
expect(commits[1].subject).toBe("second commit");
expect(commits[2].subject).toBe("initial");
for (const c of commits) {
expect(c.sha).toMatch(/^[0-9a-f]{40}$/);
expect(c.shortSha.length).toBeGreaterThanOrEqual(7);
expect(c.sha.startsWith(c.shortSha)).toBe(true);
expect(c.author).toBe("Review Core");
expect(c.relativeDate.length).toBeGreaterThan(0);
}
});
test("getGitContext includes recentCommits for the picker", async () => {
const repoDir = initRepo();
writeFileSync(join(repoDir, "tracked.txt"), "second\n", "utf-8");
git(repoDir, ["add", "tracked.txt"]);
git(repoDir, ["commit", "-m", "second commit"]);
const runtime = makeRuntime(repoDir);
const context = await getGitContext(runtime, repoDir);
expect(context.recentCommits).toBeDefined();
expect(context.recentCommits!.length).toBe(2);
expect(context.recentCommits![0].subject).toBe("second commit");
});
test("parseWorktreeDiffType recognises every DiffType suffix, including merge-base", () => {
// Regression guard: every local diff type must round-trip through the
// worktree-prefixed form. Missing `merge-base` here previously routed
+67 -3
View File
@@ -71,6 +71,19 @@ export interface JjEvoLogEntry {
age?: string;
}
export interface RecentCommit {
/** Full SHA — sent back as the diff base. */
sha: string;
/** Abbreviated SHA for display. */
shortSha: string;
/** First line of the commit message. */
subject: string;
/** Human-readable age string, e.g. "2 hours ago". */
relativeDate: string;
/** Committer-name; shown after the subject in the picker. */
author: string;
}
export interface GitContext {
currentBranch: string;
defaultBranch: string;
@@ -83,6 +96,8 @@ export interface GitContext {
vcsType?: "git" | "jj" | "p4";
/** Evolution log entries for the current jj change (jj only). */
jjEvologs?: JjEvoLogEntry[];
/** HEAD ancestry, newest first. Powers the commit-based baseline picker (#709). */
recentCommits?: RecentCommit[];
}
export interface DiffResult {
@@ -218,6 +233,45 @@ export async function detectRemoteDefaultBranch(
}
}
const RECENT_COMMIT_LIMIT_DEFAULT = 20;
// US (\x1F) separator avoids collisions with commit subjects, author names, and
// dates while staying compatible with `git log --pretty=format`.
const COMMIT_FIELD_SEP = "\x1f";
/**
* Walk HEAD's ancestry and return the most-recent commits for the
* commit-baseline picker. Single `git log` call — fast (~ms).
*/
export async function listRecentCommits(
runtime: ReviewGitRuntime,
cwd?: string,
limit: number = RECENT_COMMIT_LIMIT_DEFAULT,
): Promise<RecentCommit[]> {
const fmt = ["%H", "%h", "%s", "%cr", "%an"].join(COMMIT_FIELD_SEP);
const result = await runtime.runGit(
["log", `--max-count=${limit}`, `--pretty=format:${fmt}`, "HEAD"],
{ cwd },
);
if (result.exitCode !== 0) return [];
const commits: RecentCommit[] = [];
for (const line of result.stdout.split("\n")) {
if (!line) continue;
const parts = line.split(COMMIT_FIELD_SEP);
if (parts.length < 5) continue;
// If a subject contains a literal US byte the split over-divides. sha/
// shortSha are fixed-shape at the start and relativeDate/author at the
// end, so rejoin everything between back into the subject.
const sha = parts[0];
const shortSha = parts[1];
const author = parts[parts.length - 1];
const relativeDate = parts[parts.length - 2];
const subject = parts.slice(2, parts.length - 2).join(COMMIT_FIELD_SEP);
commits.push({ sha, shortSha, subject, relativeDate, author });
}
return commits;
}
export async function listBranches(
runtime: ReviewGitRuntime,
cwd?: string,
@@ -324,10 +378,11 @@ export async function getGitContext(
runtime: ReviewGitRuntime,
cwd?: string,
): Promise<GitContext> {
const [currentBranch, defaultBranch, availableBranches] = await Promise.all([
const [currentBranch, defaultBranch, availableBranches, recentCommits] = await Promise.all([
getCurrentBranch(runtime, cwd),
getDefaultBranch(runtime, cwd),
listBranches(runtime, cwd),
listRecentCommits(runtime, cwd),
]);
const diffOptions: DiffOption[] = [
@@ -383,6 +438,7 @@ export async function getGitContext(
},
cwd,
vcsType: "git",
recentCommits,
};
}
@@ -439,6 +495,14 @@ async function getUntrackedFileDiffs(
return diffs.join("");
}
/**
* If `ref` looks like a full or long hex SHA, return its 7-char prefix for
* display. Branch names, tags, and `HEAD~N` pass through unchanged.
*/
function displayRef(ref: string): string {
return /^[0-9a-f]{7,}$/i.test(ref) ? ref.slice(0, 7) : ref;
}
function assertGitSuccess(
result: GitCommandResult,
args: string[],
@@ -619,7 +683,7 @@ export async function runGitDiff(
branchDiffArgs,
);
patch = branchDiff.stdout;
label = `Changes vs ${defaultBranch}`;
label = `Changes vs ${displayRef(defaultBranch)}`;
break;
}
@@ -644,7 +708,7 @@ export async function runGitDiff(
mergeBaseDiffArgs,
);
patch = mergeBaseDiff.stdout;
label = `PR diff vs ${defaultBranch}`;
label = `PR diff vs ${displayRef(defaultBranch)}`;
break;
}
+1
View File
@@ -15,6 +15,7 @@ export type {
WorktreeInfo,
GitContext,
JjEvoLogEntry,
RecentCommit,
AvailableBranches,
CompareTargetConfig,
CompareTargetPickerCopy,