From d59e90b54d409beb4f6d0b1724dbad6c49300d9c Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 13 May 2026 06:30:00 -0700 Subject: [PATCH 1/2] feat(review): pick a commit as the diff base (#709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/review-editor/App.tsx | 3 + .../components/BaseBranchPicker.tsx | 136 ++++++++++++++++-- .../review-editor/components/FileTree.tsx | 6 +- packages/shared/review-core.test.ts | 40 ++++++ packages/shared/review-core.ts | 70 ++++++++- packages/shared/types.ts | 1 + 6 files changed, 244 insertions(+), 12 deletions(-) diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index aa11e0ced..7b6aca616 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -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} diff --git a/packages/review-editor/components/BaseBranchPicker.tsx b/packages/review-editor/components/BaseBranchPicker.tsx index 384a216d7..e41a798ae 100644 --- a/packages/review-editor/components/BaseBranchPicker.tsx +++ b/packages/review-editor/components/BaseBranchPicker.tsx @@ -1,6 +1,6 @@ import React, { 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,27 @@ 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[]; +} + +// 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 = ({ @@ -18,23 +39,43 @@ export const BaseBranchPicker: React.FC = ({ onSelectBase, disabled, copy, + recentCommits, }) => { const [open, setOpen] = useState(false); const [query, setQuery] = useState(''); const searchRef = useRef(null); const { local, remote } = availableBranches; + const commits = recentCommits ?? []; + 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]); + + // 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); - const handleSelect = (branch: string) => { - onSelectBase(branch); + const handleSelect = (ref: string) => { + onSelectBase(ref); setOpen(false); setQuery(''); }; @@ -45,6 +86,12 @@ export const BaseBranchPicker: React.FC = ({ setQuery(''); }; + const noResults = + filtered.local.length === 0 && + filtered.remote.length === 0 && + filtered.commits.length === 0 && + !showUseAsBase; + const isCustom = selectedBase !== detectedBase; return ( @@ -69,7 +116,7 @@ export const BaseBranchPicker: React.FC = ({ {copy.triggerLabel} - {selectedBase} + {chipLabel(selectedBase)} = ({ 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(); @@ -99,11 +146,36 @@ export const BaseBranchPicker: React.FC = ({ value={query} onChange={(e) => setQuery(e.target.value)} placeholder={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" />
- {filtered.local.length === 0 && filtered.remote.length === 0 && ( + {showUseAsBase && ( +
+ +
+ )} + {noResults && (
{copy.emptyText}
@@ -126,6 +198,14 @@ export const BaseBranchPicker: React.FC = ({ onSelect={handleSelect} /> )} + {filtered.commits.length > 0 && ( + + )}
{isCustom && (
@@ -193,3 +273,43 @@ const BranchGroup: React.FC = ({ })}
); + +interface CommitGroupProps { + title: string; + commits: RecentCommit[]; + selectedBase: string; + onSelect: (sha: string) => void; +} + +const CommitGroup: React.FC = ({ title, commits, selectedBase, onSelect }) => ( +
+
+ {title} +
+ {commits.map((c) => { + const isSelected = c.sha === selectedBase || c.shortSha === selectedBase; + return ( + + ); + })} +
+); diff --git a/packages/review-editor/components/FileTree.tsx b/packages/review-editor/components/FileTree.tsx index 6c0aa8fa0..f5c3d6022 100644 --- a/packages/review-editor/components/FileTree.tsx +++ b/packages/review-editor/components/FileTree.tsx @@ -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 = ({ detectedBase, onSelectBase, compareTarget, + recentCommits, jjEvologs, detectedEvoBase, stagedFiles, @@ -418,6 +421,7 @@ export const FileTree: React.FC = ({ onSelectBase={onSelectBase} disabled={isLoadingDiff} copy={compareTarget.picker} + recentCommits={recentCommits} /> diff --git a/packages/shared/review-core.test.ts b/packages/shared/review-core.test.ts index 082552e1f..aecbc29c3 100644 --- a/packages/shared/review-core.test.ts +++ b/packages/shared/review-core.test.ts @@ -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 diff --git a/packages/shared/review-core.ts b/packages/shared/review-core.ts index e79a703bd..2ad78fdbb 100644 --- a/packages/shared/review-core.ts +++ b/packages/shared/review-core.ts @@ -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 { + 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 { - 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; } diff --git a/packages/shared/types.ts b/packages/shared/types.ts index 711badbfc..ac07ae68e 100644 --- a/packages/shared/types.ts +++ b/packages/shared/types.ts @@ -15,6 +15,7 @@ export type { WorktreeInfo, GitContext, JjEvoLogEntry, + RecentCommit, AvailableBranches, CompareTargetConfig, CompareTargetPickerCopy, From 3d68fa6e754ceee68b117d5be5ffb893654a18e9 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 13 May 2026 06:42:17 -0700 Subject: [PATCH 2/2] feat(review): tabbed Branches/Commits in base picker (#709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../components/BaseBranchPicker.tsx | 171 +++++++++++------- 1 file changed, 108 insertions(+), 63 deletions(-) diff --git a/packages/review-editor/components/BaseBranchPicker.tsx b/packages/review-editor/components/BaseBranchPicker.tsx index e41a798ae..e4c944710 100644 --- a/packages/review-editor/components/BaseBranchPicker.tsx +++ b/packages/review-editor/components/BaseBranchPicker.tsx @@ -1,4 +1,4 @@ -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, RecentCommit } from '@plannotator/shared/types'; @@ -13,6 +13,8 @@ interface BaseBranchPickerProps { 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). @@ -43,10 +45,12 @@ export const BaseBranchPicker: React.FC = ({ }) => { const [open, setOpen] = useState(false); const [query, setQuery] = useState(''); + const [tab, setTab] = useState('branches'); const searchRef = useRef(null); const { local, remote } = availableBranches; const commits = recentCommits ?? []; + const hasCommits = commits.length > 0; const filtered = useMemo(() => { const q = query.trim().toLowerCase(); @@ -74,32 +78,78 @@ export const BaseBranchPicker: React.FC = ({ !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 noResults = - filtered.local.length === 0 && - filtered.remote.length === 0 && - filtered.commits.length === 0 && - !showUseAsBase; - const isCustom = selectedBase !== detectedBase; + const branchesContent = ( + <> + {filtered.local.length === 0 && filtered.remote.length === 0 ? ( +
{copy.emptyText}
+ ) : ( + <> + {filtered.local.length > 0 && ( + + )} + {filtered.remote.length > 0 && ( + + )} + + )} + + ); + + const commitsContent = ( + <> + {filtered.commits.length === 0 ? ( +
No matching commits.
+ ) : ( + + )} + + ); + return ( { setOpen(v); - if (!v) setQuery(''); + if (!v) { + setQuery(''); + setTab('branches'); + } }} > @@ -145,7 +195,7 @@ export const BaseBranchPicker: React.FC = ({ 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. @@ -157,55 +207,34 @@ export const BaseBranchPicker: React.FC = ({ 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" /> + {showUseAsBase && ( +
+ +
+ )} + {hasCommits && ( +
+ setTab('branches')}> + Branches + + setTab('commits')}> + Commits + +
+ )}
- {showUseAsBase && ( -
- -
- )} - {noResults && ( -
- {copy.emptyText} -
- )} - {filtered.local.length > 0 && ( - - )} - {filtered.remote.length > 0 && ( - - )} - {filtered.commits.length > 0 && ( - - )} + {hasCommits && tab === 'commits' ? commitsContent : branchesContent}
{isCustom && (
@@ -224,6 +253,26 @@ export const BaseBranchPicker: React.FC = ({ ); }; +interface TabButtonProps { + active: boolean; + onClick: () => void; + children: React.ReactNode; +} + +const TabButton: React.FC = ({ active, onClick, children }) => ( + +); + interface BranchGroupProps { title: string; branches: string[]; @@ -274,18 +323,14 @@ const BranchGroup: React.FC = ({
); -interface CommitGroupProps { - title: string; +interface CommitListProps { commits: RecentCommit[]; selectedBase: string; onSelect: (sha: string) => void; } -const CommitGroup: React.FC = ({ title, commits, selectedBase, onSelect }) => ( +const CommitList: React.FC = ({ commits, selectedBase, onSelect }) => (
-
- {title} -
{commits.map((c) => { const isSelected = c.sha === selectedBase || c.shortSha === selectedBase; return (