Repository navigation
Conversation
📝 WalkthroughWalkthroughThis PR integrates offline and online punctuation support into the HarmonyOS SherpaOnnx project by adding native C++ API bindings, TypeScript/ETS wrapper classes, public module exports, updated package dependencies, and a test implementation that demonstrates punctuation processing functionality. Changes
Sequence DiagramsequenceDiagram
participant UI as Index Component
participant Punct as OfflinePunctuation
participant Native as libsherpa_onnx (Native)
UI->>Punct: new OfflinePunctuation(modelConfig)
activate Punct
Punct->>Native: createOfflinePunctuation(config)
Native-->>Punct: handle
Punct->>Punct: store handle
deactivate Punct
UI->>Punct: offlinePunctuationAddPunct(text)
activate Punct
Punct->>Native: offlinePunctuationAddPunct(handle, text)
Native-->>Punct: punctuated_text
Punct-->>UI: punctuated_text
deactivate Punct
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 @jilinmo1, 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! 此拉取请求旨在为 HarmonyOS 上的 SherpaOnnxHar 应用引入标点处理能力。通过集成离线标点模块,并提供一个基础的UI测试入口,它为提升语音识别结果的文本可读性奠定了基础,使得处理后的文本更符合语言规范。 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
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/cpp/types/libsherpa_onnx/Index.d.ts:
- Around line 30-34: The parameter lists for offlinePunctuationAddPunct and
onlinePunctuationAddPunct are missing a space after the comma; update their
declarations so the comma is followed by a space (e.g., change "(handle:
object,text: string)" to "(handle: object, text: string)") to match the file's
existing formatting conventions used by functions like createOnlinePunctuation
and createOfflinePunctuation.
In
@harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/NonStreamingAsr.ets:
- Around line 8-9: Remove the unused imports createOfflinePunctuation and
offlinePunctuationAddPunct from the import list in NonStreamingAsr.ets; locate
the import statement that includes these symbols and delete them so only used
symbols remain (they are used in StreamingAsr.ets and not referenced in
NonStreamingAsr).
In
@harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.ets:
- Around line 99-113: The constructor in class OfflinePunctuation uses a
misleading variable name `onlineConfig`; rename it to `offlineConfig` throughout
the constructor to match the class semantics: update the declaration
(OfflinePunctuationConfig), the assignment `offlineConfig.model = config`, and
the call to `createOfflinePunctuation(offlineConfig)` so all references inside
the constructor (in OfflinePunctuation) are consistently `offlineConfig`.
🧹 Nitpick comments (4)
harmony-os/SherpaOnnxHar/sherpa_onnx/Index.ets (1)
40-41: Consider module organization: "Offline" exports from "StreamingAsr".The exports look correct, but semantically it's unusual to export
OfflinePunctuationandOfflinePunctuationModelConfigfromStreamingAsr(which typically implies online/streaming). Consider whether these belong inNonStreamingAsrfor consistency with the naming convention used elsewhere (e.g.,OfflineRecognizerinNonStreamingAsr).harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.ets (3)
11-12: Unused imports detected.
createOnlinePunctuationandonlinePunctuationAddPunctare imported but never used in this file. Consider removing them to keep the imports clean, or add the correspondingOnlinePunctuationclass implementation if intended.
44-48: Inconsistent naming convention.Properties
cnn_bilstmandbpe_vocabuse snake_case, while other config classes in this file use camelCase (e.g.,numThreads,bpeVocabinOnlineModelConfig). If this is intentional to match native API field names, consider adding a comment to clarify; otherwise, align with the existing camelCase convention.
118-118: Commented-out code for punctuation integration.Multiple commented-out lines (118, 149, 155-156, 191) indicate incomplete integration of punctuation into
OnlineRecognizer. If this is intentional scaffolding for future work, consider:
- Adding a TODO comment explaining the plan, or
- Removing the commented code and implementing it in a follow-up PR.
Leaving unexplained commented code makes it unclear whether this is dead code or pending work.
Also applies to: 149-149, 155-156, 191-191
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
harmony-os/SherpaOnnxHar/entry/oh-package.json5harmony-os/SherpaOnnxHar/entry/src/main/ets/pages/Index.etsharmony-os/SherpaOnnxHar/sherpa_onnx/Index.etsharmony-os/SherpaOnnxHar/sherpa_onnx/src/main/cpp/types/libsherpa_onnx/Index.d.tsharmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/NonStreamingAsr.etsharmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.ets
🧰 Additional context used
🧬 Code graph analysis (1)
harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/cpp/types/libsherpa_onnx/Index.d.ts (3)
nodejs-addon-examples/test_asr_non_streaming_nemo_canary.js (1)
config(6-27)nodejs-examples/test-offline-nemo-canary.js (2)
config(10-25)text(39-39)sherpa-onnx/csrc/lexicon.cc (2)
text(164-164)text(269-269)
🔇 Additional comments (2)
harmony-os/SherpaOnnxHar/entry/oh-package.json5 (1)
8-10: LGTM!The local package dependency reference is correctly configured to point to the sibling
sherpa_onnxmodule.harmony-os/SherpaOnnxHar/entry/src/main/ets/pages/Index.ets (1)
17-22: LGTM for test UI.The click handler properly invokes the test method to exercise the punctuation functionality.
| test(){ | ||
| const modelConfig = new OfflinePunctuationModelConfig(); | ||
| const punctModelDir = 'sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12-int8'; | ||
| modelConfig.ctTransformer = `${punctModelDir}/model.int8.onnx`; | ||
| let punctuation: OfflinePunctuation = new OfflinePunctuation(modelConfig) | ||
| let text = punctuation.offlinePunctuationAddPunct('这是一个测试你好吗How are you我很好thank you are you ok谢谢你') | ||
| } |
There was a problem hiding this comment.
Test result is discarded without any feedback.
The punctuation result on line 11 is stored in text but never used (not logged, displayed, or returned). For testing purposes, consider adding console.log(text) or displaying the result to verify the functionality works correctly.
Additionally, there's no error handling if the model files are missing or the punctuation operation fails.
Suggested improvement
test(){
const modelConfig = new OfflinePunctuationModelConfig();
const punctModelDir = 'sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12-int8';
modelConfig.ctTransformer = `${punctModelDir}/model.int8.onnx`;
let punctuation: OfflinePunctuation = new OfflinePunctuation(modelConfig)
let text = punctuation.offlinePunctuationAddPunct('这是一个测试你好吗How are you我很好thank you are you ok谢谢你')
+ console.log('Punctuated text:', text);
}📝 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.
| test(){ | |
| const modelConfig = new OfflinePunctuationModelConfig(); | |
| const punctModelDir = 'sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12-int8'; | |
| modelConfig.ctTransformer = `${punctModelDir}/model.int8.onnx`; | |
| let punctuation: OfflinePunctuation = new OfflinePunctuation(modelConfig) | |
| let text = punctuation.offlinePunctuationAddPunct('这是一个测试你好吗How are you我很好thank you are you ok谢谢你') | |
| } | |
| test(){ | |
| const modelConfig = new OfflinePunctuationModelConfig(); | |
| const punctModelDir = 'sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12-int8'; | |
| modelConfig.ctTransformer = `${punctModelDir}/model.int8.onnx`; | |
| let punctuation: OfflinePunctuation = new OfflinePunctuation(modelConfig) | |
| let text = punctuation.offlinePunctuationAddPunct('这是一个测试你好吗How are you我很好thank you are you ok谢谢你') | |
| console.log('Punctuated text:', text); | |
| } |
| export const createOnlinePunctuation: (config: object) => object; | ||
| export const createOfflinePunctuation: (config: object) => object; | ||
| export const offlinePunctuationAddPunct: (handle: object,text: string) => string; | ||
| export const onlinePunctuationAddPunct: (handle: object,text: string) => string; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the Index.d.ts file exists and read it to verify the formatting inconsistency
fd Index.d.tsRepository: k2-fsa/sherpa-onnx
Length of output: 143
🏁 Script executed:
# Search for the specific file and read relevant lines
find . -name "Index.d.ts" -type f -exec cat -n {} +Repository: k2-fsa/sherpa-onnx
Length of output: 6478
🏁 Script executed:
# Search for onlinePunctuation usage across the repository
rg -n "onlinePunctuation" --type tsRepository: k2-fsa/sherpa-onnx
Length of output: 227
🏁 Script executed:
# Also search in JavaScript/TypeScript files more broadly
rg -n "onlinePunctuation" -t js -t ts -t tsx -t jsxRepository: k2-fsa/sherpa-onnx
Length of output: 89
🏁 Script executed:
# Search for createOnlinePunctuation specifically
rg -n "createOnlinePunctuation"Repository: k2-fsa/sherpa-onnx
Length of output: 556
Missing space after comma in parameter lists.
Lines 32-33 are missing a space after the comma in the parameter list, inconsistent with other function declarations in this file (e.g., lines 6, 13, 63).
Formatting fix
-export const offlinePunctuationAddPunct: (handle: object,text: string) => string;
-export const onlinePunctuationAddPunct: (handle: object,text: string) => string;
+export const offlinePunctuationAddPunct: (handle: object, text: string) => string;
+export const onlinePunctuationAddPunct: (handle: object, text: string) => string;📝 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.
| export const createOnlinePunctuation: (config: object) => object; | |
| export const createOfflinePunctuation: (config: object) => object; | |
| export const offlinePunctuationAddPunct: (handle: object,text: string) => string; | |
| export const onlinePunctuationAddPunct: (handle: object,text: string) => string; | |
| export const createOnlinePunctuation: (config: object) => object; | |
| export const createOfflinePunctuation: (config: object) => object; | |
| export const offlinePunctuationAddPunct: (handle: object, text: string) => string; | |
| export const onlinePunctuationAddPunct: (handle: object, text: string) => string; |
🤖 Prompt for AI Agents
In
@harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/cpp/types/libsherpa_onnx/Index.d.ts
around lines 30 - 34, The parameter lists for offlinePunctuationAddPunct and
onlinePunctuationAddPunct are missing a space after the comma; update their
declarations so the comma is followed by a space (e.g., change "(handle:
object,text: string)" to "(handle: object, text: string)") to match the file's
existing formatting conventions used by functions like createOnlinePunctuation
and createOfflinePunctuation.
| createOfflinePunctuation, | ||
| offlinePunctuationAddPunct, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if these imports are used in NonStreamingAsr.ets
echo "=== Checking usage in NonStreamingAsr.ets ==="
rg -n "createOfflinePunctuation|offlinePunctuationAddPunct" harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/NonStreamingAsr.ets
echo "=== Checking where these are actually used ==="
rg -n "createOfflinePunctuation|offlinePunctuationAddPunct" --type-add 'ets:*.ets' --type ets harmony-os/
echo "=== Checking if they are used in StreamingAsr.ets specifically ==="
rg -n "createOfflinePunctuation|offlinePunctuationAddPunct" harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.etsRepository: k2-fsa/sherpa-onnx
Length of output: 1968
Remove unused imports from this file.
createOfflinePunctuation and offlinePunctuationAddPunct are imported but never used in NonStreamingAsr.ets. These imports are used in StreamingAsr.ets, so they can be safely removed here.
🤖 Prompt for AI Agents
In
@harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/NonStreamingAsr.ets
around lines 8 - 9, Remove the unused imports createOfflinePunctuation and
offlinePunctuationAddPunct from the import list in NonStreamingAsr.ets; locate
the import statement that includes these symbols and delete them so only used
symbols remain (they are used in StreamingAsr.ets and not referenced in
NonStreamingAsr).
| export class OfflinePunctuation { | ||
| public handle: object; | ||
| // public config: OfflinePunctuationConfig | ||
|
|
||
| constructor(config: OfflinePunctuationModelConfig) { | ||
| let onlineConfig = new OfflinePunctuationConfig() | ||
| onlineConfig.model = config | ||
| this.handle = createOfflinePunctuation(onlineConfig) | ||
| } | ||
|
|
||
| offlinePunctuationAddPunct(text: string): string { | ||
| let result = offlinePunctuationAddPunct(this.handle, text) | ||
| return result | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.ets | head -120 | tail -30Repository: k2-fsa/sherpa-onnx
Length of output: 1208
🏁 Script executed:
# Search for destructor/cleanup patterns in the codebase
rg -A 5 "destructor|destroy\(\)|cleanup|release\(\)|free\(\)" harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/ --type tsRepository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
# Check for similar native handle patterns in the file
rg -B 5 -A 5 "createOfflinePunctuation|createOnlineRecognizer|handle.*createOn" harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.etsRepository: k2-fsa/sherpa-onnx
Length of output: 1465
🏁 Script executed:
# Look at the entire OfflinePunctuation class and surrounding context
sed -n '90,120p' harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.etsRepository: k2-fsa/sherpa-onnx
Length of output: 1041
🏁 Script executed:
# Search for any release, destroy, or free calls on handles
rg "handle.*release|handle.*destroy|handle.*free|\.release\(|\.destroy\(|\.free\(" harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/ --type tsRepository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
# Check if there's documentation or comments about resource management
rg -i "resource|cleanup|garbage|collect|leak|release|free|destructor" harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/ --type tsRepository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
# Search the entire StreamingAsr.ets for any cleanup patterns or comments
rg -i "cleanup|release|destroy|free|leak" harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.etsRepository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
# Check the native library binding or any .d.ts files that might document cleanup
find harmony-os/SherpaOnnxHar -name "*.d.ts" -o -name "*libsherpa*" | head -20Repository: k2-fsa/sherpa-onnx
Length of output: 214
🏁 Script executed:
cat -n harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/cpp/types/libsherpa_onnx/Index.d.tsRepository: k2-fsa/sherpa-onnx
Length of output: 6478
🏁 Script executed:
# Also check if there's a README or documentation about the library
find harmony-os/SherpaOnnxHar -name "README*" -o -name "*.md" | grep -E "(README|doc)" | head -10Repository: k2-fsa/sherpa-onnx
Length of output: 294
🏁 Script executed:
# Check README files for any documentation about resource management
grep -i "cleanup\|release\|destroy\|free\|resource\|leak\|garbage" harmony-os/SherpaOnnxHar/README.md harmony-os/SherpaOnnxHar/sherpa_onnx/README.mdRepository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
# Check the C API README which might have documentation
head -100 harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/cpp/include/sherpa-onnx/c-api/README.mdRepository: k2-fsa/sherpa-onnx
Length of output: 389
🏁 Script executed:
# Check if there are any Java or Kotlin examples that might show cleanup patterns
find harmony-os -name "*.java" -o -name "*.kt" | xargs grep -l "OfflinePunctuation" 2>/dev/null | head -5Repository: k2-fsa/sherpa-onnx
Length of output: 44
Misleading variable name: onlineConfig should be offlineConfig.
Line 104 uses onlineConfig for the OfflinePunctuation class, which is inconsistent and confusing. Rename it to offlineConfig for clarity:
Proposed fix
constructor(config: OfflinePunctuationModelConfig) {
- let onlineConfig = new OfflinePunctuationConfig()
- onlineConfig.model = config
- this.handle = createOfflinePunctuation(onlineConfig)
+ let offlineConfig = new OfflinePunctuationConfig()
+ offlineConfig.model = config
+ this.handle = createOfflinePunctuation(offlineConfig)
}📝 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.
| export class OfflinePunctuation { | |
| public handle: object; | |
| // public config: OfflinePunctuationConfig | |
| constructor(config: OfflinePunctuationModelConfig) { | |
| let onlineConfig = new OfflinePunctuationConfig() | |
| onlineConfig.model = config | |
| this.handle = createOfflinePunctuation(onlineConfig) | |
| } | |
| offlinePunctuationAddPunct(text: string): string { | |
| let result = offlinePunctuationAddPunct(this.handle, text) | |
| return result | |
| } | |
| } | |
| export class OfflinePunctuation { | |
| public handle: object; | |
| // public config: OfflinePunctuationConfig | |
| constructor(config: OfflinePunctuationModelConfig) { | |
| let offlineConfig = new OfflinePunctuationConfig() | |
| offlineConfig.model = config | |
| this.handle = createOfflinePunctuation(offlineConfig) | |
| } | |
| offlinePunctuationAddPunct(text: string): string { | |
| let result = offlinePunctuationAddPunct(this.handle, text) | |
| return result | |
| } | |
| } |
🤖 Prompt for AI Agents
In
@harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/ets/components/StreamingAsr.ets
around lines 99 - 113, The constructor in class OfflinePunctuation uses a
misleading variable name `onlineConfig`; rename it to `offlineConfig` throughout
the constructor to match the class semantics: update the declaration
(OfflinePunctuationConfig), the assignment `offlineConfig.model = config`, and
the call to `createOfflinePunctuation(offlineConfig)` so all references inside
the constructor (in OfflinePunctuation) are consistently `offlineConfig`.
| const punctModelDir = 'sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12-int8'; | ||
| modelConfig.ctTransformer = `${punctModelDir}/model.int8.onnx`; |
| const punctModelDir = 'sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12-int8'; | ||
| modelConfig.ctTransformer = `${punctModelDir}/model.int8.onnx`; | ||
| let punctuation: OfflinePunctuation = new OfflinePunctuation(modelConfig) | ||
| let text = punctuation.offlinePunctuationAddPunct('这是一个测试你好吗How are you我很好thank you are you ok谢谢你') |
| export class OnlinePunctuationModelConfig { | ||
| public cnn_bilstm: string = ''; | ||
| public bpe_vocab: string = ''; | ||
| public provider: string = ''; | ||
| } |
There was a problem hiding this comment.
在 OnlinePunctuationModelConfig 类中,属性 cnn_bilstm 和 bpe_vocab 使用了 snake_case 命名法。然而,在 TypeScript/ETS 中,更常见的约定是为类属性使用 camelCase,就像在 OfflinePunctuationModelConfig 中看到的 ctTransformer 一样。为了保持代码风格的一致性,建议将它们重命名为 cnnBilstm 和 bpeVocab。
export class OnlinePunctuationModelConfig {
public cnnBilstm: string = '';
public bpeVocab: string = '';
public provider: string = '';
}
| export class OfflinePunctuation { | ||
| public handle: object; | ||
| // public config: OfflinePunctuationConfig | ||
|
|
||
| constructor(config: OfflinePunctuationModelConfig) { | ||
| let onlineConfig = new OfflinePunctuationConfig() | ||
| onlineConfig.model = config | ||
| this.handle = createOfflinePunctuation(onlineConfig) | ||
| } | ||
|
|
||
| offlinePunctuationAddPunct(text: string): string { | ||
| let result = offlinePunctuationAddPunct(this.handle, text) | ||
| return result | ||
| } | ||
| } |
|
|
||
| export class OfflinePunctuation { | ||
| public handle: object; | ||
| // public config: OfflinePunctuationConfig |
| // public config: OfflinePunctuationConfig | ||
|
|
||
| constructor(config: OfflinePunctuationModelConfig) { | ||
| let onlineConfig = new OfflinePunctuationConfig() |
| export class OnlineRecognizerConfig { | ||
| public featConfig: FeatureConfig = new FeatureConfig(); | ||
| public modelConfig: OnlineModelConfig = new OnlineModelConfig(); | ||
| // public punctuation: OfflinePunctuationModelConfig = new OfflinePunctuationModelConfig(); |
|
|
||
| export class OnlineRecognizer { | ||
| public handle: object; | ||
| // public punct: object; |
| // let configp = new OfflinePunctuationModelConfig() | ||
| // this.punct = createOfflinePunctuation(configp) |
| r.tokens = o.tokens; | ||
| r.json = jsonStr; | ||
|
|
||
| // r.text = offlinePunctuationAddPunct(this.punct, r.text) |
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.