Skip to content

refactor(producer): unify render requests#2162

Open
jrusso1020 wants to merge 1 commit into
07-10-refactor_engine_type_capture_failuresfrom
07-10-refactor_producer_unify_render_requests
Open

refactor(producer): unify render requests#2162
jrusso1020 wants to merge 1 commit into
07-10-refactor_engine_type_capture_failuresfrom
07-10-refactor_producer_unify_render_requests

Conversation

@jrusso1020

@jrusso1020 jrusso1020 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

What

Define one serializable RenderRequest and normalize environment input at the boundary.

Why

CLI, Docker, server, and distributed adapters copied option bags and silently dropped fields.

How

Add a shared request schema, explicit adapters, round-trip validation, and thread it through local/server/distributed entry points.

Test plan

  • render request round-trip, server parsing, CLI Docker, and distributed validation tests
  • Stack-wide lint, format, build, typecheck, and relevant integration gates

jrusso1020 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

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.
Learn more

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

@jrusso1020

Copy link
Copy Markdown
Collaborator Author

🤖 Automated code-review pass — LGTM, no blocking issues found.

Reviewed the incremental diff (new renderRequest.ts + CLI local/Docker, server, and distributed plan.ts wiring). Traced every producer of the old option-bag shape into the unified RenderRequest; field mappings are complete and consistent.

Verification notes (things that looked risky but check out):

  • CLI renderLocal maps all prior createRenderJob fields (fps/quality/format/gifLoop/workers/useGpu/hdrMode/crf/videoBitrate/videoFrameFormat/variables/entryFile/outputResolution/debug/strictness + producerConfigengineConfig, logger via runtime). Nothing dropped.
  • Docker forwards --protocol-timeout/--player-ready-timeout verbatim as ms (no /1000), which is correct: both CLI flags are documented in ms, unlike --browser-timeout which is seconds. render.ts:652-653.
  • buildRenderJobConfig(input, absoluteOutputPath, log)absoluteOutputPath is in scope at both call sites (server.ts:621, server.ts:697). Server RenderInput.fps is always a {num,den} rational (parseServerFps), so the new assertPositiveFps won't reject valid server requests; format is defaulted to "mp4".
  • plan.ts:728/746: switching producerConfig: config.producerConfigproducerConfig: cfg actually strengthens the invariant — the synthetic plan job now always pins browserGpuMode:"software" + forceScreenshot:false instead of potentially leaking env-derived values when producerConfig was undefined.
  • engineConfig added to DistributedRenderConfig is intentionally NOT omitted from SerializableDistributedRenderConfig (it's the JSON-safe replacement for the runtime-only producerConfig) and is now walked by validateDistributedRenderConfig for injection safety. Consistent.

Non-blocking nits (no need to gate on these):

  • packages/producer/src/renderRequest.ts:70-71(fps.num as number) / (fps.den as number) casts. Repo CLAUDE.md says "avoid as T assertions; prefer type guards." Defensible here since Number.isInteger() already runtime-guards, but a typeof fps.num === "number" narrowing would satisfy the convention without the cast. [PLAUSIBLE]
  • distributedConfigFromRequest, renderRequestFromDistributedConfig, and serialize/parseRenderRequest are exported and tested but have no production caller yet (only createRenderRequest/renderConfigFromRequest are wired into CLI + server). Expected for a stacked refactor — flagging only so a later PR in the stack actually threads them through the distributed entry point. [PLAUSIBLE]

@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from 138330b to e66085f Compare July 11, 2026 07:50
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from 41f77de to 1bfdbc3 Compare July 11, 2026 08:17
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch 2 times, most recently from 18adac0 to 15e7001 Compare July 11, 2026 08:21
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from 1bfdbc3 to 20de555 Compare July 11, 2026 08:21
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from 15e7001 to 7c8943b Compare July 11, 2026 08:53
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from 20de555 to 77e0579 Compare July 11, 2026 08:53
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from 7c8943b to da682ce Compare July 11, 2026 17:05
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from 6c95eaf to da8a304 Compare July 11, 2026 17:30
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch 2 times, most recently from 1b9c7d5 to d963271 Compare July 11, 2026 17:38
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from 070ce95 to bbdc1f1 Compare July 11, 2026 18:04
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch 3 times, most recently from deae328 to d528a8a Compare July 11, 2026 18:16
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch 2 times, most recently from 6cc349b to cf81bc1 Compare July 11, 2026 18:21
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from d528a8a to 978b8d0 Compare July 11, 2026 18:21
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from cf81bc1 to fadf8b0 Compare July 12, 2026 04:01
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from 978b8d0 to 8289249 Compare July 12, 2026 04:01
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from fadf8b0 to ae6787b Compare July 12, 2026 04:32
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from 8289249 to 03a0460 Compare July 12, 2026 04:32
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from ae6787b to f7d95e9 Compare July 12, 2026 04:58
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from 03a0460 to 94de4cb Compare July 12, 2026 04:58
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from f7d95e9 to 96ad318 Compare July 12, 2026 08:54
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from 94de4cb to 7b6f721 Compare July 12, 2026 08:54
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_engine_type_capture_failures branch from 96ad318 to f998c3f Compare July 12, 2026 09:11
@jrusso1020 jrusso1020 force-pushed the 07-10-refactor_producer_unify_render_requests branch from 7b6f721 to 4fbd296 Compare July 12, 2026 09:11
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.

1 participant