[benvinegar]

Problem

Building keyboard navigation stops reads geometry.plannedRows for every reviewed file. That eagerly materializes and retains the full row-content plans intended to stay lazy until copy selection needs them, even when a file has no notes.

Change

• Record selectable semantic note IDs and owning hunk indexes alongside measured row bounds, using the existing planned note rows.
• Build ordinary diff navigation stops from bounds and line cursors without reading lazy copy-selection rows. Keep the existing alternate file-view row path.
• Preserve root/reply order and exclude drafts and decoration-only notes from note stops.
• Add getter-trap regressions for split/unified note-free navigation and semantic notes, plus real-PTY saved-note navigation through repeated resizes.
• Extend the existing geometry memory diagnostic to exercise navigation before first copy, and add a patch Changeset.

This is a standalone follow-up to merged #1200, based on main 4b159469. The geometry cache design, source fingerprinting, theme/content ownership, and bounded cache slots are unchanged. The existing key now also includes semantic note ID and draft/source classification because bounds retain selectable note targets. It does not include the broader experimental cache redesign. This belongs in the terminal navigation projection, not shared review semantics or an extension.

Independent benchmark results

Compared pristine main with only the navigation benchmark instrumentation against this extraction, on the same Linux x64 host using Bun 1.3.14 and the same dependencies. Five fresh-process samples per version; execution order alternated, with no concurrent benchmark/test workloads. Mounted moderate fixture: 180 files × 120 lines. Huge fixture: 1,001 files including one 50,000-line file. Values below are medians; heap is decimal MB of JavaScript heap, not process RSS.

Metric	Main	This PR
Huge post-first-frame heap	291.41 MB	214.13 MB (−26.5%)
Huge post-navigation heap	314.92 MB	237.44 MB (−24.6%)
Moderate post-navigation heap	60.54 MB	52.79 MB (−12.8%)
Balanced-stream geometry	13.34 ms	12.86 ms
Huge cold first frame	60.06 ms	63.59 ms
Huge navigation median / p95	28.76 / 85.99 ms	30.83 / 85.66 ms
Moderate navigation median / p95	23.50 / 42.91 ms	25.45 / 43.63 ms
Huge scroll p95	3.71 ms	4.34 ms
Twelve resizes, timed work	338.61 ms	329.82 ms

These measurements support the memory reduction, not a claim of universally faster interactions. Some timing medians increased; the huge navigation benchmark measures only four presses per process. No interaction/cold-geometry metric crossed the existing combined 15% and 5 ms regression criterion in these runs.

The intended tradeoff is deferred copy work: in the geometry diagnostic, explicitly materializing all 180 copy-row plans after navigation takes 9.29 ms rather than 0.07 ms, because navigation no longer prebuilds them. This is an all-files diagnostic, not a measured single-file copy latency. Copy still retains row plans once requested. These synthetic results do not explain the original private incident's retention owner.

Benchmark command, run once per version per round:

bun run benchmarks/run.ts --samples 1 \
  --script render-layout.ts --script interaction-latency.ts \
  --script huge-stream.ts --script resize-memory.ts --script geometry-memory.ts \
  --out <result.json>

Verification

• bun test packages/hunk/src/ui/lib/reviewVerticalStops.test.ts packages/hunk/src/ui/diff/diffSectionGeometry.test.ts packages/hunk/src/ui/components/panes/copySelection.test.ts — 97 passed.
• bun run test — 5,146 passed, 13 skipped, 0 failed across both shards.
• bun run test:integration — 266 passed, 1 skipped, 0 failed.
• bun test ./test/review-conformance — 118 passed.
• bun run test:tty-smoke — 11 passed.
• bun run typecheck, bun run deps:check, targeted oxfmt/oxlint, and git diff --check — passed.
• bun run packages/hunk/src/main.tsx diff --help and bun run install:bin — passed.

No intended visual or interaction change and no media attached. The real-PTY regression uses an actual file-pair diff in unified mode, 30 rows, 80/100/120 columns, and verifies saved-note selection/navigation/deletion without hover after resizing; TTY smoke also executed. Tested on Linux only; macOS and Windows were not run.

