fix(review): terminal consumes core's note-anchor, reveal, and command-lowering policies

[benvinegar]

The substantive PR of the cleanup series (follows #736): the terminal was running parallel implementations of three policies that src/core/review/ already owns, with real divergences. Each finding is one commit; the terminal now consumes core and the parallel paths are deleted.

1. Note placement comes from core anchor resolution — fixes a visible bug

Core's resolveReviewNoteAnchor decides which hunk owns a note and which line it prefers, including a fallback branch for ranges the current patch collapsed. The terminal ignored it: reviewRenderPlan.ts re-derived ownership by intersecting each note's raw oldRange/newRange with rendered rows, and DiffPane built visible notes from raw annotations, discarding the anchors reviewProjection had already computed. The divergence was user-visible: a note anchored to lines that no rendered row contains (collapsed into a gap, or an expanded-away range) found no matching row and fell back to fileLineRows[0] — the note rendered at the top of the file instead of beside its owning hunk. All note kinds now flow through core's resolution once, the render plan places notes by the resolved anchor (nearest row at-or-after the preferred line within the owning hunk), and the containment path is deleted. Regression tests pin the collapsed-range case at the render-plan level plus a PTY test observing the fix live; the changeset is patch for this fix.

2. Reveal target comes from selectActiveRevealNoteId

Core's selector (draft in the selected hunk wins; else earliest anchor line, arrival order breaking ties) had zero production importers while DiffPane re-decided the target geometrically — first note row whose measured top falls inside the selected hunk's bounds, i.e. rendered order, a different tiebreak. DiffPane now resolves the note row by the id the selector returns (via the existing rowBoundsByStableKey mapping) and the geometric scan is gone. This intentionally changes which note wins when several share a hunk; the core tiebreak is pinned in a UI-level test.

3. Commands execute the review effects the catalog declares

lowerAppCommandToReviewIntent existed so the command catalog's declared review effects would be the ones that run, but its only consumer was its own test — useReviewController restated the same three intents by hand (notes/set-visibility, the gap toggle, notes/start-draft). The three handlers now route through the lowering; terminal-owned bookkeeping (draft ids, cursor restore points, viewport reveals) stays in the hook wrapping the produced intent. Tests assert the catalog effect and the executed intent agree for all three commands.

Gates

bun run typecheck clean. Full bun test: 2844 tests, failures limited to the environment's pre-existing baseline (two @axe-core/playwright website spec errors and PTY flakes under parallel load — the wiring-relevant suites plus the flaky PTY chrome test pass 88/88 in isolation). bun run lint 0/0; bun run format:check clean. Rebased onto current main (post-#736). A real-TTY smoke run isn't possible in this headless environment; PTY integration coverage stands in.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
hunk-web	Ignored	Preview	Aug 14, 2026 8:18pm

[greptile-apps]

Greptile Summary

The PR centralizes terminal review behavior around core note anchoring, reveal selection, and command lowering.

• Resolves visible-note anchors once through core and places collapsed or expanded-gap notes within their owning hunks.
• Uses the shared reveal policy to select a note by ID and map it to measured geometry.
• Routes note visibility, gap expansion, and draft creation through catalog-declared review effects while preserving terminal bookkeeping.

Confidence Score: 4/5

The note-ID collision should be fixed before merging because it can make note-preferring navigation reveal a different annotation.

The new ID-based reveal path relies on identifiers that can collide when generated index fallbacks overlap explicit annotation IDs, causing geometry lookup to return the wrong note row.

Files Needing Attention: src/ui/components/panes/DiffPane.tsx

Important Files Changed

Filename	Overview
src/ui/lib/agentAnnotations.ts	Adds the shared construction path that resolves anchors for all visible terminal note kinds.
src/ui/diff/reviewRenderPlan.ts	Places notes using resolved owner and preferred-line data rather than re-deriving ownership from rendered-row containment.
src/ui/components/panes/DiffPane.tsx	Adopts shared anchor and reveal policies, but synthesized note-ID collisions can make reveal target the wrong rendered row.
src/ui/hooks/useReviewController.ts	Lowers three built-in commands through the catalog while retaining source-load, cursor, draft-ID, and reveal bookkeeping.
src/core/review/selectors.ts	Extracts a surface-independent reveal-note policy preserving draft priority, anchor ordering, and arrival-order ties.
src/core/commandCatalog.ts	Adds total built-in lookup and renderer-supplied note locations for command lowering.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Annotations and drafts] --> B[Core anchor resolution]
  B --> C[VisibleAgentNote]
  C --> D[Review render plan]
  D --> E[Measured row geometry]
  C --> F[Core reveal selector]
  F --> G[Selected note ID]
  G --> E
  H[Terminal command] --> I[Command catalog lowering]
  I --> J[Review intent]
  J --> K[Review store and terminal bookkeeping]

Loading

Prompt To Fix All With AI

### Issue 1
src/ui/components/panes/DiffPane.tsx:495-511
**Colliding note IDs break reveal**

When an annotation without an ID is at index `N` and another annotation has the explicit ID `"N"`, both receive the same synthesized ID. The reveal selector can choose one note while `rowBoundsByStableKey` resolves that shared ID to the other note's row, causing navigation to reveal the wrong annotation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "refactor(review): run the review effects..." | Re-trigger Greptile}