Skip to content

Add Qwen3-ASR support - #3399

Merged
csukuangfj merged 6 commits into
k2-fsa:masterfrom
Wasser1462:feature/qwen3-asr-support
Mar 25, 2026
Merged

csukuangfj merged 6 commits into
k2-fsa:masterfrom
Wasser1462:feature/qwen3-asr-support

Conversation

@Wasser1462

@Wasser1462 Wasser1462 commented Mar 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds offline Qwen3-ASR to sherpa-onnx: model path, tokenizer wiring, and hooks in sherpa-onnx-offline and sherpa-onnx-vad-with-offline-asr. Tested with Qwen3-ASR-0.6B and Qwen3-ASR-1.7B ONNX builds.

Models & export

Item Link
ONNX (ModelScope) https://modelscope.cn/models/zengshuishui/Qwen3-ASR-onnx
Conversion / export scripts https://github.com/Wasser1462/Qwen3-ASR-onnx

Scope & caveats

  • CPU only for now; RTF numbers below are CPU, 4 threads, provider=cpu.
  • No GPU numbers yet (hardware unavailable on my side).
  • Part of the work was done with AI tooling; review and nitpicks are very welcome—happy to revise anything that does not match project conventions.

Example: sherpa-onnx-offline (0.6B)

./bin/sherpa-onnx-offline \
  --qwen3-asr-conv-frontend=/path/to/model_0.6B/conv_frontend.onnx \
  --qwen3-asr-encoder=/path/to/model_0.6B/encoder.int8.onnx \
  --qwen3-asr-decoder=/path/to/model_0.6B/decoder.int8.onnx \
  --qwen3-asr-tokenizer=/path/to/tokenizer \
  --qwen3-asr-max-total-len=512 \
  --qwen3-asr-max-new-tokens=128 \
  --qwen3-asr-temperature=1e-6 \
  --qwen3-asr-top-p=0.8 \
  --qwen3-asr-seed=42 \
  --num-threads=4 \
  --provider=cpu \
  /path/to/zh.wav

Example output (text may include model-style prefixes such as language Chinese<asr_text>):

{
  "lang": "",
  "emotion": "",
  "event": "",
  "text": "language Chinese<asr_text>开放时间:早上九点至下午五点。",
  "timestamps": [],
  "durations": [],
  "tokens": [
    "language",
    " Chinese",
    "<asr_text>",
    "开放",
    "时间",
    ":",
    "早上",
    "九",
    "点",
    "至",
    "下午",
    "五",
    "点",
    "。"
  ],
  "ys_log_probs": [],
  "words": []
}

Example: sherpa-onnx-vad-with-offline-asr (1.7B)

./bin/sherpa-onnx-vad-with-offline-asr \
  --silero-vad-model=/path/to/silero_vad.onnx \
  --qwen3-asr-conv-frontend=/path/to/model_1.7B/conv_frontend.onnx \
  --qwen3-asr-encoder=/path/to/model_1.7B/encoder.int8.onnx \
  --qwen3-asr-decoder=/path/to/model_1.7B/decoder.int8.onnx \
  --qwen3-asr-tokenizer=/path/to/tokenizer \
  --qwen3-asr-max-total-len=512 \
  --qwen3-asr-max-new-tokens=128 \
  --qwen3-asr-temperature=1e-6 \
  --qwen3-asr-top-p=0.8 \
  --qwen3-asr-seed=42 \
  --num-threads=4 \
  --provider=cpu \
  /path/to/song.wav

Example output (time range → transcript per VAD segment):

24.134 -- 47.452: language Chinese<asr_text>仍然倚在失眠夜,望天边星宿,仍然听见小提琴,如泣似诉在挑逗。为何只剩一弯月,留在我的天空?这晚以后,音讯隔住。
48.070 -- 71.452: language Chinese<asr_text>人如天上的明月,是不可擁有;情如曲過止,遺留無可挽救,再分別。為何只是失望,填密我的空虛?這晚夜沒有吻別。
...

CPU benchmark (reference only)

Machine: Intel Xeon Gold 6448Y, x86_64, 32 cores — 4 threads, CPU provider.

Model Binary / input Audio (s) Elapsed (s) RTF
0.6B offline, zh.wav 5.616 0.689 0.123
0.6B VAD + offline, song.wav 291.200 26.458 0.091
0.6B offline, multi-file set 221.935 33.385 0.150
1.7B offline, far_4.wav 9.079 2.039 0.225
1.7B VAD + offline, song.wav 291.200 35.316 0.121
1.7B offline, multi-file set 221.935 46.849 0.211

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Qwen3-ASR offline speech recognition support across C, C++, and Python APIs
    • New example applications demonstrating Qwen3-ASR usage in C, C++, and Python
    • Added comprehensive model configuration with support for generation parameters (max tokens, temperature, top-p sampling)
    • Extended Swift and WebAssembly API support for Qwen3-ASR
    • Added tokenizer with UTF-8 aware encoding/decoding and streaming capabilities
  • Chores

    • Updated build configuration to include Qwen3-ASR implementation modules

