fix: discard the warmup capture frame in the parallel coordinator

[mvanhorn]

What

Discards the throwaway warmup frame that each parallel worker captures before its first real seek has painted, so no fully transparent frame is emitted at a worker chunk boundary.

Why

In parallel capture, a worker's very first capture runs before the seek it issued has painted, so the first frame of every worker chunk came back fully transparent. Issue #2477 reported a transparent frame at each chunk boundary. The single-worker path already warms up and discards that first frame; the parallel coordinator was missing the same step.

How

parallelCoordinator now calls discardWarmupCapture(session) immediately after a worker session is initialized, mirroring the existing single-worker warmup. frameCapture.ts was left unchanged because its existing helper already restores the frame counters, so accounting stays correct after the discard.

Test plan

• Unit tests added/updated

Added parallelCoordinator tests covering call ordering (warmup before the first real capture), worker 0 vs non-zero workers, frame accounting after the discard, warmup failure, abort handling, and session cleanup. Targeted Vitest: 30 passed. oxfmt and oxlint are clean on the changed files.

Fixes #2477

[miguel-heygen]

Blocking finding

[P1] The warmup neither targets the failing boundary nor preserves the compositor's monotonic-time contract — packages/engine/src/services/parallelCoordinator.ts:421-428

I reproduced #2477 from the base before evaluating this patch: at e038cc93c, executeWorkerTask initializes a fresh session and immediately enters captureFrameRange; the first operation for worker 1 is therefore the seek/capture for frame 36, with no paint-settlement barrier. That matches the reported transparent frames at 36/72/108.

This change does not warm that failing seek. discardWarmupCapture(session) uses its defaults, frame 0/time 0 (frameCapture.ts:3250-3262), even when the task starts at frame 36/time 1.2. My current-head negative proof asserted the helper call for runWorker(1, 36) and received only [session], not [session, 36, 1.2]. The new test at parallelCoordinator.test.ts:63-80 mocks the helper as a no-op and checks only call order, so it passes without exercising the frame/time contract.

More critically, blindly capturing a warmup frame reinstates a known compositor deadlock. The current production path documents why discardWarmupCapture is intentionally absent: a second beginFrame at an already-used frameTimeTicks wedges Chrome (packages/producer/src/services/distributed/renderChunk.ts:579-582). Commit 24f64f02c removed the same approach after real multi-chunk validation found both same-time and backward-time deadlocks. This PR calls the helper for worker 0 at frame 0/time 0, immediately followed by the real frame 0/time 0 capture, which is exactly the same-time failure mode. Passing each worker's startFrame would still duplicate that timestamp; passing startFrame - 1 was the already-reverted backward-time failure.

Please fix the actual first-seek paint race without issuing an extra screenshot/beginFrame at a duplicate or backward compositor timestamp—for example, a seek/paint-settlement primitive separated from capture. The regression must reproduce the real boundary: render/decode the transparent multi-worker fixture, assert non-transparent output at interior starts, and prove worker 0 completes without a protocol timeout. A mocked “warmup was called” assertion cannot establish either invariant.

What is good

The failure and abort tests at parallelCoordinator.test.ts:91-115 correctly preserve session cleanup if a pre-capture stage fails. That cleanup discipline should remain in the eventual fix.

CI snapshot: core build/lint/typecheck/test, Windows render/tests, producer suites, and CodeQL are green at 82803ad95598a9e690817f5f8162ac095178f2fa; several regression shards are still running. The blocker above is deterministic and independent of those pending lanes.

Verdict: REQUEST CHANGES

Reasoning: #2477 is reproducible in the base call graph, but this patch warms frame 0 rather than the worker boundary and reintroduces a previously validated Chrome compositor deadlock pattern. It needs a monotonic-time-safe paint-settlement fix and a real boundary-render regression test.

[yahelshiran-rvr]

Two code changes that might address both review findings:

1. Pass the worker's actual boundary instead of relying on the defaults, so the failing seek is the one that gets warmed:

   await discardWarmupCapture(
   session,
   task.startFrame,
   task.startFrame * captureOptions.fps.den / captureOptions.fps.num
   );

With the current defaults (frame 0 / time 0) the warmup seeks a frame the worker never renders, so the boundary race at task.startFrame is untouched.

2. Guard the warmup to screenshot mode:

   if (session.captureMode !== "beginframe") { ... }

The deadlock documented in renderChunk.ts:579-582 (and hit by 24f64f02c) is a beginFrame timestamp contract problem: the warmup spends a compositor tick, so the real capture repeats frameTimeTicks and wedges. Screenshot capture has no compositor clock, a repeated seek to the same time is idempotent there, so the deadlock surface doesn't exist in that mode.

That would leave beginFrame mode still needing the seek/paint-settlement primitive the review asks for, but a mode-guarded, startFrame-targeted warmup could fix the screenshot path (where transparent overlay renders live) without reintroducing the reverted failure modes.

WDYT? @mvanhorn

[miguel-heygen]

Re-review after taking over the fix at exact head 121a02e0ec9aa69c57e1fbf5e14759ef0ad8cf46.

The original unconditional warmup was unsafe because BeginFrame/drawElement capture cannot be repeated at the same compositor timestamp without risking the deadlock previously removed in 24f64f02c. The revised implementation confines paint settlement to the affected path: fresh non-zero screenshot workers only, at the exact first worker frame/time. Worker 0 and both compositor-driven capture modes are explicitly excluded, and the helper rejects non-screenshot sessions defensively. Cleanup on warmup failure/abort remains covered.

Evidence:

• TDD pinned exact frame/time, worker-0 exclusion, BeginFrame/drawElement exclusion, cleanup, and abort behavior; focused suite: 40/40.
• Full @hyperframes/engine suite with repo-compatible static FFmpeg: 44 files passed (1 skipped), 1008 tests passed (3 skipped).
• Engine typecheck, build, oxfmt, oxlint, fallow, and commit hooks passed.
• Real four-worker transparent WebM regression render passed 60/60 frames; alpha-plane samples remained nonzero across every worker boundary.
• Fresh CI at this exact head is fully green, including Linux tests, both Windows render/test lanes, CodeQL, Preview parity, and all eight producer regression shards.

Verdict: APPROVE

Reasoning: The patch now fixes the screenshot-worker paint race at its actual boundary without reintroducing the duplicate-timestamp compositor deadlock, and tests plus complete current-head CI cover the relevant modes and multi-worker transparency path.

— Deepwork

Exact reporter failure was not reproduced; approval was premature. Reopening verification gate until RED→GREEN evidence exists.

[miguel-heygen]

Verification correction at head 121a02e0ec9aa69c57e1fbf5e14759ef0ad8cf46.

The revised patch is internally safe against the known duplicate-BeginFrame deadlock and passes synthetic multi-worker transparency coverage, but I did not reproduce the reporter's exact #2477 failure before the change. Therefore I cannot distinguish with sufficient evidence between (a) the first-paint race this patch addresses and (b) a second composition/environment-specific cause. I dismissed my approval; this remains a candidate fix pending a reporter artifact or equivalent deterministic RED→GREEN reproduction.

Verdict: COMMENT

Reasoning: Safety and regression evidence are strong, but root-cause closure is not proven without reproducing the reported blank-boundary failure and demonstrating that this change removes it.

— Deepwork