Skip to content

ci(studio): run the edit accuracy slice on PRs and the full grid in the merge queue - #5566

Merged
jrusso1020 merged 3 commits into
mainfrom
ci/edit-accuracy-gating
Oct 14, 2026
Merged

jrusso1020 merged 3 commits into
mainfrom
ci/edit-accuracy-gating

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

What

Make the Studio edit accuracy gate cheaper, while running the full grid before anything reaches main.

  • Narrower trigger. The gate now runs only for changes on the bench's path: Studio, studio-server, core, parsers, player, sdk, lint, the CLI preview server, and the engine and producer modules it captures frames with. Test files, fixtures and docs don't trigger it.
  • Pull requests run the pr slice in 5 shards. That's zoom 100 with no rotation, 435 of 2,061 cases.
  • The merge queue runs the full grid in 20 shards.
  • Newly passing cases that aren't in baseline.json yet are reported, not failed.
  • The gate fails if any shard didn't report (--shards N).

Why

  • Cost. The gate used about 83% of all CI runner time, roughly 8 runner hours per push, and it ran on PRs that can't affect it. fix(engine): keep the rightmost 8 columns of SDR MP4 at widths like 1080 #5559, a two-line ffmpeg filter change in the encoder, paid for 20 shards.
  • Failures, from 170 failed gate runs since 9/30:
    • About 45% were newly passing cases that hadn't been banked yet.
    • About 21% were build or missing-shard failures.
    • About 18% were main drifting: the same case set failed on many unrelated PRs at once.
    • About 14% look like real catches, all from Studio, core or player changes.
  • Main isn't protected today. The gate never ran in the merge queue, so a regression that merged showed up as everyone else's failure.

Related work

Refs #4765 (adds the gate), #4866 (12 → 20 shards).

How

  • ci.yml:
    • adds a separate paths-filter step with exclusions, using predicate-quantifier: some-with-excludes, based on merge_group.base_sha in the queue;
    • the shard matrix and grid depend on the event;
    • Build runs when the gate does.
  • grid.mjs: pr is now zoom 100 and rotation 0 across both nestings. It held a regressed case for 10 of the 11 real catches; the old slice held 6.
  • ratchet.mjs:
    • newly passing cases become a notice;
    • on the slice, the gate compares only the cases that ran;
    • --shards checks every shard reported;
    • only a full run writes a bankable baseline.
  • check-test-reachability.mjs: accepts ! exclusions, but only in a step that opts into some-with-excludes.

The required check name, "Studio: edit accuracy gate", is unchanged. It still skips, and so passes, when the filter doesn't match.

Estimated cost: about 6,700 runner hours a week down to about 2,200.

Test plan

  • Unit tests added/updated (ratchet.test.mjs, check-test-reachability.test.mjs)
  • actionlint clean
  • Path patterns checked with picomatch. fix(engine): keep the rightmost 8 columns of SDR MP4 at widths like 1080 #5559's files no longer match; Studio, core and player files do.
  • First merge queue run: confirm 20 shards, that strategy.job-total renders in the job names, and that the baseline comes from base_sha.

For review:

  • The queue's 60 minute timeout. Build plus a full run is about 30 min before any runner wait.
  • The engine and producer file lists are explicit, so a new import into those modules needs a filter entry.
  • Banking newly passing cases is still manual.

No visible change

CI configuration and bench tooling only. No product code changes.

— Rames

🤖 Generated with Claude Code

…he merge queue

The edit accuracy gate used about 83% of CI job-minutes, ran on changes that
cannot reach the bench (encoder, render and CLI command code), and never ran
in the merge queue, so main could drift and fail every later PR.

- Path filter: its own paths-filter step with some-with-excludes, limited to
  what the bench runs: Studio, studio-server, core, parsers, player, sdk, lint,
  the CLI preview server path, and the engine and producer modules in the
  import closure of browserManager, frameCapture and both fileServers. Test
  files, fixtures and docs no longer trigger it. The bench's webmcp-host.mjs,
  which it imports but the old filter missed, now does.
- merge_group: the filter's base is the event's base_sha there. In the queue
  the full grid always runs (20 shards) whenever the filter matches. The queue
  is the last point before main, and nothing else measures the full grid
  now that PRs run a slice. Skipping it would let zoom 50/200 and rotated
  regressions land on main, where they fail every later PR against the
  banked baseline. That drift caused about a fifth of past gate failures.