@dosubot dosubot Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Mar 24, 2026
@coderabbitai

coderabbitai Bot commented Mar 24, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR introduces comprehensive Qwen3-ASR offline speech recognition support to sherpa-onnx across C, C++, and Python APIs. It adds model configuration structures, a tokenizer implementation, LLM-based recognizer pipeline with KV-cache management, and example programs in three languages. Supporting changes include FunASR-nano tokenizer template refactoring and WebAssembly size assertion updates.

Changes

Cohort / File(s) Summary
Qwen3-ASR Core Model & Config
sherpa-onnx/csrc/offline-qwen3-asr-model-config.h, sherpa-onnx/csrc/offline-qwen3-asr-model-config.cc, sherpa-onnx/csrc/offline-qwen3-asr-model.h, sherpa-onnx/csrc/offline-qwen3-asr-model.cc
New configuration struct with model paths and decoding parameters; model implementation with three-stage forward passes (conv_frontend, encoder, LLM decoder), KV-cache creation/update, and CUDA I/O binding support.
Tokenizer Implementation
sherpa-onnx/csrc/qwen-asr-tokenizer.h, sherpa-onnx/csrc/qwen-asr-tokenizer.cc
Qwen3-ASR tokenizer with UTF-8 awareness, BPE merging, special token handling, and platform-specific resource loading for Android/OHOS.
Recognition Pipeline
sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.h, sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc
Offline recognizer implementation with audio mel-feature preprocessing, silent-frame trimming, prompt construction, and autoregressive text generation with temperature/top-p sampling.
OfflineRecognizer Factory & Config
sherpa-onnx/csrc/offline-recognizer-impl.cc, sherpa-onnx/csrc/offline-model-config.h, sherpa-onnx/csrc/offline-model-config.cc
Factory dispatch extension to instantiate Qwen3-ASR recognizer; config struct extended with qwen3_asr member; Register/Validate/ToString logic updated.
C API Bindings
sherpa-onnx/c-api/c-api.h, sherpa-onnx/c-api/c-api.cc
New exported C structures for Qwen3-ASR model config; config population with defaults.
C++ API Bindings
sherpa-onnx/c-api/cxx-api.h, sherpa-onnx/c-api/cxx-api.cc
C++ wrapper struct for Qwen3-ASR config with defaults; mapping logic from C++ to C API.
Python API Bindings
sherpa-onnx/python/csrc/offline-qwen3-asr-model-config.h, sherpa-onnx/python/csrc/offline-qwen3-asr-model-config.cc, sherpa-onnx/python/csrc/offline-model-config.cc, sherpa-onnx/python/sherpa_onnx/__init__.py, sherpa-onnx/python/sherpa_onnx/offline_recognizer.py
Pybind registration for Qwen3-ASR config; Python classmethod from_qwen3_asr(...) for convenience; config export in __init__.py.
Example Programs
c-api-examples/qwen3-asr-c-api.c, cxx-api-examples/qwen3-asr-cxx-api.cc, python-api-examples/offline-qwen3-asr-decode-files.py
Standalone examples demonstrating offline Qwen3-ASR transcription in C, C++, and Python with timing/RTF computation.
Build Configuration
c-api-examples/CMakeLists.txt, cxx-api-examples/CMakeLists.txt, sherpa-onnx/csrc/CMakeLists.txt, sherpa-onnx/python/csrc/CMakeLists.txt
New executable targets for C/C++ examples; source files added to core library and Python module builds.
Swift API Bindings
swift-api-examples/SherpaOnnx.swift
New factory function sherpaOnnxOfflineQwen3ASRModelConfig(...); extended sherpaOnnxOfflineModelConfig(...) signature with optional qwen3Asr parameter.
FunASR-Nano Tokenizer Refactoring
sherpa-onnx/csrc/funasr-nano-tokenizer.h, sherpa-onnx/csrc/funasr-nano-tokenizer.cc
Android/OHOS platform-specific constructors and Init overloads consolidated into single templated versions with explicit instantiation.
WebAssembly Updates
wasm/nodejs/sherpa-onnx-wasm-nodejs.cc
Static assertion for SherpaOnnxOfflineModelConfig size updated to include SherpaOnnxOfflineQwen3ASRModelConfig.

Sequence Diagram

