feat(review): make inline notes keyboard selectable

.changeset/bright-notes-navigate.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": minor
+---
+
+Add visible keyboard selection for review notes. Line-by-line movement now stops on every comment and reply in rendered order; saving or clicking a note makes it active, with next/previous note navigation and persistent edit, reply, and delete shortcuts on the active note.

docs/keybindings.md

@@ -115,7 +115,8 @@ Review and shared commands:
 | `hunk.review.alignCurrentLineTop`              | Align current line to viewport top             | _(none)_                     |
 | `hunk.review.clearSelection`                   | Clear the active visual selection              | _(none)_                     |
 | `hunk.review.copySelection`                    | Copy the active visual selection               | `y`                          |
-| `hunk.review.editActiveNote`                   | Edit the active review note                    | `E`                          |
+| `hunk.review.deleteActiveNote`                 | Delete active review note                      | `D`                          |
+| `hunk.review.editActiveNote`                   | Edit active review note                        | `E`                          |
 | `hunk.review.editSelectedFile`                 | Open the selected file in your editor          | `e`                          |
 | `hunk.review.focusFilter`                      | Focus the file filter                          | `/`                          |
 | `hunk.review.halfPageDown`                     | Scroll down half a page                        | `d`, `ctrl+d`                |
@@ -126,19 +127,21 @@ Review and shared commands:
 | `hunk.review.nextAnnotatedHunk`                | Next annotated hunk                            | `}`                          |
 | `hunk.review.nextFile`                         | Next file                                      | `.`                          |
 | `hunk.review.nextHunk`                         | Next hunk                                      | `]`                          |
+| `hunk.review.nextNote`                         | Next review note                               | `n`                          |
 | `hunk.review.pageDown`                         | Scroll down one page                           | `pagedown`, `space`, `f`     |
 | `hunk.review.pageUp`                           | Scroll up one page                             | `pageup`, `b`, `shift+space` |
 | `hunk.review.previousAnnotatedFile`            | Previous annotated file                        | _(none)_                     |
 | `hunk.review.previousAnnotatedHunk`            | Previous annotated hunk                        | `{`                          |
 | `hunk.review.previousFile`                     | Previous file                                  | `,`                          |
 | `hunk.review.previousHunk`                     | Previous hunk                                  | `[`                          |
-| `hunk.review.replyToActiveNote`                | Reply to the active review note                | `R`                          |
+| `hunk.review.previousNote`                     | Previous review note                           | `N`                          |
+| `hunk.review.replyToActiveNote`                | Reply to active review note                    | `R`                          |
 | `hunk.review.scrollCodeLeft`                   | Scroll code left (shifted scrolls fast)        | `left`, `shift+left`         |
 | `hunk.review.scrollCodeRight`                  | Scroll code right (shifted scrolls fast)       | `right`, `shift+right`       |
 | `hunk.review.startNote`                        | Add a review note                              | `c`                          |
 | `hunk.review.startVisualSelection`             | Start visual line selection                    | `v`                          |
-| `hunk.review.stepDown`                         | Scroll down one row                            | `down`, `j`                  |
-| `hunk.review.stepUp`                           | Scroll up one row                              | `up`, `k`                    |
+| `hunk.review.stepDown`                         | Move down one line or note                     | `down`, `j`                  |
+| `hunk.review.stepUp`                           | Move up one line or note                       | `up`, `k`                    |
 | `hunk.review.toggleHunkGap`                    | Expand or collapse the selected context        | `z`                          |
 | `hunk.view.applyFilePresentationToAllMatching` | Apply current file presentation to all matches | _(none)_                     |
 | `hunk.view.cursorLineNumber`                   | Mark the current line number                   | _(none)_                     |

packages/hunk/src/core/review/actions.ts

@@ -23,6 +23,8 @@ export type ReviewAction =
       hunkIndex: number;
       /** Omitted by state reconciliation, which must not disturb the reviewer's viewport. */
       reveal?: ReviewRevealRequest;
+      /** Set only by exact-note navigation; every other selection clears explicit note focus. */
+      activeNoteId?: string;
     }
   | { type: "filter/set"; filter: string }
   | { type: "notes/set-visibility"; visible: boolean }

packages/hunk/src/core/review/intents.test.ts

@@ -122,6 +122,76 @@ describe("selection movement intent", () => {
     });
   });
 
