fix(ui): preserve scroll position across comment add/edit/delete

CHANGELOG.md

@@ -8,6 +8,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
 
 ## [Unreleased]
 
+### Fixed
+
+- Preserve scroll position when a comment is added, edited, or deleted (and when `.dunk/comments.json` reloads externally). The review pane re-anchors on a surviving diff row so inserting or removing inline comment cards no longer pushes the code you were reading out from under the viewport.
+
 ## [0.12.1] - 2026-05-11
 
 ### Fixed

src/core/jj.test.ts

@@ -1,9 +1,13 @@
-import { afterEach, describe, expect, test } from "bun:test";
+import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test";
 import { mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { buildJjDiffArgs, runJjText } from "./jj";
 
+// jj sub-process spawns are dramatically slower on Windows (~70× the Linux time observed in CI),
+// so give the suite generous headroom to absorb that overhead without flaking.
+setDefaultTimeout(30_000);
+
 const tempDirs: string[] = [];
 const realJjTest = Bun.which("jj") ? test : test.skip;
 

src/ui/components/panes/DiffPane.tsx

@@ -879,39 +879,71 @@ export function DiffPane({
     const layoutChanged = previousLayoutRef.current !== layout;
     const explicitLayoutToggle = previousLayoutToggleRequestIdRef.current !== layoutToggleRequestId;
     const wrapChanged = previousWrapLinesRef.current !== wrapLines;
+    const filesChanged = previousFilesRef.current !== files;
     const previousSectionMetrics = previousSectionGeometryRef.current;
     const previousFiles = previousFilesRef.current;
+    const shouldRestoreViewport = layoutChanged || wrapChanged || filesChanged;
 
-    if ((layoutChanged || wrapChanged) && previousSectionMetrics && previousFiles.length > 0) {
+    if (shouldRestoreViewport && previousSectionMetrics && previousFiles.length > 0) {
       const previousSectionHeaderHeights = buildInStreamFileHeaderHeights(previousFiles);
       const previousScrollTop =
         // Prefer the synchronously captured pre-toggle position so anchor restoration does not
-        // race the polling-based viewport snapshot.
+        // race the polling-based viewport snapshot. For files-only changes (comment add/edit/delete
+        // and external `.dunk/comments.json` reloads) the live scrollTop is still the pre-swap
+        // position because OpenTUI does not auto-reset scrollTop when content height changes.
         wrapChanged && wrapToggleScrollTop != null
           ? wrapToggleScrollTop
           : layoutChanged && explicitLayoutToggle && layoutToggleScrollTop != null
             ? layoutToggleScrollTop
             : (scrollRef.current?.scrollTop ??
               Math.max(prevScrollTopRef.current, scrollViewport.top));
-      const anchor = findViewportRowAnchor(
-        previousFiles,
-        previousSectionMetrics,
-        previousScrollTop,
-        previousSectionHeaderHeights,
-        lastViewportRowAnchorRef.current?.stableKey,
-      );
-      if (anchor) {
-        const nextTop = resolveViewportRowAnchorTop(
-          files,
-          sectionGeometry,
-          anchor,
-          sectionHeaderHeights,
+      // Resolve a viewport anchor using `previousScrollTop` against the pre-swap geometry, then
+      // translate it into the post-swap geometry. Returns null when no row in the new geometry
+      // covers the captured anchor (e.g. the only comment in a file was deleted).
+      const resolveStrategy = (options?: { preferDiffRows?: boolean }) => {
+        const candidate = findViewportRowAnchor(
+          previousFiles,
+          previousSectionMetrics,
+          previousScrollTop,
+          previousSectionHeaderHeights,
+          lastViewportRowAnchorRef.current?.stableKey,
+          options,
         );
+        const top = candidate
+          ? resolveViewportRowAnchorTop(files, sectionGeometry, candidate, sectionHeaderHeights)
+          : null;
+        return { anchor: candidate, top };
+      };
+      // Prefer the natural row anchor first. Inline-note stable keys are note-id based, so a
+      // comment-edit reload still resolves cleanly and keeps the reader's position inside the
+      // card. Only when the natural anchor cannot resolve in the new geometry do we fall back
+      // to a survivable diff row — that path handles deletes, where the inline-note row is gone.
+      const natural = resolveStrategy();
+      const filesOnlyChange = filesChanged && !layoutChanged && !wrapChanged;
+      const diffRowFallback =
+        natural.top === null && filesOnlyChange ? resolveStrategy({ preferDiffRows: true }) : null;
+      const anchor = natural.top !== null ? natural.anchor : diffRowFallback?.anchor;
+      const resolvedTop = natural.top !== null ? natural.top : (diffRowFallback?.top ?? null);
+      // When the anchor row vanishes (rare: e.g. the only comment in a file was deleted from a
+      // viewport sitting on it, or a filter removed the anchored file), fall back to clamping the
+      // pre-mutation scrollTop into the new content extent so the user does not snap to file top.
+      const fallbackTop =
+        resolvedTop === null && filesChanged
+          ? clampReviewScrollTop(
+              previousScrollTop,
+              Math.max(scrollViewport.height, scrollRef.current?.viewport.height ?? 0),
+            )
+          : null;
+      const targetTop = resolvedTop ?? fallbackTop;
+
+      if (targetTop !== null) {
         const restoreViewportAnchor = () => {
-          scrollRef.current?.scrollTo(nextTop);
+          scrollRef.current?.scrollTo(targetTop);
         };
 
-        lastViewportRowAnchorRef.current = anchor;
+        if (anchor && resolvedTop !== null) {
+          lastViewportRowAnchorRef.current = anchor;
+        }
         suppressViewportSelectionSync();
         restoreViewportAnchor();
         // Retry across a couple of repaint cycles so the restored top-row anchor sticks
@@ -937,11 +969,13 @@ export function DiffPane({
     previousSectionGeometryRef.current = sectionGeometry;
     previousFilesRef.current = files;
   }, [
+    clampReviewScrollTop,
     files,
     layout,
     layoutToggleRequestId,
     layoutToggleScrollTop,
     scrollRef,
+    scrollViewport.height,
     scrollViewport.top,
     sectionGeometry,
     sectionHeaderHeights,

src/ui/components/ui-components.test.tsx

@@ -540,7 +540,10 @@ describe("UI components", () => {
 
     try {
       await settleDiffPane(setup);
-      const frame = setup.captureCharFrame();
+      const frame = await waitForFrame(
+        setup,
+        (text) => text.includes("11 - export const line11 = 11;") && !text.includes("intro.ts"),
+      );
 
       expect(frame).toContain("11 - export const line11 = 11;");
       expect(frame).toContain("11 + export const line11 = 1100;");
@@ -907,7 +910,12 @@ describe("UI components", () => {
 
     try {
       await settleDiffPane(setup);
-      const frame = setup.captureCharFrame();
+      const frame = await waitForFrame(
+        setup,
+        (text) =>
+          text.includes("16 - export const line16 = 16;") &&
+          !text.includes("2 - export const line2 = 2;"),
+      );
 
       expect(frame).toContain("export const line11 = 11;");
       expect(frame).toContain("14 - export const line14 = 14;");
@@ -946,7 +954,12 @@ describe("UI components", () => {
 
     try {
       await settleDiffPane(setup);
-      const frame = setup.captureCharFrame();
+      const frame = await waitForFrame(
+        setup,
+        (text) =>
+          text.includes("18   export const line18 = 18;") &&
+          !text.includes("2 - export const line2 = 2;"),
+      );
 
       expect(frame).toContain("11   export const line11 = 11;");
       expect(frame).toContain("14 + export const line14 = 'this is a");
@@ -1014,7 +1027,9 @@ describe("UI components", () => {
 
     try {
       await settleDiffPane(setup);
-      const frame = setup.captureCharFrame();
+      const frame = await waitForFrame(setup, (text) =>
+        text.includes("Keep the selected hunk visible with its note."),
+      );
 
       // The minimal note card (left accent + title + body, no box border) keeps
       // the selected hunk and its inline comment visible together.
@@ -1080,7 +1095,7 @@ describe("UI components", () => {
 
     try {
       await settleDiffPane(setupWithout);
-      const frameWithout = setupWithout.captureCharFrame();
+      const frameWithout = await waitForFrame(setupWithout, (text) => text.includes("line57"));
 
       // dunk context (lines near 57-59) should be visible at the top.
       expect(frameWithout).toContain("line57");
@@ -1110,7 +1125,9 @@ describe("UI components", () => {
 
     try {
       await settleDiffPane(setupWith);
-      const frameWith = setupWith.captureCharFrame();
+      const frameWith = await waitForFrame(setupWith, (text) =>
+        text.includes("Note anchored on second hunk."),
+      );
 
       // Note should be visible.
       expect(frameWith).toContain("Note anchored on second hunk.");

src/ui/diff/rowWindowing.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test";
-import type { DiffSectionGeometry } from "../lib/diffSectionGeometry";
+import type { DiffSectionGeometry, DiffSectionRowBounds } from "../lib/diffSectionGeometry";
 import type { PlannedReviewRow } from "./reviewRenderPlan";
 import { resolveVisiblePlannedRowWindow } from "./rowWindowing";
 
@@ -26,10 +26,11 @@ function createTestSectionGeometry(
   rowBounds: Array<{ key: string; top: number; height: number }>,
   bodyHeight: number,
 ): DiffSectionGeometry {
-  const normalizedRowBounds = rowBounds.map((row) => ({
+  const normalizedRowBounds: DiffSectionRowBounds[] = rowBounds.map((row) => ({
     ...row,
     stableKey: row.key,
     stableKeys: [row.key],
+    kind: "diff-row",
   }));
 
   return {

src/ui/lib/diffSectionGeometry.ts

@@ -14,6 +14,8 @@ export interface DiffSectionRowBounds extends VerticalBounds {
   key: string;
   stableKey: string;
   stableKeys: string[];
+  /** Distinguishes content rows from inline comment cards so anchor pickers can prefer survivable rows. */
+  kind: PlannedReviewRow["kind"];
 }
 
 /** Cached placeholder sizing and hunk navigation geometry for one file section. */
@@ -146,10 +148,11 @@ export function measureDiffSectionGeometry(
       row.stableKey,
       ...(row.kind === "diff-row" ? (row.stableAliasKeys ?? []) : []),
     ];
-    const rowBoundsEntry = {
+    const rowBoundsEntry: DiffSectionRowBounds = {
       key: row.key,
       stableKey: row.stableKey,
       stableKeys,
+      kind: row.kind,
       // Record both the starting top and the measured height so callers can translate between
       // scroll positions and stable review-row identities across wrap/layout changes.
       top: bodyHeight,