Skip to content

refactor(producer): extract audioStage from executeRenderJob#726

Open
jrusso1020 wants to merge 1 commit into
mainfrom
05-12-refactor_producer_extract_audiostage_from_executerenderjob
Open

refactor(producer): extract audioStage from executeRenderJob#726
jrusso1020 wants to merge 1 commit into
mainfrom
05-12-refactor_producer_extract_audiostage_from_executerenderjob

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented May 12, 2026

Stacked on #725 (PR 1.4). Rebase to `main` once that merges.

What

Phase 1 PR 1.5 of the distributed-render refactor. Moves the audio mixing sub-stage of executeRenderJob into packages/producer/src/services/render/stages/audioStage.ts. Trivial extraction — the body is a thin wrapper around processCompositionAudio.

Source range moved: ~25 lines between the "── Stage 3: Audio processing ──" comment and the start of "── Stage 4: Frame capture ──".

Why

Continues the Phase 1 mechanical extraction. Phase 1 ships zero new functionality.

How

  • New file audioStage.ts exports runAudioStage(input) → AudioStageResult returning { audioOutputPath, hasAudio, audioProcessMs }.
  • Sequencer calls the stage at the same code point with the same inputs.
  • Removes the now-unused processCompositionAudio import from renderOrchestrator.ts (oxlint flagged it).

Preserved invariants

  • audioOutputPath is still join(workDir, "audio.aac") regardless of whether any audio was produced.
  • hasAudio reflects audioResult.success from processCompositionAudio; it is false when there are no audio elements (call is skipped) and also when the mixer returns success: false.
  • perfStages.audioProcessMs is set whether or not the mixer ran, at the same code point.
  • The 20% "Processing audio tracks" updateJobStatus fires at the same point.

Test plan

  • bunx oxlint + bunx oxfmt --check — clean.
  • bun run --filter @hyperframes/producer typecheck + build — clean.
  • bun test packages/producer/src/services/ — 176 pass, same single pre-existing failure unrelated to this PR.
  • docker run hyperframes-producer:test --sequential font-variant-numeric many-cuts variables-prod — 3/3 PASS, audio correlations 1.000 / 0.994 / 0.975 (identical to prior PRs in the stack; font-variant-numeric and many-cuts exercise the audio path).
  • Full regression matrix on CI via the regression workflow.

🤖 Generated with Claude Code

Move the audio mixing sub-stage of `executeRenderJob` into
`services/render/stages/audioStage.ts`. Trivial wrapper around
`processCompositionAudio` with the same skip-when-empty path.

No behavior change:
- `audioOutputPath` is still `join(workDir, "audio.aac")` regardless of
  whether the composition has audio.
- `hasAudio` still reflects `audioResult.success` (false when no audio
  elements or when the mixer returns success: false).
- `perfStages.audioProcessMs` is set at the same end-of-stage point
  whether or not the mixer ran.
- The "Processing audio tracks" progress callback fires at 20% at the
  same code point.

Removes the now-unused `processCompositionAudio` import from the
orchestrator (oxlint flagged it).

Verified inside `Dockerfile.test` against `font-variant-numeric`,
`many-cuts`, `variables-prod` — 3/3 pass with audio correlations
1.000 / 0.994 / 0.975 (identical to prior PRs in the stack).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vanceingalls
vanceingalls previously approved these changes May 12, 2026
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve.

Trivial extraction. audioStage is a thin wrapper around processCompositionAudio. Body lifted byte-clean: same audioOutputPath = join(workDir, "audio.aac") even when audio is empty, same hasAudio = audioResult.success, same perfStages.audioProcessMs timer semantics (set whether or not the call ran).

No-op concerns are also handled cleanly — the stage's timer covers both branches via stage3StartDate.now() - stage3Start, identical to the pre-extraction code.

Nits:

  • audioStage.ts:18job: RenderJob is declared on AudioStageInput but never read in the body. #735 actually removes this, so it's self-cleaning, but worth flagging that future stage inputs should be added only when consumed (foundation review feedback).
  • Same runtime cycle pattern as the other stages — audioStage.ts:3 imports RenderJob from ../../renderOrchestrator.js (type-only, but the import line is still there). Type-only imports are erased so this is harmless at runtime; #735 will tighten it.

Praise: smallest stage in the stack, smallest diff, easiest to verify. The pattern of explicitly preserving "the timer is still set so the perf summary stays consistent across renders" in the JSDoc is exactly the discipline this stack needs.

— Vai

Base automatically changed from 05-12-refactor_producer_extract_extractvideosstage_add_materializesymlinks_param to main May 12, 2026 02:59
@jrusso1020 jrusso1020 dismissed vanceingalls’s stale review May 12, 2026 02:59

The base branch was changed.

Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean mechanical extraction — no behavior changes, no introduced bugs. Verified imports, error handling, and cleanup invariants are preserved. LGTM. — Magi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants