perf(engine): extraction-phase instrumentation

[jrusso1020]

What

Adds per-phase timings and counters to extractAllVideoFrames and surfaces them on the producer's RenderPerfSummary as videoExtractBreakdown alongside a new tmpPeakBytes workDir size sample.

Why

Phase 2 video extraction has five distinct sub-phases (resolve, HDR probe, HDR preflight, VFR probe, VFR preflight, per-video extract) and today they collapse into a single videoExtractMs stage timing. That makes every subsequent perf PR in this stack immeasurable — you can't tell whether a win came from cache hits, preflight scope reduction, or pure extraction speed.

This PR is foundational for PR #445 (segment-scope HDR preflight) and PR #446 (content-addressed extraction cache).

How

• New ExtractionPhaseBreakdown type with resolveMs, hdrProbeMs, hdrPreflightMs/Count, vfrProbeMs, vfrPreflightMs/Count, extractMs, cacheHits, cacheMisses. Populated inline with Date.now() wrappers — overhead is sub-millisecond on every phase.
• Returned on ExtractionResult.phaseBreakdown.
• Producer extends RenderPerfSummary with videoExtractBreakdown?: ExtractionPhaseBreakdown and tmpPeakBytes?: number. tmpPeakBytes is sampled from the workDir right before cleanup via a new recursive-size helper that swallows errors (purely observational — a missing workDir must never fail the render).

No changes to the capture-lifecycle resource tracking — earlier versions of this instrumentation plumbed injector LRU stats through RenderOrchestrator, which conflicted hard with upstream #371 (buildHdrCaptureOptions refactor). Dropped that piece for a marginal observability loss.

Test plan

Validation on packages/producer/tests/vfr-screen-recording:

"videoExtractBreakdown": {
  "resolveMs": 0, "hdrProbeMs": 0, "hdrPreflightMs": 0, "hdrPreflightCount": 0,
  "vfrProbeMs": 0, "vfrPreflightMs": 166, "vfrPreflightCount": 1,
  "extractMs": 97, "cacheHits": 0, "cacheMisses": 0
},
"tmpPeakBytes": 4578598

Total elapsed within noise of pre-PR baseline (2665 → 2673 → 3228ms across hosts).

• Unit test: phase-breakdown assertion added to videoFrameExtractor.test.ts
• Lint + format (oxlint + oxfmt)
• Typecheck (engine + producer)
• Manual perf validation against VFR fixture

[jrusso1020]

• feat(cli): forward perf breakdown + tmpPeakBytes to PostHog telemetry #454
• perf(engine): content-addressed extraction cache for video frames #446
• perf(engine): segment-scope SDR→HDR preflight #445
• perf(engine): extraction-phase instrumentation #444 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miguel-heygen]

Review: perf(engine): extraction-phase instrumentation

Clean additive-only change — zero deletions, zero regression risk. The ExtractionPhaseBreakdown type is well-typed, the forward-looking cache placeholder fields (cacheHits, cacheMisses) avoid a type-breaking change in #446, and the sampleDirectoryBytes helper is defensively coded (iterative stack, error-swallowing).

P2 — Should fix

vfrPreflightMs subtraction creates asymmetric semantics.

breakdown.vfrPreflightMs = Date.now() - vfrPreflightStart - breakdown.vfrProbeMs;

hdrPreflightMs wraps the entire HDR preflight block (probe + analysis + conversion), but vfrPreflightMs deliberately excludes probe time. A consumer looking at both *PreflightMs fields would expect analogous scopes. Either:

• (a) Rename to vfrTranscodeMs to match what it actually measures, or
• (b) Include probe time in vfrPreflightMs (matching hdrPreflightMs) — the separate vfrProbeMs already provides the finer decomposition.

Option (b) is cleaner.

JSDoc references phase3ExtractMs but the field is named extractMs.

*   - phase3ExtractMs wraps the parallel `extractVideoFramesRange` calls

P3 — Suggestions

• Test coverage is VFR-only. HDR-path fields (hdrProbeMs, hdrPreflightMs, hdrPreflightCount) are never asserted — even a simple expect(...).toBeGreaterThanOrEqual(0) per field would catch initialization bugs.
• sampleDirectoryBytes follows symlinks via statSync. Using lstatSync and skipping symlinks would be more accurate for "bytes on disk in workDir." Minor since the workDir is engine-controlled.
• tmpPeakBytes returns 0 when workDir is missing, but the type is number | undefined. Using undefined for the missing-directory case would be more semantically precise.

Verdict: Approve with minor fixes

The instrumentation overhead is sub-ms (a handful of Date.now() calls). The design cleanly enables #445 and #446. Fix the vfrPreflightMs semantics and the JSDoc typo before merge.

[jrusso1020]

Pushed 5cfabff (on top of a rebase onto current main). Addressed both P2 items from <https://github.com/heygen-com/hyperframes/pull/444#pullrequestreview-4164953230|your review>:

1. vfrPreflightMs semantics — went with option (b). vfrPreflightMs is now the full wall-clock from vfrPreflightStart (same shape as hdrPreflightMs), and the separate vfrProbeMs carves out the probe-only time for the finer decomposition. Also added a JSDoc note clarifying that both *PreflightMs fields include their probe-time siblings, so future readers don't make the same "subtract probe" assumption.
2. JSDoc phase3ExtractMs → extractMs — fixed.

On the P3s, kept as-is for this PR since they're orthogonal to the semantic fix:

• HDR-path test assertions — happy to add in a follow-up; the instant-safety version (a single toBeGreaterThanOrEqual(0) on each HDR field) would catch init bugs without requiring an HDR fixture.
• sampleDirectoryBytes → lstatSync — agree it's more precise. The engine-controlled workDir is the narrow reason it's fine today, but a one-line swap is cheap.
• tmpPeakBytes → undefined on missing workDir — agree semantically. Current zero is a valid proxy for "directory already gone, can't measure" but undefined reads better.

All three would land clean in a single small follow-up PR; happy to do that after this stack merges so the downstream PRs aren't re-validated against moving semantics.

[miguel-heygen]

Re-checked the live head. The vfrPreflightMs semantics and extractMs JSDoc fixes are in place, the latest CI is green, and I found no remaining blockers.