feat(review): navigation intents, command catalog, and the address grammar (Phase 1 PR 3)

[benvinegar]

Phase 1 PR 3 — the final Phase 1 PR of the browser-review rebuild (docs/browser-review-rebuild.md). Stacked on PR 2 (#719).

What

Three commits, each green standalone:

1. One shared navigation walk — src/core/review/navigation.ts plans selection/move (hunk | file | annotated-hunk | annotated-file scopes, explicit per-scope wrap policy) and selection/select-file; both the terminal keyboard path and the agent-session comment navigation now plan through it. src/ui/lib/hunks.ts deleted and tombstoned in the boundary suite.
2. Command identity split out of the terminal table — src/core/commandCatalog.ts holds id/title/category/default chords/resolution locus as data; semantic commands declare their review effect as data with one lowerAppCommandToReviewIntent. The terminal's dispatch table is built from the catalog; menus and help read through it. Keyboard behavior byte-identical.
3. Semantic address grammar — src/core/review/address.ts, formatReviewAddress/parseReviewAddress over semantic keys (never array indices), round-trip + strict-rejection tests. No consumers yet by design (browser Phase 5, URL fragments Phase 6).

Repays (four-part definition, recorded inline in the audit doc): B1, B2, B4, B5, B9, B11 fully for existing sites; B3, B6, B7 core side (browser halves Phase 5); B8 primitive landed with the terminal site deliberately open (see residuals); F1–F3 catalog side; G3 grammar. B10/B12 untouched (Phase 3, wire).

What changed, in diagrams

Navigation: three walks → one

flowchart TB
    subgraph before["Before — same question, three answers"]
        K1["terminal keyboard<br/>ui/lib/hunks.ts cursors + carry rule"]
        S1["agent session navigation<br/>separate walk, parity by comment only"]
        W1["browser (prototype)<br/>would have been the third"]
    end
    subgraph after["After — one planner, callers supply facts"]
        NAV["core/review/navigation.ts<br/>selection/move · selection/select-file<br/>REVIEW_SELECTION_WRAP_POLICY"]
        K2["terminal keyboard"] -->|"ReviewIntent"| NAV
        S2["hunk session comment nav"] -->|"same intent"| NAV
        B2x["browser — Phase 5"] -.->|"same intent, over the wire"| NAV
    end

Loading

The wrap/clamp asymmetry that was previously incidental is now a named policy: plain hunk/file navigation clamps at edges, annotated-file navigation wraps — REVIEW_SELECTION_WRAP_POLICY states it once, and the multi-step carry rule for annotated jumps is in the planner with a conformance fixture pinning it.

Commands: one fused table → catalog + per-client bindings

flowchart LR
    subgraph before2["Before — appCommands.ts fused three things"]
        FUSED["id + title + chords<br/>+ KeyEvent matchers<br/>+ App-state closures"]
        MENUS1["menus/help read the table"]
    end
    subgraph after2["After"]
        CAT["core/commandCatalog.ts<br/>id · title · category · chords · locus<br/>semantic entries: review effect as data"]
        TERM["ui/lib/appCommands.ts<br/>matchers + handlers only,<br/>built FROM the catalog"]
        LOWER["lowerAppCommandToReviewIntent<br/>(one statement of 'which way does ] move')"]
        MENUS2["menus · help · future palette"]
        CAT --> TERM
        CAT --> LOWER
        CAT --> MENUS2
        WEB["browser — Phase 5"] -.->|"renders palette + bindings<br/>from the same catalog"| CAT
    end

Loading

Catalog↔handler parity is a compile error, not just a test: AppCommandId is derived from the catalog literal and the terminal handler map is keyed by it. A parity test additionally pins that menus and the dispatch table contain exactly the catalog's built-ins.

Where each command resolves

flowchart TB
    KEY["key press / menu click"] --> MATCH["terminal matcher (unchanged chords)"]
    MATCH --> LOCUS{"catalog locus"}
    LOCUS -->|semantic| INT["lower to ReviewIntent → planner → store<br/>(navigation, filter, toggle notes)"]
    LOCUS -->|client-local| LOCAL["terminal handler<br/>(scroll, layout, theme, help)"]
    LOCUS -->|host-only| HOST["host effect<br/>(quit, refresh, $EDITOR)"]
    INT --> ALL["every attached client re-renders"]

Loading

Residuals recorded in the audit (deliberate)

• B4 correction: the audit assumed the terminal rendered "no file" for a filtered-out selection — in fact it kept rendering the selected file. Terminal behavior won; selectNormalizedSelection yields fileKey: null only when nothing is visible.
• B8: selectNotesByHunk (owner-index grouping) landed, but DiffPane still groups by range overlap — converting is user-visible and belongs in a behavior-changing PR, not this one.
• B5: committed-vs-live filter timing still undecided (planning reads immediate, rendering reads one-render-deferred).
• Two long-standing navigation quirks are now named rather than incidental (edge-hunk re-reveal; annotated-file ring entry at index 0).
• expansion/toggle and the note-draft intent are the two named gaps in the catalog's semantic set (SEMANTIC_COMMANDS_WITHOUT_REVIEW_EFFECT) — Phase 2 fills them, which is what lets z and c reach a remote client.

Design notes (fresh implementation, not a port)

• Navigation plans over an explicit ReviewNavigationModel + caller-supplied ReviewAnnotationIndex — core never reaches into a renderer's diff model for facts it can't own.
• Catalog placement: src/core/commandCatalog.ts — UI vocabulary doesn't belong in core/review (pure review semantics), and src/ui is unreachable from a browser bundle. No react/opentui/node imports; string chords only. Phase 5 note: the web boundary gate's allowed-targets list must add this path when the browser adopts it.
• Address grammar: slash path with percent-encoded identifiers, fragment-safe, strict parsing.

Gates

• bun run typecheck, bun run lint (0/0), bun run format — clean; empty changeset.
• bun test: 2457 pass / 5 fail — failures identical to the base commit's (dev-dep ×2 + three known flakes, each passing in isolation); strictly fewer failures than baseline's run.
• bun run test:integration (PTY): 105/106; the one failure reproduces identically on the base commit. PTY files untouched.
• Boundary suite green with the first tombstone entry (src/ui/lib/hunks.ts — B1).
• Real source invocation smoke (bun run src/main.tsx -- diff) renders correctly.

---

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 13, 2026 12:49am

[greptile-apps]

Greptile Summary

The PR centralizes review navigation, command identity, filtering/selection derivations, and semantic address parsing for future terminal and browser consumers.

• Adds shared selection movement and file-jump planning with explicit reveal and wrap policies.
• Extracts built-in command metadata into a renderer-neutral catalog while retaining client-specific handlers.
• Adds strict semantic address formatting/parsing and expands conformance coverage.

Confidence Score: 4/5

The PR appears safe to merge after addressing the non-blocking documentation-rule violation in the navigation tests.

The shared navigation, command-catalog, and address paths have focused coverage, and the only accepted issue is missing required comments on two test helpers.

Files Needing Attention: src/core/review/navigation.test.ts

Important Files Changed

Filename	Overview
src/core/review/navigation.ts	Introduces the shared cursor walk, per-scope wrap policy, and reveal behavior for review navigation.
src/ui/hooks/useReviewController.ts	Routes terminal and session navigation through shared intents while retaining terminal-specific projection and lifecycle behavior.
src/core/commandCatalog.ts	Defines shared built-in command identity, resolution locus, default chords, and semantic review effects.
src/core/review/address.ts	Adds strict percent-encoded semantic review-address formatting and parsing with no consumers yet.
src/core/review/navigation.test.ts	Provides broad navigation-policy coverage, but two new helper functions omit required TSDoc comments.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Input[Keyboard, menu, or session request] --> Catalog[Shared command catalog]
  Catalog --> Intent[Review intent]
  Intent --> Planner[Shared navigation planner]
  Planner --> Store[Review store]
  Store --> Terminal[Terminal projection]
  Store -. future .-> Browser[Browser projection]

Loading

Prompt To Fix All With AI

### Issue 1
src/core/review/navigation.test.ts:35-41
**Undocumented navigation test helpers**

The new `model` and `at` helpers lack the required TSDoc-style explanations, leaving their fixture roles undocumented and violating the repository's function-comment standard.

```suggestion
/** Build a navigation model over the shared test files and supplied annotations. */
function model(annotations = EMPTY_REVIEW_ANNOTATION_INDEX): ReviewNavigationModel {
  return { files: FILES, annotations };
}

/** Build a semantic selection at one file and hunk position. */
function at(fileKey: string | null, hunkIndex: number): ReviewSemanticSelection {
  return { fileKey, hunkIndex };
}
```

---

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

_{Reviews (1): Last reviewed commit: "feat(review): add the semantic address g..." | Re-trigger Greptile}

[greptile-apps]

Undocumented navigation test helpers

The new model and at helpers lack the required TSDoc-style explanations, leaving their fixture roles undocumented and violating the repository's function-comment standard.

Suggested change

	function model(annotations = EMPTY_REVIEW_ANNOTATION_INDEX): ReviewNavigationModel {
	return { files: FILES, annotations };
	}
	function at(fileKey: string | null, hunkIndex: number): ReviewSemanticSelection {
	return { fileKey, hunkIndex };
	}
	/** Build a navigation model over the shared test files and supplied annotations. */
	function model(annotations = EMPTY_REVIEW_ANNOTATION_INDEX): ReviewNavigationModel {
	return { files: FILES, annotations };
	}
	/** Build a semantic selection at one file and hunk position. */
	function at(fileKey: string | null, hunkIndex: number): ReviewSemanticSelection {
	return { fileKey, hunkIndex };
	}

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/core/review/navigation.test.ts
Line: 35-41

Comment:
**Undocumented navigation test helpers**

The new `model` and `at` helpers lack the required TSDoc-style explanations, leaving their fixture roles undocumented and violating the repository's function-comment standard.

```suggestion
/** Build a navigation model over the shared test files and supplied annotations. */
function model(annotations = EMPTY_REVIEW_ANNOTATION_INDEX): ReviewNavigationModel {
  return { files: FILES, annotations };
}

/** Build a semantic selection at one file and hunk position. */
function at(fileKey: string | null, hunkIndex: number): ReviewSemanticSelection {
  return { fileKey, hunkIndex };
}
```

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))

---

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!