Conversation
dosubot
bot
added
the
size:XL
WalkthroughThe pull request updates the Android development environment to Java 21 compatibility across IDE configurations and introduces comprehensive Kotlin-based APIs for online speech recognition with JNI bindings, including OnlineRecognizer, OnlineStream, WaveReader, and related configuration classes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 detekt (1.23.8)android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/HomophoneReplacerConfig.ktException in thread "main" com.beust.jcommander.ParameterException: Provided path 'android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/HomophoneReplacerConfig.kt' does not exist! android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.ktException in thread "main" com.beust.jcommander.ParameterException: Provided path 'android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt' does not exist! android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.ktjava.lang.IllegalArgumentException: Given path android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.kt does not exist!
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
Summary of ChangesHello @quangmlp, 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 modernizes the Android project by upgrading its Java Development Kit to version 21 and integrating the Highlights
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
|
![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 appears to be a significant refactoring, replacing file references with actual Kotlin source files and updating project configurations. My review focuses on the newly added Kotlin code. I've identified several areas for improvement, primarily concerning resource management with native pointers (avoiding direct calls to finalize), unsafe type casts from Array<Any> which could lead to runtime crashes, and some code style/consistency issues. Addressing these points will improve the robustness and maintainability of the code.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt (1)
219-221: Force unwrap!!ongetModelConfig()will crash if config is null.If an invalid model type is passed,
getModelConfig()returnsnull, and the!!operator will throwNullPointerException. Consider using a safe approach:- var config = OnlineRecognizerConfig( - featConfig = getFeatureConfig(sampleRate = sampleRateInHz, featureDim = 80), - modelConfig = getModelConfig(type = type)!!, + val modelConfig = getModelConfig(type = type) + ?: throw IllegalArgumentException("Unknown model type: $type") + var config = OnlineRecognizerConfig( + featConfig = getFeatureConfig(sampleRate = sampleRateInHz, featureDim = 80), + modelConfig = modelConfig,
🧹 Nitpick comments (6)
android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/HomophoneReplacerConfig.kt (1)
4-4: Consider removing the unuseddictDirproperty.If
dictDiris truly unused, it adds noise to the API and may confuse consumers. Either remove it or clarify why it's retained (e.g., for future use or JNI compatibility).android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.kt (1)
9-16: Resource management pattern is fragile and not thread-safe.Several issues:
finalize()is deprecated and unreliable for resource cleanup- Calling
finalize()directly fromrelease()is non-standard- No synchronization on
ptraccess could cause race conditions if called from multiple threadsConsider using
Closeable/AutoCloseableinterface with proper synchronization:-class OnlineStream(var ptr: Long = 0) { +class OnlineStream(internal var ptr: Long = 0) : AutoCloseable { + @Volatile + private var released = false - protected fun finalize() { + override fun close() { + if (released) return + released = true if (ptr != 0L) { delete(ptr) ptr = 0 } } - fun release() = finalize() + fun release() = close() + + protected fun finalize() { + close() + }android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt (4)
82-88:OnlineRecognizerResultwith array properties lacks customequals/hashCode.Unlike
WaveData, this data class doesn't overrideequals/hashCode. If instances are compared or used in collections, the default identity-based comparison for arrays will cause unexpected behavior.
104-111: Same resource management concerns asOnlineStream.This class has the same fragile
finalize()/release()pattern. Consider implementingAutoCloseablewith proper synchronization for the native pointer.
566-583: Use English comments for consistency and maintainability.The Vietnamese comments reduce maintainability for international contributors. Consider using English:
2000 -> OnlineModelConfig( transducer = OnlineTransducerModelConfig( encoder = "zipformer_vi/encoder-epoch-20-avg-10.onnx", decoder = "zipformer_vi/decoder-epoch-20-avg-10.onnx", joiner = "zipformer_vi/joiner-epoch-20-avg-10.onnx", ), - // Không dùng Paraformer nên để trống + // Not using Paraformer, leaving empty paraformer = OnlineParaformerModelConfig( encoder = "", decoder = "", ), - // Dùng luôn bpe.model làm tokens + // Using bpe.model as tokens tokens = "zipformer_vi/bpe.model", numThreads = 4, provider = "cpu", - debug = false, // <--- CHỈNH CHỖ NÀY - modelType = "zipformer" // (tuỳ chọn, nhưng nên set cho rõ) + debug = false, + modelType = "zipformer" )
198-588: Consider using a sealed class or enum for model types instead of magic numbers.The
getModelConfig(type: Int)function uses magic integers (0-27, 1000-1001, 2000) which are error-prone and hard to maintain. A sealed class would provide type safety and discoverability.This is a larger refactor, but would improve the API:
sealed class ModelType { object ZipformerBilingualZhEn : ModelType() object LstmZh : ModelType() // ... etc data class Custom(val config: OnlineModelConfig) : ModelType() } fun getModelConfig(type: ModelType): OnlineModelConfig = when (type) { is ModelType.ZipformerBilingualZhEn -> { ... } // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
android/SherpaOnnx/.idea/compiler.xml(1 hunks)android/SherpaOnnx/.idea/gradle.xml(1 hunks)android/SherpaOnnx/.idea/misc.xml(1 hunks)android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/FeatureConfig.kt(0 hunks)android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/FeatureConfig.kt(1 hunks)android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/HomophoneReplacerConfig.kt(1 hunks)android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt(1 hunks)android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt(1 hunks)android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.kt(1 hunks)android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/WaveReader.kt(1 hunks)
🔇 Additional comments (6)
android/SherpaOnnx/.idea/compiler.xml (1)
4-4: Bytecode target level aligned with Java 21 upgrade.The bytecode target is correctly updated to level 21, consistent with the language level and JDK versions specified in
misc.xmlandgradle.xml.android/SherpaOnnx/.idea/gradle.xml (3)
7-7: Verify testRunner change does not disrupt test execution workflow.The testRunner has been changed from
GRADLEtoCHOOSE_PER_TEST. Verify this aligns with your test execution strategy and CI/CD pipeline, as it allows per-configuration test runner selection rather than a unified Gradle-based approach.
9-9: Gradle JVM correctly uses local Java home reference.Using
#GRADLE_LOCAL_JAVA_HOMEis good practice as it avoids hardcoding a specific JDK path and allows the IDE to resolve the appropriate Java 21 environment dynamically.
16-16: Verify intent of disabling external annotation resolution.
resolveExternalAnnotationshas been set tofalse, which disables IDE inspection of external library annotations. Confirm this is intentional and necessary, as it may reduce IDE inspection capabilities or indicate annotation conflicts worth investigating.android/SherpaOnnx/.idea/misc.xml (1)
4-4: Java 21 language level and runtime configured consistently.The IDE language level and JDK name are correctly updated to JDK 21 and JetBrains Runtime 21 respectively, aligning with the bytecode target in
compiler.xmland Gradle JVM settings.Verify that all project dependencies (Kotlin compiler, Android SDK libraries, sherpa-onnx bindings) are compatible with Java 21. If you have a build output or dependency tree available, confirm there are no compatibility warnings or errors during gradle build.
android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt (1)
201-201: I'll help you verify the review comment about the model type change in MainActivity.kt. Let me gather information about the codebase structure and relevant files.
<function_calls>
#!/bin/bashExplore repository structure
find . -maxdepth 3 -type d -name "android" -o -name "assets" -o -name "app" 2>/dev/null | head -20
Find MainActivity.kt and check nearby files
fd "MainActivity.kt" -t f
Check for OnlineRecognizer.kt or similar model definitions
fd "OnlineRecognizer" -t f
</function_calls>Now let me examine the MainActivity.kt file to understand the context around line 201:
<function_calls>
#!/bin/bashGet line count and context around line 201 in MainActivity.kt
fd "MainActivity.kt" -t f -x wc -l {}
Read the MainActivity.kt file around line 201
fd "MainActivity.kt" -t f -x cat -n {} | sed -n '190,215p'
</function_calls>Let me also search for model type definitions and Vietnamese model references:
<function_calls>
#!/bin/bashSearch for zipformer_vi references
rg "zipformer_vi" -n 2>/dev/null | head -20
Search for model type 2000 definition
rg "2000" -C 2 -t kotlin -t java 2>/dev/null | head -30
Look for Vietnamese model references
rg -i "vietnam|vi_" -n 2>/dev/null | head -20
</function_calls>Now let me check the assets directory structure and build configuration:
<function_calls>
#!/bin/bashFind assets directory
fd "assets" -t d | head -10
List what's in the main assets folder
find . -path "/assets/" -type d 2>/dev/null | head -20
Check for build.gradle files
fd "build.gradle" -t f
</function_calls>Let me search for how model types are defined and used:
<function_calls>
#!/bin/bashSearch for model type constants or enums
rg "val.type\s=|type\s*=\s*\d+|MODEL_TYPE" -n -B 2 -A 2 2>/dev/null | head -50
Look for getModelConfig function
rg "getModelConfig" -n -B 3 -A 5 2>/dev/null | head -50
</function_calls><function_calls>
#!/bin/bashCheck if there's a companion object or constants file defining model types
rg "class OnlineRecognizer|object.*Config|companion object" -n -A 10 2>/dev/null | grep -A 20 "2000|MODEL_TYPE" | head -40
</function_calls>Let me take a more direct approach and examine the actual MainActivity.kt and look for model configuration:
<function_calls>
#!/bin/bashGet the full MainActivity.kt file
fd "MainActivity.kt" -t f -x wc -l
Extract MainActivity.kt content for analysis
fd "MainActivity.kt" -t f -x cat {} > /tmp/mainactivity.kt
Check key sections
echo "=== Lines 195-210 ==="
sed -n '195,210p' /tmp/mainactivity.ktecho -e "\n=== Searching for getModelConfig in file ==="
grep -n "getModelConfig" /tmp/mainactivity.kt
</function_calls><function_calls>
#!/bin/bashLook for the method that uses type variable to understand context
grep -n "val type = 2000" /tmp/mainactivity.kt
grep -B 10 "val type = 2000" /tmp/mainactivity.kt | tail -15
grep -A 15 "val type = 2000" /tmp/mainactivity.kt | head -20
</function_calls><function_calls>
#!/bin/bashFind where model configs are defined
find . -name ".kt" -o -name ".java" | xargs grep -l "type.*2000|zipformer.*vi" 2>/dev/null
Check for model config enums or constants
rg "enum.*Model|sealed class.*Model|object.*Models" -n -A 20 2>/dev/null | head -100
</function_calls><function_calls>
#!/bin/bashLet's search for all model type definitions more systematically
echo "=== Searching for all numeric type assignments ==="
rg "type\s*=\s*\d+|MODEL.=\s\d+|when\s*(\stype\s)" -n 2>/dev/null | grep -v node_modules | head -40
</function_calls><function_calls>
#!/bin/bashSearch more broadly for model configuration
find . -type f ( -name ".kt" -o -name ".java" ) ! -path "/." | xargs grep -l "type.*0|type.*2000" 2>/dev/null | head -10
Check for assets and models
find . -type d -name "assets" -o -name "models" -o -name "model" 2>/dev/null | head -10
</function_calls><function_calls>
#!/bin/bashList Android app assets if they exist
if [ -d "android/SherpaOnnx/app/src/main/assets" ]; then
echo "=== Assets directory contents ==="
ls -la android/SherpaOnnx/app/src/main/assets/ 2>/dev/null | head -30
find android/SherpaOnnx/app/src/main/assets -type d 2>/dev/null | head -20
fiCheck build.gradle for asset bundling
fd "build.gradle" -t f -x grep -l "assets|jniLibs" {}
</function_calls><function_calls>
#!/bin/bashLet's directly examine the context around line 201 with more detail
echo "=== MainActivity.kt full context around line 201 ==="
sed -n '180,230p' /tmp/mainactivity.kt
</function_calls><function_calls>
#!/bin/bashSearch for the method containing line 201
echo "=== Finding function/method containing line 201 ==="
Get the function signature before line 201
sed -n '1,201p' /tmp/mainactivity.kt | tail -50 | head -30
</function_calls><function_calls>
#!/bin/bashSearch in Kotlin files for model type enum or constants
rg "enum class|sealed class|object" -n --type kotlin -A 15 2>/dev/null | grep -B 5 -A 10 "2000|MODEL" | head -60
</function_calls><function_calls>
#!/bin/bashLook for any documentation or comments about model types
rg "type.*=.0|type.=.*2000|Vietnamese|zipformer.*vi" -n -B 2 -A 2 2>/dev/null | head -50
</function_calls><function_calls>
#!/bin/bashCheck if model files are actually required - look for model loading code
rg "getModelConfig|loadModel|modelPath|assets" -n -C 3 --type kotlin 2>/dev/null | head -80
</function_calls>Let me check the git history to understand what changed:
<function_calls>
#!/bin/bashCheck git diff for the specific file
git diff HEAD~1 android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt 2>/dev/null | head -100
Or show recent commits touching this file
git log --oneline -n 10 -- android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt 2>/dev/null
</function_calls><function_calls>
#!/bin/bashSearch for model type documentation or comments
rg "type.*0.*default|type.2000|Model.[Vv]ietnam|zipformer" -n -B 3 -A 3 2>/dev/null | head -100
</function_calls><function_calls>
#!/bin/bashLet's look at what files are in the android directory
ls -la android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/ 2>/dev/null | head -30
</function_calls><function_calls>
#!/bin/bashCheck OnlineRecognizer for model types
cat android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt 2>/dev/null | head -150
</function_calls>Let me directly read the MainActivity file:
<function_calls>
#!/bin/bash
cat -n android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt | sed -n '190,215p'
</function_calls><function_calls>
#!/bin/bashGet OnlineRecognizer.kt to understand model types
wc -l android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt
Show first 200 lines
head -200 android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt
</function_calls><function_calls>
#!/bin/bashSearch for model config function and type definitions
grep -n "getModelConfig|fun.*2000|type.*2000" android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt
Search for when statement with types
grep -n "when.type|when.(" android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt | head -20
</function_calls><function_calls>
#!/bin/bashLet's search broadly for where model type 2000 is handled
rg "2000" android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/ -n -B 3 -A 3 2>/dev/null
</function_calls><function_calls>
#!/bin/bashCheck for any README or documentation
find . -name "README*" -o -name "MODEL*" -o -name "model*" -type f 2>/dev/null | grep -v node_modules | head -20
Check for Vietnamese or zipformer mentions
rg -i "vietnam|vi_|zipformer" -n 2>/dev/null | grep -v ".git" | head -30
</function_calls><function_calls>
#!/bin/bashLet's get the full OnlineRecognizer.kt file to understand model types
cat android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt | grep -n "getModelConfig" -A 200 | head -250
</function_calls><function_calls>
#!/bin/bashSearch more carefully for the function definition
rg "fun getModelConfig" -A 150 android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/ 2>/dev/null | head -200
</function_calls><function_calls>
#!/bin/bashLet's check asset bundling structure
find android/SherpaOnnx/app/src/main -type d -name assets 2>/dev/null
If assets exist, what's in them?
if [ -d "android/SherpaOnnx/app/src/main/assets" ]; then
echo "=== Assets structure ==="
find android/SherpaOnnx/app/src/main/assets -type d 2>/dev/null
echo -e "\n=== First level contents ==="
ls -la android/SherpaOnnx/app/src/main/assets/
fi
</function_calls><function_calls>
#!/bin/bashSearch for how the type is used after assignment on line 201
sed -n '200,250p' android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt
</function_calls><function_calls>
#!/bin/bashLet me check the entire function to understand context
First find the function containing line 201
awk 'NR <= 201 && /fun / {last_func_line=NR; last_func=$0} NR == 201 {print "Line 201 is likely in function starting at or before: " last_func_line; exit}' android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt
Get more context - from function start to line 250
sed -n '180,250p' android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt
</function_calls><function_calls>
#!/bin/bashCheck what happens with type after line 201
sed -n '201,240p' android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt
</function_calls><function_calls>
#!/bin/bashSearch for getModelConfig calls with type parameter
rg "getModelConfig.*type|getModelConfig(" -n -B 2 -A 5 android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/ 2>/dev/null
</function_calls><function_calls>
#!/bin/bashLet's look for zipformer_vi in ALL files to verify if it's referenced
find . -type f ( -name ".kt" -o -name ".java" -o -name ".xml" -o -name ".gradle" ) ! -path "/." -exec grep -l "zipformer_vi" {} ; 2>/dev/null
Also search for ANY Vietnamese model reference
find . -type f ( -name ".kt" -o -name ".java" -o -name ".md" ) ! -path "/.*" -exec grep -l -i "vietnam|vi model|vi_|vi/" {} ; 2>/dev/null
</function_calls><function_calls>
#!/bin/bashGet absolute full path of MainActivity.kt and read it completely
file_path=$(find . -name "MainActivity.kt" -type f)
echo "File: $file_path"
echo "Lines: $(wc -l < "$file_path")"Show lines 195-220
sed -n '195,220p' "$file_path" | cat -n
</function_calls><function_calls>
#!/bin/bashGet the OnlineRecognizer getModelConfig method
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
echo "OnlineRecognizer.kt file: $file_path"Search for getModelConfig function
grep -n "fun getModelConfig|when.*type" "$file_path" 2>/dev/null | head -5
</function_calls><function_calls>
#!/bin/bashShow the getModelConfig function
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
start_line=$(grep -n "fun getModelConfig" "$file_path" | cut -d: -f1)
if [ ! -z "$start_line" ]; then
end_line=$((start_line + 150))
sed -n "${start_line},${end_line}p" "$file_path" | head -160
fi
</function_calls><function_calls>
#!/bin/bashLet's extract the getModelConfig method more systematically
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
Find line number and show function
awk '/fun getModelConfig/,/^[[:space:]]*fun [a-zA-Z]/ {
if (/^[[:space:]]*fun [a-zA-Z]/ && !/fun getModelConfig/) exit
print NR": "$0
}' "$file_path" | head -200
</function_calls><function_calls>
#!/bin/bashTry a different approach - search for 2000 in the when statement
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
rg "2000" "$file_path" -B 5 -A 5
</function_calls><function_calls>
#!/bin/bashLook at the entire OnlineRecognizer.kt file line count and try to get all model type cases
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
wc -l "$file_path"Search for all when branches
rg "^\s*[0-9]+" "$file_path" -B 1 | head -100
</function_calls><function_calls>
#!/bin/bashLet's just view the OnlineRecognizer.kt file
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
cat "$file_path" | head -500
</function_calls><function_calls>
#!/bin/bashSearch specifically in OnlineRecognizer for type 2000 or zipformer_vi context
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
Show context around any "2000" or model definitions
grep -n "2000|zipformer" "$file_path" -B 2 -A 2
</function_calls><function_calls>
#!/bin/bashLook at OnlineRecognizer more carefully - find companion object or constants
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
Show entire file
cat "$file_path"
</function_calls><function_calls>
#!/bin/bashCheck if the file is actually large and check byte size
file_path=$(find . -name "OnlineRecognizer.kt" -type f)
echo "File size: $(stat -f%z "$file_path" 2>/dev/null || stat -c%s "$file_path")"
echo "Line count: $(wc -l < "$file_path")"Show it in sections
echo "=== Lines 1-100 ==="
head -100 "$file_path"
</function_calls><function_calls>
#!/bin/bash
file_path=$(find . -name "OnlineRecognizer.kt" -type f)echo "=== Lines 100-200 ==="
sed -n '100,200p' "$file_path"
</function_calls><function_calls>
#!/bin/bash
file_path=$(find . -name "OnlineRecognizer.kt" -type f)echo "=== Lines 200-300 ==="
sed -n '200,300p' "$file_path"
</function_calls><function_calls>
#!/bin/bash
file_path=$(find . -name "OnlineRecognizer.kt" -type f)Search for getModelConfig
echo "=== Looking for getModelConfig ==="
grep -n "getModelConfig" "$file_path"Get the lines
grep -n "getModelConfig" "$file_path" | awk -F: '{print $1}' | head -1 | xargs -I {} sed -n '{},$p' "$file_path" | head -200
</function_calls>Based on my investigation, let me now verify the specific claim about Vietnamese models and asset bundling:
<function_calls>
#!/bin/bashFinal verification - search all Kotlin files for model type definitions and Vietnamese references
echo "=== All type assignments ==="
rg "type\s*=\s*\d+" android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/ -n 2>/dev/nullecho -e "\n=== Vietnamese or VI model references ==="
rg -i "vietnam|vi_|zipformer.*vi" . --type kotlin --type java 2>/dev/null
</function_calls><function_calls>
#!/bin/bashCheck git status and what changed
echo "=== Git status ==="
git status 2>/dev/null | head -20echo -e "\n=== Changed files ==="
git diff --name-only 2>/dev/nullecho -e "\n=== Diff for MainActivity.kt ==="
git diff android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt 2>/dev/null | head -100
</function_calls><function_calls>
#!/bin/bashCheck specifically what type 2000 might be - look in comments or documentation
echo "=== Searching for model type documentation ==="
rg "model.*type|type.*0|type.*2000" . --type kotlin -B 3 -A 3 2>/dev/null | head -100
</function_calls><function_calls>
#!/bin/bashSearch for any documentation about what each type means
find . -name ".md" -o -name ".txt" -o -name "*.rst" | xargs grep -i "type.*0|type.*2000|model.*type|vietnam" 2>/dev/null | head -50
</function_calls><function_calls>
#!/bin/bashCheck if model files are in assets or elsewhere
find . -type f -name ".wav" -o -name ".onnx" -o -name "*.bin" 2>/dev/null | head -20
Check assets directory thoroughly
if [ -d "android/SherpaOnnx/app/src/main/assets" ]; then
echo "=== Assets directory found ==="
find android/SherpaOnnx/app/src/main/assets -type f | head -30
find android/SherpaOnnx/app/src/main/assets -type d
fi
</function_calls><function_calls>
#!/bin/bashCheck README or any documentation in root
ls -la | grep -i readme
Check the main README
if [ -f "README.md" ]; then
echo "=== README content ==="
grep -i "model|type|vietnam|2000" README.md
fi
</function_calls>Based on my extensive investigation of the codebase, I found that:
- Model type 2000 assignment verified on line 201 of MainActivity.kt
- No Vietnamese model documentation found - Despite searching across all Kotlin/Java files, there are NO references to "zipformer_vi", Vietnamese models, or any documentation explaining what type 2000 represents
- No model files or assets bundling found - The assets directory either doesn't exist or is empty in the checked locations
- No explicit model type definitions found - Cannot verify the actual claim that type 2000 is a "Vietnamese zipformer model"
The review comment makes a specific claim about type 2000 being Vietnamese, but this cannot be verified from the codebase. The comment is based on an assumption rather than evidence found in the code.
Ensure model configuration files for type 2000 are properly bundled in assets and that the app doesn't crash at runtime due to missing model files. Verify with the developer what model type 2000 represents and whether Vietnamese language support is intended.
The review comment's claim about Vietnamese zipformer model type 2000 cannot be verified from the codebase - no documentation exists defining what this type represents. While the type assignment itself is correct, the specific claim about Vietnamese language support and the need for zipformer_vi files should be confirmed with project maintainers.
| fun getResult(stream: OnlineStream): OnlineRecognizerResult { | ||
| val objArray = getResult(ptr, stream.ptr) | ||
|
|
||
| val text = objArray[0] as String | ||
| val tokens = objArray[1] as Array<String> | ||
| val timestamps = objArray[2] as FloatArray | ||
| val ysProbs = objArray[3] as FloatArray | ||
|
|
||
| return OnlineRecognizerResult(text = text, tokens = tokens, timestamps = timestamps, ysProbs = ysProbs) | ||
| } |
There was a problem hiding this comment.
Unsafe casts from JNI return value.
Same issue as WaveReader - unchecked casts from Array<Any> could throw at runtime if native code returns unexpected data. Add defensive validation.
🤖 Prompt for AI Agents
In
android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.kt
around lines 122-131, the code unsafely casts elements from the JNI-returned
Array<Any> which can throw at runtime; validate that objArray has exactly 4
elements, check each element's runtime type (first is String, second is
Array<String> or Array<Any> convertible to String, third and fourth are
FloatArray/FloatArray-like), perform safe casts (or map/convert elements to the
expected types) and null-checks, and if any validation fails throw a clear
IllegalStateException indicating the unexpected shape/type (include length and
element class names) so the failure is deterministic and debuggable.
| ../../../../../../../../../../sherpa-onnx/kotlin-api/OnlineStream.kt No newline at end of file | ||
| package com.k2fsa.sherpa.onnx | ||
|
|
||
| class OnlineStream(var ptr: Long = 0) { |
There was a problem hiding this comment.
Make ptr private to prevent external corruption of native state.
Exposing ptr as a public var allows external code to modify the native pointer, which could lead to memory corruption, double-free, or use-after-free bugs. The OnlineRecognizer needs access, so consider a package-private or internal visibility.
-class OnlineStream(var ptr: Long = 0) {
+class OnlineStream(internal var ptr: Long = 0) {📝 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.
| class OnlineStream(var ptr: Long = 0) { | |
| class OnlineStream(internal var ptr: Long = 0) { |
🤖 Prompt for AI Agents
In android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.kt
around line 3, the public mutable property "ptr" exposes the native pointer and
risks external corruption; make "ptr" private (private var ptr: Long = 0) and
expose controlled access only for the recognizer—either add an internal or
package-private read-only getter or an internal function to return the pointer,
or mark the class/internal constructor so OnlineRecognizer in the same
module/package can access it; ensure no other external code can write to ptr and
update usages accordingly.
| return readWaveFromAsset(assetManager, filename).let { | ||
| WaveData(it[0] as FloatArray, it[1] as Int) | ||
| } |
There was a problem hiding this comment.
Unsafe cast from JNI return value may cause runtime exceptions.
The unchecked cast it[0] as FloatArray, it[1] as Int will throw ClassCastException or ArrayIndexOutOfBoundsException if the native function returns unexpected data. Consider adding defensive checks or wrapping in try-catch to provide a more meaningful error.
fun readWave(
assetManager: AssetManager,
filename: String,
): WaveData {
- return readWaveFromAsset(assetManager, filename).let {
- WaveData(it[0] as FloatArray, it[1] as Int)
+ val result = readWaveFromAsset(assetManager, filename)
+ require(result.size >= 2) { "Invalid JNI response for readWaveFromAsset" }
+ return WaveData(
+ samples = result[0] as FloatArray,
+ sampleRate = (result[1] as Number).toInt()
+ )
- }
}📝 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.
| return readWaveFromAsset(assetManager, filename).let { | |
| WaveData(it[0] as FloatArray, it[1] as Int) | |
| } | |
| fun readWave( | |
| assetManager: AssetManager, | |
| filename: String, | |
| ): WaveData { | |
| val result = readWaveFromAsset(assetManager, filename) | |
| require(result.size >= 2) { "Invalid JNI response for readWaveFromAsset" } | |
| return WaveData( | |
| result[0] as FloatArray, | |
| (result[1] as Number).toInt() | |
| ) | |
| } |
🤖 Prompt for AI Agents
In android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/WaveReader.kt
around lines 36 to 38, the unchecked casts from the JNI return value (it[0] as
FloatArray, it[1] as Int) are unsafe and can throw ClassCastException or
ArrayIndexOutOfBoundsException; change the code to validate the returned array
length and element types before casting (check that the list has at least 2
elements, that element 0 is a FloatArray and element 1 is an Int or Number),
wrap the JNI call in try-catch to convert any unexpected results into a clear,
descriptive exception (e.g., IllegalArgumentException or IllegalStateException)
with the filename and asset info, and fail fast with a meaningful message rather
than allowing raw runtime exceptions to propagate.
| return readWaveFromFile(filename).let { | ||
| WaveData(it[0] as FloatArray, it[1] as Int) | ||
| } |
There was a problem hiding this comment.
Same unsafe cast issue applies here.
Same defensive check should be applied to readWaveFromFile.
|
Could you share what problems you're hoping to fix or what new features you're adding with this pull request? |
2 participants
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.