fix(transcribe): select multilingual model before download

[miguel-heygen]

What

Explicit non-English transcription now downloads the matching multilingual Whisper model immediately instead of first downloading the unusable English-only default.

Why

hyperframes transcribe --language de selected small.en, started its large download, and only planned to switch to small afterward. Users could spend time and bandwidth on the wrong model before German transcription even began.

How

The initial model resolver strips .en when an explicit non-English language or locale is supplied. English locale variants and already-multilingual models remain unchanged.

Test plan

• Unit tests added/updated: explicit German, English locale variant, and multilingual-model cases (12 tests passed)
• CLI typecheck passed
• Targeted formatting passed
• Full monorepo build passed
• Documentation updated (not applicable)

---

[vanceingalls]

Additive review — no prior reviewers on this one.

Strengths

• initialModelForLanguage (packages/cli/src/whisper/transcribe.ts:223) is a small, pure, testable helper with the right ordering: resolve-then-download. const model = initialModelForLanguage(options?.model ?? DEFAULT_MODEL, options?.language); at line 240 is threaded before ensureModel, which is exactly what the PR title promises.
• Locale-variant handling is correct: .trim().toLowerCase().split(/[-_]/, 1)[0] collapses en-US / zh_CN / EN / whitespace-padded input to a base code before the check. The en-US unit test pins this.
• Already-multilingual models pass through untouched (large-v3 test), so the branch is scoped to the .en variants — no wide-default regression on the healthy English or user-picks-multilingual paths.

Blocker (Rule 2 — audit every site that satisfies the contract)

The exact failure mode ("download .en, then re-download the multilingual model when explicit language ≠ en") still exists in packages/cli/src/commands/init.ts. Two sibling sites call ensureModel(modelFlag, …) directly, bypassing initialModelForLanguage, and only afterward call transcribe(…, { model: modelFlag, language: languageFlag }) — where the helper finally kicks in and downloads the correct model:

• packages/cli/src/commands/init.ts:827 (non-interactive hyperframes init --language de …)
• packages/cli/src/commands/init.ts:1005 (interactive init after the "Generate captions from audio?" confirm)

Repro: hyperframes init --video foo.mp4 --language de → init downloads small.en (from DEFAULT_MODEL inside ensureModel(undefined)), then transcribe downloads small. Two downloads, first one wasted — precisely the user impact the PR description calls out ("spend time and bandwidth on the wrong model").

Suggested fix (mirror the transcribe.ts wiring at both call sites):

const initialModel = initialModelForLanguage(modelFlag ?? DEFAULT_MODEL, languageFlag);
await ensureModel(initialModel, { onProgress: … });

DEFAULT_MODEL and initialModelForLanguage are both already exported from the manager / transcribe modules.

Nits

• Add a initialModelForLanguage("small.en", undefined) case to transcribe.test.ts — the no-language path is the majority production case and only covered indirectly today.
• Consider a one-line onProgress note when the helper switches (Switching to small (multilingual) for de…) so the user sees why the model differs from --model — matches the tone of the existing Detected ${detectedLanguage} — switching… message at transcribe.ts:286.

Notes

• Merge conflict on main is author-side to-do, not a reviewer blocker; review is on head 92f4fde.
• All required checks green on the PR branch (CLI smoke, Typecheck, Test, Format, Fallow audit, CodeQL).
• Parakeet engine has its own fixed model and no .en variants — no cross-scaffold work needed there.

Verdict: REQUEST CHANGES
Reasoning: Helper + wiring inside transcribe() is right, but two sibling ensureModel call sites in commands/init.ts still take the pre-fix path with the same failure mode; Rule 2 makes that a blocker rather than a follow-up.

— Via

[miguel-heygen]

Two sibling sites call ensureModel(modelFlag, …) directly, bypassing initialModelForLanguage.

Addressed at fe3ea2d04: rebased onto current main, compute initialTranscriptionModel = initialModelForLanguage(modelFlag ?? DEFAULT_MODEL, languageFlag) once, and use it for both the non-interactive and interactive eager downloads. This removes the wasted .en download before multilingual transcription.

Add a initialModelForLanguage("small.en", undefined) case.

Added. I also added a wiring regression test that fails unless both init call sites use the resolved model and no direct ensureModel(modelFlag) call remains.

Validation:

• RED confirmed on the original wiring: init regression failed because initialTranscriptionModel was absent.
• bunx vitest run packages/cli/src/commands/init.test.ts packages/cli/src/whisper/transcribe.test.ts — 85/85 passed.
• bun run --filter @hyperframes/cli typecheck — passed.
• changed-file oxlint + oxfmt — passed.
• pre-commit tracked-artifacts/lint/format/fallow/typecheck — passed.

The optional progress-message idea was kept out of this scoped correctness fix; the download selection is now correct without adding new CLI output behavior.

