feat: Redesign review tab with phase selection, commit navigation, and consolidated toolbar

- Add BranchManager.listCommits() and diffCommit() for commit-level navigation
- Add getPhaseReviewCommits and getCommitDiff tRPC procedures
- New ReviewHeader: consolidated toolbar with phase selector pills, branch info,
  stats, integrated preview controls, and approve/reject actions
- New CommitNav: horizontal commit strip with "All changes" + individual commits,
  each showing hash, message, and change stats
- Slim down ReviewSidebar: file list only with dimming for out-of-scope files
  when viewing a single commit
- ReviewTab orchestrates all pieces in a single bordered card layout
This commit is contained in:
Lukas May
2026-03-05 10:20:43 +01:00
parent f91ed5ab2d
commit c58e0ea77e
12 changed files with 739 additions and 225 deletions

View File

@@ -6,7 +6,7 @@
* a worktree to be checked out. * a worktree to be checked out.
*/ */
import type { MergeResult } from './types.js'; import type { MergeResult, BranchCommit } from './types.js';
export interface BranchManager { export interface BranchManager {
/** /**
@@ -45,4 +45,15 @@ export interface BranchManager {
* since local branches may not include all remote branches. * since local branches may not include all remote branches.
*/ */
remoteBranchExists(repoPath: string, branch: string): Promise<boolean>; remoteBranchExists(repoPath: string, branch: string): Promise<boolean>;
/**
* List commits that headBranch has but baseBranch doesn't.
* Used for commit-level navigation in code review.
*/
listCommits(repoPath: string, baseBranch: string, headBranch: string): Promise<BranchCommit[]>;
/**
* Get the raw unified diff for a single commit.
*/
diffCommit(repoPath: string, commitHash: string): Promise<string>;
} }

View File

@@ -11,7 +11,7 @@ import { mkdtempSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os'; import { tmpdir } from 'node:os';
import { simpleGit } from 'simple-git'; import { simpleGit } from 'simple-git';
import type { BranchManager } from './branch-manager.js'; import type { BranchManager } from './branch-manager.js';
import type { MergeResult } from './types.js'; import type { MergeResult, BranchCommit } from './types.js';
import { createModuleLogger } from '../logger/index.js'; import { createModuleLogger } from '../logger/index.js';
const log = createModuleLogger('branch-manager'); const log = createModuleLogger('branch-manager');
@@ -116,4 +116,28 @@ export class SimpleGitBranchManager implements BranchManager {
return false; return false;
} }
} }
async listCommits(repoPath: string, baseBranch: string, headBranch: string): Promise<BranchCommit[]> {
const git = simpleGit(repoPath);
const logResult = await git.log({ from: baseBranch, to: headBranch, '--stat': null });
return logResult.all.map((entry) => {
const diffStat = entry.diff;
return {
hash: entry.hash,
shortHash: entry.hash.slice(0, 7),
message: entry.message,
author: entry.author_name,
date: entry.date,
filesChanged: diffStat?.files?.length ?? 0,
insertions: diffStat?.insertions ?? 0,
deletions: diffStat?.deletions ?? 0,
};
});
}
async diffCommit(repoPath: string, commitHash: string): Promise<string> {
const git = simpleGit(repoPath);
return git.diff([`${commitHash}~1`, commitHash]);
}
} }

View File

@@ -58,6 +58,33 @@ export interface MergeResult {
message: string; message: string;
} }
// =============================================================================
// Branch Commit Info
// =============================================================================
/**
* Represents a single commit in a branch's history.
* Used for commit-level navigation in code review.
*/
export interface BranchCommit {
/** Full commit hash */
hash: string;
/** Short commit hash (7 chars) */
shortHash: string;
/** First line of commit message */
message: string;
/** Author name */
author: string;
/** ISO date string */
date: string;
/** Number of files changed */
filesChanged: number;
/** Number of insertions */
insertions: number;
/** Number of deletions */
deletions: number;
}
// ============================================================================= // =============================================================================
// WorktreeManager Port Interface // WorktreeManager Port Interface
// ============================================================================= // =============================================================================

View File

@@ -234,5 +234,69 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
await orchestrator.approveAndMergePhase(input.phaseId); await orchestrator.approveAndMergePhase(input.phaseId);
return { success: true }; return { success: true };
}), }),
getPhaseReviewCommits: publicProcedure
.input(z.object({ phaseId: z.string().min(1) }))
.query(async ({ ctx, input }) => {
const phaseRepo = requirePhaseRepository(ctx);
const initiativeRepo = requireInitiativeRepository(ctx);
const projectRepo = requireProjectRepository(ctx);
const branchManager = requireBranchManager(ctx);
const phase = await phaseRepo.findById(input.phaseId);
if (!phase) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Phase '${input.phaseId}' not found` });
}
if (phase.status !== 'pending_review') {
throw new TRPCError({ code: 'BAD_REQUEST', message: `Phase is not pending review (status: ${phase.status})` });
}
const initiative = await initiativeRepo.findById(phase.initiativeId);
if (!initiative?.branch) {
throw new TRPCError({ code: 'BAD_REQUEST', message: 'Initiative has no branch configured' });
}
const initBranch = initiative.branch;
const phBranch = phaseBranchName(initBranch, phase.name);
const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId);
const allCommits: Array<{ hash: string; shortHash: string; message: string; author: string; date: string; filesChanged: number; insertions: number; deletions: number }> = [];
for (const project of projects) {
const clonePath = await ensureProjectClone(project, ctx.workspaceRoot!);
const commits = await branchManager.listCommits(clonePath, initBranch, phBranch);
allCommits.push(...commits);
}
return { commits: allCommits, sourceBranch: phBranch, targetBranch: initBranch };
}),
getCommitDiff: publicProcedure
.input(z.object({ phaseId: z.string().min(1), commitHash: z.string().min(1) }))
.query(async ({ ctx, input }) => {
const phaseRepo = requirePhaseRepository(ctx);
const projectRepo = requireProjectRepository(ctx);
const branchManager = requireBranchManager(ctx);
const phase = await phaseRepo.findById(input.phaseId);
if (!phase) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Phase '${input.phaseId}' not found` });
}
const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId);
let rawDiff = '';
for (const project of projects) {
const clonePath = await ensureProjectClone(project, ctx.workspaceRoot!);
try {
const diff = await branchManager.diffCommit(clonePath, input.commitHash);
if (diff) rawDiff += diff + '\n';
} catch {
// commit not in this project clone
}
}
return { rawDiff };
}),
}; };
} }

View File

@@ -0,0 +1,116 @@
import { GitCommitHorizontal, Plus, Minus, FileCode } from "lucide-react";
import type { CommitInfo } from "./types";
interface CommitNavProps {
commits: CommitInfo[];
selectedCommit: string | null; // null = "all changes"
onSelectCommit: (hash: string | null) => void;
isLoading: boolean;
}
export function CommitNav({
commits,
selectedCommit,
onSelectCommit,
isLoading,
}: CommitNavProps) {
if (isLoading || commits.length === 0) return null;
// Don't show navigator for single-commit phases
if (commits.length === 1) return null;
return (
<div className="border-b border-border/50 bg-muted/20">
<div className="flex items-center gap-0 px-4 overflow-x-auto">
{/* "All changes" pill */}
<CommitPill
label="All changes"
sublabel={`${commits.length} commits`}
isActive={selectedCommit === null}
onClick={() => onSelectCommit(null)}
/>
<div className="w-px h-5 bg-border mx-1 shrink-0" />
{/* Individual commit pills - most recent first */}
{commits.map((commit) => (
<CommitPill
key={commit.hash}
label={commit.shortHash}
sublabel={truncateMessage(commit.message)}
stats={{ files: commit.filesChanged, add: commit.insertions, del: commit.deletions }}
isActive={selectedCommit === commit.hash}
onClick={() => onSelectCommit(commit.hash)}
isMono
/>
))}
</div>
</div>
);
}
interface CommitPillProps {
label: string;
sublabel: string;
stats?: { files: number; add: number; del: number };
isActive: boolean;
onClick: () => void;
isMono?: boolean;
}
function CommitPill({
label,
sublabel,
stats,
isActive,
onClick,
isMono,
}: CommitPillProps) {
return (
<button
onClick={onClick}
className={`
flex items-center gap-2 px-3 py-2 text-[11px] whitespace-nowrap
transition-colors duration-fast border-b-2 shrink-0
${isActive
? "border-primary text-foreground"
: "border-transparent text-muted-foreground hover:text-foreground hover:bg-muted/50"
}
`}
>
{isMono ? (
<GitCommitHorizontal className="h-3 w-3 shrink-0 text-muted-foreground" />
) : null}
<span className={isMono ? "font-mono font-medium" : "font-medium"}>
{label}
</span>
<span className="text-muted-foreground truncate max-w-[180px]">
{sublabel}
</span>
{stats && stats.files > 0 && (
<span className="flex items-center gap-1.5 text-[10px]">
<span className="flex items-center gap-0.5 text-muted-foreground">
<FileCode className="h-2.5 w-2.5" />
{stats.files}
</span>
<span className="flex items-center gap-0 text-diff-add-fg">
<Plus className="h-2.5 w-2.5" />
{stats.add}
</span>
<span className="flex items-center gap-0 text-diff-remove-fg">
<Minus className="h-2.5 w-2.5" />
{stats.del}
</span>
</span>
)}
</button>
);
}
function truncateMessage(msg: string): string {
const firstLine = msg.split("\n")[0];
return firstLine.length > 50 ? firstLine.slice(0, 47) + "..." : firstLine;
}

View File

