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.

Zoom-anchor layout writes now immediately publish their actual scroll through the existing viewport snapshot publisher. 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. Both explicit and fallback anchor paths publish before paint.

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.

The requested single quiet buttons/pinch pair on the corrective head produced these figures. One pair does not establish causality, and performance acceptance remains pending.

Gesture	Main FPS	Corrective head FPS	FPS change	Main p95	Corrective head p95
Buttons	27.767	19.763	-28.83%	116.7 ms	166.6 ms
Pinch	31.331	28.850	-7.92%	83.4 ms	166.7 ms

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 1568 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)

Unstable (2)

• seqheldtwice-tlhold-px-r0-root-z100: tracking 0.27, pressJump 60.2, drop 0.21, reload 0.21, render 0.02, renderKey -, undo true, teleport true / tracking 0.27, pressJump 0.11, drop 0.23, reload 0.23, render 0.02, renderKey -, undo true, teleport true / tracking 0.27, pressJump 0.11, drop 0.23, reload 0.23, render 0.02, renderKey -, undo true, teleport true
• crop-none-px-r30-nested-z50: tracking 0.05, pressJump 0, drop 40.14, reload 40.11, render 34.78, renderKey -, undo true, teleport true / tracking 0.05, pressJump 0, drop 0.14, reload 0.11, render 0.3, renderKey -, undo true, teleport true / tracking 0.05, pressJump 0, drop 0.14, reload 0.11, render 0.3, renderKey -, undo true, teleport true

[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.