[james-russo-rames-d-jusso]

Reviewed at fe3ea2d047. Traced initialModelForLanguage against the 4 tested cases + the untested boundaries (empty language string, "auto", locale-with-underscore), and verified both eager ensureModel call sites in init.ts route through the same computed variable.

Blockers

(none)

Concerns

(none)

Nits

(none)

Green notes

🟢 initialModelForLanguage handles every plausible input branch correctly. The condition triple (baseLanguage && baseLanguage !== "en" && model.endsWith(".en")) covers exactly the cases where stripping .en is right:

• ("small.en", undefined) → keep — baseLanguage falsy. Tested.
• ("small.en", "de") → strip → "small". Tested.
• ("small.en", "en-US") → keep — baseLanguage === "en". Tested.
• ("large-v3", "de") → keep — model isn't .en-suffixed. Tested.
• ("small.en", "") → keep — baseLanguage after trim is empty string, falsy. Untested but correct by short-circuit.
• ("small.en", " ") → keep — same, .trim() strips before check. Untested but correct.
• ("small.en", "EN-US") → keep — .toLowerCase() normalizes, then base is "en". Untested but correct.
• ("small.enable", "de") → keep — .endsWith(".en") on "small.enable" is false (last 3 chars are "ble"). Not a real model name, but proves the suffix match is precise. Untested but correct.
• ("small.en", "auto") → strip → "small". Untested; correct semantically (auto-detect wants multilingual).

The .slice(0, -3) is safe because endsWith(".en") guarantees exactly 3 trailing characters. No accidental over/under-strip.

🟢 Two eager-download call sites in init.ts are collapsed to a single computed variable. Old:

await ensureModel(modelFlag);           // line 871 (transcribe branch)
await ensureModel(modelFlag, {...});    // line 1049 (with-progress branch)

Both would independently need the fix. New:

const initialTranscriptionModel = initialModelForLanguage(
  modelFlag ?? DEFAULT_MODEL,
  languageFlag,
);
// ...
await ensureModel(initialTranscriptionModel);
await ensureModel(initialTranscriptionModel, {...});

Compute-once at command entry — drift between the two sites is impossible. Nice invariant.

🟢 Source-match regression test in init.test.ts is a defensive shape guard. It runs initSource.match(...) against the raw source string:

