@@ -2,7 +2,7 @@
 title: 'hub-client `useEffect` audit: phased refactor'
 date: 2026-10-24
 description: 'Phased refactor of the refactor-worthy useEffect sites in hub-client: stale-dep fixes, useSyncExternalStore conversions, derived-state elimination, then measured wide-dep performance work.'
-status: draft
+status: in-progress  # Phases 1–2 done and open as stacked PRs (#924, #925); Phases 3–5 not started.
 ---
 
 ## Overview
@@ -62,7 +62,7 @@ positive, `FileSidebar.tsx:280`, was dropped from Phase 1).
 
 ## Phase 1 — Correctness: stale deps, suppressions, leaks
 
-**Done 2026-10-27 (bd-lsf2v5n0).** Findings that shaped the landing:
+**Done 2026-10-27 (bd-lsf2v5n0); open as PR #924.** Findings that shaped the landing:
 
 - The App.tsx:830 fix landed as a factory
   (`src/services/connectionHandlers.ts#createConnectionChangeHandler`)
@@ -123,6 +123,54 @@ positive, `FileSidebar.tsx:280`, was dropped from Phase 1).
 
 ## Phase 2 — `useSyncExternalStore` conversions
 
+**Done 2026-10-27 (bd-06s9sufx); open as PR #925, stacked on #924.** Findings that shaped the landing:
+
+- The `branchService` prerequisite landed as a version counter plus per-path
+  snapshot cache (`getBranchSnapshot(path)`), covering `branches` and
+  `activeBranchId` in one object. Every mutation already funneled through
+  `notify()`, which now bumps the version; the cache also re-resolves when
+  the path\'s docId changes (a file handle can appear without any
+  branchService event, and must not pin the empty snapshot). The
+  push-model escape hatch was not needed.
+- `usePresence` needed the same memoization one layer down:
+  `getRemotePresences()` returned a fresh array per call, so the service
+  now memoizes it on a version bumped at the top of `notifySubscribers` —
+  before the visibility gate, so data freshness is not deferred even when
+  subscriber callbacks are. One intended semantic difference from the old
+  push model: an unrelated re-render while the tab is hidden now reads
+  fresh presence data rather than the last pushed array (fresher, never
+  staler). `enabled: false` keeps its old behavior via a shared
+  `EMPTY_REMOTE_USERS` snapshot.
+- `usePreference` converted directly: all preference values are primitives,
+  so `getPreference(key)` is a stable snapshot by value, and the writer\'s
+  local echo was redundant (`setPreference` dispatches synchronously).
+- `useNow(intervalMs)` is one shared ticking store per interval (interval
+  runs only while subscribed; clock refreshes on resubscribe so a remount
+  never reads a stale timestamp). `useOnlineStatus()` is the textbook
+  `navigator.onLine` + online/offline `useSyncExternalStore`. Both
+  `SyncStatusBadge` and `ConnectionStatusDialog` now use both hooks; the
+  badge\'s old immediate-refresh-on-online-event is approximated by the
+  connectivity flip re-rendering and the clock catching up within one
+  tick (≤ 1 s, imperceptible for "n minutes ago" text).
+- Characterization tests were pinned before touching each uncovered site:
+  `useDocBranches.test.tsx` and `ConnectionStatusDialog.test.tsx` (the
+  dialog had no coverage). A stale-cleanup memoization test was drafted
+  and dropped: existing `stale presence cleanup` tests already cover
+  expiry, and its interval predates `vi.useFakeTimers()` in the test.
+- Verification: `npm run test:ci` unit and integration legs green; the
+  WASM leg has 3 golden-parity failures (`pandocBook`, `pandocRequest`,
+  `smokeAll`) that reproduce identically on the clean tree — the
+  pre-existing announcement-string skew Phase 1 documented, unrelated to
+  this phase. **Root-caused 2026-10-27 on rebase:** the skew was a stale
+  local WASM build against post-announcement goldens; after `npm run
+  build:wasm` all three files pass, and the full `test:ci` on the rebased
+  stack is green (1720 unit + 225 integration + 434 WASM). `npm run
+  build` green; eslint clean on touched files (one pre-existing
+  `usePresence` ref-cleanup warning, unchanged). Not
+  browser-smoked: no browser session was run for this phase (refactor
+  with characterization tests; the badge/dialog render paths are covered
+  by `SyncStatusBadge.test.tsx` and the new dialog test).
+
 Only `useDocBranches` can actually tear under concurrent rendering: it bumps
 a version counter in the subscription callback and re-reads `branchService`
 during render. `usePreference` and `usePresence` instead push the data into
@@ -131,7 +179,7 @@ tearing; converting them is simplification, not a correctness fix, and they
 are safe to defer. Replace with `useSyncExternalStore`, following the
 existing in-repo template.
 
