feat(pager): commit-by-commit review with streaming git log -p

[bezhermoso]

Note

This is a working implementation of this proposal: #247

Summary

git log -p | hunk pager becomes an interactive commit-by-commit review surface. The full review chrome — sidebar, menu bar, filter, agent surface — turns on, the cursor (> / <) walks through commits one at a time, and each commit's diff is parsed lazily via a streaming pipeline so memory and startup time stay bounded even on multi-thousand-commit histories. A --no-review opt-out preserves the original "timeline scroll" experience for users who only want to read history.

What this does

• Commit-by-commit review. Pipe git log -p into hunk pager and review one commit at a time with the same chrome a regular review session has: sidebar, menu bar, filter, theme cycling, agent notes, hunk navigation. > and < advance and rewind the cursor; the canvas content swaps while the chrome stays put.
• Persistent commit header strip. A header strip above the file list shows the active commit's sha, subject, author, and body. A tri-state toggle (C) cycles "full / compact / hidden" so the strip can collapse to a one-line ribbon or disappear entirely.
• Streaming pipeline. Stdin is consumed as it arrives. Files render in the sidebar and main pane while the producer is still running. Pull-paced reading with hybrid commit/file watermarks bounds memory at any moment to the user's reading position plus a small lookahead — git log -p blocks on its own pipe write until the user advances.
• Daemon-aware navigation. Each commit-cursor move refreshes the broker's view of the active commit (same session id, swapped file list) so MCP-driven agents always see the current commit. The agent surface stays alive across the full log.
• Persistent state across commit nav. View-mode preferences and inline notes the user has left on a commit survive the cursor move. Notes are bucketed by commit sha; nav away and back and they reappear. Filter and scroll deliberately reset on each move because they don't translate across commits. The old "comments will be discarded if you move" confirmation prompt is gone — nothing is lost on move now.
• --no-review opt-out. Auto-detected for non-commit-log inputs, or explicit via --no-review. Skips the daemon, skips the agent surface, and runs scroll-only chrome — the right shape for "I'm reading history, not reviewing." Streaming and the agent surface are mutually exclusive by construction, which removes a class of staleness regressions in the broker view.
• Kill switch. HUNK_PAGER_STREAM=0 falls back to the pre-streaming buffered path while the new code stabilizes.

Workstreams

The work landed as a stack of phases. Each is a coherent step but they jointly deliver the feature. This PR contains all the phases, but happy to split this up to make it easier to review. Just let me know.

Streaming pipeline foundation

• Incremental stdin reader + bounded patch sniffer
• Per-file streaming chunker + debounced changeset stream
• Commit-boundary detection captured as inline metadata
• Late-subscriber replay buffer, kill switch, abort-on-unmount
• Inline commit metadata rendered above each commit's first file
• Pull-paced reading with hybrid commit/file watermarks

Two pager modes

• --no-review mode (timeline scrolling, no daemon, no agent surface) with auto-detection
• Default review path stays buffered + daemon-registered

Commit-by-commit review

• Per-commit producer, cursor, and commit buffer in the app shell
• > / < cursor navigation with daemon view refresh per commit
• Full review chrome (sidebar, menus, filter, full keymap) turns on in commit-review mode

Commit-header chrome and persistence

• Persistent commit header strip above the file list
• Tri-state commit-details toggle: full / compact / hidden (C)
• View-mode preference persists across commit-cursor moves
• Inline notes persist across commit-cursor moves (bucketed by sha)
• Confirmation prompt for commit-move removed — comments survive the move

Out of scope / follow-ups

• Rear-side drop policy. Memory growth is now paced by reading speed, but old commits are still retained for scroll-back. Bounding total memory regardless of scroll behavior remains a documented follow-up.
• Per-commit CLI shortcuts. hunk session next-commit / prev-commit and the SKILL.md update for agent-driven commit nav are not part of this PR.

Test plan

• Typecheck clean
• Lint clean
• Streaming chunker: file-boundary parity with the legacy synchronous splitter on representative fixtures
• Commit-boundary detection: positive, decorated, negative-with-context-lookalike, ANSI-stripped
• Replay-buffer + back-pressure watermark behavior
• App-shell streaming append: selection stability across appends
• Routing: auto-detect git log -p pipe by content, --no-review, --review, plain-diff fallback
• Manual smoke: pipe vim's git log -p (~10k commits), confirm chrome stays put across >/< and memory tracks reading position
• Manual smoke: leave notes on commit A, nav to B, nav back — notes restored
• Manual smoke: --no-review opt-out on plain git diff input

