Skip to content

Fix Qwen3-ASR hotwords handling#3477

Merged
csukuangfj merged 1 commit into
k2-fsa:masterfrom
csukuangfj:fix-qwen3-asr-hotwords
Apr 7, 2026
Merged

Fix Qwen3-ASR hotwords handling#3477
csukuangfj merged 1 commit into
k2-fsa:masterfrom
csukuangfj:fix-qwen3-asr-hotwords

Conversation

@csukuangfj

@csukuangfj csukuangfj commented Apr 7, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes
    • Improved hotwords handling in speech recognition to properly use model-level configuration defaults when stream-specific options are not provided.

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 7, 2026
@coderabbitai

coderabbitai Bot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0d3067ab-cc53-414a-a4e1-739f42ecb0bd

📥 Commits

Reviewing files that changed from the base of the PR and between 178cee0 and 565d097.

📒 Files selected for processing (2)
  • dotnet-examples/non-streaming-qwen3-asr-decode-files/Program.cs
  • sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc

📝 Walkthrough

Walkthrough

Documentation link update in a .NET example and refinement to hotword prompt generation in Qwen3 ASR implementation. The hotwords feature now defaults to model-level configuration when stream-specific options are absent, with minor code reformatting applied to both files.

Changes

Cohort / File(s) Summary
Documentation & Formatting
dotnet-examples/non-streaming-qwen3-asr-decode-files/Program.cs
Updated reference link from qwen3-asr.html to qwen3-asr, normalized line endings, and adjusted file header formatting. No functional changes.
Qwen3 ASR Configuration
sherpa-onnx/csrc/offline-recognizer-qwen3-asr-impl.cc
Modified hotwords prompt generation to use model-level default (qwen3_config.hotwords) when stream lacks explicit hotwords option. Minor code reformatting in token post-processing and condition checking without behavioral changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Suggested labels

size:XS

Poem

🐰 A link hops to its fresh new home,
Hotwords now find defaults to roam,
Code tidied up with gentle care,
Small tweaks float lightly through the air! ✨

✨ 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.

@csukuangfj csukuangfj merged commit 23185f8 into k2-fsa:master Apr 7, 2026
0 of 27 checks passed

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request implements a fallback to global hotwords in the Qwen3 ASR decoder and includes minor documentation and formatting improvements. A potential crash was identified in the C++ implementation where a null configuration value could be passed to a string constructor.

Comment on lines +712 to +713
stream->HasOption("hotwords") ? stream->GetOption("hotwords")
: qwen3_config.hotwords);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The logic correctly implements a fallback to the global qwen3_config.hotwords when no per-stream hotwords are provided. However, if qwen3_config.hotwords is a const char * (as defined in the C API struct SherpaOnnxOfflineQwen3ASRModelConfig) and it happens to be nullptr, this ternary expression will cause a crash when constructing the std::string argument for Qwen3FormatHotwordsForPrompt. While the C++ configuration usually initializes this to an empty string, adding a null check or ensuring it defaults to "" is safer.

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

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant