feat(skills): instrument general-video renders with --skill attribution#2424
Open
kiritowoo wants to merge 1 commit into
Open
feat(skills): instrument general-video renders with --skill attribution#2424kiritowoo wants to merge 1 commit into
kiritowoo wants to merge 1 commit into
Conversation
general-video was the only major workflow skill without render attribution (the Jun 24 instrumentation pass missed it) -- its usage was visible only through agents spontaneously adding the flag, undercounting a skill that is now among the top two by volume. Adds a dedicated 'Render (only on explicit request)' section with the exact command, matching the pattern of the other workflow skills, and steers away from the scaffold's npm run render (which carries no attribution). Live-tested with headless agent runs on an observed machine (transcript ground truth): a checklist-only placement was NOT sufficient (agents reproduce the command shape but drop non-functional flags); the dedicated section with 'use exactly this command' wording carries the flag at the same rate as the other instrumented skills. Note for future skill testing: the self-update preamble pulls main and overwrites locally modified copies -- strip it in test deployments.
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 4933d436.
Small, focused docs change. Verified the mechanism claims: the new section's npx hyperframes render --skill=general-video --quality high --output renders/video.mp4 shape matches three sibling workflow skills verbatim (faceless-explainer, pr-to-video, product-launch-video), and the --skill=<slug> flag is real — normalized via normalizeSkillSlug at packages/cli/src/commands/render.ts:432 and warned-and-ignored if the value isn't a valid slug, so general-video will land as-is on telemetry.
Concerns
Skills: manifest in syncCI check is failing.skills-manifest.jsonis not updated in the diff — SKILL.md content changed but the manifest wasn't regenerated. Blocks merge until the manifest sync is caught up (usually a one-command regen; check the existingregenerate-manifestscript or whatever the skills package uses).
Nits
- Framing check on the PR description: "general-video was the only major workflow skill without render attribution." Grepping the workspace, other skills without
--skill=attribution in their SKILL.md includeslideshowandwebsite-to-video— though those don't have anynpx hyperframes rendercommand at all in their SKILL.md, so agents using them wouldn't have a canonical shape to attach the flag to. Reads like the framing you're going for is "the only skill with substantial render volume where the flag was missing from the canonical command", which matches your "top two by volume" claim. Not a blocker, just noting the exact phrasing narrows more than it looks. - The self-update-preamble strip-in-test note in the commit message is a genuinely useful debugging breadcrumb — is there a good place to persist that in a testing README so the next skill-author doesn't rediscover it? (Not for this PR.)
What I didn't verify
- Whether the "checklist-only vs dedicated section" behavioral difference on headless agents holds up in Wenbo's transcript ground truth — taking the observation as reported.
LGTM aside from the manifest sync CI.
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.
general-video was the only major workflow skill without render attribution (the Jun 24 instrumentation pass missed it) -- its usage was visible only through agents spontaneously adding the flag, undercounting a skill that is now among the top two by volume.
Adds a dedicated 'Render (only on explicit request)' section with the exact command, matching the pattern of the other workflow skills, and steers away from the scaffold's npm run render (which carries no attribution).
Live-tested with headless agent runs on an observed machine (transcript ground truth): a checklist-only placement was NOT sufficient (agents reproduce the command shape but drop non-functional flags); the dedicated section with 'use exactly this command' wording carries the flag at the same rate as the other instrumented skills. Note for future skill testing: the self-update preamble pulls main and overwrites locally modified copies -- strip it in test deployments.
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?