feat(audio): add translations endpoint - #639
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdded ChangesAudio translations
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AudioTranslations as handler.AudioTranslations
participant audioService
participant Router
participant AudioTranslationProvider
Client->>AudioTranslations: POST multipart translation request
AudioTranslations->>audioService: CreateTranslation
audioService->>Router: CreateTranslation with parsed request
Router->>AudioTranslationProvider: CreateTranslation
AudioTranslationProvider-->>Router: AudioResponse
Router-->>audioService: AudioResponse
audioService-->>AudioTranslations: Translation response
Possibly related PRs
Suggested reviewers: Poem
🚥 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/server/audio_service.go (1)
269-301: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject transcription-only fields on translation requests.
The translation parser silently ignores
languageandtimestamp_granularities. The endpoint contract requires these fields to be unsupported, not discarded. Return an invalid-request error when either field is present.
internal/server/audio_service.go#L269-L301: Detectlanguage,timestamp_granularities, andtimestamp_granularities[]for translations and reject the request.internal/server/audio_service_test.go#L253-L280: Split valid and invalid requests. Assert a 400 response when a translation request includes unsupported fields.Based on PR objectives,
/v1/audio/translationsmust not accept alanguageinput parameter.🤖 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 `@internal/server/audio_service.go` around lines 269 - 301, Update audioTranscriptionRequestFromForm in internal/server/audio_service.go lines 269-301 to reject translation requests when language, timestamp_granularities, or timestamp_granularities[] is present, returning an invalid-request error instead of ignoring the fields; retain existing parsing for transcription requests. Update internal/server/audio_service_test.go lines 253-280 to separate valid and invalid translation cases and assert a 400 response for each unsupported field.
🤖 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 `@internal/providers/openai/audio_test.go`:
- Around line 68-126: Extend the audio provider tests with negative cases for
CreateTranslation: verify a nil request returns “audio translation request is
required,” and a request with neither File nor FileReader returns “file is
required.” Prefer a table-driven test near
TestCreateTranslation_UsesTranslationMultipartShape, preserving the existing
translation-shape assertions.
In `@internal/providers/router_test.go`:
- Around line 384-420: Convert the affected tests to table-driven coverage: in
internal/providers/router_test.go lines 384-420, extend
TestRouterCreateTranslation with provider-failure and invalid-translation-input
cases, verifying CreateTranslation propagates configured provider errors; in
internal/server/audio_service_test.go lines 243-284, add invalid-field and
provider-error cases and assert each HTTP status and error envelope; in
internal/usage/audio_test.go lines 103-108, use a table covering both
transcription and translation endpoint attribution.
---
Outside diff comments:
In `@internal/server/audio_service.go`:
- Around line 269-301: Update audioTranscriptionRequestFromForm in
internal/server/audio_service.go lines 269-301 to reject translation requests
when language, timestamp_granularities, or timestamp_granularities[] is present,
returning an invalid-request error instead of ignoring the fields; retain
existing parsing for transcription requests. Update
internal/server/audio_service_test.go lines 253-280 to separate valid and
invalid translation cases and assert a 400 response for each unsupported field.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 64e530b2-b284-44f8-88a6-65a6ee68f7f7
📒 Files selected for processing (14)
cmd/gomodel/docs/docs.godocs/openapi.jsoninternal/core/interfaces.gointernal/providers/groq/groq.gointernal/providers/openai/audio.gointernal/providers/openai/audio_test.gointernal/providers/router.gointernal/providers/router_test.gointernal/server/audio_service.gointernal/server/audio_service_test.gointernal/server/handlers.gointernal/server/http.gointernal/usage/audio.gointernal/usage/audio_test.go
Confidence Score: 5/5The PR is safe to merge; no blocking failure remains. Focused execution confirmed the configured multipart-memory behavior and preserved normal transcription success and invalid-request handling.
 T-Rex LogsWhat T-Rex did
