test(editor): extract swap height-reservation into a tested hook (#308 F1)

The +42-line height-reservation logic lived inline in PageEditor on the risky
static→live swap path with zero tests (F1). Extract it into
useSwapHeightReservation(showStatic, menuContainerRef) → { reservedHeight,
captureReservation }, mirroring the sibling useScrollRestoreOnSwap extraction,
so its release guard and cap are directly unit-testable.

Pure extraction — behavior identical. The capture stays a synchronous callback
the editor invokes in the collab-sync effect (reading swapWrapperRef.offsetHeight
while the static content is still mounted, before setShowStatic(false)); a
post-transition effect inside the hook would read the collapsed live height and
be wrong. The rAF release loop (release at scrollHeight >= reserved, or the
RELEASE_CAP_MS=4000 cap) and cancelAnimationFrame cleanup moved verbatim.

Tests (use-swap-height-reservation.test.ts) cover 4 branches, mutation-verified:
(a) capture → reservedHeight; (b) release when live content reaches reserved;
(c) release at the cap when it never does; (d) non-swap → stays null.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/client/src/components/ui/agent-avatar-stack.test.tsx

@@ -2,7 +2,7 @@ import { describe, it, expect, vi } from "vitest";
 import { render, screen, fireEvent } from "@testing-library/react";
 import { MantineProvider } from "@mantine/core";
 import { Provider, createStore } from "jotai";
-import { AgentAvatarStack, agentGlyphBackground } from "./agent-avatar-stack";
+import { AgentAvatarStack } from "./agent-avatar-stack";
 import {
   activeAiChatIdAtom,
   aiChatWindowOpenAtom,
@@ -26,23 +26,6 @@ function renderStack(props: Props) {
   return { store, ...utils };
 }
 
-describe("agentGlyphBackground", () => {
-  it("is deterministic for a given agent name", () => {
-    expect(agentGlyphBackground("Researcher")).toBe(
-      agentGlyphBackground("Researcher"),
-    );
-  });
-
-  it("differs by name and stays a fixed dark shade (readable emoji)", () => {
-    expect(agentGlyphBackground("Researcher")).not.toBe(
-      agentGlyphBackground("Нарратор"),
-    );
-    // Only the hue varies; saturation/lightness are pinned low so the glyph is
-    // always a dark circle.
-    expect(agentGlyphBackground("Нарратор")).toMatch(/^hsl\(\d+, 45%, 24%\)$/);
-  });
-});
-
 describe("AgentAvatarStack", () => {
   it("internal chat WITH role: emoji glyph in front + human launcher behind", () => {
     const { container } = renderStack({

apps/client/src/components/ui/agent-avatar-stack.tsx

@@ -23,34 +23,14 @@ export interface LauncherInfo {
   avatarUrl?: string | null;
 }
 
+// Same violet token as the former AiAgentBadge (which used color="violet").
+const AGENT_COLOR = "violet";
 const GLYPH_SIZE = 38;
 const LAUNCHER_SIZE = 22;
-// How far the launcher avatar sticks out past the agent's top-right corner, so
+// How far the launcher avatar sticks out past the agent's bottom-right corner, so
 // the "human behind" reads as behind (lower z-index) yet stays clearly visible.
 const LAUNCHER_OVERHANG = 8;
 
-// Small deterministic string hash (same algorithm as custom-avatar's initials
-// hash) used to pick a stable per-agent glyph color.
-function hashName(input: string): number {
-  let hash = 0;
-  for (let i = 0; i < input.length; i += 1) {
-    hash = (hash << 5) - hash + input.charCodeAt(i);
-    hash |= 0;
-  }
-  return Math.abs(hash);
-}
-
-/**
- * Deterministic DARK background for an emoji/sparkles agent glyph. The hue is
- * derived from the agent-name hash so distinct agents get distinct circles;
- * saturation and lightness are pinned low ("shifted into darkness") so a bright
- * emoji or the white sparkles icon stays legible on top (#300).
- */
-export function agentGlyphBackground(name: string): string {
-  const hue = hashName(name) % 360;
-  return `hsl(${hue}, 45%, 24%)`;
-}
-
 /**
  * The front avatar. Image-source priority (#300):
  *   1. agent.avatarUrl -> a real avatar image (external MCP agent account).
@@ -68,18 +48,9 @@ function AgentGlyph({ agent }: { agent: AgentInfo }) {
     );
   }
 
-  // Emoji/sparkles glyphs sit on a per-agent dark circle (hashed from the agent
-  // name) so different agents are visually distinct, while the dark background
-  // keeps the emoji / white sparkles icon readable.
-  const bg = agentGlyphBackground(agent.name);
-  const glyphStyles = {
-    root: { background: bg },
-    placeholder: { background: bg, color: "var(--mantine-color-white)" },
-  };
-
   if (agent.emoji) {
     return (
-      <Avatar size={GLYPH_SIZE} radius="xl" variant="filled" styles={glyphStyles}>
+      <Avatar size={GLYPH_SIZE} radius="xl" color={AGENT_COLOR} variant="filled">
         <span style={{ fontSize: Math.round(GLYPH_SIZE * 0.5) }} aria-hidden>
           {agent.emoji}
         </span>
@@ -88,7 +59,7 @@ function AgentGlyph({ agent }: { agent: AgentInfo }) {
   }
 
   return (
-    <Avatar size={GLYPH_SIZE} radius="xl" variant="filled" styles={glyphStyles}>
+    <Avatar size={GLYPH_SIZE} radius="xl" color={AGENT_COLOR} variant="filled">
       <IconSparkles size={Math.round(GLYPH_SIZE * 0.55)} stroke={2} />
     </Avatar>
   );
@@ -185,7 +156,7 @@ export function AgentAvatarStack({
         : {})}
     >
       {launcher && (
-        <Box pos="absolute" top={0} right={0} style={{ zIndex: 0 }}>
+        <Box pos="absolute" bottom={0} right={0} style={{ zIndex: 0 }}>
           <CustomAvatar
             size={LAUNCHER_SIZE}
             avatarUrl={launcher.avatarUrl}
@@ -194,8 +165,8 @@ export function AgentAvatarStack({
           />
         </Box>
       )}
-      {/* The agent glyph keeps its own size (flex-centered in the container); the
-          launcher overhangs it by LAUNCHER_OVERHANG at the top-right and stays visible. */}
+      {/* Pin the agent glyph to the top-left at its own size; the launcher then
+          overhangs it by LAUNCHER_OVERHANG at the bottom-right and stays visible. */}
       <Box
         style={{
           position: "relative",

apps/client/src/features/editor/hooks/use-swap-height-reservation.test.ts

@@ -0,0 +1,164 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { renderHook, act } from "@testing-library/react";
+import type { RefObject } from "react";
+import { useSwapHeightReservation } from "./use-swap-height-reservation";
+
+// Controllable fake requestAnimationFrame. jsdom's rAF is timer-driven and hard
+// to step deterministically, so we install a manual queue: `tickRaf()` drains the
+// callbacks scheduled so far (a callback that reschedules enqueues a new one for
+// the NEXT tick), letting each test advance the release loop frame by frame.
+let rafQueue: Array<{ id: number; cb: FrameRequestCallback }> = [];
+let nextRafId = 1;
+let realRaf: typeof globalThis.requestAnimationFrame;
+let realCancel: typeof globalThis.cancelAnimationFrame;
+
+function tickRaf(): void {
+  const current = rafQueue;
+  rafQueue = [];
+  for (const { cb } of current) cb(0);
+}
+
+// A mutable stand-in for the live-content container. The hook only reads
+// `scrollHeight`, so tests drive the release condition by mutating this.
+function makeMenuRef(): {
+  ref: RefObject<HTMLElement | null>;
+  setScrollHeight: (h: number) => void;
+} {
+  const el = { scrollHeight: 0 };
+  return {
+    ref: { current: el } as unknown as RefObject<HTMLElement | null>,
+    setScrollHeight: (h: number) => {
+      el.scrollHeight = h;
+    },
+  };
+}
+
+const H = 1000;
+
+describe("useSwapHeightReservation", () => {
+  beforeEach(() => {
+    rafQueue = [];
+    nextRafId = 1;
+    realRaf = globalThis.requestAnimationFrame;
+    realCancel = globalThis.cancelAnimationFrame;
+    globalThis.requestAnimationFrame = ((cb: FrameRequestCallback) => {
+      const id = nextRafId++;
+      rafQueue.push({ id, cb });
+      return id;
+    }) as typeof globalThis.requestAnimationFrame;
+    globalThis.cancelAnimationFrame = ((id: number) => {
+      rafQueue = rafQueue.filter((e) => e.id !== id);
+    }) as typeof globalThis.cancelAnimationFrame;
+  });
+
+  afterEach(() => {
+    globalThis.requestAnimationFrame = realRaf;
+    globalThis.cancelAnimationFrame = realCancel;
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  // (a) reserve-on-swap: the captured height becomes `reservedHeight`, the value
+  // that drives the swap wrapper's minHeight. Captured while static is still up,
+  // then the swap flips showStatic; before any release frame runs the reservation
+  // is held at exactly H.
+  it("(a) holds the captured height as reservedHeight after the swap (drives minHeight)", () => {
+    const { ref, setScrollHeight } = makeMenuRef();
+    setScrollHeight(0); // live content not laid out yet -> release cannot fire.
+    const { result, rerender } = renderHook(
+      ({ showStatic }) => useSwapHeightReservation(showStatic, ref),
+      { initialProps: { showStatic: true } },
+    );
+
+    // Capture happens synchronously at the swap point (static still shown).
+    act(() => {
+      result.current.captureReservation(H);
+    });
+    // The swap flips to the live branch.
+    rerender({ showStatic: false });
+
+    expect(result.current.reservedHeight).toBe(H);
+  });
+
+  // (b) release when the live content is tall enough. Guard is `>=`: with
+  // liveHeight === H the reservation releases. This FAILS if the guard direction
+  // were `<` (liveHeight === H is not `< H`, so it would never release).
+  it("(b) releases once live content reaches the reserved height", () => {
+    const { ref, setScrollHeight } = makeMenuRef();
+    setScrollHeight(0);
+    const { result, rerender } = renderHook(
+      ({ showStatic }) => useSwapHeightReservation(showStatic, ref),
+      { initialProps: { showStatic: true } },
+    );
+
+    act(() => {
+      result.current.captureReservation(H);
+    });
+    rerender({ showStatic: false });
+    expect(result.current.reservedHeight).toBe(H); // still reserved (short live doc)
+
+    // Live editor finishes laying out to the reserved height.
+    setScrollHeight(H);
+    act(() => {
+      tickRaf();
+    });
+
+    expect(result.current.reservedHeight).toBeNull();
+  });
+
+  // (c) cap escape: the live content never reaches the reserved height, so the
+  // height match never fires; the reservation must still release at the 4000ms
+  // cap (no stuck reservation / dead space). This FAILS if there were no cap: the
+  // loop would poll forever while scrollHeight stays below H.
+  it("(c) releases at the 4000ms cap when live content stays too short", () => {
+    // Only fake Date so `Date.now()` (the cap clock) is controllable; leave our
+    // manual rAF queue in place (default fake timers would replace it).
+    vi.useFakeTimers({ toFake: ["Date"] });
+    vi.setSystemTime(0);
+    const { ref, setScrollHeight } = makeMenuRef();
+    setScrollHeight(H - 100); // always shorter than reserved -> height match never fires.
+    const { result, rerender } = renderHook(
+      ({ showStatic }) => useSwapHeightReservation(showStatic, ref),
+      { initialProps: { showStatic: true } },
+    );
+
+    act(() => {
+      result.current.captureReservation(H);
+    });
+    rerender({ showStatic: false });
+
+    // A few frames pass but time has not reached the cap: still reserved.
+    act(() => {
+      tickRaf();
+    });
+    act(() => {
+      tickRaf();
+    });
+    expect(result.current.reservedHeight).toBe(H);
+
+    // Advance past the cap; the next frame releases even though the live content
+    // is still shorter than the reservation.
+    vi.setSystemTime(4001);
+    act(() => {
+      tickRaf();
+    });
+
+    expect(result.current.reservedHeight).toBeNull();
+  });
+
+  // (d) non-swap: without a capture (and while static is shown) there is no
+  // reservation and the release loop never arms, so no rAF is scheduled.
+  it("(d) reserves nothing and arms no loop when the swap never happens", () => {
+    const { ref } = makeMenuRef();
+    const { result } = renderHook(() =>
+      useSwapHeightReservation(true, ref),
+    );
+
+    expect(result.current.reservedHeight).toBeNull();
+    expect(rafQueue.length).toBe(0); // release loop never armed
+    act(() => {
+      tickRaf();
+    });
+    expect(result.current.reservedHeight).toBeNull();
+  });
+});

apps/client/src/features/editor/hooks/use-swap-height-reservation.ts

@@ -0,0 +1,79 @@
+import { RefObject, useCallback, useEffect, useState } from "react";
+
+// Last-resort release deadline. The primary release is the live-content height
+// match below; this cap only exists so a slow/short live doc can never pin the
+// reservation forever. It is generous (well past when the live content normally
+// reaches the reserved height — it renders the SAME content as the static copy)
+// so a slow load doesn't release mid-render and reintroduce the collapse.
+const RELEASE_CAP_MS = 4000;
+
+/**
+ * Reserves the document height across the static -> live editor swap.
+ *
+ * The live editor lays out its content over a few frames, so replacing the
+ * (full-height) static copy with it momentarily shrinks the document; the
+ * browser then clamps window scroll to the top, which yanked the reader off
+ * their restored reading position (and threw their scroll to 0 if they were
+ * scrolling at that moment). Pinning a min-height on the swap wrapper keeps the
+ * document tall through the swap so the scroll position simply survives (#266).
+ * `reservedHeight === null` means no reservation is active.
+ *
+ * The capture is intentionally a CALLBACK the page editor invokes, NOT something
+ * this hook derives by watching `showStatic`. The height MUST be read
+ * synchronously while the static content is still mounted (full natural height),
+ * right before the flip to the live branch. By the time any post-transition
+ * effect here could run, `showStatic` is already false and the wrapper shows the
+ * live/collapsed content, so `offsetHeight` would be wrong. So page-editor calls
+ * `captureReservation(wrapper.offsetHeight)` inside its collab-sync effect,
+ * before `setShowStatic(false)`, preserving that exact timing.
+ *
+ * @param showStatic       whether the static (cached) content is still shown.
+ * @param menuContainerRef the live-branch content container. It is a descendant
+ *   of the swap wrapper inside the live branch, so its `scrollHeight` is the live
+ *   content height (not inflated by the ancestor min-height reservation).
+ */
+export function useSwapHeightReservation(
+  showStatic: boolean,
+  menuContainerRef: RefObject<HTMLElement | null>,
+): {
+  reservedHeight: number | null;
+  captureReservation: (height: number | null) => void;
+} {
+  const [reservedHeight, setReservedHeight] = useState<number | null>(null);
+
+  // Capture the current (static, full-height) content height BEFORE the swap so
+  // the wrapper can reserve it while the live editor lays out — otherwise the
+  // transient shrink clamps window scroll to the top. The caller reads
+  // `offsetHeight` synchronously at the swap point and hands it here.
+  const captureReservation = useCallback(
+    (height: number | null) => setReservedHeight(height),
+    [],
+  );
+
+  // Release the reserved height once the live editor's content has laid out to
+  // at least the reserved height (so removing the reservation cannot collapse
+  // the document). The primary release is that height match; the cap is only a
+  // last-resort so we never pin forever. A shorter-than-reserved live doc (rare:
+  // stale/longer cache) releases at the cap, leaving only harmless bottom dead
+  // space until then.
+  useEffect(() => {
+    if (showStatic || reservedHeight == null) return;
+    let raf = 0;
+    const startedAt = Date.now();
+    const check = () => {
+      const liveHeight = menuContainerRef.current?.scrollHeight ?? 0;
+      if (
+        liveHeight >= reservedHeight ||
+        Date.now() - startedAt > RELEASE_CAP_MS
+      ) {
+        setReservedHeight(null);
+        return;
+      }
+      raf = requestAnimationFrame(check);
+    };
+    raf = requestAnimationFrame(check);
+    return () => cancelAnimationFrame(raf);
+  }, [showStatic, reservedHeight, menuContainerRef]);
+
+  return { reservedHeight, captureReservation };
+}

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

@@ -79,6 +79,7 @@ import { jwtDecode } from "jwt-decode";
 import { searchSpotlight } from "@/features/search/constants.ts";
 import { useEditorScroll } from "./hooks/use-editor-scroll";
 import { useScrollRestoreOnSwap } from "./hooks/use-scroll-position";
+import { useSwapHeightReservation } from "./hooks/use-swap-height-reservation";
 import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
 import ColumnsMenu from "@/features/editor/components/columns/columns-menu.tsx";
 import { TransclusionLookupProvider } from "@/features/editor/components/transclusion/transclusion-lookup-context";
@@ -449,6 +450,22 @@ export default function PageEditor({
   const hasConnectedOnceRef = useRef(false);
   const [showStatic, setShowStatic] = useState(true);
 
+  // Reserved height held across the static -> live editor swap. The live editor
+  // lays out its content over a few frames, so replacing the (full-height) static
+  // copy with it momentarily shrinks the document; the browser then clamps window
+  // scroll to the top, which yanked the reader off their restored reading position
+  // (and threw their scroll to 0 if they were scrolling at that moment). Pinning a
+  // min-height on the swap wrapper keeps the document tall through the swap so the
+  // scroll position simply survives. `null` = no reservation active.
+  const swapWrapperRef = useRef<HTMLDivElement | null>(null);
+  // Reserve/release wiring lives in the hook so its capture trigger and release
+  // guard/cap are directly unit-testable. Capture stays synchronous at the swap
+  // point (see the collab-sync effect below); the hook only owns the release.
+  const { reservedHeight, captureReservation } = useSwapHeightReservation(
+    showStatic,
+    menuContainerRef,
+  );
+
   useEffect(() => {
     const timeout = setTimeout(() => {
       if (yjsConnectionStatus === WebSocketStatus.Connecting || !isSynced) {
@@ -477,6 +494,10 @@ export default function PageEditor({
       isCollabSynced(yjsConnectionStatus, isSynced)
     ) {
       hasConnectedOnceRef.current = true;
+      // Capture the current (static, full-height) content height BEFORE the swap
+      // so the wrapper can reserve it while the live editor lays out — otherwise
+      // the transient shrink clamps window scroll to the top.
+      captureReservation(swapWrapperRef.current?.offsetHeight ?? null);
       setShowStatic(false);
     }
   }, [yjsConnectionStatus, isSynced]);
@@ -490,6 +511,12 @@ export default function PageEditor({
     <TransclusionLookupProvider>
       <PageEmbedLookupProvider>
         <PageEmbedAncestryProvider hostPageId={pageId}>
+      <div
+        ref={swapWrapperRef}
+        style={
+          reservedHeight != null ? { minHeight: reservedHeight } : undefined
+        }
+      >
       {showStatic ? (
         <div style={{ position: "relative" }}>
           {/* Surface the pre-sync read-only window so edits typed before the
@@ -577,6 +604,7 @@ export default function PageEditor({
           ></div>
         </div>
       )}
+      </div>
         </PageEmbedAncestryProvider>
       </PageEmbedLookupProvider>
     </TransclusionLookupProvider>