Add Go API for Cohere Transcribe - #3466
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dosubot
Bot
added
the
size:L
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds Cohere Transcribe offline transcription support to the Go language bindings for sherpa-onnx. It introduces a new Go example application, CI test infrastructure, module configurations, and extends the core Go bindings with CohereTranscribe model configuration structures. Changes
Sequence DiagramsequenceDiagram
participant App as Go Application
participant Lib as sherpa-onnx Go Lib
participant C as C/C++ Backend
participant Model as ONNX Model
App->>Lib: Create OfflineRecognizerConfig<br/>(with CohereTranscribeModelConfig)
Lib->>C: newCOfflineRecognizerConfig<br/>(populate cohere_transcribe fields)
App->>Lib: Initialize OfflineRecognizer
Lib->>Model: Load encoder/decoder ONNX
Model-->>Lib: Models loaded
Lib-->>App: Recognizer ready
App->>Lib: Create OfflineStream
Lib-->>App: Stream created
App->>Lib: AcceptWaveform(samples, sample_rate)
Lib->>C: Feed audio to recognizer
App->>Lib: Decode(stream)
C->>Model: Run ONNX inference
Model-->>C: Transcription output
Lib->>Lib: GetResult()
Lib-->>App: Return transcribed text
App->>Lib: Cleanup (defer)
Lib->>C: freeCOfflineRecognizerConfig<br/>(free cohere_transcribe strings)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 adds support for the Cohere Transcribe model to the Go API, including the necessary configuration structures and C-binding updates. It also introduces a new example for non-streaming transcription. The review feedback highlights several critical issues: a potential nil pointer dereference when accessing transcription results, an incorrect relative path in a go.mod file, and files in the internal directory that appear to be broken symbolic links. Additionally, it is suggested to set the language parameter within the configuration struct rather than via stream options for better consistency with the library's design.
| result := stream.GetResult() | ||
|
|
||
| log.Println("Text: " + strings.ToLower(result.Text)) |
There was a problem hiding this comment.
stream.GetResult() can return nil if no recognition result is available (e.g., if the audio is empty or decoding fails). Accessing result.Text without a nil check will cause a runtime panic. Additionally, using strings.ToLower on the output is generally discouraged for examples as it discards the formatting (casing) provided by the model when UsePunct is enabled.
result := stream.GetResult()
if result != nil {
log.Println("Text: " + result.Text)
}|
|
||
| go 1.17 | ||
|
|
||
| replace github.com/k2-fsa/sherpa-onnx-go/sherpa_onnx => ../ |
There was a problem hiding this comment.
The relative path in the replace directive is incorrect. The sherpa_onnx package is located in scripts/go/, which is two levels up from this directory (../../), not one level (../).
| replace github.com/k2-fsa/sherpa-onnx-go/sherpa_onnx => ../ | |
| replace github.com/k2-fsa/sherpa-onnx-go/sherpa_onnx => ../../ |
| @@ -0,0 +1 @@ | |||
| ../../../../go-api-examples/non-streaming-cohere-transcribe-decode-files/main.go No newline at end of file | |||
There was a problem hiding this comment.
This file contains a relative path string instead of valid Go source code. It appears that a symbolic link was intended but was committed as a regular text file. This will cause the Go compiler to fail in this directory.
| @@ -0,0 +1 @@ | |||
| ../../../../go-api-examples/non-streaming-cohere-transcribe-decode-files/run.sh No newline at end of file | |||
There was a problem hiding this comment.
This file contains a relative path string instead of a valid shell script. It appears that a symbolic link was intended but was committed as a regular text file.
| stream := sherpa.NewOfflineStream(recognizer) | ||
| defer sherpa.DeleteOfflineStream(stream) | ||
|
|
||
| stream.SetOption("language", "en") |
There was a problem hiding this comment.
The OfflineCohereTranscribeModelConfig struct (defined in sherpa_onnx.go) includes a Language field. It is more consistent with the design of this library to set the language in the configuration during initialization rather than using stream.SetOption("language", "en") on the stream, especially for offline models.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Tests