fix(ui): stop wheel scroll and stale hover from drifting comment targets off the highlighted line

.changeset/wheel-roundtrip-comment-target.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+Fix stale comment targets after scrolling or stepping the cursor.

packages/hunk/src/ui/App.tsx

@@ -1261,6 +1261,13 @@ export function App({
   const activeReplyableNoteId = selectActiveReplyableReviewNoteId(reviewSnapshot);
   const activeRemovableNote = selectActiveRemovableReviewNote(reviewSnapshot);
 
+  // The current line owns implicit comment targeting. A hover affordance armed at an
+  // earlier pointer position retires whenever the cursor moves, so stepping or scrolling
+  // cannot leave a stale hover target between the visible highlight and the next note.
+  useEffect(() => {
+    onActiveAddNoteAffordanceChange(null);
+  }, [onActiveAddNoteAffordanceChange, review.lineCursor]);
+
   // One dispatch table for every app-level shortcut: the built-in commands
   // over App's live callbacks, then extension commands, so built-ins always
   // win a key and extension order follows load order.

packages/hunk/src/ui/AppHost.interactions.test.tsx

@@ -3039,6 +3039,64 @@ describe("App interactions", () => {
     }
   });
 
+  test("stepping the cursor retires a stale hover before the next draft opens", async () => {
+    const setup = await testRender(
+      <AppHost bootstrap={createLineScrollBootstrap(false, "unified")} />,
+      {
+        width: 120,
+        height: 26,
+      },
+    );
+
+    try {
+      await flush(setup);
+      await act(async () => {
+        await Bun.sleep(60);
+        await setup.renderOnce();
+      });
+
+      // Hover the row showing line02 so its add-note affordance arms.
+      const frame = setup.captureCharFrame();
+      const hover = frame
+        .split("\n")
+        .map((line, index) => [index, line] as const)
+        .find(([, line]) => line.includes("export const line02 = 2;"));
+      expect(hover).toBeDefined();
+      await act(async () => {
+        await setup.mockMouse.moveTo(10, hover![0]);
+        await setup.renderOnce();
+      });
+      await waitForFrame(setup, (next) => next.includes("[+]"));
+
+      // Step the cursor down several lines; the pointer stays where it was.
+      await act(async () => {
+        await setup.mockInput.pressKeys(["\x1b[B", "\x1b[B", "\x1b[B"]);
+      });
+      await flush(setup);
+      await act(async () => {
+        await Bun.sleep(60);
+        await setup.renderOnce();
+      });
+
+      // The draft must anchor at the stepped cursor, not the stale hover row.
+      await act(async () => {
+        await setup.mockInput.pressKey("c");
+      });
+      await flush(setup);
+      await act(async () => {
+        await Bun.sleep(60);
+        await setup.renderOnce();
+      });
+
+      const withDraft = setup.captureCharFrame();
+      expect(withDraft).toMatch(/Draft note.*L4/);
+    } finally {
+      await act(async () => {
+        setup.renderer.destroy();
+      });
+    }
+  });
+
   test("draft note focus suppresses app shortcuts while accepting typed shortcut keys", async () => {
     const setup = await testRender(<AppHost bootstrap={createBootstrap()} />, {
       width: 240,

packages/hunk/src/ui/components/panes/DiffPane.tsx

@@ -884,6 +884,12 @@ export function DiffPane({
   // Initialized to null so the first render never fires a selection change; a real scroll
   // is required before passive viewport-follow selection can trigger.
   const lastViewportSelectionTopRef = useRef<number | null>(null);
+  // Stops that successive clamps moved the current line away from, innermost last, so a
+  // multi-hop wheel roundtrip can unwind back to the pre-clamp stop.
+  const lineCursorBeforeClampRef = useRef<LineCursor[]>([]);
+  // The cursor this effect last acted on, so deliberate cursor moves outside a clamp reset
+  // the unwind memory above instead of leaving a displaced stop behind.
+  const lastClampCursorRef = useRef<LineCursor | null>(null);
   const lastViewportRowAnchorRef = useRef<ViewportRowAnchor | null>(null);
   // Track the previous selected anchor to detect actual selection changes.
   const prevSelectedAnchorIdRef = useRef<string | null>(null);
@@ -1618,15 +1624,33 @@ export function DiffPane({
       return;
     }
 
+    // A deliberate cursor move (a click, a step) ends any clamp unwind: the reviewer chose
+    // a new stop, so a later scroll must not restore the displaced one.
+    if (lineCursor !== lastClampCursorRef.current) {
+      lineCursorBeforeClampRef.current = [];
+      lastClampCursorRef.current = lineCursor;
+    }
+
     if (lineCursors.length > 0) {
+      // Each clamp away from the current stop pushes where it came from, and each restore
+      // pops it back, so a multi-hop wheel roundtrip unwinds to the pre-clamp stop instead
+      // of parking on an edge the scroll crossed.
+      const clampedFrom = lineCursorBeforeClampRef.current.at(-1) ?? null;
       const clampedCursor = clampLineCursorToViewport({
         boundsOf: lineCursorBoundsOf,
         current: lineCursor,
+        clampedFrom,
         cursors: lineCursors,
         scrollTop: scrollViewport.top,
         viewportHeight: scrollViewport.height,
       });
       if (clampedCursor && clampedCursor !== lineCursor) {
+        if (clampedCursor === clampedFrom) {
+          lineCursorBeforeClampRef.current.pop();
+        } else if (lineCursor) {
+          lineCursorBeforeClampRef.current.push(lineCursor);
+        }
+        lastClampCursorRef.current = clampedCursor;
         onViewportLineCursorChange?.(clampedCursor);
       }
       return;

packages/hunk/src/ui/components/panes/copySelection.test.ts

@@ -996,16 +996,33 @@ describe("copySelectionDragIsClick", () => {
     visualRow,
   });
 
-  test("accepts one-cell mouse jitter around a click", () => {
+  test("accepts one-cell mouse jitter within the clicked row", () => {
     expect(
       copySelectionDragIsClick({
         anchor: point(20, 8),
-        focus: point(21, 9),
+        focus: point(21, 8),
         moved: true,
       }),
     ).toBe(true);
   });
 
+  test("treats a drag onto a second visual row as a selection, not a jittered click", () => {
+    expect(
+      copySelectionDragIsClick({
+        anchor: point(20, 8),
+        focus: point(21, 9),
+        moved: true,
+      }),
+    ).toBe(false);
+    expect(
+      copySelectionDragIsClick({
+        anchor: point(20, 8),
+        focus: point(20, 9),
+        moved: true,
+      }),
+    ).toBe(false);
+  });
+
   test("rejects deliberate drags and double-click expansion", () => {
     expect(
       copySelectionDragIsClick({

packages/hunk/src/ui/components/panes/copySelection.ts

@@ -169,7 +169,13 @@ export function findLineCursorForClick({
 
 const COPY_SELECTION_CLICK_SLOP_CELLS = 1;
 
-/** Treat one-cell pointer jitter as a click while preserving deliberate and expanded drags. */
+/**
+ * Treat pointer jitter around a click as a click while preserving deliberate drags.
+ *
+ * Jitter slop is horizontal only: a drag that reaches a different visual row is a
+ * deliberate selection, so a two-row range commits instead of being swallowed as a
+ * jittered click. Double-click expansion always copies.
+ */
 export function copySelectionDragIsClick(drag: CopySelectionDrag) {
   if (drag.expanded) {
     return false;
@@ -197,7 +203,7 @@ export function copySelectionDragIsClick(drag: CopySelectionDrag) {
   return (
     drag.anchor.kind === "review-row" &&
     drag.focus.kind === "review-row" &&
-    Math.abs(drag.anchor.visualRow - drag.focus.visualRow) <= COPY_SELECTION_CLICK_SLOP_CELLS
+    drag.anchor.visualRow === drag.focus.visualRow
   );
 }
 

packages/hunk/src/ui/lib/lineCursors.test.ts

@@ -491,4 +491,30 @@ describe("clampLineCursorToViewport", () => {
       }),
     ).toEqual(cursors[0]!);
   });
+
+  test("restores the remembered stop once the viewport returns to it", () => {
+    expect(
+      clampLineCursorToViewport({
+        boundsOf,
+        current: cursors[2]!,
+        clampedFrom: cursors[0]!,
+        cursors,
+        scrollTop: 0,
+        viewportHeight: 3,
+      }),
+    ).toEqual(cursors[0]!);
+  });
+
+  test("keeps the clamped stop while the remembered one is still off-screen", () => {
+    expect(
+      clampLineCursorToViewport({
+        boundsOf,
+        current: cursors[2]!,
+        clampedFrom: cursors[0]!,
+        cursors,
+        scrollTop: 2,
+        viewportHeight: 2,
+      }),
+    ).toEqual(cursors[2]!);
+  });
 });

packages/hunk/src/ui/lib/lineCursors.ts

@@ -302,18 +302,41 @@ export function clampLineCursorToViewport({
   cursors,
   scrollTop,
   viewportHeight,
+  clampedFrom,
 }: {
   boundsOf: LineCursorBoundsLookup;
   current: LineCursor | null;
   cursors: LineCursor[];
   scrollTop: number;
   viewportHeight: number;
+  /**
+   * The stop the current line was on before the clamp moved it, when the caller knows it.
+   *
+   * Wheel roundtrips move the viewport past a line and back; without this memory the clamp
+   * re-adopts an edge stop on the way out and then keeps it on the way home, so the comment
+   * target ends up a few lines (or one gap) away from the line the reviewer is reading.
+   */
+  clampedFrom?: LineCursor | null;
 }): LineCursor | null {
   if (cursors.length === 0 || viewportHeight <= 0) {
     return current;
   }
 
   const viewportBottom = scrollTop + viewportHeight;
+
+  // A return journey restores the stop the last clamp moved away from, when that stop is
+  // visible again and the viewport only moves while the pointer scrolls.
+  if (clampedFrom) {
+    const fromBounds = boundsOf(clampedFrom);
+    if (
+      fromBounds &&
+      fromBounds.top >= scrollTop &&
+      fromBounds.top + fromBounds.height <= viewportBottom
+    ) {
+      return clampedFrom;
+    }
+  }
+
   const currentBounds = current ? boundsOf(current) : undefined;
   if (
     currentBounds &&