Repository navigation
ci: add the Engine verify oracle suite for export, playback, seek, audio, recording, captions and upload - #2645
Merged
Merged
Conversation
…, per-case failure collection
A test-only scripted feature lets tests register exact sample timings under a path. The FFmpeg decoder then reads them through a scripted demuxer and the AVAssetReader decoder through a scripted reader that starts at the CMTime the real code computes. Without the feature the product build only gains a single-variant enum around the native reader.
Runs the real AVAssetReader and FFmpeg decoders on scripted media with seeded request sequences (export and playback order, double requests, timelines with backward jumps, transitions, seeks, scrubs, jumps back into old pts holes, reads past the end) and checks every answer against the newest frame at or before the request on the decoder grid, plus read-ahead and reset budgets. Known failures on main are strict per-seed expected failures.
…ess mode, main baseline
Adds AudioOutput::new_offline behind the default-off `test-hooks` feature: a sink with no device and no clock whose caller pulls each block. Sources are built by the same install_source as the device and headless sinks, and each pull waits until the samples it reads are rendered, so the PCM depends only on the play/seek/pull sequence. The video playhead sync step is factored out of render_source_block unchanged so the sink can apply it before waiting.
Reference audio model (cuts, speed modes, reorders, transitions, track and clip volume, mute, timeline music, Studio Sound) and a marker checker: every mic, system audio and music marker must be heard exactly once at the modelled time (5 ms) and level, nowhere else, in sync with the video markers. Covers MP4 through the app's and the plain export paths, MOV and GIF, offline playback through the editor's real output source pipeline, and pause/play/seek transport, where every span's PCM must equal what a fresh output produces for it (the check that catches cached replays diverging with Studio Sound).
…multi-segment Studio)
…epeats, shifts and level errors
Runs the real Studio share (multipart) and Instant live segment upload code, and the upload queue's quit/relaunch resume, against an in-process mock of the Cap upload API and object store on a paused tokio clock. The mock injects scripted faults per request (status codes, resets mid-body, network outages, slow and hung responses, cancel/quit trips) and checks the protocol: part order, etags, Content-MD5, auth, and the segment manifest behind each verification receipt. Every case asserts byte-identical stored objects, monotonic progress and the final state or error, and holds requests, media PUTs, manifest PUTs, re-uploaded bytes and orphaned multipart uploads to budgets equal to main's behaviour. Retry backoff, outages and the 5 minute request timeout and retry window cost virtual time only: the 23 cases take about 1 s of wall time. The only non-test change times the Instant progress-manifest interval with tokio::time::Instant, which is std::time::Instant outside tests.
cap-verify-expect holds the [[xfail]] format every engine suite uses (case globs, class, platforms, seeds, flaky, reason, link), the strict verdict (unlisted failures and fixed xfails both fail), the per-case JSON records written to CAP_VERIFY_REPORT_DIR, and a bless command that rewrites an expectations file from the records of one or more platforms without touching what other platforms or unrun cases expect. The decoder model fuzzer now uses it and writes records, replacing its private parser and CAP_DECODER_MODEL_BLESS.
engine-verify.yml plans which suites a change can reach through the workspace dependency graph (docs and web changes skip in seconds), runs FAST on PR pushes and FULL with the verify-full label, in the merge queue and nightly, one job per platform (macOS only for the AVFoundation suites, platform-independent logic on Linux only), with an optional nextest archive plus shards mode, a fixture cache keyed by the generator sources, a per-job summary of failing frames and repro commands, and a single Engine verify gate. scripts/verify/run.sh runs the same selection locally.
… an output oracle
…se WARP rendering tests
The FFmpeg decoder model was blessed on Linux only, so its 32 entries were scoped to Linux. The model is deterministic: Windows reproduces exactly the same classes and seeds, and macOS passes with them. cargo test -p cap-rendering runs the model on every platform (the A/V Sync Tests workflow does on Windows and macOS), so the entries apply everywhere.
The A/V Sync Tests workflow runs every cap-rendering test, which now includes the decoder model fuzzing. On a loaded hosted runner one of its sequences can miss the 2 s decode deadline. Engine verify runs the model on its own runners with its own report.
Member
Author
|
@greptileai review |
Recovered Studio recordings keep each track's start time since #2642, so recording/studio-recovery/crash no longer fails start-time, and its screen to mic sync (+4.1 ms) no longer needs the consequence entry.
FAST runs six kill points plus static-screen per arm: 7 cases on Linux and Windows, 16 on macOS (two arms, equality and muxer-kill). A retry can run cases its timed-out first attempt never reached, so its records count.
The retry rollback kept the discarded attempt's class problems, counts, measures and unobserved classes, so a class seen in both attempts counted twice. Case::checkpoint and Case::rollback cover all analysis state.
Member
Author
|
@greptileai review |
richiemcilroy
marked this pull request as ready for review
October 16, 2026 14:16
| # where they test platform code. The "Engine verify" job is the one to require. | ||
|
|
||
| on: | ||
| pull_request: |
There was a problem hiding this comment.
P2: Fork pull requests can repeatedly consume the Blacksmith runner pool with full engine verification
Fork PRs can trigger up to 9 Blacksmith FULL-tier test shards; cancellation is per PR, so separate PRs can multiply runner load.
Keep fork PR jobs on GitHub-hosted runners or enforce a global Blacksmith concurrency/quota limit.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name=".github/workflows/engine-verify.yml">
<violation number="1" location=".github/workflows/engine-verify.yml:16">
<priority>P2</priority>
<title>Fork pull requests can repeatedly consume the Blacksmith runner pool with full engine verification</title>
<evidence>The new `pull_request` trigger at `.github/workflows/engine-verify.yml:16-17` admits fork PRs; the workflow comment at lines 208-210 confirms fork PRs run with a read-only token. A PR that changes an engine crate is escalated to FULL at lines 164-178, then dispatched through the platform matrix at lines 228-245. `scripts/verify/plan.py:41-65` makes changes under `crates/rendering/**`, `crates/recording/**`, and other engine roots select that tier. The added runner matrix in `scripts/verify/suites.json:304-330` fans a FULL run out to 3 Linux, 2 macOS, and 4 Windows Blacksmith shards. These are configured as self-hosted runner labels in `.github/actionlint.yaml:1-6`. The jobs execute tests on each run (`engine-verify-os.yml:112-117`), with a 60-minute job timeout; `.config/nextest.toml:35-41` documents seek storms taking 240-570 seconds per decoder and up to about 900 seconds per shard. Restored build and fixture caches do not skip test execution. The per-PR concurrency group at `engine-verify.yml:59-65` cancels superseded runs for that PR, but distinct PR numbers use distinct groups and the checkout contains no global concurrency cap or per-actor quota. An external contributor can therefore open or update multiple fork PRs targeting engine paths to occupy additional shared Blacksmith CI capacity. This is incremental over the base: `repository-base/.github/workflows/ci.yml:251-284` ran its Linux Rust tests on `ubuntu-24.04`, and lines 286-301 used GitHub-hosted macOS, Windows, and Ubuntu runners; the base had no Engine verify workflow or Blacksmith runner matrix. The PR also moves existing CI jobs to Blacksmith, including the Linux Rust test runner (`.github/workflows/ci.yml:256`) and cross-platform build runners (`:296-301`).</evidence>
<recommendation>Keep fork-controlled PR workloads on GitHub-hosted runners and reserve Blacksmith runners for trusted branches, or apply an enforceable global concurrency/quota limit to PR jobs before dispatching the FULL matrix. Preserve the per-job timeouts and cache strategy, but do not rely on per-PR cancellation to protect the shared runner pool from multiple PRs.</recommendation>
</violation>
</file>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Engine verify: oracle regression suite for the GPUI desktop engines
Synthetic video frames carry machine-readable identities, and timed audio markers identify each source. Independent reference models define the expected frames and audio positions. The oracles decode the real output and compare it with those references. Known product bugs are strict expected failures, so a fix that lands without updating them fails CI, and a new failure fails CI.
flowchart LR F["fixtures<br/>marker video + FSK audio markers<br/>(synthetic, and real Studio / Instant<br/>pipelines recorded in real time)"] --> P["product code<br/>export, playback, seek, decoders,<br/>audio output, Instant finalize,<br/>captions, upload queue"] P --> O["oracle<br/>decode output, read markers"] M["reference model<br/>timeline, audio and decoder models"] --> O O --> R["case record (JSON line)<br/>classes: stale, future, sync,<br/>timestamps, audio-missing, ..."] R --> J{"judge against<br/>expectations.toml"} J -->|class not listed| FAIL J -->|strict class gone| XPASS["FAIL: remove the entry"] J -->|listed and seen| XFAIL[ok]What it checks
OutputPipelines andfinalize_instant_output, checked for frame timestamps and A/V sync against the emitted truth.cargo test --bin cap-gpui).Tiers and timing
scripts/verify/plan.pyverify-fulllabel, merge queue, nightly, dispatchmacos-vt20 minplan.pymaps changed files to suites through the workspace dependency graph. Docs and web-only changes run nothing; anyapps/desktop-gpui/**change runs every suite.unsupported. The long jitter-camera and seek-storm cases run on Linux and macOS.macos-vtjob repeats export and audio suites on the hardware encoder for every FULL run and changes reaching encoding paths.Expectations model
One format for every suite (
crates/verify/expectations.toml,crates/rendering/src/decoder/model/expectations.toml,crates/rendering/tests/expectations.toml):audio-micandaudio-system,sync-micandsync-system, and the real-capture source pairs. A known failure in one source cannot hide a new failure in the other.scripts/verify/bless.sh --run <id>can propose expectation updates from all platforms. Those proposals require manual review of the mechanism and bound. Every new or expanded proposal has a TODO reason that the Plan guard rejects until explained. JSONL reports carry their source host; proposals use that host and preserve unobserved host slices. Legacy reports without host provenance cannot remove or narrow existing entries, and entries covering every host remain intact.Guard warnings and escalation
scripts/verify/guards.tomllists high-risk regions (encoder timestamps, audio mixing and Studio Sound, decoder seek and frame selection, recording timestamps and sync, Instant finalize, recovery, upload retry) and guard sentinels: small tests pinned to one mechanism (E02encoder pts rebase incrates/enc-ffmpeg/src/base.rs,A09Studio Sound seek reset incrates/audio/src/voice.rs).needs-deep-reviewlabel. A failing or missing sentinel is a warning in the job summary, not a silent pass.Running it locally
Each failing case prints a one-line repro with its
CAP_VERIFY_CASE.Product bugs it found
The suite records the following product findings. Merged fixes have their obsolete expectations removed; open fixes remain subject to the final proof and baseline comparisons.
amix, so system audio landed up to 60 ms before the microphoneKnown limits
macos-vtcovering the hardware VideoToolbox encoder.Verify recordingout of the required checks for now: Studio recordings occasionally miss screen frames 3 to 6 right after the start gate (1 in about 20 runs, not the capture-service drop signature), and one Studio camera frame was lost mid-recording once in 13 runs with no machine stall.Baseline comparisons
Both baselines must run the final suite before comparison. Missing baseline APIs are N/A and do not count as passes. Required source tracks absent from a baseline recording are explicitly unobserved; newly measured classes appear in a separate coverage-gained column, while main is still judged against its own expectations. Other new failing classes and larger failure magnitudes or shares are reported as worse. A class observed on the baseline that becomes unobserved on main is reported as worse and counted in its own coverage-lost column, including when the baseline class passed. Simultaneous coverage gains do not hide losses.
The environment-dependent real-capture
missing-after-holdclass requires at least three independent runs per side on the same OS image. Main is worse only when its per-run counts are all above the baseline range; overlapping ranges and insufficient repetitions remain explicit comparison outcomes.Three FULL Engine runs per side, and three real-capture repeats per macOS version per side. Cells count cases worse / better than the baseline.
Every worse case has a cause:
export/camera-pts-late,export/jitter-camera-120s, AVF decoder-modelcamera-fast/sequential/exportandsyn/edlist/export,seek/vfr-holds/into-holds60/avf,seek/seek-native/storm30/avfediting/export/basic/seed-5,multi-segment/seed-7,same-clip-transition/forward/ffmpegseek/seek-native/storm30/ffmpegplayback/basic/transition/seq60-from0/avfProduct code touched
Test seams only, behind
cfg(test)or features that release builds never enable: the scripted decoder (crates/video-decode,scripted), an offline audio sink (crates/editor,test-hooks), synthetic camera and microphone feeds (crates/recording,test-utils), and the decoder model (cfg(test)incrates/rendering). Three behaviour-identical refactors make code reachable from tests:transcribe_project_withandcaption_segments_from_tokensinapps/desktop-gpui/src/transcription.rs,sync_video_playheadincrates/editor/src/audio_output.rs, and theReaderSourceenum incrates/video-decode/src/avassetreader.rs(one variant in release builds). The upload manifest throttle usestokio::time::Instant, which isstd::time::Instantunless a test pauses tokio's clock.

Confidence Score: 5/5
The changes since the previous review appear safe to merge.Summary
Adds engine regression tests that compare real media output with independent frame and audio references.
Reviews (5) · Last reviewed commit: "test: build the pending-fragment bound i..." · Reviewed by Greptile