@@ -0,0 +1,255 @@
import {
Check,
X,
GitBranch,
FileCode,
Plus,
Minus,
ExternalLink,
Loader2,
Square,
CircleDot,
RotateCcw,
ArrowRight,
} from "lucide-react";
import { Button } from "@/components/ui/button";
import { Badge } from "@/components/ui/badge";
import type { FileDiff, ReviewStatus } from "./types";
interface PhaseOption {
id: string;
name: string;
}
interface PreviewState {
status: "idle" | "building" | "running" | "failed";
url?: string;
onStart: () => void;
onStop: () => void;
isStarting: boolean;
isStopping: boolean;
}
interface ReviewHeaderProps {
phases: PhaseOption[];
activePhaseId: string | null;
onPhaseSelect: (id: string) => void;
phaseName: string;
sourceBranch: string;
targetBranch: string;
files: FileDiff[];
status: ReviewStatus;
unresolvedCount: number;
onApprove: () => void;
onRequestChanges: () => void;
preview: PreviewState | null;
}
export function ReviewHeader({
phases,
activePhaseId,
onPhaseSelect,
phaseName,
sourceBranch,
targetBranch,
files,
status,
unresolvedCount,
onApprove,
onRequestChanges,
preview,
}: ReviewHeaderProps) {
const totalAdditions = files.reduce((s, f) => s + f.additions, 0);
const totalDeletions = files.reduce((s, f) => s + f.deletions, 0);
return (
<div className="border-b border-border bg-card/80 backdrop-blur-sm">
{/* Phase selector row */}
{phases.length > 1 && (
<div className="flex items-center gap-1 px-4 pt-3 pb-2 border-b border-border/50">
<span className="text-[10px] font-medium text-muted-foreground uppercase tracking-wider mr-2 shrink-0">
Phases
</span>
<div className="flex gap-1 overflow-x-auto">
{phases.map((phase) => {
const isActive = phase.id === activePhaseId;
return (
<button
key={phase.id}
onClick={() => onPhaseSelect(phase.id)}
className={`
flex items-center gap-1.5 rounded-md px-2.5 py-1 text-xs font-medium
transition-all duration-fast whitespace-nowrap
${isActive
? "bg-primary/10 text-primary border border-primary/20 shadow-xs"
: "text-muted-foreground hover:text-foreground hover:bg-muted border border-transparent"
}
`}
>
<span
className={`h-1.5 w-1.5 rounded-full shrink-0 ${
isActive ? "bg-primary" : "bg-status-warning-dot"
}`}
/>
{phase.name}
</button>
);
})}
</div>
</div>
)}
{/* Main toolbar row */}
<div className="flex items-center gap-3 px-4 py-2.5">
{/* Left: branch info + stats */}
<div className="flex items-center gap-3 min-w-0 flex-1">
<h2 className="text-sm font-semibold truncate shrink-0">
{phaseName}
</h2>
{sourceBranch && (
<div className="flex items-center gap-1 text-[11px] text-muted-foreground font-mono shrink-0">
<GitBranch className="h-3 w-3 shrink-0" />
<span className="truncate max-w-[140px]" title={sourceBranch}>
{truncateBranch(sourceBranch)}
</span>
<ArrowRight className="h-2.5 w-2.5 shrink-0 text-muted-foreground/50" />
<span className="truncate max-w-[140px]" title={targetBranch}>
{truncateBranch(targetBranch)}
</span>
</div>
)}
<div className="flex items-center gap-2.5 text-[11px] shrink-0">
<span className="flex items-center gap-0.5 text-muted-foreground">
<FileCode className="h-3 w-3" />
{files.length}
</span>
<span className="flex items-center gap-0.5 text-diff-add-fg">
<Plus className="h-3 w-3" />
{totalAdditions}
</span>
<span className="flex items-center gap-0.5 text-diff-remove-fg">
<Minus className="h-3 w-3" />
{totalDeletions}
</span>
</div>
</div>
{/* Right: preview + actions */}
<div className="flex items-center gap-2 shrink-0">
{/* Preview controls */}
{preview && <PreviewControls preview={preview} />}
{/* Review status / actions */}
{status === "pending" && (
<>
<Button
variant="outline"
size="sm"
onClick={onRequestChanges}
className="h-7 text-xs"
>
<X className="h-3 w-3" />
Request Changes
</Button>
<Button
size="sm"
onClick={onApprove}
disabled={unresolvedCount > 0}
className="h-7 text-xs"
>
<Check className="h-3 w-3" />
{unresolvedCount > 0
? `${unresolvedCount} unresolved`
: "Approve & Merge"}
</Button>
</>
)}
{status === "approved" && (
<Badge variant="success" size="xs">
<Check className="h-3 w-3" />
Approved
</Badge>
)}
{status === "changes_requested" && (
<Badge variant="warning" size="xs">
<X className="h-3 w-3" />
Changes Requested
</Badge>
)}
</div>
</div>
</div>
);
}
function PreviewControls({ preview }: { preview: PreviewState }) {
if (preview.status === "building" || preview.isStarting) {
return (
<div className="flex items-center gap-1.5 text-xs text-status-active-fg">
<Loader2 className="h-3 w-3 animate-spin" />
<span>Building...</span>
</div>
);
}
if (preview.status === "running") {
return (
<div className="flex items-center gap-1.5">
<a
href={preview.url}
target="_blank"
rel="noopener noreferrer"
className="flex items-center gap-1 text-xs text-status-success-fg hover:underline"
>
<CircleDot className="h-3 w-3" />
Preview
<ExternalLink className="h-2.5 w-2.5" />
</a>
<Button
variant="ghost"
size="sm"
onClick={preview.onStop}
disabled={preview.isStopping}
className="h-6 w-6 p-0"
>
<Square className="h-2.5 w-2.5" />
</Button>
</div>
);
}
if (preview.status === "failed") {
return (
<Button
variant="ghost"
size="sm"
onClick={preview.onStart}
className="h-7 text-xs text-status-error-fg"
>
<RotateCcw className="h-3 w-3" />
Retry Preview
</Button>
);
}
return (
<Button
variant="ghost"
size="sm"
onClick={preview.onStart}
disabled={preview.isStarting}
className="h-7 text-xs"
>
<ExternalLink className="h-3 w-3" />
Preview
</Button>
);
}
function truncateBranch(branch: string): string {
const parts = branch.split("/");
if (parts.length <= 2) return branch;
return parts.slice(-2).join("/");
}

