fix(studio): restore golden-branch timeline behaviors dropped by the stack rebuild

[ukimsanov]

What

Restores six timeline behaviors from the reviewed studio-dnd stack (studio-dnd/consolidated-tip) that the Studio rebuild (#2291) dropped or regressed, fixes the stale timelineZones.ts that #2279 introduced and #2291 never repaired, and adds two small sidebar-asset UX improvements.

Why

A golden-branch-vs-main comparison after the Studio cutover found five verified regressions behind user-visible symptoms, plus one new bug found by pointer-driving the result:

1. Gridlines + non-sticky ruler — feat(studio): revamps Studio + improves code quality #2291 absorbed most of the golden "timeline visual refresh" (086fdabda) but not its TimelineRuler.tsx/PlayerControls.tsx hunks: main kept the pre-refresh ruler that draws full-height gridlines (visible in the gaps above/below opaque tracks) and scrolls away vertically.
2. Stale timelineZones.ts — main still ran feat(studio): timeline collision and placement model #2279's z-driven lane pack, while its own timelineClipDragCommit.ts — main's post-feat(studio): revamps Studio + improves code quality #2291 rewrite, which kept golden's normalizeToZones call contract (correction per review: the file itself equals main's, not golden's) — has insert-band commits that contractually depend on the golden track-driven normalizeToZones (lane = authored data-track-index; z = paint order only).
3. Track creation failed silently — persistTimelineBatchEdit treated any no-op batch member (attributes already at target values, which a track-insert renumber legitimately produces) as a fatal targeting error, aborting and rolling back the whole insert. The golden batch patcher skips no-op members (timelineElementsMove.ts:116).
4. Canvas blink on every move/resize — the rebuilt timing fallback discarded the server's rewritten GSAP scriptText and called reloadPreview() (a full iframe remount) unconditionally; golden soft-reloads the script in place and full-reloads only when it can't.
5. Duration readout dead on trim — the rebuild kept only a grow-only root-duration ratchet; golden persists content-driven grow-AND-shrink and syncs the readout from the just-patched preview DOM at gesture release.
6. Stacking-sync contract violation — readClipZIndex was reverted to fabricate 0 for unresolvable clips while timelineStackingSync still implements the golden Number.isFinite exclusion contract, skewing z-boundary math.

How

• TimelineRuler.tsx adopted from golden (sticky top-0, beat-lines-only background, frame labels); timeDisplayMode lifted into playerStore (persisted via studio UI preferences); PlayerControls toggle store-backed (also drops the transport-bar borderTop, a cosmetic hunk of the same golden visual-refresh commit — disclosed per review).
• timelineZones.ts + test replaced with the golden stable-track-lanes version; timelineClipDragCommit.test.ts updated surgically (main-only optimistic-revision coverage kept).
• persistTimelineBatchEdit skips no-op members and returns early when nothing changed; regression tests added.
• New hooks/timelineTimingSync.ts (extracted to stay under the 600-line gate): GsapMutationStatus carries scriptText, syncTimingEditPreview soft-reloads via applySoftReload, group edits soft-reload only when every change targets the active comp (one full reload otherwise). The server already returned scriptText; the client had discarded it.
• Move/resize patch builders end with setCompositionDurationToContent(furthestClipEndFromSource(...)) (grow-or-shrink); syncPreviewContentDuration updates the store + live root data-duration synchronously at release; delete shrinks too. Main's optimistic-revision rollback, operation tags, PostHog commit events, and runGestureTransaction canvas paths are untouched.
• timelineAssetDrop.ts restored to golden: drops land on the drop track (no overlap bump to max-track+1), data-hf-id stamped, audio gets data-volume.
• UX: sidebar asset click opens a compact non-modal preview over the canvas (dismiss on outside click / Escape / playback / seek), and clicking an already-added asset scrolls the timeline to that clip's time and lane (new pure timelineRevealScroll math + clipRevealRequest store channel; vertical-only in fit zoom).

Test plan

• Unit tests added/updated (batch no-op skip, reveal-scroll math, preview dismiss rule, store reveal channel, zones contract, asset-drop no-bump/hf-id) — studio suite 2040 passing; the 18 failures in telemetry/* + SnapToolbar fail identically on pristine main (Node-26 localStorage environment issue)
• tsc --noEmit clean; oxlint/oxfmt clean; full workspace build green; all pre-commit gates (filesize, fallow, lint, format, typecheck) pass
• Manual pointer-driven verification on a real project: ruler stays pinned while scrolling (gridlines gone); moving/trimming a clip never remounts the preview iframe (instrumented marker survives; GSAP tween positions rewritten in place on disk); duration readout updates 40→37→40 on shrink/stretch at release; dragging a clip into the top insert band creates a new top track and renumbers lanes correctly on disk; single-clip move persists exactly one element's attributes (file-diff invariant)

[miguel-heygen]

Review: restore golden timeline behaviors

Reviewed the diff three ways: PR vs main, PR vs golden (studio-dnd/consolidated-tip), and semantic verification of the files that were reinterpreted rather than copied. Method: for each of the six restore claims I checked that the regression on main is real, that the restored code matches golden, and (for the non-identical files) that the port preserves golden's semantics against main's post-#2291 surrounding code.

Overall this is sound and mergeable for its stated scope. Every regression claim I sampled is truthful, the six exact-restore files are byte-identical to golden with their dependency contracts intact, and the load-bearing soft-reload ("blink") fix is faithful to golden, including the authored-opacity restore path. Requesting changes for three small, user-visible fixes plus a documentation correction; the rest are follow-ups.

Fidelity summary

Claim	Regression real	Restore faithful
#1 Gridlines + sticky ruler	yes	byte-identical to golden
#2 Stale timelineZones (z to track lanes)	yes	byte-identical; consumer contract holds
#3 Batch no-op silent fail	yes	faithful, but shares golden's latent flaw
#4 Canvas blink (soft-reload)	yes	core logic byte-identical; one new edge risk
#5 Duration grow-and-shrink	yes	server path correct; two gaps
#6 Stacking-sync NaN contract	yes	traced end-to-end, no || 0 coercion
Asset-drop restore + reveal/preview UX	net-new	three UX bugs

The NaN exclusion chain (#6) was traced Timeline.tsx to timelineClipDragCommit.ts to computeStackingPatches with no coercion, so restoring readClipZIndex to return NaN correctly activates the pre-existing Number.isFinite filter. The soft/full reload gating (#4) is byte-identical to golden, and the mixed-file group edit does exactly one full reload when a change targets a non-active comp.

Please fix before merge

1. Stuck preview overlay when revealing an already-added asset
components/sidebar/AssetCard.tsx:151-158 (and the mirror in AudioRow.tsx): the used reveal branch does setSelectedElementId + requestClipReveal + return and never calls clearPreviewAsset. Repro: open the preview overlay on a not-yet-added asset A, then click a different, already-added asset B in the sidebar. A stays stuck open while the timeline scrolls to reveal B underneath it. The only in-scope callers of clearPreviewAsset are the overlay's own dismiss handlers and project-switch, so nothing clears it here. Add a clearPreviewAsset() to the reveal branch.

2. Duration readout is not rolled back on a failed write
Golden captures previousDuration and reverts both the store and the live root in its catch (timelineElementsMove.ts:312-314). The PR sets the duration optimistically via syncPreviewContentDuration at gesture release (useTimelineGroupEditing.ts:222,311) but has no previousDuration capture and no rollback. A failed save leaves the store and live root data-duration advertising a length that was never persisted, until the next reload. This is on the default (server) path, not gated by any flag. Restore golden's rollback.

3. Dismiss-on-playback is edge-triggered only
components/nle/AssetPreviewOverlay.tsx:102-108 evaluates shouldDismissAssetPreview only against future usePlayerStore.subscribe mutations. Playback drives currentTime outside Zustand in the RAF loop and only writes the store at the end of the range or on loop wrap, so a preview opened while playback is already running does not dismiss until then, contrary to the "dismiss on playback" contract. Check the current store state once at subscribe time (or gate opening the preview on !isPlaying).

Worth fixing while this code is open

4. Batch no-op skip cannot distinguish a legitimate no-op from a mistargeted member
hooks/timelineEditingHelpers.ts:328 treats patched === current as a skip. But applyPatchByTarget (via findTagByTarget in utils/sourcePatcher.ts) returns the string unchanged both when the element is already at the target value (legitimate no-op) and when the target is not found at all (a real error). So a wrong domId/hfId/selector is silently dropped instead of surfacing. This is faithful to golden (timelineElementsMove.ts has the same ambiguity), so it is not a regression this PR introduces, and it is why the primary claim (track-insert renumber no longer aborts) is correctly fixed. But the single-element path throws in exactly this case, so the two paths now disagree. Cheap root-cause fix: resolve the target with findTagByTarget first and throw on null, only treating byte-identical output as a no-op when the target actually resolved.

5. New silent stale-preview surface introduced by the history-fold extraction
hooks/timelineTimingSync.ts: foldGsapMutationIntoHistory runs the GSAP server rewrite, then does a readFileContent/recordEdit fold step afterward. If that post-mutation step throws, it propagates to finishTimelineTimingFallback's catch, which calls onGsapError (console only) and returns, discarding the already-rewritten scriptText with no syncTimingEditPreview and no reloadPreview. The preview is left showing the pre-rewrite GSAP positions with no recovery. Golden's inline path had no fold step between the mutation and the sync, so this failure mode is new to the extraction. Sync the preview in a finally, or catch the fold step separately so a history-record failure never suppresses the reload.

6. timelineRevealScroll has no guard for a degenerate viewport
player/components/timelineRevealScroll.ts:60: windowSize = windowEnd - windowStart is never guarded against <= 0. When the scroll container is smaller than stickyStart + 2 * padding (reachable at the minimum timeline height combined with the caption-mode footer), the oversized-clip branch still fires and returns a target that places the clip top at stickyStart + padding, off-screen in a viewport that small. The reveal silently scrolls but the clip stays hidden. No test covers windowSize <= 0. Add a guard and a test.

Non-blocking

• The description's claim that timelineClipDragCommit.ts is "already golden" on main is incorrect: its blob equals main's, not golden's (it is missing golden's beginTimelineOptimisticGesture race guard, a separate feature). This is functionally harmless because the normalizeToZones call site is identical and only reads .track/.key, but the description misrepresents a dependency; please correct it.
• player/components/PlayerControls.tsx bundles an undisclosed cosmetic change (removal of the transport-bar borderTop) not covered by any claim. Harmless, but call it out or drop it.
• The SDK fast path (sdkTimingPersist) writes clip attributes but not root data-duration, so a shrink routed through it would leave the persisted root duration stale. This is currently dormant (STUDIO_SDK_CUTOVER_ENABLED defaults false, and the caller falls through to the server path when it returns false), but it is a real gap to close before that flag flips on.

Duration edge cases (deleting the furthest clip, empty timeline, clip at time 0) are all handled correctly on the server path: setCompositionDurationToContent rejects a content end <= 0 and keeps the prior duration, and the start < 0 exclusion correctly counts a clip at start 0.

[ukimsanov]

Second commit (fefbde4) — found while feel-testing the first round, plus an adversarial review pass over the whole z/lane pipeline:

Vertical lane moves didn't persist — three independent causes, all fixed:

1. timelineMoveAdapter stripped the track from lane-reorder persists (a remnant of the old z-driven lane model).
2. Display lanes vs authored file tracks are different coordinate spaces: normalizeToZones packs sparse authored tracks onto contiguous display lanes, and lane edits persisted the lane number — silently re-targeting the wrong row in any file whose numbering isn't 0-contiguous (very common: DOM-index fallbacks, gaps). Elements now record authoredTrack; lane changes persist in file space while the store stays in lane space. Live-verified on a sparse file.
3. The runtime's normalizeTrackAssignments split same-kind tracks at discovery and renumbered everything, so authored indices never round-tripped. Removed (the studio-dnd stack had already deleted it with the same rationale; render never reads track index).

Review findings fixed: parseInt(...) || dropping authored track 0; single-clip fallback persisting moves without the lane; lane-change z-sync ignoring group time shifts; track inserts persisting colliding renumbers around locked clips (now refused); computeStackingPatches comparing leaf z across CSS stacking contexts (now partitioned by stackingContextId).

Timeline geometry: 20% fit-zoom trailing headroom (single shared constant), playhead line center exactly on GUTTER + t·pps (DOM-verified delta 0), frame-mode ruler steps snap to whole frames, hour steps for far zoom-out, exact-multiple tick positions.

Known follow-up (deliberately not in this PR): ~1,000 lines of dead old-model code (the z-driven vertical-drag pipeline: timelineLayerDrag.ts, applyTimelineStackingReorder, resolveTimelineMove's stacking branch, timelineTrackOrder.ts, timelineDropIndicator.ts) — verified zero production callers; deleting is a separate cleanup.

[miguel-heygen]

@ukimsanov rebase main

[miguel-heygen]

Follow-up review: commit 2 (fefbde4) z/lane pipeline, single-source-of-truth pass

Thanks for the second round. The vertical-lane persistence fixes and the display-lane vs authored-track split are the right architecture, and the adversarial pass caught real bugs. Reviewing this commit specifically through a single-source-of-truth lens (one owner per decision, one authoritative source per field, no stale plumbing) surfaces that the fix is incomplete at two construction boundaries, ships one mechanism that is inert in production, and leaves two z-models coexisting. Findings are ranked; each was traced to file and line.

The common root cause

createTimelineElementFromManifestClip (packages/studio/src/player/lib/timelineDOM.ts:115) is the single translation boundary from the runtime manifest to a studio TimelineElement. It sets track: clip.track but drops two authoritative fields that the runtime computes: authoredTrack and stackingContextId. Findings 1, 2, and 4 all trace back here. Carrying both fields through this factory is the single-source-of-truth fix; the drag helpers should read them from a complete element rather than reconstruct them from display state.

Ranked findings

1. Critical: expanded sub-composition drags still persist a display lane into data-track-index.
The factory above never sets authoredTrack. buildChildElements (packages/studio/src/player/hooks/useExpandedTimelineElements.ts, around line 166) then overwrites track with display.track + offset, permanently discarding any authored value. authoredTrackForLane (packages/studio/src/player/components/timelineClipDragCommit.ts:169-175) only searches the top-level elements array, which does not contain expanded children, so dragging an expanded child onto an empty display lane returns the display-lane integer as the authored track. That is the exact coordinate-space bug this commit set out to fix, still reachable for sub-composition children on a sparse file.

2. High: the new stackingContextId partitioning is unwired in production.
stackingContextId is declared on the store type (packages/studio/src/player/store/playerStore.ts:44) and the wire type, computed by the runtime (packages/core/src/runtime/timeline.ts:404,513,568), and consumed by timelineStackingSync (packages/studio/src/player/components/timelineStackingSync.ts:301). But createTimelineElementFromManifestClip never copies it, and every runtime clip routes through that factory, so every production clip resolves to null (root context) and the per-context partitioning is a no-op. The new stacking tests inject stackingContextId directly, so they pass while production never exercises the wiring.

3. High: optimistic store updates never refresh authoredTrack.
A second drag that lands before the reload completes computes the authored track from stale store data.

4. High: spill-lane drops have no distinct authored identity, and the guiding comment is wrong.
authoredTrackForLane's docstring states overlap sub-lane spills are "never a lane-move target," but Timeline.tsx's trackOrder (grouped by el.track from packTrackLanes) includes every display sub-lane, including spill lanes, as legal drop targets. A drag onto a spilled lane is reachable; the persisted track then collapses on reload. Please fix the comment and the handling together.

5. Medium: the non-atomic single-element move path drops vertical-only moves.
packages/studio/src/hooks/useTimelineEditing.ts:144-161: applyTimelineStackingReorder runs, then if (!startChanged) return reorderDone returns before the file-track persist below it. A pure lane change (no time change) routed through the onMoveElement-only fallback writes nothing to the file.

6. Medium: two z-models coexist (stale plumbing).
The old z-reorder vocabulary (timelineLayerDrag.ts resolveTimelineLayerZIndexChanges, applyTimelineStackingReorder, TimelineMoveInput.stackingElement, plus their tests) is dead by dataflow: the drag-commit path passes onMoveElement a StartTrack with no stackingReorder, so applyTimelineStackingReorder (useTimelineEditing.ts:152) always early-returns on a null intent, and the only producer branch (timelineEditing.ts:132) is unreachable because timelineClipDragPreview.ts never passes stackingElement. No current drag double-writes z-index and data-track-index, so this is not a live bug. But the model is still typed, still executed during preview (it computes a stackingReorder that is then discarded at commit), and still test-covered, and the description's "zero production callers" is imprecise: applyTimelineStackingReorder has a live call site, just fed undefined. Deferring the deletion is reasonable, but please track it as stale plumbing rather than describe it as already dead.

7. Low: stackingContextId equality is recomputed in three places inside computeStackingPatches rather than through one canonical contextKey helper.

Test-coverage gap (the tell)

Every new test injects authoredTrack and stackingContextId directly rather than constructing elements through createTimelineElementFromManifestClip, which is why findings 1 and 2 shipped green. A single pipeline test from sparse authored tracks through expansion, drag commit, and file patch would have caught both. Please add coverage that crosses the real factory boundary.

Requesting changes for findings 1 and 2 (correctness and a dead feature); 3 through 5 are worth fixing in the same pass, and 6 through 7 are cleanup that can follow.

[ukimsanov]

Third commit (56da0bb) — adversarial review of the canvas z-order context menu (Bring to Front / Forward / Backward / Send to Back), requested after feel-testing.

Resolver verdict: sound. Tie-aware (equal z → DOM order, matching CSS), minimal-churn fast path, band-constrained renumber fallback, no negative z, no unbounded growth. Live-verified: bring-to-front = one atomic single-element write; menu items disable correctly at the extremes from fresh state.

Glue defects fixed:

1. Ship-blocker — the menu pre-applied style.zIndex + position: relative before the commit hook ran, so the hook's getComputedStyle === 'static' check never fired and the position patch was never persisted → z-order on nested/static elements applied visually then silently reverted at the post-commit reload. Same pre-apply made failure rollback restore the mutated values. Menu no longer pre-applies (the hook already applies both live, synchronously).
2. Store-sync entry.key plumbing had zero production callers — wired at all three call sites via a new deriveTimelineStoreKey helper, so store zIndex updates synchronously instead of waiting for full reload.
3. Client ignored the server's per-patch matched[] — unresolvable siblings now warn + report z-reorder-unmatched telemetry instead of persisting partially in silence.
4. <template>/<noscript> no longer count as painting siblings (renumber fallbacks were writing z-index into <template> tags in source).
5. Distinct z actions within 300ms no longer coalesce into one undo entry (action kind added to the default coalesce key).

Deferred to the dead-code cleanup follow-up: multi-file partial-persist orphaning (unreachable from the menu), unifying LayersPanel's stamp-everything computeReorderZValues onto the menu's minimal resolver, and the dead z-derives-lanes cluster.

Gates: all pre-commit hooks green; studio suite 2058 passing (same 18 pre-existing environmental failures).

[ukimsanov]

Fourth commit (b751d00): flashless z-order commits (skip the redundant iframe remount when every persisted op is inline-style and every patch matched — verified live: an iframe-scoped marker survives the action) + Bring Forward/Send Backward now step over the next visibly overlapping sibling instead of invisible time-inactive neighbors, with menu enable-states matching (no more 'enabled but nothing happens') and a 600ms studio-overlay flash on the crossed element. All gates green; studio suite 2076 passing (same 18 environmental).

[ukimsanov]

@miguel-heygen Both reviews addressed in 77c4fca — point-by-point:

Review 1:

• ✅ Stuck preview overlay: the used/reveal branch in AssetCard/AudioRow now calls clearPreviewAsset(); component tests mount both with a preview open on a different asset.
• ✅ Duration rollback: new captureDurationRollback (golden's previousDuration pattern) snapshots store + live root data-duration BEFORE the optimistic sync and restores both in every move/resize/delete/group-edit failure path; 5 failed-persist tests.
• ✅ Dismiss-on-playback: the overlay now evaluates the current store state once on open — already-running playback dismisses immediately; subscription kept for the rest.
• ✅ Batch no-op vs mistargeted member: persistTimelineBatchEdit resolves the target via findTagByTarget first and throws on null (same wording as the single path); identical output is a no-op only after the target resolved.
• ✅ History-fold failure: failure domains split — a post-mutation fold error is surfaced separately (onFoldError) and the rewritten script still syncs to the preview; a mutation failure still skips the sync.
• ✅ Degenerate viewport: revealAxis returns no-scroll when windowSize <= 0, tested.
• ✅ CodeQL ×3: encodeURIComponent(projectId) at all three fetch sites.
• ✅ Description corrected (the timelineClipDragCommit "already golden" claim) and the PlayerControls borderTop removal disclosed.

Review 2:

• ✅ Findings 1+2 (factory boundary): createTimelineElementFromManifestClip now carries authoredTrack (manifest clip.track is authored-verbatim post the runtime fix) and stackingContextId (wire type already had it — playbackTypes.ts:42); buildChildElements preserves both on expanded children; authoredTrackForLane scopes occupants to the dragged clip's sourceFile with a nearest-same-file-lane fallback, so an expanded child on a sparse file can no longer persist a display integer or borrow a foreign file's coordinate space.
• ✅ Finding 3: optimistic store writes mirror the persisted track into authoredTrack (rolled back on failure), so consecutive drags before a reload resolve fresh.
• ✅ Finding 4: comment + handling reconciled — a spill sub-lane drop is a legitimate same-track join (occupants share the authored track by construction; deterministic re-pack); the false "never a lane-move target" docstring is rewritten, and the foreign-file wrong-value path is closed by the sourceFile scoping.
• ✅ Finding 5: the single-element fallback persists vertical-only moves (early return requires neither start nor track changed; live DOM patch includes data-track-index; SDK start-only path bypassed when the track changed).
• ✅ Finding 7: canonical contextKey helper.
• ✅ Test-coverage ask: new timelineTrackPersistPipeline.test.ts crosses the REAL factory boundary — sparse manifest (tracks 3/7, mixed kinds) → factory → expansion → normalizeToZones → commitDraggedClipMove → asserts the persisted attribute is the authored value, no injected fields.
• Finding 6: agreed and rephrased — the old z-model is stale plumbing with a live-but-null-fed call site, not "dead"; its deletion is tracked for the follow-up cleanup PR rather than this one.

Full studio suite 2104 passing (the 18 failures are the pre-existing Node localStorage environment set, identical on pristine main); all pre-commit gates green. Re-requesting review.

[miguel-heygen]

Approving: both review rounds verified fixed in 77c4fca

Verified the fixes adversarially against the tree (not taken from the summary), and ran the affected suites (timelineStackingSync, timelineTrackPersistPipeline, timelineClipDragCommit, useTimelineEditing) at 77c4fca, 84 tests green. Every blocking finding from both rounds is genuinely fixed:

Round 1

• Stuck preview overlay: clearPreviewAsset() now runs in the reveal branch of both AssetCard and AudioRow, with a component test mounting a preview open on a different asset.
• Duration rollback: captureDurationRollback snapshots store + live root before the optimistic sync and restores both on failure across move/resize/delete and both group-edit paths.
• Dismiss-on-playback: the overlay now evaluates store state on open.
• Batch no-op root cause: persistTimelineBatchEdit resolves the target with findTagByTarget first and throws on null, so identical output is a no-op only after the target resolved. This is the mistarget-vs-no-op distinction I asked for.
• History-fold failure split: a post-mutation fold error is surfaced via onFoldError and the rewritten script still syncs; a mutation failure still skips the sync. Two distinct tests.
• Degenerate viewport guard and the three encodeURIComponent(projectId) fixes confirmed.

Round 2 (single-source-of-truth)

• Findings 1+2: createTimelineElementFromManifestClip now carries both authoredTrack and stackingContextId; buildChildElements preserves them on expanded children; authoredTrackForLane is scoped to the dragged clip's sourceFile. The new timelineTrackPersistPipeline.test.ts crosses the real factory boundary (sparse tracks 3/7 through factory, expansion, normalize, commit) and asserts the persisted attribute is the authored value, no injected fields. This closes the critical persist bug and wires the previously inert stackingContextId partitioning.
• Finding 3: optimistic writes mirror the persisted track into authoredTrack (rolled back on failure), so a second drag before reload resolves fresh.
• Finding 4: the false "never a lane-move target" docstring is corrected and a spill-lane drop is handled as a same-track join.
• Finding 5: the single-element fallback now persists vertical-only moves (early return requires neither start nor track changed; live patch includes data-track-index; SDK start-only path bypassed when the track changed).
• Finding 7: a canonical contextKey helper now owns stackingContextId equality.
• Finding 6: accepted as stale plumbing, deletion tracked for a follow-up cleanup PR. Framing corrected.

Two non-blocking follow-ups (fine to land after)

1. Duration rollback has failed-persist tests for move/resize/group but not for the delete path. The delete-path code is correct by inspection; add the assertion to match.
2. The dismiss-on-playback on-open check inlines only the isPlaying branch rather than reusing the shared shouldDismissAssetPreview, so a preview opened at the exact moment a seek is already in flight (isPlaying: false, requestedSeekTime != null) is not dismissed until currentTime diverges. Narrow, and a small duplication of the dismiss predicate. Reuse shouldDismissAssetPreview on open to close both.

One thing outstanding before merge

The branch is not rebased onto main (merge-base is 95fa14b2; main has since advanced). That rebase was requested earlier and still needs to happen before this goes in.

Approving on correctness. Nice work on the point-by-point turnaround, and on the pipeline test that crosses the real factory boundary, which is what makes findings 1 and 2 stay fixed.

[miguel-heygen]

Please rebase onto main instead of merging main in

The code is good, I re-verified that the five PR commits are content-unchanged from the head I approved (range-diff clean) and CI is green on the current head. The only issue is history hygiene.

The conflict was resolved by merging origin/main into the branch (merge commit 8013b40bf, "Merge remote-tracking branch 'origin/main'..."), but we keep this repo on a linear history. Please rebase onto main instead so the branch is the five fix commits replayed on top of current main, with no merge commit:

git fetch origin
git rebase origin/main
git push --force-with-lease

Once it's rebased clean (no merge commit, linear on main) I'll re-approve right away. Nothing about the code needs to change, this is just to drop the merge commit and keep the history straight.

[ukimsanov]

Rebased as requested: the branch is now the six fix commits replayed linearly on top of current main (d04bcbf), no merge commit (git log --merges empty). Tree verified identical to the head you approved plus main's own commits — the only delta in PR-touched files is a single blank line in timelineEditingHelpers.ts (the rebase carries the commit's exact blob; the merge resolution had introduced one extra newline). The sixth commit adds your two non-blocking follow-ups: the delete-path duration-rollback test (asserts the toast + store and live-root revert — that path swallows the error by design) and the on-open dismiss now reuses shouldDismissAssetPreview (closes the opened-during-pending-seek gap; test added). Suites: studio 2111 / core 1224 passing (same 18 pre-existing environmental failures). The earlier Producer: integration tests failure was the byte-identical-retry flake — passed on rerun with zero changes.

[miguel-heygen]

Rebased clean and linear on main (no merge commit), and commit 6 also closed both non-blocking follow-ups (delete-path rollback test + shared shouldDismissAssetPreview on open). Re-verified: commit 1's rebase conflict resolution correctly adapts to main's postRuntimeControlMessage refactor, commits 2-5 identical to the previously-approved head, commit 6 correct. CI green except the regression-shards still finishing (no failures). Re-approving.