sequenceDiagram
    participant User
    participant App as Application
    participant Recognizer as OfflineRecognizer
    participant Model as OfflineQwen3ASRModel
    participant Tokenizer as QwenAsrTokenizer
    participant ONNX as ONNX Runtime
    
    User->>App: Provide WAV file + config
    App->>Recognizer: Create with Qwen3-ASR config
    Recognizer->>Model: Load model (conv/encoder/decoder)
    Recognizer->>Tokenizer: Initialize tokenizer
    Recognizer->>ONNX: Create sessions
    
    App->>Recognizer: Read WAV audio
    Recognizer->>Model: ForwardConvFrontend(audio_features)
    Model->>ONNX: Run conv_frontend session
    ONNX-->>Model: Conv output
    
    Recognizer->>Model: ForwardEncoder(conv_output, attention_mask)
    Model->>ONNX: Run encoder session
    ONNX-->>Model: Audio features
    
    Recognizer->>Model: CreateEmptyKVCache()
    Model-->>Recognizer: KV cache tensors
    
    Recognizer->>Model: ForwardLLM(input_ids, audio_features, cache_kv)
    Model->>ONNX: Run decoder session (autoregressive loop)
    ONNX-->>Model: Logits + KV deltas
    
    Model->>Recognizer: ApplyKvDeltaInplace(cache_kv, kv_delta)
    Recognizer->>Tokenizer: Decode token_ids to text
    Tokenizer-->>Recognizer: Recognized text
    Recognizer-->>App: Recognition result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • #3221: Parallel support for new offline ASR model family with equivalent OfflineModelConfig, factory dispatch, and API binding modifications.
  • #2936: FunASR-nano model support added via similar changes to OfflineModelConfig, recognizer factory, and Python/C bindings.
  • #2773: C API extension with new offline ASR model config structures and example programs.

Suggested reviewers

  • csukuangfj

Poem

🐰 A tokenizer hops through bytes so fine,
With KV-cache whispers and models in line,
Qwen3 arrives in the sherpa warren,
Audio transcribed—our feast grows broader'n!
C, C++, Python—all tongues now speak true,
The rabbit's proud feast of ASR: beep boop 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.48% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add Qwen3-ASR support' directly and clearly summarizes the main feature being added across the entire changeset, which is support for the Qwen3-ASR offline speech recognition model.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello, 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 significantly expands the capabilities of sherpa-onnx by integrating offline Qwen3-ASR models. It provides the necessary infrastructure, including model loading, configuration, and a specialized tokenizer, to enable accurate speech recognition for these models. The addition of C, C++, and Python examples ensures ease of adoption for developers looking to leverage Qwen3-ASR in their applications.

Highlights

  • Qwen3-ASR Integration: Added comprehensive support for offline Qwen3-ASR models, including both 0.6B and 1.7B ONNX builds, enabling robust speech recognition capabilities.
  • API Examples: Introduced new C, C++, and Python API examples to demonstrate the usage and integration of Qwen3-ASR within the sherpa-onnx framework.
  • Model Configuration: Implemented dedicated model configuration structures and logic for Qwen3-ASR, covering frontend, encoder, decoder, and tokenizer paths, along with decoding parameters like max sequence length, new tokens, temperature, and top-p sampling.
  • Tokenizer Implementation: Developed a custom Qwen-ASR tokenizer with byte-pair encoding (BPE) and UTF-8 handling, crucial for processing the model's specific tokenization scheme.
  • Feature Extraction Enhancements: Modified the offline stream's feature extraction to support Whisper-style processing, aligning with the requirements of Qwen3-ASR models.
  • Current Limitations: The current implementation is optimized for CPU-only inference, with GPU support planned for future iterations.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@csukuangfj

Copy link
Copy Markdown
Collaborator

Thanks! I am reviewing it.

@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (4)
sherpa-onnx/csrc/CMakeLists.txt (1)

260-263: Please add a small core smoke test for the new Qwen3 tokenizer/config path.

