ci(studio): run the timeline viewport gate on studio changes - #2884
ci(studio): run the timeline viewport gate on studio changes#2884miguel-heygen wants to merge 4 commits into
Conversation
8caed04 to
d9fc26a
Compare
9a4b176 to
0f2b4c6
Compare
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
d9fc26a to
ea6f930
Compare
0f2b4c6 to
4f63c92
Compare
|
Exact pushed tip
One immediately preceding virtualized invocation passed every DOM budget but reached only 3/5 responsiveness quorum because two samples recorded isolated 52ms and 61ms long tasks. The clean rerun recorded no long tasks. This is included for transparency; the gate correctly rejected the noisy invocation rather than hiding it. The GitHub Preview parity rerun is now green as well, including ffmpeg installation and the Chrome parity job that previously timed out. |
ea6f930 to
3e47aa7
Compare
546751c to
d82a1d1
Compare
3e47aa7 to
1acce7a
Compare
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
🟠 Mechanism looks sound at exact head 1acce7a60a0344880a967e3126c680eeb7615d60, with one required landing check and one evidence-quality follow-up.
Reviewed only d82a1d12..1acce7a6 (the workflow + gate-script delta), not the preceding performance implementation.
Landing gate
The new Studio: timeline viewport gate has not run on this exact head. This PR is stacked on #2883 rather than main, while the workflow only triggers for pull_request.branches: [main]; the exact-head check list contains preview/regression checks but no execution of the job introduced here. The local Chrome evidence is useful, but it is not proof that the GitHub Actions wiring, dependency build, dual-server startup, hosted-runner budgets, and artifact upload work together.
Before stamping/merging, restack this PR onto main and require Studio: timeline viewport gate to pass on the final exact SHA. The added edited trigger is specifically intended to make that happen when Graphite restores the base.
P2 — the “both arms” step stops after the first failing arm
The job uses set -euo pipefail, then runs the default arm before the virtualized arm. If the default arm fails, the shell exits immediately: the virtualized build is never tested and the always-upload step contains only /tmp/timeline-gate-default.json (or no complete JSON if failure occurs before final output). That is weaker than the step name and PR claim that both arms run and evidence is retained on failure.
This does not create a false green, so I am not treating it as a correctness blocker. For useful red-run evidence, capture each arm's status, run both regardless, then exit nonzero if either failed.
The rest of the wiring checks out: Studio/core/server changes activate the gate, the tracked fixture is copied into the expected project location, the two module-load configurations use separate Vite processes, CI gets a dedicated budget tier, the gate asserts the observed flag state, and the latest push-only file-size range correction is valid.
git diff --check passes.
— Magi
1acce7a to
923e96b
Compare
d82a1d1 to
b97d8c7
Compare
923e96b to
b2908dd
Compare
b2908dd to
f366401
Compare
fd2138d to
392cd8a
Compare
fe5b251 to
a1cc721
Compare
The gate has existed since the row virtualization stack landed but nothing under `.github/` referenced it, so it only ever ran when someone ran it by hand. That is how the flag-off scroll regression reached eight merged-ready PRs without anything noticing. Adds a `studio-timeline-viewport` job that boots two Studio dev servers, one per flag state, and runs both arms of the gate against them. Two servers are needed because row virtualization is read from `import.meta.env` at module load, so one process cannot serve both builds. Scoped to a new `studio` paths filter rather than the broad `code` one: the gate only says anything about `packages/studio`, `packages/core` and `packages/studio-server`. Adds a `ci` tier. It applies the constrained budgets without any emulation, because a hosted runner is already slower and noisier than the machine the strict numbers were recorded on, while the existing `low-resource` tier would throttle it a further 4x and measure the throttle rather than the build. The fixture composition is tracked under `tests/e2e/fixtures` but Studio resolves projects from the gitignored `data/projects`, so the job copies it into place instead of a project directory being committed. Both arms run in about 7 seconds each locally, so the job cost is almost entirely dependency install and the workspace build it shares with `studio-load-smoke`.
a1cc721 to
25503ec
Compare
Fallow audit reportFound 22 findings. Duplication (21)
Health (1)
Generated by fallow. |
|
Exact stacked-head evidence is now green for
I temporarily based this PR on |
1 participant
What
Adds a
studio-timeline-viewportCI job that runs both arms of the timeline viewport gate: the default build at 1,000 elements and the row-virtualized build at 50,000.Supporting changes:
studiopaths filter, so the job runs onpackages/studio,packages/coreandpackages/studio-serverrather than on the broadcodefilter.citier in the gate script. It applies the constrained budgets with no emulation.packages/studio/tests/e2e/fixtures/timeline-virtualizationintopackages/studio/data/projects, because that is where Studio resolves projects from and it is gitignored.Why
The gate has existed since the row virtualization stack landed, but nothing under
.github/referenced it, so it only ran when someone ran it by hand. That is how a scroll regression in the default build reached eight ready-to-merge PRs with no signal: the gate takesSTUDIO_URLfrom the environment and never sets the flag itself, so every run anyone did happened to measure the enabled build.A gate nobody schedules is documentation, not a gate.
How
Two dev servers, one per flag state. Row virtualization is read from
import.meta.envat module load, so a single process cannot serve both builds. The boot-and-wait shape is copied from the existingstudio-load-smokejob, including the workspace build steps that letvite.config.tsresolve the workspace packages through theirnodeexport condition.The
citier exists because neither existing non-primary tier fits a hosted runner.low-resourcethrottles the CPU a further 4x, which on an already slower and noisier machine measures the throttle rather than the build, andhigh-dprchanges the scale factor.citakes the constrained budgets (75ms interaction p95, 50ms frame interval p95) and leaves emulation alone.Chrome resolution needs no new setup: the gate already probes
/usr/bin/google-chrome, which the hosted image provides. The Chrome major is only pinned whenTIMELINE_CHROME_MAJORis set, and this job does not set it.Gate evidence is uploaded as an artifact on success and failure, since a red run is exactly when someone needs to read it.
Test plan
Both arms were run locally against the
citier, on the same dev servers CI will boot, using the same commands the job runs:Each arm takes about 7 seconds, so the job cost is almost entirely the dependency install and workspace build it shares with
studio-load-smoke.The workflow file was parsed to confirm the job and the new filter output are wired up.
One thing to watch, stated plainly rather than discovered later: the virtualized arm passes 4 of 5 runs, which is exactly the required threshold. The failing run is a 53-58ms long task against a 50ms limit, reproducibly on the fourth measured run. That is the residual window-swap tail, it predates this PR, and the constrained budgets do not relax
longTaskLimitMs. On a slower runner more runs could cross it and this arm would go red. This PR's own CI run is the real measurement; if the virtualized arm proves flaky there, the honest fix is to address the tail or hold that arm back, not to widen the budget.