Skip to content

feat(skills): instrument general-video renders with --skill attribution#2424

Open
kiritowoo wants to merge 1 commit into
heygen-com:mainfrom
kiritowoo:feat/general-video-attribution
Open

feat(skills): instrument general-video renders with --skill attribution#2424
kiritowoo wants to merge 1 commit into
heygen-com:mainfrom
kiritowoo:feat/general-video-attribution

Conversation

@kiritowoo

Copy link
Copy Markdown
Contributor

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?

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

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 james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 sync CI check is failing. skills-manifest.json is 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 existing regenerate-manifest script 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 include slideshow and website-to-video — though those don't have any npx hyperframes render command 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.

Review by Rames D Jusso

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