These sources extend sherpa-onnx-core, but the gtest list later in this file is unchanged. A round-trip tokenizer test or config-validation smoke test would keep this path from regressing silently.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sherpa-onnx/csrc/CMakeLists.txt` around lines 260 - 263, Add a small
gtest-based smoke test that exercises the new Qwen3 tokenizer/config code and
wire it into the existing test target: create a test file (e.g.,
tests/test_qwen3_tokenizer.cc) that performs a simple round-trip or
config-validation using the tokenizer/config types implemented in
qwen-asr-tokenizer.cc and offline-qwen3-asr-model-config.cc (for example,
serialize/deserialize a config or tokenize->detokenize a sample). Then add that
test source to the gtest sources list in CMakeLists.txt (the same place where
other test .cc files are listed) so the test is built and run with the existing
sherpa-onnx-core tests. Ensure the test target name matches the project’s
convention and the test includes the relevant headers for qwen-asr-tokenizer and
offline-qwen3-asr-model-config.
wasm/nodejs/sherpa-onnx-wasm-nodejs.cc (1)

48-49: Add a dedicated size invariant for SherpaOnnxOfflineQwen3ASRModelConfig.

This file already keeps per-struct layout checks for the other offline model configs. Relying only on the aggregate SherpaOnnxOfflineModelConfig assert makes future drift in the new Qwen3 layout harder to diagnose.

Possible follow-up
 static_assert(sizeof(SherpaOnnxOfflineFunASRNanoModelConfig) == 13 * 4, "");
+static_assert(sizeof(SherpaOnnxOfflineQwen3ASRModelConfig) == 9 * 4, "");
 static_assert(sizeof(SherpaOnnxOfflineDolphinModelConfig) == 4, "");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wasm/nodejs/sherpa-onnx-wasm-nodejs.cc` around lines 48 - 49, Add a dedicated
size invariant for SherpaOnnxOfflineQwen3ASRModelConfig similar to the existing
per-struct layout checks: add a static_assert (or the same ASSERT used for other
structs) that verifies sizeof(SherpaOnnxOfflineQwen3ASRModelConfig) equals the
expected byte size so layout drift is caught independently of
SherpaOnnxOfflineModelConfig; place this check alongside the other struct sizeof
assertions and update the expected size constant to the correct value for Qwen3
if necessary.
sherpa-onnx/c-api/cxx-api.h (1)

582-592: Optional: add field-level doc comments for API consistency.