+  test("moves between exact visible stored notes and carries the active identity atomically", () => {
+    const state = {
+      ...createTestReviewState(undefined, { showAgentNotes: true }),
+      liveNotes: [
+        createTestStoredNote({ id: "later", fileKey: "alpha", hunkIndex: 0, line: 8 }),
+        createTestStoredNote({ id: "next-file", fileKey: "beta", hunkIndex: 1, line: 21 }),
+        createTestStoredNote({ id: "earlier", fileKey: "alpha", hunkIndex: 0, line: 2 }),
+      ],
+    };
+
+    expect(planReviewIntent(state, { type: "selection/move", scope: "note", delta: 1 })).toEqual({
+      actions: [
+        {
+          type: "selection/select",
+          fileKey: "alpha",
+          hunkIndex: 0,
+          activeNoteId: "earlier",
+          reveal: { anchor: "hunk", scrollToNote: true },
+        },
+      ],
+      outcome: { type: "selection/changed", fileKey: "alpha", hunkIndex: 0 },
+    });
+
+    const activeLater = { ...state, activeNoteId: "later" };
+    expect(
+      planReviewIntent(activeLater, { type: "selection/move", scope: "note", delta: 1 }).actions[0],
+    ).toMatchObject({ fileKey: "beta", hunkIndex: 1, activeNoteId: "next-file" });
+    expect(
+      planReviewIntent(
+        { ...activeLater, filter: "alpha" },
+        { type: "selection/move", scope: "note", delta: 1 },
+      ),
+    ).toEqual({ actions: [] });
+  });
+
+  test("selects one exact visible note and rejects a mismatched location", () => {
+    const state = {
+      ...createTestReviewState(undefined, { showAgentNotes: true }),
+      liveNotes: [createTestStoredNote({ id: "target", fileKey: "alpha", hunkIndex: 0 })],
+    };
+    const reveal = { anchor: "none" as const, scrollToNote: false };
+
+    expect(
+      planReviewIntent(state, {
+        type: "selection/select",
+        fileKey: "alpha",
+        hunkIndex: 0,
+        activeNoteId: "target",
+        reveal,
+      }).actions,
+    ).toEqual([
+      {
+        type: "selection/select",
+        fileKey: "alpha",
+        hunkIndex: 0,
+        activeNoteId: "target",
+        reveal,
+      },
+    ]);
+    expect(() =>
+      planReviewIntent(state, {
+        type: "selection/select",
+        fileKey: "beta",
+        hunkIndex: 0,
+        activeNoteId: "target",
+        reveal,
+      }),
+    ).toThrow(ReviewIntentPlanningError);
+  });
+
   test("requires the annotation index for annotated navigation", () => {
     const state = createTestReviewState();
 
@@ -215,6 +285,22 @@ describe("viewport anchor intent", () => {
     expect(next.selection).toEqual({ fileKey: "beta", hunkIndex: 1 });
     expect(next.reveal).toEqual({ fileTopToken: 0, hunkToken: 0, scrollToNote: false });
   });
+
+  test("preserves exact note focus when the viewport reports the same hunk", () => {
+    const state = {
+      ...createTestReviewState(),
+      activeNoteId: "note-2",
+      selection: { fileKey: "alpha", hunkIndex: 1 },
+    };
+
+    expect(
+      planReviewIntent(state, {
+        type: "selection/anchor",
+        fileKey: "alpha",
+        hunkIndex: 1,
+      }).actions[0],
+    ).toMatchObject({ activeNoteId: "note-2" });
+  });
 });
 
 describe("user note creation", () => {

packages/hunk/src/core/review/intents.ts

@@ -31,6 +31,8 @@ import {
   isReviewNoteWithinClearScope,
   reviewNoteCurrentOwnerHunkIndex,
   reviewNoteHasDescendants,
+  selectActiveStoredReviewNote,
+  selectNavigableStoredReviewNotes,
   selectNormalizedSelection,
   selectReviewFileByKey,
   selectReviewNavigationFiles,
@@ -86,7 +88,14 @@ export interface ReviewIntentFacts {
 
 export type ReviewIntent =
   /** Select one hunk outright, revealing it the way the caller asks. */
-  | { type: "selection/select"; fileKey: string; hunkIndex: number; reveal: ReviewRevealRequest }
+  | {
+      type: "selection/select";
+      fileKey: string;
+      hunkIndex: number;
+      reveal: ReviewRevealRequest;
+      /** Exact visible stored note to focus instead of the selected hunk's source line. */
+      activeNoteId?: string;
+    }
   /** Step the selection through one navigable scope; the scope decides wrap and reveal. */
   | { type: "selection/move"; scope: ReviewSelectionScope; delta: number }
   /** Jump to one file, landing on its first hunk. */
@@ -344,9 +353,18 @@ function planSelection(
   fileKey: string,
   hunkIndex: number,
   reveal: ReviewRevealRequest,
+  activeNoteId?: string,
 ): ReviewIntentPlan {
   return {
-    actions: [{ type: "selection/select", fileKey, hunkIndex, reveal }],
+    actions: [
+      {
+        type: "selection/select",
+        fileKey,
+        hunkIndex,
+        reveal,
+        ...(activeNoteId ? { activeNoteId } : {}),
+      },
+    ],
     outcome: { type: "selection/changed", fileKey, hunkIndex },
   };
 }
@@ -375,13 +393,21 @@ function planSelectionMove(
     {
       files: selectReviewNavigationFiles(state),
       annotations: facts.annotations ?? EMPTY_REVIEW_ANNOTATION_INDEX,
+      notes: selectNavigableStoredReviewNotes(state).map((item) => ({
+        fileKey: item.fileKey,
+        hunkIndex: item.hunkIndex,
+        noteId: item.entry.note.id,
+      })),
+      activeNoteId: selectActiveStoredReviewNote(state)?.note.id,
     },
     selectNormalizedSelection(state),
     { scope: intent.scope, delta: intent.delta },
   );
   // A refused move publishes nothing at all: no selection change, and no reveal token
   // bump that would scroll a viewport for a key press that went nowhere.
-  return target ? planSelection(target.fileKey, target.hunkIndex, target.reveal) : { actions: [] };
+  return target
+    ? planSelection(target.fileKey, target.hunkIndex, target.reveal, target.activeNoteId)
+    : { actions: [] };
 }
 
 /**
@@ -814,13 +840,25 @@ export function planReviewIntent(
       // Only the file is required: an out-of-range hunk clamps rather than rejecting, so
       // a stale index from a reloaded file still lands the reviewer somewhere real.
       const file = requireReviewFile(state, intent.fileKey);
+      if (intent.activeNoteId) {
+        const target = selectNavigableStoredReviewNotes(state).find(
+          (item) => item.entry.note.id === intent.activeNoteId,
+        );
+        if (!target || target.fileKey !== file.key || target.hunkIndex !== intent.hunkIndex) {
+          throw new ReviewIntentPlanningError(
+            "note-not-found",
+            `No visible review note matches id ${intent.activeNoteId} at the requested selection.`,
+          );
+        }
+      }
       return {
         actions: [
           {
             type: "selection/select",
             fileKey: file.key,
             hunkIndex: intent.hunkIndex,
             reveal: intent.reveal,
+            ...(intent.activeNoteId ? { activeNoteId: intent.activeNoteId } : {}),
           },
         ],
       };
@@ -839,13 +877,23 @@ export function planReviewIntent(
     }
     case "selection/anchor": {
       const file = requireReviewFile(state, intent.fileKey);
+      const anchoredHunkIndex = Math.min(
+        Math.max(intent.hunkIndex, 0),
+        Math.max(0, file.hunks.length - 1),
+      );
+      const current = selectNormalizedSelection(state);
+      const preservesActiveNote =
+        current.fileKey === file.key && current.hunkIndex === anchoredHunkIndex;
       return {
         actions: [
           {
             type: "selection/select",
             fileKey: file.key,
             hunkIndex: intent.hunkIndex,
             reveal: REVIEW_VIEWPORT_ANCHOR_REVEAL,
+            ...(preservesActiveNote && state.activeNoteId
+              ? { activeNoteId: state.activeNoteId }
+              : {}),
           },
         ],
       };

packages/hunk/src/core/review/navigation.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, test } from "bun:test";
 import {
   EMPTY_REVIEW_ANNOTATION_INDEX,
+  planReviewNoteMove,
   planReviewSelectionMove,
   REVIEW_SELECTION_WRAP_POLICY,
   reviewAnnotatedCursors,
@@ -59,6 +60,7 @@ describe("review selection movement", () => {
     expect(REVIEW_SELECTION_WRAP_POLICY).toEqual({
       hunk: "clamp",
       file: "clamp",
+      note: "clamp",
       "annotated-hunk": "clamp",
       "annotated-file": "wrap",
     });
@@ -103,6 +105,34 @@ describe("review selection movement", () => {
     });
   });
 
+  test("moves exact notes in order and refuses to reselect at an edge", () => {
+    const notes = [
+      { noteId: "one", fileKey: "alpha", hunkIndex: 0 },
+      { noteId: "two", fileKey: "alpha", hunkIndex: 0 },
+      { noteId: "three", fileKey: "beta", hunkIndex: 1 },
+    ];
+
+    expect(planReviewNoteMove(FILES, notes, at("alpha", 0), "one", 1)).toEqual({
+      fileKey: "alpha",
+      hunkIndex: 0,
+      activeNoteId: "two",
+      reveal: { anchor: "hunk", scrollToNote: true },
+    });
+    expect(planReviewNoteMove(FILES, notes, at("alpha", 0), "two", 1)?.activeNoteId).toBe("three");
+    expect(planReviewNoteMove(FILES, notes, at("beta", 1), "three", 1)).toBeNull();
+    expect(planReviewNoteMove(FILES, notes, at("alpha", 0), "one", -1)).toBeNull();
+    expect(planReviewNoteMove(FILES, notes, at("beta", 0), undefined, 1)?.activeNoteId).toBe(
+      "three",
+    );
+    expect(planReviewNoteMove(FILES, notes, at("beta", 0), undefined, -1)?.activeNoteId).toBe(
+      "two",
+    );
+    expect(move({ ...model(), notes, activeNoteId: "two" }, at("alpha", 0), "note", 1)).toEqual({
+      at: "beta:1",
+      reveal: { anchor: "hunk", scrollToNote: true },
+    });
+  });
+
   test("steps files onto their first hunk and refuses a move that would go nowhere", () => {
     expect(move(model(), at("alpha", 1), "file", 1)).toEqual({
       at: "beta:0",