View File

@@ -1,186 +1,109 @@
import { import {
Check,
X,
MessageSquare, MessageSquare,
GitBranch,
FileCode, FileCode,
Plus, Plus,
Minus, Minus,
Circle, Circle,
CheckCircle2, CheckCircle2,
} from "lucide-react"; } from "lucide-react";
import { Button } from "@/components/ui/button"; import type { FileDiff, ReviewComment } from "./types";
import { Badge } from "@/components/ui/badge";
import type { FileDiff, ReviewComment, ReviewStatus } from "./types";
interface ReviewSidebarProps { interface ReviewSidebarProps {
title: string;
description: string;
author: string;
status: ReviewStatus;
sourceBranch: string;
targetBranch: string;
files: FileDiff[]; files: FileDiff[];
comments: ReviewComment[]; comments: ReviewComment[];
onApprove: () => void;
onRequestChanges: () => void;
onFileClick: (filePath: string) => void; onFileClick: (filePath: string) => void;
selectedCommit: string | null;
activeFiles: FileDiff[];
} }
export function ReviewSidebar({ export function ReviewSidebar({
title,
description,
author,
status,
sourceBranch,
targetBranch,
files, files,
comments, comments,
onApprove,
onRequestChanges,
onFileClick, onFileClick,
selectedCommit,
activeFiles,
}: ReviewSidebarProps) { }: ReviewSidebarProps) {
const unresolvedCount = comments.filter((c) => !c.resolved).length; const unresolvedCount = comments.filter((c) => !c.resolved).length;
const resolvedCount = comments.filter((c) => c.resolved).length; const resolvedCount = comments.filter((c) => c.resolved).length;
const totalAdditions = files.reduce((s, f) => s + f.additions, 0);
const totalDeletions = files.reduce((s, f) => s + f.deletions, 0); // Build a set of files visible in the current diff view
const activeFilePaths = new Set(activeFiles.map((f) => f.newPath));
return ( return (
<div className="space-y-5"> <div className="space-y-4">
{/* Review info */}
<div className="space-y-3">
<div className="flex items-start justify-between gap-2">
<h3 className="text-sm font-semibold leading-tight">{title}</h3>
<ReviewStatusBadge status={status} />
</div>
<p className="text-xs text-muted-foreground leading-relaxed">
{description}
</p>
<div className="flex items-center gap-1.5 text-xs text-muted-foreground">
<span className="font-medium text-foreground">{author}</span>
</div>
<div className="flex items-center gap-1.5 text-xs text-muted-foreground font-mono">
<GitBranch className="h-3 w-3" />
<span>{sourceBranch}</span>
<span className="text-muted-foreground/50">&rarr;</span>
<span>{targetBranch}</span>
</div>
</div>
{/* Actions */}
<div className="space-y-2">
{status === "pending" && (
<>
<Button
className="w-full"
size="sm"
onClick={onApprove}
disabled={unresolvedCount > 0}
>
<Check className="h-3.5 w-3.5 mr-1" />
{unresolvedCount > 0
? `Resolve ${unresolvedCount} thread${unresolvedCount > 1 ? "s" : ""} first`
: "Approve"}
</Button>
<Button
className="w-full"
variant="outline"
size="sm"
onClick={onRequestChanges}
>
<X className="h-3.5 w-3.5 mr-1" />
Request Changes
</Button>
</>
)}
{status === "approved" && (
<div className="flex items-center gap-2 rounded-md bg-status-success-bg border border-status-success-border px-3 py-2 text-xs text-status-success-fg">
<Check className="h-4 w-4" />
<span className="font-medium">Approved</span>
</div>
)}
{status === "changes_requested" && (
<div className="flex items-center gap-2 rounded-md bg-status-warning-bg border border-status-warning-border px-3 py-2 text-xs text-status-warning-fg">
<X className="h-4 w-4" />
<span className="font-medium">Changes Requested</span>
</div>
)}
</div>
{/* Comment summary */} {/* Comment summary */}
<div className="space-y-2"> {comments.length > 0 && (
<h4 className="text-xs font-semibold text-muted-foreground uppercase tracking-wider"> <div className="space-y-1.5">
Discussions <h4 className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider">
</h4> Discussions
<div className="flex items-center gap-3 text-xs"> </h4>
<span className="flex items-center gap-1 text-muted-foreground"> <div className="flex items-center gap-3 text-xs">
<MessageSquare className="h-3 w-3" /> <span className="flex items-center gap-1 text-muted-foreground">
{comments.length} comment{comments.length !== 1 ? "s" : ""} <MessageSquare className="h-3 w-3" />
</span> {comments.length}
{resolvedCount > 0 && (
<span className="flex items-center gap-1 text-status-success-fg">
<CheckCircle2 className="h-3 w-3" />
{resolvedCount} resolved
</span> </span>
)} {resolvedCount > 0 && (
{unresolvedCount > 0 && ( <span className="flex items-center gap-1 text-status-success-fg">
<span className="flex items-center gap-1 text-status-warning-fg"> <CheckCircle2 className="h-3 w-3" />
<Circle className="h-3 w-3" /> {resolvedCount}
{unresolvedCount} open </span>
</span> )}
)} {unresolvedCount > 0 && (
<span className="flex items-center gap-1 text-status-warning-fg">
<Circle className="h-3 w-3" />
{unresolvedCount}
</span>
)}
</div>
</div> </div>
</div> )}
{/* Stats */}
<div className="space-y-2">
<h4 className="text-xs font-semibold text-muted-foreground uppercase tracking-wider">
Changes
</h4>
<div className="flex items-center gap-3 text-xs">
<span className="flex items-center gap-1">
<FileCode className="h-3 w-3 text-muted-foreground" />
{files.length} file{files.length !== 1 ? "s" : ""}
</span>
<span className="flex items-center gap-0.5 text-diff-add-fg">
<Plus className="h-3 w-3" />
{totalAdditions}
</span>
<span className="flex items-center gap-0.5 text-diff-remove-fg">
<Minus className="h-3 w-3" />
{totalDeletions}
</span>
</div>
</div>
{/* File list */} {/* File list */}
<div className="space-y-1"> <div className="space-y-0.5">
<h4 className="text-xs font-semibold text-muted-foreground uppercase tracking-wider"> <h4 className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider mb-1.5">
Files Files
{selectedCommit && (
<span className="font-normal ml-1 normal-case">
({activeFiles.length} in commit)
</span>
)}
</h4> </h4>
{files.map((file) => { {files.map((file) => {
const fileCommentCount = comments.filter( const fileCommentCount = comments.filter(
(c) => c.filePath === file.newPath (c) => c.filePath === file.newPath,
).length; ).length;
const isInView = activeFilePaths.has(file.newPath);
const dimmed = selectedCommit && !isInView;
return ( return (
<button <button
key={file.newPath} key={file.newPath}
className="flex w-full items-center gap-2 rounded px-2 py-1.5 text-left text-xs hover:bg-accent/50 transition-colors group" className={`
flex w-full items-center gap-1.5 rounded px-2 py-1 text-left text-[11px]
hover:bg-accent/50 transition-colors group
${dimmed ? "opacity-35" : ""}
`}
onClick={() => onFileClick(file.newPath)} onClick={() => onFileClick(file.newPath)}
> >
<FileCode className="h-3 w-3 text-muted-foreground shrink-0" /> <FileCode className="h-3 w-3 text-muted-foreground shrink-0" />
<span className="truncate flex-1 font-mono text-[11px]"> <span className="truncate flex-1 font-mono">
{file.newPath.split("/").pop()} {formatFilePath(file.newPath)}
</span> </span>
<span className="flex items-center gap-1.5 shrink-0"> <span className="flex items-center gap-1 shrink-0">
{fileCommentCount > 0 && ( {fileCommentCount > 0 && (
<span className="flex items-center gap-0.5 text-muted-foreground"> <span className="flex items-center gap-0.5 text-muted-foreground">
<MessageSquare className="h-2.5 w-2.5" /> <MessageSquare className="h-2.5 w-2.5" />
{fileCommentCount} {fileCommentCount}
</span> </span>
)} )}
<span className="text-diff-add-fg text-[10px]">+{file.additions}</span> <span className="text-diff-add-fg text-[10px]">
<span className="text-diff-remove-fg text-[10px]">-{file.deletions}</span> <Plus className="h-2.5 w-2.5 inline" />
{file.additions}
</span>
<span className="text-diff-remove-fg text-[10px]">
<Minus className="h-2.5 w-2.5 inline" />
{file.deletions}
</span>
</span> </span>
</button> </button>
); );
@@ -190,24 +113,9 @@ export function ReviewSidebar({
); );
} }
function ReviewStatusBadge({ status }: { status: ReviewStatus }) { /** Show filename with parent directory for context */
if (status === "approved") { function formatFilePath(path: string): string {
return ( const parts = path.split("/");
<Badge variant="success" size="xs"> if (parts.length <= 2) return path;
Approved return parts.slice(-2).join("/");
</Badge>
);
}
if (status === "changes_requested") {
return (
<Badge variant="warning" size="xs">
Changes Requested
</Badge>
);
}
return (
<Badge variant="secondary" size="xs">
Pending Review
</Badge>
);
} }

View File

@@ -1,10 +1,12 @@
import { useState, useCallback, useMemo, useRef } from "react"; import { useState, useCallback, useMemo, useRef } from "react";
import { toast } from "sonner"; import { toast } from "sonner";
import { Loader2 } from "lucide-react";
import { trpc } from "@/lib/trpc"; import { trpc } from "@/lib/trpc";
import { parseUnifiedDiff } from "./parse-diff"; import { parseUnifiedDiff } from "./parse-diff";
import { DiffViewer } from "./DiffViewer"; import { DiffViewer } from "./DiffViewer";
import { ReviewSidebar } from "./ReviewSidebar"; import { ReviewSidebar } from "./ReviewSidebar";
import { PreviewPanel } from "./PreviewPanel"; import { ReviewHeader } from "./ReviewHeader";
import { CommitNav } from "./CommitNav";
import type { ReviewComment, ReviewStatus, DiffLine } from "./types"; import type { ReviewComment, ReviewStatus, DiffLine } from "./types";
interface ReviewTabProps { interface ReviewTabProps {
@@ -14,6 +16,7 @@ interface ReviewTabProps {
export function ReviewTab({ initiativeId }: ReviewTabProps) { export function ReviewTab({ initiativeId }: ReviewTabProps) {
const [comments, setComments] = useState<ReviewComment[]>([]); const [comments, setComments] = useState<ReviewComment[]>([]);
const [status, setStatus] = useState<ReviewStatus>("pending"); const [status, setStatus] = useState<ReviewStatus>("pending");
const [selectedCommit, setSelectedCommit] = useState<string | null>(null);
const fileRefs = useRef<Map<string, HTMLDivElement>>(new Map()); const fileRefs = useRef<Map<string, HTMLDivElement>>(new Map());
// Fetch phases for this initiative // Fetch phases for this initiative
@@ -31,27 +34,111 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
const projectsQuery = trpc.getInitiativeProjects.useQuery({ initiativeId }); const projectsQuery = trpc.getInitiativeProjects.useQuery({ initiativeId });
const firstProjectId = projectsQuery.data?.[0]?.id ?? null; const firstProjectId = projectsQuery.data?.[0]?.id ?? null;
// Fetch diff for active phase // Fetch full branch diff for active phase
const diffQuery = trpc.getPhaseReviewDiff.useQuery( const diffQuery = trpc.getPhaseReviewDiff.useQuery(
{ phaseId: activePhaseId! }, { phaseId: activePhaseId! },
{ enabled: !!activePhaseId }, { enabled: !!activePhaseId },
); );
// Fetch commits for active phase
const commitsQuery = trpc.getPhaseReviewCommits.useQuery(
{ phaseId: activePhaseId! },
{ enabled: !!activePhaseId },
);
const commits = commitsQuery.data?.commits ?? [];
// Fetch single-commit diff when a commit is selected
const commitDiffQuery = trpc.getCommitDiff.useQuery(
{ phaseId: activePhaseId!, commitHash: selectedCommit! },
{ enabled: !!activePhaseId && !!selectedCommit },
);
// Preview state
const previewsQuery = trpc.listPreviews.useQuery(
{ initiativeId },
{ refetchInterval: 3000 },
);
const existingPreview = previewsQuery.data?.find(
(p) => p.phaseId === activePhaseId || p.initiativeId === initiativeId,
);
const [activePreviewId, setActivePreviewId] = useState<string | null>(null);
const previewStatusQuery = trpc.getPreviewStatus.useQuery(
{ previewId: activePreviewId ?? existingPreview?.id ?? "" },
{
enabled: !!(activePreviewId ?? existingPreview?.id),
refetchInterval: 3000,
},
);
const preview = previewStatusQuery.data ?? existingPreview;
const sourceBranch = diffQuery.data?.sourceBranch ?? commitsQuery.data?.sourceBranch ?? "";
const startPreview = trpc.startPreview.useMutation({
onSuccess: (data) => {
setActivePreviewId(data.id);
toast.success(`Preview running at http://localhost:${data.port}`);
},
onError: (err) => toast.error(`Preview failed: ${err.message}`),
});
const stopPreview = trpc.stopPreview.useMutation({
onSuccess: () => {
setActivePreviewId(null);
toast.success("Preview stopped");
previewsQuery.refetch();
},
onError: (err) => toast.error(`Failed to stop: ${err.message}`),
});
const previewState = firstProjectId && sourceBranch
? {
status: startPreview.isPending
? ("building" as const)
: preview?.status === "running"
? ("running" as const)
: preview?.status === "building"
? ("building" as const)
: preview?.status === "failed"
? ("failed" as const)
: ("idle" as const),
url: preview?.port ? `http://localhost:${preview.port}` : undefined,
onStart: () =>
startPreview.mutate({
initiativeId,
phaseId: activePhaseId ?? undefined,
projectId: firstProjectId,
branch: sourceBranch,
}),
onStop: () => {
const id = activePreviewId ?? existingPreview?.id;
if (id) stopPreview.mutate({ previewId: id });
},
isStarting: startPreview.isPending,
isStopping: stopPreview.isPending,
}
: null;
const approveMutation = trpc.approvePhaseReview.useMutation({ const approveMutation = trpc.approvePhaseReview.useMutation({
onSuccess: () => { onSuccess: () => {
setStatus("approved"); setStatus("approved");
toast.success("Phase approved and merged"); toast.success("Phase approved and merged");
phasesQuery.refetch(); phasesQuery.refetch();
}, },
onError: (err) => { onError: (err) => toast.error(err.message),
toast.error(err.message);
},
}); });
// Determine which diff to display
const activeDiffRaw = selectedCommit
? commitDiffQuery.data?.rawDiff
: diffQuery.data?.rawDiff;
const files = useMemo(() => { const files = useMemo(() => {
if (!diffQuery.data?.rawDiff) return []; if (!activeDiffRaw) return [];
return parseUnifiedDiff(diffQuery.data.rawDiff); return parseUnifiedDiff(activeDiffRaw);
}, [diffQuery.data?.rawDiff]); }, [activeDiffRaw]);
const isDiffLoading = selectedCommit
? commitDiffQuery.isLoading
: diffQuery.isLoading;
const handleAddComment = useCallback( const handleAddComment = useCallback(
(filePath: string, lineNumber: number, lineType: DiffLine["type"], body: string) => { (filePath: string, lineNumber: number, lineType: DiffLine["type"], body: string) => {
@@ -68,18 +155,18 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
setComments((prev) => [...prev, newComment]); setComments((prev) => [...prev, newComment]);
toast.success("Comment added"); toast.success("Comment added");
}, },
[] [],
); );
const handleResolveComment = useCallback((commentId: string) => { const handleResolveComment = useCallback((commentId: string) => {
setComments((prev) => setComments((prev) =>
prev.map((c) => (c.id === commentId ? { ...c, resolved: true } : c)) prev.map((c) => (c.id === commentId ? { ...c, resolved: true } : c)),
); );
}, []); }, []);
const handleUnresolveComment = useCallback((commentId: string) => { const handleUnresolveComment = useCallback((commentId: string) => {
setComments((prev) => setComments((prev) =>
prev.map((c) => (c.id === commentId ? { ...c, resolved: false } : c)) prev.map((c) => (c.id === commentId ? { ...c, resolved: false } : c)),
); );
}, []); }, []);
@@ -102,6 +189,15 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
} }
}, []); }, []);
const handlePhaseSelect = useCallback((id: string) => {
setSelectedPhaseId(id);
setSelectedCommit(null);
setStatus("pending");
setComments([]);
}, []);
const unresolvedCount = comments.filter((c) => !c.resolved).length;
if (pendingReviewPhases.length === 0) { if (pendingReviewPhases.length === 0) {
return ( return (
<div className="flex h-64 items-center justify-center text-muted-foreground"> <div className="flex h-64 items-center justify-center text-muted-foreground">
@@ -110,58 +206,58 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
); );
} }
const activePhaseName = diffQuery.data?.phaseName ?? pendingReviewPhases.find(p => p.id === activePhaseId)?.name ?? "Phase"; const activePhaseName =
const sourceBranch = diffQuery.data?.sourceBranch ?? ""; diffQuery.data?.phaseName ??
pendingReviewPhases.find((p) => p.id === activePhaseId)?.name ??
"Phase";
// All files from the full branch diff (for sidebar file list)
const allFiles = useMemo(() => {
if (!diffQuery.data?.rawDiff) return [];
return parseUnifiedDiff(diffQuery.data.rawDiff);
}, [diffQuery.data?.rawDiff]);
return ( return (
<div className="space-y-4"> <div className="rounded-lg border border-border overflow-hidden bg-card">
{/* Phase selector if multiple pending */} {/* Header: phase selector + toolbar */}
{pendingReviewPhases.length > 1 && ( <ReviewHeader
<div className="flex gap-2"> phases={pendingReviewPhases.map((p) => ({ id: p.id, name: p.name }))}
{pendingReviewPhases.map((phase) => ( activePhaseId={activePhaseId}
<button onPhaseSelect={handlePhaseSelect}
key={phase.id} phaseName={activePhaseName}
onClick={() => setSelectedPhaseId(phase.id)} sourceBranch={sourceBranch}
className={`rounded-md border px-3 py-1.5 text-sm transition-colors ${ targetBranch={diffQuery.data?.targetBranch ?? commitsQuery.data?.targetBranch ?? ""}
phase.id === activePhaseId files={allFiles}
? "border-primary bg-primary/10 font-medium" status={status}
: "border-border hover:bg-muted" unresolvedCount={unresolvedCount}
}`} onApprove={handleApprove}
> onRequestChanges={handleRequestChanges}
{phase.name} preview={previewState}
</button> />
))}
</div>
)}
{/* Preview panel */} {/* Commit navigation strip */}
{firstProjectId && sourceBranch && ( <CommitNav
<PreviewPanel commits={commits}
initiativeId={initiativeId} selectedCommit={selectedCommit}
phaseId={activePhaseId ?? undefined} onSelectCommit={setSelectedCommit}
projectId={firstProjectId} isLoading={commitsQuery.isLoading}
branch={sourceBranch} />
/>
)}
{diffQuery.isLoading ? ( {/* Main content area */}
<div className="flex h-64 items-center justify-center text-muted-foreground"> {isDiffLoading ? (
<div className="flex h-64 items-center justify-center text-muted-foreground gap-2">
<Loader2 className="h-4 w-4 animate-spin" />
Loading diff... Loading diff...
</div> </div>
) : ( ) : (
<div className="grid grid-cols-1 gap-6 lg:grid-cols-[1fr_300px]"> <div className="grid grid-cols-1 lg:grid-cols-[1fr_260px]">
{/* Left: Diff */} {/* Left: Diff */}
<div className="min-w-0"> <div className="min-w-0 p-4">
<div className="flex items-center justify-between border-b border-border pb-3 mb-4">
<h2 className="text-lg font-semibold">Review: {activePhaseName}</h2>
<span className="text-xs text-muted-foreground">
{comments.filter((c) => !c.resolved).length} unresolved thread
{comments.filter((c) => !c.resolved).length !== 1 ? "s" : ""}
</span>
</div>
{files.length === 0 ? ( {files.length === 0 ? (
<div className="flex h-32 items-center justify-center text-muted-foreground"> <div className="flex h-32 items-center justify-center text-muted-foreground text-sm">
No changes in this phase {selectedCommit
? "No changes in this commit"
: "No changes in this phase"}
</div> </div>
) : ( ) : (
<DiffViewer <DiffViewer
@@ -175,19 +271,13 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
</div> </div>
{/* Right: Sidebar */} {/* Right: Sidebar */}
<div className="w-full lg:w-[300px]"> <div className="border-l border-border p-4">
<ReviewSidebar <ReviewSidebar
title={`Phase: ${activePhaseName}`} files={allFiles}
description={`Review changes from phase "${activePhaseName}" before merging into the initiative branch.`}
author="system"
status={status}
sourceBranch={sourceBranch}
targetBranch={diffQuery.data?.targetBranch ?? ""}
files={files}
comments={comments} comments={comments}
onApprove={handleApprove}
onRequestChanges={handleRequestChanges}
onFileClick={handleFileClick} onFileClick={handleFileClick}
selectedCommit={selectedCommit}
activeFiles={files}
/> />
</div> </div>
</div> </div>

View File

@@ -35,6 +35,17 @@ export interface ReviewComment {
export type ReviewStatus = "pending" | "approved" | "changes_requested"; export type ReviewStatus = "pending" | "approved" | "changes_requested";
export interface CommitInfo {
hash: string;
shortHash: string;
message: string;
author: string;
date: string;
filesChanged: number;
insertions: number;
deletions: number;
}
export interface ReviewSummary { export interface ReviewSummary {
id: string; id: string;
title: string; title: string;

View File

@@ -111,10 +111,12 @@ The initiative detail page has three tabs managed via local state (not URL param
### Review Components (`src/components/review/`) ### Review Components (`src/components/review/`)
| Component | Purpose | | Component | Purpose |
|-----------|---------| |-----------|---------|
| `ReviewTab` | Review tab container with diff viewer and preview integration | | `ReviewTab` | Review tab container — orchestrates header, commit nav, diff, sidebar, and preview |
| `ReviewSidebar` | Review info, actions, file list, comment summary | | `ReviewHeader` | Consolidated toolbar: phase selector pills, branch info, stats, preview controls, approve/reject actions |
| `CommitNav` | Horizontal commit navigation strip — "All changes" + individual commit pills with stats |
| `ReviewSidebar` | File list with comment counts, dimmed files when viewing single commit |
| `DiffViewer` | Unified diff renderer with inline comments | | `DiffViewer` | Unified diff renderer with inline comments |
| `PreviewPanel` | Docker preview status: building/running/failed with start/stop | | `PreviewPanel` | Docker preview status: building/running/failed with start/stop (legacy, now integrated into ReviewHeader) |
| `ProposalCard` | Individual proposal display | | `ProposalCard` | Individual proposal display |
### UI Primitives (`src/components/ui/`) ### UI Primitives (`src/components/ui/`)

View File

@@ -43,6 +43,8 @@ Worktrees stored in `.cw-worktrees/` subdirectory of the repo. Each agent gets a
| `deleteBranch(repoPath, branch)` | Delete local branch (no-op if missing) | | `deleteBranch(repoPath, branch)` | Delete local branch (no-op if missing) |
| `branchExists(repoPath, branch)` | Check local branches | | `branchExists(repoPath, branch)` | Check local branches |
| `remoteBranchExists(repoPath, branch)` | Check remote tracking branches (`origin/<branch>`) | | `remoteBranchExists(repoPath, branch)` | Check remote tracking branches (`origin/<branch>`) |
| `listCommits(repoPath, base, head)` | List commits head has that base doesn't (with stats) |
| `diffCommit(repoPath, commitHash)` | Get unified diff for a single commit |
`remoteBranchExists` is used by `registerProject` and `updateProject` to validate that a project's default branch actually exists in the cloned repository before saving. `remoteBranchExists` is used by `registerProject` and `updateProject` to validate that a project's default branch actually exists in the cloned repository before saving.

View File

@@ -109,6 +109,10 @@ Each procedure uses `require*Repository(ctx)` helpers that throw `TRPCError(INTE
| listInitiativePhaseDependencies | query | All dependency edges | | listInitiativePhaseDependencies | query | All dependency edges |
| getPhaseDependencies | query | What this phase depends on | | getPhaseDependencies | query | What this phase depends on |
| getPhaseDependents | query | What depends on this phase | | getPhaseDependents | query | What depends on this phase |
| getPhaseReviewDiff | query | Full branch diff for pending_review phase |
| getPhaseReviewCommits | query | List commits between initiative and phase branch |
| getCommitDiff | query | Diff for a single commit (by hash) in a phase |
| approvePhaseReview | mutation | Approve and merge phase branch |
### Phase Dispatch ### Phase Dispatch
| Procedure | Type | Description | | Procedure | Type | Description |