feat(studio): track timeline performance

[miguel-heygen]

Summary

• sample one aggregate studio_timeline_performance event after a timeline scroll burst, at most once per minute per mounted timeline
• record raw total/mounted clip counts, row count, timeline DOM size, viewport size, zoom mode, scroll-to-frame latency, and frame interval
• use the existing privacy-aware Studio telemetry client, so opt-out, Do Not Track, development suppression, anonymous identity, and IP suppression stay centralized
• never send project IDs, clip IDs, labels, paths, or other project content

Rollout note

Land this telemetry before enabling timeline virtualization in production to establish a baseline. This PR measures the change but does not enable virtualization.

Validation

• bun run --cwd packages/studio typecheck
• bun run --cwd packages/studio test — 3,123 passed
• changed-file oxlint, oxfmt, fallow, file-size, tracked-artifact, and commit hooks passed

[vanceingalls]

R1 review @ eed3552

Verdict: APPROVE — pure additive telemetry (232/-0), privacy-clean (aggregate counts + timings only, no project content), correctly throttled (rAF-scoped latency sample + 400ms idle debounce + 60s inter-event floor per mounted timeline), lifecycle-safe (rAF and timer both torn down on unmount). Rollout note is correct: land the measurement before flipping virtualization so the "before" baseline is captured.
Grade: A Rubric: CORRECT

Approach. New hook useTimelinePerformanceTelemetry at packages/studio/src/player/components/useTimelinePerformanceTelemetry.ts. On every onScroll, if no rAF is in flight, queues one via requestAnimationFrame and captures a per-frame scroll-to-paint latency into frameLatencies plus a frame-to-frame interval into frameIntervals. Every scroll (also) resets a 400ms idle debounce; when the debounce fires with no rAF in flight, the accumulator is summarised (p95 + max latency, p95 frame interval) and, if ≥60s has passed since the last emit, sent through trackStudioTimelinePerformance → trackEvent("studio_timeline_performance", …). Timeline.tsx wires the hook and passes expandedElements.length, displayLayout.displayTrackOrder.length, and zoomMode as the aggregate context.

Findings.