- PRs: the existing `pr` grid becomes zoom 100 with no rotation in both
  nestings (435 of 2061 cases), in 5 shards. Of the 11 PRs whose real
  regressions the full gate caught, this slice held a regressed case for 10.
- Gate: newly passing cases missing from baseline.json are listed in the
  comment and as a notice, not failed (45% of past gate failures). A partial
  grid compares only the cases it ran. `--shards` fails the gate when a shard
  reported no results, and only a full grid writes a bankable baseline.json.
- The required check "Studio: edit accuracy gate" keeps its name. It is
  skipped when the filter does not match, as before.
- check-test-reachability models negated globs as exclusions under
  some-with-excludes and still fails closed elsewhere.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 435 (base branch 435), smooth 289 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 (2)

  • crop-none-px-r0-nested-z100: tracking 0.04, pressJump 0, drop 40.07, reload 40.07, render 40.03, renderKey -, undo true, teleport true / tracking 0.04, pressJump 0, drop 0.08, reload 0.08, render 0.03, renderKey -, undo true, teleport true / tracking 0.04, pressJump 0, drop 0.08, reload 0.08, render 0.03, renderKey -, undo true, teleport true
  • crop-none-px-r0-root-z100: tracking 0.04, pressJump 0, drop 40.03, reload 40.03, render 39.98, renderKey -, undo true, teleport true / tracking 0.04, pressJump 0, drop 0.06, reload 0.06, render 0.03, renderKey -, undo true, teleport true / tracking 0.04, pressJump 0, drop 0.06, reload 0.06, render 0.03, renderKey -, undo true, teleport true

@jerrai-bot-heygen jerrai-bot-heygen 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.

Reviewed exact head afe5a7d5c338fa9df55cfacfb2a3e3e5b35c2bf0 first in the stack. The active main ruleset requires Studio: edit accuracy gate and a merge queue; this workflow is configured for 5 PR shards and 20 queue shards, and the exact-head five-shard PR slice executed green. But the new path filter misses the root build manifest and the workspace installer that the accuracy job itself executes, letting relevant changes skip both the PR slice and full grid. See inline repro and requested path-filter regression. I have not observed the first 20-shard merge-queue run, so this review does not claim it executed. The shortened/notice behavior is otherwise internally coherent in the inspected ratchet; the filter gap blocks approval. No merge by me. — Jerrai

Comment thread .github/workflows/ci.yml
- "packages/aws-lambda/scripts/probe-beginframe.ts"
- "packages/engine/src/services/browserManager.ts"
- "package.json"
- "scripts/ci/base-file.sh"

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.

The path filter omits two executed inputs: root package.json (build selects and builds Studio/core/player/etc. at line 14) and scripts/ci/install-workspace-dependencies.sh (run by both Build and every edit-accuracy shard in this workflow). A PR changing either can change the built Studio or bench dependencies while edit_accuracy is false; both the five-shard PR slice and required full-grid merge-queue gate are then skipped. Please include these in this filter and add positive path-filter tests for each, alongside the already included bun.lock and scripts/ci/base-file.sh.

The path filter missed the root package.json, the dependency install script
and the ffmpeg setup action, which Build and every shard run. A change to any
of them could alter the bench while the gate skipped. Add them, and a test that
every local input of the jobs that build and run the bench is in the filter.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@jerrai-bot-heygen jerrai-bot-heygen 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.

Re-reviewed exact head 10ad72113c5ab171f62e3b7e3e6771c6684c8d84, superseding my earlier filter-gap change request. The new filter includes root package.json, scripts/ci/install-workspace-dependencies.sh, and the local ffmpeg preparation action consumed by Build/the accuracy shards. The new test positively asserts these inputs and negatively checks excluded tests/docs; exact-head hosted Test reachability ran both new filter assertions (30 tests passed, zero orphans). The existing path-filter change is otherwise scoped to Studio/core/player/capture inputs; the main ruleset requires Studio: edit accuracy gate and merge queue, with a configured five-shard PR slice and twenty-shard queue full grid. The prior head's five-shard slice executed green, while the first exact-new-head full queue grid has not been observed; fresh long accuracy shards were not yet completed at this review. This is code-quality approval, not certification that the queue grid has executed or permission to merge. — Jerrai

@jrusso1020
jrusso1020 enabled auto-merge October 13, 2026 23:21

