feat(engine): preflight psnr filter availability and force-fallback to screenshot on missing - #3418
Conversation
…o screenshot on missing ## What Adds a one-shot ffmpeg-psnr filter probe at drawElement session bootstrap. When the resident ffmpeg is missing or lacks libpostproc (no `psnr` filter), the capture-session router now force-fallbacks to the screenshot capture path and emits a `de_gate_reason = "ffmpeg_no_psnr_filter"` telemetry signal via the existing `render_complete` breakdown. Also tightens `psnrForDiskSample`'s catch: infrastructure-class ffmpeg failures (ENOENT, "No such filter") no longer silently skip the sample — they abort the render so the safety net cannot fail-open post-preflight. ## Why The drawElement self-verify safety net (parallelCoordinator's `psnrForDiskSample` → `psnrDb`) shells to `ffmpeg -lavfi psnr`. If ffmpeg is missing, or was compiled without libpostproc (so the `psnr` filter is absent), every per-sample compare throws. The existing catch swallows the error and returns `null` — callers treat that as "skip this sample" and the render completes with the safety net inoperative. Field signal (⭐ 9/10 CLI feedback, Slack ts=1787380767.210079, hyperframes 0.8.7, darwin/arm64, tid=93ff9910-2207-45c2-bc1f-54c0b347d4fe): > "host ffmpeg lacked psnr filter used by drawElement self-verification, > but render completed." The user's frames happened to be byte-identical so no visual damage shipped — but the safety net silently wasn't running. Any future compositor-damage bug on that host would have shipped straight through. ## How Two-part fix, both in `packages/engine`: 1. New `utils/psnrFilterAvailability.ts` — cached probe that runs `ffmpeg -hide_banner -filters` once per process and word-boundary- matches `psnr` in the output. Any failure (ENOENT, non-zero exit, timeout, unparseable output) returns `false`; never rejects. 2. Wired into `services/frameCapture.ts` `initDrawElementOrTransparentBackground` right after the Chrome capability probe: when useDrawElement resolves true and the preflight returns false, set `session.deGateReason = "ffmpeg_no_psnr_filter"` (same low-cardinality bucket every other DE gate uses; flows through `getCapturePerfSummary` → `render_complete.de_gate_reason` in PostHog), emit a stderr warning naming what's missing, and call `routeToFallback()` — the same fail-graceful shape as the SwiftShader / CSS-effect / at-risk-timeline gates. Skipped under `HF_FORCE_DRAWELEMENT=1` (matches the diagnostic knob's policy of bypassing every other gate). Belt-and-braces: `psnrForDiskSample` now discriminates infrastructure- class failures (ENOENT / "No such filter" / "Unknown filter") from per-sample noise (readFile races, transient EPERM). Only the former re-throw — per-sample noise still returns `null` (skipped sample). The preflight normally catches this at bootstrap; the re-throw covers ffmpeg-swapped-mid-render. ## Test plan - [x] Unit tests added: `packages/engine/src/utils/psnrFilterAvailability.test.ts` — mocked `execFile` covers: `psnr` present → true; `psnr` absent → false; ENOENT → false; non-zero exit → false; result memoized + reset works; substring-not-word-boundary → false. - [x] Unit tests added: `isFfmpegInfrastructureFailure` in `packages/engine/src/services/parallelCoordinator.test.ts` covers ENOENT, "No such filter", "Unknown filter", per-sample EACCES, parse errors, null/non-object. - [x] `bun run test` — `packages/engine/src/utils/psnrFilterAvailability.test.ts` (6 tests) + `packages/engine/src/services/parallelCoordinator.test.ts` (50 tests) + `frameCapture.test.ts` (26 tests) all pass. Pre-existing ffprobe test failures (4) on the base commit are unrelated (missing PNG fixture bytes — the file is 129 B on disk, likely LFS-stored). - [x] `bunx tsc --noEmit -p packages/engine/tsconfig.json` — clean. - [x] `bunx oxlint <files>` — 0 warnings, 0 errors. - [x] `bunx oxfmt --check <files>` — clean. Not covered here: an integration test that boots `initDrawElementOrTransparentBackground` end-to-end. That path is Puppeteer-driven and has no unit-scale bootstrap harness in the repository — the pure preflight + pure discriminator coverage above are what this PR can prove at the vitest layer. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
miga-heygen
left a comment
There was a problem hiding this comment.
Review
Approve. Independent read at current head.
Preflight probe: isPsnrFilterAvailable is a clean one-shot cached probe — process-lifetime memoization is correct (ffmpeg doesn't change mid-CLI-invocation), and the probe() catch-all converts every failure shape to false. The word-boundary regex /(^|\s)psnr(\s|$)/m correctly avoids substring matches against hypothetical filter names or banner text.
Gate integration: Slots in at the right position after the Chrome capability probe, follows the identical pattern as the SwiftShader/CSS-effect/at-risk-timeline gates (deGateReason + deFallbackTrigger + routeToFallback()). Correctly skipped under HF_FORCE_DRAWELEMENT=1.
Belt-and-braces discriminator: isFfmpegInfrastructureFailure correctly separates infrastructure failures (ENOENT, "No such filter", "Unknown filter") from per-sample noise (EACCES, parse errors). The ENOENT aliasing edge case (spawn ENOENT vs fs ENOENT) is documented and the false-positive-toward-abort choice is the right safety call. The re-throw in psnrForDiskSample closes the fail-open gap for the mid-render case that bypassed the preflight.
Tests: 6 probe tests (present, absent, ENOENT, non-zero exit, memoization + reset, substring-not-word-boundary) + 5 discriminator test groups covering all three infrastructure patterns plus negative cases. Good coverage.
No issues. Ship it.
— Miga
What
Adds a one-shot ffmpeg-psnr filter probe at drawElement session bootstrap.
When the resident ffmpeg is missing or lacks libpostproc (no
psnrfilter),the capture-session router now force-fallbacks to the screenshot capture
path and emits a
de_gate_reason = \"ffmpeg_no_psnr_filter\"telemetrysignal via the existing
render_completebreakdown.Also tightens
psnrForDiskSample's catch: infrastructure-class ffmpegfailures (ENOENT, "No such filter") no longer silently skip the sample —
they abort the render so the safety net cannot fail-open post-preflight.
Why
The drawElement self-verify safety net (parallelCoordinator's
psnrForDiskSample→psnrDb) shells toffmpeg -lavfi psnr. If ffmpegis missing, or was compiled without libpostproc (so the
psnrfilter isabsent), every per-sample compare throws. The existing catch swallows the
error and returns
null— callers treat that as "skip this sample" andthe render completes with the safety net inoperative.
Field signal
⭐ 9/10 CLI feedback, Slack ts=
1787380767.210079, hyperframes0.8.7,darwin/arm64, tid=93ff9910-2207-45c2-bc1f-54c0b347d4fe. Exact usercomment:
The user's frames happened to be byte-identical so no visual damage
shipped — but the safety net silently wasn't running. Any future
compositor-damage bug on that host would have shipped straight through.
How
Two-part fix, both in
packages/engine:New
utils/psnrFilterAvailability.ts— cached probe that runsffmpeg -hide_banner -filtersonce per process and word-boundary-matches
psnrin the output. Any failure (ENOENT, non-zero exit,timeout, unparseable output) returns
false; never rejects.Wired into
services/frameCapture.tsinitDrawElementOrTransparentBackgroundright after the Chrome capability probe: when
useDrawElementresolvestrue and the preflight returns false, set
session.deGateReason = \"ffmpeg_no_psnr_filter\"(same low-cardinalitybucket every other DE gate uses; flows through
getCapturePerfSummary→
render_complete.de_gate_reasonin PostHog), emit a stderr warningnaming what's missing, and call
routeToFallback()— the samefail-graceful shape as the SwiftShader / CSS-effect / at-risk-timeline
gates. Skipped under
HF_FORCE_DRAWELEMENT=1(matches the diagnosticknob's policy of bypassing every other gate).
Belt-and-braces:
psnrForDiskSamplenow discriminates infrastructure-class failures (ENOENT / "No such filter" / "Unknown filter") from
per-sample noise (readFile races, transient EPERM). Only the former
re-throw — per-sample noise still returns
null(skipped sample). Thepreflight normally catches this at bootstrap; the re-throw covers
ffmpeg-swapped-mid-render.
Telemetry
Uses the existing
deGateReason/deFallbackTriggersurface (both setto the low-cardinality bucket
\"ffmpeg_no_psnr_filter\") rather thanadding a new event. PostHog blast-radius query is
render_complete WHERE de_gate_reason = 'ffmpeg_no_psnr_filter'— sameshape as every other DE fallback breakdown.
Test plan
packages/engine/src/utils/psnrFilterAvailability.test.ts— mockedexecFilecovers:psnrpresent → true;psnrabsent → false; ENOENT→ false; non-zero exit → false; result memoized + reset works;
substring-not-word-boundary → false.
isFfmpegInfrastructureFailureinpackages/engine/src/services/parallelCoordinator.test.tscoversENOENT, "No such filter", "Unknown filter", per-sample EACCES,
parse errors, null/non-object.
packages/engine/src/utils/psnrFilterAvailability.test.ts(6) +packages/engine/src/services/parallelCoordinator.test.ts(50) +frameCapture.test.ts(26) +drawElementService.test.ts(12) +parallelCoordinator-peerAbort.test.ts(1) all pass. Pre-existingffprobe.test.tsfailures (4) on the base commit are unrelated(missing PNG fixture bytes — the file is 129 B on disk, likely
LFS-stored).
bunx tsc --noEmit -p packages/engine/tsconfig.json— clean.bunx oxlint <files>— 0 warnings, 0 errors.bunx oxfmt --check <files>— clean.Not covered here: an integration test that boots
initDrawElementOrTransparentBackgroundend-to-end. That path isPuppeteer-driven and has no unit-scale bootstrap harness in the
repository — the pure preflight + pure discriminator coverage above are
what this PR can prove at the vitest layer. Manual repro of the field
signal (rename
ffmpegon $PATH → run a DE render → confirm stderrwarning + screenshot capture path + telemetry breakdown) is the
recommended integration step before merge.
— Via