This PR description was generated by Pi using gpt-6.1-sol

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated
hunk-web	Ignored	Preview	Oct 18, 2026 10:16pm UTC

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Keeps copy-selection rows lazy during keyboard navigation.

• Keyboard navigation leaves copy-selection rows lazy until copying needs them.

No actionable issues found. Review conclusions come from code inspection; no tests were run during this review.

_{Reviews (1) · Last reviewed commit: "fix(ui): keep copy-selection rows lazy d..." · Reviewed by Greptile}

[benvinegar]

Code review found one cache-invalidation issue: measured noteTarget depends on the semantic note ID and draft/source classification, but the existing note cache key omitted those values. Equivalent-height cards could reuse the previous note's keyboard target or leave a draft selectable.

Fixed in 46e145f by including both dependencies in the existing key, with split/unified regression coverage for replacement IDs and saved↔draft transitions. The cache design and bounded slots remain unchanged. The new tests reproduced stale targets before the fix and pass afterward.

No further actionable findings from reviewing the diff, lazy-copy consumers, note-stop ordering, and alternate file-view path. Final local verification passed: 97 focused tests; 5,146 full-suite tests (13 skipped); 266 PTY tests (1 skipped); 118 review-conformance tests; 11 real-TTY smoke tests; typecheck; dependency checks; and targeted lint/format checks. Waiting for the final revision's remaining CI checks before merging.

This comment was generated by Pi using gpt-6.1-sol

[squad-robert]

Independent post-merge check: the eager copy-row materialization reproduces on base, and this change removes it and reduces retained JS heap in a real CLI/navigation workload. Tested exact head 46e145f87b36d1996c74c15eb7f20bf8a67e173c against base 4b1594692ff67e5680ab7cfe72573d39177ff031, Bun 1.4.2/Linux x64, identical locked dependencies.

Real CLI/PTY measurements: generated disposable Git repositories with 120-line TypeScript files, changing lines 37–84 in each file. Ran source diff --mode split --no-sidebar at 240×40, waited for the actual first file to render, then made ten ] hunk jumps, waiting for the expected file's source text after each jump. A small explicit observer extension sampled bun:jsc heap stats after two full GCs separated by an event-loop turn. No production instrumentation, reload loop, or resize loop.

Workload/sample	Base JS heap	Head JS heap
180 files, initial frame, median of 3 fresh processes each	51.8 MiB	44.1 MiB
180 files, after ten hunk jumps, median of those 3 runs	60.8 MiB	53.2 MiB (12.5% lower)
1,000 files, after ten hunk jumps, one fresh pair	175.9 MiB	133.2 MiB
180 files, unified mode after ten jumps, one fresh pair	70.9 MiB	60.4 MiB

Execution order alternated for the three main pairs; processes ran serially. RSS was noisy, so it's not the success criterion. Captured initial/post-navigation terminal frames also matched between base/head. This is a different fixture from the PR's 50,000-line giant-file workload, not a reproduction of its exact benchmark numbers or proof of universally faster interactions.

Independent projection/copy check: across 24 cases (split/unified × wrapped/unwrapped × widths 80/160/240 × notes/no notes), used two Unicode-containing files with semantic roots/replies, drafts, and decoration-only cards. A forwarding getter counted access to each geometry's plannedRows without changing its result. Navigation reads it for both files on base, neither on head. Actual navigation-stop arrays and rendered copy-selection text match exactly across revisions in all 24 cases; root/reply order and exclusion of drafts/decorations were asserted. Copy assertions require both files' source text and Unicode to survive, so empty output can't pass. This verifies the copy projection, not an operating-system clipboard round trip or first-copy latency.

Corroboration: copied the head navigation test file only into the isolated base archive; identical note-free getter-trap regressions give 2 failures on base / 2 passes on head. Head focused navigation/geometry/copy tests 97 pass, real-PTY notes tests 32 pass (including the resized saved-note case), review conformance 118 pass, and typecheck passes.

No actionable regression found within these checks. Full repository/PTY suites, macOS/Windows, the exact giant-file measurements, and first-copy timing were not independently tested. Deferred copy work is still a tradeoff, not eliminated work. No claim of an unbounded leak or its resolution here.

This comment was generated by Pi using GPT-6.1 Sol