From bc3ea5cd3b2dbaba35a059a5550b437f4a123206 Mon Sep 17 00:00:00 2001 From: Nick Raziborsky Date: Mon, 13 Apr 2026 15:31:49 -0700 Subject: [PATCH] feat: add configurable base-branch review mode Extend the native diff review window with a branch-aware review scope that compares merge-base(baseRef, HEAD) to HEAD. This adds a base-branch scope, persisted base-ref config, one-off base-ref overrides, and UI updates that surface the active base ref in the review window. Users can now set an explicit base ref or use the special 'default' setting, which resolves to the repository's default remote branch. The implementation preserves the existing Glimpse/Monaco review workflow while making branch review the primary path, defaulting the UI to changed regions only with wrap disabled for a cleaner review surface. --- README.md | 90 +++++++++++++++++++++++++++++--- src/config.ts | 96 ++++++++++++++++++++++++++++++++++ src/git.ts | 139 +++++++++++++++++++++++++++++++++++++++++++++++-- src/index.ts | 118 +++++++++++++++++++++++++++++++++++++---- src/prompt.ts | 21 +++++--- src/types.ts | 17 +++++- web/app.js | 45 +++++++++++++--- web/index.html | 6 ++- 8 files changed, 493 insertions(+), 39 deletions(-) create mode 100644 src/config.ts diff --git a/README.md b/README.md index 7bd7643..9e791a0 100644 --- a/README.md +++ b/README.md @@ -1,14 +1,8 @@ # pi-diff-review -This is pure slop, see: https://pi.dev/session/#d4ce533cedbd60040f2622dc3db950e2 - -It is my hope, that someone takes this idea and makes it gud. - Native diff review window for pi, powered by [Glimpse](https://github.com/hazat/glimpse) and Monaco. -``` -pi install git:https://github.com/badlogic/pi-diff-review -``` +This fork extends the original package with a branch-aware review mode built around a configurable base ref. ## What it does @@ -17,13 +11,93 @@ Adds a `/diff-review` command to pi. The command: 1. opens a native review window -2. lets you switch between `git diff`, `last commit`, and `all files` scopes +2. lets you switch between `base branch`, `git diff`, `last commit`, and `all files` scopes 3. shows a collapsible sidebar with fuzzy file search 4. shows git status markers in the sidebar for changed files and untracked files 5. lazy-loads file contents on demand as you switch files and scopes 6. lets you draft comments on the original side, modified side, or whole file 7. inserts the resulting feedback prompt into the pi editor when you submit +## New branch-aware behavior + +- adds a **base branch** scope that compares `merge-base(, HEAD)` to `HEAD` +- supports a configurable base ref via `/diff-review-set-base` +- supports a special `default` base ref setting that resolves to the repo's default remote branch +- defaults the review window to: + - initial scope: `base branch` + - hide unchanged regions: `on` + - wrap lines: `off` +- supports a one-off base ref override with `/diff-review ` +- shows the active resolved base ref in the window chrome +- shows effective config with `/diff-review-config` + +## Commands + +### `/diff-review` + +Open the native review window using the saved config. + +```bash +/diff-review +``` + +Override the base ref for one review session: + +```bash +/diff-review origin/main +``` + +### `/diff-review-set-base` + +Persist the default base ref for the current project: + +```bash +/diff-review-set-base origin/main +``` + +Use the repository default remote branch: + +```bash +/diff-review-set-base default +``` + +Persist the setting globally: + +```bash +/diff-review-set-base --global default +``` + +### `/diff-review-config` + +Show the configured base ref, the effective resolved base ref, and the config file paths. + +## Config files + +Project-local override: + +```text +/.pi/diff-review.json +``` + +Global fallback: + +```text +~/.pi/agent/pi-diff-review.json +``` + +Example: + +```json +{ + "defaultBaseRef": "default", + "preferredInitialScope": "base-branch", + "preferredHideUnchanged": true, + "preferredWrapLines": false, + "preferredSidebarCollapsed": false, + "autoFetchBaseRef": true +} +``` + ## Requirements - macOS, Linux, or Windows diff --git a/src/config.ts b/src/config.ts new file mode 100644 index 0000000..4571c1b --- /dev/null +++ b/src/config.ts @@ -0,0 +1,96 @@ +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { homedir } from "node:os"; +import { dirname, join } from "node:path"; +import type { ReviewScope, ReviewWindowPreferences } from "./types.js"; + +export interface ReviewConfig extends ReviewWindowPreferences { + autoFetchBaseRef: boolean; +} + +export interface ReviewConfigLocations { + globalConfigPath: string; + projectConfigPath: string; +} + +export const DEFAULT_REVIEW_CONFIG: ReviewConfig = { + defaultBaseRef: "default", + preferredInitialScope: "base-branch", + preferredHideUnchanged: true, + preferredWrapLines: false, + preferredSidebarCollapsed: false, + autoFetchBaseRef: true, +}; + +function isReviewScope(value: unknown): value is ReviewScope { + return value === "base-branch" || value === "git-diff" || value === "last-commit" || value === "all-files"; +} + +function sanitizeConfig(value: unknown): Partial { + if (value == null || typeof value !== "object") return {}; + + const input = value as Record; + const result: Partial = {}; + + if (typeof input.defaultBaseRef === "string" && input.defaultBaseRef.trim().length > 0) { + result.defaultBaseRef = input.defaultBaseRef.trim(); + } + if (isReviewScope(input.preferredInitialScope)) { + result.preferredInitialScope = input.preferredInitialScope; + } + if (typeof input.preferredHideUnchanged === "boolean") { + result.preferredHideUnchanged = input.preferredHideUnchanged; + } + if (typeof input.preferredWrapLines === "boolean") { + result.preferredWrapLines = input.preferredWrapLines; + } + if (typeof input.preferredSidebarCollapsed === "boolean") { + result.preferredSidebarCollapsed = input.preferredSidebarCollapsed; + } + if (typeof input.autoFetchBaseRef === "boolean") { + result.autoFetchBaseRef = input.autoFetchBaseRef; + } + + return result; +} + +async function readJsonFile(path: string): Promise> { + try { + const contents = await readFile(path, "utf8"); + return sanitizeConfig(JSON.parse(contents)); + } catch { + return {}; + } +} + +export function getReviewConfigLocations(repoRoot: string): ReviewConfigLocations { + return { + globalConfigPath: join(homedir(), ".pi", "agent", "pi-diff-review.json"), + projectConfigPath: join(repoRoot, ".pi", "diff-review.json"), + }; +} + +export async function loadReviewConfig(repoRoot: string): Promise { + const { globalConfigPath, projectConfigPath } = getReviewConfigLocations(repoRoot); + const globalConfig = await readJsonFile(globalConfigPath); + const projectConfig = await readJsonFile(projectConfigPath); + + return { + ...DEFAULT_REVIEW_CONFIG, + ...globalConfig, + ...projectConfig, + }; +} + +export async function saveReviewConfig(repoRoot: string, patch: Partial, scope: "global" | "project"): Promise { + const { globalConfigPath, projectConfigPath } = getReviewConfigLocations(repoRoot); + const path = scope === "global" ? globalConfigPath : projectConfigPath; + const current = await readJsonFile(path); + const next = { + ...current, + ...sanitizeConfig(patch), + } satisfies Partial; + + await mkdir(dirname(path), { recursive: true }); + await writeFile(path, `${JSON.stringify(next, null, 2)}\n`, "utf8"); + return path; +} diff --git a/src/git.ts b/src/git.ts index aa7123c..d390c69 100644 --- a/src/git.ts +++ b/src/git.ts @@ -1,6 +1,7 @@ import { readFile } from "node:fs/promises"; import { extname, join } from "node:path"; import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; +import type { ReviewConfig } from "./config.js"; import type { ChangeStatus, ReviewFile, ReviewFileComparison, ReviewFileContents, ReviewScope } from "./types.js"; interface ChangedPath { @@ -13,8 +14,10 @@ interface ReviewFileSeed { path: string; worktreeStatus: ChangeStatus | null; hasWorkingTreeFile: boolean; + inBaseBranch: boolean; inGitDiff: boolean; inLastCommit: boolean; + baseBranch: ReviewFileComparison | null; gitDiff: ReviewFileComparison | null; lastCommit: ReviewFileComparison | null; } @@ -153,10 +156,17 @@ function toComparison(change: ChangedPath): ReviewFileComparison { }; } -function buildReviewFileId(path: string, hasWorkingTreeFile: boolean, gitDiff: ReviewFileComparison | null, lastCommit: ReviewFileComparison | null): string { +function buildReviewFileId( + path: string, + hasWorkingTreeFile: boolean, + baseBranch: ReviewFileComparison | null, + gitDiff: ReviewFileComparison | null, + lastCommit: ReviewFileComparison | null, +): string { return [ path, hasWorkingTreeFile ? "working" : "gone", + baseBranch?.displayPath ?? "", gitDiff?.displayPath ?? "", lastCommit?.displayPath ?? "", ].join("::"); @@ -164,12 +174,14 @@ function buildReviewFileId(path: string, hasWorkingTreeFile: boolean, gitDiff: R function createReviewFile(seed: ReviewFileSeed): ReviewFile { return { - id: buildReviewFileId(seed.path, seed.hasWorkingTreeFile, seed.gitDiff, seed.lastCommit), + id: buildReviewFileId(seed.path, seed.hasWorkingTreeFile, seed.baseBranch, seed.gitDiff, seed.lastCommit), path: seed.path, worktreeStatus: seed.worktreeStatus, hasWorkingTreeFile: seed.hasWorkingTreeFile, + inBaseBranch: seed.inBaseBranch, inGitDiff: seed.inGitDiff, inLastCommit: seed.inLastCommit, + baseBranch: seed.baseBranch, gitDiff: seed.gitDiff, lastCommit: seed.lastCommit, }; @@ -256,10 +268,79 @@ function upsertSeed(seeds: Map, key: string, create: () return seed; } -export async function getReviewWindowData(pi: ExtensionAPI, cwd: string): Promise<{ repoRoot: string; files: ReviewFile[] }> { +async function getRemoteNames(pi: ExtensionAPI, repoRoot: string): Promise { + const output = await runGitAllowFailure(pi, repoRoot, ["remote"]); + return parseTrackedPaths(output); +} + +async function maybeFetchRemote(pi: ExtensionAPI, repoRoot: string, remote: string, autoFetch: boolean): Promise { + if (!autoFetch || remote.length === 0) return; + await runGitAllowFailure(pi, repoRoot, ["fetch", remote, "--prune"]); +} + +async function getUpstreamRemoteName(pi: ExtensionAPI, repoRoot: string): Promise { + const output = await runGitAllowFailure(pi, repoRoot, ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"]); + const upstream = output.trim(); + if (upstream.length === 0 || !upstream.includes("/")) return null; + return upstream.split("/")[0] ?? null; +} + +async function resolveDefaultBaseRef(pi: ExtensionAPI, repoRoot: string, autoFetch: boolean): Promise { + const remotes = await getRemoteNames(pi, repoRoot); + const candidateOrder = [await getUpstreamRemoteName(pi, repoRoot), "origin", ...remotes] + .filter((remote, index, all): remote is string => remote != null && remote.length > 0 && all.indexOf(remote) === index); + + for (const remote of candidateOrder) { + await maybeFetchRemote(pi, repoRoot, remote, autoFetch); + const symbolicRef = (await runGitAllowFailure(pi, repoRoot, ["symbolic-ref", "--quiet", "--short", `refs/remotes/${remote}/HEAD`])).trim(); + if (symbolicRef.length > 0) { + return symbolicRef; + } + + const revParseRef = (await runGitAllowFailure(pi, repoRoot, ["rev-parse", "--abbrev-ref", `${remote}/HEAD`])).trim(); + if (revParseRef.length > 0 && revParseRef !== `${remote}/HEAD`) { + return revParseRef; + } + } + + throw new Error("Could not resolve the repository default remote branch. Set one explicitly with /diff-review-set-base ."); +} + +export async function resolveBaseRef(pi: ExtensionAPI, repoRoot: string, configuredBaseRef: string, autoFetch: boolean): Promise { + if (configuredBaseRef === "default") { + return resolveDefaultBaseRef(pi, repoRoot, autoFetch); + } + + const [remoteCandidate] = configuredBaseRef.split("/"); + const remotes = await getRemoteNames(pi, repoRoot); + if (remoteCandidate != null && remotes.includes(remoteCandidate)) { + await maybeFetchRemote(pi, repoRoot, remoteCandidate, autoFetch); + } + + return configuredBaseRef; +} + +async function getMergeBase(pi: ExtensionAPI, repoRoot: string, baseRef: string): Promise { + const output = await runGit(pi, repoRoot, ["merge-base", baseRef, "HEAD"]); + return output.trim(); +} + +async function getBaseBranchNameStatus(pi: ExtensionAPI, repoRoot: string, config: ReviewConfig): Promise { + const mergeBase = await getMergeBase(pi, repoRoot, config.defaultBaseRef); + return runGit(pi, repoRoot, ["diff", "--find-renames", "-M", "--name-status", mergeBase, "HEAD", "--"]); +} + +export async function getReviewWindowData( + pi: ExtensionAPI, + cwd: string, + config: ReviewConfig, +): Promise<{ repoRoot: string; files: ReviewFile[] }> { const repoRoot = await getRepoRoot(pi, cwd); const repositoryHasHead = await hasHead(pi, repoRoot); + const baseBranchOutput = repositoryHasHead + ? await getBaseBranchNameStatus(pi, repoRoot, config) + : ""; const trackedDiffOutput = repositoryHasHead ? await runGit(pi, repoRoot, ["diff", "--find-renames", "-M", "--name-status", "HEAD", "--"]) : ""; @@ -270,6 +351,8 @@ export async function getReviewWindowData(pi: ExtensionAPI, cwd: string): Promis ? await runGitAllowFailure(pi, repoRoot, ["diff-tree", "--root", "--find-renames", "-M", "--name-status", "--no-commit-id", "-r", "HEAD"]) : ""; + const baseBranchChanges = parseNameStatus(baseBranchOutput) + .filter((change) => isReviewableFilePath(change.newPath ?? change.oldPath ?? "")); const worktreeChanges = mergeChangedPaths(parseNameStatus(trackedDiffOutput), parseUntrackedPaths(untrackedOutput)) .filter((change) => isReviewableFilePath(change.newPath ?? change.oldPath ?? "")); const deletedPaths = new Set(parseTrackedPaths(deletedFilesOutput)); @@ -286,21 +369,42 @@ export async function getReviewWindowData(pi: ExtensionAPI, cwd: string): Promis path, worktreeStatus: null, hasWorkingTreeFile: true, + inBaseBranch: false, inGitDiff: false, inLastCommit: false, + baseBranch: null, gitDiff: null, lastCommit: null, }); } + for (const change of baseBranchChanges) { + const key = change.newPath ?? change.oldPath ?? toDisplayPath(change); + const seed = upsertSeed(seeds, key, () => ({ + path: key, + worktreeStatus: null, + hasWorkingTreeFile: change.newPath != null && currentPaths.includes(change.newPath), + inBaseBranch: false, + inGitDiff: false, + inLastCommit: false, + baseBranch: null, + gitDiff: null, + lastCommit: null, + })); + seed.inBaseBranch = true; + seed.baseBranch = toComparison(change); + } + for (const change of worktreeChanges) { const key = change.newPath ?? change.oldPath ?? toDisplayPath(change); const seed = upsertSeed(seeds, key, () => ({ path: key, worktreeStatus: null, hasWorkingTreeFile: change.newPath != null, + inBaseBranch: false, inGitDiff: false, inLastCommit: false, + baseBranch: null, gitDiff: null, lastCommit: null, })); @@ -316,8 +420,10 @@ export async function getReviewWindowData(pi: ExtensionAPI, cwd: string): Promis path: key, worktreeStatus: null, hasWorkingTreeFile: change.newPath != null && currentPaths.includes(change.newPath), + inBaseBranch: false, inGitDiff: false, inLastCommit: false, + baseBranch: null, gitDiff: null, lastCommit: null, })); @@ -332,7 +438,13 @@ export async function getReviewWindowData(pi: ExtensionAPI, cwd: string): Promis return { repoRoot, files }; } -export async function loadReviewFileContents(pi: ExtensionAPI, repoRoot: string, file: ReviewFile, scope: ReviewScope): Promise { +export async function loadReviewFileContents( + pi: ExtensionAPI, + repoRoot: string, + file: ReviewFile, + scope: ReviewScope, + config: ReviewConfig, +): Promise { if (scope === "all-files") { const content = file.hasWorkingTreeFile ? await getWorkingTreeContent(repoRoot, file.path) : ""; return { @@ -341,6 +453,25 @@ export async function loadReviewFileContents(pi: ExtensionAPI, repoRoot: string, }; } + if (scope === "base-branch") { + const comparison = file.baseBranch; + if (comparison == null) { + return { + originalContent: "", + modifiedContent: "", + }; + } + + const mergeBase = await getMergeBase(pi, repoRoot, config.defaultBaseRef); + const originalContent = comparison.oldPath == null ? "" : await getRevisionContent(pi, repoRoot, mergeBase, comparison.oldPath); + const modifiedContent = comparison.newPath == null ? "" : await getRevisionContent(pi, repoRoot, "HEAD", comparison.newPath); + + return { + originalContent, + modifiedContent, + }; + } + const comparison = scope === "git-diff" ? file.gitDiff : file.lastCommit; if (comparison == null) { return { diff --git a/src/index.ts b/src/index.ts index 824dc52..68b4feb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,8 @@ import type { ExtensionAPI, ExtensionCommandContext } from "@mariozechner/pi-coding-agent"; -import { Key, matchesKey, truncateToWidth } from "@mariozechner/pi-tui"; +import { Key, matchesKey, truncateToWidth, Text } from "@mariozechner/pi-tui"; import { open, type GlimpseWindow } from "glimpseui"; -import { getReviewWindowData, loadReviewFileContents } from "./git.js"; +import { getReviewConfigLocations, loadReviewConfig, saveReviewConfig, type ReviewConfig } from "./config.js"; +import { getRepoRoot, getReviewWindowData, loadReviewFileContents, resolveBaseRef } from "./git.js"; import { composeReviewPrompt } from "./prompt.js"; import type { ReviewCancelPayload, @@ -10,6 +11,7 @@ import type { ReviewHostMessage, ReviewRequestFilePayload, ReviewSubmitPayload, + ReviewWindowData, ReviewWindowMessage, } from "./types.js"; import { buildReviewHtml } from "./ui.js"; @@ -32,6 +34,32 @@ function escapeForInlineScript(value: string): string { return value.replace(//g, "\\u003e").replace(/&/g, "\\u0026"); } +function parseBaseOverride(args: string): string | undefined { + const trimmed = args.trim(); + return trimmed.length > 0 ? trimmed : undefined; +} + +function formatConfigSummary( + config: ReviewConfig, + effectiveBaseRef: string, + locations: { globalConfigPath: string; projectConfigPath: string }, +): string { + return [ + "diff-review config", + "", + `configured defaultBaseRef: ${config.defaultBaseRef}`, + `effective base ref: ${effectiveBaseRef}`, + `preferredInitialScope: ${config.preferredInitialScope}`, + `preferredHideUnchanged: ${String(config.preferredHideUnchanged)}`, + `preferredWrapLines: ${String(config.preferredWrapLines)}`, + `preferredSidebarCollapsed: ${String(config.preferredSidebarCollapsed)}`, + `autoFetchBaseRef: ${String(config.autoFetchBaseRef)}`, + "", + `project config: ${locations.projectConfigPath}`, + `global config: ${locations.globalConfigPath}`, + ].join("\n"); +} + export default function (pi: ExtensionAPI) { let activeWindow: GlimpseWindow | null = null; let activeWaitingUIDismiss: (() => void) | null = null; @@ -111,23 +139,49 @@ export default function (pi: ExtensionAPI) { }; } - async function reviewRepository(ctx: ExtensionCommandContext): Promise { + async function buildWindowData(ctx: ExtensionCommandContext, baseOverride?: string): Promise<{ config: ReviewConfig; data: ReviewWindowData }> { + const repoRoot = await getRepoRoot(pi, ctx.cwd); + const storedConfig = await loadReviewConfig(repoRoot); + const configuredBaseRef = baseOverride ?? storedConfig.defaultBaseRef; + const effectiveBaseRef = await resolveBaseRef(pi, repoRoot, configuredBaseRef, storedConfig.autoFetchBaseRef); + const config: ReviewConfig = { + ...storedConfig, + defaultBaseRef: effectiveBaseRef, + }; + const { files } = await getReviewWindowData(pi, ctx.cwd, config); + + return { + config, + data: { + repoRoot, + files, + defaultBaseRef: effectiveBaseRef, + preferredInitialScope: config.preferredInitialScope, + preferredHideUnchanged: config.preferredHideUnchanged, + preferredWrapLines: config.preferredWrapLines, + preferredSidebarCollapsed: config.preferredSidebarCollapsed, + }, + }; + } + + async function reviewRepository(ctx: ExtensionCommandContext, baseOverride?: string): Promise { if (activeWindow != null) { ctx.ui.notify("A review window is already open.", "warning"); return; } - const { repoRoot, files } = await getReviewWindowData(pi, ctx.cwd); + const { config, data } = await buildWindowData(ctx, baseOverride); + const { repoRoot, files } = data; if (files.length === 0) { ctx.ui.notify("No reviewable files found.", "info"); return; } - const html = buildReviewHtml({ repoRoot, files }); + const html = buildReviewHtml(data); const window = open(html, { width: 1680, height: 1020, - title: "pi review", + title: `pi review • ${config.defaultBaseRef}`, }); activeWindow = window; @@ -146,12 +200,12 @@ export default function (pi: ExtensionAPI) { const cached = contentCache.get(cacheKey); if (cached != null) return cached; - const pending = loadReviewFileContents(pi, repoRoot, file, scope); + const pending = loadReviewFileContents(pi, repoRoot, file, scope, config); contentCache.set(cacheKey, pending); return pending; }; - ctx.ui.notify("Opened native review window.", "info"); + ctx.ui.notify(`Opened native review window (${config.defaultBaseRef}).`, "info"); try { const terminalMessagePromise = new Promise((resolve, reject) => { @@ -258,7 +312,7 @@ export default function (pi: ExtensionAPI) { return; } - const prompt = composeReviewPrompt(files, message); + const prompt = composeReviewPrompt(files, message, config.defaultBaseRef); ctx.ui.setEditorText(prompt); ctx.ui.notify("Inserted review feedback into the editor.", "info"); } catch (error) { @@ -270,9 +324,51 @@ export default function (pi: ExtensionAPI) { } pi.registerCommand("diff-review", { - description: "Open a native review window with git diff, last commit, and all files scopes", + description: "Open a native review window. Optional args: pass a base ref for one-off branch comparison.", + handler: async (args, ctx) => { + await reviewRepository(ctx, parseBaseOverride(args)); + }, + }); + + pi.registerMessageRenderer("diff-review-config", (message) => new Text(String(message.content), 0, 0)); + + pi.registerCommand("diff-review-set-base", { + description: "Persist the default base ref for diff review. Usage: /diff-review-set-base [--global] ", + handler: async (args, ctx) => { + const trimmed = args.trim(); + if (trimmed.length === 0) { + ctx.ui.notify("Usage: /diff-review-set-base [--global] ", "warning"); + return; + } + + const globalPrefix = "--global "; + const scope = trimmed.startsWith(globalPrefix) ? "global" : "project"; + const baseRef = (scope === "global" ? trimmed.slice(globalPrefix.length) : trimmed).trim(); + if (baseRef.length === 0) { + ctx.ui.notify("Usage: /diff-review-set-base [--global] ", "warning"); + return; + } + + const repoRoot = await getRepoRoot(pi, ctx.cwd); + const currentConfig = await loadReviewConfig(repoRoot); + const configPath = await saveReviewConfig(repoRoot, { defaultBaseRef: baseRef }, scope); + const effectiveBaseRef = await resolveBaseRef(pi, repoRoot, baseRef, currentConfig.autoFetchBaseRef); + ctx.ui.notify(`Saved default base ref ${baseRef} (${effectiveBaseRef}) to ${configPath}`, "info"); + }, + }); + + pi.registerCommand("diff-review-config", { + description: "Show the effective diff-review config and config file paths", handler: async (_args, ctx) => { - await reviewRepository(ctx); + const repoRoot = await getRepoRoot(pi, ctx.cwd); + const config = await loadReviewConfig(repoRoot); + const effectiveBaseRef = await resolveBaseRef(pi, repoRoot, config.defaultBaseRef, config.autoFetchBaseRef); + const locations = getReviewConfigLocations(repoRoot); + pi.sendMessage({ + customType: "diff-review-config", + content: formatConfigSummary(config, effectiveBaseRef, locations), + display: true, + }); }, }); diff --git a/src/prompt.ts b/src/prompt.ts index 352218d..e5a011e 100644 --- a/src/prompt.ts +++ b/src/prompt.ts @@ -1,7 +1,8 @@ import type { DiffReviewComment, ReviewFile, ReviewScope, ReviewSubmitPayload } from "./types.js"; -function formatScopeLabel(scope: ReviewScope): string { +function formatScopeLabel(scope: ReviewScope, baseRef: string): string { switch (scope) { + case "base-branch": return `base branch (${baseRef})`; case "git-diff": return "git diff"; case "last-commit": return "last commit"; default: return "all files"; @@ -10,13 +11,19 @@ function formatScopeLabel(scope: ReviewScope): string { function getCommentFilePath(file: ReviewFile | undefined, scope: ReviewScope): string { if (file == null) return "(unknown file)"; - const comparison = scope === "git-diff" ? file.gitDiff : scope === "last-commit" ? file.lastCommit : null; + const comparison = scope === "base-branch" + ? file.baseBranch + : scope === "git-diff" + ? file.gitDiff + : scope === "last-commit" + ? file.lastCommit + : null; return comparison?.displayPath ?? file.path; } -function formatLocation(comment: DiffReviewComment, file: ReviewFile | undefined): string { +function formatLocation(comment: DiffReviewComment, file: ReviewFile | undefined, baseRef: string): string { const filePath = getCommentFilePath(file, comment.scope); - const scopePrefix = `[${formatScopeLabel(comment.scope)}] `; + const scopePrefix = `[${formatScopeLabel(comment.scope, baseRef)}] `; if (comment.side === "file" || comment.startLine == null) { return `${scopePrefix}${filePath}`; @@ -34,12 +41,14 @@ function formatLocation(comment: DiffReviewComment, file: ReviewFile | undefined return `${scopePrefix}${filePath}:${range}${suffix}`; } -export function composeReviewPrompt(files: ReviewFile[], payload: ReviewSubmitPayload): string { +export function composeReviewPrompt(files: ReviewFile[], payload: ReviewSubmitPayload, baseRef: string): string { const fileMap = new Map(files.map((file) => [file.id, file])); const lines: string[] = []; lines.push("Please address the following feedback"); lines.push(""); + lines.push(`Primary compare base: ${baseRef}`); + lines.push(""); const overallComment = payload.overallComment.trim(); if (overallComment.length > 0) { @@ -49,7 +58,7 @@ export function composeReviewPrompt(files: ReviewFile[], payload: ReviewSubmitPa payload.comments.forEach((comment, index) => { const file = fileMap.get(comment.fileId); - lines.push(`${index + 1}. ${formatLocation(comment, file)}`); + lines.push(`${index + 1}. ${formatLocation(comment, file, baseRef)}`); lines.push(` ${comment.body.trim()}`); lines.push(""); }); diff --git a/src/types.ts b/src/types.ts index 3a1e00b..cf7be97 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,4 +1,4 @@ -export type ReviewScope = "git-diff" | "last-commit" | "all-files"; +export type ReviewScope = "base-branch" | "git-diff" | "last-commit" | "all-files"; export type ChangeStatus = "modified" | "added" | "deleted" | "renamed"; @@ -16,8 +16,10 @@ export interface ReviewFile { path: string; worktreeStatus: ChangeStatus | null; hasWorkingTreeFile: boolean; + inBaseBranch: boolean; inGitDiff: boolean; inLastCommit: boolean; + baseBranch: ReviewFileComparison | null; gitDiff: ReviewFileComparison | null; lastCommit: ReviewFileComparison | null; } @@ -77,7 +79,20 @@ export interface ReviewFileErrorMessage { export type ReviewHostMessage = ReviewFileDataMessage | ReviewFileErrorMessage; +export interface ReviewWindowPreferences { + defaultBaseRef: string; + preferredInitialScope: ReviewScope; + preferredHideUnchanged: boolean; + preferredWrapLines: boolean; + preferredSidebarCollapsed: boolean; +} + export interface ReviewWindowData { repoRoot: string; files: ReviewFile[]; + defaultBaseRef: string; + preferredInitialScope: ReviewScope; + preferredHideUnchanged: boolean; + preferredWrapLines: boolean; + preferredSidebarCollapsed: boolean; } diff --git a/web/app.js b/web/app.js index 68b0f5d..a3eba4e 100644 --- a/web/app.js +++ b/web/app.js @@ -2,19 +2,31 @@ const reviewData = JSON.parse(document.getElementById("diff-review-data").textCo const state = { activeFileId: null, - currentScope: reviewData.files.some((file) => file.inGitDiff) - ? "git-diff" - : reviewData.files.some((file) => file.inLastCommit) - ? "last-commit" - : "all-files", + currentScope: reviewData.preferredInitialScope && reviewData.files.some((file) => file[ + reviewData.preferredInitialScope === "base-branch" + ? "inBaseBranch" + : reviewData.preferredInitialScope === "git-diff" + ? "inGitDiff" + : reviewData.preferredInitialScope === "last-commit" + ? "inLastCommit" + : "hasWorkingTreeFile" + ]) + ? reviewData.preferredInitialScope + : reviewData.files.some((file) => file.inBaseBranch) + ? "base-branch" + : reviewData.files.some((file) => file.inGitDiff) + ? "git-diff" + : reviewData.files.some((file) => file.inLastCommit) + ? "last-commit" + : "all-files", comments: [], overallComment: "", - hideUnchanged: false, - wrapLines: true, + hideUnchanged: reviewData.preferredHideUnchanged ?? true, + wrapLines: reviewData.preferredWrapLines ?? false, collapsedDirs: {}, reviewedFiles: {}, scrollPositions: {}, - sidebarCollapsed: false, + sidebarCollapsed: reviewData.preferredSidebarCollapsed ?? false, fileFilter: "", fileContents: {}, fileErrors: {}, @@ -25,10 +37,12 @@ const sidebarEl = document.getElementById("sidebar"); const sidebarTitleEl = document.getElementById("sidebar-title"); const sidebarSearchInputEl = document.getElementById("sidebar-search-input"); const toggleSidebarButton = document.getElementById("toggle-sidebar-button"); +const scopeBaseBranchButton = document.getElementById("scope-base-branch-button"); const scopeDiffButton = document.getElementById("scope-diff-button"); const scopeLastCommitButton = document.getElementById("scope-last-commit-button"); const scopeAllButton = document.getElementById("scope-all-button"); const windowTitleEl = document.getElementById("window-title"); +const baseRefBadgeEl = document.getElementById("base-ref-badge"); const repoRootEl = document.getElementById("repo-root"); const fileTreeEl = document.getElementById("file-tree"); const summaryEl = document.getElementById("summary"); @@ -46,6 +60,7 @@ const toggleWrapButton = document.getElementById("toggle-wrap-button"); repoRootEl.textContent = reviewData.repoRoot || ""; windowTitleEl.textContent = "Review"; +baseRefBadgeEl.textContent = reviewData.defaultBaseRef ? `base ${reviewData.defaultBaseRef}` : "no base ref"; let monacoApi = null; let diffEditor = null; @@ -86,6 +101,7 @@ function inferLanguage(path) { function scopeLabel(scope) { switch (scope) { + case "base-branch": return "Base branch"; case "git-diff": return "Git diff"; case "last-commit": return "Last commit"; default: return "All files"; @@ -94,6 +110,8 @@ function scopeLabel(scope) { function scopeHint(scope) { switch (scope) { + case "base-branch": + return `Review branch changes from merge-base(${reviewData.defaultBaseRef}, HEAD) to HEAD. Hover or click line numbers in the gutter to add an inline comment.`; case "git-diff": return "Review working tree changes against HEAD. Hover or click line numbers in the gutter to add an inline comment."; case "last-commit": @@ -123,6 +141,8 @@ function isFileReviewed(fileId) { function getScopedFiles() { switch (state.currentScope) { + case "base-branch": + return reviewData.files.filter((file) => file.inBaseBranch); case "git-diff": return reviewData.files.filter((file) => file.inGitDiff); case "last-commit": @@ -150,6 +170,7 @@ function activeFile() { function getScopeComparison(file, scope = state.currentScope) { if (!file) return null; + if (scope === "base-branch") return file.baseBranch; if (scope === "git-diff") return file.gitDiff; if (scope === "last-commit") return file.lastCommit; return null; @@ -477,6 +498,7 @@ function updateSidebarLayout() { function updateScopeButtons() { const counts = { + baseBranch: reviewData.files.filter((file) => file.inBaseBranch).length, diff: reviewData.files.filter((file) => file.inGitDiff).length, lastCommit: reviewData.files.filter((file) => file.inLastCommit).length, all: reviewData.files.filter((file) => file.hasWorkingTreeFile).length, @@ -491,10 +513,12 @@ function updateScopeButtons() { : "cursor-pointer rounded-md border border-review-border bg-review-panel px-2.5 py-1 text-[11px] font-medium text-review-text hover:bg-[#21262d]"; }; + scopeBaseBranchButton.textContent = `Base branch${counts.baseBranch > 0 ? ` (${counts.baseBranch})` : ""}`; scopeDiffButton.textContent = `Git diff${counts.diff > 0 ? ` (${counts.diff})` : ""}`; scopeLastCommitButton.textContent = `Last commit${counts.lastCommit > 0 ? ` (${counts.lastCommit})` : ""}`; scopeAllButton.textContent = `All files${counts.all > 0 ? ` (${counts.all})` : ""}`; + applyButtonClasses(scopeBaseBranchButton, state.currentScope === "base-branch", counts.baseBranch === 0); applyButtonClasses(scopeDiffButton, state.currentScope === "git-diff", counts.diff === 0); applyButtonClasses(scopeLastCommitButton, state.currentScope === "last-commit", counts.lastCommit === 0); applyButtonClasses(scopeAllButton, state.currentScope === "all-files", counts.all === 0); @@ -1007,6 +1031,7 @@ function setupMonaco() { function switchScope(scope) { const hasScopeFiles = { + "base-branch": reviewData.files.some((file) => file.inBaseBranch), "git-diff": reviewData.files.some((file) => file.inGitDiff), "last-commit": reviewData.files.some((file) => file.inLastCommit), "all-files": reviewData.files.some((file) => file.hasWorkingTreeFile), @@ -1069,6 +1094,10 @@ toggleReviewedButton.addEventListener("click", () => { renderTree(); }); +scopeBaseBranchButton.addEventListener("click", () => { + switchScope("base-branch"); +}); + scopeDiffButton.addEventListener("click", () => { switchScope("git-diff"); }); diff --git a/web/index.html b/web/index.html index 7c36a63..e0c9a2c 100644 --- a/web/index.html +++ b/web/index.html @@ -129,7 +129,10 @@
-
Review
+
+
Review
+ +
@@ -149,6 +152,7 @@
Fuzzy filter
+