feat(telemetry): attribute renders to the authoring workflow skill

[kiritowoo]

Add an optional --skill flag to hyperframes render and tag the render_complete / render_error events with authoring_skill, so render usage can be broken down per authoring workflow. The value is slug-gated (a malformed value is ignored) and the existing anonymous / opt-out telemetry pipeline is otherwise unchanged.

Each end-user workflow that renders now passes --skill=<name> on its render command: embedded-captions, faceless-explainer, graphic-overlays, motion-graphics, music-to-video, pr-to-video, product-launch-video, remotion-to-hyperframes, website-to-video.

Not instrumented, by design: general-video renders freeform with no canonical render command to attach to, and slideshow produces an interactive deck rather than a rendered video. Both can follow up if per-skill numbers are wanted.

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)

[miga-heygen]

Review: feat(telemetry): attribute renders to the authoring workflow skill

Clean, well-scoped change. The slug-gated --skill flag is a smart approach — keeps the anonymous telemetry pipeline safe from PII / high-cardinality strings while giving you per-workflow breakdowns.

What I checked

1. All three render paths instrumented — renderOptionsBase (chunked/streaming), renderDocker, and renderLocal all thread authoringSkill through. Error path via handleRenderError and success path via trackRenderMetrics both emit authoring_skill. No missed path.

2. Slug validation — /^[a-z0-9][a-z0-9-]{0,63}$/ is tight: lowercase + digits + hyphens, max 64 chars, can't start with a hyphen. Invalid values degrade to undefined (field omitted from the event), which is the right silent-fallback for telemetry. No crash, no empty string leaking in.

3. Skills instrumented — All 9 render-producing skills updated: embedded-captions, faceless-explainer, graphic-overlays, motion-graphics, music-to-video, pr-to-video, product-launch-video, remotion-to-hyperframes, website-to-video. The two exclusions (general-video: freeform, slideshow: interactive deck) are documented in the PR body — makes sense.

4. Telemetry event mapping — Both trackRenderComplete and trackRenderError map authoringSkill → authoring_skill on the emitted event object. Consistent snake_case in the payload, camelCase in TS — matches the existing convention.

5. CI — All 30 checks green.

Minor nit (non-blocking)

The shell script in embedded-captions/scripts/render-and-composite.sh uses the space-separated flag format (--skill embedded-captions) while all SKILL.md files use equals-sign format (--skill=faceless-explainer). Both parse identically with citty, so purely cosmetic — but picking one convention for grep-ability would be nice.

LGTM — ship it. 🚢

— Review by Miga

[james-russo-rames-d-jusso]

Verified at HEAD 08d25918.

Layering on Miga's review (posted ~5 min ago) — he already covered the three render paths, slug regex tightness, 9 skills + 2 documented exclusions, and the snake_case/camelCase mapping. All confirmed independently. Adding a few cross-cutting concerns the field-level read missed.

✅ What I confirmed

• authoringSkill plumbed through renderOptionsBase (render.ts:774), renderDocker (render.ts:832, 1195), renderLocal (render.ts:858), handleRenderError (render.ts:1435), and trackRenderMetrics (render.ts:1481). Both success and failure paths emit authoring_skill — important since render_error is exactly the cohort we'd want per-skill breakdowns on.
• Regex ^[a-z0-9][a-z0-9-]{0,63}$ correctly accepts all 9 instrumented slugs and rejects empty / whitespace / uppercase / underscore / unicode / leading-dash / overlong. Trailing-dash IS accepted (trailing- → true) — minor and not exploitable through anonymous telemetry, just noting.
• The 9 callers all pass the slug matching their directory name. Verified each.

🟡 Concerns (non-blocking)

1. Studio renders silently emit authoring_skill: null. packages/cli/src/server/studioRenderTelemetry.ts:106,125 (emitStudioRenderError / emitStudioRenderComplete) doesn't accept or forward an authoring skill — by design, studio renders aren't skill-driven. But this means downstream PostHog cohorts on authoring_skill IS NULL will mix two populations: (a) CLI renders where the caller didn't pass --skill (hyperframes-cli SKILL.md, hyperframes-core SKILL.md, hyperframes-registry/references/contributing.md all show un-flagged npx hyperframes render invocations to LLMs), and (b) all studio renders. The source: "cli" | "studio" tag disambiguates if the dashboard remembers to filter on it. Worth a one-line callout in the PR body so the consumer dashboard author knows the partition.