This new public config struct is the only nearby one without per-field comments; adding brief docs would keep cxx-api.h style uniform.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sherpa-onnx/c-api/cxx-api.h` around lines 582 - 592, Add brief per-field doc
comments to the public struct OfflineQwen3ASRModelConfig to match the style of
nearby config structs: document conv_frontend, encoder, decoder, tokenizer as
path/identifier strings for model components; max_total_len and max_new_tokens
as token-length limits with their default semantics; temperature and top_p as
sampling/hyperparameter controls; and seed as RNG seed. Place single-line
comments above each member (referencing OfflineQwen3ASRModelConfig and the
specific members conv_frontend, encoder, decoder, tokenizer, max_total_len,
max_new_tokens, temperature, top_p, seed) following the file’s existing comment
style.
sherpa-onnx/csrc/offline-stream.cc (1)

53-71: Consider extracting shared frame/mel option initialization.

Lines 54-66 largely duplicate Lines 73-87. A small helper for common option assignment would reduce drift risk between whisper/non-whisper branches.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sherpa-onnx/csrc/offline-stream.cc` around lines 53 - 71, Extract the
duplicated frame/mel option assignment into a small helper (e.g.,
PopulateFrameAndMelOptions) that takes the source config and a reference to the
target options (or the object owning them) and sets frame_opts.dither,
snip_edges, samp_freq, frame_shift_ms, frame_length_ms, remove_dc_offset,
window_type and mel_opts.num_bins, high_freq, low_freq, is_librosa; then replace
the duplicated blocks with calls to this helper before constructing
knf::WhisperFeatureOptions and creating whisper_fbank_ (use the same helper from
the non-whisper branch as well so opts_.frame_opts and opts_.mel_opts are
populated consistently).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@sherpa-onnx/c-api/c-api.h`:
- Line 1068: You changed the public C ABI by adding the qwen3_asr field to
SherpaOnnxOfflineModelConfig (which is embedded in
SherpaOnnxOfflineRecognizerConfig); instead of extending the existing structs,
introduce a versioned config and constructor: create
SherpaOnnxOfflineModelConfigV2 that includes qwen3_asr, add a corresponding
factory/creation function (e.g., sherpa_onnx_offline_model_config_v2_create or a
recognizer create that accepts the V2 type), and keep the original
SherpaOnnxOfflineModelConfig and its create/recognizer APIs unchanged so older
binaries remain compatible; ensure the library checks which struct version is
passed (or exposes distinct symbols) and document that callers must opt into V2
to use qwen3_asr.

In `@sherpa-onnx/csrc/offline-qwen3-asr-model-config.cc`:
- Around line 83-95: The current validation checks only vocab.json and
merges.txt but not tokenizer_config.json, causing later initialization failures
in the tokenizer reader; update the same validation block that uses the
tokenizer variable and FileExists to also verify tokenizer_config.json exists,
and on failure call SHERPA_ONNX_LOGE with a similar message referencing
tokenizer.c_str() and return false so partially copied tokenizers are rejected
early.

In `@sherpa-onnx/csrc/offline-recognizer-impl.cc`:
- Around line 223-225: The manager-backed factory overload
OfflineRecognizerImpl::Create(Manager *mgr, const OfflineRecognizerConfig&
config, ...) is missing the Qwen3 dispatch; add the same conditional that checks
config.model_config.qwen3_asr.conv_frontend.empty() (or non-empty as done in the
other overload) and return
std::make_unique<OfflineRecognizerQwen3ASRImpl>(config) for that case so
Manager-backed creation can instantiate Qwen3-ASR; ensure you follow the exact
symbol names OfflineRecognizerImpl::Create, OfflineRecognizerConfig, and
OfflineRecognizerQwen3ASRImpl when locating the code to modify.

In `@sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc`:
- Around line 821-846: The loop is passing the wrong cache position (cur_len
already includes the newly pushed token) causing KV cache misalignment; change
the cache position passed to next_cache_position to be context_len + (step - 1)
(or equivalently use cur_len - 1) so the first generated token is written at
position context_len; update the creation of next_cache_position (the Ort::Value
created for cache positions used in the decoder call) to use that corrected
value instead of cur_len, keeping other tensors (one_tensor,
next_attention_mask) unchanged.

In `@sherpa-onnx/csrc/qwen-asr-tokenizer.cc`:
- Around line 1144-1187: The tokenizer initialization currently doesn't validate
presence of required Qwen control tokens, so missing tokens like "<|audio_pad|>"
or "<|im_end|>" let construction succeed with invalid eos/pad IDs and later
break InitPromptTemplateIds; update the initialization (after ParseAddedTokens
and BuildSpecialTokens and before returning) to explicitly check token2id_ for
required markers ("<|audio_pad|>", "<|im_end|>", and any other Qwen control
tokens used by OfflineRecognizerQwen3ASRImpl::InitPromptTemplateIds) and if any
are missing, log an error via the same logging mechanism and return/fail
initialization (or throw) so the caller cannot proceed with invalid
eos_token_id_, pad_token_id_, or im_end_token_id_; reference token2id_,
eos_token_id_, pad_token_id_, im_end_token_id_ to locate where to add these
validations.
- Around line 1269-1297: Decode() currently calls DecodeBytes(buffer)
unconditionally and can emit a partial UTF-8 codepoint at sequence end; change
QwenAsrTokenizer::Decode to reuse the pending-byte handling from
GetTokenStringStreaming: before calling DecodeBytes(buffer) (both inside the
special-token branch and the final flush), inspect buffer bytes for the last
valid UTF-8 boundary, call DecodeBytes only on the complete-prefix portion, and
save the trailing incomplete bytes into a member (e.g., pending_bytes_ or the
existing streaming buffer) instead of appending them; ensure
IsSpecialToken/IsSkippableSpecialToken handling also flushes only complete bytes
and moves leftovers to the pending storage.

In `@sherpa-onnx/python/csrc/offline-model-config.cc`:
- Around line 68-71: The new binding added for OfflineModelConfig places
qwen3_asr as an optional positional argument that shifts existing positional
parameters; fix by making qwen3_asr keyword-only or preserve positional order
via a factory lambda. Locate the pybind11 init/binding for OfflineModelConfig in
offline-model-config.cc (the init signature that includes qwen3_asr,
telespeech_ctc, tokens, num_threads, etc.) and either insert a py::kw_only()
before the qwen3_asr py::arg(...) so callers must use qwen3_asr=... or replace
the init<> with a small lambda/factory that accepts the old positional
parameters in the original order and handles qwen3_asr as an optional keyword,
applying the same change for the other occurrence noted around the later block
(lines ~88-92).

In `@sherpa-onnx/python/sherpa_onnx/offline_recognizer.py`:
- Around line 415-416: The public constructor/factory in offline_recognizer.py
exposes a configurable feature_dim but
OfflineRecognizerQwen3ASRImpl::InitFeatConfig() always uses 128; fix this by
making the API truthful: either remove/hardcode feature_dim to 128 in the
factory/constructor or validate the passed feature_dim and raise a ValueError if
it is not 128. Update the spots that define the signature (the
constructor/factory that currently takes feature_dim and the other duplicated
location) to enforce or hardcode 128 so the Python config cannot diverge from
InitFeatConfig().

---

Nitpick comments:
In `@sherpa-onnx/c-api/cxx-api.h`:
- Around line 582-592: Add brief per-field doc comments to the public struct
OfflineQwen3ASRModelConfig to match the style of nearby config structs: document
conv_frontend, encoder, decoder, tokenizer as path/identifier strings for model
components; max_total_len and max_new_tokens as token-length limits with their
default semantics; temperature and top_p as sampling/hyperparameter controls;
and seed as RNG seed. Place single-line comments above each member (referencing
OfflineQwen3ASRModelConfig and the specific members conv_frontend, encoder,
decoder, tokenizer, max_total_len, max_new_tokens, temperature, top_p, seed)
following the file’s existing comment style.

In `@sherpa-onnx/csrc/CMakeLists.txt`:
- Around line 260-263: Add a small gtest-based smoke test that exercises the new
Qwen3 tokenizer/config code and wire it into the existing test target: create a
test file (e.g., tests/test_qwen3_tokenizer.cc) that performs a simple
round-trip or config-validation using the tokenizer/config types implemented in
qwen-asr-tokenizer.cc and offline-qwen3-asr-model-config.cc (for example,
serialize/deserialize a config or tokenize->detokenize a sample). Then add that
test source to the gtest sources list in CMakeLists.txt (the same place where
other test .cc files are listed) so the test is built and run with the existing
sherpa-onnx-core tests. Ensure the test target name matches the project’s
convention and the test includes the relevant headers for qwen-asr-tokenizer and
offline-qwen3-asr-model-config.

In `@sherpa-onnx/csrc/offline-stream.cc`:
- Around line 53-71: Extract the duplicated frame/mel option assignment into a
small helper (e.g., PopulateFrameAndMelOptions) that takes the source config and
a reference to the target options (or the object owning them) and sets
frame_opts.dither, snip_edges, samp_freq, frame_shift_ms, frame_length_ms,
remove_dc_offset, window_type and mel_opts.num_bins, high_freq, low_freq,
is_librosa; then replace the duplicated blocks with calls to this helper before
constructing knf::WhisperFeatureOptions and creating whisper_fbank_ (use the
same helper from the non-whisper branch as well so opts_.frame_opts and
opts_.mel_opts are populated consistently).

In `@wasm/nodejs/sherpa-onnx-wasm-nodejs.cc`:
- Around line 48-49: Add a dedicated size invariant for
SherpaOnnxOfflineQwen3ASRModelConfig similar to the existing per-struct layout
checks: add a static_assert (or the same ASSERT used for other structs) that
verifies sizeof(SherpaOnnxOfflineQwen3ASRModelConfig) equals the expected byte
size so layout drift is caught independently of SherpaOnnxOfflineModelConfig;
place this check alongside the other struct sizeof assertions and update the
expected size constant to the correct value for Qwen3 if necessary.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 74682a7b-5f3a-4267-9d1e-0088f5643bd6

📥 Commits

Reviewing files that changed from the base of the PR and between 5092b44 and 0f57f87.

📒 Files selected for processing (29)
  • c-api-examples/CMakeLists.txt
  • c-api-examples/qwen3-asr-c-api.c
  • cxx-api-examples/CMakeLists.txt
  • cxx-api-examples/qwen3-asr-cxx-api.cc
  • python-api-examples/offline-qwen3-asr-decode-files.py
  • sherpa-onnx/c-api/c-api.cc
  • sherpa-onnx/c-api/c-api.h
  • sherpa-onnx/c-api/cxx-api.cc
  • sherpa-onnx/c-api/cxx-api.h
  • sherpa-onnx/csrc/CMakeLists.txt
  • sherpa-onnx/csrc/offline-model-config.cc
  • sherpa-onnx/csrc/offline-model-config.h
  • sherpa-onnx/csrc/offline-qwen3-asr-model-config.cc
  • sherpa-onnx/csrc/offline-qwen3-asr-model-config.h
  • sherpa-onnx/csrc/offline-qwen3-asr-model.cc
  • sherpa-onnx/csrc/offline-qwen3-asr-model.h
  • sherpa-onnx/csrc/offline-recognizer-impl.cc
  • sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc
  • sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.h
  • sherpa-onnx/csrc/offline-stream.cc
  • sherpa-onnx/csrc/qwen-asr-tokenizer.cc
  • sherpa-onnx/csrc/qwen-asr-tokenizer.h
  • sherpa-onnx/python/csrc/CMakeLists.txt
  • sherpa-onnx/python/csrc/offline-model-config.cc
  • sherpa-onnx/python/csrc/offline-qwen3-asr-model-config.cc
  • sherpa-onnx/python/csrc/offline-qwen3-asr-model-config.h
  • sherpa-onnx/python/sherpa_onnx/__init__.py
  • sherpa-onnx/python/sherpa_onnx/offline_recognizer.py
  • wasm/nodejs/sherpa-onnx-wasm-nodejs.cc

Comment thread sherpa-onnx/c-api/c-api.h
Comment thread sherpa-onnx/csrc/offline-qwen3-asr-model-config.cc
Comment thread sherpa-onnx/csrc/offline-recognizer-impl.cc
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc
Comment thread sherpa-onnx/csrc/qwen-asr-tokenizer.cc
Comment thread sherpa-onnx/csrc/qwen-asr-tokenizer.cc
Comment thread sherpa-onnx/python/csrc/offline-model-config.cc Outdated
Comment thread sherpa-onnx/python/sherpa_onnx/offline_recognizer.py
Comment thread sherpa-onnx/csrc/offline-qwen3-asr-model.cc Outdated
Comment thread sherpa-onnx/csrc/offline-qwen3-asr-model.cc
Comment thread sherpa-onnx/csrc/offline-qwen3-asr-model.cc
Comment thread sherpa-onnx/csrc/offline-qwen3-asr-model.cc Outdated
Comment thread sherpa-onnx/csrc/offline-qwen3-asr-model.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-impl.cc
Comment thread sherpa-onnx/csrc/offline-recognizer-impl.cc
Comment thread sherpa-onnx/csrc/qwen-asr-tokenizer.h Outdated
Comment thread sherpa-onnx/csrc/qwen-asr-tokenizer.h Outdated
Comment thread sherpa-onnx/csrc/offline-stream.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated

@csukuangfj csukuangfj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! Looks great to me.

Left only some minor comments.

Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated
Comment thread sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc Outdated
Comment thread sherpa-onnx/python/sherpa_onnx/offline_recognizer.py Outdated
Comment thread sherpa-onnx/csrc/offline-qwen3-asr-model-config.h

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (2)
sherpa-onnx/csrc/qwen-asr-tokenizer.cc (2)

1246-1272: ⚠️ Potential issue | 🟠 Major

Reuse the streaming UTF-8 boundary handling in Decode().

DecodeBytes(buffer) is appended directly here, so a truncated multibyte sequence can leak invalid UTF-8 into result.text. This also skips the boundary handling that GetTokenStringStreaming() already applies around special tokens, so <|im_end|>/<|im_start|> can merge byte fragments across token boundaries.

🔧 Suggested fix
 std::string QwenAsrTokenizer::Decode(const std::vector<int64_t> &token_ids) {
   std::string ans;
   std::string buffer;
+  std::string pending_bytes;
 
   for (int64_t id : token_ids) {
@@
     const std::string &token = id2token_[static_cast<size_t>(id)];
     if (IsSpecialToken(token)) {
-      if (IsSkippableSpecialToken(token)) {
-        continue;
-      }
       if (!buffer.empty()) {
-        ans.append(DecodeBytes(buffer));
+        pending_bytes.append(DecodeBytes(buffer));
+        ans.append(
+            ConsumeAvailableUtf8(&pending_bytes, /*flush_incomplete=*/true));
         buffer.clear();
       }
+      if (IsSkippableSpecialToken(token)) {
+        continue;
+      }
       ans.append(token);
     } else {
       buffer.append(token);
@@
 
   if (!buffer.empty()) {
-    ans.append(DecodeBytes(buffer));
+    pending_bytes.append(DecodeBytes(buffer));
   }
+
+  ans.append(
+      ConsumeAvailableUtf8(&pending_bytes, /*flush_incomplete=*/true));
 
   return ans;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sherpa-onnx/csrc/qwen-asr-tokenizer.cc` around lines 1246 - 1272,
