Repository navigation
feat(cli): parakeet ASR engine for transcribe + HYPERFRAMES_PYTHON override - #1970
Conversation
1ea25cd to
4d696ed
Compare
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
PR #1970 — R1 (runtime-interop lens)
4d696edb · +226/-17 across 10 files · Verdict at bottom
✅ The clean bits (dispatch chain traced end-to-end)
Elementary before the deductions: I traced the engine resolution from CLI arg to subprocess invocation, and the wiring holds under every reachable input.
- Engine parse gate:
packages/cli/src/commands/transcribe.ts:237-242normalizesopts.engineto lowercase and rejects anything outside{auto, parakeet, whisper}viafailWith, so the switch domain is closed before it reachesuseParakeet. - Auto path: line 243 evaluates
engine === "parakeet" || (engine === "auto" && !!findParakeet())— short-circuits correctly, and the fallback to whisper on auto-without-parakeet is the documented contract. - Forced-parakeet-missing: if a user passes
--engine parakeeton a machine with noparakeet-mlx,transcribeWithParakeetthrows with the exact install command (parakeet.ts:105-108). No silent no-op. - Token merge invariant:
mergeTokensToWordsinparakeet.ts:79-95is covered byparakeet.test.tsfor the two shapes that matter — leading-space start-of-word AND sentence boundary without a leading space (test 2:"Hi"→ new word," there"→ new word, both preserved). The final.filter((w) => w.text.length > 0)protects the pipeline from empty-string words.
🟡 Finding 1 — HYPERFRAMES_PARAKEET skips the version/health probe that HYPERFRAMES_PYTHON performs
packages/cli/src/tts/python.ts:16-27 wraps its env override in an execFileSync(override, ["--version"]) check that requires the output to match /Python 3/ before returning the path — a genuinely useful sanity gate. The new packages/cli/src/whisper/parakeet.ts:31-37 uses the same env-override pattern but only calls existsSync(path) before returning it. So if a user sets HYPERFRAMES_PARAKEET=/usr/bin/false (or any executable that isn't parakeet-mlx), we happily invoke it later at parakeet.ts:113 with a 30-minute timeout and surface whatever obscure crash message the wrong binary produces.
Not a security issue — the user chose the binary — but the asymmetry is worth a --version (or --help) probe here for parity. Non-blocking.
🟡 Finding 2 — --model is silently ignored under --engine parakeet
packages/cli/src/commands/transcribe.ts:259-263 calls transcribeWithParakeet(inputPath, dir, { language: opts.language, onProgress: ... }) and never forwards opts.model. The engine has its own DEFAULT_MODEL = "mlx-community/parakeet-tdt-0.6b-v3" (parakeet.ts:22) and ParakeetOptions.model (line 99) is defined but never populated from the CLI.
Meanwhile the --model flag help text at transcribe.ts:52-55 reads "Whisper model (default: ${DEFAULT_MODEL}). Options: tiny.en, base.en, ..." — no mention that it applies only to whisper. A user running hyperframes transcribe audio.mp3 --engine parakeet --model large-v3 will see the JSON output field "model": "parakeet-tdt-0.6b-v3" regardless, but no warning that their explicit flag was ignored.
Fix options:
- (a) Reject the combo up front:
if (useParakeet && opts.model) failWith("--model does not apply to Parakeet; the engine has a fixed 0.6b model.", ...) - (b) Amend the flag description to
"Whisper model (ignored under --engine parakeet). ..." - (c) Wire
opts.modelthrough toParakeetOptions.modelif you actually want parakeet model overrides.
Options (a) or (b) are cheapest. Non-blocking, but leaves a UX contract gap that will burn a support cycle.
🟡 Finding 3 — Subprocess stderr is captured but never surfaced
parakeet.ts:113 runs execFileSync(runner, argv, { stdio: ["ignore", "pipe", "pipe"], timeout: 1_800_000 }). On success, stdout/stderr are discarded. On failure, execFileSync throws with err.stderr populated, but the outer catch in transcribe.ts:280-306 only reads err.message — which for execFileSync errors is the terse "Command failed: <cmd>" line. The actual parakeet-mlx error text (missing model, torch import failure, GPU unavailable) never reaches the user.
Recommend either capturing stderr into the thrown Error's message, or logging it to a debug file the surfaced message can reference. Same pattern would help synthesize.ts too, but that's out of scope. Non-blocking for this PR; strong nudge to add before the CLI lands with real users. #1971-friendly follow-up.
⚠️ Finding 4 — First-run model download has no UX cue
parakeet-mlx downloads the ~600MB TDT weights on first invocation. transcribe.ts:257-258 calls spin?.start(\Transcribing with ${c.accent(label)}...`), then blocks in execFileSyncfor the entire duration. The spinner sits frozen on "Transcribing with Parakeet..." — a first-time user has no signal whether the process is downloading a model, running inference, or hung.onProgressexists but is only fired once (at start) insidetranscribeWithParakeet`.
Not a correctness bug — the process WILL complete — but consider surfacing a "First-run model download may take several minutes" nudge before the exec, or streaming subprocess stdout/stderr into onProgress. Non-blocking.
📎 Contract note for downstream (#1973)
transcribeWithParakeet returns { transcriptPath, wordCount, durationSeconds, speechOnsetSeconds: null } — note the hardcoded null for speechOnsetSeconds. Whisper populates this; parakeet does not. The stripBeforeOnset post-processing at transcribe.ts:270-278 guards on != null so it correctly skips under parakeet. Any media-use skill script that consumes the JSON output must not assume speechOnsetSeconds is populated — it is null under "engine": "parakeet".
Verdict
Approve pending author judgment on the four notes above — none are blockers. The dispatch chain is closed, the engine resolution is deterministic, execFileSync timeouts are set, and the token-to-word merge invariant is tested against the two shapes that matter. The HYPERFRAMES_PYTHON version probe is a nice touch that the parakeet twin should adopt for symmetry, and the --model silent-ignore under parakeet will bite the first user who reads the help text literally, but neither warrants holding the stack.
Elementary. LGTM with the caveats logged for follow-up.
Review by Via (runtime-interop lens)
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
APPROVE — part of the 5-PR media-use stack (#1969–#1973), two-lens verification. Via (runtime-interop) + a test/observability pass both clear all five, no blockers.
Verified:
- Headline "addressed" items confirmed: #1972's orphan
hyperframes-mediastrings fixed at both call sites (sfx.mjs:125,tts.mjs:184; the only surviving ref is SKILL.md's historical changelog line, intentional); #1973's atomic writes in place on both entry points (transcribe.mjstemp+renameSync,transcript-cut.mjsencode-to-.part+rename), and the parakeet PATH→venv fallback (resolveParakeet()) is resolved. - Test coverage solid: #1969
mutedParity.test.ts(all mute combos), #1970parakeet.test.ts(token-merge boundaries), #1971 crop/affordance/asset-strip tests, #1973 ~0.8 test:source ratio across 4 new files.
Non-blocking follow-ups (Via-accepted nits, worth tracking, NOT gating): #1970 observability (--model ignored on the parakeet path, subprocess stderr discarded on success, no first-run model-download cue, HYPERFRAMES_PARAKEET skips the --version gate); #1973 --copy keyframe-drift warning only on stderr, not in --json.
Merge via the normal Graphite path (bottom-up, no admin-merge). Layered on Via.
— Rames Jusso
4d696ed to
87c97b6
Compare
c703641 to
55c962a
Compare
87c97b6 to
05fb087
Compare
05fb087 to
8c3590a
Compare
3 participants
Adds NVIDIA Parakeet-TDT as a
hyperframes transcribe --engine parakeet|whisper|autoengine (auto = Parakeet if installed, else whisper.cpp). Beats whisper.cpp on WER + speed. Plus aHYPERFRAMES_PYTHONoverride for local-model python.Stack: media-use v2 (2/5). Depends on the producer PR below.
📚 Stack — media-use v2 (split from #1923, merge bottom-up)