• P1: none.
• P2: none.
• Non-blocker (semantic — onScroll is not userscroll): onScroll fires for both user-initiated and programmatic scroll changes. Timeline.tsx already restores scrollLeft after post-edit reload (comment: // restored across post-edit reload at :478) — that programmatic write produces an onScroll event, which the hook counts as a scroll burst. Single-event bursts also produce a valid emit (a 1-sample p95 is that single sample), so restoration scrolls can appear as very short bursts with pseudo-latency reflecting the immediate paint. If Studio has (or later gains) an autoscroll-follow-playhead feature, every playback second becomes a synthetic "scroll burst." Two mitigations, either is fine — filter downstream on scroll_sample_count >= N, or add a flag on the hook to only sample bursts where the scroll delta accumulates past a threshold. Not blocking the merge; worth noting on the dashboard so we don't misread restoration/autofollow as user-scroll pain.
• Non-blocker (detached-DOM edge): scroll element captured in the 400ms setTimeout closure may be detached from the tree if a conditional-parent unmount fires without unmounting Timeline itself between the last scroll and the debounce firing. clientWidth/clientHeight on a detached element return 0, so the emit would carry viewport_width: 0, viewport_height: 0. Rare, self-signalling (0 is easy to filter), and the useMountEffect cleanup already covers the Timeline-unmount case. Fine to leave.
• Non-blocker (hook-layer test gap): the new spec only exercises summarizeTimelinePerformance (the pure summarizer). The rAF/idle/60s-floor state machine has no direct test — sensible, since jsdom's rAF is faked and rAF timing tests are fragile, but a future refactor that inverts the 60s floor or drops the frameRequest === 0 guard would ship untriggered. A shallow-render fake-timers test around useTimelinePerformanceTelemetry (mocking requestAnimationFrame with queueMicrotask) would close this cheaply if you want a red-if-broken safety net.
• Non-blocker (rate-limit is per-mount, not per-user-per-timeline): unmount/remount resets lastEmittedAt to -Infinity, so a route change that remounts Timeline within 60s of the last emit will emit again on the very next burst. Not "wrong" — the observability event volume is bounded — but downstream aggregation should not treat one-emit-per-minute as a hard invariant.

Correctness — state machine.

• rAF de-duplication: state.frameRequest === 0 gate ensures at most one rAF outstanding at a time. Additional scroll events within the same frame collapse into a single latency sample, keyed off the first scroll's now. Correct — this measures "first-scroll → next-paint" not "each-scroll → next-paint," which is what you want for a jank signal.
• Idle debounce: every scroll clears the prior timeout and sets a new 400ms one. When it fires and rAF is still pending, we cancelAnimationFrame + resetMeasurements — no emit for a burst whose paint didn't land. Correct.
• 60s floor: emittedAt - state.lastEmittedAt >= MIN_EVENT_INTERVAL_MS. Initial lastEmittedAt = Number.NEGATIVE_INFINITY so the first burst always emits. Correct.
• Cleanup: useMountEffect return-cleanup cancels both rAF and idle timeout on unmount. useMountEffect at packages/studio/src/hooks/useMountEffect.ts is useEffect(effect, []) — cleanup runs exactly once, on unmount. Correct.

Correctness — summarizeTimelinePerformance.

• percentile uses nearest-rank: ceil(N * p) - 1 clamped to [0, N-1]. For N=4 p=0.95 → index 3 (the max); for N=3 p=0.95 → index 2 (the max). Both match test expectations 80 and 45. For N=1 → index 0. Small-N p95 is just the max — that's the "nearest-rank" definition and is fine here.
• Returns null when frameLatencies is empty, so !sample check in the emit path is redundant-safe. frameIntervalP95 === undefined when frameIntervals is empty (first burst has one frame → no interval yet); the spread ...(frameIntervalP95 === undefined ? {} : { frame_interval_p95_ms: frameIntervalP95 }) correctly makes the field optional. Test covers the "no frame interval" case implicitly by NOT setting up multi-frame samples in the second test (which returns null because latencies are also empty).
• Math.max(...frameLatencies) — bounded to ~1 per rAF per 400ms window ≈ 24 samples/second bursts. No stack-size concern.
• scroll.querySelectorAll("*").length and scroll.querySelectorAll('[data-clip="true"]').length walk the subtree once per emit (≤ once per 60s per mounted timeline). Cost fine even on a 10k-element pre-virtualization timeline.

Privacy check. The emitted event ships:

• Aggregate counts: total_clip_count, mounted_clip_count, total_row_count, timeline_dom_node_count.
• Layout: viewport_width, viewport_height, zoom_mode.
• Timings: scroll_sample_count, scroll_frame_latency_p95_ms, scroll_frame_latency_max_ms, frame_interval_p95_ms?.

No project IDs, clip IDs, labels, paths, or asset URLs. zoom_mode is a low-cardinality enum ("fit" / "manual" from what the test exercises). Passes the PR body's privacy claim.

The privacy layer (opt-out, DNT, dev-suppression, anon identity, IP-suppression) lives in trackEvent, which this PR does not touch — so the claim "stay centralized" is contingent on that upstream layer, not something this PR is asserting new about.

PostHog event contract. studio_timeline_performance is a new event name. Property shape is fully-typed via StudioTimelinePerformanceSample in events.ts. frame_interval_p95_ms is the sole optional field (correctly modelled as ?:), so downstream queries need properties.frame_interval_p95_ms is not null when filtering on it. Everything else is number or string — nothing PII-shaped.

Diff-size / churn. +232 / -0 across 5 files. New helper module (132 lines) + new spec (54 lines) + typed event (18 lines in events.ts) + one test case in events.test.ts + 7-line wiring in Timeline.tsx. Additive, no deletions, minimal wiring surface.

CI. mergeStateStatus: BLOCKED at time of review with REVIEW_REQUIRED blocking; the rollup shows Detect changes ✓, changed-file oxlint/oxfmt/fallow/file-size/tracked-artifact/commit hooks passing per PR body. Nothing failing that would gate my approval.

Verdict rationale. Clean, scoped observability addition ahead of a behavior change. The three non-blockers are dashboarding concerns (filter on scroll_sample_count >= N, be aware programmatic scrolls surface here) rather than code defects. State machine is correct, cleanup path is correct, privacy contract is honoured. Ship.

— Via

[james-russo-rames-d-jusso]

Reviewed at eed35524.

Well-scoped baseline-before-flip telemetry PR — pure summarize function is separated from the timing hook, the sample surface is aggregate-only (no PII), 60s throttle + scroll-idle-burst detection are reasonable, and data-clip="true" matches the existing selector at packages/studio/src/player/components/useTimelineActiveClips.ts:37 so mounted_clip_count is consistent with the runtime's own notion of "mounted clip". Rollout note lines up with the baseline-first sequencing pattern.

Three items inline (test-coverage gap, stale-context risk, and frame_interval_p95_ms semantics/naming). Plus two body-level items below.

Nits

• zoom_mode: string in packages/studio/src/telemetry/events.ts:36 is looser than needed — the actual zoom mode is a small union ("fit" | "manual" | ...). Narrowing the field type keeps PostHog schema drift on this event honest.
• The throttle silently drops the sample when it fires within 60s of the last emit (resetMeasurements runs unconditionally at the end of the idle callback). Alternative: buffer the latest sample and emit at the next 60s tick, so a spike that lands in the throttled window still gets recorded. Trade-off; either is defensible for a baseline metric, but worth naming which behavior is intended.

Questions

• Rollout note says "land this before enabling timeline virtualization to establish a baseline" — is there a linked Linear ticket / feature flag for the virtualization flip that this metric will feed into? Would help the reader understand the intended baseline duration + decision criteria.
• Is there already a PostHog dashboard queued up to consume studio_timeline_performance, or is that follow-up?

What I didn't verify

• The unrelated-looking Smoke: global install red on CI — assumed pre-existing / unrelated to this PR's scope; call out if that's wrong.
• Whether the privacy-aware Studio telemetry client's opt-out / DNT / anonymous-identity plumbing actually short-circuits trackEvent("studio_timeline_performance", ...) end-to-end. Trusted the PR body claim.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Stale context at emit time. recordTimelineScroll is re-created each render and captures context in its closure; the setTimeout callback (which fires 400ms after the last scroll) uses whichever closure was current when that final scroll was recorded. If a re-render happens between the last scroll and the flush — expandedElements.length changed, zoomMode flipped, tracks were reordered — the emitted event carries pre-flush values. mounted_clip_count is read fresh from the DOM at emit time, so it stays current; total_clip_count / total_row_count / zoom_mode can lag. Trivial fix: const contextRef = useRef(context); contextRef.current = context; at the top of the hook, then read contextRef.current inside the setTimeout callback. Non-blocking, but worth doing before the baseline rows land in the dashboard.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 frame_interval_p95_ms semantics: this measures the gap between two sampled rAF callbacks — but a rAF is only scheduled when frameRequest === 0 AND a new scroll event fires. So the interval reflects time-between-scrolls + rAF-delay, dominated by the user's scroll cadence during pauses, not the browser's actual paint cadence. In a smooth continuous scroll at 60 events/sec it should approximate 16ms; with brief user pauses between scrolls it spikes for reasons unrelated to timeline render cost. Two options: (a) rename to something like scroll_sample_interval_p95_ms to make the meaning explicit, or (b) if the intent is to detect frame drops during a burst, schedule rAFs continuously during the burst rather than one-per-scroll — that gives a true inter-frame interval. Flagging so you can decide before the dashboard interprets this as "frame cadence".

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Test coverage is summarizeTimelinePerformance-only — the hook's timing/lifecycle machinery (60s throttle, idle-timer cancellation, pending-rAF abort when the tab was backgrounded, useMountEffect cleanup canceling in-flight rAF + timer on unmount) is entirely untested. Given the whole point of this telemetry is to fire responsibly — at most once per minute, dropped when the browser pauses, cleaned up on unmount — the throttle + cleanup paths are the ones most likely to regress silently. A vi.useFakeTimers() + rAF stub set of cases exercising: (a) two bursts <60s apart → only first emits, (b) unmount mid-burst → no lingering timer/rAF, (c) burst with frameRequest still pending when idle fires → sample discarded — would nail the hook contract without a real DOM.

— Review by Rames D Jusso