fix(sequences): make the owned frame provider StrictMode-remount safe

[vutuanlinh2k2]

Follow-up to #59 (stacked on its branch — the diff here is just the frame-provider change). Addresses the second weak concern from the value-audit on #59.

Problem

TimelineEditor's owned VideoFrameProvider used the same anti-pattern the clock fix removed: created in a `useMemo` (ref-cached), disposed in a separate `useEffect` cleanup (TimelineEditor.tsx:171-181, pre-fix). Under React StrictMode's mount → unmount → remount probe the cleanup disposes the provider and nulls the ref, and — since the probe re-runs effects but not render — nothing rebuilds it. Render is left holding a disposed provider.

Why it didn't crash like the clock (and why it's still worth fixing)

The provider is consumed only by `PreviewCanvas`, via `await provider.drawFrame(...)` inside an async, try/caught paint loop (PreviewCanvas.tsx:105, 139-149). `drawFrame` is async, so an `acquire`-on-disposed throw becomes a rejected promise that's caught and surfaced as a non-fatal `drawError`. There is no synchronous provider call inside an effect or render, so it can't trip the route error boundary the way the clock's synchronous `seek` did. Realistic impact: a dev-only blank/again-degraded preview after a StrictMode remount — not a crash. Still a real latent lifecycle defect, and it leaves the two resources inconsistent.

Fix

Move provider creation and disposal into one effect keyed on `props.frameProvider`, exposed through state — mirroring the clock fix:

• Every (re)mount rebuilds the owned provider; render-time consumers share the live instance.
• An ownership ref (`ownsFrameProviderRef`) ensures a caller-supplied provider is used as-is and never disposed here; only a provider the editor creates is disposed (including the initial placeholder, via dispose-the-outgoing).

Testing

Two new tests under `StrictMode`:

• A mount keeps a live owned provider after the probe — fails on the prior pattern (every created instance ends disposed), passes with the rebuild.
• A caller-supplied provider is never disposed across the probe or a real unmount (locks the ownership invariant).

`pnpm typecheck` ✅ · `pnpm test` ✅ (1221 passed) · `pnpm build` ✅

Note: the StrictMode tests require the clock fix from #59 to mount the editor at all, which is why this is stacked on that branch. Once #59 merges, GitHub will retarget this PR's base to `main`.

[tangletools]

✅ Auto-approved PR — 16279e76

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

_{tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-15T08:27:12Z}

[tangletools]

🟢 Value Audit — sound

Verdict	sound
Concerns	0 (none)
Heuristic	0.0s
Duplication	0.0s
Interrogation	153.9s (2 bridge agents)
Total	153.9s

💰 Value — sound

Applies the same StrictMode-safe, effect-owned lifecycle to the optional owned VideoFrameProvider that the prior clock fix established; adds targeted regression tests and preserves caller-provider ownership. A coherent follow-up fix that should ship.

• What it does: Moves ownership of the editor's optional VideoFrameProvider from a render-time useMemo (ref-cached) + separate useEffect cleanup into a single effect keyed on props.frameProvider, with the live instance exposed through useState (TimelineEditor.tsx:196-210). An ownsFrameProviderRef tracks whether the current instance was created by the editor; only editor-owned providers are disposed, w
• Goals it achieves: Eliminates the latent lifecycle defect where React StrictMode's remount probe disposes the ref-cached owned provider and leaves render holding a disposed instance (previously: create in useMemo, dispose in useEffect cleanup). It makes the editor's disposable-resource lifecycle consistent with the playback-clock fix already merged in commits 4338a24 and 170fa4f, and it hardens the ownership inv
• Assessment: Good change, built in the grain of the codebase. It directly mirrors the established clock pattern in the same file, keeps the fix localized, and is well-commented. The tests are specific to the defect and ownership invariant rather than generic coverage. No existing reusable abstraction is bypassed, and the scope is proportionate to the lifecycle bug being fixed.
• Better / existing approach: none — this is the right approach. I searched the repo for an existing disposable-resource hook or StrictMode lifecycle utility (grep for useLifecycle, useDisposable, useResource, useMount, StrictMode in tests, and dispose usage across src/sequences-react) and found no shared implementation. With only two resources (clock and frame provider) using this pattern, both in the same compone

