Skip to content

feat(skills): pre-plan persistent-subject choreography (Addition C)#551

Open
vanceingalls wants to merge 1 commit intomainfrom
vance/04-28-persistent-choreography
Open

feat(skills): pre-plan persistent-subject choreography (Addition C)#551
vanceingalls wants to merge 1 commit intomainfrom
vance/04-28-persistent-choreography

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

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?

  • Unit tests added/updated
  • Manual testing performed
  • Documentation updated (if applicable)

Copy link
Copy Markdown
Collaborator Author

vanceingalls commented Apr 29, 2026

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

Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Staff review

TL;DR: references/multi-scene.md is the strongest piece of this stack — the Scene Fragment Spec (3 markers, ID prefix, prohibited patterns) is exactly the kind of explicit guardrail that converts implicit failure modes into checkable rules. The persistent-subject choreography YAML is a good fix for a real problem (subject scaling up into reserved region collides with content). But there's a runtime claim that doesn't match the codebase, R4 is still undefined, and the "Phase 2b" / "Step 0a" pattern continues the accreted-edit numbering smell.


Blocking

1. @hyperframes/core/assemble doesn't exist. Phase 3's example code:

const { assembleScenes } = await import("@hyperframes/core/assemble");

will throw Cannot find module. I checked packages/core/package.json — the subpath exports are ./lint, ./compiler, ./runtime, ./studio-api, ./text, ./registry. There is no ./assemble, and no assemble* source file under packages/core/src/. Either ship the assembler in this PR (add packages/core/src/assemble/index.ts + the export) and write tests against it, or change Phase 3 to a documented procedure that doesn't depend on a nonexistent module.

2. R4 still not defined. Grepped the cumulative state at the top of the stack — R4 (persistent-element continuity morph) is referenced in #550 and used as the gating condition here ("When R4 applies...") but never has a definition anywhere. If this PR introduces the persistent-overlay rule, define R4 explicitly (one-liner is fine: "R4 — when a named real-world subject persists across 2+ scenes, render it in a shared overlay layer outside .scene containers and migrate it across transitions"). Otherwise readers can't tell when the rule fires.

3. Phase 2b without Phase 2a is the same accreted-edit smell as #549's 0a/0b. Renumber: "Phase 1: Scaffold + Scene subagents" / "Phase 2: Streaming evaluation" / "Phase 3: Assembly" reads cleanly.


Significant

4. 1920×1080 is hard-coded into the choreography YAML. The example uses { x: 1380, y: 540 }, { w: 540, h: 680 }, etc. Hyperframes supports any dimension declared via data-width/data-height (square, 1080×1920, etc.). Either generalize the YAML (percentages, or "relative to the composition's declared dimensions"), or label the example explicitly as "for a 1920×1080 composition; scale to your data-width/data-height."

5. multi-scene.md is 188 lines and has multiple self-contained sub-specs. The Scene Fragment Spec alone is dense and consumed by both the assembler code and the orchestrator. Worth extracting to references/scene-fragment-spec.md so the assembler can link it independently. Trade-off accepted if you'd rather keep one file — but note that 188 lines is at the upper end of "reference loaded just-in-time."

6. Density target is unmotivated. "15+ animated elements? 3 parallax layers?" appears in Phase 2b's content validation as a checkable target with no source or rationale. Where do these numbers come from? Are they enforced anywhere? Either link to the rule's origin (house-style.md? a benchmark?) or drop.

7. Persistent-subject scaffold contract is incomplete. The text says "the scaffold owns the subject's DOM + timeline" and "the scaffold authors the subject's tweens between choreography positions" — but the scaffold-author's instructions don't show how the choreography YAML translates to scaffold tweens (which .to() calls, with what eases, on what timeline). If the scaffold is supposed to consume the YAML, give it an example. If a separate "scaffold scaffolder" handles this, name it.

8. Stacks on #549's design.md schema gap. Scene subagents receive "the design.md (or its values summarized)" — but the schema isn't pinned down in #549, so "summarized" can mean anything.


Smaller things

  • Contrast section wording: "Small text (under 24px) has no large-text exemption" reads as an exception to a rule that wasn't stated. WCAG AA is 4.5:1 for normal text, 3:1 for large text (24px+ regular / 19px+ bold). If the policy is "always 4.5:1, no exceptions" just say that — it's clearer.
  • "Maximum 2 retries per scene — if a scene fails 3 times...": 2 retries = 3 attempts total. Fine, but worth confirming the orchestrator counts the same way (some implementations would call this "3 retries").
  • Visibility kills: tl.set("#sceneN", { visibility: "hidden" }, exitEndTime)tl is referenced before the scaffold's gsap.timeline({ paused: true }) line is shown. Reorder or add a one-line "the scaffold's timeline variable is tl" so the example is self-contained.
  • "Run npx hyperframes lint" / "Run npx hyperframes validate if available": both commands exist in packages/cli/src/commands/ (verified). Drop "if available" on validate.
  • Empty PR template + no test plan, same as #549/#550.

