refactor(producer): move updateJobStatus to render/shared.ts#737
Conversation
First of several focused PRs that flatten the runtime cycle between the capture stages and `renderOrchestrator.ts` (documented as a known follow-up across PRs 1.6 / 1.7 / 1.8 / 1.9). `updateJobStatus` was the most-imported orchestrator helper: 5 of the 6 capture / encode / assemble stages reach back into the orchestrator for it. Moving it to `render/shared.ts` (where the other small cross-cutting utilities already live) breaks the runtime cycle for five stages in one move: - captureStage - captureStreamingStage - captureHdrStage - encodeStage - assembleStage Each of those stages now imports `updateJobStatus` from `../shared.js` at runtime, and the only thing they pull from `renderOrchestrator.js` is type-only (`RenderJob`, `ProgressCallback`, etc.) — type imports are erased at runtime, so no cycle. The orchestrator's own internal call sites (`updateJobStatus(...)` for the inline progress updates and the `complete` / `failed` / `cancelled` transitions) are unchanged in body; they now import the function from the same shared module. Follow-up PRs will move: - `executeDiskCaptureWithAdaptiveRetry` + capture-retry helpers (breaks the captureStage cycle entirely) - The six HDR helpers + `resolveCompositeTransfer` (breaks captureHdrStage) - `collectVideoMetadataHints`, `collectVideoReadinessSkipIds`, `materializeExtractedFramesForCompiledDir` (breaks extractVideosStage) No behavior change. Verified inside `Dockerfile.test`: font-variant-numeric, many-cuts, gsap-letters-render-compat, hdr-regression — 4/4 PASS with identical audio correlations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
vanceingalls
left a comment
There was a problem hiding this comment.
Verdict: approve.
updateJobStatus moves from renderOrchestrator.ts to render/shared.ts. Six stages (assembleStage, captureHdrStage, captureStage, captureStreamingStage, encodeStage, plus the sequencer itself) update their imports.
Verified the move is safe:
- Function body lifted byte-clean — same signature, same
job.completedAt = new Date()set on the terminal"failed"/"complete"transitions, sameif (onProgress) onProgress(job, stage)callback firing order. - All callers updated.
grepforupdateJobStatusacross the producer would find:shared.tsdefines it; the 5 stages +renderOrchestrator.tsimport it fromshared.ts. No stale orchestrator imports of the symbol. - The orchestrator's own use of
updateJobStatusinsideexecuteRenderJobis preserved — it just imports from the new location.
This is the audit-all-sites rule applied correctly: every caller updated, every dead orchestrator export removed. Function is referenced from ≥2 sites (in fact 6), no dead-code risk.
Important (non-blocking, cross-stack):
executeDiskCaptureWithAdaptiveRetrystill lives inrenderOrchestrator.tsand is imported bycaptureStage.ts. Same goes for the six HDR helpers (createHdrPerfCollector,addHdrTiming,closeHdrVideoFrameSource,blitHdrVideoLayer,blitHdrImageLayer,compositeHdrFrame) imported bycaptureHdrStage.ts, andmaterializeExtractedFramesForCompiledDir+ the twocollectVideo*helpers imported byextractVideosStage.ts. The runtime cycle is reduced but not broken — every capture/extract stage still imports something runtime from the orchestrator. Worth a follow-up consolidation PR to finish the job; otherwise the import-cycle smell stays in the codebase and gets harder to clean up as chunk rendering adds more stages.
Praise: smallest mechanical change in the stack, biggest structural payoff — shared.ts becomes the canonical home for stage-callable helpers, which is the right destination for the rest of the orchestrator's currently-cycled helpers. Lands the pattern even if it doesn't finish the migration.
— 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

What
Brief description of the change.
Why
Why is this change needed?
How
How was this implemented? Any notable design decisions?
Test plan
How was this tested?