Skip to content

fix(skills): align TTS docs with CLI contract#2483

Open
miguel-heygen wants to merge 1 commit into
mainfrom
fix/tts-docs-cli-contract
Open

fix(skills): align TTS docs with CLI contract#2483
miguel-heygen wants to merge 1 commit into
mainfrom
fix/tts-docs-cli-contract

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • stop advertising --provider and --words on the published Kokoro-only hyperframes tts command
  • route cloud-provider and word-timestamp examples through the bundled HeyGen helper
  • pin the CLI/docs contract with focused tests

Current reproduction

npx --yes hyperframes@0.7.58 tts --provider kokoro --text test
Error: Unknown flag: --provider

The installed TTS reference currently documents that flag. Removing it succeeds, matching the reporter's Windows workflow.

Refreshes the exact closed backlog fix from #2296 after the requested new field reproduction made it current again. GitHub would not reopen the closed PR after its branch was refreshed, so this PR carries the same scope rebased on current main.

Verification

  • coreSkillContent.test.ts: 3/3 passing
  • skills manifest regenerated
  • oxfmt --check on all changed files
  • diff whitespace check

@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 b465ccf.

Docs align with the shipped CLI: hyperframes tts is Kokoro-only and doesn't accept --provider or --words. Cloud-provider + word-timestamp examples reroute through the bundled heygen-tts.mjs REST helper. Contract pinned by focused test on tts.md + tts-to-captions.md.

Blockers

(none)

Concerns

(none)

Nits

(none of material significance)

Green notes

🟢 Primary-source verification against packages/cli/src/commands/tts.ts — the defineCommand args block at lines 43-83 declares only input, text-file, output, voice, speed, lang, list, json. No --provider, no --words. The meta description is literally "Generate speech audio from text using a local AI model (Kokoro-82M)". Docs claim matches source. ✓

🟢 Regex regression pins are well-scoped. /hyperframes tts[^\n]*--provider/ matches only lines that mention hyperframes tts and --provider on the SAME line. Verified via grep: post-PR, zero matches for that pattern in skills/ or docs/. Other legitimate --provider uses in the repo (skills/media-use/scripts/resolve.mjs --provider, skills/product-launch-video/SKILL.md's audio.mjs --provider, skills/media-use/SKILL.md referring to the resolver's flag) are correctly untouched by the regex. ✓

🟢 Red-first shape verified. Origin/main's tts.md has hyperframes tts "Hello" --provider kokoro — the new regex assertions would fail there, pass here. Real regression pin.

🟢 heygen-tts.mjs bundled-helper flags are unchanged--words, --voice, --speed, --lang, --output still documented at tts.md:45-53. The rewrite correctly repoints readers from the (non-existent) hyperframes tts --words to node skills/media-use/audio/scripts/heygen-tts.mjs --words. ✓

🟢 script-format.md To-TTS section now correctly names both paths (hyperframes tts for Kokoro; bundled helper for word timestamps) with a pointer to tts.md for the contract. No orphaned "--provider from the header" reference remaining.

🟢 The ## Provider chain## Available routes rename matches the new reality: the CLI itself no longer picks a provider; it's Kokoro-only, and cloud providers are opt-in via separate helpers. Table content unchanged (still lists HeyGen / ElevenLabs / Kokoro as the three routes) — appropriate scope.

🟢 Skills-manifest hashes updated for hyperframes-core and media-use — the two skills whose reference files changed. gen-skills-manifest.ts --check would flag any drift; PR notes it was regenerated by hook.

🟢 --speed reads as Kokoro-only in the updated pacing note: "The hyperframes tts command honors --speed for Kokoro. Provider-specific helpers document their own pacing controls." Matches source: tts.ts:65-69 declares --speed on the CLI; heygen-tts.mjs handles pacing via HeyGen API options separately. ✓

🟢 CI green at b465ccf.

Verdict framing

Docs are now correct, contract is pinned by test, backlog-refresh scope kept clean. LGTM from my side.

Review by Rames D Jusso

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at b465ccf (R1) — lens-split with Rames covering primary-source arg verification, regex pin scoping, and CI.

Position

RIGHT. The field signal — hyperframes@0.7.58 tts --provider kokoro returning Unknown flag — is a docs bug, not a CLI bug. Verified via packages/cli/src/commands/tts.ts at head: defineCommand.meta.description is literally "Generate speech audio from text using a local AI model (Kokoro-82M)", and the args block declares only input, text-file, output, voice, speed, lang, list, json. synthesize() imports from ../tts/synthesize.js (Kokoro path only) — there is no provider dispatch to add --provider to. The intentional design is CLI = Kokoro-only + scripts/heygen-tts.mjs = HeyGen REST. This PR aligns the docs with that reality; it does not paper over a CLI regression.

Prior-reviewer delta

Rames covered defineCommand args, regex regression scope, red-first shape, bundled-helper flag preservation, --speed mapping, manifest hashes, and CI. Nothing to overturn. My lens adds two follow-up-scoped items he didn't flag.

Concerns

(none blocking — PR is scoped tightly and correct)

Nits / follow-ups

  • 🟡 Regression test is TTS-scoped only; no general docs↔CLI parity harness. coreSkillContent.test.ts's new describe("media-use TTS documentation") block hard-codes the two files (tts.md, tts-to-captions.md) and two flag names (--provider, --words). A general harness that iterates every defineCommand({args}) across packages/cli/src/commands/*.ts and greps skills/**/*.md for hyperframes <cmd> ... --<flag> fences would catch future drift without requiring a per-flag test. Follow-up, not a blocker for this PR.

  • 🟡 Extrapolation found one adjacent drift, out of this PR's scope. skills/talking-head-recut/SKILL.md:1175 — an inline # comment says # a single --at ignores --out, but snapshot declares output with alias o only (no out alias); the flag is --output / -o. tts is the file that has the ["o","out"] alias pair, so this looks like a mid-write cross-command confusion. Doesn't need to land here, but worth a separate one-line fix or a general parity harness (see above).

  • 🟢 "Available routes" table lists ElevenLabs with env trigger $ELEVENLABS_API_KEY but the surrounding prose only offers a bundled helper for HeyGen. The updated tts.md removes the CLI's ElevenLabs path (correctly — the CLI never had one on this branch) but leaves the table as an ecosystem summary. Not misleading given the header rename to "Available routes" and the prose above it, but a future reader chasing the ElevenLabs row will find no matching runner in this repo. Consider adding a "helper / integration" column, or a footnote — non-blocking.

Green notes (non-overlapping with Rames)

  • 🟢 Bundled helper reachability verified. skills/media-use/audio/scripts/heygen-tts.mjs exists at head; the new tts-to-captions.md and tts.md invocations are runnable, not aspirational.

  • 🟢 Field-signal → fix-direction calibration. The remove-flag direction is correct precisely because the CLI meta advertises Kokoro-only. If the CLI had ever advertised multi-provider and regressed to Kokoro, the fix would be inverted (restore provider dispatch). Not this case.

  • 🟢 Refresh-scope hygiene. PR body cites #2296 as the closed original; diff is byte-close to a rebase of that scope. No creep, no unrelated churn.

Verdict

LGTM on the shipped scope. Two follow-ups above are new work, not blockers.

Review by Via

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.

3 participants