QwenAsrTokenizer::Decode currently appends DecodeBytes(buffer) directly which
can leak truncated UTF-8 and merge byte fragments across token boundaries;
replace direct DecodeBytes calls with the streaming-safe helper
GetTokenStringStreaming so boundary handling is reused: when hitting a special
token (after flushing buffer) call GetTokenStringStreaming on the special token
and on the flushed buffer instead of DecodeBytes, and likewise use
GetTokenStringStreaming for the final buffer flush at the end of Decode; update
references around id2token_, IsSpecialToken, and IsSkippableSpecialToken to
ensure every appended piece goes through GetTokenStringStreaming.

1136-1179: ⚠️ Potential issue | 🟠 Major

Fail fast when the required Qwen control tokens are missing.

OfflineRecognizerQwen3ASRImpl::InitPromptTemplateIds() builds prompts with <|im_start|>, <|audio_start|>, <|audio_pad|>, <|audio_end|>, and <|im_end|> in sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc, Lines 318-332. If any of those tokens are absent here, Encode() silently byte-encodes the literal marker text instead, and eos_token_id_ can remain -1, so generation proceeds with a broken prompt rather than failing early.

🔧 Suggested guard
   ParseAddedTokens(config_content, &token2id_, &id2token_);
   BuildSpecialTokens(token2id_, &special_tokens_);