🎯 Usefulness — sound

The change fixes the default owned frame-provider lifecycle in TimelineEditor to match the clock StrictMode-remount fix already landed in the same file; it is reachable, seam-consistent, and tested.

• Integration: TimelineEditor is the exported root of the sequences-react component surface (src/sequences-react/components/index.tsx:7) and has a lazy code-split entry (src/sequences-react/lazy.tsx:10). The owned provider is the default path whenever a caller omits frameProvider (contracts.ts:161), and the instance flows to PreviewCanvas (TimelineEditor.tsx:611), TimelineTrackRow (line 697), and TimelineClipC
• Fit with existing patterns: It extends the exact effect-owned lifecycle pattern already applied to the playback clock in commits 4338a24/170fa4f, with the same placeholder-state + effect-replace + dispose-outgoing shape. It also preserves the ownership seam: caller-supplied providers are never disposed by the editor, which matches the contract that frameProvider is an optional override (contracts.ts:160-161).
• Real-world viability: The effect-owned lifecycle correctly rebuilds the provider across React StrictMode's mount/unmount/remount probe instead of leaving a disposed instance in render. Ownership handoff is covered in both directions: an owned provider is disposed when switching to a caller-supplied one, and external providers are never disposed on unmount or remount. The async draw paths in PreviewCanvas and TimelineCl

No concerns — sound change, no better or existing approach found. ✅

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260615T083128Z}

[tangletools]

✅ No Blockers — 16279e76

Readiness 89/100 · Confidence 70/100 · 3 findings (3 low)

	deepseek	glm	aggregate
Readiness	92	89	89
Confidence	70	70	70
Correctness	92	89	89
Security	92	89	89
Testing	92	89	89
Architecture	92	89	89

Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision.

🟡 LOW Render/effect timing gap during provider switch — src/sequences-react/components/TimelineEditor.tsx

When props.frameProvider changes value mid-lifecycle, the render uses stale state (frameProvider from previous effect) before the new effect updates it. This is a one-render-cycle gap inherited from the clock pattern (same structure at lines 157-187). The stale provider is either still-alive (if switching away from owned) or caller-managed (if switching to caller's). In the owned→caller case, the previous cleanup disposes the owned provider, and the new effect's frameProviderRef.current.dispose() is harmless (idempotent). No user-visible bug, but worth documenting as a conscious tradeoff.

🟡 LOW useState initializer leaks one empty provider under StrictMode double-invoke — src/sequences-react/components/TimelineEditor.tsx

Line 196-198: useState<VideoFrameProvider>(() => props.frameProvider ?? createVideoElementFrameProvider()). In React StrictMode development, the initializer runs twice; React discards the first result without disposing it. The leaked provider holds only empty Maps (video pool, image pool, kind cache, draw queue) — no DOM elements or timers — so it is immediately GC-eligible. This is a pre-existing pattern also present in the clock initializer at line 158-160 and is development-only. No p

🟡 LOW Owned-provider survival assertion is loose — tests/sequences/timeline-editor-components.test.ts

Line 515: expect(created.some((instance) => !instance.disposed)).toBe(true) asserts at-least-one-undisposed rather than exactly-one. Under the fix, StrictMode produces 3 instances (placeholder from useState init, effect-created, remount-created) where exactly 1 survives undisposed. The looser assertion would still pass if the component leaked multiple live providers (e.g. failed to dispose the placeholder). Not a bug in the current code — the component correctly disposes — but a stricter created.filter(i => !i.disposed).length === 1 would catch a future leak regression the current assertion misses.

---

_{tangletools · 2026-06-15T08:38:33Z · trace}