Skip to content

build(deps)!: make TTS engine SDKs optional peer dependencies - #36

Open
Joilence wants to merge 10 commits into
shreyaskarnik:mainfrom
Joilence:feat/optional-engine-deps
Open

build(deps)!: make TTS engine SDKs optional peer dependencies#36
Joilence wants to merge 10 commits into
shreyaskarnik:mainfrom
Joilence:feat/optional-engine-deps

Conversation

@Joilence

@Joilence Joilence commented Aug 13, 2026

Copy link
Copy Markdown

Co-drafted PR with AI, self-reviewed.


Why

npm i @argo-video/cli@0.38.1 installs 920 MB. Most is ONNX runtime only Kokoro needs, plus four cloud SDKs which are likely not need all of.

optionalDependencies is not opt-in: npm installs those by default and only tolerates install failure. peerDependenciesMeta.optional is the one field npm never auto-installs.

What

All six engine SDKs (kokoro-js, @huggingface/transformers, openai, @elevenlabs/elevenlabs-js, @google/genai, sarvamai) become optional peers. Users install only what they use.

Install Before After
base, no engine 920 MB 27 MB
+ OpenAI 920 MB 47 MB
+ Kokoro 920 MB 435 MB

Kokoro users also gain: dropping the forced transformers v4 upgrade keeps the onnxruntime-node that kokoro-js already wanted.

  • src/tts/transcribe.ts now imports @huggingface/transformers dynamically. The top-level import was reachable from src/cli.ts, so it broke every command including validate and export when the package was absent. This blocked the whole change.
  • All seven SDK load sites go through one importOptional() wrapper. The previous bare catch reported any load failure as "not installed", misdirecting users whose package is present but whose native binding is not.
  • @huggingface/transformers is ^3.5.1 || ^4.2.0 so it dedupes against kokoro-js (^3.5.1). Pinning ^4 yields a second nested copy plus a second ONNX runtime, since overrides only apply in a root package.json.
  • Cloud SDK ranges are open-ended. An optional peer still constrains the consumer's copy, so a narrow bound makes npm i fail with ERESOLVE for anyone on a newer major, including people who never use that engine. openai shipped three majors in fourteen months. Call surface verified identical on 4.104.0, 5.23.2, 6.49.0, 7.4.0.
  • engines.mlxAudio() is unaffected: it speaks HTTP to a local Python server, no npm package to resolve.
  • Four adapters keep a loose destructure. Removing any surfaced two pre-existing SDK call bugs (ElevenLabs model_id vs modelId; Sarvam constructs the CJS default, but sarvamai exports SarvamAI named). Both need API keys to test, so this PR preserves existing runtime behaviour rather than mixing in a fix.

Touches src/cli.ts in two small hunks, overlapping open #30. Happy to rebase once that lands.

What users see

Fresh install is 27 MB with no engine. argo doctor says exactly what is missing and what to run:

✓ install mode           project
! engine/kokoro          not installed. npm i kokoro-js
! engine/openai          not installed. npm i openai
! engine/elevenlabs      not installed. npm i @elevenlabs/elevenlabs-js
! engine/gemini          not installed. npm i @google/genai
! engine/sarvam          not installed. npm i sarvamai
! engine/transformers    not installed. npm i @huggingface/transformers
! engine/transcribe      not installed. npm i @huggingface/transformers
! engines                No npm TTS engine installed. npm i kokoro-js (or use engines.mlxAudio)

argo init ends with the same nudge, so new projects hit it before their first run:

No TTS engine is installed yet. Pick one:
  local, no API key:  npm i kokoro-js
  cloud, needs a key: npm i openai
  see all engines:    npx argo doctor

Reaching TTS without the package fails at the first clip, naming the command again:

Kokoro local TTS requires 'kokoro-js', which is an optional dependency and is not installed.
  Install it with: npm i kokoro-js

Global installs get different commands, unprompted, because global trees do not hoist and two separate npm i -g runs leave a second ONNX copy:

✓ install mode           global
! engine/kokoro          not installed. npm i -g kokoro-js @huggingface/transformers@^3
! engine/openai          not installed. npm i -g openai

Following that command verbatim resolves it, and lights up transcription for free:

✓ engine/kokoro          installed
✓ engine/transcribe      installed
All good, 7 warning(s)

Silent demos (no text in the manifest) need no engine at all and keep working on the 27 MB install. demos/ci-smoke is exactly this case, so CI covers the path.

