refactor(review): extract a renderer-neutral review store into core (Phase 1 PR 1)

[benvinegar]

Phase 1 PR 1 of the browser-review rebuild (docs/browser-review-rebuild.md). Stacked on the Phase 0 guardrails branch.

What

Extracts a renderer-neutral review store into src/core/review/ — types / state / actions / reducer / store / selectors / intents, each with colocated tests — and refactors the terminal (useReviewController, App, DiffPane type import) onto it. Behavior-neutral: no user-visible change, empty changeset.

• State + named policies: ReviewState plus the value policies that read it (isRenderableStoredReviewNote, reviewLineAnchor, reviewNoteOwnerHunkIndex, reviewNoteAnchorLine, ReviewRevealRequest) — one named answer per policy instead of inline conditionals per consumer.
• Single-transition reducer with identity-stable no-ops; synchronous three-method store (getSnapshot / subscribe / dispatch).
• Typed intents (planReviewIntent / applyReviewIntent) with a typed outcome map and planning errors.
• Terminal boundary adapter src/ui/lib/reviewProjection.ts owns projection between the terminal's DiffFile model and the semantic store (UserReviewNote / DraftReviewNote moved here).
• useReviewController drops from 14 useState hooks to 3; the 3 mirroring refs are deleted; showAgentNotes moves from App state into shared review state (G1 classification: shared).

Implemented fresh against the plan and audit — the prototype branch was used as a semantic reference only; deliberate design divergences are documented in the plan's terms (named reveal requests instead of flag fields, reuse of existing ReviewNoteSource, trimmed store surface, no dead exports).

What changed, in diagrams

State ownership: before

Fourteen useState hooks in one controller, three refs mirroring state for synchronous reads, one review-relevant flag owned by App, and follow-up effects reconciling it all after the fact:

flowchart TB
    subgraph app["App.tsx"]
        SAN["showAgentNotes"]
    end
    subgraph urc["useReviewController — 14 useState hooks"]
        SEL["selectedFileId · selectedHunkIndex"]
        FIL["filter"]
        REV["fileTopAlignRequestId · hunkRevealRequestId · scrollToNote"]
        NOTES["liveCommentsByFileId · userNotesByFileId · draftNote"]
        EXP["expandedGapsByFileId"]
        SRC["sourceStatusByFileId"]
        SNAP["filesSnapshot"]
        CUR["lineCursor · lineCursorRevealRequestId"]
    end
    subgraph refs["Mirroring refs (sync reads)"]
        R1["sourceStatusRef"]
        R2["expandedGapsRef"]
        R3["savedDraftIdRef"]
    end
    SRC -. "mirrored each render" .-> R1
    EXP -. "mirrored each render" .-> R2
    NOTES -. "mirrored on save" .-> R3
    SNAP -. "follow-up effect clamps" .-> SEL
    FIL -. "follow-up effect re-selects" .-> SEL

Loading

State ownership: after

