feat(review): explain agent-generated findings with Ask AI

[leoreisdias]

Context

Review agents can add useful findings directly to the code review, but following up on one currently means copying its text and rebuilding the relevant code context by hand.

This adds an Explain finding action that hands the finding to Ask AI in place. The response independently checks the finding before explaining the behavior and impact, then offers a brief recommendation when the finding holds.

What changed

• Adds an Explain action to registered review-agent findings across the diff, all-files, sidebar, guided review, and agent-job detail surfaces.
• Opens Ask AI and submits the finding with its own file, normalized range, side, selected code, finding text, and review-agent reasoning—never the user's pending line selection.
• Treats the agent-job registry as the provenance authority instead of trusting a free-form agent- source prefix.
• Fences finding text and reasoning as untrusted data in the composed prompt.
• Degrades commit-mismatched and stale line anchors to honest file context instead of attaching unrelated code or line numbers.
• Hides the action when Ask AI is unavailable and disables it while Ask AI is creating or streaming a response.
• Registers the DOM interaction test in CI.

Demo

Review focus

• buildExplainFindingRequest owns the context boundary: general findings stay fileless, file findings carry only the path, valid line findings carry normalized coordinates and extracted code, and stale/historical ranges fall back to file context with an explicit note.
• Explain eligibility comes from membership in agentJobs.jobs[].source; user annotations and GitHub/GitLab review comments remain ineligible.
• The all-files renderer wrappers republish Pierre portals only when Explain availability, registered sources, or loading state changes—not on Ask AI SSE chunks or unrelated application renders.
• The shared action row is intentionally always visible on touch devices because hover is unavailable there. This also improves access to the existing edit/copy/delete actions on those cards.
• Guided Review uses the current virtualized GuideFileCard path rather than restoring the removed legacy guide viewer.

Test plan

• bun test packages/review-editor — 170 passed, 29 skipped
• CI DOM test command, including CommentActions.test.tsx — 84 passed
• Guided Review and all-files lifecycle DOM tests — 22 passed
• bun run typecheck
• bun run build:review
• bun run build:hook
• Add the interaction GIF above

[backnotprop]

Review (at 9b64cbe0)

Thanks for this. The core design is sound: all five surfaces route through one handler, the pending line-selection is genuinely untouched (cleanest part of the PR), the hidden/disabled gating holds against the real session state machine, and the explain request rides the normal Ask AI path so it gets the changes-under-review preamble correctly. Typecheck clean, suite green, and explainFinding.test.ts survived mutation testing on all four attempted mutations. Verdict: needs changes, all of it well-scoped.

Required

1. CommentActions.test.tsx never runs in CI. It is DOM-gated but not registered in the DOM_TESTS=1 step of .github/workflows/test.yml, so the only test guarding the disabled-while-streaming claim silently skips (verified: 2 pass with the flag, 2 skip without). The test itself has teeth (removing disabled fails it). Add the path to that workflow step; this repo has been bitten by exactly this before.

2. The all-files render callbacks change identity on every render, not on availability changes. useAIChat returns a bare object literal, so askAI, then handleExplainAnnotation, then the panel props, then the new useCallback wrappers in AllFilesCodeView.tsx:864 and :2119 all get new identities every ReviewApp render. Pierre's SlotPortals is memoized on exactly those props, and before this PR both renderers were useStableCallback, so it never re-rendered. After, every visible file header and inline annotation re-renders per Ask AI SSE chunk and per search keystroke. Verified perf-only (portal keys and element types are stable, so card state survives), but this is the large-diff lag class. The fix is one token per site: depend on Boolean(onExplainAnnotation) (plus isAILoading) instead of the callback identity, which restores the behavior the PR body describes.

3. Tighten provenance from prefix to registry membership, and fence the finding text. isAgentGeneratedFinding keys on source.startsWith('agent-'), but POST /api/external-annotations applies zero validation to source (the one free-form field in the validator), and PATCH bypasses the transform entirely, so any local caller can relabel an annotation to agent-* and get one-click submission into an Ask AI session that can hold tool permissions. The authoritative registry is already in scope: agentJobs.jobs has a required source field, so a Set membership check is a one-liner in App.tsx. Relatedly, the composed message currently ends with the untrusted finding text unfenced, directly after the instruction block. The codebase's own convention fences untrusted content (buildDefaultPrompt wraps scope text and selected code in code fences); do the same for the finding and reasoning blocks and label them as data.

Verified clean on the adjacent worries: user-authored annotations never carry source, and GitHub/GitLab review comments become CommentAnnotation, which has no source field and whose sidebar branch passes no onExplain, so neither can ever grow the button.

4. Commit-scoped findings produce confidently wrong code. buildExplainFindingRequest drops commitSha, so a finding made on a commit:<sha> diff, explained after switching views, extracts code from the currently active patch at the historical line numbers and presents it as the finding's code with no warning. The export path already solved this exact problem (exportFeedback.ts:105 emits an anchored-to-that-commit note); the explain path needs the same guard, or should skip selectedCode on a commit mismatch.

Worth fixing while in there

5. Inverted ranges are passed raw (lines 16-12, empty extraction). Both established helpers normalize with Math.min/Math.max; explainFinding.ts should too, especially since external callers control these fields.

6. Stale ranges after a diff refresh degrade to misleading rather than fileless: the agent gets a path plus line numbers that may no longer exist, with no selectedCode and no signal. Prefer degrading to a fileless request (or flagging the missing extraction in the prompt).

7. ReviewAgentJobDetailPanel is the only surface that skips the isAgentGeneratedFinding check, and a non-matching row there gets a button that silently no-ops (the App.tsx guard returns without feedback). Add the check for consistency, and consider a toast on the no-op path.

8. [@media(hover:none)]:opacity-100 on the shared action row makes edit/copy/delete permanently visible on touch for every comment card, including PR-comment cards, which contradicts the "GitHub/GitLab review comments unchanged" claim. Probably desirable, but it deserves a mention in the PR body. Also group/finding in the job panel is now vestigial after the CopyButton relocation.

9. Nit: explainFinding.ts:717 inlines annotation.scope ?? 'line' instead of using annotationScope() from annotationDisplay.ts, the documented single source of truth.

The two full-suite failures during review were the documented timing flakes (diff-fingerprint, file-browser-watch); both pass in isolation and are unrelated to this PR.

[leoreisdias]

Thanks for the review! I will work on them

[leoreisdias]

Addressed in c4c9f9b4 — thanks for the thorough review.

• Registered CommentActions.test.tsx in the CI DOM step.
• Stabilized the all-files Pierre portal renderers around Explain availability, registered-source membership, and loading state rather than the callback identity.
• Replaced prefix-based eligibility with agentJobs.jobs[].source membership and fenced finding/reasoning content as untrusted data.
• Normalized inverted ranges and degraded commit-mismatched or stale line anchors to explicit file context.
• Applied the same eligibility check in job detail, removed the vestigial group class, and reused annotationScope().
• Kept touch action rows visible intentionally and documented that shared behavior in the PR description.

Verification: 170 review-editor tests passed, the 84-test CI DOM command passed, the 22 Guided Review/all-files DOM tests passed, typecheck passed, and both review/hook production builds passed.