-- [ ] Prerequisite: `branchService` memoizes its snapshots (version counter
+- [x] Prerequisite: `branchService` memoizes its snapshots (version counter
   plus cached per-path snapshot) so `getSnapshot` returns a stable reference
   — `getBranches(path)` currently returns a fresh array per call, which
   `useSyncExternalStore` cannot tolerate. The snapshot must cover everything
@@ -142,14 +190,14 @@ existing in-repo template.
   `getBranches(path)` and `getActiveBranchId(path)`, with `subscribe`
   updating both — also eliminates tearing and has no snapshot-stability
   requirement.
-- [ ] `src/hooks/useDocBranches.ts:15` — replace
+- [x] `src/hooks/useDocBranches.ts:15` — replace
   `useEffect(() => subscribe(...), [])` + `setVersion` with
   `useSyncExternalStore`.
-- [ ] `src/hooks/usePreference.ts:40` — convert the custom-event
+- [x] `src/hooks/usePreference.ts:40` — convert the custom-event
   subscription (simplification, not a tear-fix; safe to defer or drop).
-- [ ] `src/hooks/usePresence.ts:185` — convert the `onPresenceChange`
+- [x] `src/hooks/usePresence.ts:185` — convert the `onPresenceChange`
   subscription (simplification, not a tear-fix; safe to defer or drop).
-- [ ] Add two shared hooks: `useNow(intervalMs)` (one ticking store on
+- [x] Add two shared hooks: `useNow(intervalMs)` (one ticking store on
   `useSyncExternalStore`) and `useOnlineStatus()` (`navigator.onLine` plus
   online/offline events — a textbook `useSyncExternalStore`). Replace the
   duplicated 1 s ticker + online/offline listener blocks in
@@ -158,7 +206,7 @@ existing in-repo template.
   connectivity during render (`SyncStatusBadge.tsx:122` reads
   `navigator.onLine` at render time, today freshened only by the ticker
   re-render; `useOnlineStatus()` turns that into a proper subscription).
-- [ ] `src/components/debug-inspector/DebugInspectorPanel.tsx:75` and `:99` —
+- [x] `src/components/debug-inspector/DebugInspectorPanel.tsx:75` and `:99` —
   document why polling is intentional in a debug-only panel (a code comment
   is enough); do not convert.
 
 
@@ -0,0 +1,84 @@
+/**
+ * Characterization tests for ConnectionStatusDialog, pinned before the
+ * useNow/useOnlineStatus conversion (bd-06s9sufx): the dialog refreshes
+ * its relative-time cells once per second while open, and reflects the
+ * browser's online/offline state.
+ *
+ * @vitest-environment jsdom
+ */
+
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { render, screen, act, cleanup } from '@testing-library/react';
+import ConnectionStatusDialog from './ConnectionStatusDialog';
+
+const mocks = vi.hoisted(() => ({
+  lastRemoteChangeAt: null as number | null,
+}));
+
+vi.mock('@quarto/preview-runtime', () => ({
+  getConnectionInfo: () => ({
+    wsReadyState: 1,
+    wsUrl: null,
+    peers: [{ peerId: 'peer-1' }],
+  }),
+  getIndexHandle: () => ({ documentId: 'doc-index' }),
+  getFileHandle: () => ({ documentId: 'doc-file' }),
+}));
+
+vi.mock('@quarto/quarto-sync-client', () => ({
+  getDocSyncActivityWithPersisted: () => ({
+    lastSyncMessageAt: null,
+    lastEphemeralMessageAt: null,
+    lastRemoteChangeAt: mocks.lastRemoteChangeAt,
+    lastLocalChangeAt: null,
+    lastLocalDeliveredAt: null,
+  }),
+  getDocRemoteChange: () => null,
+  getConnectionLog: () => [],
+}));
+
+describe('ConnectionStatusDialog', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.useRealTimers();
+    localStorage.clear();
+  });
+
+  it('refreshes relative-time cells once per second while open', () => {
+    mocks.lastRemoteChangeAt = Date.now();
+    render(<ConnectionStatusDialog currentFilePath="report.qmd" onClose={() => {}} />);
+
+    // Both columns (project + file) show the just-landed change.
+    const initial = screen.getAllByText('0s ago');
+    expect(initial.length).toBeGreaterThan(0);
+
+    act(() => {
+      vi.advanceTimersByTime(65_000);
+    });
+
+    expect(screen.getAllByText('1m ago').length).toBeGreaterThan(0);
+    expect(screen.queryAllByText('0s ago')).toHaveLength(0);
+  });
+
+  it('shows the browser network state, refreshed on offline/online events', () => {
+    const onLine = vi.spyOn(navigator, 'onLine', 'get').mockReturnValue(true);
+    render(<ConnectionStatusDialog currentFilePath="report.qmd" onClose={() => {}} />);
+    expect(screen.getByText('Online')).toBeDefined();
+
+    onLine.mockReturnValue(false);
+    act(() => {
+      window.dispatchEvent(new Event('offline'));
+    });
+    expect(screen.getByText('Offline')).toBeDefined();
+
+    onLine.mockReturnValue(true);
+    act(() => {
+      window.dispatchEvent(new Event('online'));
+    });
+    expect(screen.getByText('Online')).toBeDefined();
+  });
+});
@@ -8,7 +8,7 @@
  * Refreshes once per second while open.
  */
 
