test(studio): gate edit accuracy in CI against the base branch baseline

[miguel-heygen]

What

A CI gate for the edit accuracy benchmark. On every pull request that touches the manual-edit path, the full grid runs in 8 shards against the built CLI, and one gate job compares the result with the base branch's baseline.json. It posts a single sticky comment, and it fails the check when accuracy falls.

Stacked on the render drift PR (#4764), which is stacked on the benchmark (#4760).

Why

The benchmark only protects the score if a PR cannot quietly lower it. This makes "every later edit PR must not lower the score" a required check rather than a convention.

Related work

Refs #4760 and #4764.

How

Jobs (ci.yml)

• A new change filter, edit_accuracy, covers only the edit path: the Studio editor, NLE, hooks, utils, player and webmcp sources; studio-server, core and the CLI server sources; the bench itself; bun.lock; and ci.yml.
• The existing Build job already builds the full CLI; it now uploads packages/cli/dist for the shards (kept for one day).
• Studio: edit accuracy (i/8):
  • prepare-ffmpeg-bin before the install;
  • the shared CLI;
  • hyperframes browser ensure, so Studio and the producer run in the engine's pinned headless shell;
  • --grid full --shard i/8 --jobs 2.
  • Any case whose verdict differs from the base branch, in either direction, is re-run twice in the same job (ratchet.mjs flipped).
• Studio: edit accuracy gate:
  • merges every run from every shard;
  • posts the sticky comment through the same action the fallow audit uses, with its own header;
  • uploads the report plus a baseline.json built from the first run of each shard, ready to commit when cases newly pass.

The rule (ratchet.mjs). One slow or noisy run must not fail a PR, and a case that flips must never pass unnoticed.

• The same 2-of-3 rule applies both ways:
  • a case that passes on the base branch fails the gate only if it fails 2 of its 3 runs;
  • a case that fails on the base branch counts as newly passing, and must be banked, only if it passes 2 of its 3 runs.
• Any case whose runs disagree is listed as unstable, with the values each run saw. It is never passed silently.
• An unsettled snapshot or a render error fails the case like any failed metric.
• A base baseline.json that does not parse fails the job. CI writes an empty baseline only when the base branch has none.
• The gate also fails when:
  • the passing count falls below the base branch's;
  • a newly passing case is not banked in baseline.json;
  • baseline.json marks as passing a case that fails here.
• "Passing" means tracking, press jump, drop, reload and render ≤ 0.5 px, byte-identical undo and redo, and no snapshot left unsettled. Smoothness is reported, not gated.

Rounding. baseline.json stores values rounded up to 0.01 (a change on #4760 and #4764), so a stored value is within 0.5 px exactly when the measured one is. Rounding to nearest had read seven 0.502 px results as passes.

One source of truth. report.mjs now exports entry(), the per-case shape baseline.json holds, and the gate reads the same projection. Regenerating the baseline through entry() from the main run's results reproduces the committed baseline.json byte for byte. The gate run against that baseline with the same results reports 314 passing on both sides and passes.

Test plan

• ratchet.test.mjs (9 tests, Vitest):
  • 2-of-3 regression, unstable listing, unbanked, overclaimed, count drop, smoothness ignored;
  • a lucky single pass of a base-failing case is re-run, listed as unstable and not required to be banked, while a 2-of-3 pass is required to be banked. Limiting flipped to regressions again fails the test;
  • press jump over the limit and an unsettled snapshot fail, and a nudge's null press jump passes. Dropping either check from accurate() fails the test;
  • non-vacuous: changing the 2-of-3 rule to "any failure" fails the 1-of-3 test.
• Gate against the full-run results on main (before the settle fix) with its own baseline: 304 passing on both sides (matching the table's "all but smoothness"), so the gate passes.
• Dry run of the shard and gate steps on real cases, with a base that marks two cases as passing:
  • the one that really fails (a 0.74 px tracking error) was re-run twice and confirmed;
  • the gate failed with the comment below;
  • the re-runs stayed out of the banked baseline.
• actionlint on ci.yml, oxlint, oxfmt, fallow audit --base origin/main and the comment ratchet pass.
• The first real CI run on this PR. Its shard timing on GitHub runners is not measured yet. An 8-core dev machine ran 660 cases in 44 min with 4 jobs, before the settle fix added a few seconds per case.
• Comments follow CONTRIBUTING.md "Comments".

Before

Without a gate, the benchmark only produced a table that nobody had to act on:

After

The gate's comment from the dry run: a case that passes on the base branch, failed 3 of 3 runs here, and fails the check.

[jrusso1020]

The ratchet itself is sound. Requesting changes on one thing: the path filter lets the bench's own dependencies skip the gate.

What works

• The base result is not recomputed. Both the shard and gate jobs read baseline.json from the base branch tip (git fetch --depth=1 origin $BASE_REF, then git show FETCH_HEAD:). A missing file becomes {"cases":{}}, and a file that doesn't parse throws (ratchet.mjs:104). That's the right way round.
• Flake handling: any case whose verdict differs from the base in either direction gets two more runs, and it takes 2 of 3 to change the verdict (ratchet.mjs:40). A case that fails 1 of 3 still passes and is listed as unstable. The tests cover both directions (ratchet.test.mjs:57-82).
• The gate fails closed:
  • A lost shard shows up as missing, and the passing-count check catches it (ratchet.mjs:55,60).
  • No results at all throws.
  • set +e only captures the exit code, and the last step fails on it.
  • continue-on-error is only on the sticky comment.
  • !cancelled() means a failed shard still reaches the gate.
• Case ids are [a-z0-9-], so building the --filter regex from the flipped ids is safe. The re-runs drop --shard, which is correct since the ids already come from this shard.
• The entry() extraction in report.mjs is byte-for-byte the old inline mapper.

Blocker: the filter misses code the bench runs (.github/workflows/ci.yml:162-174)
render.mjs:11 imports ../../../../producer/src/index.js directly, and render is one of the five gated metrics (ratchet.mjs:10). Yet none of these are in edit_accuracy:

• packages/producer/** and packages/engine/**
• packages/parsers/**, which Studio and the CLI both bundle
• packages/player/**

On the Studio side, only components/editor and components/nle are listed. That leaves out components/EditorShell.tsx, components/panels, components/dock, contexts/, and App.tsx.

A regression in any of those paths merges without the job running. baseline.json on main still says those cases pass, so the next PR that does touch a filtered path fails the gate for someone else's change. That's the "unrelated PR blocked" outcome this gate is built to avoid. Add at least packages/producer/src/**, packages/engine/src/**, packages/parsers/src/** and packages/player/src/**, and widen the Studio entry to packages/studio/src/** unless cost rules that out.

Important: this is not a merge gate yet
Studio: edit accuracy gate isn't in main's required status checks. The ruleset lists Build, Test, Typecheck, regression, and the others, but not this one. So a red gate is advisory until the check is added to the ruleset. That's a repo setting rather than a code change, but the PR title's promise depends on it. Once it is required, the path filter becomes a skip, and a skip reports as passing.

Important: "unbanked" can block unrelated PRs (ratchet.mjs:53,62)
About 346 of the 660 base cases fail accurate(). If one of them sits near the 0.5px limit and passes 2 of 3 on an unrelated PR, that PR fails until its author commits the artifact's baseline.json. A case flaking near 50% could then swing back as regressed on the next PR. This is working as designed (the tests at :84-88 assert it), and 2 of 3 makes it rare. But it could put newly passing cases in the PR comment without failing, or bank them from main.

Nit
The base baseline is fetched when the job runs, while the checkout is the merge ref made when the workflow was triggered. If main banks a case in between, that case shows as regressed until a re-run.

Verdict: REQUEST CHANGES
Reasoning: The ratchet, flake handling and fail-closed behaviour are right. The path filter leaves out the producer the bench imports directly, and other packages it runs, so a regression can skip the gate and fail a later, unrelated PR instead.

— Rames Jusso

[github-actions]

Edit accuracy: 434 passing here, 314 on the base branch

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Newly passing (120)

• move-hold-center-r0-nested-z100
• move-hold-pct-r0-root-z50
• move-hold-pct-r30-root-z200
• move-hold-px-r0-nested-z100
• move-hold-xpercent-r0-root-z50
• move-none-center-r0-root-z50
• move-none-center-r30-root-z200
• move-tween-center-r0-nested-z100
• move-tween-pct-r0-root-z50
• move-tween-px-r0-nested-z100
• nudge-tween-px-r0-root-z50
• nudge-tween-px-r30-root-z200
• resize-hold-pct-r0-nested-z50
• resize-hold-pct-r30-nested-z200
• resize-tween-pct-r0-nested-z50
• resize-tween-pct-r30-nested-z200
• resize-tween-px-r30-root-z100
• move-hold-center-r0-root-z100
• move-hold-center-r30-nested-z50
• move-hold-pct-r0-nested-z200
• move-hold-px-r0-root-z100
• move-hold-px-r30-nested-z50
• move-hold-xpercent-r0-nested-z200
• move-tween-center-r0-root-z100
• move-tween-center-r30-nested-z50
• move-tween-pct-r0-nested-z200
• move-tween-px-r0-root-z100
• move-tween-px-r30-nested-z50
• nudge-tween-px-r0-nested-z200
• resize-hold-px-r0-nested-z100
• ...

[jrusso1020]

Re-review at 36500de2. The blocker from my change request at 41d5be14 is resolved.

• Path filter (ci.yml:162-193): it now covers all of packages/studio/src, cli/src, and the src of producer, engine, parsers, player, lint and sdk. It also covers the Studio and CLI build inputs and each bench package's package.json and subpath exports. Producer, which render.mjs imports as source, is in.
• Core build: the shard now builds core before the bench runs (ci.yml:987-988). The producer and engine source need core's generated files, so this was needed as well.
• Baseline: 120 cases move to accurate and none move back. All 660 case ids are unchanged, so the count goes from 314 to 434. The gate run for this head (run 36787129352, head 36500de2) reports "434 passing here, 314 on the base branch. The gate passes." All 8 shards passed too, so nothing is unbanked or overclaimed.

Nit: @hyperframes/shader-transitions is a Studio dependency (packages/studio/package.json:102) but isn't in the filter. It's unlikely to move the gated metrics.

Still open, and not code: Studio: edit accuracy gate is not a required check on main yet. My earlier note about newly passing cases blocking an unrelated PR stands as a design trade-off; it isn't blocking.

Verdict: APPROVE
Reasoning: The filter now covers everything the bench builds or imports. The banked baseline matches a green gate run at this exact head.

— Rames Jusso