diff --git a/apps/desktop/src/App.vue b/apps/desktop/src/App.vue index f24546e6..d7ba20bd 100644 --- a/apps/desktop/src/App.vue +++ b/apps/desktop/src/App.vue @@ -70,6 +70,7 @@ const EditCommitOverlay = defineAsyncComponent(() => import("./components/EditCo const SplitCommitModal = defineAsyncComponent(() => import("./components/SplitCommitModal.vue")); const BranchDirtySwitchModal = defineAsyncComponent(() => import("./components/BranchDirtySwitchModal.vue")); const SecretsFindingsModal = defineAsyncComponent(() => import("./components/SecretsFindingsModal.vue")); +const CommitReviewModal = defineAsyncComponent(() => import("./components/CommitReviewModal.vue")); import { useStashMessage } from "./composables/useStashMessage"; import { useAIProvider } from "./composables/useAIProvider"; import { usePrPanel, PR_PANEL_KEY } from "./composables/usePrPanel"; @@ -95,6 +96,9 @@ import { useScheduler } from "./composables/useScheduler"; import { useRepoPoller } from "./composables/useRepoPoller"; import { useLaunchpadPoller } from "./composables/useLaunchpadPoller"; import { useSecretsScanner } from "./composables/useSecretsScanner"; +import { useCommitReview } from "./composables/useCommitReview"; +import { useCommitReviewNav } from "./composables/useCommitReviewNav"; +import { resolveCommitReviewShortcut } from "./composables/commitReviewKeymap"; import { useLaunchpadPrs } from "./composables/useLaunchpadPrs"; import { diffLaunchpad, isBotAuthor, type LaunchpadEvent } from "./composables/useLaunchpadNotifications"; import { osNotify } from "./composables/useOsNotification"; @@ -116,7 +120,7 @@ import { import { gitStash, gitStashPop, gitStashList, openInEditor, setGitConfig, gitDiscard, gitAddToGitignore, gitDeleteBranch, gitDeleteTag, gitDeleteRemoteTag, gitRemoteInfo, gitUnpushedTags, gitPushTags, gitMergeBase, gitResetToCommit, gitCommitSubmoduleChanges, gitSubmoduleCheckUpdates, scratchWorktreeCreate, scratchWorktreeDiscard, scratchWorktreeMergeBack, gitWorktreeList, gitWorktreeRemove, type CommitSubmoduleChange } from "./utils/backend"; import { useCommitActions } from "./composables/useCommitActions"; -const { t } = useI18n(); +const { t, locale } = useI18n(); const { settings, refreshSettings } = useSettings(); const { saveMemory } = useResolutionMemory(); @@ -126,6 +130,10 @@ const showSecretsModal = ref(false); /** Trailers of the last commit attempt — reused by the findings modal's "Commit anyway" so it * doesn't need its own copy of RepoSidebar's Signed-off-by trailer logic. */ let lastAttemptedCommitTrailers = ""; + +// v3.7.0 — Commit Review (local, opt-in, off by default; see useCommitReview.ts). +const commitReview = useCommitReview(); +const showCommitReviewModal = ref(false); // `useNetworkStatus` covers `navigator.onLine` — kept around because // `useScheduler` already consumes it and we don't want to retire that path // in this commit. `useConnectivity` (F1) adds a real probe-based signal @@ -267,6 +275,71 @@ const { worktreeBranches, } = useGitRepo({ confirm: askConfirm }); +// v3.7.0 — Commit Review: template ref to the mounted DiffViewer (its +// `scrollToFinding` is called by Task 2's nav composable), and the findings +// for the file currently displayed. Findings are index-scoped (reviewed the +// staged diff, not the working tree), so never paint them on an unstaged diff. +const diffViewerRef = ref(null); +const findingsForSelectedFile = computed(() => + repoSelectedFileStaged.value && repoSelectedFile.value + ? commitReview.findings.value.filter((f) => f.path === repoSelectedFile.value) + : [], +); + +/** Task 2 (v3.7.0) — `n`/`p` cycle every finding across staged files, + * switching the selected file and scrolling the diff to each. */ +const commitReviewNav = useCommitReviewNav({ + findings: commitReview.findings, + selectFile: (path, staged) => repoSelectFile(path, staged), + diffHandle: diffViewerRef, + onDismiss: (id) => commitReview.dismiss(id), + onHelp: () => showCommitReviewNavHelp(), +}); + +/** Shared transient toast for Commit Review's own non-error feedback (the + * `?` help reminder, a clean-pass confirmation) — reuses the existing toast + * affordance rather than inventing a second one (verifier issue #5). */ +function showCommitReviewToast(title: string, detail: string) { + if (successTimer != null) { window.clearTimeout(successTimer); successTimer = null; } + successToastLeaving.value = false; + successToast.value = title; + successToastDetail.value = detail; + successTimer = window.setTimeout(dismissToast, 3000); +} + +/** `?` — a one-line toast (per the plan: reuse the existing toast + * affordance, no new help modal). */ +function showCommitReviewNavHelp() { + showCommitReviewToast( + t("commitReview.navHelp"), + `${t("commitReview.navNext")} (N) · ${t("commitReview.navPrev")} (P) · ${t("commitReview.dismiss")} (X)`, + ); +} + +/** A completed review with zero findings otherwise produces no feedback at + * all, indistinguishable from "didn't run" or "failed" (verifier issue + * #5) — `commitReview.summaryClean` already exists but was unreachable. + * Called from `reviewStaged` below only when the run actually completed + * (not skipped, not superseded) and produced no error. */ +function showCommitReviewCleanToast() { + showCommitReviewToast(t("commitReview.summaryClean"), ""); +} + +/** Task 2 (v3.7.0) — the commit-review keymap is only "active" (bare-letter + * n/p/x reserved) in the Changes view, with the feature on and at least + * one finding to navigate. */ +const commitReviewShortcutActive = computed( + () => viewMode.value === "changes" && settings.value.commitReviewEnabled && commitReview.findings.value.length > 0, +); + +/** Verifier issue #4 — a failed review must never fail silently. Reuse the + * existing error-toast banner (`repoError`) rather than inventing a second + * one; this mirrors every other place in this file that funnels a failure + * into `repoError.value`. */ +watch(commitReview.lastError, (val) => { + if (val) repoError.value = val; +}); + // Monorepo scope (v2.21.0) — restore persisted scope on repo open. const { loadScope } = useWorkspaceScope(); @@ -331,6 +404,12 @@ const prPanel = usePrPanel(prCwd, { await repoRefresh(); await loadBranches(); }, + // v3.7.0 — Commit Review: resume its queue on the same visibilitychange + // → visible edge usePrPanel already reuses for the PR pre-review queue, + // instead of standing up a second listener. Without this, starting a + // review then hiding the tab would leave the queue paused on + // document.hidden forever. + onVisibilityResume: () => commitReview.resume(), }); provide(PR_PANEL_KEY, prPanel); const issuePanel = useIssuePanel(prCwd); @@ -1016,6 +1095,11 @@ const repoSidebarProps = computed(() => ({ visibleFileIdx: historyVisibleFileIdx.value, gitUser: currentGitUser.value, secretFindingsCount: secretsScanner.activeFindings.value.length, + commitReviewEnabled: settings.value.commitReviewEnabled, + commitReviewRunning: commitReview.running.value, + commitReviewFindingsCount: commitReview.findings.value.length, + commitReviewProgress: commitReview.progress.value, + reviewFindingsByFile: commitReview.findingsByFile.value, })); /** @@ -1065,6 +1149,33 @@ function onSecretsCommitAnyway() { void doCommit(lastAttemptedCommitTrailers); } +/** v3.7.0 — "Review staged changes" button handler. Shows a brief clean-pass + * confirmation when the run actually completed (not skipped, not + * superseded by a newer run) with no error and zero findings — otherwise a + * clean review is indistinguishable from "didn't run" or "failed" + * (verifier issue #5). A failed run is separately surfaced via the + * `commitReview.lastError` watcher above (issue #4). */ +async function onReviewStagedClicked() { + const ran = await commitReview.run(repoFolderPath.value ?? "", locale.value); + if (ran && !commitReview.lastError.value && commitReview.findings.value.length === 0) { + showCommitReviewCleanToast(); + } +} + +/** v3.7.0 — "Jump to" in the findings modal: select the finding's file + * (always staged — findings are index-scoped) and scroll the diff to it. */ +function onJumpToCommitReviewFinding(id: string) { + const finding = commitReview.findings.value.find((f) => f.id === id); + if (!finding) return; + if (repoSelectedFile.value !== finding.path || !repoSelectedFileStaged.value) { + repoSelectFile(finding.path, true); + } + showCommitReviewModal.value = false; + void nextTick(() => { + diffViewerRef.value?.scrollToFinding?.(finding.line, finding.side); + }); +} + const repoSidebarListeners = { select: (path: string, staged: boolean) => onRepoFileSelect(path, staged), changeView: (mode: ViewMode) => onViewModeChange(mode), @@ -1088,6 +1199,8 @@ const repoSidebarListeners = { deleteBranch: (name: string, hasLocal: boolean, hasRemote: boolean, remoteName?: string) => handleDeleteBranchRequest(name, hasLocal, hasRemote, remoteName), openSecrets: () => { showSecretsModal.value = true; }, + reviewStaged: () => { void onReviewStagedClicked(); }, + openCommitReview: () => { showCommitReviewModal.value = true; }, }; // Trigger a (debounced) secrets scan whenever the staged set changes, or when a repo is @@ -1100,6 +1213,12 @@ watch( } else { secretsScanner.findings.value = []; } + // v3.7.0 — Commit Review: a staged-set/repo change invalidates whatever + // findings are on screen (the diff they reviewed no longer exists). + // Never auto-run here — the pass only runs on the explicit "Review + // staged changes" click (decision D5; the one-shot re-review after a + // "Fix with agent" handoff is Task 3, out of scope for this PR). + commitReview.reset(); }, { immediate: true }, ); @@ -2810,6 +2929,16 @@ function onKeyDown(e: KeyboardEvent) { } return; } + // Task 2 (v3.7.0) — n/p/x cycle/dismiss commit-review findings in the + // Changes view. Checked early, before the mod-key ladder, and the + // resolver itself guards editable targets (the commit summary/description + // fields live in this same view) and inactivity — see commitReviewKeymap.ts. + const commitReviewAction = resolveCommitReviewShortcut(e, { active: commitReviewShortcutActive.value }); + if (commitReviewAction) { + e.preventDefault(); + commitReviewNav.dispatch(commitReviewAction); + return; + } if (mod && e.key === "t") { // Cmd+T — new tab (open folder picker) e.preventDefault(); @@ -3349,10 +3478,12 @@ onUnmounted(() => { - + @select-dir-file="(path) => repoSelectFile(path, false)" + @dismiss-finding="(id) => commitReview.dismiss(id)" />
{ @close="showSecretsModal = false" /> + + +
+ + {{ reviewFindingsByFile![file.path] }}
+ + {{ reviewFindingsByFile![row.path] }}
+ + +
+ +
+
+
+
+ {{ t('settings.commitReview.title') }} + {{ t('settings.commitReview.hint') }} +
+
+ +
+ + {{ t('settings.commitReview.enabledHint') }} +
+ + +
+
diff --git a/apps/desktop/src/components/__tests__/CommitReviewModal.test.ts b/apps/desktop/src/components/__tests__/CommitReviewModal.test.ts new file mode 100644 index 00000000..d8966188 --- /dev/null +++ b/apps/desktop/src/components/__tests__/CommitReviewModal.test.ts @@ -0,0 +1,96 @@ +/** + * Task 1b (v3.7.0) — `CommitReviewModal`: summary + severity-sorted finding + * list + jump/dismiss/close. Mounted with native `createApp`, mirroring + * `SecretsFindingsModal.test.ts`. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { createApp, type App } from "vue"; +import CommitReviewModal from "../CommitReviewModal.vue"; +import type { ReviewFinding } from "../../composables/usePrPreReview"; + +function finding(overrides: Partial = {}): ReviewFinding { + return { + id: "f1", + path: "src/a.ts", + line: 1, + side: "RIGHT", + severity: "nit", + confidence: 50, + title: "A nit", + detail: "A nit detail", + ...overrides, + }; +} + +const findings: ReviewFinding[] = [ + finding({ id: "nit-1", severity: "nit", confidence: 90, title: "High-confidence nit" }), + finding({ id: "risk-1", severity: "risk", confidence: 60, title: "A risk", path: "src/b.ts", line: 5 }), + finding({ id: "suggestion-1", severity: "suggestion", confidence: 80, title: "A suggestion" }), +]; + +let app: App | null = null; +let container: HTMLElement; + +function mount(props: Record = {}) { + container = document.createElement("div"); + document.body.appendChild(container); + app = createApp(CommitReviewModal, { findings, ...props }); + app.mount(container); +} + +beforeEach(() => { + localStorage.clear(); +}); + +afterEach(() => { + app?.unmount(); + app = null; + container?.remove(); +}); + +describe("CommitReviewModal", () => { + it("renders findings sorted by severity (risk > suggestion > nit), then confidence descending", () => { + mount(); + const rows = document.querySelectorAll(".crm-item"); + expect(rows.length).toBe(3); + expect(rows[0].textContent).toContain("A risk"); + expect(rows[1].textContent).toContain("A suggestion"); + expect(rows[2].textContent).toContain("High-confidence nit"); + }); + + it("renders path:line, confidence, title, and detail for each finding", () => { + mount(); + const rows = document.querySelectorAll(".crm-item"); + expect(rows[0].textContent).toContain("src/b.ts"); + expect(rows[0].textContent).toContain("5"); + expect(rows[0].textContent).toContain("60"); + expect(rows[0].textContent).toContain("A risk"); + }); + + it("shows the empty state when there are no findings", () => { + mount({ findings: [] }); + expect(document.querySelector(".crm-empty")).not.toBeNull(); + expect(document.querySelectorAll(".crm-item").length).toBe(0); + }); + + it("emits jump with the finding's id when Jump to is clicked", () => { + const onJump = vi.fn(); + mount({ onJump }); + document.querySelectorAll(".crm-item__jump")[0].click(); + expect(onJump).toHaveBeenCalledWith("risk-1"); + }); + + it("emits dismiss with the finding's id when Dismiss is clicked", () => { + const onDismiss = vi.fn(); + mount({ onDismiss }); + document.querySelectorAll(".crm-item__dismiss")[0].click(); + expect(onDismiss).toHaveBeenCalledWith("risk-1"); + }); + + it("emits close when the footer Close action is clicked", () => { + const onClose = vi.fn(); + mount({ onClose }); + document.querySelector(".crm-footer-close")!.click(); + expect(onClose).toHaveBeenCalled(); + }); +}); diff --git a/apps/desktop/src/components/__tests__/DiffViewer-findings.test.ts b/apps/desktop/src/components/__tests__/DiffViewer-findings.test.ts new file mode 100644 index 00000000..06c32c4d --- /dev/null +++ b/apps/desktop/src/components/__tests__/DiffViewer-findings.test.ts @@ -0,0 +1,144 @@ +/** + * Task 1b (v3.7.0) — inline commit-review finding rows in `DiffViewer` + * (inline mode). Mounted with the native `createApp` (no @vue/test-utils + * dep), mirroring `PrInlineDiff.test.ts`. + */ +import { describe, it, expect, afterEach } from "vitest"; +import { createApp, nextTick, type App } from "vue"; +import DiffViewer from "../DiffViewer.vue"; +import type { GitDiff, DiffLine } from "../../utils/backend"; +import type { ReviewFinding } from "../../composables/usePrPreReview"; + +function line(type: DiffLine["type"], content: string, oldLineNo?: number, newLineNo?: number): DiffLine { + return { type, content, oldLineNo, newLineNo }; +} + +function smallDiff(): GitDiff { + return { + path: "src/foo.ts", + hunks: [ + { + header: "@@ -1,3 +1,3 @@", + oldStart: 1, oldCount: 3, newStart: 1, newCount: 3, + lines: [ + line("context", "unchanged", 1, 1), + line("delete", "old value", 2, undefined), + line("add", "new value", undefined, 2), + ], + }, + ], + }; +} + +function finding(overrides: Partial = {}): ReviewFinding { + return { + id: "f1", + path: "src/foo.ts", + line: 2, + side: "RIGHT", + severity: "risk", + confidence: 80, + title: "Risk title", + detail: "Risk detail", + ...overrides, + }; +} + +interface MountResult { app: App; container: HTMLDivElement; vm: any } + +function mountDiff(props: Record): MountResult { + const container = document.createElement("div"); + document.body.appendChild(container); + const app = createApp(DiffViewer as any, props); + const vm = app.mount(container); + return { app, container, vm }; +} + +function unmount({ app, container }: MountResult) { + app.unmount(); + if (container.parentNode) container.parentNode.removeChild(container); +} + +describe("DiffViewer — inline commit-review finding rows", () => { + let mounted: MountResult | null = null; + + afterEach(() => { + if (mounted) unmount(mounted); + mounted = null; + }); + + it("renders one finding row anchored on the added line, with the right severity class", async () => { + mounted = mountDiff({ + diff: smallDiff(), + filePath: "src/foo.ts", + diffMode: "inline", + findings: [finding({ severity: "risk" })], + }); + await nextTick(); + + const rows = mounted.container.querySelectorAll(".diff-finding-row"); + expect(rows.length).toBe(1); + expect(rows[0].querySelector(".diff-finding-severity")?.classList.contains("diff-finding-severity--risk")).toBe(true); + expect(rows[0].textContent).toContain("Risk title"); + expect(rows[0].textContent).toContain("Risk detail"); + expect(rows[0].textContent).toContain("80"); + + // Regression guard: the must stay a plain table-cell (no display + // override) so its colspan actually spans the row — the flex layout + // lives on the inner `.diff-finding-body` wrapper instead. A `td` with + // `display: flex` silently loses `colspan` in the CSS box model. + const cell = rows[0].querySelector("td.diff-finding-cell") as HTMLTableCellElement; + expect(cell).not.toBeNull(); + expect(cell.colSpan).toBeGreaterThan(1); + expect(cell.classList.contains("diff-finding-body")).toBe(false); + expect(cell.querySelector(":scope > .diff-finding-body")).not.toBeNull(); + }); + + it("renders no row (and does not throw) for a finding whose line isn't in the diff", async () => { + mounted = mountDiff({ + diff: smallDiff(), + filePath: "src/foo.ts", + diffMode: "inline", + findings: [finding({ id: "orphan", line: 99, severity: "nit" })], + }); + await nextTick(); + + expect(mounted.container.querySelectorAll(".diff-finding-row").length).toBe(0); + }); + + it("emits dismiss-finding with the finding's id when Dismiss is clicked", async () => { + let dismissedId: string | null = null; + mounted = mountDiff({ + diff: smallDiff(), + filePath: "src/foo.ts", + diffMode: "inline", + findings: [finding({ id: "f-dismiss" })], + onDismissFinding: (id: string) => { dismissedId = id; }, + }); + await nextTick(); + + mounted.container.querySelector(".diff-finding-dismiss")!.click(); + expect(dismissedId).toBe("f-dismiss"); + }); + + it("does not merge LEFT and RIGHT findings that share the same line number", async () => { + mounted = mountDiff({ + diff: smallDiff(), + filePath: "src/foo.ts", + diffMode: "inline", + findings: [ + finding({ id: "right-2", side: "RIGHT", line: 2, title: "Right finding" }), + finding({ id: "left-2", side: "LEFT", line: 2, title: "Left finding" }), + ], + }); + await nextTick(); + + const rows = mounted.container.querySelectorAll(".diff-finding-row"); + expect(rows.length).toBe(2); + // Each row carries exactly one finding, not a merged pair. + expect(rows[0].textContent).toContain("Left finding"); + expect(rows[0].textContent).not.toContain("Right finding"); + expect(rows[1].textContent).toContain("Right finding"); + expect(rows[1].textContent).not.toContain("Left finding"); + }); +}); diff --git a/apps/desktop/src/composables/__tests__/commitReviewKeymap.test.ts b/apps/desktop/src/composables/__tests__/commitReviewKeymap.test.ts new file mode 100644 index 00000000..ff4f37ed --- /dev/null +++ b/apps/desktop/src/composables/__tests__/commitReviewKeymap.test.ts @@ -0,0 +1,71 @@ +/** + * Task 2 (v3.7.0) — pure keymap resolver for Commit Review finding + * navigation. Mirrors `usePrReviewKeymap.test.ts`'s structure. + */ +import { describe, it, expect } from "vitest"; +import { resolveCommitReviewShortcut } from "../commitReviewKeymap"; + +function kd(key: string, opts: Partial = {}, target?: EventTarget): KeyboardEvent { + const e = new KeyboardEvent("keydown", { key, ...opts }); + if (target) Object.defineProperty(e, "target", { value: target }); + return e; +} + +describe("resolveCommitReviewShortcut", () => { + const active = { active: true }; + + it("maps n/p/x to their actions", () => { + expect(resolveCommitReviewShortcut(kd("n"), active)).toBe("next-finding"); + expect(resolveCommitReviewShortcut(kd("p"), active)).toBe("prev-finding"); + expect(resolveCommitReviewShortcut(kd("x"), active)).toBe("dismiss-finding"); + }); + + it("maps ? (Shift+/ on every standard keyboard layout) to help", () => { + // Regression guard: a resolver that bails on ANY held modifier (including + // shiftKey) before reaching the "?" case makes this branch unreachable on + // a real keyboard, since "?" cannot be typed without Shift. + expect(resolveCommitReviewShortcut(kd("?", { shiftKey: true }), active)).toBe("help"); + }); + + it("returns null for every mapped key when inactive", () => { + const inactive = { active: false }; + expect(resolveCommitReviewShortcut(kd("n"), inactive)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("p"), inactive)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("x"), inactive)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("?"), inactive)).toBeNull(); + }); + + it("returns null for INPUT/TEXTAREA/contenteditable targets: bare letters must be inert while typing", () => { + const input = document.createElement("input"); + const textarea = document.createElement("textarea"); + const div = document.createElement("div"); + div.setAttribute("contenteditable", "true"); + expect(resolveCommitReviewShortcut(kd("n", {}, input), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("p", {}, textarea), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("x", {}, div), active)).toBeNull(); + }); + + it("returns null when meta/ctrl/alt is held, for any mapped key including ?", () => { + expect(resolveCommitReviewShortcut(kd("n", { metaKey: true }), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("n", { ctrlKey: true }), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("n", { altKey: true }), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("?", { shiftKey: true, metaKey: true }), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("?", { shiftKey: true, ctrlKey: true }), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("?", { shiftKey: true, altKey: true }), active)).toBeNull(); + }); + + it("returns null when shift is held for the bare-letter keys (n/p/x), unlike ?", () => { + expect(resolveCommitReviewShortcut(kd("n", { shiftKey: true }), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("p", { shiftKey: true }), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("x", { shiftKey: true }), active)).toBeNull(); + }); + + it("never maps Escape or ⌘⇧L — those stay owned by App.vue's global handlers", () => { + expect(resolveCommitReviewShortcut(kd("Escape"), active)).toBeNull(); + expect(resolveCommitReviewShortcut(kd("l", { metaKey: true, shiftKey: true }), active)).toBeNull(); + }); + + it("returns null for unmapped keys", () => { + expect(resolveCommitReviewShortcut(kd("z"), active)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/composables/__tests__/useCommitReview.test.ts b/apps/desktop/src/composables/__tests__/useCommitReview.test.ts new file mode 100644 index 00000000..793cef43 --- /dev/null +++ b/apps/desktop/src/composables/__tests__/useCommitReview.test.ts @@ -0,0 +1,289 @@ +/** + * Task 1a (v3.7.0) — `useCommitReview` orchestrator: staged-diff AI review + * engine, opt-in and off by default. Mocks `../../utils/backend` and + * `../useAIProvider` per the established convention (`usePrPreReview.test.ts`). + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +const rawPromptMock = vi.fn(); +const isAvailableRef = { value: true }; +const getGitBlameMock = vi.fn(); +const gitExecMock = vi.fn(); + +vi.mock("../useAIProvider", () => ({ + useAIProvider: () => ({ + isAvailable: isAvailableRef, + rawPrompt: (...a: unknown[]) => rawPromptMock(...a), + }), +})); + +vi.mock("../../utils/backend", () => ({ + getGitBlame: (...a: unknown[]) => getGitBlameMock(...a), + gitExec: (...a: unknown[]) => gitExecMock(...a), +})); + +import { useCommitReview, COMMIT_REVIEW_MAX_FILES, COMMIT_REVIEW_MAX_BYTES } from "../useCommitReview"; +import { useSettings, defaultAppSettings } from "../useSettings"; + +function diffFor(path: string): string { + return [ + `diff --git a/${path} b/${path}`, + "index 111..222 100644", + `--- a/${path}`, + `+++ b/${path}`, + "@@ -1,1 +1,1 @@", + "-old", + "+new", + ].join("\n"); +} + +function gitExecOk(stdout: string) { + return { stdout, stderr: "", exitCode: 0 }; +} + +function enableCommitReview(overrides: Partial = {}) { + const { settings } = useSettings(); + settings.value = { ...defaultAppSettings, commitReviewEnabled: true, ...overrides }; +} + +function setHidden(hidden: boolean) { + Object.defineProperty(document, "hidden", { value: hidden, configurable: true }); +} + +describe("useCommitReview", () => { + beforeEach(() => { + rawPromptMock.mockReset(); + getGitBlameMock.mockReset().mockResolvedValue([]); + gitExecMock.mockReset(); + isAvailableRef.value = true; + const { settings } = useSettings(); + settings.value = { ...defaultAppSettings }; + }); + + afterEach(() => { + setHidden(false); + }); + + it("performs zero IPC and zero LLM calls when the setting is disabled, and reports it did not run", async () => { + const { settings } = useSettings(); + settings.value = { ...defaultAppSettings, commitReviewEnabled: false }; + const review = useCommitReview(); + const ran = await review.run("/repo", "en"); + expect(gitExecMock).not.toHaveBeenCalled(); + expect(rawPromptMock).not.toHaveBeenCalled(); + expect(review.findings.value).toEqual([]); + expect(ran).toBe(false); + }); + + it("performs zero IPC and zero LLM calls when the AI provider is unavailable, and reports it did not run", async () => { + enableCommitReview(); + isAvailableRef.value = false; + const review = useCommitReview(); + const ran = await review.run("/repo", "en"); + expect(gitExecMock).not.toHaveBeenCalled(); + expect(rawPromptMock).not.toHaveBeenCalled(); + expect(review.findings.value).toEqual([]); + expect(ran).toBe(false); + }); + + it("reviews two staged files and aggregates findings + progress + per-file counts", async () => { + enableCommitReview(); + gitExecMock.mockResolvedValue(gitExecOk(`${diffFor("a.ts")}\n${diffFor("b.ts")}`)); + rawPromptMock + .mockResolvedValueOnce('[{"line": 1, "title": "finding a", "confidence": 80}]') + .mockResolvedValueOnce('[{"line": 1, "title": "finding b", "confidence": 80}]'); + + const review = useCommitReview(); + const ran = await review.run("/repo", "en"); + + expect(review.findings.value).toHaveLength(2); + expect(review.progress.value).toEqual({ done: 2, total: 2 }); + expect(review.findingsByFile.value).toEqual({ "a.ts": 1, "b.ts": 1 }); + expect(ran).toBe(true); + }); + + it("sets lastError (never throws) and leaves findings empty when gitExec exits non-zero", async () => { + enableCommitReview(); + gitExecMock.mockResolvedValue({ stdout: "", stderr: "fatal: not a git repository", exitCode: 128 }); + + const review = useCommitReview(); + let ran: boolean | undefined; + await expect((async () => { ran = await review.run("/repo", "en"); })()).resolves.toBeUndefined(); + expect(review.findings.value).toEqual([]); + expect(review.lastError.value).toBeTruthy(); + expect(rawPromptMock).not.toHaveBeenCalled(); + // A full attempt did occur (it just failed) — distinct from the + // disabled/unavailable "never even tried" case above. + expect(ran).toBe(true); + }); + + it("makes no LLM call on an empty staged diff (clean index is not an error), and reports it ran", async () => { + enableCommitReview(); + gitExecMock.mockResolvedValue(gitExecOk("")); + + const review = useCommitReview(); + const ran = await review.run("/repo", "en"); + + expect(rawPromptMock).not.toHaveBeenCalled(); + expect(review.findings.value).toEqual([]); + expect(review.lastError.value).toBeNull(); + expect(ran).toBe(true); + }); + + it("aborts a run in flight when a second run() starts, painting no stale findings", async () => { + enableCommitReview(); + let resolveFirst!: (v: string) => void; + const pending = new Promise((resolve) => { resolveFirst = resolve; }); + + gitExecMock + .mockResolvedValueOnce(gitExecOk(diffFor("a.ts"))) + .mockResolvedValueOnce(gitExecOk(diffFor("c.ts"))); + rawPromptMock + .mockImplementationOnce(() => pending) + .mockResolvedValueOnce('[{"line": 1, "title": "finding c", "confidence": 80}]'); + + const review = useCommitReview(); + const firstRun = review.run("/repo", "en"); // starts, blocks on rawPrompt for a.ts + // Flush every pending microtask chain (gitExec, queue's waitWhileHidden, + // getGitBlame) so execution actually reaches the blocked rawPrompt call + // before the second run starts — a macrotask tick drains all of them + // since none of those awaits depend on a timer themselves. + await new Promise((resolve) => setTimeout(resolve, 0)); + + const secondRun = review.run("/repo", "en"); // aborts the first, reviews c.ts + await secondRun; + resolveFirst('[{"line": 1, "title": "finding a (stale)", "confidence": 80}]'); + await firstRun; + await Promise.resolve(); + + expect(review.findings.value.map((f) => f.title)).toEqual(["finding c"]); + }); + + it("reset() clears findings, error, and aborts any run in flight", async () => { + enableCommitReview(); + gitExecMock.mockResolvedValue(gitExecOk(diffFor("a.ts"))); + rawPromptMock.mockResolvedValue('[{"line": 1, "title": "finding a", "confidence": 80}]'); + + const review = useCommitReview(); + await review.run("/repo", "en"); + expect(review.findings.value).toHaveLength(1); + + review.reset(); + expect(review.findings.value).toEqual([]); + expect(review.rawFindings.value).toEqual([]); + expect(review.lastError.value).toBeNull(); + }); + + it("keeps a below-threshold finding in rawFindings but filters it out of findings", async () => { + enableCommitReview({ reviewAiConfidenceThreshold: 60 }); + gitExecMock.mockResolvedValue(gitExecOk(diffFor("a.ts"))); + rawPromptMock.mockResolvedValue('[{"line": 1, "title": "low-confidence finding", "confidence": 20}]'); + + const review = useCommitReview(); + await review.run("/repo", "en"); + + expect(review.findings.value).toEqual([]); + expect(review.rawFindings.value).toHaveLength(1); + }); + + it("caps the staged file count at exactly COMMIT_REVIEW_MAX_FILES and marks truncated", async () => { + enableCommitReview(); + const manyFiles = Array.from({ length: 45 }, (_, i) => diffFor(`f${i}.ts`)).join("\n"); + gitExecMock.mockResolvedValue(gitExecOk(manyFiles)); + rawPromptMock.mockResolvedValue("[]"); + + const review = useCommitReview(); + await review.run("/repo", "en"); + + // Tightened from toBeLessThanOrEqual: that assertion passed even with 0 + // calls, so it never actually proved the cap triggered. 45 small files + // fit well within the byte budget, so the file-count cap is the only + // thing that can be limiting here — exactly 40 calls. + expect(rawPromptMock.mock.calls.length).toBe(COMMIT_REVIEW_MAX_FILES); + expect(review.truncated.value).toBe(true); + }); + + it("truncates by the byte budget when a single file's diff exceeds it, excluding files after it", async () => { + enableCommitReview(); + const hugeAddedLine = "+" + "x".repeat(COMMIT_REVIEW_MAX_BYTES + 1000); + const hugeDiff = [ + "diff --git a/huge.ts b/huge.ts", + "index 111..222 100644", + "--- a/huge.ts", + "+++ b/huge.ts", + "@@ -1,1 +1,1 @@", + "-old", + hugeAddedLine, + ].join("\n"); + gitExecMock.mockResolvedValue(gitExecOk(`${hugeDiff}\n${diffFor("small.ts")}`)); + rawPromptMock.mockResolvedValue("[]"); + + const review = useCommitReview(); + await review.run("/repo", "en"); + + // huge.ts alone exhausts the byte budget, so small.ts (after it in the + // diff) is never sent through the review pass. + expect(rawPromptMock).toHaveBeenCalledTimes(1); + expect(review.truncated.value).toBe(true); + }); + + it("resume() unblocks a run paused on document.hidden and lets it complete (never stays wedged)", async () => { + enableCommitReview(); + gitExecMock.mockResolvedValue(gitExecOk(diffFor("a.ts"))); + rawPromptMock.mockResolvedValue('[{"line": 1, "title": "finding a", "confidence": 80}]'); + + setHidden(true); + const review = useCommitReview(); + const runPromise = review.run("/repo", "en"); + + // Give the run a beat to reach the queue's document.hidden pause — it + // must not have called rawPrompt yet, and `running` must still be true + // (this is the exact hang the fix guards against: without resume() ever + // being called, this promise would never settle). + await new Promise((resolve) => setTimeout(resolve, 5)); + expect(rawPromptMock).not.toHaveBeenCalled(); + expect(review.running.value).toBe(true); + + setHidden(false); + review.resume(); + await runPromise; + + expect(rawPromptMock).toHaveBeenCalledTimes(1); + expect(review.findings.value).toHaveLength(1); + expect(review.running.value).toBe(false); + }); + + it("does not let a stale aborted run's cleanup clobber a newer run's running/progress state", async () => { + enableCommitReview(); + let resolveFirst!: (v: string) => void; + const pending = new Promise((resolve) => { resolveFirst = resolve; }); + + gitExecMock + .mockResolvedValueOnce(gitExecOk(diffFor("a.ts"))) + .mockResolvedValueOnce(gitExecOk(diffFor("c.ts"))); + rawPromptMock + .mockImplementationOnce(() => pending) + .mockResolvedValueOnce("[]"); + + const review = useCommitReview(); + const firstRun = review.run("/repo", "en"); // blocks on rawPrompt for a.ts + await new Promise((resolve) => setTimeout(resolve, 0)); + + const secondRun = review.run("/repo", "en"); // aborts the first, reviews c.ts + await secondRun; + + // Second run completed fully — running/progress must reflect it. + expect(review.running.value).toBe(false); + expect(review.progress.value).toEqual({ done: 1, total: 1 }); + + // Now let the FIRST run's stale in-flight analyzeOne resolve. Its own + // (now-orphaned) queue instance settles independently — it must not + // touch the second run's running/progress state. + resolveFirst("[]"); + await firstRun; + await Promise.resolve(); + + expect(review.running.value).toBe(false); + expect(review.progress.value).toEqual({ done: 1, total: 1 }); + }); +}); diff --git a/apps/desktop/src/composables/__tests__/useCommitReviewNav.test.ts b/apps/desktop/src/composables/__tests__/useCommitReviewNav.test.ts new file mode 100644 index 00000000..7b67da37 --- /dev/null +++ b/apps/desktop/src/composables/__tests__/useCommitReviewNav.test.ts @@ -0,0 +1,123 @@ +/** + * Task 2 (v3.7.0) — `useCommitReviewNav`: finding-to-finding cycling across + * staged files. Port of `usePrReviewNav.ts`'s `jumpToFinding`, adapted to + * switch the selected staged file via an injected `selectFile` callback + * instead of a PR's file sidebar. + */ +import { describe, it, expect, vi, beforeEach, type Mock } from "vitest"; +import { ref, nextTick } from "vue"; +import { useCommitReviewNav } from "../useCommitReviewNav"; +import type { ReviewFinding } from "../usePrPreReview"; + +function finding(overrides: Partial = {}): ReviewFinding { + return { + id: "f1", + path: "a.ts", + line: 1, + side: "RIGHT", + severity: "nit", + confidence: 50, + title: "t", + detail: "d", + ...overrides, + }; +} + +describe("useCommitReviewNav", () => { + let selectFile: Mock<(path: string, staged: boolean) => void>; + let scrollToFinding: Mock<(line: number, side: "LEFT" | "RIGHT") => void>; + let onDismiss: Mock<(id: string) => void>; + let onHelp: Mock<() => void>; + + beforeEach(() => { + selectFile = vi.fn<(path: string, staged: boolean) => void>(); + scrollToFinding = vi.fn<(line: number, side: "LEFT" | "RIGHT") => void>(); + onDismiss = vi.fn<(id: string) => void>(); + onHelp = vi.fn<() => void>(); + }); + + function setup(findingsList: ReviewFinding[]) { + const findings = ref(findingsList); + const diffHandle = ref({ scrollToFinding }); + const nav = useCommitReviewNav({ findings, selectFile, diffHandle, onDismiss, onHelp }); + return { nav, findings }; + } + + it("wraps forward from -1 to the first finding", async () => { + const { nav } = setup([finding({ id: "a" }), finding({ id: "b" })]); + nav.jumpToFinding(1); + expect(nav.currentFindingIdx.value).toBe(0); + await nextTick(); + expect(selectFile).toHaveBeenCalledWith("a.ts", true); + }); + + it("wraps backward from -1 to the last finding", () => { + const { nav } = setup([finding({ id: "a" }), finding({ id: "b" }), finding({ id: "c" })]); + nav.jumpToFinding(-1); + expect(nav.currentFindingIdx.value).toBe(2); + }); + + it("wraps around past the end forward, and before the start backward", () => { + const { nav } = setup([finding({ id: "a" }), finding({ id: "b" })]); + nav.jumpToFinding(1); // -> 0 + nav.jumpToFinding(1); // -> 1 + nav.jumpToFinding(1); // -> wraps to 0 + expect(nav.currentFindingIdx.value).toBe(0); + nav.jumpToFinding(-1); // -> wraps to 1 + expect(nav.currentFindingIdx.value).toBe(1); + }); + + it("switches file via the injected selectFile(path, true) callback on a cross-file jump", () => { + const { nav } = setup([ + finding({ id: "a", path: "a.ts" }), + finding({ id: "b", path: "b.ts", severity: "risk" }), + ]); + nav.jumpToFinding(1); + nav.jumpToFinding(1); + expect(selectFile).toHaveBeenLastCalledWith(expect.any(String), true); + }); + + it("calls diffHandle.scrollToFinding with the finding's line/side after nextTick", async () => { + const { nav } = setup([finding({ id: "a", line: 7, side: "LEFT" })]); + nav.jumpToFinding(1); + expect(scrollToFinding).not.toHaveBeenCalled(); + await nextTick(); + expect(scrollToFinding).toHaveBeenCalledWith(7, "LEFT"); + }); + + it("is a no-op on an empty findings list", () => { + const { nav } = setup([]); + nav.jumpToFinding(1); + expect(nav.currentFindingIdx.value).toBe(-1); + expect(selectFile).not.toHaveBeenCalled(); + }); + + it("current is null with no findings, and the current finding once positioned", () => { + const { nav } = setup([finding({ id: "a" })]); + expect(nav.current.value).toBeNull(); + nav.jumpToFinding(1); + expect(nav.current.value?.id).toBe("a"); + }); + + it("dismiss advances the cursor sanely and never lands out of range", () => { + const { nav, findings } = setup([finding({ id: "a" }), finding({ id: "b" }), finding({ id: "c" })]); + nav.jumpToFinding(1); + nav.jumpToFinding(1); // now on "b" (idx 1) + expect(nav.current.value?.id).toBe("b"); + + onDismiss.mockImplementation((id: string) => { + findings.value = findings.value.filter((f) => f.id !== id); + }); + nav.dispatch("dismiss-finding"); + expect(onDismiss).toHaveBeenCalledWith("b"); + // List shrank to ["a", "c"] (2 items) — cursor must stay in range. + expect(nav.currentFindingIdx.value).toBeLessThan(2); + expect(nav.currentFindingIdx.value).toBeGreaterThanOrEqual(-1); + }); + + it("dispatch('help') calls the onHelp callback", () => { + const { nav } = setup([finding()]); + nav.dispatch("help"); + expect(onHelp).toHaveBeenCalled(); + }); +}); diff --git a/apps/desktop/src/composables/__tests__/usePrPanel-visibility-resume.test.ts b/apps/desktop/src/composables/__tests__/usePrPanel-visibility-resume.test.ts new file mode 100644 index 00000000..cafc5696 --- /dev/null +++ b/apps/desktop/src/composables/__tests__/usePrPanel-visibility-resume.test.ts @@ -0,0 +1,55 @@ +/** + * Verifier fix (v3.7.0, issue #1) — `usePrPanel`'s `visibilitychange` + * handler must call the injected `onVisibilityResume` option on the same + * hidden → visible edge it already uses to resume the PR pre-review queue, + * so `useCommitReview`'s queue (a different instance entirely) never stays + * wedged on `document.hidden` forever with nothing to wake it. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { ref } from "vue"; + +vi.mock("@/utils/backend", async () => { + const actual = await vi.importActual("@/utils/backend"); + return { + ...actual, + gitFileCount: vi.fn(async () => 0), + gitRemoteInfo: vi.fn(async () => ({ url: "", host: "github.com", owner: "o", repo: "r" })), + ghForkInfo: vi.fn(async () => ({ isFork: false, origin: "", parent: "" })), + }; +}); + +import { usePrPanel } from "../usePrPanel"; + +function setHidden(hidden: boolean) { + Object.defineProperty(document, "hidden", { value: hidden, configurable: true }); +} + +describe("usePrPanel — visibilitychange resume wiring", () => { + beforeEach(() => { + setHidden(false); + }); + + it("calls onVisibilityResume on the hidden -> visible edge, reusing the single listener", () => { + const onVisibilityResume = vi.fn(); + // Empty cwd: the poll-tick side effects (loadPrs/revalidateOpenDetail) + // no-op on a falsy cwd, so this test only ever exercises the + // visibility wiring, not the PR-fetch machinery. + usePrPanel(ref(""), { onVisibilityResume }); + + setHidden(true); + document.dispatchEvent(new Event("visibilitychange")); + expect(onVisibilityResume).not.toHaveBeenCalled(); + + setHidden(false); + document.dispatchEvent(new Event("visibilitychange")); + expect(onVisibilityResume).toHaveBeenCalledTimes(1); + }); + + it("never throws when onVisibilityResume is omitted", () => { + usePrPanel(ref("")); + setHidden(true); + document.dispatchEvent(new Event("visibilitychange")); + setHidden(false); + expect(() => document.dispatchEvent(new Event("visibilitychange"))).not.toThrow(); + }); +}); diff --git a/apps/desktop/src/composables/__tests__/usePrPreReview.test.ts b/apps/desktop/src/composables/__tests__/usePrPreReview.test.ts index 9e14a9b6..756d75dd 100644 --- a/apps/desktop/src/composables/__tests__/usePrPreReview.test.ts +++ b/apps/desktop/src/composables/__tests__/usePrPreReview.test.ts @@ -182,4 +182,28 @@ describe("usePrPreReview.analyzeFile", () => { const [, userPrompt] = rawPromptMock.mock.calls[0]; expect(userPrompt).toContain("carol: initial impl"); }); + + // Task 0 (v3.7.0) — the engine's prompt is scope-parametrized so + // `useCommitReview` (v3.7.0) can reuse it over the staged diff instead of + // a PR. Default is unchanged ("pr"); severity scale, confidence rules, and + // JSON contract stay byte-identical across scopes — only the framing + // sentence and the dependency-signal sentence change. + it("defaults to the pull-request framing when scope is omitted", async () => { + rawPromptMock.mockResolvedValue("[]"); + const { analyzeFile } = usePrPreReview(); + const file = diffFile("a.ts", [{ type: "add", content: "x", newLineNo: 1 }]); + await analyzeFile(file, { cwd: "/repo", otherDiffFiles: [] }); + const [systemPrompt] = rawPromptMock.mock.calls[0]; + expect(systemPrompt).toContain("pull request"); + }); + + it("uses the staged-commit framing (and never 'pull request') when scope is 'commit'", async () => { + rawPromptMock.mockResolvedValue("[]"); + const { analyzeFile } = usePrPreReview(); + const file = diffFile("a.ts", [{ type: "add", content: "x", newLineNo: 1 }]); + await analyzeFile(file, { cwd: "/repo", otherDiffFiles: [], scope: "commit" }); + const [systemPrompt] = rawPromptMock.mock.calls[0]; + expect(systemPrompt).not.toContain("pull request"); + expect(systemPrompt).toContain("staged changes"); + }); }); diff --git a/apps/desktop/src/composables/__tests__/useSettings-commitReview.test.ts b/apps/desktop/src/composables/__tests__/useSettings-commitReview.test.ts new file mode 100644 index 00000000..35aa0bea --- /dev/null +++ b/apps/desktop/src/composables/__tests__/useSettings-commitReview.test.ts @@ -0,0 +1,38 @@ +/** + * Task 1a (v3.7.0) — Commit Review settings: defaults + persistence round-trip. + * Mirrors `useSettings-reviewAi.test.ts`'s structure. + */ +import { describe, it, expect, beforeEach } from "vitest"; +import { defaultAppSettings, loadSettings, saveSettings } from "../useSettings"; + +describe("Commit Review settings (v3.7.0)", () => { + beforeEach(() => { + localStorage.clear(); + }); + + it("defaults to opt-in-off: commitReviewEnabled false, commitReviewAutoReReview true", () => { + expect(defaultAppSettings.commitReviewEnabled).toBe(false); + expect(defaultAppSettings.commitReviewAutoReReview).toBe(true); + }); + + it("round-trips through save/load", () => { + saveSettings({ + ...defaultAppSettings, + commitReviewEnabled: true, + commitReviewAutoReReview: false, + }); + const loaded = loadSettings(); + expect(loaded.commitReviewEnabled).toBe(true); + expect(loaded.commitReviewAutoReReview).toBe(false); + }); + + it("loadSettings backfills defaults for a stored payload missing the new fields", () => { + const legacy = { ...defaultAppSettings } as Record; + delete legacy.commitReviewEnabled; + delete legacy.commitReviewAutoReReview; + localStorage.setItem("gitwand-settings", JSON.stringify(legacy)); + const loaded = loadSettings(); + expect(loaded.commitReviewEnabled).toBe(false); + expect(loaded.commitReviewAutoReReview).toBe(true); + }); +}); diff --git a/apps/desktop/src/composables/commitReviewKeymap.ts b/apps/desktop/src/composables/commitReviewKeymap.ts new file mode 100644 index 00000000..356fc243 --- /dev/null +++ b/apps/desktop/src/composables/commitReviewKeymap.ts @@ -0,0 +1,49 @@ +/** + * commitReviewKeymap.ts + * + * Task 2 (v3.7.0) — pure keymap resolver for cycling Commit Review findings + * in the Changes view. Mirrors `usePrReviewKeymap.ts`'s pattern: a pure + * function from a `KeyboardEvent` + activity context to a discriminated + * action, no side effects, no Vue state. + * + * Unlike `usePrReviewKeymap` (which relies on its host, the PR Diff tab, to + * focus-guard before ever calling the resolver), this resolver guards + * `isEditableTarget` itself (decision D6, v3.7.0 plan): the commit summary + * input and description textarea live in the *same* view as the findings + * list, so a bare `n`/`p`/`x` must be inert while the user is typing there + * — the host cannot assume the keydown never originates from an editable + * field the way the PR Diff tab's host can. + */ +import { isEditableTarget } from "../utils/editableTarget"; + +export type CommitReviewAction = "next-finding" | "prev-finding" | "dismiss-finding" | "open-findings" | "help"; + +/** + * Resolve a keydown into a Commit Review action, or `null` when: the + * Changes view/findings aren't active (`ctx.active`), meta/ctrl/alt is + * held, or the event target is an editable element. Never maps `Escape` or + * `⌘⇧L` — those stay owned by `App.vue`'s global handlers. + * + * Note on Shift: only meta/ctrl/alt are rejected up front. Shift is checked + * per-key below instead, because "?" requires Shift on every standard + * keyboard layout (it is Shift+/) — a resolver that bails on ANY held + * modifier including shiftKey would make the "?" case unreachable in + * practice. `usePrReviewKeymap.ts` has the same shape for the same reason + * (it maps J/K/V, whose Shift variants are distinct actions). + */ +export function resolveCommitReviewShortcut( + e: KeyboardEvent, + ctx: { active: boolean }, +): CommitReviewAction | null { + if (!ctx.active) return null; + if (e.metaKey || e.ctrlKey || e.altKey) return null; + if (isEditableTarget(e.target)) return null; + + switch (e.key) { + case "n": return e.shiftKey ? null : "next-finding"; + case "p": return e.shiftKey ? null : "prev-finding"; + case "x": return e.shiftKey ? null : "dismiss-finding"; + case "?": return "help"; + default: return null; + } +} diff --git a/apps/desktop/src/composables/useCommitReview.ts b/apps/desktop/src/composables/useCommitReview.ts new file mode 100644 index 00000000..0083c35b --- /dev/null +++ b/apps/desktop/src/composables/useCommitReview.ts @@ -0,0 +1,258 @@ +/** + * useCommitReview.ts + * + * Task 1a (v3.7.0) — staged-diff AI review orchestrator ("Commit Review", + * roadmap bullet 1). Opt-in, off by default: with `commitReviewEnabled` + * false this composable performs zero IPC and zero LLM calls. + * + * Reuses the exact same engine as the v3.6.0 PR pre-review pass + * (`usePrPreReview.analyzeFile`, `scope: "commit"` — Task 0), the same + * sequential/abortable/visibility-gated queue (`usePrReviewQueue`), and the + * same confidence-threshold + top-N cap + dismissal filter + * (`usePrFindingFilter`) — see AGENTS.md/plan decision D3: no new + * commit-specific threshold/cap settings, the existing `reviewAi*` knobs + * are reused. + * + * The staged diff is fetched via the existing `gitExec(["diff", "--cached", + * "--no-color"])` primitive (precedent: `useCommitMessage.ts`) — no new + * Tauri command. + */ +import { ref, shallowRef, computed, type ComputedRef, type Ref } from "vue"; +import { gitExec } from "../utils/backend"; +import { indexDiffFiles, parseFileDiff } from "../utils/unifiedDiff"; +import { usePrPreReview, type ReviewFinding } from "./usePrPreReview"; +import { usePrReviewQueue } from "./usePrReviewQueue"; +import { filterFindings, normalizeFindingClass } from "./usePrFindingFilter"; +import { useSettings } from "./useSettings"; +import { useAIProvider } from "./useAIProvider"; +import { useI18n } from "./useI18n"; + +/** + * Hard cap on the number of staged files sent through the review pass — a + * huge staged tree (e.g. a vendored dependency bump) must not fan out + * hundreds of LLM calls. Plan decision D12 flags this number as a guess + * worth validating against a real large staged change before merge. + */ +export const COMMIT_REVIEW_MAX_FILES = 40; + +/** + * Hard cap on the total staged-diff bytes sent through the review pass, + * applied in file-slice order (earlier files in the diff win). Same D12 + * caveat as `COMMIT_REVIEW_MAX_FILES`. + */ +export const COMMIT_REVIEW_MAX_BYTES = 400_000; + +export interface UseCommitReviewOptions { + /** + * Reserved for parity with `useSecretsScanner`'s `debounceMs` — that + * composable's `scan()` is debounced because it's driven by a staged-set + * *watcher*. `run()` here is invoked directly by an explicit user click + * (Task 1b's "Review staged changes" button); Task 3's one-shot + * re-review-after-fix is the only future watcher-driven call site, and + * that phase is out of scope for this PR. Not applied within this PR. + * Default 400. + */ + debounceMs?: number; +} + +export interface CommitReviewResult { + /** Filtered (threshold + cap + session-dismissed) — what the UI renders. */ + findings: Ref; + /** Raw findings from the last completed/in-flight run, unfiltered. */ + rawFindings: Ref; + running: ComputedRef; + progress: ComputedRef<{ done: number; total: number }>; + lastError: Ref; + /** Per-file finding count for the staged list (Task 2's sidebar chips). */ + findingsByFile: ComputedRef>; + /** Deterministic i18n one-liner composed from the findings (decision D2 — + * no second LLM call for the summary). */ + summary: ComputedRef; + /** True once the staged diff was truncated by the file-count or byte cap. */ + truncated: Ref; + /** + * Runs the review pass. Resolves to `true` once a full attempt actually + * happened (even if it errored, or the staged diff was empty) — `false` + * when the call never really tried: the feature is disabled, the AI + * provider is unavailable, `cwd` is empty, or this run was superseded by + * a newer one before it got anywhere. Callers use this to distinguish + * "ran with zero findings" from "did not run" (verifier issue #5). + */ + run: (cwd: string, locale: string) => Promise; + /** Abort any in-flight run and clear all state (repo switch, post-commit, + * or a staged-set change invalidating a stale review). */ + reset: () => void; + /** Session-only, class-normalized dismissal — mirrors the PR pre-review + * dismissal contract. */ + dismiss: (id: string) => void; + /** Wake the queue after `document.hidden` flips back. Call this from the + * host's existing `visibilitychange` handler — never add a second + * listener. Always resumes whichever run is currently active. */ + resume: () => void; +} + +export function useCommitReview(_opts: UseCommitReviewOptions = {}): CommitReviewResult { + const { settings } = useSettings(); + const { t } = useI18n(); + const ai = useAIProvider(); + const { analyzeFile } = usePrPreReview(); + + // Verifier issue #6: each run() gets its own `usePrReviewQueue()` instance + // rather than sharing one across every call. `usePrReviewQueue.run()` has + // a `try { ... } finally { running.value = false }` — if run A is aborted + // while its in-flight `analyzeOne` is still resolving, and run B has + // already started, A's stale `finally` can fire *after* B, incorrectly + // flipping shared running/done/total state back to A's. Giving every run + // its own queue instance means a stale run's cleanup only ever touches + // refs nothing else reads anymore — `activeQueue` always points at the + // most recent one, so a stale queue's `finally` becomes a no-op as far as + // the exposed `running`/`progress` are concerned. + // + // `shallowRef` (not `ref`): the queue object holds `done`/`total`/`running` + // as nested Refs. A plain `ref()` would make this container reactive, + // and Vue auto-unwraps nested refs read off a reactive object — turning + // `activeQueue.value.done` into an already-unwrapped number instead of + // the Ref itself, silently breaking every `.value` access below. + const activeQueue = shallowRef(usePrReviewQueue()); + + const rawFindings = ref([]); + const lastError = ref(null); + const dismissedClasses = ref>(new Set()); + const truncated = ref(false); + + let abortController: AbortController | null = null; + + const findings = computed(() => + filterFindings(rawFindings.value, { + threshold: settings.value.reviewAiConfidenceThreshold, + cap: settings.value.reviewAiMaxFindings, + dismissed: dismissedClasses.value, + }), + ); + + const progress = computed(() => ({ done: activeQueue.value.done.value, total: activeQueue.value.total.value })); + const running = computed(() => activeQueue.value.running.value); + + const findingsByFile = computed>(() => { + const counts: Record = {}; + for (const f of findings.value) counts[f.path] = (counts[f.path] ?? 0) + 1; + return counts; + }); + + const summary = computed(() => { + const list = findings.value; + if (list.length === 0) return t("commitReview.summaryClean"); + const risk = list.filter((f) => f.severity === "risk").length; + const suggestion = list.filter((f) => f.severity === "suggestion").length; + const nit = list.filter((f) => f.severity === "nit").length; + const files = new Set(list.map((f) => f.path)).size; + return t("commitReview.summaryCounts", risk, suggestion, nit, files); + }); + + function stop() { + abortController?.abort(); + abortController = null; + } + + function reset() { + stop(); + rawFindings.value = []; + lastError.value = null; + truncated.value = false; + } + + async function run(cwd: string, locale: string): Promise { + // A new run always supersedes whatever is in flight — abort first so a + // stale in-flight run can never paint findings after this one starts. + stop(); + rawFindings.value = []; + lastError.value = null; + truncated.value = false; + + // Opt-in feature: zero IPC, zero LLM call when disabled/unavailable. + // Never even attempted — callers must not treat this as "ran clean". + if (!settings.value.commitReviewEnabled || !ai.isAvailable.value || !cwd) return false; + + const controller = new AbortController(); + abortController = controller; + // Fresh queue instance for this run (verifier issue #6) — see the + // `activeQueue` declaration above for why. + const queue = usePrReviewQueue(); + activeQueue.value = queue; + + try { + const res = await gitExec(cwd, ["diff", "--cached", "--no-color"]); + if (controller.signal.aborted) return false; + + if (res.exitCode !== 0) { + lastError.value = (res.stderr ?? "").trim() || t("errors.commitReviewFailed"); + return true; // a full attempt happened, it just failed + } + // A clean index is not an error — no findings, no toast. + if (!res.stdout.trim()) return true; + + let slices = indexDiffFiles(res.stdout); + if (slices.length > COMMIT_REVIEW_MAX_FILES) { + slices = slices.slice(0, COMMIT_REVIEW_MAX_FILES); + truncated.value = true; + } + + let budget = COMMIT_REVIEW_MAX_BYTES; + const bounded: typeof slices = []; + for (const slice of slices) { + if (budget <= 0) { + truncated.value = true; + break; + } + bounded.push(slice); + budget -= slice.raw.length; + } + + const files = bounded.map((s) => parseFileDiff(s.raw)).filter((f) => f.hunks.length > 0); + if (!files.length) return true; + + await queue.run( + files, + (file) => analyzeFile(file, { cwd, locale, otherDiffFiles: files, scope: "commit" }), + { + onFinding: (finding) => { + if (controller.signal.aborted) return; + rawFindings.value = [...rawFindings.value, finding]; + }, + signal: controller.signal, + }, + ); + return !controller.signal.aborted; + } catch (err) { + if (controller.signal.aborted) return false; + lastError.value = err instanceof Error ? err.message : String(err); + return true; + } finally { + if (abortController === controller) abortController = null; + } + } + + function dismiss(id: string) { + const finding = rawFindings.value.find((f) => f.id === id); + if (!finding) return; + const cls = normalizeFindingClass(finding); + dismissedClasses.value = new Set([...dismissedClasses.value, cls]); + } + + return { + findings, + rawFindings, + running, + progress, + lastError, + findingsByFile, + summary, + truncated, + run, + reset, + dismiss, + // Always resumes whichever queue instance is currently active — see + // `activeQueue` above. + resume: () => activeQueue.value.resume(), + }; +} diff --git a/apps/desktop/src/composables/useCommitReviewNav.ts b/apps/desktop/src/composables/useCommitReviewNav.ts new file mode 100644 index 00000000..9780c4bb --- /dev/null +++ b/apps/desktop/src/composables/useCommitReviewNav.ts @@ -0,0 +1,110 @@ +/** + * useCommitReviewNav.ts + * + * Task 2 (v3.7.0) — finding-to-finding cycling across staged files. Port of + * `usePrReviewNav.ts`'s `jumpToFinding` (`N`/`P` cycling the PR's + * confidence-sorted findings, wrapping, switching file, scrolling into + * view), adapted so the "file" being switched is a staged file in the + * Changes view rather than a PR's diff sidebar — the host injects + * `selectFile` instead of this composable owning a `selectedDiffFile` ref. + */ +import { ref, computed, nextTick, type ComputedRef, type Ref } from "vue"; +import type { ReviewFinding } from "./usePrPreReview"; +import type { CommitReviewAction } from "./commitReviewKeymap"; + +/** Lower rank = more severe, matching `CommitReviewModal.vue`'s sort. */ +const SEVERITY_RANK: Record = { risk: 0, suggestion: 1, nit: 2 }; + +/** Severity-sorted (risk > suggestion > nit), then confidence descending — + * the single source of truth for "finding order" shared by the modal's + * list and this composable's `N`/`P` cycling, so what you see in the + * modal is exactly what `N`/`P` steps through. */ +export function sortFindingsForReview(findings: ReviewFinding[]): ReviewFinding[] { + return [...findings].sort((a, b) => { + const rankDiff = SEVERITY_RANK[a.severity] - SEVERITY_RANK[b.severity]; + return rankDiff !== 0 ? rankDiff : b.confidence - a.confidence; + }); +} + +/** Minimal duck-typed handle for the mounted `DiffViewer` instance — just + * enough to scroll to a finding, without this composable depending on the + * component's full instance type. */ +export interface CommitReviewDiffHandle { + scrollToFinding(line: number, side: "LEFT" | "RIGHT"): void; +} + +export interface UseCommitReviewNavOptions { + /** The (unsorted) active findings — this composable sorts them itself so + * the cycling order always matches the modal's severity/confidence order. */ + findings: Ref; + /** Switch the selected file in the Changes view. Findings are index-scoped, + * so `staged` is always `true` here. */ + selectFile: (path: string, staged: boolean) => void; + /** Ref to the mounted `DiffViewer` instance (or null before mount / when a + * non-diff view is showing). */ + diffHandle: Ref; + /** Dismiss the finding at the cursor (session-only, class-normalized — + * same contract as `useCommitReview.dismiss`). */ + onDismiss: (id: string) => void; + onHelp: () => void; +} + +export interface UseCommitReviewNavResult { + currentFindingIdx: Ref; + current: ComputedRef; + jumpToFinding: (delta: 1 | -1) => void; + dismissCurrent: () => void; + dispatch: (action: CommitReviewAction) => void; +} + +export function useCommitReviewNav(opts: UseCommitReviewNavOptions): UseCommitReviewNavResult { + /** -1 means "none yet" — the first `N` press lands on index 0 (or the + * last index on the first `P`). */ + const currentFindingIdx = ref(-1); + + const sortedFindings = computed(() => sortFindingsForReview(opts.findings.value)); + + const current = computed(() => sortedFindings.value[currentFindingIdx.value] ?? null); + + /** `N`/`P` — cycle through the severity/confidence-sorted findings list, + * wrapping around, switching file and scrolling to the finding's line. */ + function jumpToFinding(delta: 1 | -1) { + const list = sortedFindings.value; + if (!list.length) return; + const base = currentFindingIdx.value === -1 ? (delta > 0 ? -1 : 0) : currentFindingIdx.value; + const idx = ((base + delta) % list.length + list.length) % list.length; + currentFindingIdx.value = idx; + const target = list[idx]; + opts.selectFile(target.path, true); + void nextTick(() => { + opts.diffHandle.value?.scrollToFinding(target.line, target.side); + }); + } + + /** `X` — dismiss the finding at the cursor. The list shrinks by one right + * after (session-only dismissal filters it out), so the cursor is + * clamped against the *post*-dismiss list — it must never land out of + * range (regression guard: an off-by-one here would point past the end + * of a shrunk list). */ + function dismissCurrent() { + const before = sortedFindings.value; + if (!before.length || currentFindingIdx.value === -1) return; + const idxBefore = currentFindingIdx.value; + const target = before[idxBefore]; + opts.onDismiss(target.id); + const after = sortedFindings.value; + currentFindingIdx.value = after.length === 0 ? -1 : Math.min(idxBefore, after.length - 1); + } + + function dispatch(action: CommitReviewAction) { + switch (action) { + case "next-finding": jumpToFinding(1); break; + case "prev-finding": jumpToFinding(-1); break; + case "dismiss-finding": dismissCurrent(); break; + case "help": opts.onHelp(); break; + case "open-findings": break; // Not wired in this phase — reserved. + } + } + + return { currentFindingIdx, current, jumpToFinding, dismissCurrent, dispatch }; +} diff --git a/apps/desktop/src/composables/usePrPanel.ts b/apps/desktop/src/composables/usePrPanel.ts index e39f4799..1b1d7681 100644 --- a/apps/desktop/src/composables/usePrPanel.ts +++ b/apps/desktop/src/composables/usePrPanel.ts @@ -16,8 +16,6 @@ import { type CIAnnotation, type RemoteInfo, type GitDiff, - type DiffHunk, - type DiffLine, type PrReviewComment, type CreatePrCommentParams, type PendingReviewComment, @@ -38,6 +36,14 @@ import { getPersistedDiffMode, type DiffMode } from "../utils/diffMode"; import { requireOnline } from "../utils/networkGuard"; import { t } from "./useI18n"; import { useReviewIntelligence } from "./useReviewIntelligence"; +import { indexDiffFiles, parseFileDiff, parseUnifiedDiff } from "../utils/unifiedDiff"; + +// Task 0 (v3.7.0) — the three pure diff-parsing helpers below now live in +// `utils/unifiedDiff.ts` (no Vue, no backend import) so a commit-review +// composable never needs to import this (very large) PR panel just to parse +// a diff. Re-exported here verbatim for existing callers +// (`useReviewIntelligence.ts`, `__tests__/usePrPanel-lazy-diff.test.ts`). +export { indexDiffFiles, parseFileDiff, parseUnifiedDiff }; export const PR_PANEL_KEY = Symbol("prPanel"); @@ -80,6 +86,17 @@ export interface PrPanelOptions { * checkout happens on disk but the UI keeps showing the previous branch. */ onRepoMutated?: () => void | Promise; + /** + * Called on the same `visibilitychange` → visible edge that already + * resumes the PR pre-review queue (`intel.resumePreReviewQueue()` below). + * `usePrPanel` is instantiated once, at app root, regardless of whether + * the PR view is open — reusing its single listener here is how Commit + * Review's `useCommitReview.resume()` (v3.7.0) gets called without + * standing up a second `visibilitychange` listener. Without this, a + * commit review started while the tab is hidden would stay paused on + * `document.hidden` forever: nothing else ever calls its `resume()`. + */ + onVisibilityResume?: () => void; } // ─── Parse unified diff (lazy per-file, A1) ──────────────────────────────── @@ -88,86 +105,8 @@ export interface PrPanelOptions { // headers (cheap), and `parseFileDiff` — the actual hunk/line parse — runs // only for the file currently selected (see `ensureFileParsed` below), cached // by path so re-selecting a file never re-parses it. Both are pure (no -// composable state) so they're testable in isolation. - -/** Split a raw unified diff into lightweight per-file slices (no hunk parse). */ -export function indexDiffFiles(rawDiff: string): { path: string; raw: string }[] { - const slices: { path: string; raw: string }[] = []; - if (!rawDiff.trim()) return slices; - const lines = rawDiff.split("\n"); - let currentPath: string | null = null; - let currentLines: string[] = []; - const flush = () => { - if (currentPath !== null) slices.push({ path: currentPath, raw: currentLines.join("\n") }); - }; - for (const line of lines) { - if (line.startsWith("diff --git ")) { - flush(); - const match = line.match(/diff --git a\/(.+) b\/(.+)/); - currentPath = match ? match[2] : "unknown"; - currentLines = [line]; - continue; - } - if (currentPath !== null) currentLines.push(line); - } - flush(); - return slices; -} - -/** Parse one file's raw `diff --git …` slice (as produced by `indexDiffFiles`) - * into hunks/lines. Diff-parsing gotcha (AGENTS.md): context lines are - * detected via `line.startsWith(' ')` — a bare empty string is also treated - * as a (whitespace-stripped) context line, never as a phantom add/delete. */ -export function parseFileDiff(rawFileSlice: string): GitDiff { - const file: GitDiff = { path: "unknown", hunks: [] }; - let currentHunk: DiffHunk | null = null; - let oldLine = 0, newLine = 0; - for (const line of rawFileSlice.split("\n")) { - if (line.startsWith("diff --git ")) { - const match = line.match(/diff --git a\/(.+) b\/(.+)/); - file.path = match ? match[2] : "unknown"; - currentHunk = null; - continue; - } - if (line.startsWith("index ") || line.startsWith("--- ") || line.startsWith("+++ ") || - line.startsWith("old mode ") || line.startsWith("new mode ") || line.startsWith("new file ") || - line.startsWith("deleted file ") || line.startsWith("similarity index ") || - line.startsWith("rename from ") || line.startsWith("rename to ") || line.startsWith("Binary files ")) continue; - const hunkMatch = line.match(/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)/); - if (hunkMatch) { - currentHunk = { - header: line, - oldStart: parseInt(hunkMatch[1], 10), - oldCount: parseInt(hunkMatch[2] ?? "1", 10), - newStart: parseInt(hunkMatch[3], 10), - newCount: parseInt(hunkMatch[4] ?? "1", 10), - lines: [], - }; - file.hunks.push(currentHunk); - oldLine = parseInt(hunkMatch[1], 10); - newLine = parseInt(hunkMatch[3], 10); - continue; - } - if (currentHunk) { - if (line.startsWith("+")) { - currentHunk.lines.push({ type: "add", content: line.substring(1), newLineNo: newLine++ }); - } else if (line.startsWith("-")) { - currentHunk.lines.push({ type: "delete", content: line.substring(1), oldLineNo: oldLine++ }); - } else if (line.startsWith(" ") || line === "") { - currentHunk.lines.push({ type: "context", content: line.startsWith(" ") ? line.substring(1) : line, oldLineNo: oldLine++, newLineNo: newLine++ }); - } - } - } - return file; -} - -/** Full eager parse — composed from `indexDiffFiles` + `parseFileDiff`. - * Not used on the hot path anymore (see `ensureFileParsed`); kept for - * regression-parity tests and any caller that genuinely wants everything - * parsed up front. */ -export function parseUnifiedDiff(rawDiff: string): GitDiff[] { - return indexDiffFiles(rawDiff).map((f) => parseFileDiff(f.raw)); -} +// composable state) — moved to `utils/unifiedDiff.ts` (Task 0, v3.7.0) and +// re-exported above. export function usePrPanel(cwd: Ref, opts: PrPanelOptions = {}) { @@ -1420,6 +1359,9 @@ export function usePrPanel(cwd: Ref, opts: PrPanelOptions = {}) { // C3 — resume the pre-review queue (reuses this listener rather than // adding a second `visibilitychange` handler). intel.resumePreReviewQueue(); + // v3.7.0 — same reuse for Commit Review's queue (a different queue + // instance entirely — see `PrPanelOptions.onVisibilityResume`). + opts.onVisibilityResume?.(); } } diff --git a/apps/desktop/src/composables/usePrPreReview.ts b/apps/desktop/src/composables/usePrPreReview.ts index 5d192fe3..ba4df3d5 100644 --- a/apps/desktop/src/composables/usePrPreReview.ts +++ b/apps/desktop/src/composables/usePrPreReview.ts @@ -35,6 +35,15 @@ export interface ReviewFinding { detail: string; } +/** + * Task 0 (v3.7.0) — which diff source `analyzeFile` is reviewing. Swaps only + * the prompt's framing sentence and dependency-signal sentence; severity + * scale, confidence rules, and JSON contract stay byte-identical across + * scopes — it's the same engine either way. Default `"pr"` — no behavior + * change for any existing PR caller. + */ +export type ReviewScope = "pr" | "commit"; + export interface AnalyzeFileOptions { cwd: string; /** UI locale — drives the response language. */ @@ -46,6 +55,8 @@ export interface AnalyzeFileOptions { * a bigger budget than the single-hunk critique since findings are * file-scoped, not hunk-scoped). */ maxFileChars?: number; + /** Which diff source is being reviewed (Task 0, v3.7.0). Default `"pr"`. */ + scope?: ReviewScope; } // ─── Hop 2: dependency (extraction + cross-reference, no graph) ─────────── @@ -120,14 +131,19 @@ export function summarizeBlameForFile(blame: BlameLine[], file: GitDiff): string // ─── Hop 4: findings (strict JSON, defensive parse) ─────────────────────── -function buildSystemPrompt(locale: string): string { +function buildSystemPrompt(locale: string, scope: ReviewScope = "pr"): string { const lang = localeToAiLanguage(locale); - return `You are a senior engineer doing a pre-review pass on one file of a pull request. + const framing = + scope === "commit" + ? "one file of a commit that is about to be created (the staged changes)" + : "one file of a pull request"; + const dependencyScope = scope === "commit" ? "other files staged in this same commit" : "other files in this same PR"; + return `You are a senior engineer doing a pre-review pass on ${framing}. You will receive: - The file path. - The file's touched hunks (unified-diff content). -- A dependency signal: how many other files in this same PR import this file. +- A dependency signal: how many ${dependencyScope} import this file. - A brief history signal: recent commit summaries that touched the lines now being changed. @@ -266,7 +282,7 @@ export function usePrPreReview() { const blame = await getGitBlame(opts.cwd, file.path).catch(() => [] as BlameLine[]); const blameSummaries = summarizeBlameForFile(blame, file); - const systemPrompt = buildSystemPrompt(opts.locale ?? "en"); + const systemPrompt = buildSystemPrompt(opts.locale ?? "en", opts.scope ?? "pr"); const userPrompt = buildUserPrompt(file, importedByCount, blameSummaries, opts.maxFileChars ?? 6000); const raw = await ai.rawPrompt(systemPrompt, userPrompt); if (!raw) return []; diff --git a/apps/desktop/src/composables/usePrReviewKeymap.ts b/apps/desktop/src/composables/usePrReviewKeymap.ts index bb0ad4b4..5affc7c0 100644 --- a/apps/desktop/src/composables/usePrReviewKeymap.ts +++ b/apps/desktop/src/composables/usePrReviewKeymap.ts @@ -8,6 +8,13 @@ * owns the listener and dispatches. */ +import { isEditableTarget } from "../utils/editableTarget"; + +// Task 0 (v3.7.0) — `isEditableTarget` now lives in `utils/editableTarget.ts` +// (no Vue, no PR-review coupling). Re-exported here verbatim — existing +// tests (`usePrReviewKeymap.test.ts`) import it from this module unmodified. +export { isEditableTarget }; + export type PrReviewAction = | "next-hunk" | "prev-hunk" @@ -22,21 +29,6 @@ export type PrReviewAction = | "prev-finding" | "submit-review"; -/** True when `el` is a text input, textarea, select, or contenteditable — - * the keymap must stay completely inert while the user is typing there. */ -export function isEditableTarget(el: EventTarget | null): boolean { - if (!(el instanceof HTMLElement)) return false; - const tag = el.tagName; - if (tag === "INPUT" || tag === "TEXTAREA" || tag === "SELECT") return true; - // `isContentEditable` computes inherited editability but jsdom doesn't - // fully implement it — check the attribute directly too so this guard is - // reliable in both the browser and the test environment. - if (el.isContentEditable) return true; - const attr = el.getAttribute("contenteditable"); - if (attr === "" || attr === "true") return true; - return false; -} - /** * Resolve a keydown into a PR-review action, or `null` when the key is * unmapped, the panel isn't focused, or an unrelated modifier is held. diff --git a/apps/desktop/src/composables/useSettings.ts b/apps/desktop/src/composables/useSettings.ts index 040821ed..66454b50 100644 --- a/apps/desktop/src/composables/useSettings.ts +++ b/apps/desktop/src/composables/useSettings.ts @@ -389,6 +389,22 @@ export interface AppSettings { * high-entropy detection pass. 0 disables it. Default: 4.0. */ secretsEntropyThreshold: number; + + // ── v3.7.0 Commit Review ────────────────────────────────── + /** + * Master switch for the local, opt-in AI review pass over the staged diff + * (Commit Review). Off by default — it spends tokens on every "Review + * staged changes" click. A repo's `.gitwandrc` `commitReview.enabled` can + * override this per-repo in either direction (task 6, out of scope here). + * Default: false. + */ + commitReviewEnabled: boolean; + /** + * After a "Fix with agent" handoff (task 3, out of scope here), arm exactly + * one automatic re-review on the next staging change. Only consulted once + * that phase ships. Default: true. + */ + commitReviewAutoReReview: boolean; } export type TerminalMode = "floating" | "fullscreen" | "bottom"; @@ -481,6 +497,8 @@ export const defaultAppSettings: AppSettings = { filesHideOnNav: true, secretsScannerEnabled: true, secretsEntropyThreshold: 4.0, + commitReviewEnabled: false, + commitReviewAutoReReview: true, }; const SETTINGS_KEY = "gitwand-settings"; diff --git a/apps/desktop/src/locales/en.ts b/apps/desktop/src/locales/en.ts index eee00e39..db8c5c09 100644 --- a/apps/desktop/src/locales/en.ts +++ b/apps/desktop/src/locales/en.ts @@ -1244,6 +1244,14 @@ const en = { summary: "PR summary", summaryHint: "Generate a what/why/affected-areas summary on the Info tab.", }, + commitReview: { + title: "Commit Review", + hint: "Local, opt-in AI pass over your staged changes, run from the commit area.", + enabled: "Review staged changes", + enabledHint: "Show a \"Review staged changes\" button in the commit area. Off by default, uses the confidence threshold and cap from Review AI above.", + autoReReview: "Auto re-review after fixing with an agent", + autoReReviewHint: "Automatically run one more review the next time you stage changes after handing findings to an agent.", + }, language: "Interface language", languageAuto: "Automatic (system)", commitMessageLang: "Commit message language", @@ -2120,6 +2128,7 @@ const en = { aiResponseInvalidJson: "The AI provider's response could not be interpreted (invalid JSON).", sameBranches: "Head and base branches are identical \u2014 no PR possible.", noChangesToStash: "No local changes to stash \u2014 modify files before generating a message.", + commitReviewFailed: "Commit review failed to read the staged diff.", }, // ─── Tags panel (v1.9) ────────────────────────────────── @@ -2466,6 +2475,31 @@ const en = { ignoreConfirmTitle: "Stop scanning this file for secrets?", ignoreConfirmMessage: "This will stop flagging ANY secret pattern in {0} — permanently, written to .gitwandrc. Continue?", }, + + // ─── Commit Review (v3.7.0) ───────────────────────────── + commitReview: { + reviewButton: "Review staged changes", + reviewButtonRunning: "Reviewing... {0}/{1}", + badgeTooltip: "{0} finding(s) in the staged changes", + modalTitle: "Commit review", + modalSubtitle: "{0} finding(s) in your staged changes", + empty: "No findings", + severityRisk: "Risk", + severitySuggestion: "Suggestion", + severityNit: "Nit", + confidence: "{0}% confidence", + jumpTo: "Jump to", + dismiss: "Dismiss", + close: "Close", + truncatedNotice: "Your staged changes were large — only part of them was reviewed.", + summaryClean: "No issues found in your staged changes.", + summaryCounts: "{0} risk(s), {1} suggestion(s), {2} nit(s) across {3} file(s).", + fileCountTooltip: "{0} review finding(s) in this file", + navNext: "Next finding", + navPrev: "Previous finding", + navHelp: "Commit review shortcuts", + navEmpty: "No findings to navigate", + }, } as const; // Widen literal string types to plain `string` for locale compatibility diff --git a/apps/desktop/src/locales/es.ts b/apps/desktop/src/locales/es.ts index 97f97d58..40b984b5 100644 --- a/apps/desktop/src/locales/es.ts +++ b/apps/desktop/src/locales/es.ts @@ -1219,6 +1219,14 @@ const es: Locale = { summary: "Resumen de PR", summaryHint: "Generar un resumen qué/por qué/áreas afectadas en la pestaña Info.", }, + commitReview: { + title: "Revisión de commit", + hint: "Pase de IA local y opt-in sobre tus cambios en stage, ejecutado desde el área de commit.", + enabled: "Revisar cambios en stage", + enabledHint: "Muestra un botón «Revisar cambios en stage» en el área de commit. Desactivado por defecto, reutiliza el umbral de confianza y el máximo de Review AI de arriba.", + autoReReview: "Nueva revisión automática tras corregir con un agente", + autoReReviewHint: "Ejecutar automáticamente una revisión más la próxima vez que hagas stage de cambios después de enviar los hallazgos a un agente.", + }, language: "Idioma de la interfaz", languageAuto: "Automático (sistema)", commitMessageLang: "Idioma de los mensajes de commit", @@ -2079,6 +2087,7 @@ const es: Locale = { aiResponseInvalidJson: "La respuesta del proveedor IA no pudo interpretarse (JSON inválido).", sameBranches: "La rama fuente y la rama destino son idénticas — ninguna PR posible.", noChangesToStash: "Sin cambios locales para el stash — modifica archivos antes de generar un mensaje.", + commitReviewFailed: "La revisión de commit no pudo leer los cambios en stage.", }, // ─── Tags panel (v1.9) tags: { @@ -2424,6 +2433,31 @@ const es: Locale = { ignoreConfirmTitle: "¿Dejar de escanear este archivo en busca de secretos?", ignoreConfirmMessage: "Esto dejará de marcar CUALQUIER patrón de secreto en {0} — de forma permanente, escrito en .gitwandrc. ¿Continuar?", }, + + // ─── Commit Review (v3.7.0) ───────────────────────────── + commitReview: { + reviewButton: "Revisar cambios en stage", + reviewButtonRunning: "Revisando... {0}/{1}", + badgeTooltip: "{0} hallazgo(s) en los cambios en stage", + modalTitle: "Revisión de commit", + modalSubtitle: "{0} hallazgo(s) en tus cambios en stage", + empty: "Sin hallazgos", + severityRisk: "Riesgo", + severitySuggestion: "Sugerencia", + severityNit: "Detalle", + confidence: "{0}% de confianza", + jumpTo: "Ir a", + dismiss: "Descartar", + close: "Cerrar", + truncatedNotice: "Tus cambios en stage eran extensos — solo se revisó una parte.", + summaryClean: "No se encontraron problemas en tus cambios en stage.", + summaryCounts: "{0} riesgo(s), {1} sugerencia(s), {2} detalle(s) en {3} archivo(s).", + fileCountTooltip: "{0} hallazgo(s) de revisión en este archivo", + navNext: "Siguiente hallazgo", + navPrev: "Hallazgo anterior", + navHelp: "Atajos de la revisión de commit", + navEmpty: "Sin hallazgos para recorrer", + }, }; export default es; diff --git a/apps/desktop/src/locales/fr.ts b/apps/desktop/src/locales/fr.ts index a16d76ee..990232a6 100644 --- a/apps/desktop/src/locales/fr.ts +++ b/apps/desktop/src/locales/fr.ts @@ -1228,6 +1228,14 @@ const fr: Locale = { summary: "Résumé de PR", summaryHint: "Générer un résumé quoi/pourquoi/zones affectées dans l'onglet Info.", }, + commitReview: { + title: "Revue de commit", + hint: "Passe IA locale et opt-in sur vos changements indexés, lancée depuis la zone de commit.", + enabled: "Passer en revue les changements indexés", + enabledHint: "Afficher un bouton « Passer en revue les changements indexés » dans la zone de commit. Désactivé par défaut, réutilise le seuil de confiance et le plafond de Review AI ci-dessus.", + autoReReview: "Nouvelle revue automatique après une correction par un agent", + autoReReviewHint: "Lancer automatiquement une revue supplémentaire la prochaine fois que vous indexez des changements après avoir transmis les constats à un agent.", + }, language: "Langue de l\u2019interface", languageAuto: "Automatique (syst\u00e8me)", commitMessageLang: "Langue des messages de commit", @@ -2088,6 +2096,7 @@ const fr: Locale = { aiResponseInvalidJson: "La r\u00e9ponse du provider IA n'a pas pu \u00eatre interpr\u00e9t\u00e9e (JSON invalide).", sameBranches: "La branche source et la branche cible sont identiques \u2014 aucune PR possible.", noChangesToStash: "Aucun changement local \u00e0 stasher \u2014 modifie des fichiers avant de g\u00e9n\u00e9rer un message.", + commitReviewFailed: "La revue de commit n'a pas pu lire les changements index\u00e9s.", }, // ─── Tags panel (v1.9) tags: { @@ -2434,6 +2443,31 @@ const fr: Locale = { ignoreConfirmTitle: "Arrêter d'analyser ce fichier pour des secrets ?", ignoreConfirmMessage: "Ceci arrêtera de signaler N'IMPORTE QUEL motif de secret dans {0} — de façon permanente, écrit dans .gitwandrc. Continuer ?", }, + + // ─── Commit Review (v3.7.0) ───────────────────────────── + commitReview: { + reviewButton: "Passer en revue les changements indexés", + reviewButtonRunning: "Revue en cours... {0}/{1}", + badgeTooltip: "{0} constat(s) dans les changements indexés", + modalTitle: "Revue de commit", + modalSubtitle: "{0} constat(s) dans vos changements indexés", + empty: "Aucun constat", + severityRisk: "Risque", + severitySuggestion: "Suggestion", + severityNit: "Détail", + confidence: "{0}% de confiance", + jumpTo: "Aller à", + dismiss: "Ignorer", + close: "Fermer", + truncatedNotice: "Vos changements indexés étaient volumineux — seule une partie a été passée en revue.", + summaryClean: "Aucun problème trouvé dans vos changements indexés.", + summaryCounts: "{0} risque(s), {1} suggestion(s), {2} détail(s) sur {3} fichier(s).", + fileCountTooltip: "{0} constat(s) de revue dans ce fichier", + navNext: "Constat suivant", + navPrev: "Constat précédent", + navHelp: "Raccourcis de la revue de commit", + navEmpty: "Aucun constat à parcourir", + }, }; export default fr; diff --git a/apps/desktop/src/locales/pt-BR.ts b/apps/desktop/src/locales/pt-BR.ts index 633b102e..0208bffe 100644 --- a/apps/desktop/src/locales/pt-BR.ts +++ b/apps/desktop/src/locales/pt-BR.ts @@ -1220,6 +1220,14 @@ const ptBR: Locale = { summary: "Resumo de PR", summaryHint: "Gerar um resumo o quê/por quê/áreas afetadas na aba Info.", }, + commitReview: { + title: "Revisão de commit", + hint: "Passagem de IA local e opt-in sobre suas mudanças em stage, executada na área de commit.", + enabled: "Revisar mudanças em stage", + enabledHint: "Mostra um botão \"Revisar mudanças em stage\" na área de commit. Desativado por padrão, reutiliza o limite de confiança e o máximo do Review AI acima.", + autoReReview: "Nova revisão automática após corrigir com um agente", + autoReReviewHint: "Executar automaticamente mais uma revisão na próxima vez que você colocar mudanças em stage após enviar os achados a um agente.", + }, language: "Idioma da interface", languageAuto: "Automático (sistema)", commitMessageLang: "Idioma das mensagens de commit", @@ -2079,6 +2087,7 @@ const ptBR: Locale = { aiResponseInvalidJson: "A resposta do provedor de IA não pôde ser interpretada (JSON inválido).", sameBranches: "A branch de origem e destino são idênticas — nenhuma PR possível.", noChangesToStash: "Sem alterações locais para stash — modifique arquivos antes de gerar uma mensagem.", + commitReviewFailed: "A revisão de commit não conseguiu ler as mudanças em stage.", }, // ─── Tags panel (v1.9) tags: { @@ -2424,6 +2433,31 @@ const ptBR: Locale = { ignoreConfirmTitle: "Parar de verificar segredos neste arquivo?", ignoreConfirmMessage: "Isso vai parar de sinalizar QUALQUER padrão de segredo em {0} — permanentemente, gravado em .gitwandrc. Continuar?", }, + + // ─── Commit Review (v3.7.0) ───────────────────────────── + commitReview: { + reviewButton: "Revisar mudanças em stage", + reviewButtonRunning: "Revisando... {0}/{1}", + badgeTooltip: "{0} achado(s) nas mudanças em stage", + modalTitle: "Revisão de commit", + modalSubtitle: "{0} achado(s) nas suas mudanças em stage", + empty: "Nenhum achado", + severityRisk: "Risco", + severitySuggestion: "Sugestão", + severityNit: "Detalhe", + confidence: "{0}% de confiança", + jumpTo: "Ir para", + dismiss: "Dispensar", + close: "Fechar", + truncatedNotice: "Suas mudanças em stage eram grandes — apenas parte delas foi revisada.", + summaryClean: "Nenhum problema encontrado nas suas mudanças em stage.", + summaryCounts: "{0} risco(s), {1} sugestão(ões), {2} detalhe(s) em {3} arquivo(s).", + fileCountTooltip: "{0} achado(s) de revisão neste arquivo", + navNext: "Próximo achado", + navPrev: "Achado anterior", + navHelp: "Atalhos da revisão de commit", + navEmpty: "Nenhum achado para navegar", + }, }; export default ptBR; diff --git a/apps/desktop/src/locales/zh-CN.ts b/apps/desktop/src/locales/zh-CN.ts index da9fbd5f..84af55e0 100644 --- a/apps/desktop/src/locales/zh-CN.ts +++ b/apps/desktop/src/locales/zh-CN.ts @@ -1278,6 +1278,14 @@ const zhCN: Locale = { summary: "PR 摘要", summaryHint: "在信息标签页生成内容/原因/受影响范围摘要。", }, + commitReview: { + title: "提交审查", + hint: "本地、可选启用的 AI 分析，针对暂存的更改，从提交区域运行。", + enabled: "审查暂存的更改", + enabledHint: "在提交区域显示“审查暂存的更改”按钮。默认关闭，复用上方 Review AI 的置信度阈值和上限。", + autoReReview: "使用代理修复后自动重新审查", + autoReReviewHint: "将发现项交给代理后，下次暂存更改时自动再运行一次审查。", + }, language: "界面语言", languageAuto: "自动（跟随系统）", commitMessageLang: "提交信息语言", @@ -2064,6 +2072,7 @@ const zhCN: Locale = { aiResponseInvalidJson: "无法解析 AI 服务商的响应（JSON 无效）。", sameBranches: "源分支与目标分支相同 — 无法创建 PR。", noChangesToStash: "没有可存储的本地更改 — 请先修改文件再生成消息。", + commitReviewFailed: "提交审查未能读取暂存的更改。", }, // ─── Tags panel (v1.9) tags: { @@ -2409,6 +2418,31 @@ const zhCN: Locale = { ignoreConfirmTitle: "停止扫描此文件中的密钥？", ignoreConfirmMessage: "这将永久停止标记 {0} 中的任何密钥规则——并写入 .gitwandrc。是否继续？", }, + + // ─── 提交审查 (v3.7.0) ───────────────────────────── + commitReview: { + reviewButton: "审查暂存的更改", + reviewButtonRunning: "正在审查... {0}/{1}", + badgeTooltip: "暂存的更改中有 {0} 项发现", + modalTitle: "提交审查", + modalSubtitle: "您暂存的更改中有 {0} 项发现", + empty: "没有发现项", + severityRisk: "风险", + severitySuggestion: "建议", + severityNit: "细节", + confidence: "置信度 {0}%", + jumpTo: "跳转到", + dismiss: "忽略", + close: "关闭", + truncatedNotice: "您暂存的更改较大，仅审查了其中一部分。", + summaryClean: "您暂存的更改中未发现问题。", + summaryCounts: "{3} 个文件中共有 {0} 个风险、{1} 个建议、{2} 个细节问题。", + fileCountTooltip: "此文件中有 {0} 项审查发现", + navNext: "下一个发现项", + navPrev: "上一个发现项", + navHelp: "提交审查快捷键", + navEmpty: "没有可导航的发现项", + }, }; export default zhCN; diff --git a/apps/desktop/src/utils/__tests__/unifiedDiff.test.ts b/apps/desktop/src/utils/__tests__/unifiedDiff.test.ts new file mode 100644 index 00000000..50f6ca95 --- /dev/null +++ b/apps/desktop/src/utils/__tests__/unifiedDiff.test.ts @@ -0,0 +1,69 @@ +/** + * Task 0 (v3.7.0) — `indexDiffFiles` / `parseFileDiff` / `parseUnifiedDiff` lifted + * out of `usePrPanel.ts` into a pure, Vue-free, backend-free module so a + * commit-review composable never has to import the PR panel to parse a diff. + * + * These three assertions are copied from `usePrPanel-lazy-diff.test.ts` — the + * blank-context-line one is the diff-parsing gotcha guard (AGENTS.md): an + * empty string is a context line, never a phantom add/delete. + */ +import { describe, it, expect } from "vitest"; +import { indexDiffFiles, parseFileDiff, parseUnifiedDiff } from "../unifiedDiff"; + +const THREE_FILE_DIFF = [ + "diff --git a/a.ts b/a.ts", + "index 111..222 100644", + "--- a/a.ts", + "+++ b/a.ts", + "@@ -1,2 +1,2 @@", + " context a", + "-old a", + "+new a", + "diff --git a/b.ts b/b.ts", + "index 333..444 100644", + "--- a/b.ts", + "+++ b/b.ts", + "@@ -1,3 +1,3 @@", + " context b", + "", + "-old b", + "+new b", + "diff --git a/c.ts b/c.ts", + "index 555..666 100644", + "--- a/c.ts", + "+++ b/c.ts", + "@@ -1,1 +1,1 @@", + "-old c", + "+new c", +].join("\n"); + +describe("indexDiffFiles", () => { + it("splits a raw multi-file diff into per-file slices with correct b/ paths", () => { + const slices = indexDiffFiles(THREE_FILE_DIFF); + expect(slices.map((s) => s.path)).toEqual(["a.ts", "b.ts", "c.ts"]); + for (const s of slices) { + expect(s.raw.startsWith(`diff --git a/${s.path} b/${s.path}`)).toBe(true); + } + }); + + it("returns [] for an empty diff", () => { + expect(indexDiffFiles("")).toEqual([]); + expect(indexDiffFiles(" ")).toEqual([]); + }); +}); + +describe("parseFileDiff", () => { + it("matches parseUnifiedDiff's per-file output (regression parity)", () => { + const expected = parseUnifiedDiff(THREE_FILE_DIFF); + const slices = indexDiffFiles(THREE_FILE_DIFF); + const actual = slices.map((s) => parseFileDiff(s.raw)); + expect(actual).toEqual(expected); + }); + + it("classifies an empty-string context line as context, not add/delete", () => { + const slice = indexDiffFiles(THREE_FILE_DIFF)[1]; // b.ts has a blank context line + const parsed = parseFileDiff(slice.raw); + const blank = parsed.hunks[0].lines.find((l) => l.content === "" && l.type !== undefined); + expect(blank?.type).toBe("context"); + }); +}); diff --git a/apps/desktop/src/utils/editableTarget.ts b/apps/desktop/src/utils/editableTarget.ts new file mode 100644 index 00000000..1a62928f --- /dev/null +++ b/apps/desktop/src/utils/editableTarget.ts @@ -0,0 +1,25 @@ +/** + * editableTarget.ts + * + * Task 0 (v3.7.0) — `isEditableTarget` lifted verbatim out of + * `usePrReviewKeymap.ts` (B1, v3.6.0) so any keymap resolver (PR review, + * commit review, …) can reuse the same "is the user typing here" guard + * without depending on the PR-review keymap module. Re-exported from + * `usePrReviewKeymap.ts` for back-compat — its existing tests import it + * from there unmodified. + */ + +/** True when `el` is a text input, textarea, select, or contenteditable — + * the keymap must stay completely inert while the user is typing there. */ +export function isEditableTarget(el: EventTarget | null): boolean { + if (!(el instanceof HTMLElement)) return false; + const tag = el.tagName; + if (tag === "INPUT" || tag === "TEXTAREA" || tag === "SELECT") return true; + // `isContentEditable` computes inherited editability but jsdom doesn't + // fully implement it — check the attribute directly too so this guard is + // reliable in both the browser and the test environment. + if (el.isContentEditable) return true; + const attr = el.getAttribute("contenteditable"); + if (attr === "" || attr === "true") return true; + return false; +} diff --git a/apps/desktop/src/utils/unifiedDiff.ts b/apps/desktop/src/utils/unifiedDiff.ts new file mode 100644 index 00000000..85acbf83 --- /dev/null +++ b/apps/desktop/src/utils/unifiedDiff.ts @@ -0,0 +1,94 @@ +/** + * unifiedDiff.ts + * + * Task 0 (v3.7.0) — pure unified-diff parsing helpers, lifted verbatim out of + * `usePrPanel.ts` (A1, v3.6.0) so any composable can index/parse a raw + * unified diff without importing the (very large) PR panel. No Vue, no + * backend import — safe to use from `useCommitReview.ts` and anywhere else + * that just needs `GitDiff`s out of a raw diff string. + * + * `usePrPanel.ts` re-exports these three functions for backward + * compatibility — existing callers (`useReviewIntelligence.ts`, + * `usePrPanel-lazy-diff.test.ts`) keep importing from `../usePrPanel` + * unmodified. + */ +import type { GitDiff, DiffHunk } from "./backend"; + +/** Split a raw unified diff into lightweight per-file slices (no hunk parse). */ +export function indexDiffFiles(rawDiff: string): { path: string; raw: string }[] { + const slices: { path: string; raw: string }[] = []; + if (!rawDiff.trim()) return slices; + const lines = rawDiff.split("\n"); + let currentPath: string | null = null; + let currentLines: string[] = []; + const flush = () => { + if (currentPath !== null) slices.push({ path: currentPath, raw: currentLines.join("\n") }); + }; + for (const line of lines) { + if (line.startsWith("diff --git ")) { + flush(); + const match = line.match(/diff --git a\/(.+) b\/(.+)/); + currentPath = match ? match[2] : "unknown"; + currentLines = [line]; + continue; + } + if (currentPath !== null) currentLines.push(line); + } + flush(); + return slices; +} + +/** Parse one file's raw `diff --git …` slice (as produced by `indexDiffFiles`) + * into hunks/lines. Diff-parsing gotcha (AGENTS.md): context lines are + * detected via `line.startsWith(' ')` — a bare empty string is also treated + * as a (whitespace-stripped) context line, never as a phantom add/delete. */ +export function parseFileDiff(rawFileSlice: string): GitDiff { + const file: GitDiff = { path: "unknown", hunks: [] }; + let currentHunk: DiffHunk | null = null; + let oldLine = 0, newLine = 0; + for (const line of rawFileSlice.split("\n")) { + if (line.startsWith("diff --git ")) { + const match = line.match(/diff --git a\/(.+) b\/(.+)/); + file.path = match ? match[2] : "unknown"; + currentHunk = null; + continue; + } + if (line.startsWith("index ") || line.startsWith("--- ") || line.startsWith("+++ ") || + line.startsWith("old mode ") || line.startsWith("new mode ") || line.startsWith("new file ") || + line.startsWith("deleted file ") || line.startsWith("similarity index ") || + line.startsWith("rename from ") || line.startsWith("rename to ") || line.startsWith("Binary files ")) continue; + const hunkMatch = line.match(/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)/); + if (hunkMatch) { + currentHunk = { + header: line, + oldStart: parseInt(hunkMatch[1], 10), + oldCount: parseInt(hunkMatch[2] ?? "1", 10), + newStart: parseInt(hunkMatch[3], 10), + newCount: parseInt(hunkMatch[4] ?? "1", 10), + lines: [], + }; + file.hunks.push(currentHunk); + oldLine = parseInt(hunkMatch[1], 10); + newLine = parseInt(hunkMatch[3], 10); + continue; + } + if (currentHunk) { + if (line.startsWith("+")) { + currentHunk.lines.push({ type: "add", content: line.substring(1), newLineNo: newLine++ }); + } else if (line.startsWith("-")) { + currentHunk.lines.push({ type: "delete", content: line.substring(1), oldLineNo: oldLine++ }); + } else if (line.startsWith(" ") || line === "") { + currentHunk.lines.push({ type: "context", content: line.startsWith(" ") ? line.substring(1) : line, oldLineNo: oldLine++, newLineNo: newLine++ }); + } + } + } + return file; +} + +/** Full eager parse — composed from `indexDiffFiles` + `parseFileDiff`. + * Not used on the hot path anymore (see `usePrPanel.ensureFileParsed`); kept + * for regression-parity tests and any caller that genuinely wants + * everything parsed up front. */ +export function parseUnifiedDiff(rawDiff: string): GitDiff[] { + return indexDiffFiles(rawDiff).map((f) => parseFileDiff(f.raw)); +} diff --git a/docs/superpowers/plans/2026-08-13-v3.7.0-commit-review.md b/docs/superpowers/plans/2026-08-13-v3.7.0-commit-review.md new file mode 100644 index 00000000..3c90e5bd --- /dev/null +++ b/docs/superpowers/plans/2026-08-13-v3.7.0-commit-review.md @@ -0,0 +1,788 @@ +# v3.7.0 — Commit Review: micro AI reviews in the Changes panel + +> **For agentic workers:** REQUIRED SUB-SKILL: use `superpowers:subagent-driven-development` +> (recommended) or `superpowers:executing-plans` to implement this plan task-by-task. +> Steps use checkbox (`- [ ]`) syntax for tracking. Work the tasks **in order** — later tasks +> depend on modules introduced earlier. Every task ends with a named conventional commit; the +> branch must stay green after each commit (`cd apps/desktop && pnpm test` + `pnpm -r run test`). + +**Branch:** `feat/v3.7-commit-review` (this worktree). Baseline verified clean: `packages/core` +1056 tests pass, `apps/desktop` 689 tests pass. + +**Roadmap item:** `ROADMAP.md` → "v3.7.0 — Commit Review: micro AI reviews in the Changes panel" +(6 bullets). This plan implements those 6 bullets as tasks 1–6 plus one foundation task (task 0). +There is no separate spec file; the roadmap bullets + this plan are the spec. Every judgment call +this plan had to make is listed in **§Open decisions** at the bottom — read that section before +starting, and do not silently re-decide any of it. + +--- + +## Goal + +Bring the v3.6.0 AI pre-review pipeline to commit time, in the Changes panel, fully local: +one button in the commit area runs an AI pass over the staged diff, findings are anchored +inline in `DiffViewer`, navigable finding-to-finding, pipeable to a CLI agent, tracked across +review→fix→review iterations, and recorded as a `GitWand-Review` commit trailer via an explicit +three-state (ran / vouched / skipped) non-blocking decision. Off by default, per-repo opt-in. + +## What already exists (verified by reading the code, not assumed) + +| Piece | Where | Reusable as-is? | +|---|---|---| +| Multi-hop AI review engine, `GitDiff` in → `ReviewFinding[]` out | `apps/desktop/src/composables/usePrPreReview.ts:255-280` (`analyzeFile`) | **Yes** — already forge-agnostic and PR-agnostic in its signature. Only its *prompt* says "pull request" (`:125`). | +| `ReviewFinding` type (id/path/line/side/severity/confidence/title/detail) | `usePrPreReview.ts:25-36` | Yes, unchanged | +| Defensive JSON-array parse | `usePrPreReview.ts:211` (`parseFindings`) | Yes, unchanged | +| Sequential, abortable, visibility-gated queue | `usePrReviewQueue.ts:23-80` | Yes, unchanged | +| Confidence threshold + top-N cap + dismissal-class filter | `usePrFindingFilter.ts:18-42` | Yes, unchanged | +| Line-anchored annotation model + grouping + worst-severity | `prAnnotations.ts:19-124` (`fromFinding`, `annotationsByLine`, `worstSeverity`) | Yes, unchanged | +| Pure keymap resolver pattern + `isEditableTarget` | `usePrReviewKeymap.ts:27-77` | `isEditableTarget` reused (task 0 extracts it); the action union is PR-specific → new resolver | +| Finding-cursor nav pattern (`N`/`P`, wrap, cross-file) | `usePrReviewNav.ts:95-116` (`jumpToFinding`) | Pattern reused, new composable (different host/rows) | +| Pure unified-diff index + per-file hunk parse | `usePrPanel.ts:94` (`indexDiffFiles`), `:121` (`parseFileDiff`) | Yes — task 0 lifts them into a pure util so a commit-review composable never imports the PR panel | +| Staged diff fetch, no new command needed | `backend.ts:2227` `gitExec(cwd, ["diff","--cached","--no-color"])` — precedent: `useCommitMessage.ts:146-149` | Yes | +| Trailer assembly + append | `RepoSidebar.vue:551-560` (`buildTrailers`) → `emit("commit", trailers)` (`:786`, `:1696`) → `useGitRepo.ts:910-921` appends the block after a blank line | Yes — the `GitWand-Review` trailer needs **zero** Rust/IPC change | +| Non-blocking commit-area badge UX contract | `RepoSidebar.vue:1659-1671` (badge) + `App.vue:1026-1038` (`handleCommitRequest` confirm gate) + `SecretsFindingsModal.vue` | Pattern mirrored exactly | +| Agent PTY launch (`claude`/`codex`/`opencode`/`antigravity`) | `useTerminalSessions.ts:82-148` (`openTab`, `type` → first-class agent) + `App.vue:1551-1597` (`openTerminalTab`) + `terminalWrite` via `sessions.write` | Yes | +| AI-task scratch worktree flow | `App.vue:1681-1715` (`onNewAiTask`/`confirmNewAiTask`) + `useScratchWorktree.ts` + `useAiTasks.ts` | Yes | +| `.gitwandrc` schema + parser + read/write IPC | `packages/core/src/config.ts:219-324` / `:330-487`; `backend.ts:1479` `readGitwandrc`, `:1509` `writeGitwandrc` | Extended in task 6 (new `commitReview` block, same shape as `secrets`) | +| Pre-commit hook install/remove | `HooksPanel.vue:137-188` + `utils/secretsHook.ts` + `backend.ts:2576` `gitHookCreate` | **Collision** — see task 6; the secrets hook owns the whole `pre-commit` file today | + +**Consequence: no new `#[tauri::command]` is required for tasks 0–5.** Verified: staged diff via +`gitExec`, blame via `getGitBlame` (already used by `analyzeFile`), agent launch via `terminalOpen`, +trailers via the existing commit path, `.gitwandrc` via existing read/write, hooks via existing +`gitHookCreate`/`gitHookDelete`/`readFile`. If a task turns out to need one anyway, it also needs a +`dev-server.mjs` route + a `backend.ts` wrapper in the same commit (AGENTS.md), and parity coverage +if deterministic — stop and re-plan rather than sneaking an `invoke()` in. + +## Architecture at a glance + +``` + ┌─ utils/unifiedDiff.ts (pure: indexDiffFiles / parseFileDiff) [task 0] + │ +useCommitReview.ts ───┼─ usePrPreReview.analyzeFile (scope: "commit") [task 0] + (orchestrator) ├─ usePrReviewQueue.run (abort on staged-set change) + ├─ usePrFindingFilter.filterFindings + └─ commitReviewState.ts (pure: coverage, iterations, trailer) [task 4] + +RepoSidebar.vue → "Review staged changes" button + findings badge + per-file counts [tasks 1,2] +DiffViewer.vue → inline finding rows (severity badge + title + detail) [task 1] +CommitReviewModal.vue → summary + finding list + "Fix with agent" + iter/coverage [tasks 1,3,4] +CommitReviewDecisionModal.vue → Review / Vouch / Skip at commit time [task 5] +App.vue → wiring, keydown host, agent handoff, commit interception +``` + +## Global constraints (enforce in every task) + +- **i18n** — every user-visible string gets a key in **all 5** locales: + `apps/desktop/src/locales/en.ts`, `fr.ts`, `es.ts`, `pt-BR.ts`, `zh-CN.ts` (exact filenames — + note `pt-BR`/`zh-CN` are capitalized). `Locale = Widen` (`locales/en.ts` tail) means + **`vue-tsc` fails the build if any locale is missing a key** — so keys land in all 5 files in the + same commit as the UI that uses them. New top-level blocks introduced by this plan: + `commitReview.*`, `settings.commitReview.*`, `hooks.review*`. Never hardcode user-facing text. +- **Settings sync** — any new field goes in **both** `useSettings.ts` (`AppSettings` interface + + `defaultAppSettings`) **and** `SettingsPanel.vue` (local `Settings` interface + its default + object, `:160-163`/`:253-256` show the `reviewAi*` precedent) in the same commit. +- **Versions** — never hand-edit `package.json` / `Cargo.toml` / `tauri.conf.json` versions. + `./scripts/bump-version.sh 3.7.0` is a tag-time action, out of scope for these PRs. +- **`packages/core` stays zero-Node and browser-safe.** The only core change in this plan is the + `.gitwandrc` `commitReview` block in `config.ts` (pure JSON validation, task 6). +- **IPC** — never `invoke()` outside `utils/backend*.ts`. +- **No unconditional interval / no unconditional work.** The whole feature is opt-in and must do + literally zero work (no IPC, no LLM call, no watcher body) when `commitReviewEnabled` is false. + Follow the staged-set watch pattern at `App.vue:1095-1105` — never a `setInterval` + (`apps/desktop/CLAUDE.md` P6.4). +- **No deep watchers** on `repoFiles`/`repoStatus`/`repoDiff`. Watch specific fields + (`repoStats.staged`, `repoFolderPath`) like the secrets scanner does. +- **Sanitize** — findings come from an LLM. Render them as **plain text interpolation + (`{{ }}`)**, never `v-html`. If a future step needs markdown, it must go through + `useSafeHtml.ts` (DOMPurify). +- **Buttons are rounded squares** — `--radius-sm|md|lg`, never a pill (`apps/desktop/src/CLAUDE.md`). +- **`BaseModal` CSS** — never prefix `.bm-btn` with an ancestor selector (AGENTS.md specificity rule). +- **Diff-parsing gotcha** — this plan deliberately adds **no new diff parser**. Context lines are + detected by a leading space; `parseFileDiff` (`usePrPanel.ts:151-159`) already does this + correctly (`line.startsWith(" ") || line === ""`). Task 0 moves that function verbatim. + If you ever find yourself writing `!line.startsWith("\\")`, stop — that is the documented bug. +- **Lazy-load** every new modal via `defineAsyncComponent` in `App.vue` (pattern: `App.vue:72` + `SecretsFindingsModal`). + +## Test conventions + +- Composable/pure-module tests: `apps/desktop/src/composables/__tests__/` and + `apps/desktop/src/utils/__tests__/`, Vitest + jsdom. **Composable tests mock + `../../utils/backend` and `../useAIProvider`** — the established convention, see + `usePrPreReview.test.ts:11-20`. AGENTS.md's "real git repos, never a mocked git layer" rule + targets Rust/integration tests; it does not require spinning a real repo for a jsdom composable + test that mocks the IPC boundary. (Task 6's `.gitwandrc` parser test is pure — no repo needed.) +- Pure functions (coverage math, trailer builder, keymap resolver, hook script builder) get + hand-built inputs and **no mocks**. +- Component tests only where the render logic is the thing under test (pattern: + `components/__tests__/SecretsFindingsModal.test.ts`, `PrInlineDiff.test.ts`). +- **No Rust/parity work in this plan.** `pnpm test:parity` is unaffected (no command added). If + that changes, run it. +- Every fixed bug gets a regression test in the same commit. + +--- + +# PHASE 0 — Foundation + +## Task 0: Make the review engine diff-source-agnostic + +**Roadmap bullet 1, first half:** "Generalize `usePrHunkCritique` from PR hunks to any `GitDiff` +— the same engine as the v3.5.0 pre-review pass, pointed at the index." + +**Reading note that changes the shape of this task:** the roadmap's phrasing is slightly stale. +`usePrHunkCritique.ts` is the *older*, per-hunk, verdict+prose critique used only by +`PrInlineDiff.vue:20`; the engine the roadmap actually wants ("the same engine as the pre-review +pass") is `usePrPreReview.analyzeFile`, and that one **already takes a plain `GitDiff`** — nothing +PR-shaped in its signature (`usePrPreReview.ts:38-49, 262-277`). So "generalize" here means: +(a) stop hardcoding "pull request" in its prompt, and (b) make its pure inputs importable without +dragging in the PR panel. Do **not** rewrite `usePrHunkCritique`, do **not** rename +`usePrPreReview` (a rename churns 8 test files and `useReviewIntelligence` for zero behavioral +gain — noted as decision D1). + +**Files:** +- Create `apps/desktop/src/utils/unifiedDiff.ts` — move `indexDiffFiles` (`usePrPanel.ts:94-115`) + and `parseFileDiff` (`usePrPanel.ts:121-162`) **verbatim** (including the leading-space context + branch), plus `parseUnifiedDiff` (`:168-170`). Pure module, no Vue, no backend import. +- Modify `apps/desktop/src/composables/usePrPanel.ts` — delete the moved bodies, `import` them + from `../utils/unifiedDiff` and **re-export** (`export { indexDiffFiles, parseFileDiff, + parseUnifiedDiff }`) so `useReviewIntelligence.ts:25` and + `__tests__/usePrPanel-lazy-diff.test.ts:21` keep working untouched. +- Create `apps/desktop/src/utils/editableTarget.ts` — move `isEditableTarget` + (`usePrReviewKeymap.ts:27-38`) verbatim; re-export from `usePrReviewKeymap.ts` for back-compat + (its tests import it from there). +- Modify `apps/desktop/src/composables/usePrPreReview.ts`: + - `buildSystemPrompt(locale)` → `buildSystemPrompt(locale, scope: ReviewScope = "pr")` where + `export type ReviewScope = "pr" | "commit"`. Swap only the framing sentence and the + dependency-signal sentence: `"one file of a pull request"` → `"one file of a commit that is + about to be created (the staged changes)"`, and `"other files in this same PR"` → `"other + files staged in this same commit"`. Severity scale, confidence rules, JSON contract, and the + "never invent symbols" rule stay **byte-identical** — the whole point is one engine. + - `AnalyzeFileOptions` gains `scope?: ReviewScope` (default `"pr"`), threaded into + `buildSystemPrompt` at `:269`. +- No behavior change for any existing PR caller (default `"pr"`). + +**Interfaces produced:** +- `utils/unifiedDiff.ts`: `indexDiffFiles(raw): {path, raw}[]`, `parseFileDiff(slice): GitDiff`. +- `usePrPreReview`: `analyzeFile(file, { cwd, locale, otherDiffFiles, scope: "commit" })`. + +- [ ] **Step 1 (test-first):** Create `apps/desktop/src/utils/__tests__/unifiedDiff.test.ts` — + copy the three existing assertions from `__tests__/usePrPanel-lazy-diff.test.ts:50-80` + (3-file split, empty input, blank-context-line classification) against the new module path. + The blank-context assertion is the gotcha guard — it must fail if someone "simplifies" the + context branch. Red (module doesn't exist) → then move the code → green. +- [ ] **Step 2:** Do the move + re-export. Run `cd apps/desktop && pnpm test` — the *existing* + `usePrPanel-lazy-diff.test.ts` must still pass unmodified. That is the regression proof. +- [ ] **Step 3:** Move `isEditableTarget` + re-export; `usePrReviewKeymap.test.ts` must pass + unmodified. +- [ ] **Step 4 (test-first):** Extend `__tests__/usePrPreReview.test.ts` with two cases: + `scope` omitted → system prompt contains "pull request"; `scope: "commit"` → system prompt + contains the staged-commit framing and **not** "pull request". Assert against the captured + `rawPromptMock` first argument (the mock already exists at `:8`). Then implement. +- [ ] **Step 5:** `pnpm test` (desktop) + `pnpm -r run test` green. No i18n, no settings, no Rust. + +**Done when:** `analyzeFile` can be called with a staged-diff `GitDiff` and a commit-scoped +prompt; the two pure diff helpers are importable from `utils/` with no PR coupling; every +pre-existing test passes **unmodified**. + +**Commit:** `refactor(review): make the AI review engine diff-source-agnostic` + +--- + +# PHASE 1 — Review staged changes (roadmap bullet 1) + +## Task 1a: `useCommitReview` orchestrator + +**Files:** +- Create `apps/desktop/src/composables/useCommitReview.ts`: + ```ts + export interface UseCommitReviewOptions { debounceMs?: number } // default 400, mirrors useSecretsScanner + export interface CommitReviewResult { + findings: Ref; // filtered (threshold + cap + dismissed) + rawFindings: Ref; + running: Ref; + progress: ComputedRef<{ done: number; total: number }>; + lastError: Ref; + findingsByFile: ComputedRef>; // per-file count for the staged list (task 2) + summary: ComputedRef; // deterministic i18n one-liner (decision D2) + run: (cwd: string, locale: string) => Promise; + reset: () => void; // abort + clear (repo switch, post-commit) + dismiss: (id: string) => void; // session-only, class-normalized + } + ``` + Internals: + 1. `gitExec(cwd, ["diff","--cached","--no-color"])`; non-zero exit or empty stdout → `reset()` + and return (no findings, no error toast — a clean index is not an error). + 2. `indexDiffFiles(stdout)` → `parseFileDiff` per slice → drop files with `hunks.length === 0`. + Cap the file count at `COMMIT_REVIEW_MAX_FILES = 40` and the total staged-diff size at + ~400 KB (slice-order truncation), exposing `truncated: Ref` so the UI can say + "reviewed the first N files" — a 500-file staged tree must not fan out 500 LLM calls. + 3. `usePrReviewQueue().run(files, (f) => analyzeFile(f, { cwd, locale, otherDiffFiles: files, + scope: "commit" }), { onFinding, signal })` — one `AbortController` per run, stored at module + scope of the composable instance; a new `run()` or `reset()` aborts the previous one. + 4. `findings` = `filterFindings(rawFindings, { threshold: + settings.reviewAiConfidenceThreshold, cap: settings.reviewAiMaxFindings, dismissed })` — + reuse the *existing* review-AI settings rather than adding two near-duplicate fields + (decision D3). + 5. Guards: return immediately if `!settings.commitReviewEnabled`, `!ai.isAvailable.value`, or + `!cwd`. Never throw out of `run()` — set `lastError` (mirrors `useSecretsScanner:129-133`, + "a scan failure must never disrupt the commit flow"). + 6. Visibility: the queue already pauses on `document.hidden`; expose `resume()` and have + `App.vue` call it from an existing `visibilitychange` handler — **do not** add a second + listener (`usePrReviewQueue.ts:9-13`). +- Modify `apps/desktop/src/composables/useSettings.ts` — add to `AppSettings` + `defaultAppSettings`: + - `commitReviewEnabled: boolean` (default **false** — opt-in, it spends tokens). + - `commitReviewAutoReReview: boolean` (default **true**; only consulted after a "Fix with agent" + handoff, task 3). +- Modify `apps/desktop/src/components/SettingsPanel.vue` — same two fields in the local `Settings` + interface (`:160-163` neighborhood) + defaults (`:253-256`), and a UI block in the **AI tab** + right under the existing `settings.reviewAi` group (`:2701-2744`): a checkbox for + `commitReviewEnabled` + hint, and (shown only when enabled) the `commitReviewAutoReReview` + checkbox. Reuse `sp-group`/`sp-hint`/`updateSetting` exactly as the neighbors do. +- i18n (all 5 locales): `settings.commitReview.{title,hint,enabled,enabledHint,autoReReview, + autoReReviewHint}`, `commitReview.{summaryClean,summaryCounts,truncatedNotice}`, + `errors.commitReviewFailed`. + +- [ ] **Step 1 (test-first):** `apps/desktop/src/composables/__tests__/useCommitReview.test.ts`, + mocking `../../utils/backend` (`gitExec`, `getGitBlame`) and `../useAIProvider` + (`isAvailable`, `rawPrompt`) per `usePrPreReview.test.ts:11-20`. Cases: + 1. disabled setting → zero `gitExec` calls, zero `rawPrompt` calls, `findings` empty; + 2. AI unavailable → same; + 3. two staged files, model returns one finding each → 2 findings, `progress` reaches 2/2, + `findingsByFile` = `{a:1, b:1}`; + 4. `gitExec` exit code 1 → `findings` empty, `lastError` set, no throw; + 5. empty staged diff → no LLM call; + 6. a second `run()` while the first is in flight aborts the first (no stale findings painted); + 7. `reset()` clears findings and aborts; + 8. below-threshold finding is filtered out of `findings` but present in `rawFindings`; + 9. file-count cap: 45 staged files → at most 40 `rawPrompt` calls and `truncated === true`. +- [ ] **Step 2:** Implement `useCommitReview.ts` until green. +- [ ] **Step 3:** Add the two settings fields to **both** files + the AI-tab UI. Add the i18n keys + to all 5 locales; `pnpm build` (vue-tsc) is the check that none is missing. +- [ ] **Step 4:** Extend `__tests__/useSettings-reviewAi.test.ts` (or add + `useSettings-commitReview.test.ts`) asserting both defaults and that a persisted partial + settings object still hydrates them (the `{...defaultAppSettings, ...JSON.parse(raw)}` path, + `useSettings.ts:502`). + +**Done when:** with `commitReviewEnabled` on and a provider configured, `run()` produces filtered +findings over the staged diff; with it off, the composable performs no IPC and no LLM call. + +**Commit:** `feat(commit-review): add staged-diff AI review engine + opt-in setting` + +## Task 1b: Commit-area button + inline findings in the diff + +**Files:** +- Modify `apps/desktop/src/components/RepoSidebar.vue`: + - New props (next to `secretFindingsCount`, `:61`): `commitReviewEnabled?: boolean`, + `commitReviewRunning?: boolean`, `commitReviewFindingsCount?: number`, + `commitReviewProgress?: { done: number; total: number }`. + - New emits (next to `openSecrets`, `:95`): `reviewStaged: []`, `openCommitReview: []`. + - In `.commit-actions` (`:1658-1707`), **before** the commit button: a "Review staged changes" + button, shown only when `commitReviewEnabled && repoStats.staged > 0`, disabled while + `commitReviewRunning`, showing the existing `.commit-spinner` SVG + `done/total` while + running (copy the `commit-ai-btn` loading pattern at `:1466-1478`); and a findings badge + (same markup/skin as `.commit-secrets-badge`, `:1659-1671`, different icon/colour) shown when + `commitReviewFindingsCount > 0`, emitting `openCommitReview`. +- Modify `apps/desktop/src/components/DiffViewer.vue`: + - New optional prop `findings?: ReviewFinding[]` (default `[]`) — the findings for the file + currently displayed. Import the type from `../composables/usePrPreReview`. + - `const findingsByLine = computed(() => annotationsByLine(props.findings.map(fromFinding)))` + — reuse `prAnnotations.ts:62,81`; **do not** write a second grouping function. + - Inline mode only (`:604-633`): after each ``, when + `findingsByLine.get(\`${side}:${lineNo}\`)` is non-empty, render an extra + `` with a `colspan` cell containing, per finding: a severity + badge (`risk`/`suggestion`/`nit` → danger/warning/muted CSS vars), the confidence as + `{{ f.confidence }}%`, the `title`, the `detail`, and a "Dismiss" button emitting + `dismiss-finding`. Plain-text interpolation only — no `v-html`. + - Side-by-side mode (`:667-698`): a gutter/severity marker on the affected line only, no card + (decision D4 — full SBS finding cards are a follow-up). + - `defineExpose({ scrollToFinding })` — `scrollToFinding(line, side)` finds the rendered row + (`hunkEls`, `:177-191` for the coarse hunk scroll; then `querySelector` the + `[data-line-key]` attribute you add to each `tr`) and `scrollIntoView({block:"center"})`. + Needed by task 2; add it here so the DOM contract lands with the markup. + - New emit `dismiss-finding: [id: string]`. +- Create `apps/desktop/src/components/CommitReviewModal.vue` (`