Skip to content

fix(cli): use a healthy pinned Sherpa copy beside the CLI - #5095

Merged
miguel-heygen merged 6 commits into
mainfrom
fix/cli-sherpa-beside-cli
Oct 6, 2026
Merged

miguel-heygen merged 6 commits into
mainfrom
fix/cli-sherpa-beside-cli

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

When a pinned copy shipped beside the CLI is available, Parakeet should use it without requiring another runtime installation. Previously, installation checks and decoding only looked in the cache.

This change shares exact-version package discovery, probes native loading in a cancellable child, and passes the selected entry directly to the decode worker. A broken pinned copy emits one line naming its path and load error, then a healthy cache may be used. If neither loads, transcription reports repair guidance.

Installation JSON retains runtimeDir as the cache destination and adds runtimePath for the selected healthy entry.

Validation

Validated at d69eac0cbb7c9da57e84997d92ed8cc134f303aa.

  • Each changed test file ran alone three consecutive times: runtime selection, optional packages, worker decoding, transcription command, and installation command.
  • Real worker fixtures prove broken pinned copy plus healthy cache produces a transcript, while missing or broken cache produces a repair error.
  • Mutation checks prove selection, pinning, diagnostics, worker location, parent isolation, timeout handling, and JSON provenance assertions are non-vacuous.
  • Typecheck, lint, formatting, dead-code checks, comment-share/comment-length preflights and package build checked on Linux.

No visible change

Runtime loading and command JSON only. No Studio or player UI changes.

Limits

Fixture packages prove routing and child isolation. Real native binaries and speech model decoding are not exercised by these tests.

Small-PR note: one runtime-loading fix spans discovery, health probing, availability, installation reporting and the existing worker handoff so they cannot select different copies.

@mintlify

mintlify Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
hyperframes 🟢 Ready View Preview Oct 6, 2026, 5:05 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2055 (base branch 2055), smooth 1597 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 6, 2026 02:15

@somanshreddy somanshreddy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 11eae121 (full PR, including the merge with main). This is a comment, not an approval.

No blockers.

  • Runtime selection is consistent end to end:
    • selectRuntime tries the pinned copy beside the CLI first, probing it in a child.
    • It logs one line if that copy is broken, then probes the cache copy.
    • The decode worker gets the exact selected entry, so the probe and the decode can't pick different copies.
  • The merge with #5103 keeps both behaviours in transcribeWithSherpa: the started and completed events surround the new selection and decode.
  • The probe's switch from spawnSync to async spawn handles each way it can stop: a timeout reports "timed out" (because stoppedByCancelSignal returns false when the child was killed), Ctrl-C or abort becomes DecodeCancelled, and a crash reports its signal.

Low

  1. Each Parakeet transcription now starts an extra Node child that loads the native runtime before the decode worker loads it again (sherpa.ts transcribeWithSherpa → selectRuntime). With a broken pinned copy it starts two, and the "did not load" line prints on every run. Under --json, that line now sits among #5103's JSON progress lines on stderr. Consider caching the selection per process. Alternatively, skip the probe in transcribe and let the worker's load error carry the repair message.
  2. installedOptionalPackageVersion only trusts resolution === "entry", but no test pins that. Mutant M8, accepting the "package" resolution as well, survives. Today only sherpa-onnx-node reaches the package branch, so it has no effect yet.
  3. The started event fires before runtime selection. A failed selection leaves a started with no terminal event. Callers that pair events would need to treat the thrown error as the end.

Mutations: 7 of 8 caught against the 5 changed test files:

  • pinned copy ignored;
  • no cache fallback after a broken pinned copy;
  • any version accepted;
  • the package-resolution branch dropped;
  • "installed" ignoring the pinned copy;
  • a probe timeout counted as healthy;
  • loadBesideCli loading a package-only resolution.
    M8 survived (low 2).

What I ran: the 5 changed test files pass 91/91 (vitest, packages/cli). I didn't exercise real native sherpa or onnxruntime binaries, the same limit the PR body states.

CI at this head: 68 pass, 20 pending, 7 skipped, 0 failing so far.

— Somu

@somanshreddy somanshreddy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-check at 6692cb1e: the only change from 11eae121 is that docs/contracts/2026-10-05-sherpa-beside-cli.html was removed (GitHub compare: 1 commit ahead, 0 behind, one file removed). The code is identical, so my review at 11eae121 (5423978817) still applies unchanged: no blockers, three lows. This is a comment, not an approval.

— Somu

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at 6692cb1e18dd21afd1ded6cae176a9987370bacb after my earlier hold on the unresolved #5103 merge conflict:

  • The merge resolution in packages/cli/src/whisper/sherpa.ts:440–479 preserves both #5103's started/completed progress events and this PR's selected-runtime handoff to decode(wavPath, selected.path, signal). The worker loads that exact path; the added happy-path test pins the event sequence. The conflict that prevented an approval at d69eac0c is resolved.
  • The pinned adjacent package is exact-version checked and probed in a child, with a healthy cache fallback and cancellation kept distinct from probe failure. I checked the installer, availability reader, and decode worker paths. As Somu noted in the at-head comment, the additional native probe has a per-transcription cost and the resolution === "entry" installed-version guard lacks a direct test. The live guard is correct; the started-without-completed failure behavior and diagnostics alongside JSON progress match the pre-existing #5103 contract.
  • From the reviewed 11eae121 to this head, GitHub compare shows only removal of docs/contracts/2026-10-05-sherpa-beside-cli.html; runtime and test source are unchanged. Somu's 91/91 focused run applies to the unchanged code; I did not rerun native binaries, real audio, or packaged-CLI adjacency locally.

No remaining code-merit blocker found. The required checks visible at review time passed; this approval is not a merge action.

Verdict: APPROVE
Reasoning: The conflict resolution preserves both runtime selection and progress semantics, and the re-pinned head changes only the design document relative to the audited source.

— Review by tai (pr-review)

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 6, 2026
Merged via the queue into main with commit f6e419e Oct 6, 2026
95 checks passed
@miguel-heygen
miguel-heygen deleted the fix/cli-sherpa-beside-cli branch October 6, 2026 05:54

This branch was successfully deployed

1 active deployment
staging - docs — 6692cb1e Deployed Oct 6, 2026 by mintlify[bot]
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.

3 participants