test(producer): add webm-vp9 distributed regression fixture#952
Conversation
miguel-heygen
left a comment
There was a problem hiding this comment.
Review: test(producer): add webm-vp9 distributed regression fixture
Summary
PR 3 of 4 in the WebM distributed-rendering stack. Adds an end-to-end regression fixture for VP9/webm in distributed mode, plus the type/validation wiring across aws-lambda and producer harness code to let webm flow through the distributed pipeline.
12 files changed (+338/-35): well-scoped for what it does.
What's good
-
Fixture design is thoughtful. The composition intentionally exercises chunk-boundary continuity: crossfade straddling the frame-30 seam (alpha-plane test), continuous icon rotation (virtual clock regression detector),
chunkSize: 15producing 4 chunks / 3 seams. The HTML comment explaining why audio is omitted (Opus 20ms grain padding extending container duration) shows real codec-level understanding. -
Follows existing patterns exactly. Directory structure (
meta.json,output/,src/), meta.json shape, GSAP timeline registration viawindow.__timelines, and the PSNR threshold (30 dB) all mirror themp4-h264-sdrfixture. Thewebmformat field andchunkSizeare the only differences. -
Baseline generated via Docker (
bun run --cwd packages/producer docker:test:update webm-vp9) per CLAUDE.md's requirement.output.webmis properly tracked via Git LFS. -
Type widening is consistent.
"webm"added toFormatunions inevents.ts(3 interfaces),DistributedFormatinformatExtension.ts,downloadChunkObjectsparam inhandler.ts,ALLOWED_FORMATSinvalidateConfig.ts,RunDistributedSimulatedInput.format, andRunLambdaLocalInput.format. TheformatExtensionswitch gains the.webmcase. No gaps in the type surface. -
Harness cleanup is clean. The old
as "mp4" | "mov" | "png-sequence"cast inregression-harness.tsis replaced with a direct pass-through ofoutputFormatnow that the type naturally includes webm. Comments updated throughout to remove "webm refused" language. -
Test flip is correct.
regression-harness-distributed.test.tsflips the old "rejects format=webm" test to "accepts format=webm" with the right assertion. -
PSNR results are strong. 56.88-63.49 dB across 100 checkpoints, well above the 30 dB threshold. Indicates the closed-GOP concat-copy strategy produces near-identical output to single-pass.
One nit
Stale file-header comment in validateConfig.ts — The docblock at lines 13-18 still says:
plus the documented
webm/force-hdrrejections from §5.3 of the distributed-rendering plan
Now that webm is allowed through ALLOWED_FORMATS, this should read something like plus the documented force-hdr rejection (or just drop the webm mention). Low priority since it's a comment, but it'll confuse anyone reading the validation module's purpose.
CI status
All green. 8 regression shards passed, preflight (lint + format) passed, no failures. Graphite mergeability check is pending (expected for a stacked PR).
Verdict
Clean, well-structured test PR. The fixture is purposeful (not just a token test), the type wiring is complete, and the baseline was generated correctly via Docker. The one stale comment is trivial.
Approve -- the stale validateConfig.ts comment can be fixed in a follow-up or folded into #953.
miguel-heygen
left a comment
There was a problem hiding this comment.
APPROVE — clean regression fixture with purposeful design.
Verified:
- 2s/60-frame composition with crossfade straddling chunk boundary + continuous rotation — designed to catch VP9 chunk-seam discontinuities
- Follows existing
mp4-h264-sdrfixture patterns exactly (directory structure, meta.json, GSAP timeline) - Baseline generated via Docker per CLAUDE.md requirements, tracked via Git LFS
- Type widening complete across all interfaces (RenderChunkEvent, AssembleEvent, PlanLambdaResult, DistributedFormat, etc.)
formatExtensionswitch gains.webmcase — no missing branch- PSNR results: 56.88-63.49 dB across 100 checkpoints, well above 30 dB threshold
- Test correctly flips from "rejects webm" to "accepts webm"
Nit: file-header comment in validateConfig.ts still references "webm rejections" but webm is no longer rejected. Trivial follow-up.
All CI green.
vanceingalls
left a comment
There was a problem hiding this comment.
Additive review — @miguel-heygen already approved with coverage of fixture design, type-widening completeness, baseline LFS pinning, PSNR headroom, and the validateConfig.ts:13-18 stale-comment nit. Surfacing three gaps Magi didn't catch; none are blockers, but worth folding into the same diff before merge.
Important
.github/workflows/regression.yml:62-77—webm-vp9is not in any shard'sargs. Sibling distributed fixtures are all wired in (mp4-h264-sdrshard-3,mp4-h265-sdrshard-4,mov-proresshard-1,png-sequenceshard-6). The harness positional args atpackages/producer/src/regression-harness.ts:409are an allow-list —if (filterNames.length > 0 && !filterNames.includes(id)) return;— so the fixture lives in the tree but never runs in CI in any mode. The PR title is "add … regression fixture" but no CI regression coverage materializes; onlybun run docker:test webm-vp9(local explicit invocation) exercises it. Fix is a single-line YAML add (appendwebm-vp9to shard-3's args, which already carries the matching in-processwebm-transparencyandmp4-h264-sdrworkloads). Note that distributed-simulated mode isn't run in CI for any fixture today, so even with shard wiring the in-process-mode gate is what materializes here — but sibling-consistency still warrants the line.- Stacked-PR CI gap.
baseRefNameisfeat/producer-enable-webm-distributed, notmain..github/workflows/ci.yml:8-16filterspull_request: branches: [main], so theCIworkflow (Build / Lint / Test / Typecheck) didn't fire on this head — onlyregression,Player perf,preview-regressionran. The PR-body claims (bunx tsc --noEmit … clean,bun test … 18/18,bunx oxlint + oxfmt --check — clean) are local-only; the cross-package type widening across producer + aws-lambda would have been caught by tsc + vitest if they'd run on this head, but they didn't. Thetests/distributed/_smoke/webm-concat-copy.test.tssmoke (from #951) is the real gate on the VP9 concat-copy contract, and it didn't run here either. Land #951 first so this PR'spull_requestevents re-evaluate againstmainand the gate actually fires.
Nit
packages/producer/src/regression-harness-distributed.ts:70doc comment opens with"The four hard gates:"but only enumerates three gates after this PR drops the webm bullet (fps.den=1, fps.num ∈ {24,30,60}, hdr). Trivial sibling to Magi'svalidateConfig.tsfinding — same "removed webm bullet, didn't update prose around it" class.
Note for the stack (not a finding here)
- The CLI rejection at
packages/cli/src/commands/lambda.ts:328+packages/cli/src/commands/lambda/render.ts:26and the stale user-facing doc claims atdocs/deploy/migrating-to-hyperframes-lambda.mdx:45, 67-69, 81("Distributed mode refuses webm + HDR at plan time", "Lambda handler refuses webm with FORMAT_NOT_SUPPORTED_IN_DISTRIBUTED") are both explicitly resolved in #953. Surfacing only because the stack ships incoherent user-facing surface area in any merge order that lands #951 + #952 without #953 — keep them landing together.
Verdict: APPROVE
Reasoning: Magi covered the substantive code-review surface; the gaps I'm surfacing are CI-wiring and a parallel doc-staleness nit, all single-line fixes that don't change the fixture or the type wiring.
— Vai
5726d1e to
df2a67e
Compare
fd43c14 to
c44b130
Compare
|
Thanks Miguel — the stale docblock nit is fixed in the follow-up commit |
|
Thanks Vai — all three folded in:
Also surfaced a tangentially-related fallow-audit finding while committing: |
vanceingalls
left a comment
There was a problem hiding this comment.
Re-review at d8190d7e (was 5726d1ed on prior APPROVE id 4315967291).
Status of prior findings
- Shard wiring — RESOLVED.
.github/workflows/regression.yml:67shard-3argsnow reads"style-7-prod style-8-prod style-10-prod css-spinner-render-compat webm-transparency mp4-h264-sdr webm-vp9". Thewebm-vp9fixture is now in the harness allow-list (packages/producer/src/regression-harness.ts:409) for at least one shard's CI invocation; sibling-consistency withwebm-transparency/mp4-h264-sdrin the same shard restored. - Stacked-PR CI gap — partial / structurally unresolved at this head.
baseRefNameis stillfeat/producer-enable-webm-distributed, notmain..github/workflows/ci.yml:7-15still haspull_request: branches: [main]. The diff does addtypes: [opened, synchronize, reopened, edited]with a comment explaining the rationale ("required so the workflow re-fires when a PR's base ref is set back tomainafter a Graphite stack restack"). That's a forward-looking fix for the restack scenario but it does not retroactively run Build / Lint / Test / Typecheck on this head while it's still stacked. Verbatim CI rollup atd8190d7e: passed = Detect changes (3 jobs), Preflight (1 of 3 ran, 2 skipped), player-perf, preview-regression, WIP; pending = 8 regression-shards in progress + Graphite mergeability; failed = none. The CI workflow (Build / Lint / Test / Typecheck) is again absent from the rollup. Land #951 first (or rebase onto main once parent merges) so the cross-package type widening + distributed-smoke gate from #951 actually fires on this head before merge. - Nit (doc gates count) — RESOLVED.
packages/producer/src/regression-harness-distributed.ts:70now reads "Two hard gates:" with the matching two bullets (fps integer-only, hdr SDR-only) after the webm bullet drop. Sibling nit consistency with Magi'svalidateConfig.ts:13-18finding addressed.
Verdict: APPROVE (re-affirming).
Reasoning: Two of three findings closed cleanly. The stacked-CI gap is structurally unresolved at this head but the edited trigger addition shows intent to handle the broader restack pattern; merge discipline (land #951 first, then rebase) is the real closure here, not a code change to this PR. No new blockers surfaced in the rebase delta.
Review by Vai (re-review)
d8190d7 to
94b2a6c
Compare
|
(Self-correction on the earlier reply: I initially used |
fcfd330 to
f8ffe9f
Compare
PR #952 review nit (Miguel): the validateConfig.ts file-header comment still claimed the SDK rejects webm, but the runtime check no longer does (ALLOWED_FORMATS now includes 'webm'). Update the docblock to reflect that only force-hdr remains an SDK-side rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three follow-ups bundled together (Vai's review feedback on PR #952 plus the fallow audit finding that surfaced when the webm case was added): 1. **Wire webm-vp9 into CI regression.** The fixture was added in this PR but never appeared in any `.github/workflows/regression.yml` shard's args allowlist, so the regression harness's positional-args gate skipped it in CI. Append `webm-vp9` to shard-3 (which already carries `mp4-h264-sdr` + `webm-transparency`) so the fixture runs. 2. **Fix stale "four hard gates" prose in checkDistributedSupport docstring.** Earlier in the stack I removed the webm bullet but didn't update the count. Two gates remain (fps + hdr). 3. **Refactor `formatExtension` from switch to lookup table.** Adding the webm case made the switch dispatch's CRAP score hit 30.0 (cyclomatic = 5, plus the function's small body). Replaced with a `Record<DistributedFormat, string>` lookup, which: - drops cyclomatic from 5 → 1, - keeps exhaustiveness enforcement at compile time (TS errors if a new format gets added to `DistributedFormat` without a matching key in the Record literal), - drops the runtime `_exhaustive: never` throw, which was only guarding against an arbitrary string slipping past TS — a caller-side concern, not this function's job. The function now reads as a table lookup, which matches what it actually does, and the fallow audit now reports zero new complexity findings (down from 1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
94b2a6c to
ced2454
Compare
vanceingalls
left a comment
There was a problem hiding this comment.
Round-3 re-review at HEAD ced24546.
Delta since round 2 (d8190d7):
aws-lambda/src/sdk/validateConfig.ts— docblock now reflects that onlyforce-hdris SDK-rejected. Miguel's nit.aws-lambda/src/formatExtension.ts— switch→Record<DistributedFormat, string>refactor. Exhaustiveness preserved at compile time via the closed mapped-type annotation: adding a newDistributedFormatvariant without a matching key fails to typecheck.noUncheckedIndexedAccessdoesn't degrade this —Record<K_literal_union, V>indexes asV, notV | undefined. The dropped runtimeneverarm was guarding against type-bypassed callers, fair to push that concern back to the boundary.
Round-2 fixes verified at new HEAD:
.github/workflows/regression.yml:67—webm-vp9present in shard-3args. ✓regression-harness-distributed.ts:70— "Two hard gates" reads correctly. ✓
No new regressions. Stacked-PR CI gap still structural (closes when the stack lands; not blocking).
LGTM, ship it.
Review by Vai (re-review round 3)
PR 8.2 of the WebM distributed-rendering plan (v1.5 backlog #1; see DISTRIBUTED-RENDERING-PLAN.md §7.2). Wires libvpx-vp9 webm through the distributed pipeline now that PR 8.1 proved concat-copy works. Architectural decision: Path A (concat-copy) — based on PR 8.1's smoke test result (9/9 tests pass for both yuv420p and yuva420p VP9 streams). The simpler architecture wins; no re-encode in assemble, no encode- parallelism loss. Changes: - plan.ts: - DistributedRenderConfig.format and PlanResult.format now include "webm" — type-level acceptance matches the runtime gate. - rejectUnsupportedDistributedFormat() no longer trips on webm. HDR mp4 remains the only refused configuration. - resolveEncoderTriple() returns libvpx-vp9-software + yuva420p + preset="good" for format="webm". yuva420p preserves alpha — the format's main reason for existing for web delivery. - codec= remains rejected for non-mp4 formats (mov is always ProRes 4444; webm is always libvpx-vp9). The error message lists all four distributed-supported formats. - FormatNotSupportedInDistributedError docstring updated to reflect the new reality (only HDR is unsupported). - freezePlan.ts: LockedRenderConfig.encoder gains "libvpx-vp9-software". Mirrors libx265-software / prores-software / png-sequence in shape; the chunk worker reads this discriminant to decide encode args. - renderChunk.ts: drops the now-incorrect cast that excluded webm from buildSyntheticRenderJob's format input; tightens the preset-format cast to include webm. - assemble.ts: docstring + comment updates. The mp4/mov concat-copy path is format-agnostic — webm uses the exact same code (applyFaststart is a no-op for webm via the existing chunkEncoder.ts gate; muxVideoWithAudio already routes webm to libopus audio). - planFormatBanlist.test.ts: webm-rejection tests removed; replaced with "accepts webm" tests + a HDR+webm combo test that verifies HDR is the trip regardless of format. - plan.test.ts: new describe block pins the webm wiring contract: format="webm" produces an encoder=libvpx-vp9-software / pixelFormat=yuva420p planDir with closedGop=true and gopSize=chunkSize. - webm-concat-copy.test.ts (smoke): extended with a yuva420p variant that proves the alpha pixel format the distributed pipeline actually emits also round-trips through concat-copy. 9/9 tests pass locally. §8 format support matrix in DISTRIBUTED-RENDERING-PLAN.md is intentionally left unchanged at this PR — it flips to ✓ in PR 8.4 once the end-to-end fixture (PR 8.3) is green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…a smoke PR review feedback from Miguel and Vai on #951 caught a real bug: `plan.ts`'s `needsAlpha` disjunction excluded `"webm"`, so the plan stage froze `forceScreenshot: false` into the `LockedRenderConfig` even though distributed webm uses `yuva420p`. Every chunk worker captured opaque RGB via BeginFrame (which doesn't preserve alpha on Linux headless-shell), and libvpx-vp9 encoded uniformly-opaque alpha that the encoder then dropped — producing un-keyable webm. Two changes: 1. **plan.ts**: include `"webm"` in `needsAlpha`. Matches the in-process renderer's logic at `renderOrchestrator.ts:1469` (`const needsAlpha = isWebm || isMov || isPngSequence`); the two sites must stay in sync since the distributed pipeline's PSNR regression compares against the in-process baseline. 2. **Smoke test (yuva420p describe)**: source frames now use a real alpha gradient (`geq=a='X*255/W'` on top of `testsrc2`) instead of `testsrc2 + format=rgba` which was uniformly opaque. The decode- pix_fmt assertion is dropped (ffprobe reports `yuv420p` for VP9-with-alpha because the alpha lives in a Matroska `BlockAdditional` sidecar) and replaced with two stronger checks: - `TAG:ALPHA_MODE=1` is present on the stream — proves the encoder was actually configured for alpha - alpha plane variance after `-c:v libvpx-vp9 -i ... -pix_fmt rgba -vf extractplanes=a,signalstats` — proves the alpha sub-stream round-trips through concat-copy with spatially-varying content, not uniform/dropped alpha - decode-test gate is now exit-code-only (was `exitCode || stderr` which would flake on chatty ffmpeg `-v error` builds emitting non-fatal DTS/container notes) These checks would have caught the `needsAlpha` bug before review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
f8ffe9f to
4aecc36
Compare
ced2454 to
1795568
Compare
CI on PR #951 was failing at typecheck/build because the producer's `DistributedRenderConfig.format` widened to include webm in this PR but the aws-lambda package's narrow `"mp4" | "mov" | "png-sequence"` type literals in `events.ts`, `handler.ts`, and `validateConfig.ts` hadn't kept up. `renderToLambda.ts:87` passed `config.format` (now including webm) into a parameter typed against the narrow union, producing TS2345. This widening originally landed in PR #952 (test fixture PR) but needs to be atomic with the producer's widening here to keep each PR independently typecheck-clean. Also refactor `formatExtension` from a switch dispatch to a `Record<DistributedFormat, string>` lookup. Adding the webm case tipped the switch's CRAP to the 30.0 fallow threshold; the lookup table drops cyclomatic from 5 to 1 with the same compile-time exhaustiveness guarantee (TS errors on missing entries when `DistributedFormat` adds a new format). The runtime `_exhaustive: never` throw was only protecting against a string slipping past TS; `validateConfig.ts`'s `ALLOWED_FORMATS` already gates untrusted input at the SDK boundary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR 8.3 of the WebM distributed-rendering plan (v1.5 backlog #1; see DISTRIBUTED-RENDERING-PLAN.md §7.2). End-to-end regression coverage for the webm distributed path PRs 8.1 and 8.2 wired up. Adds packages/producer/tests/distributed/webm-vp9/ matching the mp4-h264-sdr fixture pattern: a 2-second composition (60 frames @ 30fps) with text, a crossfade across the frame-30 chunk seam, and a continuous icon rotation — exercises chunk-boundary continuity for both display contents and VP9 closed-GOP alpha encoding. `chunkSize: 15` produces 4 chunks so 3 seams are tested, and the crossfade straddles the middle seam to surface alpha-plane discontinuities introduced by alt-ref drift. Baseline regenerated inside Dockerfile.test via `bun run --cwd packages/producer docker:test:update webm-vp9`. Runs in: - in-process mode: byte-identical match against baseline ✓ - distributed-simulated mode: PSNR 56.88-63.49 dB across 100 checkpoints, well above the 30 dB threshold ✓ Wiring updates required to let webm flow through the harness: - regression-harness-distributed.ts: - checkDistributedSupport() no longer rejects webm. HDR mp4 + NTSC fps + non-{24,30,60} fps remain rejected. - RunDistributedSimulatedInput.format widened to include webm. - Docstring + comments updated. - regression-harness-distributed.test.ts: webm-rejection test replaced with "accepts format=webm" test. - regression-harness.ts: the now-incorrect format cast at the distributed-input call site is dropped; comment about why webm was excluded is replaced with "webm is now distributed-supported". - regression-harness-lambda-local-types.ts: RunLambdaLocalInput.format widened to include webm so lambda-local mode can also exercise webm fixtures end-to-end. - aws-lambda webm support (Path A through the Lambda handler): - formatExtension.ts: DistributedFormat gains "webm" → ".webm" case. - events.ts: RenderChunkEvent / AssembleEvent / PlanLambdaResult Format widened to include webm. - sdk/validateConfig.ts: ALLOWED_FORMATS gains "webm". - handler.ts: downloadChunkObjects format param widened. The Lambda handler delegates to the producer's assemble() primitive which PR 8.2 already taught to handle webm (concat-copy + applyFaststart no-op + muxVideoWithAudio with libopus); no Lambda-side rendering changes are needed beyond the type/validation surfaces above. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #952 review nit (Miguel): the validateConfig.ts file-header comment still claimed the SDK rejects webm, but the runtime check no longer does (ALLOWED_FORMATS now includes 'webm'). Update the docblock to reflect that only force-hdr remains an SDK-side rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three follow-ups bundled together (Vai's review feedback on PR #952 plus the fallow audit finding that surfaced when the webm case was added): 1. **Wire webm-vp9 into CI regression.** The fixture was added in this PR but never appeared in any `.github/workflows/regression.yml` shard's args allowlist, so the regression harness's positional-args gate skipped it in CI. Append `webm-vp9` to shard-3 (which already carries `mp4-h264-sdr` + `webm-transparency`) so the fixture runs. 2. **Fix stale "four hard gates" prose in checkDistributedSupport docstring.** Earlier in the stack I removed the webm bullet but didn't update the count. Two gates remain (fps + hdr). 3. **Refactor `formatExtension` from switch to lookup table.** Adding the webm case made the switch dispatch's CRAP score hit 30.0 (cyclomatic = 5, plus the function's small body). Replaced with a `Record<DistributedFormat, string>` lookup, which: - drops cyclomatic from 5 → 1, - keeps exhaustiveness enforcement at compile time (TS errors if a new format gets added to `DistributedFormat` without a matching key in the Record literal), - drops the runtime `_exhaustive: never` throw, which was only guarding against an arbitrary string slipping past TS — a caller-side concern, not this function's job. The function now reads as a table lookup, which matches what it actually does, and the fallow audit now reports zero new complexity findings (down from 1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1795568 to
6f6dc30
Compare
4aecc36 to
7f8a332
Compare
vanceingalls
left a comment
There was a problem hiding this comment.
Re-stamp at 6f6dc307 — restacked. Prior approve (ced2454) still applies content-wise; this is the post-restack head. All round-2/3 findings remain ADDRESSED (shard-3 wiring, gates docstring, formatExtension Record refactor).
Re-review by Vai (post-restack re-stamp)
* feat(producer): enable webm in distributed mode via concat-copy PR 8.2 of the WebM distributed-rendering plan (v1.5 backlog #1; see DISTRIBUTED-RENDERING-PLAN.md §7.2). Wires libvpx-vp9 webm through the distributed pipeline now that PR 8.1 proved concat-copy works. Architectural decision: Path A (concat-copy) — based on PR 8.1's smoke test result (9/9 tests pass for both yuv420p and yuva420p VP9 streams). The simpler architecture wins; no re-encode in assemble, no encode- parallelism loss. Changes: - plan.ts: - DistributedRenderConfig.format and PlanResult.format now include "webm" — type-level acceptance matches the runtime gate. - rejectUnsupportedDistributedFormat() no longer trips on webm. HDR mp4 remains the only refused configuration. - resolveEncoderTriple() returns libvpx-vp9-software + yuva420p + preset="good" for format="webm". yuva420p preserves alpha — the format's main reason for existing for web delivery. - codec= remains rejected for non-mp4 formats (mov is always ProRes 4444; webm is always libvpx-vp9). The error message lists all four distributed-supported formats. - FormatNotSupportedInDistributedError docstring updated to reflect the new reality (only HDR is unsupported). - freezePlan.ts: LockedRenderConfig.encoder gains "libvpx-vp9-software". Mirrors libx265-software / prores-software / png-sequence in shape; the chunk worker reads this discriminant to decide encode args. - renderChunk.ts: drops the now-incorrect cast that excluded webm from buildSyntheticRenderJob's format input; tightens the preset-format cast to include webm. - assemble.ts: docstring + comment updates. The mp4/mov concat-copy path is format-agnostic — webm uses the exact same code (applyFaststart is a no-op for webm via the existing chunkEncoder.ts gate; muxVideoWithAudio already routes webm to libopus audio). - planFormatBanlist.test.ts: webm-rejection tests removed; replaced with "accepts webm" tests + a HDR+webm combo test that verifies HDR is the trip regardless of format. - plan.test.ts: new describe block pins the webm wiring contract: format="webm" produces an encoder=libvpx-vp9-software / pixelFormat=yuva420p planDir with closedGop=true and gopSize=chunkSize. - webm-concat-copy.test.ts (smoke): extended with a yuva420p variant that proves the alpha pixel format the distributed pipeline actually emits also round-trips through concat-copy. 9/9 tests pass locally. §8 format support matrix in DISTRIBUTED-RENDERING-PLAN.md is intentionally left unchanged at this PR — it flips to ✓ in PR 8.4 once the end-to-end fixture (PR 8.3) is green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(producer): include webm in plan-time needsAlpha + strengthen alpha smoke PR review feedback from Miguel and Vai on #951 caught a real bug: `plan.ts`'s `needsAlpha` disjunction excluded `"webm"`, so the plan stage froze `forceScreenshot: false` into the `LockedRenderConfig` even though distributed webm uses `yuva420p`. Every chunk worker captured opaque RGB via BeginFrame (which doesn't preserve alpha on Linux headless-shell), and libvpx-vp9 encoded uniformly-opaque alpha that the encoder then dropped — producing un-keyable webm. Two changes: 1. **plan.ts**: include `"webm"` in `needsAlpha`. Matches the in-process renderer's logic at `renderOrchestrator.ts:1469` (`const needsAlpha = isWebm || isMov || isPngSequence`); the two sites must stay in sync since the distributed pipeline's PSNR regression compares against the in-process baseline. 2. **Smoke test (yuva420p describe)**: source frames now use a real alpha gradient (`geq=a='X*255/W'` on top of `testsrc2`) instead of `testsrc2 + format=rgba` which was uniformly opaque. The decode- pix_fmt assertion is dropped (ffprobe reports `yuv420p` for VP9-with-alpha because the alpha lives in a Matroska `BlockAdditional` sidecar) and replaced with two stronger checks: - `TAG:ALPHA_MODE=1` is present on the stream — proves the encoder was actually configured for alpha - alpha plane variance after `-c:v libvpx-vp9 -i ... -pix_fmt rgba -vf extractplanes=a,signalstats` — proves the alpha sub-stream round-trips through concat-copy with spatially-varying content, not uniform/dropped alpha - decode-test gate is now exit-code-only (was `exitCode || stderr` which would flake on chatty ffmpeg `-v error` builds emitting non-fatal DTS/container notes) These checks would have caught the `needsAlpha` bug before review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(aws-lambda): widen narrow format types to include webm CI on PR #951 was failing at typecheck/build because the producer's `DistributedRenderConfig.format` widened to include webm in this PR but the aws-lambda package's narrow `"mp4" | "mov" | "png-sequence"` type literals in `events.ts`, `handler.ts`, and `validateConfig.ts` hadn't kept up. `renderToLambda.ts:87` passed `config.format` (now including webm) into a parameter typed against the narrow union, producing TS2345. This widening originally landed in PR #952 (test fixture PR) but needs to be atomic with the producer's widening here to keep each PR independently typecheck-clean. Also refactor `formatExtension` from a switch dispatch to a `Record<DistributedFormat, string>` lookup. Adding the webm case tipped the switch's CRAP to the 30.0 fallow threshold; the lookup table drops cyclomatic from 5 to 1 with the same compile-time exhaustiveness guarantee (TS errors on missing entries when `DistributedFormat` adds a new format). The runtime `_exhaustive: never` throw was only protecting against a string slipping past TS; `validateConfig.ts`'s `ALLOWED_FORMATS` already gates untrusted input at the SDK boundary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The base branch was changed.
vanceingalls
left a comment
There was a problem hiding this comment.
Re-stamp at 6f6dc307 after base-ref auto-restack to main. No content changes since ced24546; the round-3 review's verdict + findings still apply (shard-3 wiring + Two hard gates docstring + formatExtension Record refactor all in place).
Re-stamp by Vai (post-base-restack)

Description
PR 3 of 4 in the WebM distributed-rendering series. End-to-end
regression coverage for the webm distributed path PRs #950 and #951
wired up.
Adds
packages/producer/tests/distributed/webm-vp9/matching themp4-h264-sdrfixture pattern: a 2-second composition (60 frames @30fps) with text, a crossfade across the frame-30 chunk seam, and a
continuous icon rotation.
chunkSize: 15produces 4 chunks so 3 seamsare tested, and the crossfade straddles the middle seam to surface
alpha-plane discontinuities introduced by alt-ref drift.
Baseline regenerated inside
Dockerfile.testviabun run --cwd packages/producer docker:test:update webm-vp9. Runs in:checkpoints, well above the 30 dB threshold ✓
Wiring updates required to let webm flow through the harness:
regression-harness-distributed.ts:checkDistributedSupport()nolonger rejects webm. HDR mp4 + NTSC fps + non-{24,30,60} fps remain
rejected.
RunDistributedSimulatedInput.formatwidened to includewebm.
regression-harness-distributed.test.ts: webm-rejection testreplaced with "accepts format=webm" test.
regression-harness.ts: drops the now-incorrect format cast atthe distributed-input call site.
regression-harness-lambda-local-types.ts:RunLambdaLocalInput.formatwidened so lambda-local mode can alsoexercise webm fixtures.
aws-lambda webm support:
formatExtension.ts:DistributedFormatgains"webm".events.ts:RenderChunkEvent/AssembleEvent/PlanLambdaResultFormatwidened.sdk/validateConfig.ts:ALLOWED_FORMATSgains"webm".handler.ts:downloadChunkObjectsformat param widened.Fixup commit
docs(aws-lambda): drop stale webm rejection from validateConfig docblockReview nit from Miguel: the
validateConfig.tsfile-header docblockstill claimed the SDK rejects webm. Updated to reflect that only
force-hdrremains an SDK-side rejection. Also scrubbed the docblock'sinternal planning-doc section reference at the same time.
Testing
bun run --cwd packages/producer docker:test webm-vp9(in-process) — passesbun run --cwd packages/producer docker:test -- --mode=distributed-simulated webm-vp9— passes (PSNR 56.88–63.49 dB)bun test packages/producer/src/regression-harness-distributed.test.ts— passesbunx tsc --noEmiton producer + aws-lambda — cleanbunx oxlint+bunx oxfmt --check— clean🤖 Generated with Claude Code