fix: recursive sub-composition inlining for depth-3+ nesting#2660
Merged
Conversation
…ining inlineSubCompositions collected [data-composition-src] hosts from the document once, before inlining began, then processed that fixed list in a single flat loop. A host's inlined content could introduce new hosts of its own (a sub-composition nesting another sub-composition), and those were never discovered: two-level nesting worked because the second level's host already existed in the root document, but any third level's host only appeared inside content the single pass had already finished walking, so its content silently never rendered. Host discovery now runs as a work queue: after a host is inlined, the newly inserted subtree is re-scanned for further hosts, which are enqueued with their ancestry chain. A host whose source already appears in its own ancestry is a circular reference and is reported through the existing onMissingComposition channel instead of being inlined; a depth ceiling backstops any gap in that check. Existing single- and two-level fixtures are unaffected.
…-inline The producer's per-instance runtime id assignment (assignBundledRuntimeCompositionIds) ran once over the root document's initial hosts, before inlining. With host discovery now iterating to a fixed point (previous commit), hosts revealed inside an already-inlined sub-composition were never in that pre-pass, so the identity map returned undefined for them: two sibling instances of the same sub-composition, discovered mid-traversal, both fell back to their shared authored id and clobbered each other's variablesByComp entry. hostIdentityMap is now a lazy map: a pre-pass cache hit returns unchanged, a miss reads the host's authored data-composition-id, allocates a collision-checked runtime id by scanning the live document, writes it back, and caches it. No change to the shared inliner's call signature. Existing single-instance and root-level reusable-template cases (#2064) are unaffected.
Adds a minimal three-level chain (root -> level-2 -> level-3) where the deepest level renders a distinguishing full-frame marker, following the existing sub-comp-* fixture pattern. Confirmed against the pre-fix commit (735128a) in a separate scratch checkout that the fixture fails without the previous two commits (level-3's marker absent, ~0.4B fewer matching pixels than the golden expects) and passes with them. The golden reference video was captured on the team's render host (devbox) rather than locally, to avoid PSNR drift from font/GPU differences against whatever renders the regression suite in CI. Local render against that golden also passes (PSNR ~24.5dB throughout, 0 failed frames of 100 checkpoints), confirming cross-machine consistency as well.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
[data-composition-src]hosts in a single pass before inlining began, so any composition nesting sub-compositions three or more levels deep silently dropped the deepest level's content in rendered output (preview/studio could differ from render, with no error).hostIdentityMapis now a lazy map that assigns and caches ids for hosts discovered during inlining too.packages/producer/tests/nested-subcomp-depth-3/) with a three-level nesting chain; its golden reference video was captured on the team's render host to avoid local font/GPU-specific PSNR drift.Why
Discovered as a side effect of unrelated corpus regression work: a stress fixture with three-deep nested precomps scored well on aggregate metrics with zero reported errors — a silent content-loss defect, not a crash or a flagged limitation. Confirmed via code reading that the shared inliner (
packages/core/src/compiler/inlineSubCompositions.ts) has no self-recursion and exactly one call site (packages/producer/src/services/htmlCompiler.ts, insidecompileForRender), so any hand-authored or converted composition nesting three or more levels deep is affected, not just converter output.Test plan
packages/core/src/compiler/inlineSubCompositions.test.ts— new coverage: 3-level and 4-level chains render all levels; existing 1-2 level fixtures unchanged; direct and indirect circular references are detected and reported, not looped; legitimate sibling reuse of the same sub-composition is not treated as a cycle; depth-ceiling boundary behavior.packages/producer/src/services/htmlCompiler.test.ts— new coverage: repeated sibling sub-composition instances discovered mid-traversal get unique runtime ids with no variable collisions.packages/producer/tests/nested-subcomp-depth-3/— new end-to-end regression fixture; confirmed against the parent commit (pre-fix) in a separate scratch checkout that it fails (level-3 marker absent) without this change, and passes with it. Golden captured on the team's render host; local render against that golden also passes (PSNR ~24.5dB, 0 failed frames).packages/coreandpackages/producersuites green, no regressions to existing sub-comp-* fixtures.