-import { useEffect, useMemo, useState } from 'react';
+import { useMemo } from 'react';
 import type { ReactNode } from 'react';
 import {
   getDocSyncActivityWithPersisted,
@@ -21,6 +21,8 @@ import { getFileHandle, getIndexHandle, getConnectionInfo } from '@quarto/previe
 import type { Patch } from '@automerge/automerge-repo';
 import fastDiff from 'fast-diff';
 import ModalDialog from './ModalDialog';
+import { useNow } from '../hooks/useNow';
+import { useOnlineStatus } from '../hooks/useOnlineStatus';
 import { dialogs } from '../strings';
 import './ConnectionStatusDialog.css';
 
@@ -211,25 +213,10 @@ export default function ConnectionStatusDialog({
   currentFilePath,
   onClose,
 }: ConnectionStatusDialogProps) {
-  const [now, setNow] = useState(() => Date.now());
-  const [liveBrowserOnline, setLiveBrowserOnline] = useState(() => navigator.onLine);
-
-  useEffect(() => {
-    const refresh = () => {
-      setNow(Date.now());
-      setLiveBrowserOnline(navigator.onLine);
-    };
-    const intervalId = setInterval(refresh, 1000);
-    window.addEventListener('online', refresh);
-    window.addEventListener('offline', refresh);
-    return () => {
-      clearInterval(intervalId);
-      window.removeEventListener('online', refresh);
-      window.removeEventListener('offline', refresh);
-    };
-  }, []);
-
-  const browserOnline = liveBrowserOnline;
+  // 1s clock for the relative-time cells; browser connectivity as a real
+  // subscription rather than the old poll-navigator.onLine-every-tick.
+  const now = useNow(1000);
+  const browserOnline = useOnlineStatus();
 
   // Resolve the two documents of interest; re-evaluated every tick
   // because handles appear as the project loads / files open.
 
@@ -29,6 +29,8 @@ import { useEffect, useState } from 'react';
 import { getDocSyncActivity } from '@quarto/quarto-sync-client';
 import { getFileHandle, getIndexHandle, getConnectionInfo } from '@quarto/preview-runtime';
 import ConnectionStatusDialog from './ConnectionStatusDialog';
+import { useNow } from '../hooks/useNow';
+import { useOnlineStatus } from '../hooks/useOnlineStatus';
 import { syncStatus as s } from '../strings';
 import './SyncStatusBadge.css';
 
@@ -79,21 +81,12 @@ export default function SyncStatusBadge({
   verbose = false,
   dotPosition = 'start',
 }: SyncStatusBadgeProps) {
-  const [now, setNow] = useState(() => Date.now());
+  // 1s clock for the relative-time text; connectivity as a real
+  // subscription (the `connected` check below reads it during render).
+  const now = useNow(1000);
+  const online = useOnlineStatus();
   const [showDialog, setShowDialog] = useState(false);
 
-  useEffect(() => {
-    const refresh = () => setNow(Date.now());
-    const intervalId = setInterval(refresh, 1000);
-    window.addEventListener('online', refresh);
-    window.addEventListener('offline', refresh);
-    return () => {
-      clearInterval(intervalId);
-      window.removeEventListener('online', refresh);
-      window.removeEventListener('offline', refresh);
-    };
-  }, []);
-
   // Resolve the watched doc on every tick; handles appear as the
   // project loads / files open.
   let docId: string | null = null;
@@ -119,7 +112,7 @@ export default function SyncStatusBadge({
 
   const connInfo = getConnectionInfo();
   const connected =
-    navigator.onLine &&
+    online &&
     connInfo?.wsReadyState === WS_OPEN &&
     (connInfo?.peers.length ?? 0) > 0;
 
 
@@ -72,6 +72,10 @@ function RefreshPane({
     }
   }, [get]);
   const [value, setValue] = useState<unknown>(read);
+  // Deliberate polling, not a missing subscription: the report getters
+  // read ad-hoc debug state with no change event to subscribe to, and
+  // this is a debug-only panel where a simple interval beats adding a
+  // notify surface to the inspected services. Do not "modernize".
   useEffect(() => {
     const t = setInterval(() => setValue(read()), intervalMs);
     return () => clearInterval(t);
@@ -96,6 +100,8 @@ function MessagesPane({ am }: { am: QuartoDebugAutomergeApi }) {
     }));
   }, [am]);
   const [entries, setEntries] = useState<MessageLogEntry[]>(read);
+  // Deliberate polling, as in RefreshPane: the tap's ring buffer has no
+  // subscription surface, and this debug-only pane does not justify one.
   useEffect(() => {
     const t = setInterval(() => setEntries(read()), 1000);
     return () => clearInterval(t);