@miguel-heygen miguel-heygen 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.

Independent review of exact head 10ad72113c5ab171f62e3b7e3e6771c6684c8d84.

Jerrai's root-manifest/install-script/ffmpeg-action finding is fixed. My additional transitive-input check still finds a skip gap in .github/workflows/ci.yml:244: packages/producer/src/services/frameCapture.ts:5 is the module through which render.mjs imports the actual capture functions, but the producer filter includes only index.ts, logger, generated stub, selected file servers and a few helpers. Changing that existing wrapper can change every measured render while both accuracy jobs skip. The build also executes packages/producer/scripts/build-hf-early-stub.ts and reads packages/producer/stubs/hf-early-stub.ts; neither matches. Those produce the early script injected by the producer file server. Please cover the existing transitive source/generator inputs (including relevant build configs), and extend the filter test beyond direct workflow command strings. Its localInputs() at scripts/check-edit-accuracy-filter.test.mjs:52 does not follow package scripts/imports, so its claim of covering every local input currently overstates the test.

Verification: 30 filter/reachability tests and 73 bench tests pass locally using an existing dependency environment. A new positive assertion for the two early-stub paths fails at this head; picomatch also returns false for both. The source trace above is independent of that custom glob matcher. The active main ruleset requires the accuracy gate and merge queue. Code selects 5 PR shards versus 20 full-grid queue shards, uses queue base_sha, and the body-edit reuse action only runs for PR edited events, so it cannot substitute the PR slice for a queue run. The current five accuracy shards remain in progress; no full queue run observed. Fallow audit is red (new filter matcher complexity); other reported required checks are green. No workflow dispatched, merge or queue action taken.

Verdict: REQUEST_CHANGES.
Reasoning: The path-filter coverage still allows a current bench dependency to bypass both the PR and merge-queue gate. Resolve that before relying on the narrower trigger.

— Magi (desktop-review)

@jrusso1020
jrusso1020 disabled auto-merge October 13, 2026 23:36

@jerrai-bot-heygen jerrai-bot-heygen 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.

I am withdrawing my earlier approval at this same exact 10ad72113c5ab171f62e3b7e3e6771c6684c8d84 after Magi's independent transitive-input finding, which I verified at source. The repair covers the direct root manifest, installer and ffmpeg action, but .github/workflows/ci.yml still excludes packages/producer/src/services/frameCapture.ts: the bench imports capture functions from the producer index, whose wrapper re-exports them. It also omits packages/producer/scripts/build-hf-early-stub.ts and packages/producer/stubs/hf-early-stub.ts, although the producer build runs the generator and its output affects served capture files. Changes to these current inputs can alter measured renders while both PR and queue accuracy jobs are skipped. The new local-input test follows workflow command strings, not transitive package scripts/imports, so its green assertion does not close this census. Please extend filter and a discriminating positive test, then re-check the suite. The exact-head Fallow audit check is also red; read and repair its diagnostic before re-review. Five accuracy shards/full queue run were not completed when this verdict was checked. My prior APPROVED review should not be used as clearance; no merge by me. — Jerrai

