Repository navigation
fix(core): settle slow documents against their own frame cadence - #5168
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
terencecho
left a comment
terencecho
left a comment
There was a problem hiding this comment.
Review at cc07ff1a7fb1b9504a40916ba9dc7058414774e1 — approved.
packages/core/src/compositionReadiness.ts:278-292 adds the relative-cadence path without replacing the existing strict < 50 ms fast-page qualification: two consecutive qualifying gaps are still needed, and a stall resets the streak. The shortest gap is retained for the attempt rather than increased after a stall. A focused timestamp replay showed steady 66 ms frames settling early, while 8/16 ms, alternating 16/33 ms, and zero-timestamp fast sequences settle at their previous frame. The 1.5-second cap and timeout/abort behavior remain; media and compute readiness, player generation checks, runtime first-frame reporting, and Studio's Play/shadow-promotion gates remain separate. The effective merge-base diff is the core implementation, its focused tests, and the readiness contract. Required checks passed at this head. I did not independently reproduce the PR body's browser timing distributions or mutation runs, nor run a full real-browser Studio reload.
Nonblocking limitation: An initial outlier-fast frame can anchor the minimum below the eventual slow cadence. With timestamps 0,16,82,148,214,…, later 66 ms gaps fail both < 50 and <= 1.5 × 16, so this attempt reaches the existing cap despite the subsequent regular cadence. An alternating 60/120 ms cadence likewise never makes two consecutive qualifying gaps and reaches the 1.5-second paint cap; a focused replay confirmed that delay reaches the player loader/assetsready and Studio Play readiness when the other gates have cleared. The steady-slow tests begin with a slow gap and do not exercise the first transition. These cases are conservative and no worse than the former cap behavior, but worth measuring and testing if either pattern is common on target pages. Likewise, a permanently steady cadence alone cannot prove content completion; the separate media/compute gates remain important.
— tai
Verdict: APPROVE
Reasoning: The new slow-cadence qualification improves the measured case without delaying any previously qualifying fast sequence or weakening the existing cap, cancellation, and consumer gates. The short-initial-gap case limits the improvement but does not introduce a correctness regression.
Edit accuracy: accurate 2059 (base branch 2059), smooth 1669 of thoseThe gate passes. Quarantined, measured but not gated (0) |
This branch was successfully deployed
2 participants
What changed
A steadily slow document no longer has to reach the paint wait's 1.5-second cap merely because its frames are more than 50 ms apart.
The shared core readiness input preserves the existing strict
< 50 msfast-page path. Slower gaps qualify when they are at most 1.5 times the shortest gap observed in the current readiness attempt. Two consecutive qualifying gaps are required; a stall resets the streak. The existing paint cap, shared timeout and cancellation behavior remain.Media, compute, loader and generation checks retain their roles. Studio still promotes the picture and commits its timeline together. The same input serves the player, runtime-reported first-frame readiness and Studio's Play gate.
What I measured
Controlled Linux/Chromium fixture with sixfold CPU throttling, inline content and 60 ms of main-thread work per frame. The burst case adds one 240 ms workload before returning to the ordinary cadence. Timing begins before the first workload and includes scheduling. Each row has three runs. Measurements use main's core input and executable candidate
23b8c9c8; finalcc07ff1achanges only comments and documentation. These are fixture measurements, not production latency estimates.Ordinary observed gaps were approximately 50 to 100 ms; the injected burst produced a 250 ms gap. Before implementation, I replayed 183 overlapping nine-gap windows that did not contain the injected burst:
I chose 1.5x as the smallest tested tolerance that covered all ordinary windows. The windows overlap and are not independent trials. Absolute timing is affected by shared-machine activity. The 1.5-second cap was retained, not retuned.
Review exposed a compatibility problem in using a running minimum alone: 8 ms followed by 16 ms frames, alternating 16/33 ms frames, or zero timestamps could delay fast pages. The final rule retains the old fast path exactly. Deterministic tests cover those sequences and ensure later stalls cannot raise the shortest-gap baseline.
At
23b8c9c8, the readiness file passes 35 tests with no skips in three consecutive runs. The player asset-ready group passes 32 selected tests, with 216 intentionally excluded by the name filter. Studio Player, shadow reload and player store pass 21, 37 and 70 tests respectively, with no skips. Builds, repository lint, formatting and both core typechecks pass.Nine deliberate mutations each fail one targeted test, with the other 34 excluded by the filter: removing relative readiness, narrowing tolerance, losing streak reset, removing the fast path for variable-refresh/alternating/zero cases, losing baseline ownership in either direction, and removing the cap. Source is restored before the passing runs.
At final
cc07ff1a, the 35-test readiness file passes three more consecutive runs with no skips. Repository lint, formatting, core build and both core typechecks pass with exit 0.The readiness contract records the producer and consumer boundaries.
The CI timeline viewport check currently fails: interaction p95 is 72.1 and 72.4 ms against a 58.3 ms ceiling. A serial, same-harness diagnostic with only the core readiness source changed fails on both the unmodified base and this head. That diagnostic also fails the frame budget, unlike CI, and ran under shared load, so it does not establish CI attribution or clear the failed check.
What I did NOT exercise
A full Studio reload in a real browser, macOS/Windows, or a physical variable-refresh display. The browser fixture executes the real core input; consumer tests use their existing DOM harnesses. A steady cadence can still represent unfinished work, so compute and media readiness remain separate gates. No downstream timeline commit was moved earlier.