fix(render): surface structured outcomes

[jrusso1020]

What

Promote render warnings and terminal states into structured, ordered outcomes while preserving best-effort compatibility for existing callers. Strict readiness is an explicit opt-in through bestEffort: false or --no-best-effort.

Why

Missing or unready media could be logged while the render still reported unqualified success, and async progress sinks could reorder or detach failures. The new structured surface makes degraded output observable without silently turning an omitted field into a breaking policy change.

How

Add stable RenderWarning codes, explicit outcomes, one shared strict/best-effort policy, a bounded progress representation, and an ordered publisher. Omitted bestEffort remains best-effort at every public boundary; only explicit false selects strict mode.

Audit the regression fixtures against the enforced timeline contract. The compiler now separates structural flattening from timeline identity: authored nested composition structure is preserved, and timeline registration is published under both the authored local id and runtime mount id when they differ. Non-timeline roots declare data-no-timeline.

Test plan

• Engine readiness-warning tests; producer event/outcome tests; CLI and server render tests
• Compatibility tests for omitted bestEffort, explicit strict mode, distributed warning policy, and immutable failed snapshots
• Compiler/scoping tests for mismatched authored and mounted composition ids
• style-1-prod compilation matches its golden with zero warnings/errors and all 510 frames capture with timeline readiness complete
• Full monorepo lint, format, build, typecheck, and test suites
• Full CLI suite: 134 files / 1,676 tests

[jrusso1020]

• refactor(repo): resolve changed-code audit #2174
• test(engine): isolate peer abort regression #2173
• refactor(cli): decompose render phases #2172
• refactor(cli): centralize process lifecycle #2171
• fix(cloudrun): publish adapter contract #2170
• test(repo): execute packed subpaths #2169
• refactor(repo): centralize package subpaths #2168
• refactor(core): own edit protocol contract #2167
• test(compiler): enforce preview render parity #2166
• refactor(studio): gate SDK operation families #2165
• refactor(studio): gate Acorn writer migration #2164
• refactor(producer): make capture plans immutable #2163
• refactor(producer): unify render requests #2162
• refactor(engine): type capture failures #2161
• refactor(engine): manage child process lifecycles #2160
• refactor(engine): add fingerprinted browser leases #2159
• refactor(render): centralize execution ownership #2158
• refactor(core): unify composition contract #2157
• fix(studio): enforce optimistic file concurrency #2156
• fix(studio): make sdk cutover transactional #2155
• fix(render): publish artifacts atomically #2154
• fix(render): surface structured outcomes #2153 👈 (View in Graphite)
• fix(player): own connection and media resources #2152
• fix(core): preserve runtime transport contract #2151
• fix(player): version runtime protocol #2150
• test(repo): verify packed consumers #2149
• test(repo): enforce workspace contracts #2148
• fix(core): enforce strict runtime safety #2147
• test(producer): gate source tests by execution lane #2146
• fix(core): make timing masks safe for source loaders #2145
• chore(repo): forbid tracked generated artifacts #2144
• main

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

[miguel-heygen]

Independent stack-layer review on 050fad62.

The warning-code/policy plumbing and ordered immutable snapshots are strong (packages/producer/src/services/render/renderEventPublisher.ts:25, packages/producer/src/services/renderOrchestrator.ts:522).

Blocker — packages/producer/src/services/renderOrchestrator.ts:3091: the failed terminal event is published and flushed before failedStage and errorDetails are assigned (:3093, :3100). Because OrderedRenderEventPublisher.publish() snapshots immediately (packages/producer/src/services/render/renderEventPublisher.ts:29), every progress sink receives a permanently incomplete failure snapshot; it also records failedStage only after updateJobStatus has overwritten currentStage with the formatted error string. Populate the structured failure fields (capturing the actual pre-failure stage) before publishing, and pin that contract in the publisher test.

The current regression/preflight runs are green; the UNSTABLE surface is cancelled superseded runs plus pending Graphite mergeability, not the reason for this verdict.

Verdict: REQUEST CHANGES
Reasoning: The PR promises structured ordered terminal outcomes, but the immutable failed snapshot omits the very failure structure downstream consumers need.

— Home

[james-russo-rames-d-jusso]

Reviewed at 050fad6.

