fix(studio): restore overscan and publish the zoom viewport

[miguel-heygen]

Restore the timeline's time overscan from half a viewport to a quarter viewport, the value used before the recent overscan increase. This reduces offscreen clip mounts and restores the viewport performance gate. Existing geometry and zoom-preview tests now assert the restored budget.

Every zoom scroll write now publishes the actual scroll through the existing viewport snapshot publisher before paint. Previously, the render window combined the newly committed scale with the old scroll snapshot until a later scroll event, which could unmount a visible clip for a frame. That covers the explicit and fallback anchor paths, and the zoom ending that keeps the laid-out scale and only moves the scroll (an eased zoom-out, or a pan to a range at the current scale), which now flushes its publish like the scale change beside it.

The CI path filters compare a pull request's merge commit against its current first parent, so the checks reflect changes relative to the current main branch rather than an outdated base SHA. Push and scheduled runs keep their existing behavior.

Validation of the overscan restore

• Full Studio suite: 7,726 tests passed across 691 files. Existing skipped and todo cases are unchanged.
• 93 focused geometry, zoom-input and viewport tests passed three consecutive runs. Restoring the half-viewport constant makes the geometry regression witness fail.
• Studio typecheck, player lint, formatting and diff audit passed.
• Production-browser viewport gate passed three independent runs in each arm. Enabled-arm interaction p95: 33.1, 33.1, 33.2 ms against 58.3 ms. Disabled-arm p95: 48.9, 43.7, 46.0 ms against 75 ms.

Six quiet paired runs used the same composition, browser, viewport and gesture driver against main. Values below are medians across the six runs.

Gesture	Main FPS	Restored budget FPS	FPS change	Main p95	Restored budget p95
Buttons	19.995	20.811	+4.08%	133.35 ms	141.65 ms
Slider	15.675	15.793	+0.75%	341.65 ms	308.30 ms
Pinch	32.697	36.272	+10.93%	100.00 ms	75.00 ms
Ctrl-wheel	23.437	22.988	-1.92%	199.90 ms	200.00 ms
Zoomed scrolling	23.779	23.770	-0.04%	133.30 ms	133.30 ms

Before the zoom-anchor correction, the maintainer explicitly accepted the ctrl-wheel difference as measurement noise: p95 is flat, buttons and pinch improve, and this restores the previous overscan value. This is a maintainer judgment rather than a newly introduced acceptance tolerance.

Zoom-anchor correction validation

The exact regression case uses a 1080 px viewport, 5000 px initial scroll, 100 to 109 px/s zoom, and an anchor at 55.24 s / 556 px. The previous head scrolls the DOM to 5497.16 px while retaining the old snapshot, dropping the mounted-range marker at 59 s. The new test fails on that head and passes with synchronous publication. It uses the real playhead, viewport and render-window hooks; its marker witnesses clip eligibility rather than production clip pixels.

• 116 focused hook, zoom, layout and budget tests passed three consecutive runs.
• Studio typecheck, player lint and formatting passed on the corrective head.
• Enabled and disabled viewport gates each passed three independent production-browser runs, first attempt.

Paired zoom bench at the final head: the PR's base against this head, same walk fixture, same gesture driver, headless Chrome 152 at 1440x900 on an idle Apple Silicon machine, 6 alternating rounds. Median FPS per gesture (the spread across rounds on each side is wider than every difference):

Gesture	Base FPS	Head FPS	FPS change	Base p95	Head p95
Buttons	43.1	42.4	-1.6%	67 ms	75 ms
Slider	40.7	39.8	-2.2%	67 ms	67 ms
Pinch	58.5	58.5	0%	17 ms	17 ms
Ctrl-wheel	47.0	47.7	+1.5%	33 ms	50 ms
Zoomed scrolling	56.6	56.2	-0.7%	33 ms	33 ms

The earlier single pair (buttons -28.8%, pinch p95 doubled) does not reproduce: pinch stays on the 16.7 ms frame time in every round on both sides.

• A same-scale pan is read outside React's test batching, as a browser runs it: with the publish unwrapped from flushSync the render window still shows the old scroll (expected '0' to be '6000'). A second test records each published scroll as it is sent, so publishing only on the first eased frame also fails. timelineZoomInput, useTimelinePlayhead and TimelineToolbar tests pass 80/80 three runs in a row; studio typecheck, oxlint and oxfmt pass.

Why main's viewport gate is red

