Repository navigation
Add Rust API for Moonshine v2 - #3247
Conversation
dosubot
Bot
added
the
size:L
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates the latest Moonshine v2 model architecture into the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThis PR adds Moonshine v2 offline ASR model support to the Rust bindings by introducing a merged_decoder field to the OfflineMoonshineModelConfig FFI structure, providing a complete executable example with integration into the test pipeline, and bumping package versions across related Cargo.toml files. Changes
Sequence DiagramsequenceDiagram
participant User as User/Script
participant Example as moonshine_v2.rs
participant Wrapper as Rust Wrapper<br/>(sherpa-onnx)
participant FFI as FFI Layer<br/>(sherpa-onnx-sys)
participant Backend as C Backend
User->>Example: invoke with arguments<br/>(wav, encoder, decoder, merged_decoder, tokens, provider, threads)
Example->>Example: parse arguments &<br/>load WAV file
Example->>Wrapper: build OfflineRecognizerConfig<br/>with Moonshine v2 settings
Wrapper->>FFI: to_sys() conversion<br/>maps merged_decoder field
FFI->>Backend: create OfflineRecognizer
Backend-->>FFI: recognizer instance
FFI-->>Wrapper: return recognizer
Wrapper-->>Example: OfflineRecognizer created
Example->>Wrapper: create stream &<br/>feed waveform data
Wrapper->>FFI: invoke C recognition API
FFI->>Backend: decode audio stream
Backend-->>FFI: recognition result
FFI-->>Wrapper: return decoded text
Wrapper-->>Example: result & timing metrics
Example->>User: output text & RTF metrics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for Moonshine v2 models to the Rust API. The changes include a new example, updates to the FFI bindings and safe wrappers, and a test script. The implementation is solid. I've provided one suggestion in the new example file to improve performance by avoiding unnecessary string allocations.
| recognizer_config.model_config.moonshine.encoder = Some(args.encoder.clone()); | ||
| recognizer_config.model_config.moonshine.merged_decoder = Some(args.decoder.clone()); | ||
|
|
||
| recognizer_config.model_config.tokens = Some(args.tokens.clone()); | ||
| recognizer_config.model_config.provider = Some(args.provider.clone()); |
There was a problem hiding this comment.
To improve performance and adhere to Rust's ownership principles, you can avoid cloning these String values. Since the args struct is not used after this configuration block, you can move the values directly into the recognizer_config. This prevents unnecessary memory allocations.
| recognizer_config.model_config.moonshine.encoder = Some(args.encoder.clone()); | |
| recognizer_config.model_config.moonshine.merged_decoder = Some(args.decoder.clone()); | |
| recognizer_config.model_config.tokens = Some(args.tokens.clone()); | |
| recognizer_config.model_config.provider = Some(args.provider.clone()); | |
| recognizer_config.model_config.moonshine.encoder = Some(args.encoder); | |
| recognizer_config.model_config.moonshine.merged_decoder = Some(args.decoder); | |
| recognizer_config.model_config.tokens = Some(args.tokens); | |
| recognizer_config.model_config.provider = Some(args.provider); |
There was a problem hiding this comment.
Pull request overview
Adds Moonshine v2 support to the Rust bindings by exposing the merged decoder model in the Moonshine offline config, plus a runnable Rust example and CI script coverage.
Changes:
- Extend Moonshine offline model config in both
sherpa-onnxandsherpa-onnx-systo includemerged_decoder. - Bump Rust crate versions to
0.1.9and update the examples crate to use the new version. - Add a Moonshine v2 Rust example + download/run script, and run it in the Rust CI script.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
sherpa-onnx/rust/sherpa-onnx/src/offline_asr.rs |
Adds merged_decoder to the safe Rust Moonshine config and passes it through to FFI. |
sherpa-onnx/rust/sherpa-onnx/Cargo.toml |
Bumps sherpa-onnx to 0.1.9 and updates sherpa-onnx-sys dependency version. |
sherpa-onnx/rust/sherpa-onnx-sys/src/offline_asr.rs |
Adds merged_decoder to the FFI struct to match the C API. |
sherpa-onnx/rust/sherpa-onnx-sys/Cargo.toml |
Bumps sherpa-onnx-sys to 0.1.9. |
rust-api-examples/run-moonshine-v2.sh |
New script to download Moonshine v2 models and run the example. |
rust-api-examples/examples/moonshine_v2.rs |
New offline Moonshine v2 Rust example using merged_decoder. |
rust-api-examples/Cargo.toml |
Bumps examples crate and updates sherpa-onnx dependency to 0.1.9. |
rust-api-examples/Cargo.lock |
Updates locked versions/checksums for 0.1.9. |
.github/scripts/test-rust.sh |
Runs the new Moonshine v2 script in Rust CI. |
Comments suppressed due to low confidence (1)
rust-api-examples/run-moonshine-v2.sh:9
- For consistency with other
run-*.shscripts (and to keep CI logs smaller), consider usingcurl -SsL(or similar) here;curl -SLwill emit progress output and, combined withset -xandtar xvf, can make logs very noisy.
if [ ! -f ./sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27/encoder_model.ort ]; then
curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models/sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2
tar xvf sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2
rm sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [package] | ||
| name = "sherpa-onnx" | ||
| version = "0.1.8" | ||
| version = "0.1.9" |
There was a problem hiding this comment.
OfflineMoonshineModelConfig gained a new public field (merged_decoder), which is a breaking change for downstream crates that construct the struct via a literal or destructure it in patterns. Consider bumping the crate version with a breaking-change increment (e.g., 0.2.0) or otherwise providing a backwards-compatible migration path before publishing.
| sherpa-onnx = "0.1.9" | ||
| # sherpa-onnx = { path = "../sherpa-onnx/rust/sherpa-onnx" } |
There was a problem hiding this comment.
rust-api-examples depends on sherpa-onnx from crates.io (not the in-repo path dependency). That means CI for this repo won’t exercise the code changes in sherpa-onnx/rust/…, and it also assumes 0.1.9 is already published. If the goal is to test the PR’s code, consider switching this to a path dependency in CI (or using a [patch.crates-io] override) and only using the registry version for end-user examples/releases.
| sherpa-onnx = "0.1.9" | |
| # sherpa-onnx = { path = "../sherpa-onnx/rust/sherpa-onnx" } | |
| # sherpa-onnx = "0.1.9" | |
| sherpa-onnx = { path = "../sherpa-onnx/rust/sherpa-onnx" } |
| [package] | ||
| name = "sherpa-onnx-sys" | ||
| version = "0.1.8" | ||
| version = "0.1.9" |
There was a problem hiding this comment.
This release adds a new public field to the OfflineMoonshineModelConfig FFI struct, which is a breaking API change for downstream users of sherpa-onnx-sys. Consider using a breaking-change version bump (or documenting the semver policy for 0.x) before publishing.
| version = "0.1.9" | |
| version = "0.2.0" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust-api-examples/examples/moonshine_v2.rs`:
- Around line 41-42: Validate the command-line field num_threads (the struct
field named num_threads with #[arg(...)]) before it is forwarded to the
recognizer creation call where it's used later (the use at the site that
forwards num_threads around line 59); reject or handle values <= 0 instead of
passing them through. Add a check right after parsing CLI args (or immediately
before the recognizer is constructed) that returns a clear error/exit or clamps
to a safe minimum if num_threads <= 0, and convert the validated positive value
to the expected unsigned type (usize) before passing it into the recognizer
creation/initialization call.
- Around line 103-105: The example currently only prints an error when decoding
fails (the else branch that calls eprintln!("Failed to get recognition
result")), which allows the program to exit with code 0; update that failure
branch to terminate with a non-zero exit status (for example call
std::process::exit(1) or return an Err from main) so CI sees the failure—locate
the else block around the recognition result handling in moonshine_v2.rs and
replace the simple eprintln! with an error log plus a non-zero exit/Err return.
In `@rust-api-examples/run-moonshine-v2.sh`:
- Around line 7-9: Add a SHA-256 integrity check for the downloaded artifact
before extraction: after the curl download command (the line that fetches
sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2) compute the file's
SHA-256 (using sha256sum or shasum -a 256) and compare it against a pinned
expected checksum constant; if the checksums do not match, print an error and
exit non‑zero so the subsequent tar xvf step is never run, otherwise proceed to
tar and then remove the archive. Ensure the check is fail-fast (exit on
mismatch) and references the exact filename used in the curl/tar commands so it
cannot be bypassed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
rust-api-examples/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/scripts/test-rust.shrust-api-examples/Cargo.tomlrust-api-examples/examples/moonshine_v2.rsrust-api-examples/run-moonshine-v2.shsherpa-onnx/rust/sherpa-onnx-sys/Cargo.tomlsherpa-onnx/rust/sherpa-onnx-sys/src/offline_asr.rssherpa-onnx/rust/sherpa-onnx/Cargo.tomlsherpa-onnx/rust/sherpa-onnx/src/offline_asr.rs
| #[arg(long, default_value_t = 2)] | ||
| num_threads: i32, |
There was a problem hiding this comment.
Validate num_threads before using it.
Line 41-Line 42 accepts any i32, and Line 59 forwards it directly. Reject <= 0 to prevent invalid recognizer settings.
🔧 Proposed fix
fn main() {
let args = Args::parse();
+ if args.num_threads <= 0 {
+ eprintln!("--num-threads must be > 0");
+ std::process::exit(2);
+ }
let wave = Wave::read(&args.wav).expect("Failed to read WAV file");Also applies to: 59-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust-api-examples/examples/moonshine_v2.rs` around lines 41 - 42, Validate
the command-line field num_threads (the struct field named num_threads with
#[arg(...)]) before it is forwarded to the recognizer creation call where it's
used later (the use at the site that forwards num_threads around line 59);
reject or handle values <= 0 instead of passing them through. Add a check right
after parsing CLI args (or immediately before the recognizer is constructed)
that returns a clear error/exit or clamps to a safe minimum if num_threads <= 0,
and convert the validated positive value to the expected unsigned type (usize)
before passing it into the recognizer creation/initialization call.
| } else { | ||
| eprintln!("Failed to get recognition result"); | ||
| } |
There was a problem hiding this comment.
Return a non-zero exit code when decoding fails.
Line 103-Line 105 only prints an error, so this example can still exit successfully and mask failures in CI.
🔧 Proposed fix
- } else {
- eprintln!("Failed to get recognition result");
- }
+ } else {
+ eprintln!("Failed to get recognition result");
+ std::process::exit(1);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else { | |
| eprintln!("Failed to get recognition result"); | |
| } | |
| } else { | |
| eprintln!("Failed to get recognition result"); | |
| std::process::exit(1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust-api-examples/examples/moonshine_v2.rs` around lines 103 - 105, The
example currently only prints an error when decoding fails (the else branch that
calls eprintln!("Failed to get recognition result")), which allows the program
to exit with code 0; update that failure branch to terminate with a non-zero
exit status (for example call std::process::exit(1) or return an Err from main)
so CI sees the failure—locate the else block around the recognition result
handling in moonshine_v2.rs and replace the simple eprintln! with an error log
plus a non-zero exit/Err return.
| curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models/sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2 | ||
| tar xvf sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2 | ||
| rm sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2 |
There was a problem hiding this comment.
Verify artifact integrity before extraction.
Line 7-Line 9 downloads and untars a remote archive without checksum verification. Please add a pinned SHA-256 check before tar xvf.
🔧 Proposed hardening sketch
- curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models/sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2
- tar xvf sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2
+ curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models/sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2
+ echo "<expected_sha256> sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2" | sha256sum -c -
+ tar xvf sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust-api-examples/run-moonshine-v2.sh` around lines 7 - 9, Add a SHA-256
integrity check for the downloaded artifact before extraction: after the curl
download command (the line that fetches
sherpa-onnx-moonshine-tiny-en-quantized-2026-02-27.tar.bz2) compute the file's
SHA-256 (using sha256sum or shasum -a 256) and compare it against a pinned
expected checksum constant; if the checksums do not match, print an error and
exit non‑zero so the subsequent tar xvf step is never run, otherwise proceed to
tar and then remove the archive. Ensure the check is fail-fast (exit on
mismatch) and references the exact filename used in the curl/tar commands so it
cannot be bypassed.
Summary by CodeRabbit
New Features
Chores