Breaking change

No TTS engine installs by default. Kokoro users run npm i kokoro-js, or npm i -g kokoro-js @huggingface/transformers@^3 for a global install. argo doctor, argo init, and the runtime error all print the correct command for the detected mode.

Test

  • npm test: 681 pass, 60 files.
  • npx tsx bin/argo.js pipeline ci-smoke --config demos/ci-smoke.config.mjs --browser chromium passes locally, producing the 10.0s mp4 CI verifies.
  • Packed and installed into clean projects. Engine-free install is 27 MB, zero engines resolvable per argo doctor.
  • argo pipeline on a silent demo with zero engines installed: 15.1s 1280x720 h264 mp4, no audio stream, overlays and click intact. Previously failed outright.
  • npm i kokoro-js exactly as argo doctor instructs, then argo pipeline on a narrated demo: 12.7s mp4, aac 24 kHz mono, mean -24.0 dB / max -3.1 dB over 303104 samples, subtitles generated.
  • ERESOLVE both directions: consumer on openai@^7 installs cleanly, consumer on openai@3.3.0 still correctly rejected.
  • Install-mode detection verified for project, global (from /tmp and $HOME), and a pnpm .pnpm/ store layout.
  • New test asserts no optional SDK appears in a static import position. All six are devDependencies, so nothing else can catch a regression to a top-level import. Verified to fail when one is reintroduced.

`argo doctor` gains an engines section listing which TTS engines resolve, and
`argo init` prints a hint when none is installed. Both derive the command from
the detected install mode, since project, global and npx each need different
syntax.

The roll-up warns rather than fails: mlx-audio needs no npm package, so an
absent npm engine cannot prove voiceover is broken, and doctor's exit code
gates CI for those users.
Every site that loads an engine SDK now goes through a shared wrapper that
translates a missing optional package into the exact install command, and
leaves every other failure untouched. Previously each adapter used a bare
catch that reported any load failure as "not installed", which misdirects
users whose package is present but broken.

The transcriber's `@huggingface/transformers` import becomes dynamic. A
top-level import there is reachable from the CLI entry, so it took down
every command, including `validate` and `export`, for anyone who had not
installed a 400 MB ONNX runtime.

A module-not-found alone is not enough to blame the optional package: the
same code surfaces when the package is present but one of its own
dependencies is not. onnxruntime-node requires a per-arch native binding
at runtime, so the wrapper confirms the package is genuinely absent before
rewriting the error, and the CLI now prints the underlying cause.

Install hints are resolved per install mode, because the correct command
differs. Global trees do not hoist, so Kokoro needs both packages in one
command; two separate `npm i -g` runs leave a second ONNX copy. Detection
asks the resolver whether the project reaches this same copy rather than
parsing paths, which keeps pnpm, nested npm, and Yarn PnP working.
Moves kokoro-js, @huggingface/transformers, openai, @elevenlabs/elevenlabs-js,
@google/genai and sarvamai out of dependencies and optionalDependencies and
into optional peerDependencies.

optionalDependencies does not mean opt-in: npm installs those by default and
only tolerates install failure. peerDependenciesMeta.optional is the one field
npm never auto-installs. Measured against the published 0.38.1, a base install
drops from ~920 MB to ~27 MB, ~47 MB with OpenAI, and a Kokoro user lands at
~435 MB, still less than half of what everyone paid before.

Cloud SDK ranges are open-ended. An optional peer still constrains the
consumer's copy, so pinning openai to ^4 made installation fail outright with
ERESOLVE for anyone already on v5, v6 or v7, including people who never use
that engine. openai shipped three majors in fourteen months, so an enumerated
bound goes stale within months and silently reintroduces that failure, whereas
an over-wide bound at worst surfaces an SDK error in one engine at generate()
time. The call surface is verified identical on 4.104.0, 5.23.2, 6.49.0 and
7.4.0.

@huggingface/transformers stays bounded at ^3.5.1 || ^4.2.0 so it can dedupe
against kokoro-js, which requires ^3.5.1. Pinning ^4 produces a second nested
copy plus a second ONNX runtime, because overrides only apply in a root
package.json and stop working once Argo is a dependency. The devDependency
tracks ^3.5.1 so the repo's own tree hoists one copy and CI exercises the
version consumers actually resolve.