@@
-  auto it = token2id_.find("<|im_end|>");
-  if (it != token2id_.end()) {
-    eos_token_id_ = it->second;
-    im_end_token_id_ = it->second;
-  }
+  auto require_token = [&](const char *token, int64_t *out = nullptr) {
+    auto it2 = token2id_.find(token);
+    if (it2 == token2id_.end()) {
+      SHERPA_ONNX_LOGE("Missing required Qwen control token: %s", token);
+      SHERPA_ONNX_EXIT(-1);
+    }
+    if (out) {
+      *out = it2->second;
+    }
+  };
+
+  require_token("<|im_start|>");
+  require_token("<|audio_start|>");
+  require_token("<|audio_pad|>");
+  require_token("<|audio_end|>");
+  require_token("<|im_end|>", &eos_token_id_);
+  im_end_token_id_ = eos_token_id_;
+
+  auto it = token2id_.find("<|padding|>");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sherpa-onnx/csrc/qwen-asr-tokenizer.cc` around lines 1136 - 1179, The
tokenizer currently may leave control token IDs like
eos_token_id_/im_end_token_id_ unset and allow generation to proceed; after
ParseAddedTokens/BuildSpecialTokens, validate that token2id_ contains the
required Qwen control tokens ("<|im_start|>", "<|audio_start|>",
"<|audio_pad|>", "<|audio_end|>", "<|im_end|>") and set the corresponding IDs
(e.g., eos_token_id_, im_end_token_id_, pad_token_id_ as appropriate); if any
required token is missing, fail fast by returning an error or throwing (or
calling processLogger/error handling used in your codebase) with a clear message
identifying the missing token(s) so InitPromptTemplateIds()/Encode() cannot
silently proceed with byte-encoding the literal markers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@sherpa-onnx/csrc/qwen-asr-tokenizer.cc`:
- Around line 1246-1272: QwenAsrTokenizer::Decode currently appends
DecodeBytes(buffer) directly which can leak truncated UTF-8 and merge byte
fragments across token boundaries; replace direct DecodeBytes calls with the
streaming-safe helper GetTokenStringStreaming so boundary handling is reused:
when hitting a special token (after flushing buffer) call
GetTokenStringStreaming on the special token and on the flushed buffer instead
of DecodeBytes, and likewise use GetTokenStringStreaming for the final buffer
flush at the end of Decode; update references around id2token_, IsSpecialToken,
and IsSkippableSpecialToken to ensure every appended piece goes through
GetTokenStringStreaming.
- Around line 1136-1179: The tokenizer currently may leave control token IDs
like eos_token_id_/im_end_token_id_ unset and allow generation to proceed; after
ParseAddedTokens/BuildSpecialTokens, validate that token2id_ contains the
required Qwen control tokens ("<|im_start|>", "<|audio_start|>",
"<|audio_pad|>", "<|audio_end|>", "<|im_end|>") and set the corresponding IDs
(e.g., eos_token_id_, im_end_token_id_, pad_token_id_ as appropriate); if any
required token is missing, fail fast by returning an error or throwing (or
calling processLogger/error handling used in your codebase) with a clear message
identifying the missing token(s) so InitPromptTemplateIds()/Encode() cannot
silently proceed with byte-encoding the literal markers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 24a0f63f-3912-43cc-be70-79ddacccc9d7

