fix(editor): stop title auto-focus from yanking scroll; restore after layout settles (#266)

Root cause (confirmed via Chrome DevTools on the live app): the reading-position
restore jittered on reload — it landed at the saved spot, jumped to the top, then
back. The jump was NOT a height collapse: the title editor auto-focuses ~300ms
after mount, and TipTap's focus scrolls the focused node into view. Since the
title sits at the top of the page, that yanked window scroll to the top, between
two restore passes.

- title-editor: focus with { scrollIntoView: false }, and skip the title
  auto-focus entirely when a saved reading position will be restored (else the
  caret lands in the off-screen title). hasSavedReadingPosition() is the single
  source of truth for "a position will be restored".
- use-scroll-position: restore once the document height has been STABLE for
  HEIGHT_STABLE_MS and the target is reachable, else clamp only at the timeout —
  so the saved offset lands on the same content instead of drifting via
  scroll-anchoring while images/embeds still stream in. Replaces the reachable-
  or-timeout poll (removed restoreStartRef).
- useScrollRestoreOnSwap keeps both triggers: an early on-mount one (covers the
  offline / collab-never-syncs path — the static cache stays shown) plus a
  post-swap re-assert for the final live layout; restore is idempotent so the two
  never double-scroll.
- Abort restore on genuine scroll intent (wheel/touch/scroll-keys); #hash wins.
- Tests updated for the stable-height logic, hasSavedReadingPosition, and the
  two-trigger wiring (incl. the offline path).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

apps/client/src/features/editor/hooks/use-scroll-position.test.ts

@@ -1,6 +1,9 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { renderHook, act } from "@testing-library/react";
-import { useScrollPosition } from "./use-scroll-position";
+import {
+  useScrollPosition,
+  hasSavedReadingPosition,
+} from "./use-scroll-position";
 
 const KEY_PREFIX = "gitmost:scroll-position:";
 
@@ -90,46 +93,28 @@ describe("useScrollPosition", () => {
     });
     expect(window.sessionStorage.getItem(`${KEY_PREFIX}clob`)).toBe("0");
 
-    // Restore still scrolls to 500 (the captured target), NOT the clobbered 0.
-    // If the capture were moved into an effect (after handlers register), it
-    // would read the clobbered 0 and this assertion would fail.
-    setScrollHeight(2000); // maxScroll = 1200 >= 500
+    // Content is tall enough (maxScroll = 2000 - 800 = 1200 >= 500) and stays
+    // steady, so once the height settles restore scrolls to 500 (the captured
+    // target), NOT the clobbered 0. If the capture were moved into an effect
+    // (after handlers register) it would read the clobbered 0 and this fails.
+    setScrollHeight(2000);
     act(() => {
       result.current.restoreScrollPosition();
     });
+    // Height held steady: after HEIGHT_STABLE_MS (400) the poll settles + scrolls.
+    act(() => {
+      vi.advanceTimersByTime(500);
+    });
     expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
   });
 