2. SKILL_SLUG regex duplicated between events.ts:23 and render.ts:394. Same pattern, copy-pasted. Two divergences already: events.ts does args.skill.trim() before the regex, render.ts does not — so --skill=" pr-to-video " is accepted by hyperframes events but rejected by hyperframes render. Shell callers won't hit this in practice, but it's a real semantic drift. Cheap fix: extract SKILL_SLUG + normalizeSkillSlug() into a shared util in telemetry/ (or wherever redactTelemetryString lives).

3. Cross-repo verification. authoring_skill is a new top-level property on render_complete / render_error. PostHog ingest is permissive (no schema validation), so the field will land — but is there a Datadog dashboard / PostHog insight / warehouse query already referencing this property that needs to be updated, or is this the first consumer wire-up? If new, a follow-up to land the cohort/dashboard would close the loop. (Not blocking — the field is safe to ship and accumulate before a consumer reads it.)

🟡 Nits

• PR body template placeholders. The What / Why / How / Test plan sections are template defaults ("Brief description of the change.", "Why is this change needed?", etc.). The top-paragraph summary you wrote is great and stands on its own — either drop the template scaffold or fill in even a one-liner per section, so future readers don't bounce off boilerplate.
• embedded-captions space-vs-equals form — already flagged by Miga. Agree, low-priority.
• --skill not in the curated examples table at help.ts:96-49. Citty will still surface it under hyperframes render --help, so end users can discover it. Skill-side callers wire it themselves so no functional impact. Optional.

What I didn't verify

• Whether any in-flight PostHog dashboard / cohort already filters on authoring_skill (would need to check PostHog directly, out-of-repo).
• Whether general-video and slideshow have render-invoking dev scripts I didn't enumerate that should opt-in later.

Nice scope and clean instrumentation. Approving in spirit — leaving as comment-state since I want the studio-vs-cli null partition called out in either the PR body or a follow-up.

— Review by Rames D Jusso

[miguel-heygen]

Verified current head 08d25918 after Miga and Rames D's passes.

Specific checks I repeated:

• packages/cli/src/commands/render.ts slug-gates --skill before it enters telemetry, so malformed/high-cardinality values are omitted instead of emitted.
• The value is threaded through batch, Docker, and local render paths, and both success + error telemetry paths carry it.
• packages/cli/src/telemetry/events.ts emits authoring_skill on render_complete and render_error without changing the existing opt-out/anonymous tracking gates.
• I grepped remaining hyperframes render references; the untagged ones are generic docs/test-corpus examples or the documented non-video/general exclusions, not canonical render-producing workflow entrypoints.

Agree with the existing follow-ups: shared slug normalizer, dashboard/source partitioning for authoring_skill IS NULL, and cleanup of the PR template scaffold would all be useful. None changes the merge safety of this diff. CI is green.

Verdict: APPROVE
Reasoning: The telemetry field is tightly slug-gated, wired through all render execution outcomes, and every canonical render-producing skill command is attributed without changing render behavior or telemetry opt-out semantics.

— Magi

[miga-heygen]

Re-review after cbd5a3df

All prior feedback addressed — this is tighter than v1. Here's what changed and what I verified:

Fixes applied

1. Shared slug utility (telemetry/skill.ts) — extracted SKILL_SLUG regex and normalizeSkillSlug() into a shared module. events.ts now imports SKILL_SLUG instead of defining its own copy, and render.ts calls normalizeSkillSlug() instead of inline regex. Single source of truth — no more drift risk between the two commands.

2. Stderr warning on invalid slugs — If someone passes --skill=MotionGraphics (camelCase typo), the render still proceeds but writes a diagnostic to stderr: "ignoring --skill=... — not a valid slug ... this render will be unattributed." Surfaces the mistake instead of silently losing attribution. Good DX.

3. Shell script flag format — embedded-captions/scripts/render-and-composite.sh switched from --skill embedded-captions (space-separated) to --skill=embedded-captions (equals-sign). Consistent with all the SKILL.md files now.

4. Test coverage — skill.test.ts covers: valid slugs (including boundary 64-char), whitespace trimming, all invalid value classes (empty, uppercase, underscore, leading hyphen, 65-char, unicode, non-string types). Clean.

Verification

• normalizeSkillSlug trims before validating — handles shell quoting artifacts. Returns undefined for non-string input, so normalizeSkillSlug(undefined) when --skill is omitted just drops the field. Correct.
• The stderr warning only fires when the raw input is a non-empty string AND normalizeSkillSlug returns undefined — won't spam when --skill is simply not passed. Correct.
• All three render paths + error path still thread authoringSkill through. Unchanged from v1.
• CI: most checks green, a few still pending (Windows tests, CLI smoke, global install). No failures.

Still LGTM. No new concerns.

— Review by Miga

[james-russo-rames-d-jusso]

R2 verification at cbd5a3df

