Repository navigation
fix: preserve motion blur off through cloud render requests - #6087
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed e5b09b6 together with its EF companion #59833 at 6d8aaeeb. No code blocker found. The companion answers my earlier question about where the backend support lives.
What the change does
- The CLI forwards one optional public field.
- Both producer HTTP handlers share one validation and forwarding path.
- Studio's route maps new and legacy input onto the existing producer option.
Producer (producer/src/server.ts:461,321): rejects any explicit value other than off, keeps an omitted value omitted through createRenderRequest, and forwards off.
Studio (studio-server/src/routes/render.ts:155-162): keeps the legacy previewMotionBlur=false mapping. A canonical off wins over a preview capability of true. The current Studio browser still sends the legacy capability field, so normalisation happens at the server boundary.
CLI (cli/src/commands/cloud/render.ts:222,843): validates the flag and writes snake_case page_motion_blur. A probe of the actual body-builder function confirms the off and omitted cases. The generated type's off/null shape matches the companion DTO and OpenAPI.
Resume and cache
renderOrchestrator.ts:4496feeds the setting intosegmentManifest.ts:75, andoffhashes differently from an omitted value.- The distributed and Lambda paths I inspected already force page blur off, so their behaviour doesn't change.
- EF's raw-body fingerprint includes the new field. Replaying the same Idempotency-Key is still intentionally lenient, so the setting gets no separate cache key there.
What ran
- 64 Studio-route tests and 13 segment-manifest tests passed, using borrowed dependencies and current-source aliases.
- The producer and CLI suites failed to collect locally because of stale package exports, so I'm not claiming those green.
- An isolated probe of the body-builder passed.
- The EF companion's 124 focused Python cases passed.
Not verified: an independent Chrome render, a hosted cloud call, installed Desktop behaviour, a deployment, or a Temporal-history replay.
Release order must be: compatible producer → EF #59833 → new clients. Until then, the old EF rejects the public field, and the old producer silently ignores the internal one. This approval doesn't establish live cloud support or authorise a merge or deploy.
Review by Rames (hf-review)
Edit accuracy: accurate 271 (base branch 271), smooth 157 of thoseThe gate passes. Quarantined, measured but not gated (0) |
e5b09b6 to
029ac9d
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Re-reviewed exact 029ac9d60892cc086a5b3acfb88b76914c666a69 with EF #59833 at 0d38fedf.
Need · Size · Reuse · Simpler: The distributed repair carries the existing render option through request/config conversion, freezes it in encoder metadata and consumes it during chunk capture. No parallel blur policy is added.
Explicit off is validated, preserved in both request/config directions and stored in the locked encoder. V1 plan hashes include encoder bytes and V2 manifests include its artifact digest, so explicit intent cannot share an omitted plan identity. Old plans lacking the field still use encoder.pageMotionBlur ?? "distributed-render", preserving the old capture default. Local resume hashing and the prior public producer/Studio validation remain intact.
Audited: the nine-file distributed successor, surrounding config validation/freezing/hash consumers, and original CLI/server/Studio handoffs. Trusting: authored full-render/packaged acceptance.
Executed extracted actual-function probes for off/omission through both mapping boundaries, locked encoder generation and chunk capture. Omission serializes identically to the prior locked-config function; explicit off survives. Constructor/config/GPU dependencies unrelated to this option were stubbed, so this is not full-producer execution. The EF companion's 298 scoped tests and independent alias/wire controls passed.
Local producer/Lambda suites could not collect against borrowed dependencies because core subpath exports are stale; Bun-only suites were not counted as Vitest passes. No live distributed render, Chrome output, deployment or replay was performed.
Release order remains compatible producer/workers → EF/public API → clients. Older producers can silently ignore the field; code approval does not verify rollout readiness or authorize a reviewer merge/deploy.
— hf-review
Verdict: APPROVE
Reasoning: Explicit blur intent now participates in frozen plan identity and reaches chunk capture, while old omitted plans retain their default. No blocker found in the inspected pair.
029ac9d to
763553d
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Re-reviewed exact 763553da6126f817c876f41755341b84acec71e3 against my approved 029ac9d60892cc086a5b3acfb88b76914c666a69.
Need · Size · Reuse · Simpler: This is a clean rebase of the same three commits. Git range-diff marks all three patches identical. The current tree exactly equals the clean merge of the approved head with new main aa4b53533414e5669caf6a9910c499143bbe0465 (tree f1b41c31e34d880c4aa328bf1f57e61c4854afa1); there is no additional conflict-resolution delta.
Rechecked the validation/config/freezing/chunk handoffs: explicit off remains in frozen encoder intent, and old omitted plans retain encoder.pageMotionBlur ?? "distributed-render". Inherited main changes comprise motion-blur documentation and the separately reviewed decoded-frame publication fix; neither changes those option handoffs. At their shared capture boundary, #6092 only defers publishing injected image visibility until decode completes; it neither reads nor rewrites the blur option and is awaited on the same injection path. This source read found no new option-dependent branch or cache identity change.
Audited: rebase/merge-tree equivalence and the inherited integration boundary. The earlier producer/Studio/CLI, plan-identity and EF-companion audits remain prior-revision evidence. I did not rerun their tests or full producer suites for this equivalent rebase, and do not claim a fresh live cloud render, deployment or Temporal-history replay.
Release order remains compatible producer/workers → EF/public API → clients. Older producers can silently ignore the internal field and older EF rejects the new public field. Rollout compatibility has not been independently verified; approval does not authorize merge or deployment.
— hf-review
Verdict: APPROVE
Reasoning: The rebased change preserves the reviewed implementation exactly, including distributed intent and omitted-plan defaults. No new code blocker found.
Cloud exports can now keep authored page motion blur disabled with
hyperframes cloud render --page-motion-blur off. The CLI sends the optional publicpage_motion_blurfield; omission keeps current behavior. The HTTP producer accepts and forwardspageMotionBlur: "off", and Studio preserves the same canonical value while retaining older preview clients.The cloud SDK was regenerated from the public API schema, including its existing schema updates. Producer examples were corrected to the current execution API.
Validation on Linux: producer HTTP tests (75), cloud command tests (19), and Studio route tests (64) passed three consecutive runs. Removing each forwarding edge deliberately made its regression test fail; restoring it passed. Type checks, builds, lint, formatting, process ownership and fallow checks passed. Real Chrome renders through sync and streaming HTTP handlers produced sharp output for off and authored blur when omitted; decoded frames matched between handlers for each mode.
Hosted API deployment and installed Desktop behavior were not exercised. The public API and consuming applications must deploy compatible versions before relying on the new cloud flag. Commit hooks were skipped on the development Mac; checks ran on Linux.
Distributed renders now freeze the motion blur choice in the plan, so every chunk worker renders with the same setting even if the request shape changes later. Plan and chunk tests (66 and 3) and the AWS adapter tests (39) pass three runs in a row on Linux; removing the freeze, the chunk read, or the validator check each makes a test fail.