Core refactor is genuine — installDebugLogger → createRenderFileLogger is a real fix (concurrent renders would trash each others' console.log/warn/error globally, since the old path patched the process-global object), and OrderedRenderEventPublisher correctly serialises async sinks + contains failures at the boundary. Structured CaptureWarning codes + dedup + immutable snapshots are the right shape for downstream consumption.

Concerns

• Strict-by-default is a cross-cutting policy shift. Every producer HTTP caller that doesn't send bestEffort: true now receives RenderQualityError on audio_processing_failed / media_readiness_timeout / media_load_failed / sub_timeline_readiness_timeout / sub_timeline_script_failure. Previously most of these logged and produced degraded output. In-repo callers (CLI + docker path) all wire --best-effort through cleanly, but external callers (heygen-com studio, EF preview, backend services POSTing /render) will start failing renders that used to succeed with video-only output. Is downstream ready for strict-by-default, or does this need coordinated rollout (feature flag / dual-write / progressive migration)?
• Distributed render path bypasses the new policy. See inline at renderOrchestrator.ts — the audio-error branch in packages/producer/src/services/distributed/plan.ts:908-909 still uses the pre-PR log.warn("output will be video-only") pattern and does NOT call applyRenderWarningPolicy. Strictness is honored on the monolithic path but silently ignored on distributed, which defeats one of the PR's headline properties.

Nits

• data-no-timeline opt-out is a new authoring contract. Worth a line in the composition author docs / CONTRIBUTING so external composition consumers don't hit sub_timeline_readiness_timeout on non-timeline hosts.

What I didn't verify

• Whether every downstream HTTP caller of /render already sets bestEffort: true. Workspace grep turned up only in-repo CLI wiring; external heygen-com callers are out of tree from where I sit.
• Whether the captions dual-publish contract holds across all 18 style-*-prod regression fixtures once landed.

— Review by Rames D Jusso

[jrusso1020]

Valid — addressed on the current head.\n\n- Failure metadata is now assembled before the terminal status update is published, and the original failing stage is captured before currentStage is changed.\n- The distributed planner now threads strictness into the synthetic render job and routes audio degradation through the same structured warning policy as the monolithic renderer.\n- Added regressions for the complete immutable failure snapshot and strict/best-effort distributed audio behavior.\n\nValidation: Producer typecheck; 7 render-event tests; 40 distributed-plan tests; repository lint/format/typecheck hooks; and a real top-of-stack Chrome/FFmpeg MP4 render with 100 visual checkpoints and zero failures.\n\nThe strict default remains intentional; this change closes the concrete distributed/monolithic parity hole. Any external consumer rollout coordination is operational rather than something this repository can prove in code.

[miguel-heygen]

Re-reviewed current head b9625a7206eb40ecdd71e41970b9d05f88ad896e.

The two earlier implementation blockers are addressed: terminal failure state is now fully populated before the failed snapshot is published, and distributed planning now routes audio warnings through the shared strict/best-effort policy with focused tests.

One release blocker remains: strict-by-default rollout is not ready. The external HTTP path in packages/producer/src/server.ts still interprets an omitted bestEffort as strict, while renderOrchestrator.ts turns readiness warnings into fatal errors. Required CI demonstrates the compatibility impact rather than merely suggesting it: Render on windows-latest fails because the reused-video fixture has no data-no-timeline marker and strict mode rejects the resulting sub_timeline_readiness_timeout; regression also fails, including a real style-1-prod compiler/golden timeline-layer mismatch on shard 2. There is still no guarded rollout/default migration or evidence that downstream callers which omit the new field are ready.

Please either keep best-effort as the compatibility default while callers/fixtures migrate, or land the coordinated strict rollout and make the required Windows/regression fleet green.

Verdict: REQUEST CHANGES
Reasoning: The prior correctness gaps are fixed, but strict-by-default remains an unguarded breaking rollout and required CI is red from that behavior.
— Magi

[jrusso1020]

Addressed the remaining release blocker on current head 6d1336f66.

• Omitted bestEffort now preserves the compatibility default at every public boundary; only explicit bestEffort: false or --no-best-effort selects strict readiness.
• The style-1-prod regression was a real compiler issue, not papered over: structural flattening is again limited to an exact mount-id match, while authored/runtime timeline identities are published separately when they differ.
• Added focused compatibility, compiler/scoping, distributed-policy, and immutable failure-snapshot coverage.

Local validation is green: full monorepo lint, format, build, typecheck, and tests; full CLI suite (134 files / 1,676 tests); and an actual style-1-prod compile/render completed with the golden DOM matching, zero compiler warnings/errors, timeline readiness complete, and 510/510 frames captured. Fresh required CI is running on the rewritten head.

Miguel, please re-review when CI settles.

[jrusso1020]

CI follow-up on current head 6d1336f66: every fresh required check is now green, including Windows render + tests, Producer integration on rerun, preview parity, player perf, and all eight regression shards. The review-thread scan is also clean. The prior requested changes are fully addressed and this is ready for re-review.

[miguel-heygen]

Re-reviewed rewritten head 6d1336f66b24fb3910f88921d8f04e9a005091d3 against the two prior blockers and the complete stack delta.

Both correctness issues remain fixed: terminal failure fields are populated before the immutable failed snapshot is published, and distributed audio degradation flows through the shared strict/best-effort policy. The rollout blocker is now resolved at the canonical boundaries: createRenderJob, the HTTP parser, CLI local/docker paths, and distributed planning all preserve best-effort when omitted; strict behavior requires explicit bestEffort: false, --no-best-effort, or strictness: "strict". Focused tests pin omitted/true/false behavior and the distributed strict/best-effort split.

I also checked the compiler follow-up that repaired the style-1-prod regression: mount flattening is again limited to exact composition identity while authored/runtime timeline identities remain separately registered. Local focused verification passed 168 tests across the render CLI, compiler scoping/inlining, event publisher, HTTP parser, and distributed planner. Fresh required CI is green, including Windows render/tests and all eight regression shards; cancelled entries are superseded runs with green replacements.

Verdict: APPROVE
Reasoning: The structured-outcome contracts are complete, compatibility remains best-effort by default across every public path, strictness is explicit, and the prior runtime/CI regressions are resolved with focused coverage and a fully green required fleet.

— Deepwork