mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
fix(tts): validate voice compatibility with selected provider before synthesis (#1302)
An agent's tts_voice_id (e.g. "en-US-GuyNeural" for Edge) was applied regardless of the selected TTS provider, causing OpenAI and other providers to reject unknown voice IDs. - Add internal/audio/voice_compat.go with IsVoiceCompatible, GetProviderDefaultVoice, and FilterVoiceForProvider helpers - Update TtsTool.resolveVoiceAndModel to report whether the voice came from an agent override (voiceFromAgent) - Apply applyVoiceCompat per-provider branch in Execute so incompatible agent voices fall back to the provider's default with a slog.Warn for operator visibility - Add voice_compat_test.go with table-driven tests for all cases Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
This commit is contained in:
1 parent
22efff8c0d
commit
0184678dc3
5 files changed
+164
-8
No files matched your search
@@ -32,6 +32,7 @@ require (
|
||||
github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e
|
||||
github.com/slack-go/slack v0.19.0
|
||||
github.com/spf13/cobra v1.10.2
|
||||
github.com/stretchr/testify v1.11.1
|
||||
github.com/titanous/json5 v1.0.0
|
||||
github.com/wailsapp/wails/v2 v2.12.0
|
||||
github.com/zalando/go-keyring v0.2.8
|
||||
@@ -89,6 +90,7 @@ require (
|
||||
github.com/coder/websocket v1.8.15 // indirect
|
||||
github.com/creachadair/msync v0.7.1 // indirect
|
||||
github.com/danieljoos/wincred v1.2.3 // indirect
|
||||
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
|
||||
github.com/dblohm7/wingoes v0.0.0-20240119213807-a09d6be7affa // indirect
|
||||
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
|
||||
github.com/dlclark/regexp2 v1.11.4 // indirect
|
||||
@@ -131,6 +133,7 @@ require (
|
||||
github.com/pires/go-proxyproto v0.8.1 // indirect
|
||||
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
|
||||
github.com/pkg/errors v0.9.1 // indirect
|
||||
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
|
||||
github.com/prometheus-community/pro-bing v0.4.0 // indirect
|
||||
github.com/prometheus/common v0.66.1 // indirect
|
||||
github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
package audio
|
||||
|
||||
import "strings"
|
||||
|
||||
// edgeVoiceDefaultVoice is the default Edge TTS voice.
|
||||
const edgeVoiceDefaultVoice = "en-US-AriaNeural"
|
||||
|
||||
// openaiDefaultVoice is the default OpenAI TTS voice.
|
||||
const openaiDefaultVoice = "alloy"
|
||||
|
||||
// IsVoiceCompatible reports whether the given voice ID is compatible with the
|
||||
// named TTS provider. Returns true for providers without validation rules.
|
||||
//
|
||||
// Edge voices follow the BCP-47 + Neural suffix pattern (e.g. "en-US-GuyNeural").
|
||||
// OpenAI voices are a fixed set: alloy, echo, fable, onyx, nova, shimmer.
|
||||
func IsVoiceCompatible(provider, voice string) bool {
|
||||
if voice == "" {
|
||||
return true
|
||||
}
|
||||
switch provider {
|
||||
case "edge":
|
||||
return strings.Contains(voice, "Neural")
|
||||
case "openai":
|
||||
switch voice {
|
||||
case "alloy", "echo", "fable", "onyx", "nova", "shimmer":
|
||||
return true
|
||||
}
|
||||
return false
|
||||
default:
|
||||
// No validation for other providers.
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
// GetProviderDefaultVoice returns the default voice ID for the named provider.
|
||||
// Returns an empty string for providers where the SDK selects its own default.
|
||||
func GetProviderDefaultVoice(provider string) string {
|
||||
switch provider {
|
||||
case "edge":
|
||||
return edgeVoiceDefaultVoice
|
||||
case "openai":
|
||||
return openaiDefaultVoice
|
||||
default:
|
||||
return ""
|
||||
}
|
||||
}
|
||||
|
||||
// FilterVoiceForProvider returns the voice to use for the given provider.
|
||||
// If the voice is incompatible with the provider it falls back to the
|
||||
// provider's default voice (which may be empty, signalling "use SDK default").
|
||||
// agentOverride indicates the voice came from agent configuration (not from
|
||||
// explicit tool args or tenant defaults) so that the caller can decide whether
|
||||
// to emit a warning.
|
||||
func FilterVoiceForProvider(provider, voice string, agentOverride bool) (filtered string, changed bool) {
|
||||
if IsVoiceCompatible(provider, voice) {
|
||||
return voice, false
|
||||
}
|
||||
def := GetProviderDefaultVoice(provider)
|
||||
return def, true
|
||||
}
|
||||
@@ -0,0 +1,68 @@
|
||||
package audio_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/audio"
|
||||
)
|
||||
|
||||
func TestIsVoiceCompatible(t *testing.T) {
|
||||
tests := []struct {
|
||||
provider string
|
||||
voice string
|
||||
want bool
|
||||
}{
|
||||
// Edge provider — Neural suffix required
|
||||
{"edge", "en-US-GuyNeural", true},
|
||||
{"edge", "en-US-MichelleNeural", true},
|
||||
{"edge", "alloy", false},
|
||||
{"edge", "echo", false},
|
||||
{"edge", "", true},
|
||||
// OpenAI provider — fixed set
|
||||
{"openai", "alloy", true},
|
||||
{"openai", "echo", true},
|
||||
{"openai", "fable", true},
|
||||
{"openai", "onyx", true},
|
||||
{"openai", "nova", true},
|
||||
{"openai", "shimmer", true},
|
||||
{"openai", "en-US-GuyNeural", false},
|
||||
{"openai", "", true},
|
||||
// Other providers — no validation
|
||||
{"elevenlabs", "any-voice-id", true},
|
||||
{"minimax", "en-US-GuyNeural", true},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
got := audio.IsVoiceCompatible(tc.provider, tc.voice)
|
||||
assert.Equal(t, tc.want, got, "IsVoiceCompatible(%q, %q)", tc.provider, tc.voice)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetProviderDefaultVoice(t *testing.T) {
|
||||
assert.Equal(t, "en-US-AriaNeural", audio.GetProviderDefaultVoice("edge"))
|
||||
assert.Equal(t, "alloy", audio.GetProviderDefaultVoice("openai"))
|
||||
assert.Equal(t, "", audio.GetProviderDefaultVoice("elevenlabs"))
|
||||
}
|
||||
|
||||
func TestFilterVoiceForProvider(t *testing.T) {
|
||||
// Compatible — unchanged.
|
||||
got, changed := audio.FilterVoiceForProvider("openai", "alloy", false)
|
||||
assert.False(t, changed)
|
||||
assert.Equal(t, "alloy", got)
|
||||
|
||||
// Incompatible edge voice with openai — falls back to openai default.
|
||||
got, changed = audio.FilterVoiceForProvider("openai", "en-US-GuyNeural", true)
|
||||
assert.True(t, changed)
|
||||
assert.Equal(t, "alloy", got)
|
||||
|
||||
// Incompatible openai voice with edge — falls back to edge default.
|
||||
got, changed = audio.FilterVoiceForProvider("edge", "alloy", true)
|
||||
assert.True(t, changed)
|
||||
assert.Equal(t, "en-US-AriaNeural", got)
|
||||
|
||||
// Empty voice — unchanged regardless.
|
||||
got, changed = audio.FilterVoiceForProvider("openai", "", false)
|
||||
assert.False(t, changed)
|
||||
assert.Equal(t, "", got)
|
||||
}
|
||||
+28
-3
@@ -103,7 +103,12 @@ type agentAudioConfig struct {
|
||||
// Empty return values signal "use provider default" downstream — they are not
|
||||
// errors. Missing agent snapshot emits slog.Warn so operators can spot
|
||||
// dispatch-layer regressions; missing tenant settings are quiet (common).
|
||||
func (t *TtsTool) resolveVoiceAndModel(ctx context.Context, argVoice, argModel string) (voice, model string) {
|
||||
//
|
||||
// voiceFromAgent is true when the returned voice originated from the agent's
|
||||
// tts_voice_id override (not from tool args or tenant defaults). Callers use
|
||||
// this flag to emit a warning when the voice is later found incompatible with
|
||||
// the selected provider.
|
||||
func (t *TtsTool) resolveVoiceAndModel(ctx context.Context, argVoice, argModel string) (voice, model string, voiceFromAgent bool) {
|
||||
voice, model = argVoice, argModel
|
||||
|
||||
// Pull agent-level config from the dispatcher-injected snapshot.
|
||||
@@ -136,6 +141,7 @@ func (t *TtsTool) resolveVoiceAndModel(ctx context.Context, argVoice, argModel s
|
||||
if voice == "" {
|
||||
if agentCfg.TTSVoiceID != "" {
|
||||
voice = agentCfg.TTSVoiceID
|
||||
voiceFromAgent = true
|
||||
} else if tenantCfg.DefaultVoiceID != "" {
|
||||
voice = tenantCfg.DefaultVoiceID
|
||||
}
|
||||
@@ -147,7 +153,23 @@ func (t *TtsTool) resolveVoiceAndModel(ctx context.Context, argVoice, argModel s
|
||||
model = tenantCfg.DefaultModel
|
||||
}
|
||||
}
|
||||
return voice, model
|
||||
return voice, model, voiceFromAgent
|
||||
}
|
||||
|
||||
// applyVoiceCompat checks whether voice is compatible with the named provider.
|
||||
// When incompatible and the voice came from an agent override, it logs a
|
||||
// warning and replaces the voice with the provider's default. The filtered
|
||||
// voice (possibly unchanged) is returned.
|
||||
func applyVoiceCompat(provider, voice string, voiceFromAgent bool) string {
|
||||
filtered, changed := audio.FilterVoiceForProvider(provider, voice, voiceFromAgent)
|
||||
if changed && voiceFromAgent {
|
||||
slog.Warn("tts: agent tts_voice_id is incompatible with selected provider, using provider default",
|
||||
"provider", provider,
|
||||
"agent_voice", voice,
|
||||
"fallback_voice", filtered,
|
||||
)
|
||||
}
|
||||
return filtered
|
||||
}
|
||||
|
||||
// resolvePrimary returns the effective primary provider name for the request.
|
||||
@@ -231,7 +253,7 @@ func (t *TtsTool) Execute(ctx context.Context, args map[string]any) *Result {
|
||||
providerName, _ := args["provider"].(string)
|
||||
|
||||
// Resolve voice/model via args > agent (ctx snapshot) > tenant > default.
|
||||
voice, model := t.resolveVoiceAndModel(ctx, argVoice, argModel)
|
||||
voice, model, voiceFromAgent := t.resolveVoiceAndModel(ctx, argVoice, argModel)
|
||||
|
||||
// Read generic agent TTS params once; adapt PER-ATTEMPT below (Finding #1 CRITICAL).
|
||||
// Storing generic keys here so each fallback provider gets its own adapted copy.
|
||||
@@ -264,6 +286,7 @@ func (t *TtsTool) Execute(ctx context.Context, args map[string]any) *Result {
|
||||
}
|
||||
providerName = tenantName
|
||||
}
|
||||
opts.Voice = applyVoiceCompat(providerName, opts.Voice, voiceFromAgent)
|
||||
if adapted := audio.AdaptAgentParams(genericAgentParams, providerName); len(adapted) > 0 {
|
||||
opts.Params = mergeParams(opts.Params, adapted)
|
||||
}
|
||||
@@ -272,6 +295,7 @@ func (t *TtsTool) Execute(ctx context.Context, args map[string]any) *Result {
|
||||
// Prefer the DB-configured tenant provider used by /tts and auto-TTS.
|
||||
if p, tenantName, ok := t.resolveTenantProvider(ctx, mgr, ""); ok {
|
||||
tenantOpts := opts
|
||||
tenantOpts.Voice = applyVoiceCompat(tenantName, tenantOpts.Voice, voiceFromAgent)
|
||||
if adapted := audio.AdaptAgentParams(genericAgentParams, tenantName); len(adapted) > 0 {
|
||||
tenantOpts.Params = mergeParams(opts.Params, adapted)
|
||||
}
|
||||
@@ -286,6 +310,7 @@ func (t *TtsTool) Execute(ctx context.Context, args map[string]any) *Result {
|
||||
if p, ok := mgr.GetProvider(primary); ok {
|
||||
// Adapt for the primary provider attempt specifically.
|
||||
primaryOpts := opts
|
||||
primaryOpts.Voice = applyVoiceCompat(primary, primaryOpts.Voice, voiceFromAgent)
|
||||
if adapted := audio.AdaptAgentParams(genericAgentParams, primary); len(adapted) > 0 {
|
||||
primaryOpts.Params = mergeParams(opts.Params, adapted)
|
||||
}
|
||||
|
||||
@@ -35,7 +35,7 @@ func ctxWithAgentAudio(t *testing.T, voiceID, modelID string) context.Context {
|
||||
func TestResolveVoiceAndModel_ArgsWinOverAgent(t *testing.T) {
|
||||
tool := NewTtsTool(makeTTSManager("elevenlabs"))
|
||||
ctx := ctxWithAgentAudio(t, "AGENT_V", "AGENT_M")
|
||||
v, m := tool.resolveVoiceAndModel(ctx, "ARG_V", "ARG_M")
|
||||
v, m, _ := tool.resolveVoiceAndModel(ctx, "ARG_V", "ARG_M")
|
||||
if v != "ARG_V" {
|
||||
t.Errorf("voice: got %q, want ARG_V (args must win)", v)
|
||||
}
|
||||
@@ -50,7 +50,7 @@ func TestResolveVoiceAndModel_AgentWinsOverTenantWhenArgsEmpty(t *testing.T) {
|
||||
ctx = WithBuiltinToolSettings(ctx, BuiltinToolSettings{
|
||||
"tts": rawJSON(t, map[string]string{"default_voice_id": "TENANT_V", "default_model": "TENANT_M"}),
|
||||
})
|
||||
v, m := tool.resolveVoiceAndModel(ctx, "", "")
|
||||
v, m, _ := tool.resolveVoiceAndModel(ctx, "", "")
|
||||
if v != "AGENT_V" {
|
||||
t.Errorf("voice: got %q, want AGENT_V (agent > tenant)", v)
|
||||
}
|
||||
@@ -65,7 +65,7 @@ func TestResolveVoiceAndModel_TenantFallbackWhenAgentSilent(t *testing.T) {
|
||||
ctx := WithBuiltinToolSettings(context.Background(), BuiltinToolSettings{
|
||||
"tts": rawJSON(t, map[string]string{"default_voice_id": "TENANT_V", "default_model": "TENANT_M"}),
|
||||
})
|
||||
v, m := tool.resolveVoiceAndModel(ctx, "", "")
|
||||
v, m, _ := tool.resolveVoiceAndModel(ctx, "", "")
|
||||
if v != "TENANT_V" {
|
||||
t.Errorf("voice: got %q, want TENANT_V", v)
|
||||
}
|
||||
@@ -76,7 +76,7 @@ func TestResolveVoiceAndModel_TenantFallbackWhenAgentSilent(t *testing.T) {
|
||||
|
||||
func TestResolveVoiceAndModel_EmptyAllMeansDefault(t *testing.T) {
|
||||
tool := NewTtsTool(makeTTSManager("elevenlabs"))
|
||||
v, m := tool.resolveVoiceAndModel(context.Background(), "", "")
|
||||
v, m, _ := tool.resolveVoiceAndModel(context.Background(), "", "")
|
||||
if v != "" {
|
||||
t.Errorf("voice: got %q, want empty (no sources → provider default)", v)
|
||||
}
|
||||
@@ -92,7 +92,7 @@ func TestResolveVoiceAndModel_PartialAgentConfig(t *testing.T) {
|
||||
ctx = WithBuiltinToolSettings(ctx, BuiltinToolSettings{
|
||||
"tts": rawJSON(t, map[string]string{"default_model": "TENANT_M"}),
|
||||
})
|
||||
v, m := tool.resolveVoiceAndModel(ctx, "", "")
|
||||
v, m, _ := tool.resolveVoiceAndModel(ctx, "", "")
|
||||
if v != "AGENT_V" {
|
||||
t.Errorf("voice: got %q, want AGENT_V", v)
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user