One semantic ReviewState behind a three-method store; the controller keeps only the store snapshot plus the two renderer-measured cursor values (terminal row geometry stays with the renderer that measures it, per the audit's do-not-unify list):

flowchart TB
    subgraph core["src/core/review — renderer-free, boundary-gated"]
        STATE["ReviewState<br/>selection · filter · reveal · notes<br/>expansion · source status · showAgentNotes"]
        STORE["store: getSnapshot / subscribe / dispatch"]
        STATE --- STORE
    end
    subgraph urc2["useReviewController — 3 useState hooks"]
        SNAP2["snapshot (subscribed)"]
        CUR2["lineCursor · lineCursorRevealRequestId<br/>(renderer-measured, stays terminal-side)"]
    end
    subgraph app2["App.tsx"]
        VIEW["view preferences (per-client)"]
    end
    STORE -- "subscribe → one snapshot" --> SNAP2
    SNAP2 --> app2

Loading

How a change flows now

Validation and clamping happen in the intent planner before the state changes, replacing scattered setX calls plus after-the-fact reconciliation effects:

sequenceDiagram
    participant H as key/mouse handler
    participant C as useReviewController
    participant I as intents (plan + apply)
    participant S as store
    participant R as reducer
    H->>C: select hunk / edit note / set filter
    C->>I: ReviewIntent
    Note over I: planReviewIntent —<br/>validate file, clamp hunk,<br/>typed planning errors
    I->>S: dispatch(ReviewAction)
    S->>R: reduce(state, action)
    R-->>S: next ReviewState (identity-stable no-ops)
    S-->>C: notify → single snapshot render

Loading

Module seam

flowchart LR
    subgraph core3["src/core/review (imports: core-internal + @pierre/diffs only)"]
        T3["types"] --> ST3["state"]
        ST3 --> A3["actions"] --> RD3["reducer"] --> STO3["store"]
        ST3 --> SE3["selectors"]
        ST3 --> IN3["intents"]
    end
    subgraph ui3["src/ui (terminal renderer)"]
        RP3["lib/reviewProjection.ts<br/>DiffFile ⇄ semantic adapter"]
        URC3["hooks/useReviewController"]
        APP3["App.tsx"]
        DP3["DiffPane"]
    end
    URC3 --> STO3
    URC3 --> IN3
    RP3 --> T3
    APP3 --> URC3
    DP3 --> RP3
    GATE(["scripts/source-boundaries.test.ts<br/>gates every arrow crossing the seam"]) -.- core3

Loading

Scope boundaries

Document projection, geometry primitives, and the conformance harness are Phase 1 PR 2; navigation intents and the command catalog are PR 3. Trimmed prototype surface (generations, expandedLineProof, filter selectors, additional note intents) is tracked per-phase in the plan. Nothing deleted yet → no tombstone entries.

Gates

• bun run typecheck clean; bun run lint clean (deny-warnings); formatted.
• bun test: 2311 pass / 4 fail — all four failures reproduce on the untouched base branch (missing @axe-core/playwright dev dep ×2, one pre-existing ui-components failure, one known-flaky PTY test); branch failures are a strict subset of baseline failures.
• bun run test:integration (PTY): 105/106, sole failure is the pre-existing flake, reproduced on baseline.
• Boundary suite 10/10 — seam gates active for src/core/review/ (contained in core, @pierre/diffs-only externals, node-debt map untouched).
• No test under test/pty/, test/cli/, test/session/ modified; useReviewController.test.tsx (38 tests) and AppHost.* suites pass unedited.

Repays (partial, per the spanning rule): D2 core+terminal sites (isBlankReviewNoteBody; fixture + conformance registration land with PR 2's harness).

[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 12, 2026 11:52pm

[greptile-apps]

Greptile Summary

This PR extracts semantic review state into a renderer-neutral core store and adapts the terminal review controller to project between that store and DiffFile values.

• Adds review types, state policies, reducer actions, selectors, intents, and a synchronous observable store.
• Moves terminal selection, filtering, notes, drafts, visibility, and expansion state onto the shared store.
• Adds colocated unit coverage for core transitions, intent planning, storage, selectors, and terminal projection.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking formatting inconsistency against the mounted repository guidance.

The store extraction preserves the investigated navigation, reload, and note-projection behavior, while the remaining accepted concern is limited to code-format consistency.

Files Needing Attention: src/core/review/actions.ts and the other newly added review-store modules

Important Files Changed

Filename	Overview
src/core/review/state.ts	Defines the renderer-neutral review state and named policies for note visibility, anchoring, and reveal requests.
src/core/review/reducer.ts	Implements identity-stable state transitions and content-derived state retirement during document reconciliation.
src/core/review/intents.ts	Validates semantic review operations and lowers them into reducer actions with typed outcomes.
src/core/review/store.ts	Adds a synchronous observable store that increments revisions and publishes state-changing dispatches.
src/ui/hooks/useReviewController.ts	Migrates terminal review state onto the shared store while retaining terminal-specific projection, navigation, geometry, and source-loading behavior.
src/ui/lib/reviewProjection.ts	Adapts DiffFile, live-comment, user-note, draft, and source-status values to and from the semantic store.
src/ui/App.tsx	Moves agent-note visibility into shared review state and updates application wiring accordingly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Terminal["App / useReviewController"] --> Projection["reviewProjection"]
  Projection --> Store["ReviewStore"]
  Store --> Reducer["reduceReviewState"]
  Intents["plan/applyReviewIntent"] --> Store
  Reducer --> State["ReviewState"]
  State --> Projection
  Projection --> Panes["DiffPane and session bridge"]

Loading

Prompt To Fix All With AI

### Issue 1
src/core/review/actions.ts:13
**Align new review-store formatting**

The newly added review-store modules use two-space indentation and double-quoted strings, conflicting with the repository guidance requiring four spaces and single quotes. Applying the repository convention across this new subsystem will avoid ongoing formatting churn and inconsistent examples.

---

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

_{Reviews (1): Last reviewed commit: "refactor(ui): move terminal review state..." | Re-trigger Greptile}

[greptile-apps]

Align new review-store formatting

The newly added review-store modules use two-space indentation and double-quoted strings, conflicting with the repository guidance requiring four spaces and single quotes. Applying the repository convention across this new subsystem will avoid ongoing formatting churn and inconsistent examples.

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/actions.ts
Line: 13

Comment:
**Align new review-store formatting**

The newly added review-store modules use two-space indentation and double-quoted strings, conflicting with the repository guidance requiring four spaces and single quotes. Applying the repository convention across this new subsystem will avoid ongoing formatting churn and inconsistent examples.

**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!