fix(skills): align TTS docs with CLI contract#2483
Conversation
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
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.
vanceingalls
left a comment
There was a problem hiding this comment.
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 newdescribe("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 everydefineCommand({args})acrosspackages/cli/src/commands/*.tsand grepsskills/**/*.mdforhyperframes <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, butsnapshotdeclaresoutputwith aliasoonly (nooutalias); the flag is--output/-o.ttsis 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_KEYbut 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.mjsexists at head; the newtts-to-captions.mdandtts.mdinvocations 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
Summary
--providerand--wordson the published Kokoro-onlyhyperframes ttscommandCurrent reproduction
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 passingoxfmt --checkon all changed files