fix(heygen-translate): auth gate, duration question, open-ended language input#86
Conversation
…age input Three improvements from dogfooding: - Add auth verification step before Phase 1: runs `heygen auth status` in CLI mode, asks for API key and persists via `heygen auth login` if missing. One-time setup that survives across sessions. - Add duration flexibility question to Phase 1 discovery: asks whether output must match source length, explains quality tradeoff, controls `enable_dynamic_duration` flag instead of hardcoding true. - Make target language question explicitly open-ended: no picker, no pre-assigned choices. User types freely, validation in Phase 2. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
eve-builds
left a comment
There was a problem hiding this comment.
Review
Solid dogfooding PR — all three changes earn their place. Reviewed against feat/heygen-translate, verified heygen auth status / heygen auth login exist as the PR uses them (v0.0.6), and read the surrounding doc context.
What I like
- Auth gate placement is correct. Sitting between mode detection and Phase 1 is the right spot — Lesson #21 (educate-don't-evaluate) territory: surface the failure mode early instead of letting it bite at submit time. The "one-time setup, survives across sessions" framing matters because otherwise agents will re-prompt every run.
- MCP carve-out ("auth is handled by OAuth — no check needed") prevents the agent from running CLI commands while in MCP mode. Good single-transport hygiene.
- Duration flexibility as a question, not a hardcoded default. This is the right call. The framing in the question itself ("translated speech gets enough room to be spoken at a comfortable pace") teaches the user why — that's the skill encoding expertise, not just collecting an input.
- Open-ended language input. Explicit "do NOT present a picker" rule is exactly right for this surface. Pickers force premature schema validation; free text + silent Phase 2 validation is the cleaner UX. Also future-proofs against regional variant requests like "Spanish (Mexico)" that a picker would either miss or over-engineer.
Minor — not blocking
There are two lines that now technically conflict with the new "ask the user" flow:
heygen-translate/SKILL.md:335— "Alwaysenable_dynamic_duration: trueso the dub doesn't feel rushed or stretched."heygen-translate/references/language-locale-guide.md:49— "Alwaysenable_dynamic_duration: truefor visual translation."
Both predate the duration question and now read as contradicting Phase 1 item #6. Suggest softening to "Default to enable_dynamic_duration: true unless the user explicitly needs fixed-length output" in both places. Happy to land that as a follow-up if you'd rather keep this PR scoped — but it's a 2-line tweak and avoids future agent confusion about which line wins.
Nit (optional)
The auth-failure prompt embeds https://app.heygen.com/settings?nav=API, but the CLI's heygen auth --help output points users to https://app.heygen.com/settings/api. Both work, but matching the CLI's canonical URL is one less surface for drift.
Verdict
Approving. The two Always … lines are real but cosmetic — won't break the skill, and enable_dynamic_duration: true remains the recommended default per Phase 1 item #6 so the runtime behavior is unchanged.
… question SKILL.md:335 and references/language-locale-guide.md:49 both said "Always enable_dynamic_duration: true", contradicting the new Phase 1 duration flexibility question. Updated both to reference the user's choice and warn about quality degradation on high-compression pairs when fixed-length is chosen. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
eve-builds
left a comment
There was a problem hiding this comment.
Re-review (a71a3e0)
Both flagged lines fixed cleanly:
SKILL.md:335now reads as "Dynamic duration (the Phase 1 duration flexibility question) is especially important for these pairs" — references the new question instead of contradicting it. ✅references/language-locale-guide.md:49now says "Recommendenable_dynamic_duration: truefor visual translation" instead of "Always." ✅
Bonus catch I didn't ask for: both locations now also include a fixed-length warning — "If they choose fixed-length on a high-compression pair (en→zh, en→ja), warn that quality will degrade." Turns a passive footnote into an active runtime guardrail. That's exactly the right shape for this skill (Lesson #21: encode expertise as questions + warnings, not silent agent inference).
Diff is otherwise unchanged from the first pass. Approval stands.
(URL nit — ?nav=API vs /api — still there but it was optional, doesn't break anything.)
Ship it. 🚢
Summary
Three improvements from dogfooding the translate skill:
heygen auth statusbefore Phase 1 in CLI mode. If auth fails, asks for the API key and persists viaheygen auth login. One-time setup, survives across sessions.enable_dynamic_durationinstead of hardcodingtrue.Test plan
HEYGEN_API_KEYset and no~/.heygen/credentials— should ask for the key before any discovery questionsenable_dynamic_durationrespects the answer🤖 Generated with Claude Code