Reviews (2): Last reviewed commit: "fix(audio): honor multipart memory limit" | Re-trigger Greptile |
|
Addressed the model-interaction classification finding in 7310a4e. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/server/audio_service.go (1)
269-327: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winValidate disallowed fields before reading the uploaded file into memory.
audioTranscriptionRequestFromFormreads the entire file body withio.ReadAll(lines 279-287) before it callsc.MultipartForm()and rejects translation requests that carrylanguageortimestamp_granularitiesfields (lines 289-299). A translation request that is going to be rejected for an unsupported field still pays the full cost of reading the uploaded audio into memory.Move the
c.MultipartForm()call and the disallowed-field check ahead of the file open/read, so invalid translation requests fail before the file body is buffered.♻️ Proposed reordering
func audioTranscriptionRequestFromForm(c *echo.Context, includeTranscriptionFields bool) (*core.AudioTranscriptionRequest, error) { model := strings.TrimSpace(c.FormValue("model")) if model == "" { return nil, core.NewInvalidRequestError("model is required", nil) } + form, err := c.MultipartForm() + if err != nil { + return nil, core.NewInvalidRequestError("invalid multipart form", err) + } + if !includeTranscriptionFields && form != nil { + for _, field := range []string{"language", "timestamp_granularities", "timestamp_granularities[]"} { + if _, present := form.Value[field]; present { + return nil, core.NewInvalidRequestError(field+" is not supported for audio translations", nil) + } + } + } + fileHeader, err := c.FormFile("file") if err != nil { return nil, core.NewInvalidRequestError("file is required", err) } file, err := fileHeader.Open() if err != nil { return nil, core.NewInvalidRequestError("failed to open uploaded file", err) } defer func() { _ = file.Close() }() data, err := io.ReadAll(file) if err != nil { return nil, core.NewInvalidRequestError("failed to read uploaded file", err) } - - form, err := c.MultipartForm() - if err != nil { - return nil, core.NewInvalidRequestError("invalid multipart form", err) - } - if !includeTranscriptionFields && form != nil { - for _, field := range []string{"language", "timestamp_granularities", "timestamp_granularities[]"} { - if _, present := form.Value[field]; present { - return nil, core.NewInvalidRequestError(field+" is not supported for audio translations", nil) - } - } - }🤖 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 `@internal/server/audio_service.go` around lines 269 - 327, Update audioTranscriptionRequestFromForm so c.MultipartForm() and the includeTranscriptionFields disallowed-field validation occur immediately after validating the model and before opening or reading the uploaded file. Preserve the existing invalid multipart-form and unsupported-field errors, then perform fileHeader.Open and io.ReadAll only after validation passes.
🤖 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.
Outside diff comments:
In `@internal/server/audio_service.go`:
- Around line 269-327: Update audioTranscriptionRequestFromForm so
c.MultipartForm() and the includeTranscriptionFields disallowed-field validation
occur immediately after validating the model and before opening or reading the
uploaded file. Preserve the existing invalid multipart-form and
unsupported-field errors, then perform fileHeader.Open and io.ReadAll only after
validation passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 758b6319-f644-4a20-b2f1-51ae371e878c
📒 Files selected for processing (10)
internal/auditlog/auditlog_test.gointernal/core/endpoints.gointernal/core/endpoints_test.gointernal/providers/openai/audio_test.gointernal/providers/router.gointernal/providers/router_test.gointernal/server/audio_service.gointernal/server/audio_service_test.gointernal/server/http_start_test.gointernal/usage/audio_test.go
|
Addressed the latest outside-diff review in a3aae24 by moving multipart unsupported-field validation before FormFile and io.ReadAll, so rejected translation requests do not buffer the uploaded audio. TestAudioTranslation_ErrorResponses and the complete internal/server package both pass. |
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 `@internal/server/audio_service.go`:
- Around line 275-285: Update the multipart parsing flow in the surrounding
audio request handler to call c.MultipartForm() before the first
c.FormValue("model") access, allowing Echo’s configured FormParseMaxMemory to
apply. Preserve the existing invalid-form error handling, then read model from
the already parsed request instead of triggering http.Request.FormValue parsing.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 58be71b8-672b-4172-b9b4-6c36fd11b78a
📒 Files selected for processing (1)
internal/server/audio_service.go
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
SantiagoDePolonia
left a comment
SantiagoDePolonia
left a comment
There was a problem hiding this comment.
LGTM, tested locally
|
@sidsri14 Thank you for your contribution! |
3 participants
Summary
POST /v1/audio/translationsroute and generated API docsTesting
go test -p 2 ./cmd/... ./config/... ./ext/... ./internal/... ./run/...(passes with ambientOLLAMA_MODELSunset)go mod verifydocs/openapi.jsongit diff --checkLocal limitations
go test -racecannot build because the installed Windows C compiler reports64-bit mode not compiled ingo vetreaches pre-existing duplicate JSON-tag findings ininternal/core/responses.goandinternal/core/types.gogolangci-lintis not installed locally; GitHub CI runs the repository-pinned v2.12 actionCloses #632
Summary by CodeRabbit
New Features
POST /v1/audio/translationsendpoint for translating uploaded audio into English.Documentation