Repository navigation
Export https://github.com/KittenML/KittenTTS to sherpa-onnx - #2456
Conversation
WalkthroughThis update introduces a new GitHub Actions workflow for exporting and publishing a TTS model, adds several Python scripts for model preparation and inspection, and updates the Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Shell Script (run.sh)
participant Python Scripts
participant Hugging Face
participant GitHub Release
GitHub Actions->>Shell Script (run.sh): Trigger on push or manual dispatch
Shell Script (run.sh)->>Python Scripts: Run generate_voices_bin.py
Shell Script (run.sh)->>Python Scripts: Run generate_tokens.py
Shell Script (run.sh)->>Python Scripts: Run convert_opset.py
Shell Script (run.sh)->>Python Scripts: Run show.py
Shell Script (run.sh)->>Python Scripts: Run add_meta_data.py --model
Shell Script (run.sh)->>Shell Script (run.sh): Prepare model directory
Shell Script (run.sh)->>GitHub Release: Upload archive as release asset
Shell Script (run.sh)->>Hugging Face: Clone repo, push model files
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR integrates KittenTTS nano v0.1 model into sherpa-onnx by adding metadata to the ONNX model file and providing comprehensive testing infrastructure. The main purpose is to make the KittenTTS model compatible with sherpa-onnx runtime.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/kitten-tts/nano_v0_1/test.py | Complete testing script for KittenTTS model with TTS functionality |
| scripts/kitten-tts/nano_v0_1/show.py | Simple utility to display model metadata and I/O specifications |
| scripts/kitten-tts/nano_v0_1/run.sh | Main build script orchestrating the conversion process |
| scripts/kitten-tts/nano_v0_1/generate_voices_bin.py | Converts voice embeddings from npz to binary format |
| scripts/kitten-tts/nano_v0_1/generate_tokens.py | Generates token-to-ID mapping for phonemes |
| scripts/kitten-tts/nano_v0_1/convert_opset.py | Downgrades ONNX opset version for compatibility |
| scripts/kitten-tts/nano_v0_1/add_meta_data.py | Adds sherpa-onnx compatible metadata to the model |
| .github/workflows/export-kitten.yaml | CI/CD workflow for automated model export and release |
|
|
||
| def __call__(self, text: str, voice): | ||
| tokens = phonemize_espeak(text, "en-us") |
There was a problem hiding this comment.
The language code "en-us" is hardcoded. Consider making this configurable through a parameter or constant to support different languages in the future.
| def __call__(self, text: str, voice): | |
| tokens = phonemize_espeak(text, "en-us") | |
| self.language = language | |
| def __call__(self, text: str, voice, language: str = None): | |
| lang = language if language is not None else self.language | |
| tokens = phonemize_espeak(text, lang) |
| show("./model.fp16.onnx") | ||
|
|
||
|
|
There was a problem hiding this comment.
The filename "./model.fp16.onnx" is hardcoded. Consider accepting the filename as a command-line argument for better reusability.
| show("./model.fp16.onnx") | |
| parser = argparse.ArgumentParser(description="Show ONNX model info") | |
| parser.add_argument("filename", nargs="?", default="./model.fp16.onnx", | |
| help="Path to the ONNX model file (default: ./model.fp16.onnx)") | |
| args = parser.parse_args() | |
| show(args.filename) |
|
|
||
| model = onnx.load(args.model) | ||
|
|
||
| style = np.load("./voices.npz") |
There was a problem hiding this comment.
Using list(style.keys())[0] assumes the dictionary has at least one key but doesn't handle the empty case. Consider adding a check or using a more explicit approach to get the first speaker's style shape.
| style = np.load("./voices.npz") | |
| style = np.load("./voices.npz") | |
| if not style.keys(): | |
| raise ValueError("No styles found in voices.npz. The file may be empty or corrupted.") |
|
|
||
| - name: Install Python dependencies | ||
| shell: bash | ||
| run: | |
There was a problem hiding this comment.
[nitpick] The version constraints are very specific (e.g., "numpy<=1.26.4", "onnx==1.16.0"). Consider documenting why these specific versions are required or using more flexible version ranges if possible.
| run: | | |
| run: | | |
| # NOTE: The following version constraints are required due to compatibility issues: | |
| # - numpy<=1.26.4: Newer versions may break compatibility with onnxruntime or other dependencies. | |
| # - onnx==1.16.0: This version is known to work with the current export scripts; newer versions may introduce breaking changes. | |
| # - onnxruntime==1.17.1: Required for compatibility with the exported ONNX models and numpy version. |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
scripts/kitten-tts/nano_v0_1/show.py (1)
7-19: Remove commented example output or move to docstring.The large commented block with example output clutters the code. Consider moving it to a docstring or removing it entirely.
-""" -[key: "onnx.infer" -value: "onnxruntime.quant" -, key: "onnx.quant.pre_process" -value: "onnxruntime.quant" -] -NodeArg(name='input_ids', type='tensor(int64)', shape=[1, 'sequence_length']) -NodeArg(name='style', type='tensor(float)', shape=[1, 256]) -NodeArg(name='speed', type='tensor(float)', shape=[1]) ------ -NodeArg(name='waveform', type='tensor(float)', shape=['num_samples']) -NodeArg(name='duration', type='tensor(int64)', shape=['Castduration_dim_0']) -"""scripts/kitten-tts/nano_v0_1/generate_tokens.py (2)
14-14: Remove unnecessary parentheses in range().Line 14 has redundant parentheses around
symbolsin therange(len((symbols)))expression.- for i in range(len((symbols))): + for i in range(len(symbols)):
12-16: Consider using enumerate() for better readability.The current loop manually tracks indices, but
enumerate()would be more Pythonic and readable.- dicts = {} - for i in range(len(symbols)): - dicts[symbols[i]] = i + dicts = {symbol: i for i, symbol in enumerate(symbols)}.github/workflows/export-kitten.yaml (1)
115-121: Optimize Git LFS tracking configuration.The current approach tracks individual dictionary files separately. Consider using patterns to make this more maintainable.
- git lfs track "*.onnx" - git lfs track af_dict - git lfs track ar_dict - git lfs track cmn_dict - git lfs track da_dict en_dict fa_dict hu_dict ia_dict it_dict lb_dict phondata ru_dict ta_dict - git lfs track ur_dict yue_dict + git lfs track "*.onnx" + git lfs track "*_dict" + git lfs track "phondata"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/export-kitten.yaml(1 hunks).gitignore(1 hunks)scripts/kitten-tts/nano_v0_1/add_meta_data.py(1 hunks)scripts/kitten-tts/nano_v0_1/convert_opset.py(1 hunks)scripts/kitten-tts/nano_v0_1/generate_tokens.py(1 hunks)scripts/kitten-tts/nano_v0_1/generate_voices_bin.py(1 hunks)scripts/kitten-tts/nano_v0_1/run.sh(1 hunks)scripts/kitten-tts/nano_v0_1/show.py(1 hunks)scripts/kitten-tts/nano_v0_1/test.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
📚 Learning: the sherpa-onnx jni library files are stored in hugging face repository at https://huggingface.co/cs...
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
Applied to files:
.gitignore
📚 Learning: windows 10 version 1809 and later, as well as windows 11, include built-in onnx runtime as part of w...
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/readme.md:35-36
Timestamp: 2025-08-06T04:18:28.680Z
Learning: Windows 10 version 1809 and later, as well as Windows 11, include built-in ONNX Runtime as part of Windows ML (WinML) exposed through Windows.AI.MachineLearning.dll and WinRT APIs. However, JNI libraries typically depend on standalone ONNX Runtime C/C++ libraries rather than the Windows ML API.
Applied to files:
scripts/kitten-tts/nano_v0_1/convert_opset.py
🧬 Code Graph Analysis (1)
scripts/kitten-tts/nano_v0_1/generate_voices_bin.py (6)
scripts/kitten-tts/nano_v0_1/convert_opset.py (1)
main(11-26)scripts/kitten-tts/nano_v0_1/generate_tokens.py (1)
main(19-23)scripts/kitten-tts/nano_v0_1/show.py (1)
main(40-41)scripts/kitten-tts/nano_v0_1/add_meta_data.py (1)
main(22-72)scripts/kitten-tts/nano_v0_1/test.py (1)
main(152-193)sherpa-onnx/csrc/offline-speaker-diarization-pyannote-impl.h (1)
f(476-476)
🪛 actionlint (1.7.7)
.github/workflows/export-kitten.yaml
17-17: property "version" is not defined in object type {os: string; python-version: number}
(expression)
🪛 Ruff (0.12.2)
scripts/kitten-tts/nano_v0_1/test.py
14-17: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (9)
.gitignore (1)
145-146: LGTM! Appropriate exclusion of generated files.The addition of
*.npzandvoices.binto .gitignore correctly excludes generated voice embedding files from version control, aligning with the model preparation workflow.scripts/kitten-tts/nano_v0_1/generate_voices_bin.py (1)
18-20: Dictionary comprehension usage is appropriate.The creation of
id2speakerandspeaker2iddictionaries using comprehensions is clean and efficient.scripts/kitten-tts/nano_v0_1/run.sh (2)
19-19: Consider enabling the commented test.The commented test line suggests validation should be performed. Unless there's a specific reason to disable it, consider enabling this test to ensure the pipeline works end-to-end.
Is there a specific reason the test is commented out? If not, consider enabling it:
-# ./test.py --model ./model.fp16.onnx --tokens ./tokens.txt --voice ./voices.bin +./test.py --model ./model.fp16.onnx --tokens ./tokens.txt --voices ./voices.binNote: Verify the correct argument name is
--voices(not--voice) based on the test script's argument parser.
14-18: Good workflow orchestration with proper error handling.The script correctly uses
set -exand executes the pipeline steps in logical order. The workflow from voice generation through metadata addition is well structured.scripts/kitten-tts/nano_v0_1/generate_tokens.py (1)
5-16: LGTM! Token generation logic is correct.The vocabulary generation follows the KittenTTS structure correctly, including padding, punctuation, letters, and IPA symbols in the proper order.
scripts/kitten-tts/nano_v0_1/add_meta_data.py (2)
39-55: LGTM! Metadata structure is comprehensive.The metadata dictionary contains all necessary information for the TTS model, including model type, language, sample rate, speaker mappings, and URLs. The structure aligns well with sherpa-onnx expectations.
59-61: Metadata clearing is intentional and safe
Thewhile len(model.metadata_props): model.metadata_props.pop()pattern is used across all add-metadata/export scripts in this repo to ensure stale entries aren’t accumulated. No other script reads or depends on pre-existing metadata_props, and every required key is re-added from themeta_datadictionary. No further changes needed..github/workflows/export-kitten.yaml (1)
69-88: LGTM! Conditional release logic handles different repository owners correctly.The dual release steps properly handle different scenarios based on repository ownership, using appropriate tokens and repositories.
scripts/kitten-tts/nano_v0_1/test.py (1)
170-194: LGTM! Testing loop and performance metrics are well implemented.The testing logic properly iterates through voices, measures performance with RTF calculations, and saves audio files with descriptive names.
| name: export kitten ${{ matrix.version }} | ||
| runs-on: ${{ matrix.os }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: [ubuntu-latest] | ||
| python-version: ["3.10"] | ||
|
|
There was a problem hiding this comment.
Remove unused matrix.version reference.
The workflow references ${{ matrix.version }} in the job name but doesn't define version in the matrix strategy. This creates an undefined variable reference.
- name: export kitten ${{ matrix.version }}
+ name: export kitten🧰 Tools
🪛 actionlint (1.7.7)
17-17: property "version" is not defined in object type {os: string; python-version: number}
(expression)
🤖 Prompt for AI Agents
In .github/workflows/export-kitten.yaml around lines 17 to 24, the job name uses
the undefined variable matrix.version, which is not declared in the matrix
strategy. Remove the reference to ${ matrix.version } from the job name to avoid
the undefined variable error.
| - name: Install Python dependencies | ||
| shell: bash | ||
| run: | | ||
| pip install "numpy<=1.26.4" onnx==1.16.0 onnxruntime==1.17.1 librosa soundfile piper_phonemize -f https://k2-fsa.github.io/icefall/piper_phonemize.html |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin piper_phonemize version for reproducibility.
The workflow installs piper_phonemize without a specific version, which could lead to inconsistent builds if the package is updated.
- pip install "numpy<=1.26.4" onnx==1.16.0 onnxruntime==1.17.1 librosa soundfile piper_phonemize -f https://k2-fsa.github.io/icefall/piper_phonemize.html
+ pip install "numpy<=1.26.4" onnx==1.16.0 onnxruntime==1.17.1 librosa soundfile "piper_phonemize>=1.0.0" -f https://k2-fsa.github.io/icefall/piper_phonemize.html📝 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.
| pip install "numpy<=1.26.4" onnx==1.16.0 onnxruntime==1.17.1 librosa soundfile piper_phonemize -f https://k2-fsa.github.io/icefall/piper_phonemize.html | |
| pip install "numpy<=1.26.4" onnx==1.16.0 onnxruntime==1.17.1 librosa soundfile "piper_phonemize>=1.0.0" -f https://k2-fsa.github.io/icefall/piper_phonemize.html |
🤖 Prompt for AI Agents
In .github/workflows/export-kitten.yaml at line 36, the pip install command
includes piper_phonemize without a version specifier, risking inconsistent
builds. Modify the command to pin piper_phonemize to a specific version by
adding '==<version>' after the package name, ensuring reproducible and stable
workflow runs.
| style = np.load("./voices.npz") | ||
| style_shape = style[list(style.keys())[0]].shape |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for file loading operations.
The script loads voices.npz and accesses its keys without error handling, which could fail if the file doesn't exist or has an unexpected structure.
- style = np.load("./voices.npz")
- style_shape = style[list(style.keys())[0]].shape
+ try:
+ style = np.load("./voices.npz")
+ if len(style.keys()) == 0:
+ raise ValueError("voices.npz contains no data")
+ style_shape = style[list(style.keys())[0]].shape
+ except FileNotFoundError:
+ raise FileNotFoundError("voices.npz file not found. Ensure generate_voices_bin.py was run first.")
+ except Exception as e:
+ raise RuntimeError(f"Failed to load voice embeddings: {e}")📝 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.
| style = np.load("./voices.npz") | |
| style_shape = style[list(style.keys())[0]].shape | |
| try: | |
| style = np.load("./voices.npz") | |
| if len(style.keys()) == 0: | |
| raise ValueError("voices.npz contains no data") | |
| style_shape = style[list(style.keys())[0]].shape | |
| except FileNotFoundError: | |
| raise FileNotFoundError( | |
| "voices.npz file not found. Ensure generate_voices_bin.py was run first." | |
| ) | |
| except Exception as e: | |
| raise RuntimeError(f"Failed to load voice embeddings: {e}") |
🤖 Prompt for AI Agents
In scripts/kitten-tts/nano_v0_1/add_meta_data.py around lines 28 to 29, add
error handling for loading the voices.npz file and accessing its keys. Wrap the
np.load call and subsequent key access in a try-except block to catch exceptions
like FileNotFoundError or KeyError, and handle them gracefully by logging an
error message or exiting the script with a clear explanation.
| def main(): | ||
| model = onnx.load("kitten_tts_nano_v0_1.onnx") | ||
|
|
||
| # Print current opsets | ||
| for opset in model.opset_import: | ||
| print(f"Domain: '{opset.domain}', Version: {opset.version}") | ||
|
|
||
| # Modify the opset versions (be careful!) | ||
| for opset in model.opset_import: | ||
| if opset.domain == "": # ai.onnx domain | ||
| opset.version = 19 # change from 20 to 19 | ||
| elif opset.domain == "ai.onnx.ml": | ||
| opset.version = 4 # change from 5 to 4 | ||
|
|
||
| # Save the modified model | ||
| onnx.save(model, "model.fp16.onnx") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and make filenames configurable.
The script lacks error handling and uses hardcoded filenames, which reduces flexibility and robustness.
Consider this improvement:
+import sys
+import argparse
+
+def get_args():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("--input", default="kitten_tts_nano_v0_1.onnx")
+ parser.add_argument("--output", default="model.fp16.onnx")
+ return parser.parse_args()
def main():
- model = onnx.load("kitten_tts_nano_v0_1.onnx")
+ args = get_args()
+ try:
+ model = onnx.load(args.input)
+ except Exception as e:
+ print(f"Error loading model {args.input}: {e}")
+ sys.exit(1)
# Print current opsets
for opset in model.opset_import:
print(f"Domain: '{opset.domain}', Version: {opset.version}")
# Modify the opset versions (be careful!)
+ modified = False
for opset in model.opset_import:
if opset.domain == "": # ai.onnx domain
- opset.version = 19 # change from 20 to 19
+ if opset.version == 20:
+ opset.version = 19
+ modified = True
elif opset.domain == "ai.onnx.ml":
- opset.version = 4 # change from 5 to 4
+ if opset.version == 5:
+ opset.version = 4
+ modified = True
+ if not modified:
+ print("Warning: No opset versions were modified")
+
- # Save the modified model
- onnx.save(model, "model.fp16.onnx")
+ try:
+ onnx.save(model, args.output)
+ print(f"Model saved to {args.output}")
+ except Exception as e:
+ print(f"Error saving model: {e}")
+ sys.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.
| def main(): | |
| model = onnx.load("kitten_tts_nano_v0_1.onnx") | |
| # Print current opsets | |
| for opset in model.opset_import: | |
| print(f"Domain: '{opset.domain}', Version: {opset.version}") | |
| # Modify the opset versions (be careful!) | |
| for opset in model.opset_import: | |
| if opset.domain == "": # ai.onnx domain | |
| opset.version = 19 # change from 20 to 19 | |
| elif opset.domain == "ai.onnx.ml": | |
| opset.version = 4 # change from 5 to 4 | |
| # Save the modified model | |
| onnx.save(model, "model.fp16.onnx") | |
| import sys | |
| import argparse | |
| import onnx | |
| def get_args(): | |
| parser = argparse.ArgumentParser() | |
| parser.add_argument("--input", default="kitten_tts_nano_v0_1.onnx") | |
| parser.add_argument("--output", default="model.fp16.onnx") | |
| return parser.parse_args() | |
| def main(): | |
| args = get_args() | |
| try: | |
| model = onnx.load(args.input) | |
| except Exception as e: | |
| print(f"Error loading model {args.input}: {e}") | |
| sys.exit(1) | |
| # Print current opsets | |
| for opset in model.opset_import: | |
| print(f"Domain: '{opset.domain}', Version: {opset.version}") | |
| # Modify the opset versions (be careful!) | |
| modified = False | |
| for opset in model.opset_import: | |
| if opset.domain == "": # ai.onnx domain | |
| if opset.version == 20: | |
| opset.version = 19 | |
| modified = True | |
| elif opset.domain == "ai.onnx.ml": | |
| if opset.version == 5: | |
| opset.version = 4 | |
| modified = True | |
| if not modified: | |
| print("Warning: No opset versions were modified") | |
| try: | |
| onnx.save(model, args.output) | |
| print(f"Model saved to {args.output}") | |
| except Exception as e: | |
| print(f"Error saving model: {e}") | |
| sys.exit(1) |
🤖 Prompt for AI Agents
In scripts/kitten-tts/nano_v0_1/convert_opset.py around lines 11 to 27, the
script lacks error handling and uses hardcoded filenames, limiting flexibility
and robustness. Modify the script to accept input and output filenames as
parameters or command-line arguments, and add try-except blocks around the model
loading, opset modification, and saving steps to catch and log potential errors
gracefully.
| speakers = [ | ||
| "expr-voice-2-m", | ||
| "expr-voice-2-f", | ||
| "expr-voice-3-m", | ||
| "expr-voice-3-f", | ||
| "expr-voice-4-m", | ||
| "expr-voice-4-f", | ||
| "expr-voice-5-m", | ||
| "expr-voice-5-f", | ||
| ] | ||
|
|
||
| id2speaker = {idx: speaker for idx, speaker in enumerate(speakers)} | ||
|
|
||
| speaker2id = {speaker: idx for idx, speaker in id2speaker.items()} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making speaker list data-driven and add validation.
The hardcoded speaker list could become out of sync with the actual contents of voices.npz. Consider loading speakers dynamically or validating they exist.
def main():
if Path("./voices.bin").is_file():
print("./voices.bin exists - skip")
return
- voices = np.load("./voices.npz")
+ try:
+ voices = np.load("./voices.npz")
+ except Exception as e:
+ print(f"Error loading voices.npz: {e}")
+ return
+
+ # Validate that all expected speakers exist
+ available_speakers = set(voices.keys())
+ expected_speakers = set(speakers)
+ missing_speakers = expected_speakers - available_speakers
+ if missing_speakers:
+ print(f"Warning: Missing speakers in voices.npz: {missing_speakers}")
+ print(f"Available speakers: {sorted(available_speakers)}")
+ return
with open("voices.bin", "wb") as f:
for speaker in speakers:
v = voices[speaker]
- # v.shape (1, 256)
+ if v.shape != (1, 256):
+ print(f"Warning: Speaker {speaker} has unexpected shape {v.shape}, expected (1, 256)")
f.write(v.tobytes())📝 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.
| speakers = [ | |
| "expr-voice-2-m", | |
| "expr-voice-2-f", | |
| "expr-voice-3-m", | |
| "expr-voice-3-f", | |
| "expr-voice-4-m", | |
| "expr-voice-4-f", | |
| "expr-voice-5-m", | |
| "expr-voice-5-f", | |
| ] | |
| id2speaker = {idx: speaker for idx, speaker in enumerate(speakers)} | |
| speaker2id = {speaker: idx for idx, speaker in id2speaker.items()} | |
| def main(): | |
| if Path("./voices.bin").is_file(): | |
| print("./voices.bin exists - skip") | |
| return | |
| try: | |
| voices = np.load("./voices.npz") | |
| except Exception as e: | |
| print(f"Error loading voices.npz: {e}") | |
| return | |
| # Validate that all expected speakers exist | |
| available_speakers = set(voices.keys()) | |
| expected_speakers = set(speakers) | |
| missing_speakers = expected_speakers - available_speakers | |
| if missing_speakers: | |
| print(f"Warning: Missing speakers in voices.npz: {missing_speakers}") | |
| print(f"Available speakers: {sorted(available_speakers)}") | |
| return | |
| with open("voices.bin", "wb") as f: | |
| for speaker in speakers: | |
| v = voices[speaker] | |
| if v.shape != (1, 256): | |
| print(f"Warning: Speaker {speaker} has unexpected shape {v.shape}, expected (1, 256)") | |
| f.write(v.tobytes()) |
🤖 Prompt for AI Agents
In scripts/kitten-tts/nano_v0_1/generate_voices_bin.py around lines 7 to 21, the
speaker list is hardcoded which risks being out of sync with the actual
voices.npz data. Modify the code to load the speaker list dynamically from
voices.npz or add validation logic to check that each hardcoded speaker exists
in voices.npz. This ensures the speaker data is always accurate and consistent
with the source file.
| if [ ! -f kitten_tts_nano_v0_1.onnx ]; then | ||
| curl -SL -O https://huggingface.co/KittenML/kitten-tts-nano-0.1/resolve/main/kitten_tts_nano_v0_1.onnx | ||
| fi | ||
|
|
||
| if [ ! -f voices.npz ]; then | ||
| curl -SL -O https://huggingface.co/KittenML/kitten-tts-nano-0.1/resolve/main/voices.npz | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add integrity verification for downloaded files.
Downloading files from external URLs without integrity verification poses security risks. Consider adding checksum validation.
if [ ! -f kitten_tts_nano_v0_1.onnx ]; then
curl -SL -O https://huggingface.co/KittenML/kitten-tts-nano-0.1/resolve/main/kitten_tts_nano_v0_1.onnx
+ # Verify file integrity (add expected SHA256 hash)
+ # echo "expected_hash kitten_tts_nano_v0_1.onnx" | sha256sum -c
fi
if [ ! -f voices.npz ]; then
curl -SL -O https://huggingface.co/KittenML/kitten-tts-nano-0.1/resolve/main/voices.npz
+ # Verify file integrity (add expected SHA256 hash)
+ # echo "expected_hash voices.npz" | sha256sum -c
fi📝 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.
| if [ ! -f kitten_tts_nano_v0_1.onnx ]; then | |
| curl -SL -O https://huggingface.co/KittenML/kitten-tts-nano-0.1/resolve/main/kitten_tts_nano_v0_1.onnx | |
| fi | |
| if [ ! -f voices.npz ]; then | |
| curl -SL -O https://huggingface.co/KittenML/kitten-tts-nano-0.1/resolve/main/voices.npz | |
| fi | |
| if [ ! -f kitten_tts_nano_v0_1.onnx ]; then | |
| curl -SL -O https://huggingface.co/KittenML/kitten-tts-nano-0.1/resolve/main/kitten_tts_nano_v0_1.onnx | |
| # Verify file integrity (add expected SHA256 hash) | |
| # echo "expected_hash kitten_tts_nano_v0_1.onnx" | sha256sum -c | |
| fi | |
| if [ ! -f voices.npz ]; then | |
| curl -SL -O https://huggingface.co/KittenML/kitten-tts-nano-0.1/resolve/main/voices.npz | |
| # Verify file integrity (add expected SHA256 hash) | |
| # echo "expected_hash voices.npz" | sha256sum -c | |
| fi |
🤖 Prompt for AI Agents
In scripts/kitten-tts/nano_v0_1/run.sh around lines 6 to 12, the script
downloads files without verifying their integrity. To fix this, add checksum
verification after each download by including known hash values for the files
and using a tool like sha256sum to validate the downloaded files. If the
checksum does not match, the script should report an error and exit to prevent
using corrupted or tampered files.
| def show(filename): | ||
| model = onnx.load(filename) | ||
| print(model.metadata_props) | ||
|
|
||
| session_opts = onnxruntime.SessionOptions() | ||
| session_opts.log_severity_level = 3 | ||
| sess = onnxruntime.InferenceSession( | ||
| filename, session_opts, providers=["CPUExecutionProvider"] | ||
| ) | ||
| for i in sess.get_inputs(): | ||
| print(i) | ||
|
|
||
| print("-----") | ||
|
|
||
| for i in sess.get_outputs(): | ||
| print(i) | ||
|
|
||
|
|
||
| def main(): | ||
| show("./model.fp16.onnx") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and make filename configurable.
The script should handle potential errors when loading models and allow flexible filename input.
+import sys
+import argparse
+def get_args():
+ parser = argparse.ArgumentParser(description="Inspect ONNX model")
+ parser.add_argument("--model", default="./model.fp16.onnx", help="Path to ONNX model file")
+ return parser.parse_args()
def show(filename):
- model = onnx.load(filename)
- print(model.metadata_props)
+ try:
+ model = onnx.load(filename)
+ print(model.metadata_props)
+ except Exception as e:
+ print(f"Error loading model {filename}: {e}")
+ return False
session_opts = onnxruntime.SessionOptions()
session_opts.log_severity_level = 3
- sess = onnxruntime.InferenceSession(
- filename, session_opts, providers=["CPUExecutionProvider"]
- )
- for i in sess.get_inputs():
- print(i)
+ try:
+ sess = onnxruntime.InferenceSession(
+ filename, session_opts, providers=["CPUExecutionProvider"]
+ )
+ for i in sess.get_inputs():
+ print(i)
- print("-----")
+ print("-----")
- for i in sess.get_outputs():
- print(i)
+ for i in sess.get_outputs():
+ print(i)
+ return True
+ except Exception as e:
+ print(f"Error creating inference session: {e}")
+ return False
def main():
- show("./model.fp16.onnx")
+ args = get_args()
+ if not show(args.model):
+ sys.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.
| def show(filename): | |
| model = onnx.load(filename) | |
| print(model.metadata_props) | |
| session_opts = onnxruntime.SessionOptions() | |
| session_opts.log_severity_level = 3 | |
| sess = onnxruntime.InferenceSession( | |
| filename, session_opts, providers=["CPUExecutionProvider"] | |
| ) | |
| for i in sess.get_inputs(): | |
| print(i) | |
| print("-----") | |
| for i in sess.get_outputs(): | |
| print(i) | |
| def main(): | |
| show("./model.fp16.onnx") | |
| import sys | |
| import argparse | |
| def get_args(): | |
| parser = argparse.ArgumentParser(description="Inspect ONNX model") | |
| parser.add_argument("--model", default="./model.fp16.onnx", help="Path to ONNX model file") | |
| return parser.parse_args() | |
| def show(filename): | |
| try: | |
| model = onnx.load(filename) | |
| print(model.metadata_props) | |
| except Exception as e: | |
| print(f"Error loading model {filename}: {e}") | |
| return False | |
| session_opts = onnxruntime.SessionOptions() | |
| session_opts.log_severity_level = 3 | |
| try: | |
| sess = onnxruntime.InferenceSession( | |
| filename, session_opts, providers=["CPUExecutionProvider"] | |
| ) | |
| for i in sess.get_inputs(): | |
| print(i) | |
| print("-----") | |
| for i in sess.get_outputs(): | |
| print(i) | |
| return True | |
| except Exception as e: | |
| print(f"Error creating inference session: {e}") | |
| return False | |
| def main(): | |
| args = get_args() | |
| if not show(args.model): | |
| sys.exit(1) |
🤖 Prompt for AI Agents
In scripts/kitten-tts/nano_v0_1/show.py around lines 22 to 41, add error
handling around the model loading and inference session creation to catch and
report exceptions gracefully. Also, modify the main function to accept the
filename as a parameter or from command-line arguments instead of hardcoding it,
enabling flexible filename input.
| try: | ||
| from piper_phonemize import phonemize_espeak | ||
| except Exception as ex: | ||
| raise RuntimeError( | ||
| f"{ex}\nPlease run\n" | ||
| "pip install piper_phonemize -f https://k2-fsa.github.io/icefall/piper_phonemize.html" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve exception handling with explicit chaining.
The static analysis correctly identified that the exception should be chained to preserve the original error context.
try:
from piper_phonemize import phonemize_espeak
except Exception as ex:
raise RuntimeError(
f"{ex}\nPlease run\n"
"pip install piper_phonemize -f https://k2-fsa.github.io/icefall/piper_phonemize.html"
- )
+ ) from ex📝 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.
| try: | |
| from piper_phonemize import phonemize_espeak | |
| except Exception as ex: | |
| raise RuntimeError( | |
| f"{ex}\nPlease run\n" | |
| "pip install piper_phonemize -f https://k2-fsa.github.io/icefall/piper_phonemize.html" | |
| ) | |
| try: | |
| from piper_phonemize import phonemize_espeak | |
| except Exception as ex: | |
| raise RuntimeError( | |
| f"{ex}\nPlease run\n" | |
| "pip install piper_phonemize -f https://k2-fsa.github.io/icefall/piper_phonemize.html" | |
| ) from ex |
🧰 Tools
🪛 Ruff (0.12.2)
14-17: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In scripts/kitten-tts/nano_v0_1/test.py around lines 11 to 17, the exception
raised when the import fails does not explicitly chain the original exception,
which loses the original error context. Fix this by adding "from ex" at the end
of the raise statement to explicitly chain the caught exception to the new
RuntimeError.
| fields = line.strip().split() | ||
| if len(fields) == 2: | ||
| token, idx = fields | ||
| ans[token] = int(idx) | ||
| else: | ||
| assert len(fields) == 1, (len(fields), line) | ||
| ans[" "] = int(fields[0]) | ||
| return ans |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle edge case in token parsing more robustly.
The token parsing assumes that single-field lines represent spaces, but this assumption should be validated or documented.
if len(fields) == 2:
token, idx = fields
ans[token] = int(idx)
else:
- assert len(fields) == 1, (len(fields), line)
- ans[" "] = int(fields[0])
+ if len(fields) == 1:
+ # Handle space character which may appear as single field
+ ans[" "] = int(fields[0])
+ else:
+ raise ValueError(f"Invalid token line format: {line.strip()}")📝 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.
| fields = line.strip().split() | |
| if len(fields) == 2: | |
| token, idx = fields | |
| ans[token] = int(idx) | |
| else: | |
| assert len(fields) == 1, (len(fields), line) | |
| ans[" "] = int(fields[0]) | |
| return ans | |
| fields = line.strip().split() | |
| if len(fields) == 2: | |
| token, idx = fields | |
| ans[token] = int(idx) | |
| else: | |
| if len(fields) == 1: | |
| # Handle space character which may appear as single field | |
| ans[" "] = int(fields[0]) | |
| else: | |
| raise ValueError(f"Invalid token line format: {line.strip()}") | |
| return ans |
🤖 Prompt for AI Agents
In scripts/kitten-tts/nano_v0_1/test.py around lines 65 to 72, the code assumes
that lines with a single field represent spaces without validation. To fix this,
add a check to confirm that the single field indeed corresponds to a space token
or document this assumption clearly. Alternatively, handle unexpected
single-field tokens gracefully by raising an informative error or skipping them,
ensuring more robust token parsing.
|
|
||
| tokens = list(tokens) | ||
|
|
||
| token_ids = [self.token2id[i] for i in tokens] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing tokens.
The token ID lookup could fail if phonemization produces tokens not in the vocabulary.
- token_ids = [self.token2id[i] for i in tokens]
+ try:
+ token_ids = [self.token2id[i] for i in tokens]
+ except KeyError as e:
+ raise ValueError(f"Token '{e.args[0]}' not found in vocabulary. Available tokens: {len(self.token2id)}") from e📝 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.
| token_ids = [self.token2id[i] for i in tokens] | |
| try: | |
| token_ids = [self.token2id[i] for i in tokens] | |
| except KeyError as e: | |
| raise ValueError( | |
| f"Token '{e.args[0]}' not found in vocabulary. Available tokens: {len(self.token2id)}" | |
| ) from e |
🤖 Prompt for AI Agents
In scripts/kitten-tts/nano_v0_1/test.py at line 130, the code directly looks up
token IDs without handling the case where a token might not exist in the
token2id dictionary. Modify the list comprehension to include error handling by
checking if each token exists in token2id before accessing it, and handle
missing tokens gracefully, for example by skipping them, substituting a default
ID, or raising a clear error with a descriptive message.
Will add C++ runtime in a separate PR.
Summary by CodeRabbit
New Features
Chores