Main's gate fails on the default arm at about 61 ms against 58.3 ms. The cause is the half-viewport overscan from the recent overscan increase: every scroll step mounts and unmounts more clips. Same machine, current main, only the overscan constant differs, the gate's default arm at 50,000 clips with its 4x CPU throttle, 5 alternating rounds:

	Mounted timeline nodes	Gate attempts passed	Interaction p95 (median of attempts)	Frame p95 (median)
Main (half viewport)	1474	2 of 9	293 ms	283 ms
Quarter viewport	1246	5 of 6	32 ms	16.8 ms

CI's red runs on main report the same 1474 mounted nodes. The budget is unchanged.

Before

Current main, using the performance fixture: zoom in, scroll sideways, then zoom out. This recording provides a visual control, rather than proving the isolated 59-second regression.

before.webm

After

The corrective head runs the same gestures with quarter-viewport overscan and synchronous zoom-anchor publication. Both recordings retained visible clips during all 90 sampled sideways-scroll steps. They do not replace the exact-case regression test.

after.webm

[github-actions]

Edit accuracy: accurate 2059 (base branch 2059), smooth 1485 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

[terencecho]

Review at e80df1f33a9a486e311da0f04b0f88c7da9d78c6 — approved.

The resting versus zoom-preview overscan split and zoom landing path have no verified introduced correctness issue. The scroll snapshot is published by the layout path before paint; the first intermediate render is not an observed blank frame. All five changed CI filters compare the PR merge commit with its current first parent, avoiding unrelated files from a stale event base while preserving the other event fallbacks. Focused timeline tests passed 124/124; viewport-verdict tests passed 13/13, and the current-head Studio viewport gate and all required checks passed, including Windows render. I did not run full browser E2E locally.

The optional Fallow audit reports six minor duplication/complexity findings in tests and gate code; those are not a correctness block for this review.

— tai

[terencecho]

Review at c33e89ede49a7ab295ff5d0595f31167e0606e2f — code path looks sound; recording a COMMENT rather than an approval while the author’s quiet same-bench comparison is pending. One approval would release the merge gate before the comparison they explicitly require.

The new zoom-owner motion lifetime retains the wider render window across a button or slider landing rather than narrowing and widening between steps. In a real-hook trace, the first button and slider landings remain mounted where the preceding head pruned/remounted. A second button step still has a transient unmount/remount in synchronous layout passes on both heads; no painted blank was demonstrated, so I do not attribute that to this delta or claim all transient remounts are eliminated. Focused timeline/zoom/playhead/scroll/toolbar/layout tests pass 139/139. The current viewport gate is green, but it measures Ctrl+wheel/pinch and scrolling rather than button/slider remount cost; the quiet same-bench result remains outstanding.

The current Studio test failure (VideoThumbnail.test.tsx:283) is a fixture mismatch, not a reproduced product decode regression. Its ResizeObserver double reports width 880 while the mock host still has clientWidth=440; the newly joined rest notification correctly remeasures the element’s actual width and does not request a 16-frame filmstrip. In a disposable test with clientWidth=880 matching the reported resize, 12/12 thumbnail tests pass and the 16-frame decode occurs. A separate case where the actual width changes after rest also decodes 16 frames (13/13). Please synchronize the test host width with its synthetic resize. I found no production correctness blocker in this failure, but the red test should be addressed for CI.

I will recheck the live head and complete the approval decision when the author’s quiet same-bench comparison is available. This comment does not release the merge gate.

— tai

[terencecho]

Review at 2343ff5934f1cb46a11cfe29df8c8b03b71fd1f7 — no introduced code blocker in the follow-up to my previous review.

packages/studio/src/player/components/timelineZoomInput.ts:100-113 restores preview-only notifications for thumbnail/playhead consumers and exposes a separate preview-plus-motion subscription for the render window. useTimelineClipRenderWindow.ts:45 uses that combined stream, preserving the wider window through a zoom burst; the thumbnail strip no longer receives motion-only rest remeasure events. Focused thumbnail, zoom, render-window, playhead, scroll, layout and viewport-verdict tests pass 165/165 at this head. The gatePassed boolean rewrite and zoom-rest wait retain their prior decision/timeout semantics; I syntax-checked the changed E2E scripts but did not rerun the full browser gate locally.

The quiet six-pair button/slider/gesture comparison on the final build remains marked pending in the PR and explicitly gates the author's merge plan. The green prior viewport gate measures scrolling and Ctrl+wheel/pinch blank-ruler frames, not this comparison. Since an approval would release the merge loop before the author’s stated condition is met, this is a non-gating comment rather than an approval; I will reassess when the quiet result is posted. I do not infer a regression from the noisy earlier four-pair data.

— tai

