Skip to content

fix(core): keep rebound root timelines clock-driven - #5165

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/runtime-rebind-clock-owned
Oct 7, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/runtime-rebind-clock-owned

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

When late media metadata selected a replacement root during playback, the runtime restored its position and called the root's play(). That gave GSAP's global ticker permission to advance the timeline independently of the transport clock.

The replacement root now stays paused after its position is restored. The transport remains playing and continues driving the root with explicit seeks. Child activation is unchanged.

Validation

  • A regression through the public player and media-metadata event uses real GSAP. The original code fails because the rebound root is unpaused. The fix keeps playback active and the root paused; advancing GSAP's global ticker leaves its position unchanged. Test timing is deterministic.
  • All 221 runtime-init tests pass in three consecutive runs with no skipped tests. All 35 parked-loop tests pass. Repository lint, core and runtime type checks, formatting, skill lint and comment checks pass.
  • Real Chromium cross-process fixture with late-loaded WAV metadata. Original code: transport playing, root unpaused, a 0.5-second GSAP ticker advance moves its time from 0.0832 to 0.5832. Fixed head: transport playing, root paused, the same ticker advance leaves its time at 0.1361. The controlled fixture uses a monotonic transport clock to isolate root ownership from audio-clock selection.

Known gate red

The Studio timeline viewport gate has a known overscan regression addressed by #5151. This fix changes only runtime root ownership and its regression test.

Limits

The browser evidence is from Linux. macOS, Windows and desktop-host playback have not been exercised.

The old autonomous root also could continue ticking after the transport reached its end. Keeping the root clock-driven removes that second source of motion through the end state as well.

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 7, 2026 07:15

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at cb71a652. Approving.

Why it's right. Everywhere else, the runtime keeps the root paused and lets the transport drive it:

  • transport.play() calls pauseTimelineIfPossible(tl) before clock.play() (init.ts:3854).
  • The first bind pauses after its totalTime restore (init.ts:1960).

rebindTimelineFromResolution was the one place that broke that rule by calling play() on the replacement. Deleting it makes the rebind match the bind and play paths. Nothing new is added, so on reuse and simplicity this is the smallest possible fix.

Every root-replacement site checked

  • state.capturedTimeline = appears at init.ts:1922 (first bind: pauses), :2089 and :5158 (set to null on teardown), :2297 (this fix), and :3969. The last is the player's setTimeline setter, and player.ts never calls it.
  • rebindTimelineFromResolution has a single caller (init.ts:2378, the media-metadata rebind).
  • The other play() / paused(false) sites are the child and sibling activation helpers (init.ts:1615, :4268). They're unchanged, and their comments already say the transport re-pauses or re-seeks them. Children held by the root still follow its totalTime, so they don't depend on the root's own ticker.

Checked locally

  • The new test passes at this head. With only the init.ts change reverted, it fails at expect(rebound.paused()).toBe(true), so it guards the bug with real GSAP.
  • Full init.test.ts: 221/221. With init.timingResolver.test.ts and audioFxCopy.test.ts (the parked-loop coverage) added: 251/251.

Limits, agreed with the PR body: the browser evidence is Linux only. The change is a removed call in platform-independent runtime code, so I don't expect macOS, Windows or desktop-host behaviour to differ. That's reasoning, not something I ran.

Verdict: APPROVE
Reasoning: It removes the only root-rebind path that handed the timeline to GSAP's global ticker, which aligns it with how play and the first bind already work, and the regression test fails on the old code.

— Rames

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2059 (base branch 2059), smooth 1521 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)

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 7, 2026
Merged via the queue into main with commit 9701f0d Oct 7, 2026
164 of 165 checks passed
@miguel-heygen
miguel-heygen deleted the fix/runtime-rebind-clock-owned branch October 7, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants