Skip to content

feat(core): the playback health reading says which clock the film followed - #6906

Merged
miguel-heygen merged 3 commits into
mainfrom
dwebprice-clock-source
Nov 1, 2026
Merged

miguel-heygen merged 3 commits into
mainfrom
dwebprice-clock-source

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What

The playback health reading the runtime posts (preview-health) now says which clock the film's time came from at its newest tick: clockSource is "audio" (an audio element's currentTime), "webaudio" (the WebAudio context's time) or "monotonic" (performance.now()). Studio's parsePlaybackHealth keeps it, and only one of those three values.

Why

A choppy preview often shows a high skipped share while the page itself is idle: the film's scripts and rendering take almost none of the frame, and the tick gaps sit near 17 ms. When the clock follows an audio element, currentTime moves in the audio device's buffer strides (about 15 to 25 ms on Linux, 90 ms and more at times on macOS in probes). A 60 fps film then draws only when a stride crosses a frame, which the meter counts as skipped. A host reading the health report cannot tell that case apart today, because the reading does not say which clock was in charge.

Related work

None open for this. Hosts that already read preview-health get the field with no change; a host that ignores it is unaffected.

How

  • TransportClock.followedClock() names the clock from what is attached: no audio source (or a paused one) is monotonic; an element is audio; the WebAudio time is webaudio. getSource() cannot tell the last two apart, because the runtime also attaches a held currentTimeSeconds while an audio element buffers, so the WebAudio attach in init.ts now marks itself (webAudio: true). A held position stays audio, the clock it is standing in for. getSource() and the snapshot are unchanged.
  • notePlaybackFrame takes the clock as a required input (its one caller is the runtime tick in init.ts), and the reading posts the clock of its newest tick.
  • RuntimePreviewHealthMessage and studio's PlaybackHealth carry the field; the parser drops anything that is not one of the three names, so a page cannot put free text into it.

Test plan

  • Unit tests added/updated
  • Manual testing performed
  • Documentation updated (if applicable)
  • Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug

On a Linux build box at this head, after building parsers, lint, studio-server and core: oxlint and oxfmt clean on the changed files; tsc --noEmit green in packages/core and packages/studio; clock.test.ts + playbackHealth.test.ts 63/63, previewQualitySlice.test.ts 5/5.

Each new test is red on main's source (the same run with the five source files checked out from main and the tests kept: 2 of 63 and 1 of 5 fail, exactly these):

  • clock.test.ts, "names the clock it follows: an audio element, WebAudio, or neither": no element, element, WebAudio, a held buffering time, and detach.
  • playbackHealth.test.ts, "says which clock the film's time came from": the reading follows the tick's clock from monotonic to audio to WebAudio.
  • previewQualitySlice.test.ts, "keeps the clock the runtime names, and only a clock it knows": a path in the field is dropped.

Not exercised: a real preview in a browser. The field rides an existing message, so nothing new is drawn.

No visible change

The studio side is one optional field on the stored health reading and its parser (4 lines, no .tsx/.css/.html); nothing is drawn from it, so there is no capture to take.

Size

75 lines, alone on purpose: it changes one message's shape in the runtime and the one parser that reads it, so the release that carries it can be pinned by any host on its own. There is no open runtime PR of mine to fold it into.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · head 442af530

The code itself is sound: followedClock() matches getSource(), the one notePlaybackFrame caller passes the clock, and the studio parser accepts only the three names. What blocks is CI, and the body doesn't explain either failure.

  • blocker: the required check Studio and player captures fails. The PR touches packages/studio, so the body needs ## Before / ## After sections with captures. If the change is small enough for the gate's exemption, a ## No visible change section works instead.
  • important: the Comments ratchet fails. It reports a higher comment share in clock.ts (+2 lines), playbackHealth.ts (+1) and types.ts (+1). Cut the new doc comments, or move the meaning into names and types (e.g. the webAudio flag and the clock union already say it).
Scope

Read all 8 files at head, plus TransportClock.getSource/attachAudioSource and the attach sites in init.ts (5630–5646). CI was still running other workflows at review time; the two failures above are complete.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: APPROVE — 2 resolved, 0 open, 0 new · 442af530 → 2c83eaf7. The ## No visible change section turns PR captures green, and cutting the 4 comments turns Comments green; the head changes nothing except those deletions.

@github-actions

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 119 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 (1)

  • seqplainkeys-none-center-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 31, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 31, 2026
Merged via the queue into main with commit bba3676 Nov 1, 2026
96 of 97 checks passed
@miguel-heygen
miguel-heygen deleted the dwebprice-clock-source branch November 1, 2026 00:19
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.

1 participant