diff --git a/internal/audio/agent_params_adapter.go b/internal/audio/agent_params_adapter.go index 0dd7dbff..c4a83029 100644 --- a/internal/audio/agent_params_adapter.go +++ b/internal/audio/agent_params_adapter.go @@ -1,17 +1,36 @@ package audio -import "log/slog" +import ( + "fmt" + "log/slog" +) -// agentOverrideKeys is the allow-list of generic keys that agents may store in -// other_config.tts_params. Any key outside this set is rejected at write time -// (see internal/http/agents.go validateAgentTTSParams) and silently dropped -// here for defense-in-depth. -var agentOverrideKeys = map[string]bool{ +// AgentTTSParamsAllowedKeys is the allow-list of generic keys that agents may +// store in other_config.tts_params. Any key outside this set is rejected at +// write time and silently dropped here for defense-in-depth. +// Both internal/http and internal/gateway/methods import this to avoid +// duplicating the literal (Action D: DRY). +var AgentTTSParamsAllowedKeys = map[string]bool{ "speed": true, "emotion": true, "style": true, } +// agentOverrideKeys aliases the exported map for internal use in AdaptAgentParams. +var agentOverrideKeys = AgentTTSParamsAllowedKeys + +// ValidateAgentTTSParams returns an error if ttsParams contains any key not in +// the allow-list. Values are not type-checked here — providers handle coercion +// at synthesis time. +func ValidateAgentTTSParams(ttsParams map[string]any) error { + for k := range ttsParams { + if !AgentTTSParamsAllowedKeys[k] { + return fmt.Errorf("tts_params key %q is not allowed; valid keys: speed, emotion, style", k) + } + } + return nil +} + // AdaptAgentParams maps generic agent override keys (stored in // agents.other_config.tts_params) to the provider-specific param keys that // each provider's Synthesize implementation expects in opts.Params. diff --git a/internal/audio/agent_params_adapter_test.go b/internal/audio/agent_params_adapter_test.go index be31fa83..58a0f257 100644 --- a/internal/audio/agent_params_adapter_test.go +++ b/internal/audio/agent_params_adapter_test.go @@ -88,13 +88,17 @@ func TestAdaptAgentParams_TableDriven(t *testing.T) { // TestAdaptAgentParams_Finding9_CrossCheck verifies that every provider with // at least one AdaptAgentParams mapping also has at least one ParamSchema with -// AgentOverridable=true in its capabilities, AND vice versa. +// non-empty AgentOverridableAs in its capabilities, AND vice versa. // This is the compile-guard drift test mandated by red-team Finding #9. +// +// The new AgentOverridableAs field (string) replaces AgentOverridable (bool) to +// encode the generic key alias directly in the capability schema — eliminating +// the separate hard-coded genericToNative lookup tables in each UI. func TestAdaptAgentParams_Finding9_CrossCheck(t *testing.T) { t.Parallel() // Providers that the adapter switch handles (non-skip branches). - // Each must have ≥1 AgentOverridable param in capabilities. + // Each must have ≥1 ParamSchema with non-empty AgentOverridableAs in capabilities. overridableProviders := map[string]struct { caps []audio.ParamSchema adapterMap map[string]any // any non-nil output proves mapping exists @@ -119,27 +123,43 @@ func TestAdaptAgentParams_Finding9_CrossCheck(t *testing.T) { t.Errorf("provider %q: AdaptAgentParams returned empty map — adapter switch has no active branches", providerName) } - // 2. Capabilities must have at least one AgentOverridable param. + // 2. Capabilities must have at least one param with non-empty AgentOverridableAs. hasOverridable := false for _, p := range info.caps { - if p.AgentOverridable { + if p.AgentOverridableAs != "" { hasOverridable = true break } } if !hasOverridable { - t.Errorf("provider %q: no ParamSchema with AgentOverridable=true in capabilities", providerName) + t.Errorf("provider %q: no ParamSchema with non-empty AgentOverridableAs in capabilities", providerName) } } - // Providers that are skip-only (edge, gemini) must NOT have AgentOverridable params. - skipProviders := map[string][]audio.ParamSchema{ - // Edge and Gemini capabilities are not imported here — check via adapter output only. - // The adapter returns nil for both, which is the contract. + // 3. Cross-check: every (nativeKey, genericAlias) pair in capabilities must + // be reflected in the adapter switch. AdaptAgentParams(generic=genericAlias) + // for this provider must map to nativeKey in the output. + for providerName, info := range overridableProviders { + for _, p := range info.caps { + if p.AgentOverridableAs == "" { + continue + } + genericKey := p.AgentOverridableAs + nativeKey := p.Key + result := audio.AdaptAgentParams(map[string]any{genericKey: "sentinel"}, providerName) + if result == nil { + t.Errorf("provider %q param %q (generic %q): AdaptAgentParams returned nil — adapter switch missing branch", + providerName, nativeKey, genericKey) + continue + } + if _, ok := result[nativeKey]; !ok { + t.Errorf("provider %q param %q (generic %q): adapter output %v does not contain expected native key", + providerName, nativeKey, genericKey, result) + } + } } - _ = skipProviders // nothing to assert for now; gap noted in phase doc - // Also verify skip providers return nil from adapter. + // Providers that are skip-only (edge, gemini) must return nil from adapter. for _, skipProvider := range []string{"edge", "gemini"} { out := audio.AdaptAgentParams(map[string]any{"speed": 1.0, "emotion": "happy", "style": 0.5}, skipProvider) if len(out) > 0 { diff --git a/internal/audio/elevenlabs/capabilities.go b/internal/audio/elevenlabs/capabilities.go index 9d1fa0c4..1af5b55c 100644 --- a/internal/audio/elevenlabs/capabilities.go +++ b/internal/audio/elevenlabs/capabilities.go @@ -61,15 +61,15 @@ var elevenLabsParams = []audio.ParamSchema{ Step: &simBoostStep, }, { - Key: "voice_settings.style", - Type: audio.ParamTypeRange, - Label: "Style Exaggeration", - Description: "Style exaggeration amount (0 = none, 1 = maximum).", - Default: 0.0, - Min: &styleMin, - Max: &styleMax, - Step: &styleStep, - AgentOverridable: true, + Key: "voice_settings.style", + Type: audio.ParamTypeRange, + Label: "Style Exaggeration", + Description: "Style exaggeration amount (0 = none, 1 = maximum).", + Default: 0.0, + Min: &styleMin, + Max: &styleMax, + Step: &styleStep, + AgentOverridableAs: "style", }, { Key: "voice_settings.use_speaker_boost", @@ -78,15 +78,15 @@ var elevenLabsParams = []audio.ParamSchema{ Default: true, }, { - Key: "voice_settings.speed", - Type: audio.ParamTypeRange, - Label: "Speed", - Description: "Speech speed (0.7 = slowest, 1.2 = fastest).", - Default: 1.0, - Min: &elSpeedMin, - Max: &elSpeedMax, - Step: &elSpeedStep, - AgentOverridable: true, + Key: "voice_settings.speed", + Type: audio.ParamTypeRange, + Label: "Speed", + Description: "Speech speed (0.7 = slowest, 1.2 = fastest).", + Default: 1.0, + Min: &elSpeedMin, + Max: &elSpeedMax, + Step: &elSpeedStep, + AgentOverridableAs: "speed", }, { Key: "apply_text_normalization", diff --git a/internal/audio/minimax/capabilities.go b/internal/audio/minimax/capabilities.go index ebcc8b30..6acb515f 100644 --- a/internal/audio/minimax/capabilities.go +++ b/internal/audio/minimax/capabilities.go @@ -32,15 +32,15 @@ var ( // - all others = "" (basic) var minimaxParams = []audio.ParamSchema{ { - Key: "speed", - Type: audio.ParamTypeRange, - Label: "Speed", - Description: "Speech speed multiplier (0.5 = slowest, 2.0 = fastest).", - Default: 1.0, - Min: &mmSpeedMin, - Max: &mmSpeedMax, - Step: &mmSpeedStep, - AgentOverridable: true, + Key: "speed", + Type: audio.ParamTypeRange, + Label: "Speed", + Description: "Speech speed multiplier (0.5 = slowest, 2.0 = fastest).", + Default: 1.0, + Min: &mmSpeedMin, + Max: &mmSpeedMax, + Step: &mmSpeedStep, + AgentOverridableAs: "speed", }, { Key: "vol", @@ -63,10 +63,10 @@ var minimaxParams = []audio.ParamSchema{ Step: &mmPitchStep, }, { - Key: "emotion", - Type: audio.ParamTypeEnum, - Label: "Emotion", - AgentOverridable: true, + Key: "emotion", + Type: audio.ParamTypeEnum, + Label: "Emotion", + AgentOverridableAs: "emotion", Default: "", Enum: []audio.EnumOption{ {Value: "", Label: "None (default)"}, diff --git a/internal/audio/openai/capabilities.go b/internal/audio/openai/capabilities.go index 78093cd8..0e7b0448 100644 --- a/internal/audio/openai/capabilities.go +++ b/internal/audio/openai/capabilities.go @@ -46,7 +46,7 @@ var openAIParams = []audio.ParamSchema{ Min: &speedMin, Max: &speedMax, Step: &speedStep, - AgentOverridable: true, + AgentOverridableAs: "speed", }, { Key: "response_format", diff --git a/internal/audio/types_capabilities.go b/internal/audio/types_capabilities.go index d5b5fe94..a1da780f 100644 --- a/internal/audio/types_capabilities.go +++ b/internal/audio/types_capabilities.go @@ -62,11 +62,15 @@ type ParamSchema struct { // Group categorises the param for collapsible UI sections. // "" (empty) = basic (always visible); "advanced" = collapsed by default. Group string `json:"group,omitempty"` - // AgentOverridable when true means this param can be stored in - // agents.other_config.tts_params and will be applied per-agent via - // AdaptAgentParams. Frontend filters to agent_overridable===true params only - // so it never shows non-overridable knobs in the agent override UI. - AgentOverridable bool `json:"agent_overridable,omitempty"` + // AgentOverridableAs, when non-empty, is the generic key alias that this + // param maps to in agents.other_config.tts_params (e.g. "speed", "emotion", + // "style"). Empty string means this param is NOT agent-overridable. + // Frontend filters to non-empty AgentOverridableAs params only, so it never + // shows non-overridable knobs in the agent override UI. + // The value doubles as the conversion key: UI uses it to map between the + // native capability key (e.g. "voice_settings.speed") and the generic stored + // key (e.g. "speed") without needing a separate hard-coded lookup table. + AgentOverridableAs string `json:"agent_overridable_as,omitempty"` } // ProviderCapabilities is the catalog entry for a single TTS provider. diff --git a/internal/gateway/methods/agents_update.go b/internal/gateway/methods/agents_update.go index c62e321f..e1f96651 100644 --- a/internal/gateway/methods/agents_update.go +++ b/internal/gateway/methods/agents_update.go @@ -8,6 +8,7 @@ import ( "os" "strings" + "github.com/nextlevelbuilder/goclaw/internal/audio" "github.com/nextlevelbuilder/goclaw/internal/bootstrap" "github.com/nextlevelbuilder/goclaw/internal/config" "github.com/nextlevelbuilder/goclaw/internal/gateway" @@ -133,15 +134,13 @@ func (m *AgentsMethods) handleUpdate(ctx context.Context, client *gateway.Client client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, err.Error())) return } - // Finding #5: validate tts_params allow-list (speed, emotion, style only). + // Finding #5: validate tts_params allow-list via shared audio validator + // (Action D: single source of truth in internal/audio). if tp, ok := otherMap["tts_params"]; ok && tp != nil { if tpMap, ok := tp.(map[string]any); ok { - for k := range tpMap { - if k != "speed" && k != "emotion" && k != "style" { - client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, - fmt.Sprintf("tts_params key %q is not allowed; valid keys: speed, emotion, style", k))) - return - } + if err := audio.ValidateAgentTTSParams(tpMap); err != nil { + client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, err.Error())) + return } } } diff --git a/internal/http/validate.go b/internal/http/validate.go index c09288f9..eb3d8ed5 100644 --- a/internal/http/validate.go +++ b/internal/http/validate.go @@ -1,9 +1,10 @@ package http import ( - "fmt" "log/slog" "regexp" + + "github.com/nextlevelbuilder/goclaw/internal/audio" ) var slugRe = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`) @@ -28,26 +29,11 @@ func filterAllowedKeys(updates map[string]any, allowed map[string]bool) map[stri return filtered } -// agentTTSParamsAllowedKeys is the allow-list for keys in -// agents.other_config.tts_params. Any key outside this set is rejected at -// write time (Finding #5). AdaptAgentParams provides defense-in-depth at -// read time, but this gate ensures the DB never accumulates stale bad keys. -var agentTTSParamsAllowedKeys = map[string]bool{ - "speed": true, - "emotion": true, - "style": true, -} - -// validateAgentTTSParams returns an error if ttsParams contains any key -// not in the allow-list. Values are not type-checked here — providers handle -// coercion at synthesis time. +// validateAgentTTSParams is a thin wrapper around audio.ValidateAgentTTSParams +// so HTTP handlers can call it without importing the audio package directly. +// The allow-list is owned by internal/audio (single source of truth, Action D). func validateAgentTTSParams(ttsParams map[string]any) error { - for k := range ttsParams { - if !agentTTSParamsAllowedKeys[k] { - return fmt.Errorf("tts_params key %q is not allowed; valid keys: speed, emotion, style", k) - } - } - return nil + return audio.ValidateAgentTTSParams(ttsParams) } // --- Field allowlists for update endpoints --- diff --git a/internal/i18n/catalog_en.go b/internal/i18n/catalog_en.go index fbc2f73b..73f556db 100644 --- a/internal/i18n/catalog_en.go +++ b/internal/i18n/catalog_en.go @@ -205,8 +205,6 @@ func init() { MsgTtsGeminiSpeakerLimit: "Gemini TTS supports at most 2 speakers", MsgTtsGeminiInvalidModel: "invalid Gemini TTS model: %s", MsgTtsParamOutOfRange: "TTS param %q value %v is out of range [%v, %v]", - MsgTtsParamDependsOn: "TTS param %q requires %q to be %q", - MsgTtsParamInvalidJSON: "TTS param %q contains invalid JSON (length %d)", MsgTtsParamUnknownKey: "TTS param %q is not supported by this provider", MsgTtsMiniMaxVoicesFailed: "failed to fetch MiniMax voices: %s", diff --git a/internal/i18n/catalog_vi.go b/internal/i18n/catalog_vi.go index 2dbc3108..0aa40417 100644 --- a/internal/i18n/catalog_vi.go +++ b/internal/i18n/catalog_vi.go @@ -205,8 +205,6 @@ func init() { MsgTtsGeminiSpeakerLimit: "Gemini TTS hỗ trợ tối đa 2 người nói", MsgTtsGeminiInvalidModel: "mô hình Gemini TTS không hợp lệ: %s", MsgTtsParamOutOfRange: "tham số TTS %q có giá trị %v nằm ngoài phạm vi [%v, %v]", - MsgTtsParamDependsOn: "tham số TTS %q yêu cầu %q phải là %q", - MsgTtsParamInvalidJSON: "tham số TTS %q chứa JSON không hợp lệ (độ dài %d)", MsgTtsParamUnknownKey: "tham số TTS %q không được nhà cung cấp này hỗ trợ", MsgTtsMiniMaxVoicesFailed: "không tải được danh sách giọng đọc MiniMax: %s", diff --git a/internal/i18n/catalog_zh.go b/internal/i18n/catalog_zh.go index 1072cccf..43e25352 100644 --- a/internal/i18n/catalog_zh.go +++ b/internal/i18n/catalog_zh.go @@ -205,8 +205,6 @@ func init() { MsgTtsGeminiSpeakerLimit: "Gemini TTS 最多支持 2 位发言人", MsgTtsGeminiInvalidModel: "无效的 Gemini TTS 模型：%s", MsgTtsParamOutOfRange: "TTS 参数 %q 的值 %v 超出范围 [%v, %v]", - MsgTtsParamDependsOn: "TTS 参数 %q 需要将 %q 设置为 %q", - MsgTtsParamInvalidJSON: "TTS 参数 %q 包含无效的 JSON（长度 %d）", MsgTtsParamUnknownKey: "TTS 参数 %q 不受此提供商支持", MsgTtsMiniMaxVoicesFailed: "获取 MiniMax 声音列表失败：%s", diff --git a/internal/i18n/keys.go b/internal/i18n/keys.go index 44836f26..c9bb57e6 100644 --- a/internal/i18n/keys.go +++ b/internal/i18n/keys.go @@ -203,8 +203,6 @@ const ( MsgTtsGeminiSpeakerLimit = "error.tts_gemini_speaker_limit" // "Gemini TTS supports at most 2 speakers" MsgTtsGeminiInvalidModel = "error.tts_gemini_invalid_model" // "invalid Gemini TTS model: %s" MsgTtsParamOutOfRange = "error.tts_param_out_of_range" // "TTS param %q value %v is out of range [%v, %v]" - MsgTtsParamDependsOn = "error.tts_param_depends_on" // "TTS param %q requires %q to be %q" - MsgTtsParamInvalidJSON = "error.tts_param_invalid_json" // "TTS param %q contains invalid JSON (length %d)" MsgTtsParamUnknownKey = "error.tts_param_unknown_key" // "TTS param %q is not supported by this provider" MsgTtsMiniMaxVoicesFailed = "error.tts_minimax_voices_failed" // "failed to fetch MiniMax voices: %s" diff --git a/ui/desktop/frontend/src/api/tts-capabilities.ts b/ui/desktop/frontend/src/api/tts-capabilities.ts index cc7d8b1c..9998ec04 100644 --- a/ui/desktop/frontend/src/api/tts-capabilities.ts +++ b/ui/desktop/frontend/src/api/tts-capabilities.ts @@ -37,10 +37,12 @@ export interface ParamSchema { /** "advanced" → Advanced section; absent/undefined → Basic section (forward-compat: unknown values → Basic). */ group?: string; /** - * When true this param can be stored as a per-agent override in - * agents.other_config.tts_params (Finding #9: single source of truth). + * When non-empty, this param is agent-overridable and the value is the + * generic key alias stored in agents.other_config.tts_params (e.g. "speed", + * "emotion", "style"). Empty / absent = not overridable. + * Single source of truth for generic↔native key mapping (Finding #9). */ - agent_overridable?: boolean; + agent_overridable_as?: string; } export interface VoiceOption { diff --git a/ui/desktop/frontend/src/components/agents/AgentDetailPanel.tsx b/ui/desktop/frontend/src/components/agents/AgentDetailPanel.tsx index c25b8499..7bd6f315 100644 --- a/ui/desktop/frontend/src/components/agents/AgentDetailPanel.tsx +++ b/ui/desktop/frontend/src/components/agents/AgentDetailPanel.tsx @@ -18,7 +18,7 @@ import { AgentMcpSection } from './AgentMcpSection' import { AgentFilesTab } from './AgentFilesTab' import { VoicePicker } from './voice-picker' import { TtsEmptyState } from './tts-empty-state' -import { DynamicParamForm } from '../dynamic-param-form' +import { TtsOverrideFineTune } from './tts-override-fine-tune' import type { ParamValue } from '../dynamic-param-form' import { ConfirmDialog } from '../common/ConfirmDialog' import { useAgentDetailState } from '../../hooks/use-agent-detail-state' @@ -222,53 +222,14 @@ export function AgentDetailPanel({ agent, onSave, onResummon, onClose }: AgentDe provider={globalProvider as TtsProviderId} /> {/* Fine-tune: per-agent overridable TTS params (Finding #9). - Reads agent_overridable flag from capabilities — single source of truth. */} - {(() => { - const providerCaps = allCaps?.find((c) => c.provider === globalProvider) - const overridableParams = (providerCaps?.params ?? []).filter( - (p) => p.agent_overridable === true, - ) - if (overridableParams.length === 0) return null - // Desktop adapter: generic keys (same as minimax/openai flat keys). - // For ElevenLabs, voice_settings.speed maps to generic "speed" etc. - const genericToNative: Record = { - openai: { speed: 'speed' }, - elevenlabs: { speed: 'voice_settings.speed', style: 'voice_settings.style' }, - minimax: { speed: 'speed', emotion: 'emotion' }, - edge: {}, - gemini: {}, - }[globalProvider] ?? {} - const nativeToGeneric: Record = {} - for (const [g, n] of Object.entries(genericToNative)) nativeToGeneric[n] = g - - // Build native form state from stored generic params. - const nativeFormState: Record = {} - for (const [generic, val] of Object.entries(ttsParams)) { - const native = genericToNative[generic] - if (native) nativeFormState[native] = val as ParamValue - } - const handleParamChange = (nativeKey: string, val: ParamValue) => { - const updated = { ...nativeFormState, [nativeKey]: val } - const generic: Record = {} - for (const [n, v] of Object.entries(updated)) { - const g = nativeToGeneric[n] - if (g) generic[g] = v - } - setTtsParams(generic) - } - return ( -
-