Verdict: COMMENT
Reasoning: The incremental code and focused tests resolve the motion-subscription test failure, but the author’s explicitly required final-build performance comparison has not been supplied; approval would trigger merge before that check.

[jrusso1020]

Approving at this head.

Restore. timeOverscanViewportRatio goes from 0.5 back to 0.25. 0.25 is the value from before #5109 raised it (git log -S on timelineViewportBudgets.ts). Nothing in the zoom path hardcodes the old half-viewport margin. previewNeedsLayout (timelineZoomInput.ts:147) compares the preview against drawnRange, which comes from the same getTimelineRenderTimeRange budget. With the smaller margin, a zoom-out preview lays out sooner, at about 0.67x instead of 0.5x, but it still never shows time with nothing mounted. That trade matches the bench table.

Tests. I checked the updated numbers by hand. With a 1080 px viewport the margin is 270 px, so the window is mounted to 131.8 s. In the middle-zoom case the window is mounted over 46.98 to 63.18 s, and the view at 70 px/s covers 47.75 to 62.73 s, inside it. Moving that case from 600 to 700 keeps its meaning, a zoom-out that stays inside what is mounted. The three changed Studio test files pass locally (87/87).

CI filters. I checked dorny/paths-filter at the pinned ceb8a2b8 against token: "". On a pull_request event it calls git.getChanges(base || baseSha || defaultBranch, ...), so the new base input is used, and the base is HEAD^1 of the merge ref. All five jobs check out with fetch-depth: 0, so HEAD^1 and HEAD^2 resolve. On push, schedule and merge_group runs the step is skipped, base is empty, and the filter falls back to what it did before. This is the same HEAD^1 approach the reachability job in ci.yml:51 already uses.

Nit (optional): the same 8-line step is now pasted into five workflows. If a sixth filter appears, a small composite action would keep them in step.

Verdict: APPROVE
Reasoning: This is a one-constant restore to the pre-#5109 value, and the zoom preview's layout check adapts to it. The CI base fix uses an input the pinned action actually reads in git-diff mode.

— Rames

[terencecho]

Review at f0ba5aa8f6bb8d38294a8ba5ac0e4211a6629a4c — requesting changes for a painted zoom-commit gap.

packages/studio/src/player/lib/timelineViewportBudgets.ts:64 restores the quarter-viewport margin; that is the effective production change relative to main. The existing zoom path does not publish its programmatic scroll synchronously. An earlier unmerged iteration of this PR added zoom-preview widening and immediate scroll publication, but the final head does not carry those protections. useTimelineClipRenderWindow.ts:39-45 calculates the clip range from the newly committed pixels-per-second and the old scroll snapshot. The zoom-anchor layout effect then writes the new scrollLeft in useTimelinePlayhead.ts:99-103 without calling syncScrollViewport; the virtualized scroll path in useTimelineScrollViewport.ts:72-97 publishes on a later rAF. A centered zoom can therefore paint a visible region for which the clip index has already unmounted clips.

Concrete case: 1080px viewport, 32px content origin, scrollLeft=5000, 100→109 pixels/second, anchor 55.24s at x=556. The new scroll target is 5497.16px. While the snapshot still says 5000, the quarter-window render range ends at 57.963s, but the new visible right edge is 60.047s; a clip at 59s disappears from the right side until the viewport snapshot catches up. The half-window on main reaches 60.440s and retains it. In Chromium, an isolated React harness executing the head-identical zoom, viewport, playhead, render-window, and clip-index source captured actual painted frames at that clip as red → blank → red with 0.25, versus red → red with 0.5. This is not a full Studio-app run; the harness shell and budget substitution are synthetic. Separately, the PR's earlier browser probe reported blank zoom-out frames for quarter overscan without immediate scroll publication. Its frame count and duration are not asserted for this head.

Please publish the zoom-anchor scroll snapshot synchronously before paint (as the earlier unmerged iteration did), or preserve sufficient render overscan through the landing, and add a real zoom-in/zoom-out painted-frame regression check. The current viewport gate has no blank zoom-frame assertion, so its green scroll result does not test this case. The failing captures-format check is a separate merge issue, not the reason for this code verdict.

— tai

[miguel-heygen]

Fixed in 2d9b72b. The provider passes the existing syncScrollViewport callback to the playhead hook, and both explicit and fallback zoom-anchor writes call its immediate path inside the layout effect. The 0.25 overscan margin remains unchanged.