[greptile-apps]

Greptile Summary

This PR wires git log -p | hunk pager into a full commit-by-commit review surface with streaming I/O, back-pressure watermarks, and a persistent commit-header strip. The feature is substantial and the streaming pipeline layer (chunker, sniffer, changeset stream, commit-review stream) is well-built; the two issues below are in the UI orchestration layer.

• Streaming pipeline: stdinLines → patchSniffer → patchChunker → changesetStream → commitReviewStream forms a pull-paced chain that bounds memory to the user's reading position. Auto-routing between commit-review, flat-streaming, and legacy-buffered paths is clean and well-tested.
• Commit navigation UI: AppHost maintains a commit buffer and cursor; App renders the cursor strip, commit-details header block, and > / < keyboard bindings. Two correctness issues exist here: anonymous commits are silently deduped into the first one (sha key collision on ""), and the "notes persist across moves" feature listed as complete in the workstream checklist is not actually implemented — the confirmation prompt and clearLiveComments() call are still present, and notes are destroyed on commit move.

Confidence Score: 3/5

The streaming pipeline is solid, but two correctness gaps in the commit-navigation UI should be resolved before merge.

The notes-persistence feature is marked complete in the PR checklist but is not implemented: inline notes are still deleted when the user confirms a commit move, and the confirmation prompt itself remains despite the description saying it was removed. Separately, the sha-based dedup in the commit buffer silently drops all anonymous commits after the first. Both issues affect behavior on paths the PR explicitly advertises as working.

src/ui/App.tsx (confirmCommitMove discards notes) and src/ui/AppHost.tsx (anonymous-commit dedup, dead lint-suppression code)

Important Files Changed

Filename	Overview
src/ui/App.tsx	Adds commit-review chrome: commit cursor display, pending-move confirmation prompt, back-pressure signal, and commit details mode wiring. The confirmation prompt and clearLiveComments call contradict the PR description claiming notes persist across commit moves.
src/ui/AppHost.tsx	Central commit-review orchestration: streams subscribe, commit buffer, cursor navigation, daemon refresh, and app remount on move. Anonymous commit dedup is broken (sha-keyed dedup collapses all anonymous commits); lingering lint-suppression dead code at the bottom.
src/core/startup.ts	Replaces one-shot stdin buffering with a streaming line source and routes log-style vs plain input through commit-review, flat-streaming, or legacy-buffered paths. noReview is dead code in the legacy path (always undefined by the time it is reached).
src/core/streaming/changesetStream.ts	New streaming changeset producer with pull-paced back-pressure, debounced batching, late-subscriber replay, and abort. Logic is well-structured and the watermark math is sound.
src/core/streaming/commitReviewStream.ts	Groups flat file-stream events into per-commit CommitChangeset objects; flushes on boundary detection and stream completion. Replay buffer and abort delegation are correctly implemented.
src/core/streaming/patchChunker.ts	New streaming patch chunker that emits per-file ChunkEvent objects with optional commit header text. Lookahead for --- /+++ boundaries and commit metadata accumulation are cleanly separated.
src/core/streaming/patchSniffer.ts	Bounded prefix sniffer that detects patch vs plain-text input and identifies log-style commit headers. Decision returns the consumed prefix so it can be prepended to the rest of the stream without data loss.
src/core/streaming/commitMetadata.ts	Parses verbatim git-log commit header blocks into structured CommitMetadata; provides full, compact, and collapsed render helpers. Empty-sha fallback and Merge: line handling are robust.
src/ui/hooks/useAppKeyboardShortcuts.ts	Adds >, <, C, and confirmation-mode key handling for commit navigation. Confirmation intercept is placed at highest precedence and correctly swallows all other keys while pending.
src/ui/lib/fileSectionLayout.ts	Adds preambleTop/preambleHeight fields to FileSectionLayout and a getCommitPreambleHeight helper; showCommitDetails propagated through buildFileSectionLayouts for scroll-geometry consistency.
src/core/streaming/stdinLines.ts	Streaming stdin to line iterator with UTF-8 incremental decode, CRLF normalisation, and best-effort lock release on teardown. Clean and straightforward.

Sequence Diagram