BREAKING CHANGE: no TTS engine is installed by default. Users of the default
Kokoro engine must run `npm i kokoro-js`. `argo doctor` and `argo init` report
what is missing and the command for the detected install mode.
Covers the resolution probe directly, including the version-range-suffixed
specifier form, and pins that a transitive module-not-found is not attributed
to the optional package.

Adds a static-import guard. Every optional SDK is a devDependency, so all six
resolve during tests and nothing else in the suite can notice a regression from
`await import(x)` back to a top-level import, which would break every command
for users who installed no engine.
Records that no TTS engine ships by default, the per-mode install commands, and
the one-command constraint for a global Kokoro install. Engine sizes are stated
on a single measurement basis, with the realistic end-to-end totals alongside,
since the per-package figures do not simply add up.
@shreyaskarnik

Copy link
Copy Markdown
Owner

Review

Design holds up — no static import of any of the six optional packages survives in src/, and src/tts/transcribe.ts (the blocker, and the thing #38 had to patch by hand) is now dynamic. Build passes, 670 tests pass. Five findings below; 1, 2 and 4 are worth fixing before merge.

Also: gh pr checks 36 reports no checks at all on this branch. Nothing has run against it.


1. tests/optional-imports.test.ts:2 — the guard test crashes on the project's own minimum Node, so it guards nothing

globSync from node:fs landed in Node 22.0.0. README.md:355 says "Node.js >= 18", and package.json has no engines field. On Node 20 the file dies at collection:

TypeError: (0, globSync) is not a function

Zero assertions execute. Both workflows pin node-version: 24, so it reads green in CI while doing nothing for anyone on 18/20. The file's own header comment calls itself "the only guard" against an optional peer regressing to a static import.

Worth noting the irony: src/optional-deps.ts:37 has a deliberate comment about avoiding import.meta.resolve because it's undefined before Node 18.19/20.6 — while the test next door silently requires 22+.

Fix: manual readdirSync recursion (or fast-glob, already transitively present), and add an explicit engines.node so the floor is enforced rather than asserted in prose.

2. src/optional-deps.ts:216isDepInstalled probes spec.project[0] only, so the one multi-package spec is under-checked

return resolveFrom(dirname(fileURLToPath(import.meta.url)), bareName(spec.project[0])) !== null;

KOKORO_DEP.global is ['kokoro-js', '@huggingface/transformers@^3'] — two entries, added precisely because global trees don't hoist. But the probe only ever looks at project[0] (kokoro-js), and never consults spec.global even in global/npx mode.

Failure path: global install where kokoro-js resolves but @huggingface/transformers does not. import('kokoro-js') throws ERR_MODULE_NOT_FOUND; the guard at line 202 sees isDepInstalled === true; the raw Cannot find package '@huggingface/transformers' propagates with no hint. The user never sees the single two-package command this spec exists to give them — which is exactly the ~840 MB trap the comment on line 64 is warning about.

Fix: check every entry of the mode-appropriate list.

3. src/optional-deps.ts:202 — a throw from isDepInstalled destroys the original error and crashes argo doctor

resolveFrom deliberately rethrows anything that isn't module-not-found, so isDepInstalled is not total. A truncated node_modules/<pkg>/package.json from an interrupted npm i throws ERR_INVALID_PACKAGE_CONFIG; an exports map with no matching condition throws ERR_PACKAGE_PATH_NOT_EXPORTED.

Two consequences:

  • Called from inside importOptional's catch, that throw replaces err — the actual import failure is lost.
  • argo doctor (src/cli.ts) and printEngineHintIfMissing (src/init.ts:167) propagate it and crash. doctor is the command a user runs because their install is half-broken; it's the one thing that must not die on a half-broken install.

The rethrow in resolveFrom is right — the fix belongs at the isDepInstalled boundary: degrade to false (or a distinct fail/unknown row in the doctor table) rather than throwing.

4. src/optional-deps.ts:68,74,80 — three specs hand out an unpinned @huggingface/transformers, recreating the duplicate-ONNX problem this PR exists to prevent

KOKORO_DEP.global correctly pins @huggingface/transformers@^3 because kokoro-js@1.2.1 wants ^3.5.1. But TRANSFORMERS_DEP, WHISPER_DEP and MUSICGEN_DEP all emit a bare npm i @huggingface/transformers, which resolves to v4.

Failure path: user installs Kokoro (3.8.1 hoisted to root, per the updated lockfile), later enables tts.transcribe, follows the hint printed by argo doctor or the runtime error → npm installs v4 at root and re-nests 3.x under kokoro-js/. Two ONNX runtimes: ~765 MB vs ~410 MB — the precise outcome both CLAUDE.md and the README claim this design prevents.

Fix: pin these three to @^3 as well, or make the pin conditional on whether kokoro-js is present.

5. tests/optional-imports.test.ts:24 — the regex under- and over-matches

new RegExp(`(?<!\\()\\bfrom\\s+['"]${escaped}['"]`)
  • Misses: a bare side-effect import (import '@huggingface/transformers';) or createRequire()('kokoro-js') regresses the design with the guard still green.
  • Over-matches: import type { ... } from '@huggingface/transformers' is fully erased at compile time and therefore harmless, but would be flagged and fail the build.
  • The (?<!\() lookbehind is inert either way — import('pkg') has no from for the pattern to reach in the first place.

Checked and clear

  • No static import of any of the six optional packages remains in src/ (grep-confirmed).
  • src/tts/kokoro.ts is a re-export shim, so ensureTTSEngine's import('./tts/kokoro.js') loads nothing eagerly.
  • The initPromise = null resets in kokoro.ts:42 / transformers.ts:53 are correctly ordered — the outer assignment completes before the catch callback runs.
  • MusicgenForConditionalGeneration, AutoTokenizer and pipeline(task, model) all exist in @huggingface/transformers@3.8.1, so the devDep downgrade from ^4.2.0 breaks neither musicgen.ts nor transcribe.ts. Word-level Whisper timestamps confirmed working on 3.8.1.
  • @argo-video/cli's own exports map carries the default condition detectInstallMode() relies on.

@shreyaskarnik

Copy link
Copy Markdown
Owner

Correction to my review: CI has now run and all four checks pass (Unit tests, ci-smoke on chromium/firefox/webkit). The "no checks reported" note is stale — disregard it.

Worth being precise about what that does and doesn't clear, though: it does not retire finding 1. Both workflows pin node-version: 24, and globSync exists from Node 22 on, so a green run on 24 is exactly the outcome that finding predicts. The failure only appears on Node 18/20 — the range README.md:355 currently advertises.

Reproducing it needs a different Node, not a different test:

$ node -v
v20.20.2
$ npx vitest run tests/optional-imports.test.ts
TypeError: (0, globSync) is not a function

The file dies at collection, so none of its assertions run — and it's the only thing standing between a stray static import and the whole optional-peer design. Adding engines.node would at least make the supported floor explicit and enforced rather than advertised in prose; a second CI matrix entry at the floor version would catch this class of thing generally.

Findings 2 and 4 are also invisible to CI by construction — both are about which install command gets printed to a user in a global/npx tree, which nothing exercises.

@Joilence

Copy link
Copy Markdown
Author

Thanks for the review.

1. Fixed in 5ff764c, with engines.node and a node: [20, 24] matrix in 4aeba52, kept separate so you can drop it if the floor is out of scope here.

Validating it turned up that README.md was already wrong on main. playwright-core declares "engines": {"node": ">=20"} and exits below it, so the floor was never 18:

$ node -v → v18.20.8
Test Files  10 failed | 50 passed (60)
 ❯ node_modules/playwright-core/lib/bootstrap.js:11:11

2. Left alone. kokoro-js@1.2.1 declares transformers a hard dependency, not a peer, so npm cannot install it without one. In a global tree it nests, and from Argo's directory:

kokoro-js                 -> resolvable
@huggingface/transformers -> NOT resolvable (MODULE_NOT_FOUND)
import kokoro-js -> OK, KokoroTTS: function

Kokoro loads from its private nested copy, so reporting it installed is accurate, and checking every entry would flip that row to "not installed" for a working engine. Happy to reconsider if you have a tree I did not reproduce.

3. Fixed in 3a33bdd. The same defect was one level up in detectInstallMode, which doctor.ts:129 calls first, so the table never rendered at all:

No "exports" main defined in .../@argo-video/cli/package.json
  imported from .../@argo-video/cli/dist/noop.js
argo doctor exit code: 1

One correction: ERR_INVALID_PACKAGE_CONFIG is unreachable for Argo's own manifest, since Node dies in run_main first. It still applies to isDepInstalled, which probes other packages.

4. Fixed in the same commit, with two things beyond the finding. The hints are written @3 rather than @^3, because ^ is a glob operator under zsh extendedglob and the pasted command dies before reaching npm; the two are the same range to npm, so it costs nothing. And kokoro-js is pinned too, since its peer is capped at ^1.2.1 and an unpinned hint would ERESOLVE against a future v2.

5. Fixed in 5ff764c. Subpath imports were not in your list but slipped through the same way. require.resolve stays unflagged on purpose, and the remaining gap is documented: const r = createRequire(url); r('pkg') splits the call from the specifier and the binding can be named anything, so no line scan sees it.

You were right that 2 and 4 were invisible to CI.

Found, not fixed

src/tts/engines/sarvam.ts:40 destructures { default: SarvamAI }, which is undefined in the pinned sarvamai@1.1.8. SarvamAI is a namespace object; the constructor is SarvamAIClient. So the engine throws for anyone who has the package. Pre-existing, and verifying the convert() call underneath needs a live key, so I kept it out of a packaging PR. Separate issue or PR, whichever you prefer.

@shreyaskarnik

Copy link
Copy Markdown
Owner

Reproduced all of this. Taking them in order of how wrong I was.

2 — you're right, and my finding was wrong on its premise

I assumed @huggingface/transformers could be absent while kokoro-js resolves. It can't: kokoro-js declares it a hard dependency, not a peer.

$ node -p "require('kokoro-js/package.json').dependencies"
{ '@huggingface/transformers': '^3.5.1', phonemizer: '^1.2.1' }

So npm cannot install kokoro-js without it, the nested copy always exists, and probing project[0] is the accurate answer. Checking every entry would flip a working engine to "not installed" — your read is correct. The second entry in global is there to force hoisting into one ONNX copy, not because Kokoro needs it to function, and I conflated the two. Leave it alone.

1 — confirmed, and >=20 is the right floor

Worth pinning down why we saw different things, since it changes the reasoning rather than the outcome. The locked playwright-core still declares 18:

$ node -p "require('playwright-core/package.json').version"        → 1.59.1
$ node -p "require('playwright-core/package.json').engines"        → { node: '>=18' }

That's why my Node 20 reproduction of the globSync failure ran clean past Playwright. But the devDep is ^1.59.1, so a fresh install today resolves:

$ npm view playwright-core@latest version engines
{ "version": "1.62.1", "engines": { "node": ">=20" } }

Your Node 18 run hit the newer one. So the floor isn't 18 for anyone installing now, >=20 is correct, and the README was advertising a version nobody could actually use. Keep the matrix commit — a floor asserted only in engines is the same class of problem as one asserted only in prose, and node: [20, 24] is what makes it real.

3 — good catch going one level up

detectInstallMode failing before the table renders is worse than what I reported, and argo doctor exiting 1 on the exact install it exists to diagnose is the right thing to have chased. Your correction on ERR_INVALID_PACKAGE_CONFIG is accurate — Node dies in run_main before Argo's own manifest is ever probed. It stands for isDepInstalled probing other packages, which is where I should have scoped it.

The typeof err.code === 'string' discriminator is the part I'd have missed: recovering only from tagged Node resolution errors, so an Argo bug surfaces as a bare TypeError rather than being silently answered "installed."

4 — the zsh detail is a better fix than what I asked for

I'd have written @^3 and shipped a command that dies before reaching npm for anyone with extendedglob set, which is most zsh users with a configured prompt. @3 and @^3 are the same X-range to npm, so it costs nothing. Pinning kokoro-js@1 against a future v2 ERESOLVE is beyond the finding and correct.

5 — subpath imports were a real gap

And documenting the createRequire split-binding case rather than pretending the scan catches it is the right call. A line scan fundamentally can't see it; a comment saying so is worth more than a regex that looks like it does.


On the Sarvam bug

Confirmed independently, and it's worse than "throws" — default isn't merely undefined, the constructor is a different name:

$ node --input-type=module -e "const m = await import('sarvamai'); ..."
default:         undefined
SarvamAI:        object      ← namespace, not a constructor
SarvamAIClient:  function    ← the actual client

So new SarvamAI(...) at sarvam.ts:45 throws SarvamAI is not a constructor for every user who has the package correctly installed. The engine has never worked. Agreed it doesn't belong in a packaging PR — filing it separately.


LGTM. Findings 1, 3, 4 and 5 fixed; 2 was my error. Thanks for pushing back on it with a reproduction instead of just applying it — that's the one I'd have gotten wrong twice.

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.

2 participants