fix(skills): resolve the blueprint id from a qualified blueprint: field - #3337
Open
miguel-heygen wants to merge 3 commits into
Open
fix(skills): resolve the blueprint id from a qualified blueprint: field#3337miguel-heygen wants to merge 3 commits into
blueprint: field#3337miguel-heygen wants to merge 3 commits into
Conversation
…ield visual-design.md documents `blueprint:` as the id plus a `(Reproduce)` / `(Adapt)` qualifier, and prints `dataviz-countup (Adapt)` as its worked example. The packet builder used that raw field as a filename, so a qualified blueprint looked for `<id> (Adapt).md`, found nothing, and inlined an empty string: `selectedFile()` returns "" for a missing path. Every packet shipped without the one document the frame was designed against, and the run still exited 0 with nothing on stderr. `compose (Adapt)` missed the `compose` check the same way. Parse the field into the id it names, once, so no caller resolves a raw field value against the blueprints directory. A blueprint that resolves to no file is now a named error rather than an empty section, matching how the builder already treats a missing `src` and an oversize packet. The existing tests only used bare ids, which is how the qualified form escaped; they now cover both, and the missing-file case. One owner: product-launch-video, faceless-explainer, pr-to-video and general-video all delegate to frame-packets-core.mjs. Co-Authored-By: anikam13 <22992075+anikam13@users.noreply.github.com>
Self-review catch on the previous commit. hyperframes-animation installs on demand, so its blueprints/ directory can legitimately be missing — that is a skill that isn't installed yet, not a frame naming a bad id. Throwing there turned a silent degrade into a hard failure for a valid setup. Distinguish the two: an absent blueprints/ warns and inlines nothing, exactly as an absent rules/ already does in knownRuleIds; a present library that has no file for this id still throws, because that is a typo or an unstripped qualifier. Co-Authored-By: anikam13 <22992075+anikam13@users.noreply.github.com>
CI surfaced these once an unresolvable blueprint stopped being silent. Both named ids that have never existed in hyperframes-animation/blueprints/: - faceless-explainer's frame template taught `messaging-multi-phase`, so an agent copying the template verbatim tagged a blueprint that resolves to nothing. dataviz-countup is what the same skill already uses in its own visual-design template and tests. - pr-to-video's diff-excerpt guardrail fixture used `number-lockup`. The test is about diff excerpting and the id was incidental; the frame's own `counting-dynamic-scale` rule makes dataviz-countup the natural real shape. A sweep of every `blueprint:` value across skills/ finds no others. Co-Authored-By: anikam13 <22992075+anikam13@users.noreply.github.com>
miga-heygen
previously approved these changes
Aug 19, 2026
miga-heygen
left a comment
Contributor
There was a problem hiding this comment.
Approve — parsing is correct. The regex \s*\([^)]*\)\s*$ strips trailing parenthesised qualifiers cleanly; compose null-return is case-insensitive; the absent-library vs bad-id split mirrors the existing knownRuleIds pattern. Four test cases cover bare, qualified, compose-qualified, missing-file-throws, and uninstalled-library-warns — comprehensive. Dead reference fixes (messaging-multi-phase → dataviz-countup, number-lockup → dataviz-countup) are confirmed valid by the sweep. No issues.
— Miga
miga-heygen
dismissed
their stale review
August 19, 2026 20:28
Stamp removed — was posted prematurely
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
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.
What
The frame-packet builder now parses the
blueprint:field into the blueprint id it names, instead of using the raw field as a filename. A blueprint that resolves to no file is a named error rather than a silently empty section.Why
references/visual-design.mddocumentsblueprint:as the id plus a(Reproduce)/(Adapt)qualifier, and printsdataviz-countup (Adapt)as its worked example. The resolver passed that raw value straight tojoin(animationDir, "blueprints",${blueprint}.md), so a qualified blueprint looked for<id> (Adapt).md. That file cannot exist, andselectedFile()returns""for a missing path.The result: every packet shipped without the one document the frame was designed against, and the run still exited 0 with nothing on stderr. On a storyboard whose only difference was the documented qualifier, packets dropped from 17836 bytes to 648 bytes with no
## Selected blueprintsection, and the CLI still printed✓ frame packets: 1 bounded packet(s).compose (Adapt)missed thecomposecheck the same way, so it went looking forcompose (Adapt).md.This affects
product-launch-video,faceless-explainer,pr-to-videoandgeneral-video, which all delegate to the shared builder.How
blueprintId(block)parses the field once, so no caller resolves a raw field value against the blueprints directory. The qualifier is direction for the frame worker, not part of the filename.A blueprint with no file now throws, naming the frame, the id and the path. That matches how the builder already treats a missing
srcand an oversize packet, and it means a typo fails the run instead of quietly producing a packet with no motion guidance.The docs are correct as written and are unchanged; the resolver was the side that was wrong.
Test plan
Three cases added to
frame-packets.test.mjs: a qualified id resolves to a byte-identical blueprint body across(Adapt),(Reproduce)and the bare form; a qualifiedcomposestill selects nothing; a blueprint with no file throws and leaves no partial output.The existing tests used bare ids only, which is how the documented form escaped them. Verified the two new behavioural cases fail on the parent commit and pass here.
Review notes
Two follow-up commits, both from making the failure visible.
1. Absent library vs bad id. Throwing on any unresolvable blueprint was too broad.
hyperframes-animationinstalls on demand, so an absentblueprints/directory is a skill that isn't installed yet, not a frame naming a bad id. The second commit splits them: an absent library warns and inlines nothing, exactly as an absentrules/already does inknownRuleIds; a present library with no file for this id still throws.2. Two dead blueprint references, surfaced by CI. The new error caught two ids that have never existed in
hyperframes-animation/blueprints/:skills/faceless-explainer/references/story-design.md— the frame template taughtmessaging-multi-phase, so an agent copying it verbatim tagged a blueprint that resolves to nothing. This is the bug reaching users through the docs, not just a fixture.skills/pr-to-video/scripts/workflow-guardrails.test.mjs— the diff-excerpt fixture usednumber-lockup.Both now point at
dataviz-countup, which the faceless-explainer skill already uses in its own visual-design template and tests, and which matches the pr-to-video frame's owncounting-dynamic-scalerule. A sweep of everyblueprint:value acrossskills/finds no others.