feat: extend and clarify the cli#26
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR moves the CLI to Go/Cobra, adds auth and languages subcommands, updates API-key resolution and persistence, restructures client transcription requests and endpoint handling, and rewrites build/release tooling. It also removes the Python packaging pipeline and adds Go tests across the new command and client flow. ChangesGo CLI migration and build/release updates
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Client
participant Upload
participant Transcribe
User->>CLI: transcribe source
CLI->>Client: UploadFile(local file)
Client->>Upload: POST audio
Upload-->>Client: audio_url
CLI->>Client: TranscribeAudioURL(request)
Client->>Transcribe: POST transcription request
Transcribe-->>Client: result_url
Client->>Transcribe: GET result_url
Transcribe-->>Client: transcription result
Client-->>CLI: TranscriptionResult
CLI-->>User: formatted output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
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)
cmd/config.go (1)
31-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestrict permissions on the API-key config file.
os.Createopens the file with mode0666(subject to umask), so the secret can be group/world-readable. Write the credential file with0600.🛡️ Proposed fix
- file, err := os.Create(configPath) + file, err := os.OpenFile(configPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) if err != nil { return err }🤖 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/config.go` around lines 31 - 35, The code uses os.Create(configPath) which creates the credentials file with default mode 0666 (subject to umask) making the API key potentially group/world-readable; change the creation to explicitly set permissions 0600 (owner read/write) by opening the file with os.OpenFile(configPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) (or use os.WriteFile with 0600) instead of os.Create, keeping the existing error handling and defer file.Close() logic in the same function.
🧹 Nitpick comments (5)
pkg/client/types/languages.go (2)
234-236: 💤 Low valueInternal function name leaks into a user-facing error.
ParseLanguageis called from the CLI for the--languageflag; mentioningParseLanguageshere is meaningless to end users.Proposed fix
- return "", fmt.Errorf("multiple languages %q: use ParseLanguages or comma-separated --language en,fr", s) + return "", fmt.Errorf("only one language is allowed here, got %q", s)🤖 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 234 - 236, The error returned from ParseLanguage leaks an internal function name (ParseLanguages) into a user-facing message; update the error in the ParseLanguage function so it doesn't mention ParseLanguages and instead tells the user how to pass multiple languages (e.g., "multiple languages %q: pass a comma-separated list to --language, e.g. --language en,fr" or equivalent). Ensure the new message references the input string s and the comma-separated flag usage only.
259-296: 💤 Low valueRebuilding the allowlist per token is wasteful.
allInputLanguages()allocates the full slice and is linearly scanned for every input token (O(n·m)). Consider a package-levelmap[Language]struct{}built once for O(1) lookups.Suggested approach
+var inputLanguageSet = func() map[Language]struct{} { + m := make(map[Language]struct{}, len(allInputLanguages())) + for _, l := range allInputLanguages() { + m[l] = struct{}{} + } + return m +}() ... - valid := false - for _, allowed := range allInputLanguages() { - if allowed == lang { - valid = true - break - } - } - if !valid { + if _, valid := inputLanguageSet[lang]; !valid { return nil, fmt.Errorf("unknown language %q (use ISO 639-1 codes, e.g. en,fr; run: gladia languages)", code) }🤖 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 259 - 296, The validation currently calls allInputLanguages() and linear-scans it per token which is O(n·m); replace that with a package-level set for O(1) lookups: create a var allowedInputLanguages map[Language]struct{} populated once (e.g., in an init block or as a literal) containing the same entries as allInputLanguages(), then change the validation loop (the code that uses allInputLanguages() and sets valid) to check membership via a map lookup (_, ok := allowedInputLanguages[lang]) and remove or repurpose allInputLanguages() as needed.pkg/client/transcribe.go (1)
245-245: ⚡ Quick win
errcheckflags the deferredresp.Body.Close().With errcheck enabled, the unchecked return on this changed line fails lint. Wrap to discard explicitly.
Proposed fix
- defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }()🤖 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` at line 245, Replace the bare deferred call "defer resp.Body.Close()" with an explicit discard of the returned error so errcheck is satisfied: wrap the close in an anonymous deferred function that either assigns the result to the blank identifier (e.g. defer func(){ _ = resp.Body.Close() }()) or, if you need to ensure connection reuse, drain the body first and then ignore the close error (e.g. defer func(){ _, _ = io.Copy(io.Discard, resp.Body); _ = resp.Body.Close() }()). Update the occurrence of resp.Body.Close() in pkg/client/transcribe.go accordingly..github/workflows/main.yml (1)
14-21: 💤 Low valueOptional: harden the checkout/setup steps.
Two static-analysis posture items on the action steps:
actions/checkout@v4persists theGITHUB_TOKENinto.git/configby default; setpersist-credentials: falsesince no later step pushes.- Actions are pinned to mutable tags rather than commit SHAs.
🔧 Suggested hardening
- name: Checkout code uses: actions/checkout@v4 + with: + persist-credentials: falsePinning to commit SHAs is also recommended if you want to satisfy the blanket pinning policy. Low priority for a build/test-only workflow.
🤖 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 @.github/workflows/main.yml around lines 14 - 21, Set persist-credentials: false on the actions/checkout step to avoid persisting GITHUB_TOKEN into .git/config (the checkout action block referencing actions/checkout@v4), and replace mutable tags for actions (e.g., actions/checkout@v4 and actions/setup-go@v5) with their corresponding pinned commit SHAs to harden the workflow; update the checkout step to include the persist-credentials key and change the uses entries to the canonical commit SHA references for each action.cmd/transcribe.go (1)
67-75: ⚡ Quick winPrefer a named type over an inline anonymous struct for
DiarizationConfig.Re-declaring the anonymous struct shape here couples this call site to the exact field layout in the client type; any change there silently breaks compilation and invites copy drift. Expose and use a named type (e.g.
gladia.DiarizationConfig) from the client package.🤖 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.go` around lines 67 - 75, The call site currently constructs an inline anonymous struct for DiarizationConfig which duplicates the client type shape and risks drift; replace the anonymous struct with the exported named type from the client package (e.g. use gladia.DiarizationConfig) when setting DiarizationConfig, populate its MinSpeakers, MaxSpeakers and NumberOfSpeakers fields accordingly, and ensure the client package is imported so the assignment uses the canonical type instead of a locally declared anonymous struct.
🤖 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.
Inline comments:
In `@cmd/auth.go`:
- Around line 15-25: The command currently requires the API key as a positional
argument (cobra.ExactArgs(1)) which leaks secrets; change the command to allow
zero or one arg (use cobra.MaximumNArgs(1)) and update the RunE handler to
accept either args[0] when provided or securely read the key when omitted (e.g.,
prompt the user or read from stdin without echo using a secure read like
terminal.ReadPassword or similar) before calling SaveGladiaKeyToFile; ensure
RunE handles errors from the secure read and still calls SaveGladiaKeyToFile
with the obtained key.
In `@cmd/main.go`:
- Around line 25-28: The code prints Cobra errors twice because
rootCmd.Execute() already writes errors to stderr; update the error handling to
avoid duplication by setting rootCmd.SilenceErrors = true and
rootCmd.SilenceUsage = true on the rootCmd (so Cobra won’t auto-print), then
keep the current manual error handling that calls fmt.Fprintln(os.Stderr, err)
and os.Exit(1); alternatively you can remove the manual fmt.Fprintln call and
leave SilenceErrors/SilenceUsage false—choose one approach and apply it to the
rootCmd/Execute error block.
In `@cmd/transcribe.go`:
- Around line 64-76: Transcription requests always serialize a DiarizationConfig
(and currently force NumberOfSpeakers=4), even when --diarize is false; fix this
by (1) changing the TranscriptionRequest.DiarizationConfig field in
pkg/client/transcribe.go to a pointer type (e.g. *DiarizationConfig) and add the
json tag `omitempty` so it is omitted when nil, and (2) modify cmd/transcribe.go
to only set/attach a DiarizationConfig when the diarize flag is true and remove
the hard-coded NumberOfSpeakers:4 (do not set NumberOfSpeakers unless you
intentionally want to pin the speaker count). Ensure you update uses of
TranscriptionRequest and the struct literal creation so DiarizationConfig is nil
when diarization is disabled.
In `@README.md`:
- Around line 7-13: Update the "Pre-built binaries" section in README: replace
the "Download from GitHub releases" claim with accurate wording that points
users to the repository's dist artifacts (e.g., reference main/dist/ and
examples like raw/main/dist/gladia-linux-amd64) or simply remove the releases
link and state that pre-built binaries are available under main/dist/, while
keeping the "or build from source: make build" instruction; also ensure
consistency with .github/workflows/main.yml which does not publish release
assets.
---
Outside diff comments:
In `@cmd/config.go`:
- Around line 31-35: The code uses os.Create(configPath) which creates the
credentials file with default mode 0666 (subject to umask) making the API key
potentially group/world-readable; change the creation to explicitly set
permissions 0600 (owner read/write) by opening the file with
os.OpenFile(configPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) (or use
os.WriteFile with 0600) instead of os.Create, keeping the existing error
handling and defer file.Close() logic in the same function.
---
Nitpick comments:
In @.github/workflows/main.yml:
- Around line 14-21: Set persist-credentials: false on the actions/checkout step
to avoid persisting GITHUB_TOKEN into .git/config (the checkout action block
referencing actions/checkout@v4), and replace mutable tags for actions (e.g.,
actions/checkout@v4 and actions/setup-go@v5) with their corresponding pinned
commit SHAs to harden the workflow; update the checkout step to include the
persist-credentials key and change the uses entries to the canonical commit SHA
references for each action.
In `@cmd/transcribe.go`:
- Around line 67-75: The call site currently constructs an inline anonymous
struct for DiarizationConfig which duplicates the client type shape and risks
drift; replace the anonymous struct with the exported named type from the client
package (e.g. use gladia.DiarizationConfig) when setting DiarizationConfig,
populate its MinSpeakers, MaxSpeakers and NumberOfSpeakers fields accordingly,
and ensure the client package is imported so the assignment uses the canonical
type instead of a locally declared anonymous struct.
In `@pkg/client/transcribe.go`:
- Line 245: Replace the bare deferred call "defer resp.Body.Close()" with an
explicit discard of the returned error so errcheck is satisfied: wrap the close
in an anonymous deferred function that either assigns the result to the blank
identifier (e.g. defer func(){ _ = resp.Body.Close() }()) or, if you need to
ensure connection reuse, drain the body first and then ignore the close error
(e.g. defer func(){ _, _ = io.Copy(io.Discard, resp.Body); _ = resp.Body.Close()
}()). Update the occurrence of resp.Body.Close() in pkg/client/transcribe.go
accordingly.
In `@pkg/client/types/languages.go`:
- Around line 234-236: The error returned from ParseLanguage leaks an internal
function name (ParseLanguages) into a user-facing message; update the error in
the ParseLanguage function so it doesn't mention ParseLanguages and instead
tells the user how to pass multiple languages (e.g., "multiple languages %q:
pass a comma-separated list to --language, e.g. --language en,fr" or
equivalent). Ensure the new message references the input string s and the
comma-separated flag usage only.
- Around line 259-296: The validation currently calls allInputLanguages() and
linear-scans it per token which is O(n·m); replace that with a package-level set
for O(1) lookups: create a var allowedInputLanguages map[Language]struct{}
populated once (e.g., in an init block or as a literal) containing the same
entries as allInputLanguages(), then change the validation loop (the code that
uses allInputLanguages() and sets valid) to check membership via a map lookup
(_, ok := allowedInputLanguages[lang]) and remove or repurpose
allInputLanguages() as needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 488966f7-c5d9-467f-8939-2956c2a796fd
⛔ Files ignored due to path filters (4)
go.sumis excluded by!**/*.sumpython/dist/gladia_cli.exeis excluded by!**/dist/**,!**/*.exepython/dist/linux_x64_gladiais excluded by!**/dist/**python/dist/macos_arm64_gladiais excluded by!**/dist/**
📒 Files selected for processing (21)
.github/workflows/main.yml.gitignoreMakefileREADME.mdcmd/auth.gocmd/config.gocmd/formatter.gocmd/languages.gocmd/main.gocmd/transcribe.gogo.modpkg/client/client.gopkg/client/transcribe.gopkg/client/transcription.gopkg/client/types/languages.gopython/build.batpython/build.shpython/gladia_cli.pypython/gladia_cli.specpython/requirements.txtpython/setup.py
💤 Files with no reviewable changes (7)
- python/gladia_cli.spec
- python/build.bat
- python/build.sh
- python/setup.py
- python/requirements.txt
- python/gladia_cli.py
- cmd/formatter.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@cmd/testhelpers_test.go`:
- Around line 17-31: captureStdout currently can deadlock, leak global stdout on
panic, and truncate output; fix it by redirecting os.Stdout to the pipe inside
captureStdout with a deferred restore of os.Stdout (use defer to always
restore), start a goroutine that copies from the pipe reader into a bytes.Buffer
or io.Pipe (use io.Copy or io.ReadAll) while fn() runs so the reader drains
concurrently and cannot block writers, ensure the writer end is closed after
fn() (defer w.Close() after redirect) so the reader goroutine finishes, and
return the fully-read buffer string rather than a single fixed-size Read;
reference captureStdout, the pipe reader/writer variables r and w, and ensure
defers run even if fn panics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf82adff-adbd-4190-9bef-68ba1c530ba3
📒 Files selected for processing (13)
README.mdcmd/config.gocmd/config_test.gocmd/formatter_test.gocmd/main.gocmd/print_output_test.gocmd/root_test.gocmd/testhelpers_test.gocmd/transcribe.gocmd/transcribe_test.gopkg/client/client_test.gopkg/client/transcribe_test.gopkg/client/types/languages_test.go
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/transcribe.go
- cmd/main.go
- cmd/config.go
karamouche
left a comment
There was a problem hiding this comment.
You should remove dist files from the repo by doing git rm -r dist/ pkg/dist
There was a problem hiding this comment.
@egenthon-cmd is a CD to release versions with tags planned for another PR? Also we can try to publish it on brew and all
karamouche
left a comment
There was a problem hiding this comment.
What about adding a model flag/argument ?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/codeql.yml:
- Around line 31-52: Replace mutable action tags with full commit SHAs for each
referenced action usage (actions/checkout, actions/setup-go,
github/codeql-action/init, github/codeql-action/autobuild,
github/codeql-action/analyze) in the CodeQL workflow (and mirror the same
changes in main workflow) and add persist-credentials: false to the Checkout
step to prevent credential persistence; update the uses: lines to the
corresponding pinned commit SHAs and add the persist-credentials: false key
under the Checkout step so the workflow uses fixed commits and does not retain
repo credentials after checkout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6dad7754-8a06-48b8-94f6-3648ee3c3992
⛔ Files ignored due to path filters (13)
dist/gladia-darwin-arm64is excluded by!**/dist/**dist/gladia-darwin-x86_64is excluded by!**/dist/**dist/gladia-linux-arm64is excluded by!**/dist/**dist/gladia-linux-arm7is excluded by!**/dist/**dist/gladia-linux-armhfis excluded by!**/dist/**dist/gladia-linux-i386is excluded by!**/dist/**dist/gladia-linux-x86_64is excluded by!**/dist/**pkg/dist/gladia-darwin-arm64is excluded by!**/dist/**pkg/dist/gladia-darwin-x86_64is excluded by!**/dist/**pkg/dist/gladia-linux-arm64is excluded by!**/dist/**pkg/dist/gladia-linux-armhfis excluded by!**/dist/**pkg/dist/gladia-linux-i386is excluded by!**/dist/**pkg/dist/gladia-linux-x86_64is excluded by!**/dist/**
📒 Files selected for processing (7)
.github/workflows/codeql.yml.gitignoreREADME.mdcmd/transcribe.gocmd/transcribe_test.gopkg/client/transcribe.gopkg/client/transcribe_test.go
💤 Files with no reviewable changes (1)
- .gitignore
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/transcribe.go
- pkg/client/transcribe_test.go
- pkg/client/transcribe.go
Issues fixes and comments taken into account
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In @.github/workflows/release.yml:
- Around line 15-18: The checkout action in the Checkout step is missing the
persist-credentials setting, which leaves git token authentication configured
for subsequent workflow steps. Add persist-credentials: false to the with
section of the actions/checkout@v4 action to disable credential persistence and
follow the principle of least privilege in release workflows with write
permissions.
- Line 16: Replace the floating version tags in the GitHub Actions (`@v4`,
`@v5`, `@v6`) with full commit SHAs in the release workflow. For the
actions/checkout action and the other referenced actions in the workflow,
replace each floating tag reference with its corresponding full commit SHA
(e.g., replace `@v4` with the full 40-character commit hash). This applies to
all uses of floating version tags in the release workflow to ensure supply-chain
security and prevent tag manipulation attacks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5cd4d12-651d-4ce6-abea-aa163364c17e
📒 Files selected for processing (3)
.github/workflows/release.yml.goreleaser.yamlcmd/main.go
✅ Files skipped from review due to trivial changes (1)
- .goreleaser.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/main.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/release.yml:
- Around line 21-24: The release job’s Go setup currently relies on the default
module cache behavior in actions/setup-go@v6, which should be disabled for this
privileged tag-triggered workflow. Update the Set up Go step that uses
go-version-file so caching is turned off explicitly, keeping the GoReleaser
build clean while reducing the risk of restoring a poisoned cache into the
release artifact path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45f917f6-a827-451b-920d-0fa7a39fcb67
📒 Files selected for processing (2)
.github/workflows/release.ymlREADME.md
✅ Files skipped from review due to trivial changes (1)
- README.md
Summary by CodeRabbit
transcribe(multiple output formats,--model, language/code-switching, and diarization), plusauth setto persist an API key andlanguagesto list supported codes.gladia-cliv2 transcription flow, key setup, and full command/flag guidance.