What this PR gets right

  • Scene Fragment Spec is excellent. The explicit prohibited-patterns list (<!DOCTYPE, <script>/</script> tags wrapping the GSAP section, tl.from, gsap.timeline(, bare class names without s{N}- prefix, CSS transform on GSAP-animated elements) is exactly the kind of guardrail that converts past failures into checkable rules. The "Why this prohibition exists" comments inline are the difference between "rule" and "rule that future Claude actually follows."
  • "Who runs this pipeline" section is great defensive design. Explicitly handling the nested-subagent case (no Agent/Task tool available → build serially, note the constraint in your final report) prevents the silent-broken-pipeline failure mode.
  • Persistent-subject contract addresses a real failure: scenes authored without knowing the subject's bounding box at its FINAL state, then the scaffold scales it up into reserved region. The YAML format with position, scale, size_envelope, role, reserved_region, scene_must_avoid is well-thought-out. The Vermeer example with three scenes at 1.0x → 2.1x → 0.85x is concrete enough to copy.
  • Streaming Phase 2b (evaluate as scenes finish, don't batch) matches modern subagent orchestration best practices.

Recommended path

Block on (1) — either ship the assembler module or change Phase 3 to not depend on it. Block on (2) — define R4 inline. Renumber Phase 2b → Phase 2 (3). Generalize the YAML coordinates (4). De-block on (8) once #549 lands. The Scene Fragment Spec, "Who runs this pipeline" section, and choreography YAML format are keepers.

Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Staff re-review (round 2)

TL;DR: No new commits on this branch. Still inherits #550's stack drift (which inherits #549's) — needs gt restack after #550 is restacked. Both round-1 blockers are still present in the codebase: @hyperframes/core/assemble doesn't exist and R4 is still undefined.


Round 1 blockers — status

1. @hyperframes/core/assemble doesn't exist. Still BLOCKING. I re-checked packages/core/package.json at this branch's tip (3c2e5378):

exports: ./lint, ./compiler, ./runtime, ./studio-api, ./text, ./registry,
         ./schemas/registry.json, ./schemas/registry-item.json

No ./assemble export, no source file. The Phase 3 example code:

const { assembleScenes } = await import("@hyperframes/core/assemble");

will throw Cannot find module. Either ship the assembler module in this PR (add packages/core/src/assemble/index.ts + the export + tests), or change Phase 3 to a documented procedure (e.g., concatenate scene fragments into the scaffold's <body> between markers, register timelines).

2. R4 still undefined. Still BLOCKING. Re-grepped the full cumulative state:

$ git grep -nE "^- R[0-9]|## R[0-9]|R1 |R2 |R3 |R5 " origin/vance/04-28-persistent-choreography
(no results — R4 is referenced but never numbered, defined, or paired with R1–R3/R5)

Define R4 inline at first reference: e.g., "R4 — persistent-element continuity morph: when a named real-world subject persists across 2+ scenes, render it in a shared overlay layer outside .scene containers and migrate it across transitions."

3. Phase 2b numbering. Unchanged. Still reads as accreted edit. Renumber: Phase 1 / Phase 2 (Streaming evaluation) / Phase 3 (Assembly).

4. 1920×1080 hard-coded in choreography YAML. Unchanged. Either generalize to percentages, or label the example "for a 1920×1080 composition; scale to your data-width/data-height."

5. multi-scene.md size (188 lines). Trade-off — keep or split. Not blocking.

6. Density target unmotivated ("15+ animated elements? 3 parallax layers?") — unchanged. Still needs a source or removal.

7. Persistent-subject scaffold contract incomplete. Unchanged. Show how the choreography YAML translates to scaffold tweens (which .to() calls, with what eases, on what timeline). The Vermeer YAML example needs a paired scaffold snippet.

8. design.md schema gap (carried from #549) → ✅ Resolved by #549 (google-labs-code spec).


New finding

Stack drift inherited. This PR is correctly based on #550's tip (0fd27d68), but #550 itself is 19 commits behind #549. So #551 transitively misses all of #549's recent improvements. After #550 is restacked on #549, this PR will need a corresponding restack. Watch for conflicts in references/prompt-expansion.md (which #549 rewrote into beat-direction format).


What this PR continues to get right

  • Scene Fragment Spec is the strongest piece of the stack. The explicit prohibited-patterns list (<!DOCTYPE, <script> wrapping the GSAP section, tl.from, gsap.timeline(, bare class names without s{N}- prefix, CSS transform on GSAP-animated elements) with inline "why this prohibition exists" comments is a model guardrail format.
  • "Who runs this pipeline" section correctly handles the nested-subagent case (no Agent/Task tool available → build serially) — prevents silent broken pipelines.
  • Persistent-subject choreography YAML addresses a real failure mode (subject scaling into reserved region without bbox awareness). The Vermeer 1.0x → 2.1x → 0.85x example is concrete enough to copy.
  • Streaming evaluation matches modern subagent orchestration practice.

Recommended path

  1. Either ship packages/core/src/assemble/ in this PR (with tests), or rewrite Phase 3 as a procedure that doesn't depend on a nonexistent module.
  2. Define R4 inline — one sentence is enough.
  3. Restack after #550/#549 are sorted.
  4. Renumber Phase 2b → Phase 2; generalize the YAML coordinates; pair the YAML with a scaffold-tween snippet.

@vanceingalls vanceingalls force-pushed the vance/04-28-content-culture branch 6 times, most recently from 129375a to 5cd19fe Compare April 30, 2026 00:49
@vanceingalls vanceingalls force-pushed the vance/04-28-persistent-choreography branch 3 times, most recently from 7e70c06 to a76f500 Compare April 30, 2026 03:56
@vanceingalls vanceingalls changed the base branch from vance/04-28-content-culture to graphite-base/551 April 30, 2026 03:58
@vanceingalls vanceingalls changed the base branch from graphite-base/551 to main April 30, 2026 03:58
@vanceingalls vanceingalls force-pushed the vance/04-28-persistent-choreography branch 6 times, most recently from 3dd063c to 845b40c Compare April 30, 2026 05:55
@vanceingalls vanceingalls force-pushed the vance/04-28-persistent-choreography branch from 845b40c to 321c309 Compare April 30, 2026 06:03
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.

2 participants