From 561f07ff07d58f39e85eb71fa158ad37227056fb Mon Sep 17 00:00:00 2001 From: Laurent Guitton Date: Thu, 13 Aug 2026 15:56:07 +0200 Subject: [PATCH 01/10] refactor(review): make the AI review engine diff-source-agnostic Lift indexDiffFiles/parseFileDiff/parseUnifiedDiff out of usePrPanel.ts into a pure utils/unifiedDiff.ts (no Vue, no backend import), and isEditableTarget out of usePrReviewKeymap.ts into utils/editableTarget.ts, both re-exported for back-compat so every pre-existing test keeps passing unmodified. usePrPreReview's analyzeFile gains a scope: "pr" | "commit" option that swaps only the prompt's framing and dependency-signal sentences; severity scale, confidence rules and the JSON contract stay byte-identical. This lets the v3.7.0 commit-review feature reuse the same engine over the staged diff instead of a PR, with no behavior change for existing PR callers (default stays "pr"). --- .../__tests__/usePrPreReview.test.ts | 24 +++++ apps/desktop/src/composables/usePrPanel.ts | 92 ++---------------- .../desktop/src/composables/usePrPreReview.ts | 24 ++++- .../src/composables/usePrReviewKeymap.ts | 22 ++--- .../src/utils/__tests__/unifiedDiff.test.ts | 69 ++++++++++++++ apps/desktop/src/utils/editableTarget.ts | 25 +++++ apps/desktop/src/utils/unifiedDiff.ts | 94 +++++++++++++++++++ 7 files changed, 249 insertions(+), 101 deletions(-) create mode 100644 apps/desktop/src/utils/__tests__/unifiedDiff.test.ts create mode 100644 apps/desktop/src/utils/editableTarget.ts create mode 100644 apps/desktop/src/utils/unifiedDiff.ts 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/usePrPanel.ts b/apps/desktop/src/composables/usePrPanel.ts index e39f4799..d4961aa2 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"); @@ -88,86 +94,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 = {}) { 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/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)); +} From b691ec3465a9eeab512d481daa918345fb44f50b Mon Sep 17 00:00:00 2001 From: Laurent Guitton Date: Thu, 13 Aug 2026 16:21:59 +0200 Subject: [PATCH 02/10] feat(commit-review): add staged-diff AI review engine + opt-in setting Add useCommitReview.ts, a composable that runs the v3.6.0 pre-review engine (usePrPreReview.analyzeFile, scope: "commit") over the staged diff fetched via the existing gitExec(["diff", "--cached"]) primitive. No new Tauri command. Reuses the existing usePrReviewQueue for sequential/abortable/visibility-gated execution and the existing reviewAiConfidenceThreshold/reviewAiMaxFindings settings for the threshold and cap, rather than adding commit-specific twins. Caps the reviewed staged set at 40 files / ~400 KB (slice-order truncation) so a huge staged tree cannot fan out hundreds of LLM calls, exposing a truncated flag for the UI. Off by default: add commitReviewEnabled (opt-in) and commitReviewAutoReReview settings to both useSettings.ts and SettingsPanel.vue, with a new "Commit Review" group in the AI tab. Add commitReview.* and settings.commitReview.* i18n keys to all 5 locales. --- apps/desktop/src/components/SettingsPanel.vue | 36 +++ .../__tests__/useCommitReview.test.ts | 185 ++++++++++++++ .../useSettings-commitReview.test.ts | 38 +++ .../src/composables/useCommitReview.ts | 225 ++++++++++++++++++ apps/desktop/src/composables/useSettings.ts | 18 ++ apps/desktop/src/locales/en.ts | 34 +++ apps/desktop/src/locales/es.ts | 34 +++ apps/desktop/src/locales/fr.ts | 34 +++ apps/desktop/src/locales/pt-BR.ts | 34 +++ apps/desktop/src/locales/zh-CN.ts | 34 +++ 10 files changed, 672 insertions(+) create mode 100644 apps/desktop/src/composables/__tests__/useCommitReview.test.ts create mode 100644 apps/desktop/src/composables/__tests__/useSettings-commitReview.test.ts create mode 100644 apps/desktop/src/composables/useCommitReview.ts diff --git a/apps/desktop/src/components/SettingsPanel.vue b/apps/desktop/src/components/SettingsPanel.vue index 36ee5b56..cf2578a6 100644 --- a/apps/desktop/src/components/SettingsPanel.vue +++ b/apps/desktop/src/components/SettingsPanel.vue @@ -222,6 +222,9 @@ interface Settings { // v3.5.0 Secrets scanner secretsScannerEnabled: boolean; secretsEntropyThreshold: number; + // v3.7.0 Commit Review + commitReviewEnabled: boolean; + commitReviewAutoReReview: boolean; } const defaultSettings: Settings = { @@ -309,6 +312,8 @@ const defaultSettings: Settings = { filesHideOnNav: true, secretsScannerEnabled: true, secretsEntropyThreshold: 4.0, + commitReviewEnabled: false, + commitReviewAutoReReview: true, }; function loadSettings(): Settings { @@ -2745,6 +2750,37 @@ function deleteReleaseNoteTemplate(id: string) { + +
+
+
+
+ {{ t('settings.commitReview.title') }} + {{ t('settings.commitReview.hint') }} +
+
+ +
+ + {{ t('settings.commitReview.enabledHint') }} +
+ +
+ + {{ t('settings.commitReview.autoReReviewHint') }} +
+
+
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..b21156d4 --- /dev/null +++ b/apps/desktop/src/composables/__tests__/useCommitReview.test.ts @@ -0,0 +1,185 @@ +/** + * 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 } 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 } 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 }; +} + +describe("useCommitReview", () => { + beforeEach(() => { + rawPromptMock.mockReset(); + getGitBlameMock.mockReset().mockResolvedValue([]); + gitExecMock.mockReset(); + isAvailableRef.value = true; + const { settings } = useSettings(); + settings.value = { ...defaultAppSettings }; + }); + + it("performs zero IPC and zero LLM calls when the setting is disabled", async () => { + const { settings } = useSettings(); + settings.value = { ...defaultAppSettings, commitReviewEnabled: false }; + const review = useCommitReview(); + await review.run("/repo", "en"); + expect(gitExecMock).not.toHaveBeenCalled(); + expect(rawPromptMock).not.toHaveBeenCalled(); + expect(review.findings.value).toEqual([]); + }); + + it("performs zero IPC and zero LLM calls when the AI provider is unavailable", async () => { + enableCommitReview(); + isAvailableRef.value = false; + const review = useCommitReview(); + await review.run("/repo", "en"); + expect(gitExecMock).not.toHaveBeenCalled(); + expect(rawPromptMock).not.toHaveBeenCalled(); + expect(review.findings.value).toEqual([]); + }); + + 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(); + 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 }); + }); + + 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(); + await expect(review.run("/repo", "en")).resolves.toBeUndefined(); + expect(review.findings.value).toEqual([]); + expect(review.lastError.value).toBeTruthy(); + expect(rawPromptMock).not.toHaveBeenCalled(); + }); + + it("makes no LLM call on an empty staged diff (clean index is not an error)", async () => { + enableCommitReview(); + gitExecMock.mockResolvedValue(gitExecOk("")); + + const review = useCommitReview(); + await review.run("/repo", "en"); + + expect(rawPromptMock).not.toHaveBeenCalled(); + expect(review.findings.value).toEqual([]); + expect(review.lastError.value).toBeNull(); + }); + + 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 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"); + + expect(rawPromptMock.mock.calls.length).toBeLessThanOrEqual(COMMIT_REVIEW_MAX_FILES); + expect(review.truncated.value).toBe(true); + }); +}); 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/useCommitReview.ts b/apps/desktop/src/composables/useCommitReview.ts new file mode 100644 index 00000000..bcf10102 --- /dev/null +++ b/apps/desktop/src/composables/useCommitReview.ts @@ -0,0 +1,225 @@ +/** + * 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, 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: Ref; + 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; + 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. The queue itself + * owns the pause/resume state; call this from the host's existing + * `visibilitychange` handler — never add a second listener. */ + resume: () => void; +} + +export function useCommitReview(_opts: UseCommitReviewOptions = {}): CommitReviewResult { + const { settings } = useSettings(); + const { t } = useI18n(); + const ai = useAIProvider(); + const { analyzeFile } = usePrPreReview(); + const queue = 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: queue.done.value, total: queue.total.value })); + const running = queue.running; + + 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. + if (!settings.value.commitReviewEnabled || !ai.isAvailable.value || !cwd) return; + + const controller = new AbortController(); + abortController = controller; + + try { + const res = await gitExec(cwd, ["diff", "--cached", "--no-color"]); + if (controller.signal.aborted) return; + + if (res.exitCode !== 0) { + lastError.value = (res.stderr ?? "").trim() || t("errors.commitReviewFailed"); + return; + } + // A clean index is not an error — no findings, no toast. + if (!res.stdout.trim()) return; + + 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; + + 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, + }, + ); + } catch (err) { + if (!controller.signal.aborted) { + lastError.value = err instanceof Error ? err.message : String(err); + } + } 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, + resume: queue.resume, + }; +} 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..f64cce99 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: "Reviewed the first {0} file(s) of your staged changes.", + 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..4f5c6326 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: "Se revisaron los primeros {0} archivo(s) de tus cambios en stage.", + 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..672006a9 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: "Revue effectuée sur les {0} premier(s) fichier(s) de vos changements indexés.", + 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..50c50f35 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: "Revisados os primeiros {0} arquivo(s) das suas mudanças em stage.", + 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..a064eee8 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: "已审查您暂存更改中的前 {0} 个文件。", + summaryClean: "您暂存的更改中未发现问题。", + summaryCounts: "{3} 个文件中共有 {0} 个风险、{1} 个建议、{2} 个细节问题。", + fileCountTooltip: "此文件中有 {0} 项审查发现", + navNext: "下一个发现项", + navPrev: "上一个发现项", + navHelp: "提交审查快捷键", + navEmpty: "没有可导航的发现项", + }, }; export default zhCN; From 2903a58a62bdb5225659ea39b24f275da15d26ad Mon Sep 17 00:00:00 2001 From: Laurent Guitton Date: Thu, 13 Aug 2026 17:03:41 +0200 Subject: [PATCH 03/10] feat(commit-review): review staged changes from the commit area with inline findings Wire the Commit Review engine into the UI: - RepoSidebar.vue: a "Review staged changes" button (shown only when the opt-in setting is on and something is staged) with a spinner and done/total progress, plus a findings badge that opens the summary modal. Also adds a per-file finding count chip in both the flat-list and tree-layout file renderers (RepoSidebar has two independent renderers for the file list; both needed the chip). - DiffViewer.vue: a new optional `findings` prop. Inline mode renders one finding row per anchored line (severity badge, confidence, title, detail, Dismiss), reusing the existing prAnnotations grouping so LEFT/RIGHT findings on the same line number never merge and an orphan finding (line not in the diff) renders nothing instead of throwing. Side-by-side mode gets a gutter severity marker only, no card (decision D4 - full SBS cards are a follow-up). Findings render via plain-text interpolation only, never v-html. Exposes scrollToFinding(line, side) for Task 2's navigation. - CommitReviewModal.vue: new component (modelled on SecretsFindingsModal.vue) showing the deterministic summary line and a severity-then-confidence-sorted finding list with Jump to/Dismiss per finding and an empty state. - App.vue: instantiates useCommitReview, wires the button/badge/modal, passes per-file findings to DiffViewer (index-scoped, so never painted on an unstaged diff), and extends the existing staged-set watch to reset() the review (never auto-run) whenever the staged set changes. Add commitReview.* i18n keys (all 5 locales) for the new UI strings. --- apps/desktop/src/App.vue | 63 ++++- .../src/components/CommitReviewModal.vue | 186 +++++++++++++ apps/desktop/src/components/DiffViewer.vue | 263 +++++++++++++++--- apps/desktop/src/components/RepoSidebar.vue | 123 ++++++++ .../__tests__/CommitReviewModal.test.ts | 96 +++++++ .../__tests__/DiffViewer-findings.test.ts | 134 +++++++++ apps/desktop/src/locales/en.ts | 2 +- apps/desktop/src/locales/es.ts | 2 +- apps/desktop/src/locales/fr.ts | 2 +- apps/desktop/src/locales/pt-BR.ts | 2 +- apps/desktop/src/locales/zh-CN.ts | 2 +- 11 files changed, 825 insertions(+), 50 deletions(-) create mode 100644 apps/desktop/src/components/CommitReviewModal.vue create mode 100644 apps/desktop/src/components/__tests__/CommitReviewModal.test.ts create mode 100644 apps/desktop/src/components/__tests__/DiffViewer-findings.test.ts diff --git a/apps/desktop/src/App.vue b/apps/desktop/src/App.vue index f24546e6..cc7b0ebe 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,7 @@ 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 { useLaunchpadPrs } from "./composables/useLaunchpadPrs"; import { diffLaunchpad, isBotAuthor, type LaunchpadEvent } from "./composables/useLaunchpadNotifications"; import { osNotify } from "./composables/useOsNotification"; @@ -116,7 +118,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 +128,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 +273,17 @@ 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) + : [], +); + // Monorepo scope (v2.21.0) — restore persisted scope on repo open. const { loadScope } = useWorkspaceScope(); @@ -1016,6 +1033,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 +1087,20 @@ function onSecretsCommitAnyway() { void doCommit(lastAttemptedCommitTrailers); } +/** 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 +1124,8 @@ const repoSidebarListeners = { deleteBranch: (name: string, hasLocal: boolean, hasRemote: boolean, remoteName?: string) => handleDeleteBranchRequest(name, hasLocal, hasRemote, remoteName), openSecrets: () => { showSecretsModal.value = true; }, + reviewStaged: () => { void commitReview.run(repoFolderPath.value ?? "", locale.value); }, + openCommitReview: () => { showCommitReviewModal.value = true; }, }; // Trigger a (debounced) secrets scan whenever the staged set changes, or when a repo is @@ -1100,6 +1138,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 }, ); @@ -3349,10 +3393,12 @@ onUnmounted(() => { - + @select-dir-file="(path) => repoSelectFile(path, false)" + @dismiss-finding="(id) => commitReview.dismiss(id)" />
{ @close="showSecretsModal = false" /> + + +
+ + {{ reviewFindingsByFile![file.path] }}
+ + {{ reviewFindingsByFile![row.path] }}
+ + +
- + {{ reviewFindingsByFile![file.path] }} @@ -1377,9 +1380,11 @@ function formatActivityDate(dateStr: string): string {
+ this branch is easy to miss, hence the explicit callout. + Gated on section === 'staged' — see the flat-list branch + above for why. --> {{ reviewFindingsByFile![row.path] }} diff --git a/apps/desktop/src/components/SettingsPanel.vue b/apps/desktop/src/components/SettingsPanel.vue index cf2578a6..bde358fc 100644 --- a/apps/desktop/src/components/SettingsPanel.vue +++ b/apps/desktop/src/components/SettingsPanel.vue @@ -2770,15 +2770,12 @@ function deleteReleaseNoteTemplate(id: string) { {{ t('settings.commitReview.enabledHint') }}
-
- - {{ t('settings.commitReview.autoReReviewHint') }} -
+ From ebb9f178f11f37e53ad5af52197020fe502be203 Mon Sep 17 00:00:00 2001 From: Laurent Guitton Date: Thu, 13 Aug 2026 19:05:22 +0200 Subject: [PATCH 10/10] docs: add v3.7.0 commit review implementation plan Records the step-by-step plan behind the Commit Review feature (ROADMAP.md v3.7.0), including the decisions on 3-way PR splitting and warn-only pre-commit hook scope. --- .../plans/2026-08-13-v3.7.0-commit-review.md | 788 ++++++++++++++++++ 1 file changed, 788 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-13-v3.7.0-commit-review.md 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` (`