• Regex 1: pins the exact shape initialModelForLanguage(modelFlag ?? DEFAULT_MODEL, languageFlag) — any variable-rename or arg-reorder that a future refactor accidentally introduces would fail here.
• expect(initSource.match(/await ensureModel\(initialTranscriptionModel/g)).toHaveLength(2) — locks the two-call-site invariant. A refactor that adds a third ensureModel site would need to explicitly update this count, forcing a conscious decision.
• expect(initSource).not.toMatch(/await ensureModel\(modelFlag/g) — negatively locks the OLD pattern. If a future PR reintroduces ensureModel(modelFlag) — either by revert or partial-fix — this fails.

Together, these three assertions make the fix hard to accidentally regress. Belt-and-suspenders alongside the four behavioral unit tests on the pure function.

🟢 transcribe() re-applies initialModelForLanguage at call time. transcribe.ts:406:

const model = initialModelForLanguage(options?.model ?? DEFAULT_MODEL, options?.language);

So even if a caller of transcribe() (outside init.ts) doesn't pre-normalize, the function itself picks the right model. Defense-in-depth: init.ts's eager ensureModel uses the same normalized name, and transcribe() will resolve to the same name at run time — no divergence, no mismatch. Same string flows through both stages.

🟢 DEFAULT_MODEL import lands cleanly. Added to the existing ../whisper/manager.js import line alongside hasFFmpeg. No new import site, no bundle-size or dep-graph concern.

🟢 Miga's blocker resolution is visible in the code shape. Magi's status note says the current head "computes initialModelForLanguage(modelFlag ?? DEFAULT_MODEL, languageFlag) once and uses it in both interactive/non-interactive eager download paths, with direct wiring + undefined-language regressions." That matches what I see in the diff — the compute-once pattern IS the direct-wiring resolution.

What I didn't verify

• Didn't check whether OTHER commands (beyond init) also do their own eager ensureModel before language-aware transcription. If hyperframes transcribe (standalone command) does an eager ensureModel(modelFlag) somewhere, it would need the same treatment. The PR only touches init.ts and transcribe.ts — worth a grep if the reviewer wants belt-and-suspenders on the whole surface area.
• Didn't verify DEFAULT_MODEL is small.en (or another .en model) — the fix only fires when the default is English-only. If DEFAULT_MODEL were already multilingual, the fix would be a no-op for non---model users. Trusting the PR body's framing that the problem was real (small.en was being downloaded for --language de).
• Didn't reproduce the "downloads wrong model" behavior on the pre-PR head. PR body's description of the failure mode is credible from the code shape (old ensureModel(modelFlag) with modelFlag=undefined would pick DEFAULT_MODEL = small.en regardless of language).

Merge gate — CI running

Per Magi's status: 85/85 targeted tests, CLI typecheck, format/lint/pre-commit gates all green locally. CI running on fe3ea2d047 — Test, Typecheck, Producer: unit tests, Producer: integration tests, SDK: unit + contract + smoke, Studio: load smoke, Test: runtime contract, CLI smoke (required), Build, Fallow audit still in flight at review time.

Do NOT merge until CI is green. But nothing code-shape I'd hold merge on; the direct-wiring + compute-once + regression-test triangulation is the right resolution to the R1 concern.

Verdict framing

Clean, tightly-scoped fix for a real user-hit misconfiguration. Pure function + compute-once + defensive regex-shape regression test is exactly the right combination. LGTM from my side, ready to merge from where I sit once CI clears.

— Review by Rames D Jusso

[vanceingalls]

🟢 R2 pass at fe3ea2d047 — R1 blocker resolved

Prior R1 (CHANGES_REQUESTED) at 92f4fde2 blocked on both eager ensureModel(modelFlag, …) call sites in init.ts bypassing the new helper.

Blocker verification — both paths now wired ✅

• Non-interactive path — packages/cli/src/commands/init.ts:874 — was ensureModel(modelFlag), now ensureModel(initialTranscriptionModel).
• Interactive path — packages/cli/src/commands/init.ts:1052 — was ensureModel(modelFlag, { onProgress }), now ensureModel(initialTranscriptionModel, { onProgress }).
• Compute-once at command entry — init.ts:773-776: initialTranscriptionModel = initialModelForLanguage(modelFlag ?? DEFAULT_MODEL, languageFlag). Single computation, drift between the two eager sites is now impossible by construction. Nice invariant.
• transcribe.ts:406 — unchanged from prior head; still re-applies initialModelForLanguage(options?.model ?? DEFAULT_MODEL, options?.language) before its internal ensureModel at :414. Defense-in-depth intact.

Test coverage — value + reachability locks both present ✅

Two-file coverage, addressing both discipline axes:

• Value-side (helper algorithm) — transcribe.test.ts adds 4 explicit initialModelForLanguage unit tests covering the fix case (small.en + de → small), the undefined-language regression (small.en + undefined → small.en), the locale variant (small.en + en-US → small.en), and the already-multilingual case (large-v3 + de → large-v3).
• Reachability-side (call-site wiring) — init.test.ts reads init.ts as source and runs three static assertions: (1) exact regex match on the initialModelForLanguage(modelFlag ?? DEFAULT_MODEL, languageFlag) compute-once shape, (2) ensureModel(initialTranscriptionModel appears exactly twice (locks both eager sites), (3) ensureModel(modelFlag never appears (negatively locks the old regression). Any future refactor that partially reverts, adds a third eager site, or renames the variable would fail this suite loudly.

Combined: the tests would fail if either eager site regressed OR if the helper's algorithm regressed. Both value and reachability are locked.

Cross-scaffold check — clean ✅

Ran a repo-wide search for ensureModel( at fe3ea2d047:

• packages/cli/src/tts/synthesize.ts:151 — TTS domain, separate ensureModel symbol from tts/manager.ts, TTS models don't use the .en naming convention. Out of scope.
• packages/cli/src/background-removal/inference.ts:77 — ONNX background-removal models. Different domain entirely.
• packages/cli/src/commands/transcribe.ts — no direct ensureModel call; routes into transcribe() which applies the helper at :406. Safe.

No other whisper-domain eager ensureModel sites bypass the helper. Cross-scaffold is clean.

CI state at fe3ea2d047

Snapshot at review time:

• 38 checks completed: all green or skipped. Includes Test, Typecheck, Build, Lint, Format, CLI smoke (required), Producer: integration tests, SDK: unit + contract + smoke, Fallow audit, CodeQL, regression, preview-regression, Preview parity, Studio: load smoke.
• 2 in-progress: Render on windows-latest, Tests on windows-latest. Platform-parity Windows shards; not shape-related.
• 0 failures.

Per rebase-clean-vs-CI-red discipline: Windows shards are the merge-gate, not the review-gate. Stamp is independent of their completion.

Verdict

Direct wiring at both eager sites + belt-and-suspenders source-match regression test + preserved defense-in-depth in transcribe() is exactly the right resolution to R1's blocker. Rames's parallel review at this SHA covers the value-side depth (green-notes on every helper boundary + untested-but-correct cases) — my R2 is the reachability-side complement (cross-scaffold + CI state). Stamping APPROVE.

— Via