fix: solaria 3 correct endpoint and matching behavior with the openapi#27
fix: solaria 3 correct endpoint and matching behavior with the openapi#27egenthon-cmd wants to merge 7 commits into
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe transcribe CLI now validates normalized language and model inputs, updates its help text and README examples, and posts audio URLs to ChangesTranscribe CLI and client contract update
Language display helpers
Estimated code review effort: 4 (Complex) | ~45 minutes 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
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/client/types/languages.go (1)
298-316: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated enumeration risks drift with
allInputLanguages().
allTargetLanguages()re-lists the exact same set of codes asallInputLanguages()(lines 278-296), just retyped. If a language is added/removed from one list but not the other, the two enumerations will silently diverge.Since
TargetLanguageandLanguageappear to share identical underlying string values, consider deriving one from the other:♻️ Suggested refactor
func allTargetLanguages() []TargetLanguage { - return []TargetLanguage{ - TargetLanguageAf, TargetLanguageSq, TargetLanguageAm, TargetLanguageAr, TargetLanguageHy, TargetLanguageAs, TargetLanguageAz, - ... - TargetLanguageYo, - } + langs := allInputLanguages() + targets := make([]TargetLanguage, len(langs)) + for i, l := range langs { + targets[i] = TargetLanguage(l) + } + return targets }This assumes the
Language/TargetLanguageconstant values are string-identical pairs — please confirm before applying.🤖 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/types/languages.go` around lines 298 - 316, allTargetLanguages duplicates the same language set as allInputLanguages, which can drift over time. Refactor allTargetLanguages to derive its slice from the Language list or a shared helper in pkg/client/types/languages.go, reusing the existing constants so both enumerations stay identical. Confirm the underlying string values match for Language and TargetLanguage before wiring the shared source through allInputLanguages and allTargetLanguages.
🤖 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 `@pkg/client/types/languages.go`:
- Around line 298-316: allTargetLanguages duplicates the same language set as
allInputLanguages, which can drift over time. Refactor allTargetLanguages to
derive its slice from the Language list or a shared helper in
pkg/client/types/languages.go, reusing the existing constants so both
enumerations stay identical. Confirm the underlying string values match for
Language and TargetLanguage before wiring the shared source through
allInputLanguages and allTargetLanguages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44215821-872b-458e-bc88-eddd83621585
📒 Files selected for processing (6)
cmd/root_test.gocmd/transcribe_test.gopkg/client/transcribe.gopkg/client/transcribe_test.gopkg/client/types/languages.gopkg/client/types/languages_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/client/types/languages_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/client/transcribe_test.go
- pkg/client/transcribe.go
- cmd/transcribe_test.go
egenthon-cmd
dismissed
karamouche’s stale review
Changes taken into account and fixed
…aio/gladia-cli into fix/correct-endpoint-model
2 participants
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
gladia transcribedocs with clearer guidance on--language,--cs/--code-switching, and model-specific behavior—specifically thatsolaria-3supports only one language and no code switching.--model solaria-3example with an explicit--language.solaria-3(single language only; code switching disallowed).