Skip to content

Commit 0a84ea5

Browse files
committed
feat(review): add jj review workflows
1 parent 8705e97 commit 0a84ea5

7 files changed

Lines changed: 64 additions & 33 deletions

File tree

packages/review-editor/App.tsx

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ const ReviewApp: React.FC = () => {
760760
// Prefer the server's active base (survives page refresh / reconnect)
761761
// over the detected default, so the picker rehydrates to what the
762762
// server is actually using.
763-
const initial = data.base || data.gitContext.defaultBranch || null;
763+
const initial = data.base || data.gitContext.defaultBranch || data.gitContext.compareTarget?.fallback || null;
764764
setSelectedBase(initial);
765765
setCommittedBase(initial);
766766
}
@@ -782,7 +782,7 @@ const ReviewApp: React.FC = () => {
782782
if (data.error) setDiffError(data.error);
783783
if (data.isWSL) setIsWSL(true);
784784
// Mark diff type setup as pending on first run (local mode only)
785-
if (data.diffType && !data.prMetadata && data.gitContext?.vcsType !== 'p4' && needsDiffTypeSetup()) {
785+
if (data.diffType && !data.prMetadata && data.gitContext?.vcsType !== 'p4' && data.gitContext?.vcsType !== 'jj' && needsDiffTypeSetup()) {
786786
setDiffTypeSetupPending(true);
787787
}
788788
})
@@ -1163,6 +1163,7 @@ const ReviewApp: React.FC = () => {
11631163
...prev,
11641164
defaultBranch: data.gitContext!.defaultBranch,
11651165
diffOptions: data.gitContext!.diffOptions,
1166+
compareTarget: data.gitContext!.compareTarget,
11661167
};
11671168
});
11681169
}
@@ -1190,7 +1191,7 @@ const ReviewApp: React.FC = () => {
11901191
if (branch === selectedBase) return;
11911192
const previous = selectedBase;
11921193
setSelectedBase(branch);
1193-
if (activeDiffBase === 'branch' || activeDiffBase === 'merge-base') {
1194+
if (activeDiffBase === 'branch' || activeDiffBase === 'merge-base' || activeDiffBase === 'jj-line') {
11941195
const ok = await fetchDiffSwitch(diffType, branch);
11951196
if (!ok) setSelectedBase(previous);
11961197
}
@@ -1304,7 +1305,7 @@ const ReviewApp: React.FC = () => {
13041305
// the new patch to arrive before refetching — otherwise the viewer can
13051306
// briefly pair an old patch with the new base's content.
13061307
reviewBase:
1307-
(activeDiffBase === 'branch' || activeDiffBase === 'merge-base')
1308+
(activeDiffBase === 'branch' || activeDiffBase === 'merge-base' || activeDiffBase === 'jj-line')
13081309
? committedBase ?? undefined
13091310
: undefined,
13101311
activeDiffBase,
@@ -2078,8 +2079,9 @@ const ReviewApp: React.FC = () => {
20782079
currentBranch={gitContext?.currentBranch}
20792080
availableBranches={prMetadata ? undefined : gitContext?.availableBranches}
20802081
selectedBase={prMetadata ? undefined : selectedBase ?? undefined}
2081-
detectedBase={prMetadata ? undefined : gitContext?.defaultBranch}
2082+
detectedBase={prMetadata ? undefined : gitContext?.defaultBranch || gitContext?.compareTarget?.fallback}
20822083
onSelectBase={prMetadata ? undefined : handleBaseSelect}
2084+
compareTarget={gitContext?.compareTarget}
20832085
stagedFiles={stagedFiles}
20842086
onCopyRawDiff={handleCopyDiff}
20852087
canCopyRawDiff={!!diffData?.rawPatch}
@@ -2156,6 +2158,10 @@ const ReviewApp: React.FC = () => {
21562158
{activeDiffBase === 'staged' && "No staged changes. Stage some files with git add."}
21572159
{activeDiffBase === 'unstaged' && "No unstaged changes. All changes are staged."}
21582160
{activeDiffBase === 'last-commit' && `No changes in the last commit${activeWorktreePath ? ' in this worktree' : ''}.`}
2161+
{activeDiffBase === 'jj-current' && "No changes in the current jj change."}
2162+
{activeDiffBase === 'jj-last' && "No changes in the last jj change."}
2163+
{activeDiffBase === 'jj-line' && `No changes in your line of work vs ${selectedBase || gitContext?.defaultBranch || '@-'}.`}
2164+
{activeDiffBase === 'jj-all' && "No files at the current jj change."}
21592165
{activeDiffBase === 'branch' && `No changes vs ${selectedBase || gitContext?.defaultBranch || 'main'}${activeWorktreePath ? ' in this worktree' : ''}.`}
21602166
{activeDiffBase === 'merge-base' && `No changes vs ${selectedBase || gitContext?.defaultBranch || 'main'}${activeWorktreePath ? ' in this worktree' : ''}.`}
21612167
{activeDiffBase === 'all' && `No tracked files${activeWorktreePath ? ' in this worktree' : ' in this repository'}.`}

packages/review-editor/components/BaseBranchPicker.tsx

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import React, { useMemo, useRef, useState } from 'react';
22
import * as Popover from '@radix-ui/react-popover';
3-
import type { AvailableBranches } from '@plannotator/shared/types';
3+
import type { AvailableBranches, CompareTargetPickerCopy } from '@plannotator/shared/types';
44

55
interface BaseBranchPickerProps {
66
availableBranches: AvailableBranches;
77
selectedBase: string;
88
detectedBase: string;
99
onSelectBase: (branch: string) => void;
1010
disabled?: boolean;
11+
copy: CompareTargetPickerCopy;
1112
}
1213

1314
export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
@@ -16,6 +17,7 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
1617
detectedBase,
1718
onSelectBase,
1819
disabled,
20+
copy,
1921
}) => {
2022
const [open, setOpen] = useState(false);
2123
const [query, setQuery] = useState('');
@@ -57,15 +59,15 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
5759
<button
5860
type="button"
5961
disabled={disabled}
60-
title={`Review base: ${selectedBase}`}
62+
title={`${copy.triggerTitlePrefix}: ${selectedBase}`}
6163
className={`w-full flex items-center gap-1.5 px-2.5 py-1.5 rounded text-xs font-medium transition-colors focus:outline-none focus:ring-1 focus:ring-primary/50 disabled:opacity-50 disabled:cursor-not-allowed ${
6264
isCustom
6365
? 'bg-primary/10 border border-primary/30 text-foreground'
6466
: 'bg-muted border border-transparent text-foreground'
6567
}`}
6668
>
6769
<span className="text-[10px] uppercase tracking-wide opacity-60 flex-shrink-0">
68-
base
70+
{copy.triggerLabel}
6971
</span>
7072
<span className="truncate flex-1 text-left">{selectedBase}</span>
7173
<svg
@@ -96,19 +98,19 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
9698
type="text"
9799
value={query}
98100
onChange={(e) => setQuery(e.target.value)}
99-
placeholder="Search branches…"
101+
placeholder={copy.searchPlaceholder}
100102
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"
101103
/>
102104
</div>
103105
<div className="max-h-72 overflow-y-auto py-1">
104106
{filtered.local.length === 0 && filtered.remote.length === 0 && (
105107
<div className="px-3 py-2 text-xs text-muted-foreground">
106-
No branches match.
108+
{copy.emptyText}
107109
</div>
108110
)}
109111
{filtered.local.length > 0 && (
110112
<BranchGroup
111-
title="Local"
113+
title={copy.localGroupLabel}
112114
branches={filtered.local}
113115
selectedBase={selectedBase}
114116
detectedBase={detectedBase}
@@ -117,7 +119,7 @@ export const BaseBranchPicker: React.FC<BaseBranchPickerProps> = ({
117119
)}
118120
{filtered.remote.length > 0 && (
119121
<BranchGroup
120-
title="Remote"
122+
title={copy.remoteGroupLabel}
121123
branches={filtered.remote}
122124
selectedBase={selectedBase}
123125
detectedBase={detectedBase}

packages/review-editor/components/DiffTypePicker.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ const OPTION_HINTS: Record<string, string> = {
2020
staged: "Only what you've run `git add` on.",
2121
unstaged: "What `git diff` shows with no arguments.",
2222
'last-commit': "Just your most recent commit.",
23+
'jj-current': "Changes in the current jj change.",
24+
'jj-last': "Changes in the previous jj change.",
25+
'jj-line': "Changes in your line of work from the selected bookmark or revision.",
26+
'jj-all': "Every tracked file in the current jj workspace, shown as additions.",
2327
branch: "Straight compare against the base branch (picked below). Can show commits that aren't yours if the base has new commits.",
2428
'merge-base': "Only what you've added on top of the base branch (picked below). Same as GitHub's PR view.",
2529
all: "Every tracked file at HEAD, shown as additions. Unlike Committed, which shows what changed vs a base branch, this shows the entire codebase.",

packages/review-editor/components/FileTree.tsx

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React, { useEffect, useCallback, useState, useMemo } from 'react';
22
import { CodeAnnotation } from '@plannotator/ui/types';
3-
import type { AvailableBranches, DiffOption, WorktreeInfo } from '@plannotator/shared/types';
3+
import type { AvailableBranches, CompareTargetConfig, DiffOption, WorktreeInfo } from '@plannotator/shared/types';
44
import { buildFileTree, getAncestorPaths, getAllFolderPaths, getVisualFileOrder } from '../utils/buildFileTree';
55
import { FileTreeNodeItem } from './FileTreeNode';
66
import { BaseBranchPicker } from './BaseBranchPicker';
@@ -30,11 +30,12 @@ interface FileTreeProps {
3030
activeWorktreePath?: string | null;
3131
onSelectWorktree?: (path: string | null) => void;
3232
currentBranch?: string;
33-
/** Base-branch picker — only meaningful when activeDiffType is "branch" or "merge-base". */
33+
/** Compare target picker — base branch for Git, bookmark/revision for jj. */
3434
availableBranches?: AvailableBranches;
3535
selectedBase?: string;
3636
detectedBase?: string;
3737
onSelectBase?: (branch: string) => void;
38+
compareTarget?: CompareTargetConfig;
3839
stagedFiles?: Set<string>;
3940
onCopyRawDiff?: () => void;
4041
canCopyRawDiff?: boolean;
@@ -83,6 +84,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
8384
selectedBase,
8485
detectedBase,
8586
onSelectBase,
87+
compareTarget,
8688
stagedFiles,
8789
onCopyRawDiff,
8890
canCopyRawDiff = false,
@@ -366,15 +368,16 @@ export const FileTree: React.FC<FileTreeProps> = ({
366368
</div>
367369
)}
368370

369-
{/* Base-branch picker — only relevant for branch / merge-base diff types */}
371+
{/* Compare target picker — only relevant for base-dependent diff types */}
370372
{onSelectBase &&
371373
selectedBase &&
372374
detectedBase &&
373375
availableBranches &&
374-
(activeDiffType === 'branch' || activeDiffType === 'merge-base') && (
376+
activeDiffType &&
377+
compareTarget?.diffTypes.includes(activeDiffType) && (
375378
<div className="px-2 py-1.5 border-b border-border/30 flex items-center gap-2">
376379
<span className="text-[10px] uppercase tracking-wide text-muted-foreground flex-shrink-0">
377-
compare against
380+
{compareTarget.picker.rowLabel}
378381
</span>
379382
<div className="flex-1 min-w-0">
380383
<BaseBranchPicker
@@ -383,6 +386,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
383386
detectedBase={detectedBase}
384387
onSelectBase={onSelectBase}
385388
disabled={isLoadingDiff}
389+
copy={compareTarget.picker}
386390
/>
387391
</div>
388392
</div>

packages/review-editor/utils/exportFeedback.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ describe("exportReviewFeedback", () => {
6969
expect(result).toContain("**Diff:** Committed changes vs `release/v2`");
7070
});
7171

72+
it("local mode with jj line of work: labels compare target in the header", () => {
73+
const result = exportReviewFeedback([ann()], undefined, {
74+
mode: "jj-line",
75+
base: "main",
76+
});
77+
expect(result).toContain("**Diff:** Line of work vs `main`");
78+
});
79+
7280
it("local mode with worktree path: appends worktree info", () => {
7381
const result = exportReviewFeedback([ann()], undefined, {
7482
mode: "uncommitted",

packages/review-editor/utils/exportFeedback.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ function describeDiff(ctx: FeedbackDiffContext): string {
3636
case "staged": label = "Staged changes"; break;
3737
case "unstaged": label = "Unstaged changes"; break;
3838
case "last-commit": label = "Last commit"; break;
39+
case "jj-current": label = "Current change"; break;
40+
case "jj-last": label = "Last change"; break;
41+
case "jj-line": label = base ? `Line of work vs \`${base}\`` : "Line of work"; break;
42+
case "jj-all": label = "All files"; break;
3943
case "branch": label = base ? `Branch diff vs \`${base}\`` : "Branch diff"; break;
4044
case "merge-base": label = base ? `Committed changes vs \`${base}\`` : "Committed changes"; break;
4145
case "all": label = "All files"; break;

packages/server/review.ts

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111

1212
import { isRemoteSession, getServerHostname, getServerPort } from "./remote";
1313
import type { Origin } from "@plannotator/shared/agents";
14-
import { type DiffType, type GitContext, runVcsDiff, getVcsFileContentsForDiff, canStageFiles, stageFile, unstageFile, resolveVcsCwd, validateFilePath, getVcsContext, gitRuntime } from "./vcs";
15-
import { parseWorktreeDiffType, detectRemoteDefaultBranch, resolveBaseBranch } from "@plannotator/shared/review-core";
14+
import { type DiffType, type GitContext, runVcsDiff, getVcsFileContentsForDiff, canStageFiles, stageFile, unstageFile, resolveVcsCwd, validateFilePath, getVcsContext, detectRemoteDefaultCompareTarget, gitRuntime } from "./vcs";
15+
import { parseWorktreeDiffType, resolveBaseBranch } from "@plannotator/shared/review-core";
1616
import {
1717
getPRDiffScopeOptions,
1818
getPRStackInfo,
@@ -164,15 +164,20 @@ export async function startReviewServer(
164164
// read this (not gitContext.defaultBranch) so they analyze the same diff
165165
// the reviewer is currently looking at. Honors an explicit initialBase from
166166
// the caller — e.g. programmatic Pi callers can request a non-detected base.
167-
let currentBase = options.initialBase || gitContext?.defaultBranch || "main";
167+
const detectedCompareTarget = (): string => gitContext?.defaultBranch || gitContext?.compareTarget?.fallback || "main";
168+
let currentBase = options.initialBase || detectedCompareTarget();
168169
let baseEverSwitched = false;
169170

171+
const resolveReviewBase = (requestedBase?: string): string => {
172+
return resolveBaseBranch(requestedBase, detectedCompareTarget());
173+
};
174+
170175
// Fire-and-forget: query the remote for its actual default branch. If it
171176
// arrives before the user interacts, quietly upgrade currentBase from the
172177
// local fallback (e.g. "main") to the upstream ref (e.g. "origin/main").
173178
// Non-blocking — the server is already listening by the time this resolves.
174179
if (gitContext && !options.initialBase && !isPRMode) {
175-
detectRemoteDefaultBranch(gitRuntime, gitContext.cwd).then((remote) => {
180+
detectRemoteDefaultCompareTarget(gitContext.cwd).then((remote) => {
176181
if (remote && !baseEverSwitched) currentBase = remote;
177182
});
178183
}
@@ -415,6 +420,9 @@ export async function startReviewServer(
415420
let repoInfo = isPRMode && prMetadata
416421
? { display: getDisplayRepo(prMetadata), branch: `${getMRLabel(prMetadata)} ${getMRNumberLabel(prMetadata)}` }
417422
: await getRepoInfo();
423+
if (gitContext?.repository?.displayFallback) {
424+
repoInfo = { display: repoInfo?.display || gitContext.repository.displayFallback };
425+
}
418426

419427
// Fetch current platform user (for own-PR/MR detection)
420428
let prRef = isPRMode && prMetadata ? prRefFromMetadata(prMetadata) : null;
@@ -560,12 +568,11 @@ export async function startReviewServer(
560568
currentHideWhitespace = body.hideWhitespace;
561569
}
562570

563-
const detectedBase = gitContext?.defaultBranch || "main";
564571
// Guard against non-string payloads — resolveBaseBranch calls
565572
// string methods and would throw a TypeError otherwise. Mirrors
566573
// Pi's guard so both runtimes validate identically.
567574
const requestedBase = typeof body.base === "string" ? body.base : undefined;
568-
const base = resolveBaseBranch(requestedBase, detectedBase);
575+
const base = resolveReviewBase(requestedBase);
569576
const defaultCwd = gitContext?.cwd;
570577

571578
// Run the new diff
@@ -863,11 +870,8 @@ export async function startReviewServer(
863870

864871
// Local review: read file contents from local git
865872
if (hasLocalAccess) {
866-
const detectedBase = gitContext?.defaultBranch || "main";
867-
const base = resolveBaseBranch(
868-
url.searchParams.get("base") ?? undefined,
869-
detectedBase,
870-
);
873+
const requestedBase = url.searchParams.get("base") ?? undefined;
874+
const base = resolveReviewBase(requestedBase);
871875
const defaultCwd = gitContext?.cwd;
872876
const result = await getVcsFileContentsForDiff(
873877
currentDiffType,
@@ -896,7 +900,8 @@ export async function startReviewServer(
896900

897901
// API: Stage / unstage a file (disabled when VCS doesn't support it)
898902
if (url.pathname === "/api/git-add" && req.method === "POST") {
899-
if (isPRMode || !canStageFiles(currentDiffType)) {
903+
const stageCwd = resolveVcsCwd(currentDiffType, gitContext?.cwd);
904+
if (isPRMode || !(await canStageFiles(currentDiffType, stageCwd))) {
900905
return Response.json(
901906
{ error: "Staging not available" },
902907
{ status: 400 },
@@ -908,12 +913,10 @@ export async function startReviewServer(
908913
return Response.json({ error: "Missing filePath" }, { status: 400 });
909914
}
910915

911-
const cwd = resolveVcsCwd(currentDiffType, gitContext?.cwd);
912-
913916
if (body.undo) {
914-
await unstageFile(currentDiffType, body.filePath, cwd);
917+
await unstageFile(currentDiffType, body.filePath, stageCwd);
915918
} else {
916-
await stageFile(currentDiffType, body.filePath, cwd);
919+
await stageFile(currentDiffType, body.filePath, stageCwd);
917920
}
918921

919922
return Response.json({ ok: true });

0 commit comments

Comments
 (0)