Skip to content

fix(ui): navigate comments by nearest stream position#5

Merged
amix merged 1 commit into
mainfrom
amix/comment-nav-stream-position
May 16, 2026
Merged

fix(ui): navigate comments by nearest stream position#5
amix merged 1 commit into
mainfrom
amix/comment-nav-stream-position

Conversation

@amix
Copy link
Copy Markdown
Owner

@amix amix commented May 16, 2026

Context

Backport of modem-dev/hunk#315. PR 1 of a series bringing over genuinely-missing upstream fixes (most of hunk's good work is already independently in dunk).

}/{ (next/previous comment) from a hunk that has no comment of its own snapped to the first or last comment in the entire review instead of the nearest one in review-stream order. Root cause: findNextHunkCursor fell back to cursors[0] / cursors[len-1] whenever the current hunk was absent from the annotated subset.

What was changed

  • findNextHunkCursor gains an optional streamCursors arg (defaults to cursors, so existing callers are a behavioral no-op) plus a findNearestCursorIndex helper that locates the current {fileId,hunkIndex} in full review-stream order and picks the nearest annotated cursor in the travel direction. Non-cyclic — clamps to the nearest edge, matching existing hunk navigation.
  • The full hunkCursors stream is threaded into moveToAnnotatedHunk (useReviewController), the only runtime call site. buildReviewStream derives hunkCursors and annotatedHunkCursors from the same visibleFiles atomically, so the stream and the annotated subset are always consistent.
  • Adapted to dunk's signature (precomputed cursorIndex Map) and on-disk comments model — not a copy of upstream.

Expert-reviewed (advice incorporated: added the controller-level regression test, the empty-annotated assertion, and the stream-order invariant comment).

Verification

  • bun run typecheck, bun run lint: clean.
  • hunks.test.ts + useReviewController.test.tsx: 14 pass. New unit cases cover nearest-in-stream selection (both directions), non-cyclic clamp, and empty-annotated → null. New controller-level test exercises moveToAnnotatedHunk from an unannotated middle hunk and asserts it lands on the nearest annotated hunk (not first/last) — this is the regression at the level the behavior lives.
  • No PTY test: the fix is deterministic cursor math with no rendering/timing dimension and there is no existing PTY comment-nav harness; the controller test is the correct "where behavior lives" coverage per CLAUDE.md.

🤖 Generated with Claude Code

`}`/`{` from a hunk with no comment of its own snapped to the first or
last comment in the whole review instead of the nearest one, because
`findNextHunkCursor` fell back to `cursors[0]` / `cursors[len-1]` when
the current hunk was absent from the annotated subset.

Add an optional `streamCursors` arg (defaults to `cursors`, a no-op for
existing callers) and a `findNearestCursorIndex` helper that locates the
current hunk in full review-stream order and picks the nearest annotated
cursor in the travel direction, non-cyclic. Thread the full `hunkCursors`
stream into `moveToAnnotatedHunk` — the only runtime call site.

Backports modem-dev/hunk#315, adapted to dunk's precomputed-index
signature and on-disk comments model.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@amix amix marked this pull request as ready for review May 16, 2026 19:36
@amix amix merged commit 1233f71 into main May 16, 2026
3 of 4 checks passed
@amix amix mentioned this pull request May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant