l-lessons-frameset-persistence

Lessons from the Frame Chrome Consolidation epic (#1482) and Frameset Arch v2 (#1961) follow-ups: frameset persistence, prop-sync useEffect traps, React #185 startup loops. File starts with a "Recurri...

Lessons: Frameset Persistence

Orientation: The P1–P4 recurring patterns below are still valid under Frameset Arch v2. The v1 storage model (framesets[] array, the useSavedFramesets plural hook, and the v1 migration functions) has been retired from the codebase — it survives here only as the compact v1 historical context section near the bottom, kept for archaeology. For the current (v2) singleton model, read l-lessons-frame-frameset-pin-model first.

Recurring patterns (synthesized 2026-05-20, post-2026-05-17; pruned 2026-06-06)

Read this first. The dated entries below are the chronological record — drill in when a pattern's "See" points at one.

P1 — Reference identity is a footgun for prop-sync useEffect

Trigger. A useEffect listed in a provider component reads a prop produced by provider.deserialize(leaf.props) (e.g. initialTimelineState, initialGridState) OR reads a ctx.* method and lists ctx in deps. Symptom in production: React error #185 ("Maximum update depth exceeded") at app startup under sustained parent-render pressure. Symptom in CI: nothing — Vitest/jsdom does not reproduce this (see P3).

Action. Gate the effect on a deep-value comparison against a useRef-tracked snapshot of the previous prop value (not local state — comparing to local state turns the effect into a state-revert). Never trust reference identity for objects produced by provider.deserialize() (allocates fresh every render, see packages/view-provider/src/registry.ts's renderLeaf) or makeFrameContext() (fresh every render unless memoized at the LeafRenderer level — which is now the case post-#1770 but was historically not). When adding such an effect, always extend the regression guard at e2e/startup-no-update-depth-loop.spec.ts to seed a realistic frameset that includes the new persisted-prop-to-local-state mirror.

See. 2026-05-12 (deserialize prop-sync needs value-equality gate); 2026-05-17 (ctx fresh every render; same bug class one layer up); 2026-05-17 follow-up (upstream memo fix at LeafRenderer + ctx-ref-internalize closed the ctx half).

P2 — External writes to the persisted frameset need a hook-resync

Trigger. A dialog or command (e.g. HeaderPinsDialog's saveScoped flow) writes the persisted frameset directly to disk via bridge.settings.save, then calls refreshSettings(). Symptom: the live frameset store does not reflect the change because an init-once gate (initRef.current === true) skips the re-read; the user clicks the new pin and lands on a blank page (or a stale layout).

Action. Expose a syncFromSettings(next: AppSettings) resync method from the hook. Any external writer MUST call it AFTER refreshSettings resolves, AND must pass the post-validation snapshot — the return value of refreshSettings(), not the pre-write draft. validateSettings can dedupe/drop/reset values; passing the unvalidated draft re-creates the blank-frame bug class under sanitized saves. The resync must be treated as a load, not a user mutation — gate the resulting state flush so it does not echo back as a spurious debounced re-save (v1 did this with a save-skip counter). Maintain a stable ref to the hook result for callers that can't safely add it to their dep array (would destabilize dnd-kit etc.). Lock the "hook trusts its caller" contract with a unit test: syncFromSettings stores currentFramesetId/id verbatim and does NOT cross-check it against the passed framesets — if anyone adds an internal validity cross-check, that test fails and forces a discussion about where the validation boundary lives.

See. Derived from the #1701 external-write resync work (v1 syncFromSettings mechanics retired into the v1 historical context below; the durable rule is here in P2).

P3 — Vitest/jsdom under-reports React #185 startup loops; jump to Playwright with realistic seed

Trigger. Suspected startup-deterministic regression. Unit tests pass. Production crashes at startup with "Maximum update depth exceeded." OR: the stack trace inside React's bundled module points at dispatchSetState and the call site looks correct in isolation.

Action. Skip past "read more source files" and "run more unit tests" — jump straight to a Playwright e2e seeded with the user's actual settings (via e2e:mock-settings-seed or a saved fixture that mirrors a real .zudotext.settings.json). Vitest/jsdom is more lenient than production React's update-depth counter; mounting a provider with hand-built props once per test will not reproduce the deserialize-on-every-render path that production exposes. If you see React #185 at startup, suspect a provider's prop-sync effect FIRST — not the most-recently-touched feature file. The stack trace inside React's bundled module points at the amplifier (the busy useEffect), not the cause (an ancestor re-rendering hot enough to expose a missing gate).

See. 2026-05-12 ("vitest under-reports this"); 2026-05-17 (startup-no-update-depth-loop.spec.ts is the authoritative regression guard).

P4 — Push stabilization upstream, but ref-internalize unstable caller-supplied deps before removing per-consumer workarounds

Trigger. A downstream module has a workaround (ctxRef, frameContextRef, or similar) for an upstream value that is fresh every render. Upstream gets a memoization fix and the workaround should now be removable.

Action. Verify that every caller-supplied input to the upstream memo's dep array is itself stable across parent renders. Inline arrow callbacks for onTreeChange, fresh bus objects, freshly-constructed handlers — all churn the memo and re-allocate the supposedly-stable upstream value. Either: (a) memoize the caller-supplied inputs at their call site (useCallback/useMemo), OR (b) ref-internalize them inside the upstream module (onTreeChangeRef, busRef) and remove them from the memo's deps. Add a regression test at the upstream module level that mounts with deliberately-fresh callbacks on every rerender and asserts the upstream value's reference stays identical (see packages/frameset/src/leaf-renderer.test.tsx "LeafRenderer — FrameContext identity stability"). Only after both the upstream memo and the ref-internalization are in place is it safe to remove per-consumer ctxRef-style workarounds.

See. 2026-05-17 main entry (initial per-consumer ctxRef pattern); 2026-05-17 follow-up (ctx-ref-internalize branch at LeafRenderer that finished the job and authorized removal of the per-consumer workarounds).

Still-current mechanisms

The layout Prop Persistence Pattern

Providers that offer multiple layouts persist the active layout id in props.layoutId. The full pattern (from inbox-provider.tsx and archives-list-view-provider.tsx):

1. Declare a layouts: LayoutDef[] array on the provider (or use the in-provider switcher for legacy compat).

2. Include layoutId in serialize output — always a string, validated against the set of known ids; falls back to the default id for unknown values.

3. Resolve in deserialize — unknown or missing layoutId returns the first layout's id without throwing (Frame Component Contract section 2e).

4. Set defaultProps.layoutId to the first layout's id.

5. Local state mirrors the persisted blob — the provider holds const [layoutId, setLayoutId] and syncs it from props.layoutId via useEffect. Mutations dispatch a layout-change event (e.g., INBOX_LAYOUT_CHANGE_EVENT) which the page shell picks up and routes through FramesetHandle.replaceProvider to write the new layoutId back into the persisted tree.

For the inbox specifically, the bridge is useInboxLayoutPersistence in tauri-app/renderer/hooks/use-inbox-layout-persistence.ts, which listens for INBOX_LAYOUT_CHANGE_EVENT and calls framesetCommandRef.current.replaceProvider to update the leaf's props in the saved frameset blob. Without this bridge, the layout choice would be lost on reload. (Archives has the mirror bridge useArchivesLayoutPersistence.)

Per-leaf timeline state (timelineState: InboxTimelinePersistedState) follows the same pattern: serialized alongside layoutId, deserialized defensively (all fields have defaults), and mutated through the same INBOX_LAYOUT_CHANGE_EVENT channel.

Note: under Frameset Arch v2, durable per-leaf inbox state is additionally snapshotted to AppSettings.framesetLeafState["core.inbox"] via use-inbox-leaf-state-persistence.ts — the across-quit tier. See l-lessons-frame-frameset-pin-model for the instance-cache + leaf-state contract.

Pre-Release No-Backcompat Policy: when it applies

From CLAUDE.md: schema/contract changes are allowed to be breaking — update producers and consumers in the same change and ship a single coherent version.

This applies to: provider id retirement, props shape changes (replace old props with fresh defaults; no two-way bridging), and persisted settings shape changes (bump the shape; validate on load; ignore unknown fields).

The Directory View → embedded EFE tree change is the current concrete example. There is no provider-id migration or compatibility reader. Old pre-release pins remain opaque and may render provider-not-found until removed or settings are reset. Do not translate the old component-local tree state into EFE. This may discard layout state, never the local files themselves.

Doc Cloud illustrates the paired positive rule: persist only safe leaf seeds (projectSlug, initial surface, editor layout, outline collapse) and keep tabs, raw source, guards, service responses, and credentials out of the frameset. When a cache change and tree change both need persistence, use the owning coordinator's one atomic settings patch; never race two whole-document writes.

It does NOT apply to: user data (drafts, archives, message files — these must always survive) and graceful degradation (a corrupted or incompatible blob must produce a usable, if reset, UI — never a crash or white screen).

Key lesson: "pre-release no-backcompat" means you do not need a two-way migration or to preserve the old layout. It does NOT mean you can white-screen the user — always catch and degrade gracefully.

v1 historical context (Frameset Arch v1 — retired)

The constructs below no longer exist in production. Frameset Arch v2 (#1961) replaced the AppSettings.framesets[] array + currentFramesetId with the singleton AppSettings.frameset and pins-as-templates, and the useSavedFramesets plural hook became the use-saved-frameset.ts singular hook. The v1 migration functions (normalizeLegacyLeaves, migrateRetiredSplitPaneTree, migrateEmptyLeafNodes, the BUILT_IN_FRAMESET_DEFAULTS reset/add algorithms, and the DEPRECATED_EDITOR_PROVIDER_IDS / RETIRED_SPLIT_PANE_PROVIDER_IDS sets) were deleted along with the code they migrated. This section preserves only the durable lessons; the migration-code mechanics are intentionally gone.

Durable lessons that outlived the v1 code:

• Props are opaque to the frameset. A provider-id migration only rewrites providerId and replaces props with fresh defaults — the old props blob is discarded, never transparently forwarded across a shape change (the old core.draft-editor / core.markdown-preview → core.inbox and the editor/terminal-split → unified-inbox migrations both relied on this).

• A migration may reset layout, but never white-screens the user. v1 wrapped each per-frameset migration in a try/catch, collapsed a broken tree to a safe single-leaf inbox on error, and surfaced a non-blocking toast via an onMigrationError callback. Drafts/archives survive because they live in files (inbox/draft*.md), not the frameset blob — only layout state is reset.

• Reset tree and activeFrameId together. Whenever a stale tree was reset, activeFrameId had to be reset alongside it — a stale activeFrameId pointing at a frameId absent from the fresh tree makes the chrome adapter focus a non-existent frame (silent no-op at best, crash at worst). This was Codex finding 2 in the #1512 analysis.

• Required-built-in-page integrity is now structural. v1 needed a BUILT_IN_FRAMESET_DEFAULTS map driving "reset stale" + "add missing" algorithms so inbox/archives/search always existed. Under v2 the singleton tree is rebuilt from a pin template on every pin click, so the whole "missing/corrupt required frameset" bug class is structurally gone.

• EmptyLeafNode is retired. The old { type: "empty-leaf" } union member became a normal LeafNode with providerId: "core.empty". All leaves are now LeafNode | SplitNode; no new code handles an empty-leaf member. (Provider-contract details: l-lessons-frame-component-architecture.)

Reference Files

• tauri-app/renderer/hooks/use-saved-frameset.ts — the v2 singleton frameset hook (owns AppSettings.frameset, cold-load hydration, leaf-state hydrate).

• tauri-app/renderer/view-providers/inbox-provider-layouts.ts — InboxLayoutId, InboxTimelinePersistedState, deserializeTimelineState, isInboxLayoutId.

• tauri-app/renderer/view-providers/inbox-provider.tsx — full serialize/deserialize implementation with layout persistence and the prop-sync value-equality gate (P1).

• tauri-app/renderer/hooks/use-inbox-layout-persistence.ts — the bridge that routes INBOX_LAYOUT_CHANGE_EVENT back into the FramesetHandle.

• tauri-app/renderer/view-providers/archives-list-view-provider.tsx — the layouts[] array pattern + layoutId serialize/deserialize.

• e2e/startup-no-update-depth-loop.spec.ts — the authoritative React #185 regression guard (P1, P3); extend it whenever a new provider mirrors a persisted prop into local state.

2026-05-12 — Provider prop-sync useEffect needs a value-equality gate (React #185 startup crash)

What we set out to do

Stop the replaceProvider round-trip from re-emitting INBOX_LAYOUT_CHANGE_EVENT / FRAME_LAYOUT_CHANGE_EVENT after an externally driven prop rehydration. The fix landed as commit 3c62d33a (refs #1595): update lastDispatchedRef.current = initialState before setLocalState(initialState) inside the prop-sync useEffect.

Approach we tried first

Pre-fix the lastDispatched ref synchronously, then mirror the prop into local state with setTimelineState(initialTimelineState). Dep array: [initialTimelineState]. Assumed React's Object.is bail would skip the re-render whenever the prop's values hadn't changed.

Why it went wrong (root cause)

registry.renderLeaf calls provider.deserialize(leaf.props) on every render of the leaf — see packages/view-provider/src/registry.ts renderLeaf. Each call returns a freshly allocated object. So initialTimelineState is referentially new on every parent render even when its persisted values are identical. The [initialTimelineState] dep treats every render as a change, the effect fires, and setTimelineState(newRef) schedules a re-render via Object.is-distinct references. Under sustained ancestor render pressure during mount (write-page resolving async settings, workspaceDir, savedFramesets — many useEffects landing back-to-back), React's update-depth counter hits the limit before the system settles. Production manifested as React error #185 "Maximum update depth exceeded" at app startup — the inbox provider was the visible call site, but archives-list-view-provider carried the same latent bug.

The structural mistake: a prop-sync effect that uses reference identity as the "did the prop change" signal, even though the prop is generated by a deserialize call that does not memoize its output. With reference identity, the effect can never tell "fresh deserialize of identical values" from "real value change."

What worked instead

Gate the effect on a deep-value comparison against a ref-tracked snapshot of the previous prop value. Only call setState (and mutate lastDispatchedRef) when at least one persisted field actually differs.

const prevInitialTimelineRef = useRef<InboxTimelinePersistedState>(initialTimelineState);

useEffect(() => {
  const prev = prevInitialTimelineRef.current;
  if (
    prev.direction === initialTimelineState.direction &&
    prev.cardWidth === initialTimelineState.cardWidth &&
    prev.showFullContent === initialTimelineState.showFullContent &&
    prev.sort.field === initialTimelineState.sort.field &&
    prev.sort.direction === initialTimelineState.sort.direction
  ) {
    return;
  }
  prevInitialTimelineRef.current = initialTimelineState;
  lastDispatchedTimelineRef.current = initialTimelineState;
  setTimelineState(initialTimelineState);
}, [initialTimelineState]);

The ref is compared against the prop, not against local state — comparing to local state would revert user-driven changes the moment the dispatch effect echoed them back through replaceProvider.

The regression guard lives at e2e/startup-no-update-depth-loop.spec.ts. It boots the app via the mock backend with a realistic framesets seed (inbox + archives + search, with timelineState/gridState blobs) and asserts no "Maximum update depth exceeded" appears in the console. It catches the bug in both WebKit and Chromium.

Watch for next time

• If you add a useEffect keyed on a deserialized provider prop, you must gate on a deep-value check against a useRef-tracked previous-prop snapshot. Never trust reference identity — registry.renderLeaf defeats it by design.

• Compare to the previous PROP, not to local state. A comparison against local state turns the prop-sync effect into a state-revert: the user's local change gets clobbered the next time the dispatch effect round-trips through replaceProvider.

• setState(sameValuesNewRef) is not free. React's Object.is bail prevents the re-render, but the dispatch still adds to the update-depth counter. Under ancestor render pressure (write-page mount, large frameset trees) the counter hits the limit before the bail can settle.

• Vitest/jsdom under-reports this. Mounting the provider with stable props or using rerender() between drained act() blocks will not reproduce the loop. The only reliable signal is an e2e test seeded with realistic framesets — see e2e/startup-no-update-depth-loop.spec.ts and always extend it when adding a new provider that mirrors a persisted prop into local state.

• The CI gap that hid it: every existing unit test mounted provider.render(props, ctx) with a hand-built props object created once per test — never through the deserialize-on-every-render path. CI was green; production crashed at startup.

• If you see React #185 at startup, suspect a provider's prop-sync effect first, not the most-recently-touched feature file. The stack trace inside React's bundled module points to a dispatchSetState call site that is usually the amplifier (the busy useEffect), not the cause. The cause is upstream: an ancestor re-rendering hot enough to expose the missing gate.

Would-skip-if-redoing

Diagnosing through "what changed in the latest commit?" wasted time. The bug had been latent in inbox-provider.tsx since commit 3c62d33a (May 12 05:40); the trigger was just sustained render pressure that the prompts.app settings happened to provide. When a regression looks startup-deterministic, jump straight to a Playwright reproduction with the user's real settings file seeded into e2e:mock-settings-seed — it captured the un-minified stack and pointed at the right file in one shot, after several rounds of source-reading produced nothing actionable.

2026-05-17 — FrameContext is a fresh object every render; never list ctx in a useEffect dep array (#1760)

What was fixed

ExternalFileEditorView's scroll-sync useEffect (in external-file-editor-provider.tsx) had ctx in its dep array:

useEffect(() => {
  // …
  ctx.publish<ScrollSyncPayload>(SCROLL_SYNC_CHANNEL, payload);
  return () => { ctx.publish(SCROLL_SYNC_CHANNEL, null); };
}, [activePath, ctx]); // BUG: ctx is fresh every render

Root cause

makeFrameContext (in packages/frameset/src/frameset.tsx, roughly lines 336, 362, 741) allocates a new object on every render of every leaf. Because ctx is a new reference each render, the scroll-sync effect re-fired on every render:

1. Effect runs → ctx.publish(SCROLL_SYNC_CHANNEL, payload).

2. Any subscriber to the bus calls setState(newPayload).

3. Parent re-renders → fresh ctx → effect dep changes → effect runs again.

4. Loop → React error #185 ("Maximum update depth exceeded") at startup.

The same class of bug was already documented for provider.deserialize(leaf.props) in the 2026-05-12 lesson above. makeFrameContext is the same pattern applied to ctx itself.

Fix applied

1. ctxRef pattern — keep a ref updated with the latest ctx each render; use ctxRef.current.publish(...) inside the effect instead of ctx.publish(...).

2. useMemo for the payload — the ScrollSyncPayload object is memoized on [activePath] so its identity is stable between path changes.

3. Fixed dep array — [activePath, ctx.frameId] instead of [activePath, ctx]. ctx.frameId is a stable string for the lifetime of a mounted leaf.

4. useCallback([]) for handleStateChange — the callback only writes to refs and calls the stable setActivePath setter; zero deps is safe and prevents needless child re-renders.

const ctxRef = useRef(ctx);
ctxRef.current = ctx;

const scrollSyncPayload = useMemo<ScrollSyncPayload | null>(() => {
  if (!activePath) return null;
  return {
    key: `external:${activePath}`,
    getScrollDOM: () => containerRef.current?.querySelector<HTMLElement>(".cm-scroller") ?? null,
  };
}, [activePath]);

useEffect(() => {
  ctxRef.current.publish<ScrollSyncPayload | null>(SCROLL_SYNC_CHANNEL, scrollSyncPayload);
  return () => { ctxRef.current.publish(SCROLL_SYNC_CHANNEL, null); };
  // eslint-disable-next-line react-hooks/exhaustive-deps
}, [activePath, ctx.frameId]);

Regression guard

A render-stress unit test was added to external-file-editor-provider.test.tsx (describe: "scroll-sync loop guard (#1760)"). It mounts the provider with activePath: "/a.md", re-renders 50 times each with a fresh ctx object (same frameId, different reference), then asserts that ctx.publish was called exactly twice (once on mount, once on unmount cleanup). If the bug is reintroduced, the count jumps to 102.

Critical test-authoring note: the stress test uses a stable named Wrapper component (const Wrapper: FC<...> = ...) defined outside act(). Do NOT use inline arrow wrappers (createElement(() => provider.render(...))) — each rerender() call with a new arrow function creates a new component type, which causes React to unmount+remount and fires effects on every call, defeating the assertion.

Upstream follow-up

makeFrameContext in packages/frameset/src/frameset.tsx (approx. lines 336, 362, 741) should be wrapped in useMemo at each call site so that the ctx object identity is stable as long as the leaf's identifying values are unchanged. This is tracked as a follow-up issue "Stabilize makeFrameContext via useMemo at the call sites in packages/frameset/src/frameset.tsx". Until that lands, every provider that receives ctx must apply the ctxRef pattern for any effect that calls ctx.*.

The upstream fix has landed (issue #1770, "Stabilize makeFrameContext at the 3 call sites + remove ctxRef workarounds"). makeFrameContext was extracted to packages/frameset/src/make-frame-context.ts and is now called inside a useMemo in LeafRenderer (keyed on [leaf.frameId, leaf.instanceId, framesetId, onTreeChange, bus]). The three direct call sites in frameset.tsx were removed. With ctx now stable, the ctxRef workaround in external-file-editor-provider.tsx and inbox-provider.tsx was removed — both providers now list ctx directly in their dep arrays. The per-provider ctxRef pattern is no longer needed for providers that receive ctx through LeafRenderer.

Rule going forward

ctx is now stable (memoized in LeafRenderer via useMemo). Providers CAN safely list ctx in useEffect, useMemo, or useCallback dep arrays. The old rule (always use ctxRef) is superseded — the ctxRef pattern is no longer necessary and should not be added to new providers.

Follow-up fix: ctx refs internalized inside LeafRenderer (ctx-ref-internalize branch)

The upstream memoization in LeafRenderer was incomplete. Sub-A (#1770) keyed the useMemo on [leaf.frameId, leaf.instanceId, framesetId, onTreeChange, bus], but every Frameset caller (write-page, archives-page, frameset-host-page) passes an inline arrow for onTreeChange — a fresh reference on every parent render. With onTreeChange in the deps list, the memo invalidated every render, ctx became a fresh object every render, and the React #185 loop persisted.

The fix (ctx-ref-internalize) moved onTreeChange and bus behind mutable refs inside LeafRenderer:

const onTreeChangeRef = useRef(onTreeChange);
onTreeChangeRef.current = onTreeChange;
const busRef = useRef(bus);
busRef.current = bus;

The useMemo deps were reduced to only the three truly-stable identifiers: [leaf.frameId, leaf.instanceId, framesetId]. Inside makeFrameContext, each method reads busRef.current / onTreeChangeRef.current at call time rather than capturing the value at construction time. This means the ctx object never changes identity regardless of how often callers replace the underlying callbacks.

Key rule: removing per-provider ctxRef workarounds is only safe if the upstream memoization actually stabilizes ctx — and that requires that none of the memo deps churn. Any caller-supplied value that is not guaranteed stable across parent renders must be accessed through a ref, not appear in the deps list.

The new upstream guard test lives at packages/frameset/src/leaf-renderer.test.tsx (describe: "LeafRenderer — FrameContext identity stability (regression guard #1770)"). It mounts LeafRenderer with a stable leaf but freshly allocated onTreeChange arrow and bus on every rerender (6 total), then asserts the captured ctx reference is identical across all renders. A third test asserts that after a mid-life bus swap, ctx.publish routes to the new bus (proving call-time deref). Vitest/jsdom was sufficient for this unit guard because we are testing reference identity of a memoized object — not the React update-depth counter. The e2e startup spec (e2e/startup-no-update-depth-loop.spec.ts) remains the authoritative regression guard for the #185 loop itself.