The exact 1080 px / 5000 px / 100 to 109 px/s / 55.24 s at 556 px regression is now covered with the real playhead, viewport and render-window hooks. It fails on the previous head: the DOM scroll reaches 5497.16 px but the old snapshot removes the 59-second eligibility marker. It passes with the fix and verifies the snapshot reaches 5497.16 px without advancing the deferred scroll publication. This is a clip-eligibility witness, not an isolated painted-pixel assertion.

116 focused tests passed three times, with typecheck/lint/format passing. Both production viewport arms passed three independent runs on this head. Fresh main/fix gesture recordings are attached in the body. The requested single buttons/pinch pair showed lower FPS on the corrective head; those figures are fully reported in the body and performance acceptance remains pending.

[jrusso1020]

Re-review at 2d9b72b9 (my approval was at f0ba5aa8). Commenting rather than approving, because @terencecho's changes request is still live and the performance acceptance in the body is still open. Those are his calls, and I'm not approving over them.

On the code delta, which looks right

• useTimelinePlayhead.ts now calls the provider's existing syncScrollViewport after both scroll writes in the zoom layout effect: the explicit anchor branch and the fallback branch. That's the same callback useTimelineFocusCoordinator already uses, so this reuses the publish path instead of adding one. The dependency list includes it.
• useTimelinePlayhead.test.tsx passes locally, 23/23.

Additive, on the open same-scale case
The remaining write without a publish is commitPreview in timelineZoomInput.ts:212. When the preview ends at the scale already laid out (done.pps === done.basePps, an eased zoom-out), it skips writeZoom and sets view.scroll.scrollLeft = left directly. So the zoom layout effect never runs, and the viewport only catches up on the next scroll event. That's the path @terencecho flagged. Publishing it immediately would use the same callback, but commitPreview lives outside React and would need the callback passed in, for example on the TimelineZoomViewport it already holds. I haven't seen a painted gap there either.

Verdict: COMMENT
Reasoning: The delta correctly closes the changed-scale gap. Approval waits on the live changes request and the performance decision.

— Rames

[jrusso1020]

Re-review at 247f121, covering the delta since 2d9b72b: commitPreview's same-scale scroll publish and the two new tests.

Code: looks good to me, and I'd approve on the merits.

• timelineZoomInput.ts:213-216: when a commit lands at the scale that's already laid out, it now writes the scroll and publishes it inside flushSync. That was the gap I flagged last round. When the scale does change, the layout effect has already scrolled and published, so the >= 0.5 check skips the second publish. No double publish.
• publishScroll: syncScrollViewport (useTimelinePlayhead.ts:289) goes through the immediate path, because isScrolling defaults to false. That path cancels any queued rAF publish. The callback is a useCallback(..., []), so adding it to the effect deps doesn't re-register the listeners.
• None of the commitPreview callers run inside React: a rAF step, the rest timer, pointerdown, and the comment-guarded rAF in zoomTimelineToRange. So the new flushSync can't nest inside a render or effect.
• Same-scale publishes only fire when the pan leaves the drawn range (previewNeedsLayout), not on every eased frame. That matches the flat paired bench.

Verified locally:

• The 3 touched test files pass, 80/80.
• I swapped the new flushSync(() => view.publishScroll(...)) back to a plain call, and "renders a pan at the laid-out scale before the browser paints" fails with expected '0' to be '6000', as the body says. Then I restored the file.

Why this is COMMENTED and not APPROVED: @terencecho's CHANGES_REQUESTED at f0ba5aa is still live, and the performance call is that reviewer's. The new paired bench (6 alternating rounds, every difference inside the round-to-round spread) looks like it answers the earlier single-pair -28.8% reading. I'll approve at this head once that review is cleared.

Verdict: COMMENT
Reasoning: The delta closes the same-scale publish gap, and the new test proves it. I'm holding the stamp only because a peer's changes request is still live.

— Rames

Requested on an older commit; later commits answer it (see the thread). Current head is 247f121.

[jrusso1020]

Approving at 247f121, the head I reviewed earlier today (review 5453233959). There's no new code since then. That earlier review covers the delta and the local verification:

• the 3 touched test files pass, 80/80;
• the new same-scale test fails without the flushSync publish.

The changes request from f0ba5aa was dismissed by the author with a stated reason. Its two concerns are both addressed:

• The blank strip: a zoom commit used to paint before the viewport was published. 2d9b72b fixed that for scale changes, and this head covers commits that end at the same scale.
• Performance: the 6-round alternating bench in the body puts every gesture within the round-to-round spread.

Verdict: APPROVE
Reasoning: The zoom and pan commits now publish their scroll before paint, the new test proves it, and the restored quarter-viewport overscan matches the value from before the overscan increase.

— Rames