-  it("(a3) is idempotent: re-asserting the same target does not scroll again", () => {
-    vi.useFakeTimers();
-    window.sessionStorage.setItem(`${KEY_PREFIX}once`, "500");
-    setScrollHeight(2000); // tall enough to restore synchronously
-
-    const { result } = renderHook(() => useScrollPosition("once"));
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-    expect(window.scrollTo).toHaveBeenCalledTimes(1);
-
-    // Simulate the browser now being at the restored position.
-    setScrollY(500);
-
-    // A second call (e.g. the wiring effect re-running on [showStatic, editor,
-    // restoreScrollPosition]) must NOT scroll again: the redundancy guard sees
-    // the window is already at the target and does nothing.
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-    expect(window.scrollTo).toHaveBeenCalledTimes(1);
-  });
-
   it("(b) does not restore when the URL has a #hash anchor", () => {
     vi.useFakeTimers();
     window.sessionStorage.setItem(`${KEY_PREFIX}p2`, "500");
-    // Content is ALREADY tall enough (maxScroll = 2000 - 800 = 1200 >= 500), so
-    // without the hash guard tryRestore would call scrollTo synchronously on the
-    // first tick. The assertion below therefore genuinely proves the hash guard
-    // short-circuits before any scroll (not just that the poll has not fired).
+    // Content is ALREADY tall enough (maxScroll = 2000 - 800 = 1200 >= 500) and
+    // steady, so without the hash guard the poll would settle and scroll. The
+    // assertion below therefore genuinely proves the hash guard short-circuits
+    // before any timer is even scheduled.
     setScrollHeight(2000);
     window.location.hash = "#some-heading";
 
@@ -142,6 +127,93 @@ describe("useScrollPosition", () => {
     expect(window.scrollTo).not.toHaveBeenCalled();
   });
 
+  it("(c) does nothing when nothing is saved or the saved value is <= 0", () => {
+    vi.useFakeTimers();
+
+    // Nothing saved.
+    const a = renderHook(() => useScrollPosition("nope"));
+    act(() => {
+      a.result.current.restoreScrollPosition();
+      vi.advanceTimersByTime(5000);
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+
+    // Saved value <= 0.
+    window.sessionStorage.setItem(`${KEY_PREFIX}zero`, "0");
+    const b = renderHook(() => useScrollPosition("zero"));
+    act(() => {
+      b.result.current.restoreScrollPosition();
+      vi.advanceTimersByTime(5000);
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+  });
+
+  it("(d) restores once the height has been STABLE for HEIGHT_STABLE_MS and is reachable", () => {
+    vi.useFakeTimers();
+    window.sessionStorage.setItem(`${KEY_PREFIX}p4`, "500");
+    setInnerHeight(800);
+    setScrollHeight(2000); // maxScroll = 1200 >= 500: reachable from the start.
+
+    const { result } = renderHook(() => useScrollPosition("p4"));
+    act(() => {
+      result.current.restoreScrollPosition();
+    });
+
+    // Keep flipping the height across a few 100ms ticks: every change resets the
+    // "stable since" clock, so the layout is never considered settled and restore
+    // must NOT fire yet even though the target is reachable.
+    setScrollHeight(2001);
+    act(() => {
+      vi.advanceTimersByTime(100);
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+
+    setScrollHeight(2000);
+    act(() => {
+      vi.advanceTimersByTime(100);
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+
+    setScrollHeight(2001);
+    act(() => {
+      vi.advanceTimersByTime(100);
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+
+    // Now hold the height steady at 2000 and advance past HEIGHT_STABLE_MS (400):
+    // the layout settles and restore scrolls once to the saved offset.
+    setScrollHeight(2000);
+    act(() => {
+      vi.advanceTimersByTime(500);
+    });
+    expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
+  });
+
+  it("(d2) settled-but-unreachable does NOT restore until the timeout, then clamps", () => {
+    vi.useFakeTimers();
+    window.sessionStorage.setItem(`${KEY_PREFIX}p5`, "5000");
+    setInnerHeight(800);
+    setScrollHeight(1000); // maxScroll stays 200, never reaches 5000.
+
+    const { result } = renderHook(() => useScrollPosition("p5"));
+    act(() => {
+      result.current.restoreScrollPosition();
+    });
+
+    // Past HEIGHT_STABLE_MS the height IS settled, but the target is unreachable
+    // (maxScroll 200 < 5000), so restore must keep waiting rather than scroll.
+    act(() => {
+      vi.advanceTimersByTime(500);
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+
+    // Only the MAX_RESTORE_WAIT_MS (5000) timeout fires it, clamped to maxScroll.
+    act(() => {
+      vi.advanceTimersByTime(4500);
+    });
+    expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
+  });
+
   it("(f) cancels the in-flight restore poll on unmount (no scroll on the next page)", () => {
     vi.useFakeTimers();
     window.sessionStorage.setItem(`${KEY_PREFIX}p7`, "500");
@@ -151,6 +223,7 @@ describe("useScrollPosition", () => {
     const { result, unmount } = renderHook(() => useScrollPosition("p7"));
     act(() => {
       result.current.restoreScrollPosition();
+      vi.advanceTimersByTime(300);
     });
     expect(window.scrollTo).not.toHaveBeenCalled(); // still polling
 
@@ -167,8 +240,9 @@ describe("useScrollPosition", () => {
   });
 
   it("(g) does not restore if the reader scrolled (wheel) before restore fires", () => {
+    vi.useFakeTimers();
     window.sessionStorage.setItem(`${KEY_PREFIX}g1`, "500");
-    setScrollHeight(2000); // tall enough to restore synchronously
+    setScrollHeight(2000); // reachable + steady: would settle and scroll otherwise.
 
     const { result } = renderHook(() => useScrollPosition("g1"));
 
@@ -178,12 +252,13 @@ describe("useScrollPosition", () => {
     });
     act(() => {
       result.current.restoreScrollPosition();
+      vi.advanceTimersByTime(5000);
     });
 
     expect(window.scrollTo).not.toHaveBeenCalled();
   });
 
-  it("(h) aborts an in-flight restore poll when the reader scrolls", () => {
+  it("(h) aborts an in-flight restore poll when the reader scrolls mid-wait", () => {
     vi.useFakeTimers();
     window.sessionStorage.setItem(`${KEY_PREFIX}h1`, "500");
     setInnerHeight(800);
@@ -192,6 +267,7 @@ describe("useScrollPosition", () => {
     const { result } = renderHook(() => useScrollPosition("h1"));
     act(() => {
       result.current.restoreScrollPosition();
+      vi.advanceTimersByTime(200);
     });
     expect(window.scrollTo).not.toHaveBeenCalled(); // still polling
 
@@ -201,7 +277,7 @@ describe("useScrollPosition", () => {
     });
 
     // Content of the page grows tall enough and time passes: the cancelled poll
-    // must NOT resurrect and yank the reader.
+    // must NOT resurrect and yank the reader, even after the height would settle.
     setScrollHeight(2000);
     act(() => {
       vi.advanceTimersByTime(5000);
@@ -210,8 +286,9 @@ describe("useScrollPosition", () => {
   });
 
   it("(i) a non-scroll keydown does NOT abort restore", () => {
+    vi.useFakeTimers();
     window.sessionStorage.setItem(`${KEY_PREFIX}i1`, "500");
-    setScrollHeight(2000); // tall enough to restore synchronously
+    setScrollHeight(2000); // reachable + steady.
 
     const { result } = renderHook(() => useScrollPosition("i1"));
 
@@ -222,14 +299,17 @@ describe("useScrollPosition", () => {
     act(() => {
       result.current.restoreScrollPosition();
     });
-
-    // Restore still happens: the innocuous keypress did not disable it.
+    // Height steady: restore settles and still fires — the keypress did not abort.
+    act(() => {
+      vi.advanceTimersByTime(500);
+    });
     expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
   });
 
   it("(j) a scroll keydown (Space) DOES abort restore", () => {
+    vi.useFakeTimers();
     window.sessionStorage.setItem(`${KEY_PREFIX}j1`, "500");
-    setScrollHeight(2000); // tall enough to restore synchronously
+    setScrollHeight(2000); // reachable + steady: would settle and scroll otherwise.
 
     const { result } = renderHook(() => useScrollPosition("j1"));
 
@@ -239,117 +319,10 @@ describe("useScrollPosition", () => {
     });
     act(() => {
       result.current.restoreScrollPosition();
-    });
-
-    expect(window.scrollTo).not.toHaveBeenCalled();
-  });
-
-  it("(c) does nothing when nothing is saved or the saved value is <= 0", () => {
-    // Nothing saved.
-    const a = renderHook(() => useScrollPosition("nope"));
-    act(() => {
-      a.result.current.restoreScrollPosition();
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // Saved value <= 0.
-    window.sessionStorage.setItem(`${KEY_PREFIX}zero`, "0");
-    const b = renderHook(() => useScrollPosition("zero"));
-    act(() => {
-      b.result.current.restoreScrollPosition();
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled();
-  });
-
-  it("(d) scrolls to the saved Y once the content is tall enough", () => {
-    vi.useFakeTimers();
-    window.sessionStorage.setItem(`${KEY_PREFIX}p4`, "500");
-    setInnerHeight(800);
-    setScrollHeight(100); // maxScroll = -700, target not yet reachable.
-
-    const { result } = renderHook(() => useScrollPosition("p4"));
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-
-    // Still polling: content not laid out yet.
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // Content becomes tall enough: maxScroll = 2000 - 800 = 1200 >= 500.
-    setScrollHeight(2000);
-    act(() => {
-      vi.advanceTimersByTime(100);
-    });
-
-    expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
-  });
-
-  it("(d2) clamps to the max reachable position after the timeout", () => {
-    vi.useFakeTimers();
-    window.sessionStorage.setItem(`${KEY_PREFIX}p5`, "5000");
-    setInnerHeight(800);
-    setScrollHeight(1000); // maxScroll stays 200, never reaches 5000.
-
-    const { result } = renderHook(() => useScrollPosition("p5"));
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-
-    // Advance past the 5s timeout; restore should fire clamped to maxScroll.
-    act(() => {
       vi.advanceTimersByTime(5000);
     });
 
-    expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
-  });
-
-  it("(k) shares ONE timeout budget across re-triggers (does not restart the clock)", () => {
-    // The static->live editor swap re-invokes restore. The shared budget
-    // (restoreStartRef) must measure the MAX_RESTORE_WAIT_MS (5000) deadline
-    // from the FIRST trigger, not restart it on every re-trigger. This pins
-    // the `if (restoreStartRef.current === null)` guard: a mutant that resets
-    // `restoreStartRef.current = Date.now()` on every trigger would push the
-    // deadline out to t=8000 (3000 + 5000) and fail the t=5000 assertion below.
-    vi.useFakeTimers();
-    vi.setSystemTime(0);
-    window.sessionStorage.setItem(`${KEY_PREFIX}k1`, "5000");
-    setInnerHeight(800);
-    setScrollHeight(1000); // maxScroll = 200, never reaches 5000 -> it polls.
-
-    const { result } = renderHook(() => useScrollPosition("k1"));
-
-    // First trigger at t=0: starts the shared budget and begins polling.
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
     expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // Advance to t=3000 (still polling: content short, not yet timed out).
-    act(() => {
-      vi.advanceTimersByTime(3000);
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // Second trigger at t=3000 (the swap re-assert). Under the real code the
-    // budget is shared, so `start` stays 0; under the reset-mutant it becomes 3000.
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-
-    // At t=4900 the FIRST budget has not yet elapsed (4900 - 0 < 5000): no clamp.
-    act(() => {
-      vi.advanceTimersByTime(1900);
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // At t=5000 the shared budget (measured from t=0) times out and clamps to the
-    // furthest reachable position (maxScroll = 200). The reset-mutant, measuring
-    // from t=3000, would still be waiting (5000 - 3000 = 2000 < 5000) and would
-    // NOT have scrolled here -> this assertion fails against that mutant.
-    act(() => {
-      vi.advanceTimersByTime(100);
-    });
-    expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
   });
 
   it("(e) never throws when storage access throws", () => {
@@ -372,3 +345,28 @@ describe("useScrollPosition", () => {
     }).not.toThrow();
   });
 });
+
+describe("hasSavedReadingPosition", () => {
+  beforeEach(() => {
+    window.sessionStorage.clear();
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    window.sessionStorage.clear();
+  });
+
+  it("is false when nothing is saved for the page", () => {
+    expect(hasSavedReadingPosition("absent")).toBe(false);
+  });
+
+  it("is false when the saved value is 0 (page is at the top)", () => {
+    window.sessionStorage.setItem(`${KEY_PREFIX}top`, "0");
+    expect(hasSavedReadingPosition("top")).toBe(false);
+  });
+
+  it("is true when a positive reading position is saved", () => {
+    window.sessionStorage.setItem(`${KEY_PREFIX}deep`, "500");
+    expect(hasSavedReadingPosition("deep")).toBe(true);
+  });
+});

apps/client/src/features/editor/hooks/use-scroll-position.ts

@@ -8,6 +8,10 @@ const SAVE_THROTTLE_MS = 250;
 const MAX_RESTORE_WAIT_MS = 5000;
 // How often to re-check the document height while waiting for content to load.
 const RESTORE_POLL_MS = 100;
+// The document height must stay unchanged this long before we treat the layout
+// as settled and safe to restore against — restoring while content/images are
+// still streaming in would let scroll-anchoring drift the saved offset.
+const HEIGHT_STABLE_MS = 400;
 
 // sessionStorage key prefix. sessionStorage survives an F5 in the same tab and
 // is cleared on tab close, which is exactly the lifetime we want for an MVP
@@ -57,24 +61,39 @@ function writeStorage(pageId: string, scrollY: number): void {
   }
 }
 
+/**
+ * Whether a positive reading position is saved for this page — i.e. the page
+ * will be scrolled away from the top on load. Used by the title editor to avoid
+ * auto-focusing (and thus placing the caret in) the now-off-screen title.
+ * Returns false when nothing is saved or storage is unavailable.
+ */
+export function hasSavedReadingPosition(pageId: string): boolean {
+  const y = readStorage(pageId);
+  return typeof y === "number" && y > 0;
+}
+
 /**
  * Persists and restores the window scroll position per page so a reader keeps
  * their place across a reload (F5) or reopening the document.
  *
  * Returns `restoreScrollPosition`, which the page editor calls from two triggers
- * (early, while the static/cached content is laid out, and again after the
- * static->live editor swap); it is idempotent, so re-asserting the same target is
- * a no-op. The two scroll mechanisms are mutually exclusive: if the URL has a
- * `#hash` anchor, the existing anchor-scroll logic wins and restore is a no-op.
+ * (early on mount + post-swap, after the static->live editor swap). It waits for
+ * the document height to settle, then scrolls once to the captured target; it
+ * aborts if the reader shows scroll intent first. The two scroll mechanisms are
+ * mutually exclusive: if the URL has a `#hash` anchor, the existing anchor-scroll
+ * logic wins and restore is a no-op.
  */
 export function useScrollPosition(pageId: string): {
   restoreScrollPosition: () => void;
 } {
   // CONTRACT: this hook assumes PageEditor REMOUNTS per page — page.tsx renders
   // `<MemoizedFullEditor key={page.id} ...>`, so switching pages creates a fresh
-  // hook instance with fresh refs. Restore is idempotent and interaction-gated
-  // (not single-shot): it may be called from several triggers and re-asserts the
-  // SAME captured target, which is a no-op once the window is already positioned.
+  // hook instance with fresh refs. Restore is called from two triggers (early on
+  // mount + post-swap) and waits for the document height to settle before
+  // scrolling once to the captured target; it aborts on reader scroll intent, and
+  // a `#hash` in the URL wins (restore becomes a no-op). It is idempotent enough
+  // to be re-invoked safely: an already-running poll is not restarted, and once
+  // the window is at the target the final scroll is a no-op.
   // The per-mount refs that latch are `initialTargetRef` (the captured target)
   // and `userInteractedRef` (the reader has taken over scrolling). They are NOT
   // reset when `pageId` changes in place (only the effect re-runs on [pageId]).
@@ -93,9 +112,6 @@ export function useScrollPosition(pageId: string): {
   // this, a fast SPA navigation away mid-poll would let the old page's poll fire
   // window.scrollTo against the NEW page's document (visible wrong-page scroll).
   const pollTimerRef = useRef<number | null>(null);
-  // Timestamp of the FIRST restore attempt so re-triggers (e.g. the static→live
-  // editor swap) share ONE bounded timeout budget instead of restarting it.
-  const restoreStartRef = useRef<number | null>(null);
 
   // Capture the previously-saved value synchronously during render, before the
   // effect below registers handlers that would persist the current (0) scrollY.
@@ -190,56 +206,50 @@ export function useScrollPosition(pageId: string): {
   const restoreScrollPosition = useCallback(() => {
     // The reader took over — never yank them back.
     if (userInteractedRef.current) return;
-
     // Anchor priority: a `#hash` in the URL is handled by useEditorScroll.
     if (window.location.hash) return;
-
     const targetY = initialTargetRef.current ?? 0;
     // Nothing meaningful to restore to.
     if (targetY <= 0) return;
+    // Idempotent: if a restore poll is already running, do not start a second.
+    if (pollTimerRef.current !== null) return;
 
-    // Cancel any in-flight poll before (re)starting, so overlapping triggers can
-    // never run two concurrent polls against the same target.
-    if (pollTimerRef.current !== null) {
-      window.clearTimeout(pollTimerRef.current);
-      pollTimerRef.current = null;
-    }
+    const start = Date.now();
+    let lastHeight = -1;
+    let stableSince = start;
 
-    // Share one timeout budget across re-triggers instead of restarting it.
-    if (restoreStartRef.current === null) {
-      restoreStartRef.current = Date.now();
-    }
-    const start = restoreStartRef.current;
-
-    const tryRestore = () => {
-      // Bail mid-poll if the reader started scrolling while we were waiting.
+    // Restore ONCE the document height has been stable for HEIGHT_STABLE_MS AND
+    // the target is actually reachable, so the saved pixel offset lands on the
+    // same content the reader left (restoring earlier lets scroll-anchoring drift
+    // it as content streams in above the target). The timeout is the only
+    // fallback that clamps to the furthest reachable position.
+    const tick = () => {
       if (userInteractedRef.current) {
         pollTimerRef.current = null;
         return;
       }
-
-      const maxScroll =
-        document.documentElement.scrollHeight - window.innerHeight;
-      const timedOut = Date.now() - start >= MAX_RESTORE_WAIT_MS;
-
-      // Restore once the content is tall enough to reach the target, or bail out
-      // after the timeout and scroll as far as currently possible.
-      if (maxScroll >= targetY || timedOut) {
+      const now = Date.now();
+      const height = document.documentElement.scrollHeight;
+      if (height !== lastHeight) {
+        lastHeight = height;
+        stableSince = now;
+      }
+      const maxScroll = height - window.innerHeight;
+      const settled = now - stableSince >= HEIGHT_STABLE_MS;
+      const reachable = maxScroll >= targetY;
+      const timedOut = now - start >= MAX_RESTORE_WAIT_MS;
+      if ((settled && reachable) || timedOut) {
         const top = Math.min(targetY, Math.max(maxScroll, 0));
-        // Redundancy guard: re-asserting the SAME target when already positioned
-        // is a no-op, so this hook can be called from multiple triggers safely.
         if (Math.abs(window.scrollY - top) > 1) {
           window.scrollTo({ top, behavior: "auto" });
         }
         pollTimerRef.current = null;
         return;
       }
-
-      // Stored in a ref so the effect cleanup can cancel it on unmount.
-      pollTimerRef.current = window.setTimeout(tryRestore, RESTORE_POLL_MS);
+      pollTimerRef.current = window.setTimeout(tick, RESTORE_POLL_MS);
     };
 
-    tryRestore();
+    pollTimerRef.current = window.setTimeout(tick, RESTORE_POLL_MS);
   }, []);
 
   return { restoreScrollPosition };
@@ -250,8 +260,16 @@ export function useScrollPosition(pageId: string): {
  *
  * Extracted from PageEditor so the exact restore triggers (their deps and the
  * post-swap `&& editor` guard) are directly unit-testable rather than mirrored.
- * Behaviour is unchanged: `restoreScrollPosition` is idempotent, so re-asserting
- * the same target from either trigger is a no-op.
+ * There are TWO triggers:
+ *  - an EARLY one on mount, so a reload that never reaches the live editor
+ *    (offline / collab never syncs — the static cache stays shown) still restores
+ *    the reading position instead of being stuck at the top; and
+ *  - a POST-SWAP one, so once the live editor is mounted the wait re-measures
+ *    against the FINAL layout the reader will see.
+ * Both are safe because `restoreScrollPosition` is idempotent: a running poll
+ * suppresses a second start, and once already positioned the final scroll is a
+ * no-op. The early trigger is now bounce-free because the title-focus root cause
+ * is fixed (title auto-focus is skipped when a saved position exists).
  *
  * @param pageId      the page whose scroll position is persisted/restored.
  * @param editor      the tiptap editor instance, or `null` until it is ready.
@@ -264,16 +282,18 @@ export function useScrollRestoreOnSwap(
 ): void {
   const { restoreScrollPosition } = useScrollPosition(pageId);
 
-  // Restore as early as the static (cached) content is laid out, before paint,
-  // so the reader's position is applied without a visible jump. Aborts itself if
-  // the reader has already started scrolling (handled inside the hook).
+  // Early trigger: start restoring on mount, so a reload that never reaches the
+  // live editor (offline / collab never syncs — the static cache stays shown)
+  // still restores the reading position. Restore waits for the document height to
+  // settle before scrolling, so this measures whatever layout is current (static,
+  // then the live layout if the swap happens).
   useLayoutEffect(() => {
     restoreScrollPosition();
   }, [restoreScrollPosition]);
 
-  // Re-assert once after the static -> live editor swap in case the swap reset
-  // the window scroll. Idempotent: a no-op when the position is already correct,
-  // and a no-op after the reader has interacted.
+  // Post-swap trigger: after the static -> live swap, re-invoke restore so the
+  // wait re-measures against the final live layout. Idempotent — a no-op while the
+  // early poll is still running, and a no-op once already positioned.
   useLayoutEffect(() => {
     if (!showStatic && editor) restoreScrollPosition();
   }, [showStatic, editor, restoreScrollPosition]);

apps/client/src/features/editor/page-editor.test.tsx

@@ -7,31 +7,34 @@ const KEY_PREFIX = "gitmost:scroll-position:";
 
 // NOTE ON SCOPE (F2 — reviewer-approved lighter variant).
 //
-// The real UX wiring lives in the exported `useScrollRestoreOnSwap` hook (two
-// useLayoutEffects around useScrollPosition), which PageEditor calls with the
-// same signature. A FULL PageEditor component test is impractical here and has no
-// precedent in this client: PageEditor directly constructs a
-// HocuspocusProviderWebsocket + IndexeddbPersistence, a tiptap `useEditor` with
-// collab extensions, reads jotai atoms, react-router params, the shared
-// `queryClient` from main.tsx, i18n, and mounts ~12 editor menu children. Worse,
-// the static->live swap (`showStatic` -> false) is gated on
-// `isCollabSynced(status, isLocalSynced && isRemoteSynced)`, which can only flip
-// by driving the mocked collab provider's async sync callbacks. The heaviest
-// component-test precedent in the repo (comment-hover-preview.test.tsx) mounts a
-// single leaf component with ONE mocked query; nothing mounts a feature root of
-// this weight. Reproducing all of that would test the mocks, not the wiring.
+// The real UX wiring lives in the exported `useScrollRestoreOnSwap` hook (now TWO
+// useLayoutEffect triggers around useScrollPosition — an early one on mount plus
+// a post-swap one), which PageEditor calls with the same signature. A FULL
+// PageEditor component test is impractical here and has no precedent in this
+// client: PageEditor directly constructs a HocuspocusProviderWebsocket +
+// IndexeddbPersistence, a tiptap `useEditor` with collab extensions, reads jotai
+// atoms, react-router params, the shared `queryClient` from main.tsx, i18n, and
+// mounts ~12 editor menu children. Worse, the static->live swap (`showStatic` ->
+// false) is gated on `isCollabSynced(status, isLocalSynced && isRemoteSynced)`,
+// which can only flip by driving the mocked collab provider's async sync
+// callbacks. The heaviest component-test precedent in the repo
+// (comment-hover-preview.test.tsx) mounts a single leaf component with ONE mocked
+// query; nothing mounts a feature root of this weight. Reproducing all of that
+// would test the mocks, not the wiring.
 //
 // So this file tests the REAL `useScrollRestoreOnSwap` hook — the exact code
 // PageEditor imports and calls — driving its `showStatic`/`editor` inputs the way
 // the swap does. Because it exercises the real hook (not a copy), dropping the
-// `&& editor` guard or changing the effect deps makes these tests fail; they
-// guard the production code directly (verified: removing `&& editor` reddens the
-// first test).
+// post-swap `&& editor` guard or changing the effect deps makes these tests fail;
+// they guard the production code directly (verified: removing `&& editor` reddens
+// test A's call-count assertion).
 //
-// Both tests observe the real effect via `window.scrollTo`. The stubbed
-// `window.scrollTo` never mutates `window.scrollY`, and the target is left
-// unreached, so every restore invocation that passes the guard yields exactly one
-// `scrollTo` call — making the call count a faithful proxy for restore invocations.
+// Both tests observe the real effect via `window.scrollTo`. Restore is not
+// synchronous: the hook schedules a poll that waits for the document height to
+// stay stable for HEIGHT_STABLE_MS, so the tests drive fake timers and assert the
+// eventual `scrollTo({ top, behavior: "auto" })`. The stubbed `window.scrollTo`
+// never mutates `window.scrollY`, so each effective restore produces exactly one
+// call — the test A guard is pinned via that call count.
 
 function setScrollY(value: number): void {
   Object.defineProperty(window, "scrollY", { configurable: true, value });
@@ -80,61 +83,60 @@ describe("PageEditor scroll-restore wiring (useScrollRestoreOnSwap)", () => {
     window.location.hash = "";
   });
 
-  it("re-invokes restore after the swap, with the [showStatic, editor] deps/guard", () => {
-    // Target is immediately reachable, so each restore that passes the guard
-    // scrolls synchronously. `window.scrollY` stays 0 (stubbed scrollTo never
-    // updates it), so scrollTo is called once per effective restore — a proxy for
-    // the restore invocation count.
+  it("early trigger restores on mount; post-swap re-assert is gated by `&& editor`", () => {
+    vi.useFakeTimers();
+    // Target reachable (maxScroll = 2000 - 800 = 1200 >= 500) and the height is
+    // held steady, so each restore poll settles and scrolls to 500.
     window.sessionStorage.setItem(`${KEY_PREFIX}guard`, "500");
     setInnerHeight(800);
-    setScrollHeight(2000); // maxScroll = 1200 >= 500: reachable, no polling.
+    setScrollHeight(2000); // reachable + held steady -> the poll settles
 
-    // Pre-swap: static content shown, live editor not ready. Only the early
-    // pre-paint restore fires; the post-swap effect's guard (!showStatic) blocks it.
     const { rerender } = render(
       <Host pageId="guard" showStatic={true} editor={null} />,
     );
+    // Early trigger's poll settles (>= HEIGHT_STABLE_MS) and restores once — this
+    // is the offline / collab-never-syncs path (no swap needed).
+    act(() => {
+      vi.advanceTimersByTime(500);
+    });
     expect(window.scrollTo).toHaveBeenCalledTimes(1);
 
-    // Collab reports synced (showStatic flips false) but the editor is not ready
-    // yet: the swap effect re-runs (deps [showStatic, editor] changed) but the
-    // `&& editor` guard must keep it a no-op. The early effect does NOT re-fire
-    // (its dep [restoreScrollPosition] is a stable useCallback([])).
-    // (Pins the guard: dropping `&& editor` would restore against a null editor,
-    // producing a 2nd scrollTo and failing this expectation.)
+    // showStatic flips false but editor still null: the post-swap effect re-runs
+    // but its `&& editor` guard must block it — no new restore. (Dropping
+    // `&& editor` would start a fresh poll and produce a 2nd call, failing this.)
     rerender(<Host pageId="guard" showStatic={false} editor={null} />);
+    act(() => {
+      vi.advanceTimersByTime(500);
+    });
     expect(window.scrollTo).toHaveBeenCalledTimes(1);
 
-    // The static -> live swap completes (showStatic false AND editor present): the
-    // post-swap effect re-asserts the restore exactly once more, driven solely by
-    // the [showStatic, editor] deps changing.
+    // editor present: the post-swap effect re-invokes restore -> a fresh poll
+    // settles and re-asserts (scrollY stubbed at 0, so it scrolls again).
     rerender(<Host pageId="guard" showStatic={false} editor={fakeEditor} />);
+    act(() => {
+      vi.advanceTimersByTime(500);
+    });
     expect(window.scrollTo).toHaveBeenCalledTimes(2);
   });
 
-  it("the post-swap re-assert drives a REAL restore (window.scrollTo) via the hook", () => {
-    // End-to-end through the real useScrollPosition (inside the hook): the swap
-    // re-invocation is the CAUSE of the scroll (nothing scrolls before it).
+  it("early trigger restores WITHOUT any swap once the static content settles tall enough", () => {
+    // Directly covers the offline regression: collab never syncs, so showStatic
+    // stays true and the editor stays null — yet the reading position is restored
+    // against the static cache's own layout, not left stuck at the top.
     vi.useFakeTimers();
-    window.sessionStorage.setItem(`${KEY_PREFIX}peg`, "500");
+    window.sessionStorage.setItem(`${KEY_PREFIX}offline`, "500");
     setInnerHeight(800);
-    setScrollHeight(100); // maxScroll = -700: target not reachable yet -> polls.
+    setScrollHeight(100); // too short at first -> the poll keeps waiting
 
-    // Pre-swap: the early restore runs but content is too short, so it starts
-    // polling (a pending timer) without scrolling. We never advance timers, so the
-    // early poll cannot fire on its own — isolating the swap as the sole cause.
-    const { rerender } = render(
-      <Host pageId="peg" showStatic={true} editor={null} />,
-    );
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // The live content is now laid out tall enough to reach the target.
-    setScrollHeight(2000); // maxScroll = 1200 >= 500
-
-    // The static -> live swap: the post-swap useLayoutEffect re-invokes the real
-    // hook, whose synchronous tryRestore now reaches the target and scrolls.
+    render(<Host pageId="offline" showStatic={true} editor={null} />);
     act(() => {
-      rerender(<Host pageId="peg" showStatic={false} editor={fakeEditor} />);
+      vi.advanceTimersByTime(500);
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled(); // still short, no swap
+
+    setScrollHeight(2000); // static content finished laying out
+    act(() => {
+      vi.advanceTimersByTime(500);
     });
     expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
   });

apps/client/src/features/editor/page-editor.tsx

@@ -481,9 +481,10 @@ export default function PageEditor({
     }
   }, [yjsConnectionStatus, isSynced]);
 
-  // Restore the reader's scroll position across the static -> live editor swap.
-  // The wiring (early pre-paint restore + post-swap re-assert) lives in the hook
-  // so its triggers/guard are directly unit-testable.
+  // Reading-position restore. The wiring (an early on-mount trigger that also
+  // covers the offline / collab-never-syncs path, plus a post-swap re-assert for
+  // the final layout) lives in useScrollRestoreOnSwap; both wait for the document
+  // height to settle and abort if the reader scrolls first.
   useScrollRestoreOnSwap(pageId, editor, showStatic);
 
   return (

apps/client/src/features/editor/title-editor.tsx

@@ -1,5 +1,5 @@
 import "@/features/editor/styles/index.css";
-import React, { useCallback, useEffect, useState } from "react";
+import React, { useCallback, useEffect, useRef, useState } from "react";
 import { EditorContent, useEditor } from "@tiptap/react";
 import { Document } from "@tiptap/extension-document";
 import { Heading } from "@tiptap/extension-heading";
@@ -11,6 +11,7 @@ import {
   pageEditorAtom,
   titleEditorAtom,
 } from "@/features/editor/atoms/editor-atoms";
+import { hasSavedReadingPosition } from "@/features/editor/hooks/use-scroll-position";
 import {
   updatePageData,
   useUpdateTitlePageMutation,
@@ -54,6 +55,15 @@ export function TitleEditor({
   const [activePageId, setActivePageId] = useState(pageId);
   const currentPageEditMode = useAtomValue(currentPageEditModeAtom);
 
+  // Captured synchronously on first render (before any scroll handler can clobber
+  // the stored value): does this page have a saved reading position that will be
+  // restored (scrolling away from the top)? If so we must NOT auto-focus the
+  // title, or the caret lands in the now-off-screen title.
+  const hasSavedScrollRef = useRef<boolean | null>(null);
+  if (hasSavedScrollRef.current === null) {
+    hasSavedScrollRef.current = hasSavedReadingPosition(pageId);
+  }
+
   const titleEditor = useEditor({
     extensions: [
       Document.extend({
@@ -168,10 +178,17 @@ export function TitleEditor({
   }, [pageId, title, titleEditor]);
 
   useEffect(() => {
+    // Skip title auto-focus when a saved reading position will be restored: the
+    // viewport scrolls away from the top, so focusing the top-of-page title would
+    // drop the caret off-screen (the first keystroke would go into the hidden title).
+    if (hasSavedScrollRef.current) return;
     setTimeout(() => {
       // guard against Cannot access view['hasFocus'] error
       if (!titleEditor?.isInitialized) return;
-      titleEditor?.commands?.focus("end");
+      // Focus WITHOUT scrolling: TipTap's focus scrolls the focused node into
+      // view by default, and the title sits at the very top of the page, which
+      // would fight scroll-position restoration. We only want the caret placed.
+      titleEditor?.commands?.focus("end", { scrollIntoView: false });
     }, 300);
   }, [titleEditor]);