📥 Commits

Reviewing files that changed from the base of the PR and between 5783675 and 9544e7e.

📒 Files selected for processing (14)
  • sherpa-onnx/csrc/funasr-nano-tokenizer.cc
  • sherpa-onnx/csrc/funasr-nano-tokenizer.h
  • sherpa-onnx/csrc/offline-qwen3-asr-model-config.cc
  • sherpa-onnx/csrc/offline-qwen3-asr-model-config.h
  • sherpa-onnx/csrc/offline-qwen3-asr-model.cc
  • sherpa-onnx/csrc/offline-recognizer-impl.cc
  • sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc
  • sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.h
  • sherpa-onnx/csrc/qwen-asr-tokenizer.cc
  • sherpa-onnx/csrc/qwen-asr-tokenizer.h
  • sherpa-onnx/python/csrc/offline-model-config.cc
  • sherpa-onnx/python/csrc/offline-qwen3-asr-model-config.cc
  • sherpa-onnx/python/sherpa_onnx/offline_recognizer.py
  • swift-api-examples/SherpaOnnx.swift
🚧 Files skipped from review as they are similar to previous changes (6)
  • sherpa-onnx/csrc/offline-recognizer-impl.cc
  • sherpa-onnx/python/csrc/offline-qwen3-asr-model-config.cc
  • sherpa-onnx/python/sherpa_onnx/offline_recognizer.py
  • sherpa-onnx/python/csrc/offline-model-config.cc
  • sherpa-onnx/csrc/offline-qwen3-asr-model-config.cc
  • sherpa-onnx/csrc/offline-qwen3-asr-model-config.h

@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@csukuangfj csukuangfj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants