fix(studio): fix seek after code edit, improve scrub perf, add click-to-source

[miguel-heygen]

Summary

• Fix seek-after-edit: The retiring Player's cleanup clobbered iframeRef.current during crossfade refreshes, breaking all subsequent seeking. Guard the ref cleanup to only null when it still points to the retiring Player's iframe.
• Improve scrub performance: RAF-throttle the expensive onSeek path (adapter.seek + Zustand + React re-render) during drag, while keeping slider/playhead visuals on every pointer event for instant 60fps feedback.
• Click-to-source: Clicking an element in the preview switches to the Code tab, opens the source file, and scrolls the editor to the element's opening tag.

Test plan

• Open a project in the studio, seek to a position, edit code in the Code tab, wait for preview refresh, verify seeking still works (slider + timeline playhead + preview all update)
• Drag the scrubber and timeline playhead — verify smooth 60fps visual tracking during drag
• Click an element in the preview — verify it switches to Code tab with the source file open and scrolled to the clicked element

[vanceingalls]

Three fixes in one PR — two correctness wins (seek-after-edit ref guard, scrub-perf RAF throttle) and one new UX feature (click-to-source) that needs a workflow discussion before it ships.

Calibrated strengths

• Player.tsx:271-282 — the ref-identity guard (mutableRef.current === iframe) is exactly the right shape for crossfade unmount-after-remount ordering. Comment explains the failure mode well; future readers won't undo it.
• PlayerControls.tsx:227-236 — separating visual feedback (raw event → DOM style writes) from logical seek (RAF-throttled) is the right perf seam. The catch-up seekFromClientX(pendingClientX) on cleanup (line 240) ensures the final position lands.
• useTimelineRangeSelection.ts:99-108 — same RAF pattern, with the visual fed via liveTime.notify instead of direct DOM writes. Consistent with the existing dragScrollRaf pattern.

important — Click-to-source steals the Code tab on every click, including non-intentional selections (usePreviewInteraction.ts:76-78). Currently any non-shift preview click both (a) selects the element AND (b) jumps the left sidebar to "code", opens the source file, scrolls to the tag. Two failure modes:

• User is editing foo.html in the Code tab, clicks an element from bar.html in the preview to inspect its styles via the right panel → editor switches to bar.html, cursor/scroll position in foo.html is lost.
• User is in the "compositions" or "assets" tab doing unrelated work → click in preview to select → tab swaps to "code" unexpectedly.

The PR body frames this as the desired behavior, but it conflicts with the existing select-to-inspect workflow that lands on the right panel. Recommend gating on a modifier (alt-click / cmd-click) or a double-click, or making it opt-in via a setting. Worth a workflow discussion with whoever owns Studio UX before this lands as the default.

important — openSourceForSelection has a fetch race (useFileManager.ts:185-200). pendingRevealRef is a single slot, and the fetch promise has no cancellation/generation token. If the user clicks element A, then quickly clicks element B in a different file before A's fetch resolves:

• B overwrites pendingRevealRef. When A's fetch resolves first, it applies B's target offset to A's content → either wrong scroll or null offset.
• If A's fetch resolves after B's (e.g., A is a larger file), setEditingFile({ path: aFile, content: aData }) clobbers B's already-loaded content — user thinks they're looking at B's source but they're in A.

Use an AbortController per fetch, or a monotonic request id (const reqId = ++reqIdRef.current; ... if (reqId !== reqIdRef.current) return;) before the setEditingFile write.

nit — Ref-guard asymmetry on the function-form ref branch (Player.tsx:274-276). The mutable-ref branch correctly checks mutableRef.current === iframe, but the function-ref branch still calls ref(null) unconditionally. The current sole caller (NLEPreview.tsx:91) passes a mutable ref, so this is unreachable in practice — but if anyone wires a callback ref later (or React's behavior changes), the same crossfade clobber returns. Pull iframe identity into the function-ref branch too — track the last-assigned iframe in a ref scoped to the Player and only ref(null) if it matches.

nit — SidebarTab literal duplicated in App.tsx:218. The inline "code" | "compositions" | "assets" will drift if a new tab is added. Import SidebarTab from LeftSidebar.tsx and use it. (Same shape lives in LeftSidebar.tsx:13.)

nit — CodeQL CSRF finding on useFileManager.ts:188 is a known false positive class for the studio dev-server pattern — same shape as handleFileSelect, readProjectFile, etc. The server (packages/core/src/studio-api/routes/files.ts:50) validates via isSafePath, so path traversal is bounded server-side. No action needed beyond the existing pattern, but worth a one-line // codeql-ignore or similar to silence noise on future PRs if the team cares.

Verdict: APPROVE
Reasoning: Two correctness/perf fixes are solid and well-explained. The click-to-source UX choice is the only thing that gives me pause, but it's a design call, not a correctness bug — flagging for discussion, not blocking. The fetch race is real but cosmetic (no data loss, just wrong file briefly) and easy to fix in a follow-up.

— Vai

[jrusso1020]

LGTM — three discrete fixes, all clean. Vai's two important findings are the load-bearing ones; flagging in agreement.

Audited

Fix 1 — Seek-after-edit (Player.tsx:274-281): classic crossfade-race. The new Player assigns the iframe ref BEFORE the retiring Player's cleanup runs; without the guard, the cleanup nulls the new Player's ref → all subsequent seeks fail. The identity check (mutableRef.current === iframe) is exactly the right shape — "only null if I still own this ref." ✓

Fix 2 — Scrub perf (PlayerControls.tsx + useTimelineRangeSelection.ts): same RAF-throttle pattern applied to both scrub surfaces:

• Visual update on every pointer event (progressFillRef.style.width / liveTime.notify) — instant 60fps feedback
• Expensive onSeek path (adapter + Zustand + React re-render) RAF-throttled — one call per frame max
• Cleanup cancels the RAF AND seeks the final pendingClientX so the drop position is captured

Decouples visual feedback from logical seek correctly. The final-position-capture on pointerUp prevents the "off by one event" bug where a fast drag-end could leave the playhead 1 frame behind. ✓

Fix 3 — Click-to-source: pure-additive plumbing through App.tsx → useDomEditSession → usePreviewInteraction → useFileManager.openSourceForSelection → SourceEditor reveal. findTagByTarget exported from sourcePatcher.ts to support the offset lookup. EditorView.scrollIntoView({y: "center"}) + view.focus() is the right CodeMirror call. Math.min(revealOffset, docLen) defensive clamp on the offset. ✓

Important non-blocking — both flagged by Vai, agreeing on severity

1. Click-to-source steals the Code tab on every non-shift preview click (usePreviewInteraction.ts:76-78) — I traced the new logic but missed the workflow conflict with the existing select-to-inspect pattern. User has foo.ts open in Code tab while editing → clicks preview to inspect an element → the Code tab swaps to that element's source file, losing cursor/scroll position in foo.ts. Vai's recommendation (gate on a modifier like alt/cmd, or make it opt-in via toggle) is the right escape hatch. This is the UX miss I should have caught — the lesson is to ask "does this new entry path conflict with an existing one?" not just "does the new path work in isolation."

2. Fetch race in openSourceForSelection (useFileManager.ts:185-200) — I noted pendingRevealRef as a "minor race" but Vai's framing is right: single-slot ref + no fetch cancellation = silent wrong-file-or-wrong-offset on rapid clicks across different source files. AbortController + monotonic request id is the standard fix.

Vai's nits (also agreeing)

• Player.tsx:274-276 function-ref branch still calls ref(null) unconditionally — the mutable-ref branch got the identity guard; the function-ref branch didn't (and can't easily — function refs don't expose .current). Unreachable today because all callers use mutable refs, but it's a latent re-regression vector if a function ref ever gets wired in. Could add a comment noting the asymmetry, or have callers always pass a mutable ref.
• "code" | "compositions" | "assets" literal duplicated inline in App.tsx:218 — should import the canonical SidebarTab type from LeftSidebar. If a fourth tab is added, the inline literal in App.tsx becomes a TS error far from the source of truth.

One observation from my own scan (smaller than Vai's)

• Click-to-source effect doesn't re-fire on same-offset clicks (SourceEditor.tsx:138-148): useEffect deps [revealOffset]. If the user clicks the same element twice in succession AND the offset is identical, the effect doesn't re-fire (state didn't change), so no re-scroll/re-focus. Probably benign but a version counter (revealOffset + revealNonce) or always-resetting-to-null-then-setting would force re-fire. Cosmetic.

Scope

Three independent fixes bundled. Reviewable as-is — they don't conflict — but for revert-surface cleanliness three separate PRs would let each one stand on its own merits.

CI

All required green: Test, Typecheck, Lint, Build, Format, CodeQL, Test: runtime contract, CLI smoke (required), Smoke: global install, Preview parity, preview-regression, Tests on windows-latest, Render on windows-latest, player-perf, regression, Preflight (lint + format), Analyze (actions/javascript/python). github-advanced-security comment looks like the CSRF false-positive Vai called out — server-side isSafePath bounds it.

— Review by Rames Jusso (pr-review)