The filter test now walks the files the bench actually runs: the bench
entry files, their relative and @hyperframes/* imports, the CLI commands
the bench invokes, and the build scripts, configs and files those builds
read for every package the bench reaches. Globs are matched with
picomatch, the matcher dorny/paths-filter uses.

The walk found 183 inputs the filter missed, including the producer's
frameCapture.ts, build-hf-early-stub.ts and stubs/hf-early-stub.ts, the
engine services behind engine/src/index.ts, and the CLI browser command.
The filter now covers them.

picomatch is not in the no-install Test reachability job, so the filter
test moves to test:scripts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jrusso1020

Copy link
Copy Markdown
Collaborator Author

Thanks both. Fixed at f53a5d1586.

The filter test now follows the real inputs, not just the workflow commands. scripts/check-edit-accuracy-filter.test.mjs:

  • It starts from what the jobs run: run.mjs through test:edit-accuracy, ratchet.mjs, scripts/ci/*, the ffmpeg action, and cli.ts plus the two CLI commands the bench calls (browser and preview).
  • It follows relative and @hyperframes/* imports, resolved through each package's exports or main.
  • For every package it reaches, it also follows package.json, the tsconfig chain and the build script, through bun run, --cwd, --filter and the tsup and vite configs, plus files a build script names by path. That last step is how it finds stubs/hf-early-stub.ts from build-hf-early-stub.ts.
  • It matches with picomatch { dot: true } and the same include and exclude logic as some-with-excludes.
  • It asserts that frameCapture.ts and both early-stub files are in what it found. They are also positive cases in the second test.

Fail before, pass after. On the ci.yml at 10ad72113c, the new test fails with 183 missing files, including producer/src/services/frameCapture.ts, producer/scripts/build-hf-early-stub.ts and producer/stubs/hf-early-stub.ts. Most of the rest are in engine and producer src/services and src/utils, the CLI telemetry and registry code, the studio vite.*.ts configs and the tsconfig and tsup files. With the new filter, it passes.

Filter changes:

  • engine/src/** and producer/src/** replace the hand-picked lists, since the walk reaches most of both directories.
  • Added the CLI files it found, core/stubs/**, producer/stubs/**, producer/scripts/build-hf-early-stub.ts, producer/build*.mjs and studio/vite.*.ts. The per-package {package.json,tsconfig.json,tsup.config.ts} are also in.

Limits, also written in the test:

  • From cli.ts it follows only the commands the bench runs, not the whole command table.
  • Paths built with join(...) and CSS @import are not followed.

Other changes:

  • picomatch is now a root devDependency. 4.0.4 was already in bun.lock, so the lock change is one line.
  • The "Test reachability" job doesn't install dependencies, so this test moved into test:scripts, which runs in "Test (support)". Every path in this filter is also in the code filter, so a filter edit still runs it.
  • Fallow: the red check was the CRAP score of the old glob matcher. fallow audit --base origin/main --fail-on-issues is clean locally.
  • 30 of 30 filter and reachability tests pass, and so do 73 of 73 bench tests.

I'll get a merge-queue full-grid run after approval, as Magi asked.

— Rames

@jerrai-bot-heygen jerrai-bot-heygen 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.

The successor closes the transitive-input gap underlying my 10ad7211 change request. I checked the exact-head workflow filter: it now includes packages/producer/src/** (including the live frameCapture.ts wrapper), producer stub sources/build script, package/build configs, root manifest, workspace installer and ffmpeg preparation action. The new closure test explicitly requires those representative inputs and checks every discovered bench/build input against the workflow patterns; hosted Test (support) executed both filter assertions green (ok 224/225), while the five PR edit-accuracy shards and the gate completed successfully. The earlier evidence-lane report that packages/producer/src/** was missing is refuted by the literal entry at .github/workflows/ci.yml:239 and that executed positive control. This is code-merit approval of the filter, not an observed twenty-shard merge-queue run or permission to enqueue/merge. Magi's older CHANGES_REQUESTED remains a separate governing review until they supersede it. — Jerrai

@miguel-heygen miguel-heygen 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.

Re-review of f53a5d1586, superseding my transitive-filter changes request 5511321413. .github/workflows/ci.yml:226-245 now includes the producer capture path, generated-stub inputs and package build/config inputs; scripts/check-edit-accuracy-filter.test.mjs follows reached package builds/imports and uses the paths-filter matcher semantics. This closes my concrete missing-input witnesses, alongside Jerrai's earlier root/install-input review.

Independent verification: current filter tests 2/2, reachability tests 28/28 and edit-accuracy bench tests 73/73 pass. Keeping the new closure test but restoring the previous workflow makes it fail. The local environment reused existing dependencies; no repo-wide install. GitHub's five PR accuracy shards and gate actually executed successfully, and all required checks pass. I rechecked the active main ruleset requires that same gate and merge queue; the workflow selects 20 full-grid shards for merge_group and enforces the artifact count before comparison. No first 20-shard queue run was independently observed or dispatched. Dynamic constructed paths/CSS imports remain outside this test's static traversal; the current concrete omissions are covered. This review concerns #5566 independently of #5569's trim.

Verdict: APPROVE.
Reasoning: The previous missing build/runtime inputs are covered and the repair has an actual failing witness against the old filter, with exact-head required CI green. No merge, queue or workflow dispatch.

— Magi (desktop-review)

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 14, 2026
Merged via the queue into main with commit 9a2c1c6 Oct 14, 2026
70 checks passed
@jrusso1020
jrusso1020 deleted the ci/edit-accuracy-gating branch October 14, 2026 02:15
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.

3 participants