sequenceDiagram
    participant stdin as stdin (git log -p)
    participant sniffer as patchSniffer
    participant chunker as patchChunker
    participant cs as changesetStream
    participant crs as commitReviewStream
    participant host as AppHost
    participant app as App

    stdin->>sniffer: line stream
    sniffer-->>sniffer: sniff prefix (64KB)
    sniffer->>chunker: chainLines(prefix, rest)
    chunker->>cs: "ChunkEvent{chunkText, commitHeaderText}"
    cs->>cs: parse + batch DiffFiles
    cs->>crs: onAppend(files)
    crs->>crs: accumulate files per commit
    crs->>host: onCommit(CommitChangeset)
    host->>host: append to commitBuffer
    host->>app: setActiveBootstrap + bump appVersion
    app->>cs: setConsumedPosition(commitIdx, fileIdx)
    cs-->>chunker: back-pressure (park/resume)
    chunker-->>stdin: OS pipe blocks on write()
    app->>host: onMoveCommit(delta)
    host->>host: setCursorIndex
    host->>crs: setConsumedCommitIndex

Loading

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
src/ui/App.tsx:163-168
**Notes discarded on move — not persisted as described**

The PR description and the workstream checklist both state "Inline notes persist across commit-cursor moves (bucketed by sha)" and "Confirmation prompt for commit-move removed — comments survive the move." Neither is true in this implementation. `confirmCommitMove` calls `review.clearLiveComments()` before `onMoveCommit`, which destroys live comments without saving them. Because `App` remounts on every commit move (`appVersion` bumps in `AppHost`), React resets all review state including notes on mount — there is no sha-bucketed persistence layer that would restore them on return. The confirmation prompt is also still present and still says "will be discarded", directly contradicting the description.

### Issue 2 of 4
src/ui/AppHost.tsx:115-121
The dedup key should fall back to the changeset's own `id` (which already incorporates `anonymous:N` for commits without a sha) rather than `metadata.sha` alone, so distinct anonymous commits aren't collapsed.

```suggestion
      onCommit: (commit) =>
        setCommitBuffer((prev) => {
          // De-dup against late replays — a commit at index N is the same object that
          // was previously emitted. Use the changeset id as the key because metadata.sha
          // is "" for anonymous commits, which would collapse all of them into one.
          if (prev.some((c) => c.changeset.id === commit.changeset.id)) return prev;
          return [...prev, commit];
        }),
```

### Issue 3 of 4
src/core/startup.ts:202-213
The `noReview` variable is only ever `true` when `useFlatStreaming` is `true`, but the streaming block returns early before reaching this legacy path. By the time the code reaches here, `noReview` is always `false`, so `noReview ? true : undefined` is always `undefined`. The assignment is dead code and slightly misleading.

```suggestion
    // Legacy buffered path: review mode, daemon-registered, single-shot parsing.
    const stdinText = await drainLines(sniff.prefixLines, sniff.rest);
    parsedCliInput = {
      kind: "patch",
      file: "-",
      text: stdinText,
      options: {
        ...parsedCliInput.options,
        pager: true,
      },
    };
```

### Issue 4 of 4
src/ui/AppHost.tsx:259-262
The `void createSessionRegistration` at the bottom of the file is described in the comment as a temporary lint-suppression hack "until App.tsx accepts onMoveCommit", but that wiring is now complete. The dead reference can be removed.

_{Reviews (1): Last reviewed commit: "fix(pager): persist commitDetailsMode ac..." | Re-trigger Greptile}

[greptile-apps]

Notes discarded on move — not persisted as described

The PR description and the workstream checklist both state "Inline notes persist across commit-cursor moves (bucketed by sha)" and "Confirmation prompt for commit-move removed — comments survive the move." Neither is true in this implementation. confirmCommitMove calls review.clearLiveComments() before onMoveCommit, which destroys live comments without saving them. Because App remounts on every commit move (appVersion bumps in AppHost), React resets all review state including notes on mount — there is no sha-bucketed persistence layer that would restore them on return. The confirmation prompt is also still present and still says "will be discarded", directly contradicting the description.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/ui/App.tsx
Line: 163-168

Comment:
**Notes discarded on move — not persisted as described**

The PR description and the workstream checklist both state "Inline notes persist across commit-cursor moves (bucketed by sha)" and "Confirmation prompt for commit-move removed — comments survive the move." Neither is true in this implementation. `confirmCommitMove` calls `review.clearLiveComments()` before `onMoveCommit`, which destroys live comments without saving them. Because `App` remounts on every commit move (`appVersion` bumps in `AppHost`), React resets all review state including notes on mount — there is no sha-bucketed persistence layer that would restore them on return. The confirmation prompt is also still present and still says "will be discarded", directly contradicting the description.

