refactor(producer): document executeRenderJob as a thin sequencer#735
Conversation
Final polish PR of the Phase 1 stack. Comment-only — zero code change. After PRs #725, #726, #730, #731, #733, #734, the `executeRenderJob` function now composes eight stage modules instead of inlining the pipeline. Updates the file-level JSDoc to point at each stage module and explains the orchestrator's residual responsibilities: shared resource lifetime, perf counters, error diagnostics, and the `try/finally` cleanup. Adds JSDoc on `executeRenderJob` itself summarising what it returns and when it throws. The function body is unchanged. The line count dropped from ~2,200 (pre-Phase-1) to ~880; the remainder is in-sequencer setup that doesn't naturally compose into a stage (calibration, worker resolution, HDR auto-detection, preset selection, final perf-summary assembly) plus the orchestrator's `try/finally` resource ownership. Verified inside `Dockerfile.test`: font-variant-numeric (1.000), many-cuts (0.994), variables-prod (0.975), hdr-regression (1.000) — 4/4 PASS with audio correlations identical to every prior PR in the Phase 1 stack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment + interface cleanup driven by the /simplify review. No code change beyond removing dead fields. - audioStage: drop unused `job: RenderJob` from `AudioStageInput` (the stage destructures it but never references the value). - encodeStage: drop unused `fps` + `useGpu` from `EncodeStageInput`; read both from `job.config.*` inside the stage (matches the pattern used by captureStage and captureStreamingStage). - captureStreamingStage: drop the unused `captureDurationMs` field from `CaptureStreamingStageResult` (sequencer never reads it — it uses its own `Date.now() - stage4Start` for `perfStages.captureMs`). Also drops the now-dead `streamStart` local. - captureStreamingStage: rewrite the "Known follow-up" header comment to drop the "PR 1.3.5" reference per `feedback_no_internal_track_names_in_source`. - captureHdrStage: drop the "Lifted verbatim from `executeRenderJob`" refactor-narration sentence in the header doc (the "Hard constraints preserved verbatim" list below it is real long-term documentation and stays). - Sequencer call sites updated to drop the now-removed fields. Verified inside `Dockerfile.test`: 4/4 fixtures pass with PSNR / audio correlations unchanged (font-variant-numeric, many-cuts, gsap-letters, hdr-regression). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vanceingalls
left a comment
There was a problem hiding this comment.
Verdict: approve.
Documentation + sequencer-cleanup PR. Three things happen:
- The file header on
renderOrchestrator.tsis rewritten to documentexecuteRenderJobas a thin sequencer over the eight stage modules, listing each stage with its file path. The stages are listed in execution order — useful for new readers. - A JSDoc block is added immediately above
executeRenderJobdocumenting the function as the in-process entry point. - Three stage inputs that turned out to be dead code are removed:
audioStage'sjob: RenderJob,encodeStage'sfps: FpsanduseGpu: boolean | undefined(both already available viajob.config), andcaptureStreamingStage'scaptureDurationMsfrom the result (the sequencer always recomputedDate.now() - stage4Startitself, so the field was dead).
The cleanup tightens the interfaces but doesn't change behavior. Verified each removed field had exactly zero consumers on the sequencer side.
captureHdrStage.ts's header comment loses the "Lifted verbatim from executeRenderJob" line — the lineage is documented in the PR history, no need to carry it forever. Fine.
Nits:
- The removed
captureDurationMsfield fromCaptureStreamingStageResultshould have been dropped in #731 instead of carried forward — see the #731 review nit. Folded in here is fine; future PRs in the stack should aim to land the stage-input cleanup in the same PR that introduces the stage. - The file-header block now lists stages 1, 1b, 2, 3, 4 (three files), 5, 6 — that's eight files for six "logical" stages. The "Stage 4" line spans three files (
captureStage.ts,captureStreamingStage.ts,captureHdrStage.ts) which is accurate but mildly confusing in a single-glance read. A one-line note like "Stage 4 has three implementations selected by render mode" would help.
Praise: the file-header rewrite is the right artifact at the right time — once the sequencer is actually a sequencer, the docs should say so. The pre-stack header (1. Parse composition metadata → 6. Final assembly (audio mux + faststart)) was describing what executeRenderJob used to be; the new header describes what it is now. This is the kind of "while I was here, also kept the docs honest" change that's almost always worth landing.
— Vai
miguel-heygen
left a comment
There was a problem hiding this comment.
Clean mechanical extraction — no behavior changes, no introduced bugs. Verified imports, error handling, and cleanup invariants are preserved. LGTM. — Magi
The base branch was changed.

What
Final polish PR of the Phase 1 stack. Comment-only — zero code change.
After the prior eight extraction PRs,
executeRenderJobnow composes eight stage modules instead of inlining the pipeline. This PR updates the file-level JSDoc to point at each stage module and explains the orchestrator's residual responsibilities. Also adds JSDoc onexecuteRenderJobitself.Why
Closing-out cleanup of Phase 1. The orchestrator is now a thin sequencer over the eight stage modules; the comment surface needs to reflect that so the next reader (or the next set of PRs from Phase 2) doesn't have to reconstruct the architecture from import statements.
The "thin sequencer (~150 lines)" goal in the original Phase 1 plan was aspirational — the realistic floor without further extraction is ~880 lines. The remaining lines are the in-sequencer concerns that don't naturally compose into a stage:
try/finallyresource ownership (file server, capture sessions, frame-data caches, memory sampler)These could be further factored in Phase 2 if useful, but the Phase 1 contract was "no behavior change" — and the call sites that orchestrate stages are already as thin as they can be.
How
executeRenderJobsummarising its contract (returns onceoutputPathexists; throws on cancellation / stage failure with a diagnostic summary).Phase 1 stack summary
extractVideosStage(+materializeSymlinksparam)audioStagecaptureStage(SDR disk)captureStreamingStage(single-machine fusion)captureHdrStage(Z-ordered layered composite)encodeStage+assembleStage(Stage 5 + Stage 6)Plus earlier PRs (#717, #718, #719, #720): planHash scaffold, compileStage, probeStage, render/shared.ts cycle break.
executeRenderJobwent from ~2,200 lines pre-Phase-1 to ~880 lines now — and the 880 are all sequencer / setup / cleanup, no inline stage bodies.Test plan
bunx oxlint packages/producer/src/services/renderOrchestrator.ts— clean.bunx oxfmt --check— clean.bun run --filter @hyperframes/producer typecheck+build— clean.bun test packages/producer/src/services/— 176 pass, same single pre-existing unrelated failure onmain.font-variant-numeric(1.000),many-cuts(0.994),variables-prod(0.975),hdr-regression(1.000). Audio correlations identical to every prior PR in the Phase 1 stack.regressionworkflow.🤖 Generated with Claude Code