fix: solaria 3 correct endpoint and matching behavior with the openapi#27
Open
egenthon-cmd wants to merge 2 commits into
Open
fix: solaria 3 correct endpoint and matching behavior with the openapi#27egenthon-cmd wants to merge 2 commits into
egenthon-cmd wants to merge 2 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughThe transcribe CLI now validates normalized model and language inputs, updates its help text and README examples, and posts audio URLs to ChangesTranscribe CLI and client contract update
Sequence Diagram(s)sequenceDiagram
participant "RunE"
participant "GladiaClient.TranscribeAudioURL"
participant "createAndExecuteRequest"
participant "Gladia API"
RunE->>GladiaClient.TranscribeAudioURL: normalized model and language config
GladiaClient.TranscribeAudioURL->>createAndExecuteRequest: POST /v2/pre-recorded with JSON body
createAndExecuteRequest->>Gladia API: send transcription request
Gladia API-->>createAndExecuteRequest: transcription response
createAndExecuteRequest-->>GladiaClient.TranscribeAudioURL: decoded result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/transcribe_test.go (1)
375-419: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one case that exercises
--csdirectly.This PR introduces
--cs, but the request-body coverage here still only proves--code-switching. A small table-driven variant over both flag names would pin the new alias wiring too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/transcribe_test.go` around lines 375 - 419, The transcribe command test only covers the long-form code-switching flag, so it does not verify the new `--cs` alias wiring. Update `TestTranscribeCommand_codeSwitchingWithoutLanguages` to exercise both flag names, ideally with a small table-driven subtest over `--code-switching` and `--cs`, and assert the same `language_config` payload from the `newRootCmd` execution.pkg/client/transcribe.go (1)
46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a marshal-level test for these new
omitemptyfields.This change alters the wire contract for zero-valued summarization, translation, and custom vocabulary fields, but the updated suite only asserts omission for model/language/diarization paths. A small JSON serialization test would keep this behavior from regressing silently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/client/transcribe.go` around lines 46 - 50, Add a marshal-level test in the transcribe client serialization tests to verify the new omitempty behavior for Summarization, Translation, and CustomVocabulary in the Transcribe request struct. Update or extend the existing JSON serialization coverage around the Transcribe type so that zero-valued Summarization, Translation, SummarizationConfig, TranslationConfig, and CustomVocabulary are omitted from the payload, similar to the existing model/language/diarization assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/transcribe_test.go`:
- Around line 375-419: The transcribe command test only covers the long-form
code-switching flag, so it does not verify the new `--cs` alias wiring. Update
`TestTranscribeCommand_codeSwitchingWithoutLanguages` to exercise both flag
names, ideally with a small table-driven subtest over `--code-switching` and
`--cs`, and assert the same `language_config` payload from the `newRootCmd`
execution.
In `@pkg/client/transcribe.go`:
- Around line 46-50: Add a marshal-level test in the transcribe client
serialization tests to verify the new omitempty behavior for Summarization,
Translation, and CustomVocabulary in the Transcribe request struct. Update or
extend the existing JSON serialization coverage around the Transcribe type so
that zero-valued Summarization, Translation, SummarizationConfig,
TranslationConfig, and CustomVocabulary are omitted from the payload, similar to
the existing model/language/diarization assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c09ca660-d062-465a-98f4-284c13d5ee40
📒 Files selected for processing (6)
README.mdcmd/transcribe.gocmd/transcribe_test.gopkg/client/client_test.gopkg/client/transcribe.gopkg/client/transcribe_test.go
karamouche
requested changes
Jun 26, 2026
| cmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "Show progress while transcribing") | ||
| cmd.Flags().BoolVar(&diarization, "diarize", false, "Enable speaker diarization") | ||
| cmd.Flags().StringVar(&modelFlag, "model", "", "STT model: solaria-1 or solaria-3 (default: API default)") | ||
| cmd.Flags().StringVar(&modelFlag, "model", "", "STT model: solaria-1 or solaria-3 (solaria-3 accepts at most one --language: en, fr, de, es, or it)") |
| | `--language` | — | Expected language(s), comma-separated (`en` or `en,fr,de`) | | ||
| | `--code-switching`, `--code-switch` | off | Detect language per utterance | | ||
| | `--language` | — | Expected language(s), comma-separated (`en` or `en,fr,de`); narrows detection, does not enable code switching | | ||
| | `--cs`, `--code-switching` | off | Re-detect language on each utterance (mixed-language audio; solaria-1 only) | |
There was a problem hiding this comment.
I think it's should be -cs instead, wdyt?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix for the correct endpoint. That is why Solaria 3 was throwing an error.
Add --cs for code switching as short command
Also, the solaria 3 does not throw an error when no language is detected
Summary by CodeRabbit
Documentation
--model solaria-3with an explicit--languageflag.Bug Fixes
solaria-3only accepts one language and does not support code switching.