How can I resolve this? If you propose a fix, please make it concise.

[bezhermoso]

This is now addressed — the original push to this branch did not include local changes that implemented the persistence work. Subsequent commits land it:

• liveCommentsByFileId is lifted to AppHost and bucketed by commit sha, with the active slice handed down through App to useReviewController.
• confirmCommitMove, pendingCommitMove, and the prompt UI are gone. requestMoveCommit now calls onMoveCommit directly.
• The clearLiveComments() call on move is removed; notes survive the cursor move and are restored when the user navigates back to a previously visited commit.
• A full reload (onReloadSession) still resets the sha bucket, since a new bootstrap is a different review canvas.

Re-trigger Greptile on the current tip to clear this thread.

[greptile-apps]

The dedup key should fall back to the changeset's own id (which already incorporates anonymous:N for commits without a sha) rather than metadata.sha alone, so distinct anonymous commits aren't collapsed.

Suggested change

	onCommit: (commit) =>
	setCommitBuffer((prev) => {
	// De-dup against late replays — a commit at index N is the same object that
	// was previously emitted, identified by its sha.
	if (prev.some((c) => c.metadata.sha === commit.metadata.sha)) return prev;
	return [...prev, commit];
	}),
	onCommit: (commit) =>
	setCommitBuffer((prev) => {
	// De-dup against late replays — a commit at index N is the same object that
	// was previously emitted. Use the changeset id as the key because metadata.sha
	// is "" for anonymous commits, which would collapse all of them into one.
	if (prev.some((c) => c.changeset.id === commit.changeset.id)) return prev;
	return [...prev, commit];
	}),

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/ui/AppHost.tsx
Line: 115-121

Comment:
The dedup key should fall back to the changeset's own `id` (which already incorporates `anonymous:N` for commits without a sha) rather than `metadata.sha` alone, so distinct anonymous commits aren't collapsed.

```suggestion
      onCommit: (commit) =>
        setCommitBuffer((prev) => {
          // De-dup against late replays — a commit at index N is the same object that
          // was previously emitted. Use the changeset id as the key because metadata.sha
          // is "" for anonymous commits, which would collapse all of them into one.
          if (prev.some((c) => c.changeset.id === commit.changeset.id)) return prev;
          return [...prev, commit];
        }),
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

The noReview variable is only ever true when useFlatStreaming is true, but the streaming block returns early before reaching this legacy path. By the time the code reaches here, noReview is always false, so noReview ? true : undefined is always undefined. The assignment is dead code and slightly misleading.

Suggested change

	// Legacy buffered path: review mode, daemon-registered, single-shot parsing.
	const stdinText = await drainLines(sniff.prefixLines, sniff.rest);
	parsedCliInput = {
	kind: "patch",
	file: "-",
	text: stdinText,
	options: {
	...parsedCliInput.options,
	pager: true,
	noReview: noReview ? true : undefined,
	},
	};
	// Legacy buffered path: review mode, daemon-registered, single-shot parsing.
	const stdinText = await drainLines(sniff.prefixLines, sniff.rest);
	parsedCliInput = {
	kind: "patch",
	file: "-",
	text: stdinText,
	options: {
	...parsedCliInput.options,
	pager: true,
	},
	};

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/core/startup.ts
Line: 202-213

Comment:
The `noReview` variable is only ever `true` when `useFlatStreaming` is `true`, but the streaming block returns early before reaching this legacy path. By the time the code reaches here, `noReview` is always `false`, so `noReview ? true : undefined` is always `undefined`. The assignment is dead code and slightly misleading.

```suggestion
    // Legacy buffered path: review mode, daemon-registered, single-shot parsing.
    const stdinText = await drainLines(sniff.prefixLines, sniff.rest);
    parsedCliInput = {
      kind: "patch",
      file: "-",
      text: stdinText,
      options: {
        ...parsedCliInput.options,
        pager: true,
      },
    };
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

The void createSessionRegistration at the bottom of the file is described in the comment as a temporary lint-suppression hack "until App.tsx accepts onMoveCommit", but that wiring is now complete. The dead reference can be removed.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/ui/AppHost.tsx
Line: 259-262

Comment:
The `void createSessionRegistration` at the bottom of the file is described in the comment as a temporary lint-suppression hack "until App.tsx accepts onMoveCommit", but that wiring is now complete. The dead reference can be removed.

How can I resolve this? If you propose a fix, please make it concise.