Upload Vietnamese ASR models - #3159
Conversation
Summary of ChangesHello @csukuangfj, 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 introduces a new Vietnamese Automatic Speech Recognition (ASR) model, Highlights
Changelog
Ignored Files
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 registers a new Vietnamese Zipformer 30M model across the build and deployment pipeline by adding workflow packaging steps, APK script entries, and Kotlin API configuration. One model packaging step is disabled, and a missing directory operation is fixed in an existing APK script entry. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Code Review
This pull request adds a new Vietnamese ASR model. The changes in the Python script and Kotlin code correctly define and configure the new model. I have one suggestion to improve the naming convention for the model's short name to make it more concise.
| idx=49, | ||
| lang="vi", | ||
| lang2="Vietnamese", | ||
| short_name="zipformer_vi_30M_int8_2026_02_09", |
There was a problem hiding this comment.
The short_name is quite long and includes the date, which is often omitted in other short names in this file for brevity. A more concise name would improve readability and consistency, especially since this name is used in the generated APK filename.
| short_name="zipformer_vi_30M_int8_2026_02_09", | |
| short_name="zipformer_vi_30M", |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/apk/generate-vad-asr-apk-script.py`:
- Around line 828-843: The Model entry for idx=49
("sherpa-onnx-zipformer-vi-30M-int8-2026-02-09") fails to remove bpe.model like
the similar Vietnamese model at idx=26, causing unnecessary APK bloat; update
the cmd block in the Model(...) for idx=49 to remove bpe.model (in the same way
idx=26 does) so only needed artifacts (encoder.int8.onnx, decoder.onnx,
joiner.int8.onnx, tokens.txt) are packaged, ensuring the script deletes
bpe.model before finishing.
| Model( | ||
| model_name="sherpa-onnx-zipformer-vi-30M-int8-2026-02-09", | ||
| idx=49, | ||
| lang="vi", | ||
| lang2="Vietnamese", | ||
| short_name="zipformer_vi_30M_int8_2026_02_09", | ||
| cmd=""" | ||
| pushd $model_name | ||
|
|
||
| rm -rfv test_wavs | ||
|
|
||
| ls -lh | ||
|
|
||
| popd | ||
| """, | ||
| ), |
There was a problem hiding this comment.
bpe.model is not removed, unlike the other Vietnamese zipformer model (idx=26).
The existing Vietnamese zipformer at idx=26 (line 474) removes bpe.model to reduce APK size. The Kotlin config for type 49 only references encoder.int8.onnx, decoder.onnx, joiner.int8.onnx, and tokens.txt — so bpe.model is unused and will unnecessarily inflate the APK.
Proposed fix
pushd $model_name
rm -rfv test_wavs
+ rm -fv bpe.model
ls -lh📝 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.
| Model( | |
| model_name="sherpa-onnx-zipformer-vi-30M-int8-2026-02-09", | |
| idx=49, | |
| lang="vi", | |
| lang2="Vietnamese", | |
| short_name="zipformer_vi_30M_int8_2026_02_09", | |
| cmd=""" | |
| pushd $model_name | |
| rm -rfv test_wavs | |
| ls -lh | |
| popd | |
| """, | |
| ), | |
| Model( | |
| model_name="sherpa-onnx-zipformer-vi-30M-int8-2026-02-09", | |
| idx=49, | |
| lang="vi", | |
| lang2="Vietnamese", | |
| short_name="zipformer_vi_30M_int8_2026_02_09", | |
| cmd=""" | |
| pushd $model_name | |
| rm -rfv test_wavs | |
| rm -fv bpe.model | |
| ls -lh | |
| popd | |
| """, | |
| ), |
🤖 Prompt for AI Agents
In `@scripts/apk/generate-vad-asr-apk-script.py` around lines 828 - 843, The Model
entry for idx=49 ("sherpa-onnx-zipformer-vi-30M-int8-2026-02-09") fails to
remove bpe.model like the similar Vietnamese model at idx=26, causing
unnecessary APK bloat; update the cmd block in the Model(...) for idx=49 to
remove bpe.model (in the same way idx=26 does) so only needed artifacts
(encoder.int8.onnx, decoder.onnx, joiner.int8.onnx, tokens.txt) are packaged,
ensuring the script deletes bpe.model before finishing.
The model is from
https://huggingface.co/hynt/Zipformer-30M-RNNT-6000h
Please see the doc at
https://k2-fsa.github.io/sherpa/onnx/pretrained_models/offline-transducer/zipformer-transducer-models.html#sherpa-onnx-zipformer-vi-30m-int8-2026-02-09-vietnamese
Summary by CodeRabbit
New Features
Improvements
Deprecated