feat: Commit Review, staged-diff AI review in the Changes panel (v3.7.0 part 1) - #159
Closed
devlint wants to merge 10 commits into
Closed
feat: Commit Review, staged-diff AI review in the Changes panel (v3.7.0 part 1)#159devlint wants to merge 10 commits into
devlint wants to merge 10 commits into
Conversation
added 10 commits
August 13, 2026 15:56
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").
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.
…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.
Add commitReviewKeymap.ts, a pure resolver mapping n/p/x/? to next-finding/prev-finding/dismiss-finding/help. Unlike usePrReviewKeymap (which relies on its host to focus-guard), this resolver guards isEditableTarget itself: the commit summary input and description textarea live in the same view as the findings list, so bare letters must stay inert while typing there. Add useCommitReviewNav.ts, porting usePrReviewNav's jumpToFinding to switch the selected staged file via an injected selectFile callback and scroll the mounted DiffViewer to the finding's line/side. Findings are cycled in the same severity-then-confidence order the modal displays them in (sortFindingsForReview, now the single shared source of that order, extracted out of CommitReviewModal.vue). Dismissing the finding at the cursor clamps it back into range as the list shrinks. Wire both into App.vue's existing onKeyDown, early and before the mod-key ladder, active only in the Changes view with the feature on and at least one finding. "?" surfaces a one-line reminder via the existing toast affordance rather than a new help modal. The per-file finding count chips (RepoSidebar's flat-list and tree-layout file renderers) and the reviewFindingsByFile prop were implemented together with the Task 1b RepoSidebar wiring in the previous commit, since both touched the same file in the same edit pass; this commit is the rest of Task 2 (the keymap resolver, the nav composable, and their App.vue wiring).
resolveCommitReviewShortcut bailed on any held modifier, including
shiftKey, before ever reaching the "?" case. Since "?" requires Shift on
every standard keyboard layout, that branch was dead code and the help
toast could never fire.
Only meta/ctrl/alt are rejected up front now; shift is checked per key
instead, so n/p/x stay inert with Shift held (Shift+n etc. is noise) while
"?" (Shift+/) still resolves to help. Mirrors usePrReviewKeymap.ts, which
has the same shape for the same reason.
Fixed the test that asserted kd("?") (no shiftKey) resolved to help, which
was asserting a KeyboardEvent shape a real keyboard never produces, and
added coverage for the shift-blocks-letters-but-not-question-mark
distinction plus meta/ctrl/alt still blocking "?".
The <td class="diff-finding-cell"> carried display: flex directly, which takes it out of the CSS table box model: colspan is silently ignored by the browser on a flex-display cell, so the finding card rendered squeezed into the 48px line-no column instead of spanning the row. The <td> now stays a plain table-cell (no display override, colspan works again); the flex layout moved onto a new .diff-finding-body div wrapped around the same content. jsdom cannot catch layout regressions like this one, so please eyeball it manually in a browser: open a repo with staged changes, enable Commit Review, run a review, and confirm a finding row spans the full diff width in both inline modes (with and without selectable partial-staging checkboxes, since colspan differs between the two). Added a regression assertion (colSpan > 1 and the flex wrapper is a child of the td, not the td itself) so a future refactor that moves display: flex back onto the td fails a real test instead of only a manual check.
useCommitReview wrapped a single shared usePrReviewQueue() instance and
called queue.run() again on every run(). 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 could fire after B and flip the shared
running/done/total refs back to A's state mid-run-B.
run() now creates a fresh usePrReviewQueue() instance every call; an
`activeQueue` shallowRef always points at the most recent one, and the
exposed running/progress/resume all read through it. A stale run's
cleanup only ever touches refs nothing else reads anymore, so it can no
longer clobber a newer run. (shallowRef, not ref: a plain ref would make
the queue object itself reactive, and Vue auto-unwraps its nested
done/total/running refs on access, which silently turned every .value
read into undefined once activeQueue became reactive - shallowRef keeps
the queue object itself as a plain, non-unwrapped value.)
Also changes run()'s return type from Promise<void> to Promise<boolean>:
true once a full attempt actually happened (even if it errored, the
staged diff was empty, or every file was filtered out), false when the
call never really tried (disabled, AI unavailable, empty cwd, or
superseded by a newer run before it got anywhere). This is what lets the
App.vue wiring (next commit) tell "ran with zero findings" apart from
"did not run" without adding a second signal.
Added tests: a stale aborted run's cleanup no longer clobbers a newer
run's running/progress state; the file-count cap test is tightened from
toBeLessThanOrEqual (which passed even at 0 calls) to an exact toBe(40);
a new byte-budget test proves a single oversized file's diff exhausts the
cap and excludes files after it; and a resume()-unblocks-a-hidden-run
test that proves the run actually completes instead of hanging.
…pass useCommitReview.resume() was exposed but nothing ever called it. Starting a review then hiding the tab (or minimizing) left the queue paused forever on document.hidden inside usePrReviewQueue.waitWhileHidden(): only a staged-set change (via commitReview.reset()) could unwedge it, and the button stayed disabled in the meantime. usePrPanel.ts already has the app's one visibilitychange listener (it resumes useReviewIntelligence's PR pre-review queue, a different queue instance, on the same hidden -> visible edge). Added an onVisibilityResume option to PrPanelOptions, called from that same handler, and wired commitReview.resume into it from App.vue's usePrPanel(...) call - no second listener. Also, two silent-feedback gaps in the same "Review staged changes" click handler: - A failed review (gitExec error) set commitReview.lastError but nothing read it, so a failure was indistinguishable from success. Reused the existing repoError error-toast banner via a watcher, matching every other place in this file that funnels a failure into repoError.value. - A clean review (zero findings) produced zero UI feedback, indistinguishable from "didn't run" or "failed". commitReview.summaryClean already existed but was unreachable outside the findings modal (which only opens when there are findings). The reviewStaged handler now checks run()'s new boolean return (only true once a full attempt actually happened) and shows a brief transient toast via the same affordance already used for the "?" shortcut-help reminder. No new test file for the App.vue wiring itself (no App.test.ts exists in this codebase); the new usePrPanel-visibility-resume.test.ts proves the visibility-triggered callback fires on the hidden -> visible edge and never throws when the option is omitted, and useCommitReview.test.ts (previous commit) proves resume() actually unblocks a paused run.
…file chip to staged rows commitReviewAutoReReview had a real Settings checkbox (SettingsPanel.vue, useSettings.ts, i18n x5) but zero consumer in this PR: its only purpose is arming the one-shot re-review after a "Fix with agent" handoff, which is Task 3 and explicitly out of scope here. Hid the toggle from the Commit Review settings group rather than ship a control that visibly does nothing yet; the setting field itself, its default, and the i18n strings stay in place for PR2 to wire up. Also gated RepoSidebar's per-file finding-count chip on section === 'staged' in both the flat-list and tree-layout renderers. Findings are index-scoped, so without this gate the chip also decorated the unstaged/untracked row of the same path, which never reflects the staged diff's content.
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.
This was referenced Aug 17, 2026
Closed
Owner
Author
|
Superseded by #164, which combines all three PRs into a single review surface for the full v3.7.0 Commit Review feature. |
devlint
added a commit
that referenced
this pull request
Aug 19, 2026
Combines the full v3.7.0 "Commit Review" feature, originally built and adversarially reviewed across three stacked PRs (#159, #160, #163), then further product- and code-reviewed as a whole and fixed here (see commit history for the detailed round-by-round verification trail). - Review staged changes: AI pass over the staged diff, inline findings with severity badges, n/p/x navigation - Fix with agent: pipe findings into a terminal AI agent session - Iterations & coverage tracking, bound to HEAD - Review / Vouch / Skip commit-time decision, recorded as a GitWand-Review trailer - Per-repo .gitwandrc opt-in overriding the global setting in either direction - Composable pre-commit hook merging the shipped secrets-scanner section with a new warn-only review reminder - Fixed a core reactivity bug where routine background polling silently wiped findings, plus 13 other findings from a dedicated product/code review round 1067 core + 959 desktop tests, parity suite green, clean build, zero new Tauri commands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First slice of ROADMAP.md's v3.7.0 "Commit Review" feature (inspired by git-lrc), fully local, in-panel, opt-in and off by default. Follow-up PRs will add fix-with-agent handoff + iterations/coverage tracking, and the Review/Vouch/Skip commit trailer +
.gitwandrc/hook wiring.GitDiffvia a newscopeparameter instead of being PR-only), producing inline findings with severity badges anchored in the diff.n/pcycles finding-to-finding,?shows the shortcut help, per-file finding counts shown as chips in the staged file list (flat and tree layouts).commitReviewEnabled(default off) in Settings, synced in bothuseSettings.tsandSettingsPanel.vue.gitExec/git diff --cachedpath, no Rust changes.This PR went through an implementation pass, an adversarial verification pass that found 3 blocking bugs (a permanent hang if the tab is hidden mid-review, an unreachable
?help shortcut, and acolspan-breakingdisplay:flexon a<td>) plus 4 medium issues (silent failures, silent clean-pass, a stale-abort race condition, a dead settings toggle), a fix pass, and a second verification pass that confirmed everything, before this PR was opened.Test plan
pnpm --filter @gitwand/core run test -- --run— 1056/1056 passing (unchanged, no core files touched)cd apps/desktop && pnpm test -- --run— 739/739 passing (100 files, 50 new tests added: engine, keymap, navigation, layout regression, race condition, byte-cap, visibility-resume)cd apps/desktop && pnpm build— cleanvue-tsc --noEmit+vite buildpnpm dev:web, staged changes, enable Commit Review in Settings) — jsdom cannot verify CSS layout, this is the one thing to eyeball before merge