Layering on Miga's R2 (posted ~5 min ago) — they covered the three fixes (shared util, stderr warning, equals-form script, test coverage) and verified all render paths still thread authoringSkill. Confirmed independently. Per-R1-finding ledger below.

Per-R1-finding verdict

• 🟡 #1 Studio renders mixed with un-flagged CLI in authoring_skill IS NULL cohort — ✅ resolved-already, my R1 miss. Re-grepping at HEAD: render.ts:1363 emits source: "cli", studioRenderTelemetry.ts:110,131 emit source: "studio". The partition I worried about is already enforced everywhere — any dashboard that breaks down authoring_skill IS NULL can additionally filter on source to disambiguate the two populations. No code change needed here; my R1 read missed the existing tag.
• 🟡 #2 SKILL_SLUG regex duplicated + trim divergence — ✅ resolved. packages/cli/src/telemetry/skill.ts is the new single source of truth; events.ts:3 and render.ts:67 both import from it. render.ts:395 now calls normalizeSkillSlug(args.skill) which trims internally. events.ts:47 still trims locally before calling SKILL_SLUG.test() — semantically identical to normalizeSkillSlug so no observable drift, just one redundant call site. Non-blocking polish: events.ts could also call normalizeSkillSlug for one fewer line, but the regex divergence risk is eliminated either way.
• 🟡 #3 Cross-repo PostHog dashboard / Datadog insight wired up? — ⚠️ open, non-blocking. No follow-up note in PR body, commit message, or thread. Was a question in R1 not a blocker; if there's already a consumer dashboard waiting on this property, fine — if not, would be worth a JIRA/Linear handoff before this lands.
• Nit a — PR body template placeholders — ⚠️ partial. Top-paragraph summary still excellent. What/Why/How/Test plan sections still carry boilerplate ("Brief description of the change.", "Why is this change needed?", etc.). Future-reader nit, not a blocker.
• Nit b — embedded-captions equals-form — ✅ resolved. skills/embedded-captions/scripts/render-and-composite.sh:259,261 now --skill=embedded-captions. Convention consistent with all SKILL.md files.
• Nit c — --skill not in render.ts curated examples — ❌ untouched. The examples array at packages/cli/src/commands/render.ts:15 still has no --skill example. Optional; citty exposes the flag under --help regardless.

🆕 R2-new observations

• Stderr warning for invalid slugs (render.ts:396-403) is a nice DX upgrade — a typo like --skill=MotionGraphics now prints ignoring --skill="..." — not a valid slug ... this render will be unattributed to stderr instead of silently losing attribution. The guard correctly fires only when raw input is a non-empty string and normalization dropped it, so no spam when --skill is simply omitted.
• skill.test.ts covers valid slugs, boundary 64-char, trim, and the right invalid classes (empty, whitespace-only, uppercase, underscore, leading hyphen, 65-char, unicode, non-string). Clean.
• Verified --skill=" pr-to-video " no longer diverges between hyperframes render (now trims via normalizeSkillSlug) and hyperframes events (still trims locally before the same shared regex). Both accept it now.

CI

Most checks green at cbd5a3df. A few still in-progress (Windows tests, CLI smoke, global install, CodeQL JS) — no failures visible.

Posture

LGTM at cbd5a3df. Three of three 🟡 closed (one was my misread, one resolved cleanly, one open but explicitly non-blocking). Magi's prior approval at 08d25918 may or may not survive the push depending on branch protection — deferring stamp to Magi/Miga for a clean re-approve if needed.

— Review by Rames D Jusso

[miguel-heygen]

R2 verified at current head cbd5a3df, after the review-fix commit.

Strengths / checks:

• packages/cli/src/telemetry/skill.ts centralizes SKILL_SLUG and normalizeSkillSlug, so events and render no longer carry duplicated regexes or trim drift.
• packages/cli/src/commands/render.ts warns on a non-empty invalid --skill while still allowing the render to continue unattributed, preserving telemetry-never-breaks-render behavior.
• packages/cli/src/telemetry/skill.test.ts pins valid, trimmed, invalid, boundary-length, unicode, and non-string cases.
• skills/embedded-captions/scripts/render-and-composite.sh now uses the same --skill=embedded-captions form as the SKILL.md call sites.

Layering on Miga/Rames D/wanbi: their R2 findings are addressed or correctly classified as follow-ups. The remaining dashboard consumer and help/example cleanup are non-blocking. gh pr checks is green at this head, including Windows render/tests.

Verdict: APPROVE
Reasoning: The attribution path is now slug-gated through one shared helper, every render success/error path still emits the safe authoring_skill value, invalid inputs are visible to callers without failing renders, and CI is green.

— Magi