mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-12 22:14:37 +00:00
fix(tools/read_audio): route transcription models to /v1/audio/transcriptions for openai_compat; fail-fast on missing credentials
Two bugs fixed in callProvider: 1. Silent fallback when API credentials are missing for a transcription or gemini/openai path now returns a clear error instead of falling through to a generic chat-API path that explodes confusingly (or NPEs on a nil registry). 2. openai_compat providers (e.g. DashScope) pointed at a transcription endpoint never reached /v1/audio/transcriptions because only ptype=="openai" entered the transcription branch. Move the isTranscriptionModel(model) check above the ptype switch so any OpenAI-compatible provider that exposes the transcription endpoint is routed correctly. Adds three regression tests covering the fail-fast paths.
This commit is contained in:
1 parent
17170b404e
commit
7ffcdf16d1
2 files changed
+93
-14
No files matched your search
@@ -78,13 +78,28 @@ func (t *ReadAudioTool) callProvider(ctx context.Context, cp credentialProvider,
|
||||
data, _ := params["data"].([]byte)
|
||||
mime := GetParamString(params, "mime", "audio/mpeg")
|
||||
|
||||
// Provider-specific paths require API credentials; skip when cp is nil
|
||||
// (e.g. OAuth-based providers that don't expose static keys).
|
||||
// Provider-specific paths require API credentials. Fail-fast (no silent
|
||||
// fallback to chat/completions) for any path we know won't work without
|
||||
// keys: gemini File API, openai input_audio, and any transcription-named
|
||||
// model under any ptype (e.g. openai_compat → DashScope qwen-audio).
|
||||
ptype := GetParamString(params, "_provider_type", providerTypeFromName(providerName))
|
||||
if cp == nil && (ptype == "gemini" || ptype == "openai") {
|
||||
slog.Info("read_audio: no API credentials, falling back to Chat API", "provider", providerName)
|
||||
if cp == nil && (ptype == "gemini" || ptype == "openai" || isTranscriptionModel(model)) {
|
||||
return nil, nil, fmt.Errorf("read_audio: provider %q requires API credentials for model %q", providerName, model)
|
||||
}
|
||||
if cp != nil {
|
||||
// Transcription models always go to /v1/audio/transcriptions regardless
|
||||
// of ptype — orthogonal to chat-vs-input_audio routing. Covers both
|
||||
// native openai (whisper-1, gpt-4o-transcribe) and openai_compat
|
||||
// providers exposing a /v1/audio/transcriptions endpoint.
|
||||
if isTranscriptionModel(model) {
|
||||
slog.Info("read_audio: using openai transcription API", "provider", providerName, "model", model, "size", len(data), "mime", mime)
|
||||
resp, err := openaiTranscriptionCall(ctx, cp.APIKey(), cp.APIBase(), model, data, mime)
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("openai transcription call: %w", err)
|
||||
}
|
||||
return []byte(resp.Content), resp.Usage, nil
|
||||
}
|
||||
|
||||
// Gemini: use File API (inlineData doesn't work for audio).
|
||||
if ptype == "gemini" {
|
||||
slog.Info("read_audio: using gemini file API", "provider", providerName, "model", model, "size", len(data), "mime", mime)
|
||||
@@ -95,17 +110,8 @@ func (t *ReadAudioTool) callProvider(ctx context.Context, cp credentialProvider,
|
||||
return []byte(resp.Content), resp.Usage, nil
|
||||
}
|
||||
|
||||
// OpenAI: transcription models need /v1/audio/transcriptions (multipart);
|
||||
// chat-audio models use /chat/completions with input_audio content part.
|
||||
// Native OpenAI chat-audio (gpt-4o-audio-preview etc.): input_audio content part.
|
||||
if ptype == "openai" {
|
||||
if isTranscriptionModel(model) {
|
||||
slog.Info("read_audio: using openai transcription API", "provider", providerName, "model", model, "size", len(data), "mime", mime)
|
||||
resp, err := openaiTranscriptionCall(ctx, cp.APIKey(), cp.APIBase(), model, data, mime)
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("openai transcription call: %w", err)
|
||||
}
|
||||
return []byte(resp.Content), resp.Usage, nil
|
||||
}
|
||||
slog.Info("read_audio: using openai input_audio API", "provider", providerName, "model", model, "size", len(data), "mime", mime)
|
||||
resp, err := openaiAudioCall(ctx, cp.APIKey(), cp.APIBase(), model, prompt, data, mime)
|
||||
if err != nil {
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestReadAudioCallProvider_TranscriptionModelWithoutCreds_FailsFast asserts
|
||||
// that when no API credentials are present, a transcription-named model
|
||||
// returns a clear error rather than silently falling back to chat/completions
|
||||
// (which would then explode in a confusing way for transcription-only setups).
|
||||
func TestReadAudioCallProvider_TranscriptionModelWithoutCreds_FailsFast(t *testing.T) {
|
||||
tool := &ReadAudioTool{}
|
||||
|
||||
params := map[string]any{
|
||||
"_provider_type": "openai",
|
||||
"data": []byte{0x00, 0x01},
|
||||
"mime": "audio/mpeg",
|
||||
}
|
||||
|
||||
_, _, err := tool.callProvider(context.Background(), nil, "openai", "gpt-4o-mini-transcribe", params)
|
||||
if err == nil {
|
||||
t.Fatalf("expected fail-fast error for transcription model with nil credentials, got nil")
|
||||
}
|
||||
if !strings.Contains(strings.ToLower(err.Error()), "credential") {
|
||||
t.Errorf("expected error to mention credentials, got: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestReadAudioCallProvider_TranscriptionModelWithoutCreds_OpenAICompat_FailsFast
|
||||
// covers the openai_compat ptype variant — the bug the original PR found:
|
||||
// previously a transcription model under openai_compat fell through to the
|
||||
// generic chat-API fallback because only ptype=="openai" entered the
|
||||
// transcription branch.
|
||||
func TestReadAudioCallProvider_TranscriptionModelWithoutCreds_OpenAICompat_FailsFast(t *testing.T) {
|
||||
tool := &ReadAudioTool{}
|
||||
|
||||
params := map[string]any{
|
||||
"_provider_type": "openai_compat",
|
||||
"data": []byte{0x00, 0x01},
|
||||
"mime": "audio/mpeg",
|
||||
}
|
||||
|
||||
_, _, err := tool.callProvider(context.Background(), nil, "dashscope", "whisper-1", params)
|
||||
if err == nil {
|
||||
t.Fatalf("expected fail-fast error for transcription model with nil credentials (openai_compat), got nil")
|
||||
}
|
||||
if !strings.Contains(strings.ToLower(err.Error()), "credential") {
|
||||
t.Errorf("expected error to mention credentials, got: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestReadAudioCallProvider_GeminiWithoutCreds_FailsFast preserves the existing
|
||||
// gemini fail-fast behavior (was previously a soft log + fallback that would
|
||||
// then NPE on the registry path in tests; the broader guard makes it explicit).
|
||||
func TestReadAudioCallProvider_GeminiWithoutCreds_FailsFast(t *testing.T) {
|
||||
tool := &ReadAudioTool{}
|
||||
|
||||
params := map[string]any{
|
||||
"_provider_type": "gemini",
|
||||
"data": []byte{0x00, 0x01},
|
||||
"mime": "audio/mpeg",
|
||||
}
|
||||
|
||||
_, _, err := tool.callProvider(context.Background(), nil, "gemini", "gemini-2.5-flash", params)
|
||||
if err == nil {
|
||||
t.Fatalf("expected fail-fast error for gemini with nil credentials, got nil")
|
||||
}
|
||||
if !strings.Contains(strings.ToLower(err.Error()), "credential") {
|
||||
t.Errorf("expected error to mention credentials, got: %v", err)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user