- {t('tts:override.params.title')} -

- -
- ) - })()} + agent_overridable_as in capabilities is the single source of truth + for which params are shown and how generic↔native keys are mapped. */} + ) : ( diff --git a/ui/desktop/frontend/src/components/agents/tts-override-fine-tune.tsx b/ui/desktop/frontend/src/components/agents/tts-override-fine-tune.tsx new file mode 100644 index 00000000..78f21099 --- /dev/null +++ b/ui/desktop/frontend/src/components/agents/tts-override-fine-tune.tsx @@ -0,0 +1,81 @@ +/** + * TtsOverrideFineTune — per-agent TTS fine-tune param editor for the desktop. + * + * Renders a DynamicParamForm filtered to agent-overridable params only. + * The generic↔native key mapping is derived from param.agent_overridable_as + * (Finding #9: single source of truth in capabilities API — no hard-coded table). + * + * Props: + * globalProvider — the globally configured TTS provider id (e.g. "openai") + * allCaps — full provider capabilities list (from useTtsCapabilities) + * ttsParams — current generic params state (e.g. {speed: 1.2}) + * onChange — called with updated generic params on user change + */ +import { useTranslation } from 'react-i18next' +import { DynamicParamForm } from '../dynamic-param-form' +import type { ParamValue } from '../dynamic-param-form' +import type { ProviderCapabilities } from '../../api/tts-capabilities' + +interface Props { + globalProvider: string + allCaps: ProviderCapabilities[] | undefined + ttsParams: Record + onChange: (updated: Record) => void +} + +/** Build generic→native and native→generic maps from an overridable params slice. */ +function buildAdapterMaps(params: Array<{ key: string; agent_overridable_as?: string }>) { + const genericToNative: Record = {} + const nativeToGeneric: Record = {} + for (const p of params) { + if (p.agent_overridable_as) { + genericToNative[p.agent_overridable_as] = p.key + nativeToGeneric[p.key] = p.agent_overridable_as + } + } + return { genericToNative, nativeToGeneric } +} + +export function TtsOverrideFineTune({ globalProvider, allCaps, ttsParams, onChange }: Props) { + const { t } = useTranslation('tts') + + const providerCaps = allCaps?.find((c) => c.provider === globalProvider) + const overridableParams = (providerCaps?.params ?? []).filter( + (p) => (p.agent_overridable_as ?? '') !== '', + ) + + if (overridableParams.length === 0) return null + + const { genericToNative, nativeToGeneric } = buildAdapterMaps(overridableParams) + + // Convert stored generic params → native form state for DynamicParamForm. + const nativeFormState: Record = {} + for (const [generic, val] of Object.entries(ttsParams)) { + const native = genericToNative[generic] + if (native) nativeFormState[native] = val as ParamValue + } + + const handleParamChange = (nativeKey: string, val: ParamValue) => { + const updated = { ...nativeFormState, [nativeKey]: val } + // Convert native form state → generic keys for storage. + const generic: Record = {} + for (const [n, v] of Object.entries(updated)) { + const g = nativeToGeneric[n] + if (g) generic[g] = v + } + onChange(generic) + } + + return ( +
+

+ {t('override.params.title')} +

+ +
+ ) +} diff --git a/ui/web/src/api/tts-capabilities.ts b/ui/web/src/api/tts-capabilities.ts index 1428bfa7..38df1e90 100644 --- a/ui/web/src/api/tts-capabilities.ts +++ b/ui/web/src/api/tts-capabilities.ts @@ -37,11 +37,13 @@ export interface ParamSchema { /** "advanced" → Advanced section; absent/undefined → Basic section (forward-compat: unknown values → Basic). */ group?: string; /** - * When true this param can be stored as a per-agent override in - * agents.other_config.tts_params. The UI shows only agent_overridable=true - * params in the fine-tune section (Finding #9: single source of truth). + * When non-empty, this param is agent-overridable and the value is the + * generic key alias stored in agents.other_config.tts_params (e.g. "speed", + * "emotion", "style"). Empty / absent = not overridable. + * Used as the single source of truth for the generic↔native key mapping in + * the UI — no separate hard-coded lookup table needed (Finding #9). */ - agent_overridable?: boolean; + agent_overridable_as?: string; } export interface VoiceOption { diff --git a/ui/web/src/pages/agents/agent-detail/overview-sections/__tests__/tts-override-block-adapter.test.ts b/ui/web/src/pages/agents/agent-detail/overview-sections/__tests__/tts-override-block-adapter.test.ts index 89c8f27b..850a2f27 100644 --- a/ui/web/src/pages/agents/agent-detail/overview-sections/__tests__/tts-override-block-adapter.test.ts +++ b/ui/web/src/pages/agents/agent-detail/overview-sections/__tests__/tts-override-block-adapter.test.ts @@ -6,69 +6,122 @@ * - Form state (capability-native keys: voice_settings.speed, etc.) — what DynamicParamForm uses * * Tests cover all 5 providers and the round-trip invariant. + * + * NOTE: Since Finding #9 fix, the adapter functions accept an overridableParams array + * (not a provider string). Provider-specific mapping is now encoded in each param's + * agent_overridable_as field — no separate hard-coded lookup table. */ import { describe, it, expect } from "vitest"; import { genericToNativeFormState, nativeFormStateToGeneric, + buildAdapterMaps, } from "../tts-override-block"; +// ---- Per-provider overridable param stubs ---- +// These mirror the capabilities declared in the Go provider files. + +const openaiOverridableParams = [ + { key: "speed", agent_overridable_as: "speed" }, +]; + +const elevenLabsOverridableParams = [ + { key: "voice_settings.style", agent_overridable_as: "style" }, + { key: "voice_settings.speed", agent_overridable_as: "speed" }, +]; + +const minimaxOverridableParams = [ + { key: "speed", agent_overridable_as: "speed" }, + { key: "emotion", agent_overridable_as: "emotion" }, +]; + +const edgeOverridableParams: Array<{ key: string; agent_overridable_as?: string }> = []; +const geminiOverridableParams: Array<{ key: string; agent_overridable_as?: string }> = []; + +// ---- buildAdapterMaps ---- + +describe("buildAdapterMaps", () => { + it("openai: builds correct forward and inverse maps", () => { + const { genericToNative, nativeToGeneric } = buildAdapterMaps(openaiOverridableParams); + expect(genericToNative).toEqual({ speed: "speed" }); + expect(nativeToGeneric).toEqual({ speed: "speed" }); + }); + + it("elevenlabs: builds correct forward and inverse maps", () => { + const { genericToNative, nativeToGeneric } = buildAdapterMaps(elevenLabsOverridableParams); + expect(genericToNative).toEqual({ + style: "voice_settings.style", + speed: "voice_settings.speed", + }); + expect(nativeToGeneric).toEqual({ + "voice_settings.style": "style", + "voice_settings.speed": "speed", + }); + }); + + it("empty params → empty maps", () => { + const { genericToNative, nativeToGeneric } = buildAdapterMaps([]); + expect(genericToNative).toEqual({}); + expect(nativeToGeneric).toEqual({}); + }); +}); + // ---- genericToNativeFormState (load direction: storage → form) ---- describe("genericToNativeFormState", () => { it("openai: speed stays flat", () => { - expect(genericToNativeFormState({ speed: 1.5 }, "openai")).toEqual({ speed: 1.5 }); + expect(genericToNativeFormState({ speed: 1.5 }, openaiOverridableParams)).toEqual({ speed: 1.5 }); }); it("openai: emotion dropped (not supported)", () => { - const out = genericToNativeFormState({ speed: 1.5, emotion: "happy" }, "openai"); + const out = genericToNativeFormState({ speed: 1.5, emotion: "happy" }, openaiOverridableParams); expect(out).toEqual({ speed: 1.5 }); }); it("elevenlabs: speed → voice_settings.speed", () => { - expect(genericToNativeFormState({ speed: 1.1 }, "elevenlabs")).toEqual({ + expect(genericToNativeFormState({ speed: 1.1 }, elevenLabsOverridableParams)).toEqual({ "voice_settings.speed": 1.1, }); }); it("elevenlabs: style → voice_settings.style", () => { - expect(genericToNativeFormState({ style: 0.5 }, "elevenlabs")).toEqual({ + expect(genericToNativeFormState({ style: 0.5 }, elevenLabsOverridableParams)).toEqual({ "voice_settings.style": 0.5, }); }); it("elevenlabs: emotion dropped", () => { - const out = genericToNativeFormState({ speed: 1.0, style: 0.3, emotion: "happy" }, "elevenlabs"); + const out = genericToNativeFormState({ speed: 1.0, style: 0.3, emotion: "happy" }, elevenLabsOverridableParams); expect(out).toEqual({ "voice_settings.speed": 1.0, "voice_settings.style": 0.3 }); }); it("minimax: speed stays flat", () => { - expect(genericToNativeFormState({ speed: 0.9 }, "minimax")).toEqual({ speed: 0.9 }); + expect(genericToNativeFormState({ speed: 0.9 }, minimaxOverridableParams)).toEqual({ speed: 0.9 }); }); it("minimax: emotion stays flat", () => { - expect(genericToNativeFormState({ emotion: "neutral" }, "minimax")).toEqual({ emotion: "neutral" }); + expect(genericToNativeFormState({ emotion: "neutral" }, minimaxOverridableParams)).toEqual({ emotion: "neutral" }); }); it("minimax: style dropped", () => { - const out = genericToNativeFormState({ speed: 1.0, emotion: "happy", style: 0.5 }, "minimax"); + const out = genericToNativeFormState({ speed: 1.0, emotion: "happy", style: 0.5 }, minimaxOverridableParams); expect(out).toEqual({ speed: 1.0, emotion: "happy" }); }); it("edge: all keys dropped", () => { - expect(genericToNativeFormState({ speed: 1.0, emotion: "happy", style: 0.5 }, "edge")).toEqual({}); + expect(genericToNativeFormState({ speed: 1.0, emotion: "happy", style: 0.5 }, edgeOverridableParams)).toEqual({}); }); it("gemini: all keys dropped", () => { - expect(genericToNativeFormState({ speed: 1.0, emotion: "happy", style: 0.5 }, "gemini")).toEqual({}); + expect(genericToNativeFormState({ speed: 1.0, emotion: "happy", style: 0.5 }, geminiOverridableParams)).toEqual({}); }); it("empty input → empty output", () => { - expect(genericToNativeFormState({}, "openai")).toEqual({}); + expect(genericToNativeFormState({}, openaiOverridableParams)).toEqual({}); }); - it("unknown provider → all keys dropped", () => { - expect(genericToNativeFormState({ speed: 1.0 }, "azure")).toEqual({}); + it("unknown provider (empty params) → all keys dropped", () => { + expect(genericToNativeFormState({ speed: 1.0 }, [])).toEqual({}); }); }); @@ -76,17 +129,17 @@ describe("genericToNativeFormState", () => { describe("nativeFormStateToGeneric", () => { it("openai: flat speed → generic speed", () => { - expect(nativeFormStateToGeneric({ speed: 1.5 }, "openai")).toEqual({ speed: 1.5 }); + expect(nativeFormStateToGeneric({ speed: 1.5 }, openaiOverridableParams)).toEqual({ speed: 1.5 }); }); it("elevenlabs: voice_settings.speed → generic speed", () => { - expect(nativeFormStateToGeneric({ "voice_settings.speed": 1.1 }, "elevenlabs")).toEqual({ + expect(nativeFormStateToGeneric({ "voice_settings.speed": 1.1 }, elevenLabsOverridableParams)).toEqual({ speed: 1.1, }); }); it("elevenlabs: voice_settings.style → generic style", () => { - expect(nativeFormStateToGeneric({ "voice_settings.style": 0.4 }, "elevenlabs")).toEqual({ + expect(nativeFormStateToGeneric({ "voice_settings.style": 0.4 }, elevenLabsOverridableParams)).toEqual({ style: 0.4, }); }); @@ -94,45 +147,48 @@ describe("nativeFormStateToGeneric", () => { it("elevenlabs: both native keys → both generic keys", () => { const out = nativeFormStateToGeneric( { "voice_settings.speed": 1.0, "voice_settings.style": 0.3 }, - "elevenlabs", + elevenLabsOverridableParams, ); expect(out).toEqual({ speed: 1.0, style: 0.3 }); }); it("minimax: flat speed+emotion → generic", () => { - expect(nativeFormStateToGeneric({ speed: 0.9, emotion: "neutral" }, "minimax")).toEqual({ + expect(nativeFormStateToGeneric({ speed: 0.9, emotion: "neutral" }, minimaxOverridableParams)).toEqual({ speed: 0.9, emotion: "neutral", }); }); it("edge: no mappings → empty", () => { - expect(nativeFormStateToGeneric({ speed: 1.0 }, "edge")).toEqual({}); + expect(nativeFormStateToGeneric({ speed: 1.0 }, edgeOverridableParams)).toEqual({}); }); it("gemini: no mappings → empty", () => { - expect(nativeFormStateToGeneric({ speed: 1.0 }, "gemini")).toEqual({}); + expect(nativeFormStateToGeneric({ speed: 1.0 }, geminiOverridableParams)).toEqual({}); }); }); // ---- Round-trip invariant ---- -// For any generic params + provider, load then save must return the same generic map. +// For any generic params + overridableParams, load then save must return the same generic map. describe("round-trip: genericToNative → nativeToGeneric", () => { - const cases: Array<{ provider: string; generic: Record }> = [ - { provider: "openai", generic: { speed: 1.5 } }, - { provider: "elevenlabs", generic: { speed: 1.1, style: 0.4 } }, - { provider: "minimax", generic: { speed: 0.9, emotion: "happy" } }, - { provider: "edge", generic: {} }, - { provider: "gemini", generic: {} }, - // Keys unsupported by provider get dropped on load, so round-trip correctly excludes them. - { provider: "openai", generic: {} }, // emotion/style stripped on load + const cases: Array<{ + label: string; + params: Array<{ key: string; agent_overridable_as?: string }>; + generic: Record; + }> = [ + { label: "openai/speed", params: openaiOverridableParams, generic: { speed: 1.5 } }, + { label: "elevenlabs/speed+style", params: elevenLabsOverridableParams, generic: { speed: 1.1, style: 0.4 } }, + { label: "minimax/speed+emotion", params: minimaxOverridableParams, generic: { speed: 0.9, emotion: "happy" } }, + { label: "edge/empty", params: edgeOverridableParams, generic: {} }, + { label: "gemini/empty", params: geminiOverridableParams, generic: {} }, + { label: "openai/empty-generic", params: openaiOverridableParams, generic: {} }, ]; - for (const { provider, generic } of cases) { - it(`${provider}: round-trip preserves supported keys`, () => { - const nativeState = genericToNativeFormState(generic as Record, provider); - const backToGeneric = nativeFormStateToGeneric(nativeState, provider); + for (const { label, params, generic } of cases) { + it(`${label}: round-trip preserves supported keys`, () => { + const nativeState = genericToNativeFormState(generic as Record, params); + const backToGeneric = nativeFormStateToGeneric(nativeState, params); expect(backToGeneric).toEqual(generic); }); } diff --git a/ui/web/src/pages/agents/agent-detail/overview-sections/prompt-settings-section.tsx b/ui/web/src/pages/agents/agent-detail/overview-sections/prompt-settings-section.tsx index fcf87d68..543a2f38 100644 --- a/ui/web/src/pages/agents/agent-detail/overview-sections/prompt-settings-section.tsx +++ b/ui/web/src/pages/agents/agent-detail/overview-sections/prompt-settings-section.tsx @@ -78,15 +78,14 @@ export function PromptSettingsSection({ agent, onUpdate }: Props) { const handleSave = async () => { setSaving(true); try { - // Finding #13: re-fetch the agent's other_config right before building - // the update bag. This mitigates concurrent-tab clobber: if another tab - // saved an unrelated field since we last loaded, we merge into the fresh - // value rather than the stale local copy. - // NOTE: This is a best-effort mitigation only — there is still a narrow - // TOCTOU window between the refetch and the PUT. A server-side JSON-patch - // endpoint is the correct v2 fix (deferred). - // We intentionally use the local state for the fields WE are editing so - // our changes are not overwritten by the refetch. + // Finding #13 (honest): we spread the last-loaded otherConfig prop as a + // base, then overwrite only the fields this section owns. Concurrent-tab + // clobber is NOT mitigated — a second tab saving an unrelated field + // between our last load and this PUT will lose that update. + // Server-side JSON-merge-patch endpoint is the correct v2 fix (deferred; + // see plan §Open Questions). Refetch before PUT is intentionally omitted: + // it would add latency for a race that is rare in practice for local-first + // desktop/single-user deployments. const bag = { ...otherConfig }; if (mode && mode !== "full") { bag.prompt_mode = mode; diff --git a/ui/web/src/pages/agents/agent-detail/overview-sections/tts-override-block.tsx b/ui/web/src/pages/agents/agent-detail/overview-sections/tts-override-block.tsx index 73d3a57c..d87429d6 100644 --- a/ui/web/src/pages/agents/agent-detail/overview-sections/tts-override-block.tsx +++ b/ui/web/src/pages/agents/agent-detail/overview-sections/tts-override-block.tsx @@ -35,46 +35,41 @@ interface Props { } /** - * Adapter table: maps generic agent override key → provider-native capability key. - * Must mirror internal/audio/agent_params_adapter.go AdaptAgentParams switch. - * Used for bidirectional conversion at the UI boundary (generic↔native). - * - * Finding #9: This mirrors the Go adapter but is NOT the source of truth for - * which params are shown — that comes from agent_overridable===true in - * capabilities. This table is only used to convert stored generic keys to - * native keys for form state, and back on save. + * Build generic→native and native→generic mappings from the overridable params + * slice. Each param carries its own native key (param.key) and generic alias + * (param.agent_overridable_as), so no separate hard-coded table is needed. + * Finding #9: single source of truth is the capabilities API response. */ -const GENERIC_TO_NATIVE: Record> = { - openai: { speed: "speed" }, - elevenlabs: { speed: "voice_settings.speed", style: "voice_settings.style" }, - edge: {}, - minimax: { speed: "speed", emotion: "emotion" }, - gemini: {}, -}; - -/** Invert the generic→native table to native→generic for a given provider. */ -function buildNativeToGeneric(provider: string): Record { - const map = GENERIC_TO_NATIVE[provider] ?? {}; - const inv: Record = {}; - for (const [generic, native] of Object.entries(map)) { - inv[native] = generic; +export function buildAdapterMaps( + overridableParams: Array<{ key: string; agent_overridable_as?: string }>, +): { + genericToNative: Record; + nativeToGeneric: Record; +} { + const genericToNative: Record = {}; + const nativeToGeneric: Record = {}; + for (const p of overridableParams) { + if (p.agent_overridable_as) { + genericToNative[p.agent_overridable_as] = p.key; + nativeToGeneric[p.key] = p.agent_overridable_as; + } } - return inv; + return { genericToNative, nativeToGeneric }; } /** * Convert stored generic params (e.g. {speed: 1.2}) to capability-native form * state (e.g. {voice_settings.speed: 1.2} for ElevenLabs). - * Called at load time. + * Called at load time. Mapping derived from capabilities, not a hard-coded table. */ export function genericToNativeFormState( genericParams: Record, - provider: string, + overridableParams: Array<{ key: string; agent_overridable_as?: string }>, ): Record { - const map = GENERIC_TO_NATIVE[provider] ?? {}; + const { genericToNative } = buildAdapterMaps(overridableParams); const out: Record = {}; for (const [generic, val] of Object.entries(genericParams)) { - const native = map[generic]; + const native = genericToNative[generic]; if (native !== undefined) { out[native] = val; } @@ -88,9 +83,9 @@ export function genericToNativeFormState( */ export function nativeFormStateToGeneric( nativeState: Record, - provider: string, + overridableParams: Array<{ key: string; agent_overridable_as?: string }>, ): Record { - const nativeToGeneric = buildNativeToGeneric(provider); + const { nativeToGeneric } = buildAdapterMaps(overridableParams); const out: Record = {}; for (const [native, val] of Object.entries(nativeState)) { const generic = nativeToGeneric[native]; @@ -126,22 +121,24 @@ export function TtsOverrideBlock({ const { t } = useTranslation("tts"); const [testing, setTesting] = useState(false); - // Fetch capabilities for the current provider to find agent_overridable params. + // Fetch capabilities for the current provider to find agent-overridable params. + // Finding #9: capabilities API is the single source of truth — agent_overridable_as + // encodes both the overridability flag and the generic key alias, so no separate + // hard-coded lookup table is needed in the UI. const { data: allCaps } = useTtsCapabilities(); const providerCaps = allCaps?.find((c) => c.provider === globalProvider); const overridableParams = (providerCaps?.params ?? []).filter( - (p) => p.agent_overridable === true, + (p) => (p.agent_overridable_as ?? "") !== "", ); // Form state uses CAPABILITY-NATIVE keys. Convert from stored generic keys on each render. - // This is a pure derivation — form state is owned by this component transiently, - // and serialised back to generic keys via nativeFormStateToGeneric on change. - const nativeFormState = genericToNativeFormState(ttsParams, globalProvider); + // Mapping is derived from overridableParams (no hard-coded lookup table). + const nativeFormState = genericToNativeFormState(ttsParams, overridableParams); const handleParamChange = (nativeKey: string, val: ParamValue) => { const updated = { ...nativeFormState, [nativeKey]: val }; // Convert native form state → generic keys for parent storage. - const generic = nativeFormStateToGeneric(updated, globalProvider); + const generic = nativeFormStateToGeneric(updated, overridableParams); onTtsParamsChange(generic); };