From c918ee3682f462daf0514738472b1977d37beaa9 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Thu, 2 Apr 2026 08:52:16 +0300 Subject: Align processor attribution with provider semantics --- internal/audio/gemini_provider.go | 28 +--------- internal/audio/openai_provider.go | 18 +------ internal/audio/sidecar.go | 91 +++++++++++++++++++++++++++++++++ internal/audio/sidecar_test.go | 99 ++++++++++++++++++++++++++++++++++++ internal/processor/processor.go | 10 ++-- internal/processor/processor_test.go | 93 +++++++++++++++++++++++++++++---- 6 files changed, 283 insertions(+), 56 deletions(-) (limited to 'internal') diff --git a/internal/audio/gemini_provider.go b/internal/audio/gemini_provider.go index 9900005..1d82f2d 100644 --- a/internal/audio/gemini_provider.go +++ b/internal/audio/gemini_provider.go @@ -105,23 +105,10 @@ func (p *GeminiProvider) buildPrompt(text string) string { } var prompt strings.Builder - prompt.WriteString("You are speaking Bulgarian language (български език). ") - prompt.WriteString("Pronounce the Bulgarian text with authentic Bulgarian phonetics, not Russian.") - - if speedHint := geminiSpeedHint(config.GeminiSpeed); speedHint != "" { - prompt.WriteString(" ") - prompt.WriteString(speedHint) - } - - prompt.WriteString("\n\nSpeak the following Bulgarian text:\n") + prompt.WriteString(geminiPromptInstruction(config)) + prompt.WriteString("\n") prompt.WriteString(strings.TrimSpace(text)) - if voice := strings.TrimSpace(config.GeminiVoice); voice != "" { - prompt.WriteString("\n\nUse a clear, natural delivery that matches the voice named ") - prompt.WriteString(voice) - prompt.WriteString(".") - } - return prompt.String() } @@ -166,17 +153,6 @@ func normalizeGeminiConfig(config *Config) *Config { return normalized } -func geminiSpeedHint(speed float64) string { - switch { - case speed < 0.95: - return "Speak slowly and clearly for language learners." - case speed > 1.05: - return "Speak slightly faster than normal while staying clear." - default: - return "Speak at a natural pace." - } -} - func extractAudioData(response *genai.GenerateContentResponse) ([]byte, string, error) { if response == nil { return nil, "", errors.New("no response from Gemini") diff --git a/internal/audio/openai_provider.go b/internal/audio/openai_provider.go index ca7d418..1700baf 100644 --- a/internal/audio/openai_provider.go +++ b/internal/audio/openai_provider.go @@ -146,21 +146,5 @@ func (p *OpenAIProvider) IsAvailable() error { // preprocessBulgarianText prepares Bulgarian text for clearer TTS pronunciation func (p *OpenAIProvider) preprocessBulgarianText(text string) string { - // First, clean the text and remove punctuation that shouldn't be spoken - cleanedText := strings.TrimSpace(text) - - // Remove common punctuation marks that shouldn't be pronounced - punctuationToRemove := []string{"!", "?", ".", ",", ";", ":", "\"", "'", "(", ")", "[", "]", "{", "}", "-", "—", "–"} - for _, punct := range punctuationToRemove { - cleanedText = strings.ReplaceAll(cleanedText, punct, "") - } - - // Trim any remaining whitespace - cleanedText = strings.TrimSpace(cleanedText) - - // For single words, we add subtle punctuation to create natural pauses - // This helps the TTS engine pronounce it more carefully - processedText := cleanedText // fmt.Sprintf("%s...", cleanedText) - - return processedText + return openAIProcessedText(text) } diff --git a/internal/audio/sidecar.go b/internal/audio/sidecar.go index ee591f0..be9388b 100644 --- a/internal/audio/sidecar.go +++ b/internal/audio/sidecar.go @@ -35,6 +35,97 @@ func ProcessedTextForWord(text string) string { return fmt.Sprintf("%s...", strings.TrimSpace(cleanedText)) } +// ProcessedTextForProvider returns the exact text shape the provider path sends to TTS. +func ProcessedTextForProvider(provider, text string) string { + switch strings.ToLower(strings.TrimSpace(provider)) { + case "openai": + return openAIProcessedText(text) + case "gemini": + return strings.TrimSpace(text) + default: + return strings.TrimSpace(text) + } +} + +// InstructionForProvider returns the provider-specific instruction semantics written to attribution files. +func InstructionForProvider(provider string, config *Config) string { + switch strings.ToLower(strings.TrimSpace(provider)) { + case "openai": + return openAIInstructionForAttribution(config) + case "gemini": + return geminiPromptInstruction(config) + default: + return "" + } +} + +func openAIProcessedText(text string) string { + cleanedText := strings.TrimSpace(text) + punctuationToRemove := []string{"!", "?", ".", ",", ";", ":", "\"", "'", "(", ")", "[", "]", "{", "}", "-", "—", "–"} + for _, punct := range punctuationToRemove { + cleanedText = strings.ReplaceAll(cleanedText, punct, "") + } + + return strings.TrimSpace(cleanedText) +} + +func openAIInstructionForAttribution(config *Config) string { + if config == nil { + return "" + } + + if !openAIModelUsesInstructions(config.OpenAIModel) { + return "" + } + + return strings.TrimSpace(config.OpenAIInstruction) +} + +func openAIModelUsesInstructions(model string) bool { + switch strings.TrimSpace(model) { + case "gpt-4o-mini-tts", "gpt-4o-mini-audio-preview": + return true + default: + return false + } +} + +func geminiPromptInstruction(config *Config) string { + if config == nil { + config = &Config{} + } + + var prompt strings.Builder + prompt.WriteString("You are speaking Bulgarian language (български език). ") + prompt.WriteString("Pronounce the Bulgarian text with authentic Bulgarian phonetics, not Russian.") + + if speedHint := geminiSpeedHint(config.GeminiSpeed); speedHint != "" { + prompt.WriteString(" ") + prompt.WriteString(speedHint) + } + + prompt.WriteString("\n\nSpeak the following Bulgarian text:") + + if voice := strings.TrimSpace(config.GeminiVoice); voice != "" { + prompt.WriteString("\n\nUse a clear, natural delivery that matches the voice named ") + prompt.WriteString(voice) + prompt.WriteString(".") + } + + return prompt.String() +} + +func geminiSpeedHint(speed float64) string { + switch { + case speed < 0.95: + return "Speak slowly and clearly for language learners." + case speed > 1.05: + return "Speak slightly faster than normal while staying clear." + default: + return "Speak at a natural pace." + } +} + // BuildSidecarMetadata formats the audio metadata written for GUI reloads and batch exports. func BuildSidecarMetadata(params SidecarMetadataParams) string { var b strings.Builder diff --git a/internal/audio/sidecar_test.go b/internal/audio/sidecar_test.go index 63eef6e..b832e1d 100644 --- a/internal/audio/sidecar_test.go +++ b/internal/audio/sidecar_test.go @@ -12,6 +12,105 @@ func TestProcessedTextForWord(t *testing.T) { } } +func TestProcessedTextForProvider(t *testing.T) { + tests := []struct { + name string + provider string + input string + want string + }{ + { + name: "openai strips punctuation", + provider: "openai", + input: " ябълка!? ", + want: "ябълка", + }, + { + name: "gemini keeps punctuation", + provider: "gemini", + input: " ябълка!? ", + want: "ябълка!?", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ProcessedTextForProvider(tt.provider, tt.input); got != tt.want { + t.Fatalf("ProcessedTextForProvider(%q, %q) = %q, want %q", tt.provider, tt.input, got, tt.want) + } + }) + } +} + +func TestInstructionForProvider(t *testing.T) { + tests := []struct { + name string + provider string + config *Config + want []string + wantNot []string + }{ + { + name: "openai model supports instructions", + provider: "openai", + config: &Config{ + OpenAIModel: "gpt-4o-mini-tts", + OpenAIInstruction: "Speak clearly.", + }, + want: []string{"Speak clearly."}, + }, + { + name: "openai unsupported model omits instructions", + provider: "openai", + config: &Config{ + OpenAIModel: "tts-1", + OpenAIInstruction: "Speak clearly.", + }, + want: []string{""}, + wantNot: []string{"Speak clearly."}, + }, + { + name: "gemini default voice and speed semantics", + provider: "gemini", + config: &Config{ + GeminiSpeed: 1.0, + }, + want: []string{"Speak at a natural pace.", "Speak the following Bulgarian text:"}, + }, + { + name: "gemini explicit voice semantics", + provider: "gemini", + config: &Config{ + GeminiSpeed: 0.9, + GeminiVoice: "Kore", + }, + want: []string{"Speak slowly and clearly for language learners.", "voice named Kore."}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := InstructionForProvider(tt.provider, tt.config) + for _, want := range tt.want { + if want == "" { + if got != "" { + t.Fatalf("InstructionForProvider(%q, %+v) = %q, want empty", tt.provider, tt.config, got) + } + continue + } + if !strings.Contains(got, want) { + t.Fatalf("InstructionForProvider(%q, %+v) = %q, missing %q", tt.provider, tt.config, got, want) + } + } + for _, want := range tt.wantNot { + if strings.Contains(got, want) { + t.Fatalf("InstructionForProvider(%q, %+v) = %q, unexpectedly contained %q", tt.provider, tt.config, got, want) + } + } + }) + } +} + func TestBuildSidecarMetadata(t *testing.T) { tests := []struct { name string diff --git a/internal/processor/processor.go b/internal/processor/processor.go index 3c07806..036fa79 100644 --- a/internal/processor/processor.go +++ b/internal/processor/processor.go @@ -486,7 +486,7 @@ func (p *Processor) generateAudioWithVoiceAndFilenameInDir(word, voice, filename } // Save audio attribution - if err := p.saveAudioAttribution(word, outputFile, providerConfig, word); err != nil { + if err := p.saveAudioAttribution(word, outputFile, providerConfig); err != nil { fmt.Printf(" Warning: Failed to save audio attribution: %v\n", err) } @@ -987,7 +987,10 @@ func (p *Processor) isWordFullyProcessed(word string) bool { } return true // All required files exist } -func (p *Processor) saveAudioAttribution(word, audioFile string, config *audio.Config, processedText string) error { +func (p *Processor) saveAudioAttribution(word, audioFile string, config *audio.Config) error { + processedText := audio.ProcessedTextForProvider(config.Provider, word) + instruction := audio.InstructionForProvider(config.Provider, config) + var attribution string switch strings.ToLower(strings.TrimSpace(config.Provider)) { case "gemini": @@ -996,6 +999,7 @@ func (p *Processor) saveAudioAttribution(word, audioFile string, config *audio.C Model: config.GeminiTTSModel, Voice: config.GeminiVoice, Speed: config.GeminiSpeed, + Instruction: instruction, ProcessedText: processedText, GeneratedAt: time.Now(), }) @@ -1005,7 +1009,7 @@ func (p *Processor) saveAudioAttribution(word, audioFile string, config *audio.C Model: config.OpenAIModel, Voice: config.OpenAIVoice, Speed: config.OpenAISpeed, - Instruction: config.OpenAIInstruction, + Instruction: instruction, ProcessedText: processedText, GeneratedAt: time.Now(), }) diff --git a/internal/processor/processor_test.go b/internal/processor/processor_test.go index 4d9f3ad..d6326f6 100644 --- a/internal/processor/processor_test.go +++ b/internal/processor/processor_test.go @@ -520,7 +520,7 @@ func TestGenerateAudioUsesConfiguredGeminiVoiceAndModel(t *testing.T) { flags.AudioFormat = "mp3" p := NewProcessor(flags) - if err := p.generateAudio("ябълка"); err != nil { + if err := p.generateAudio("ябълка!?"); err != nil { t.Fatalf("generateAudio() unexpected error: %v", err) } @@ -546,7 +546,7 @@ func TestGenerateAudioUsesConfiguredGeminiVoiceAndModel(t *testing.T) { t.Fatalf("GenerateAudio() output file = %q, want wav output", fakeProvider.lastOutputFile) } - wordDir := p.findCardDirectory("ябълка") + wordDir := p.findCardDirectory("ябълка!?") if wordDir == "" { t.Fatal("expected generated word directory") } @@ -575,9 +575,15 @@ func TestGenerateAudioUsesConfiguredGeminiVoiceAndModel(t *testing.T) { t.Fatalf("expected attribution file %q: %v", attrPath, err) } attribution := string(attributionData) - if !strings.Contains(attribution, "Processed text sent to TTS: ябълка") { + if !strings.Contains(attribution, "Processed text sent to TTS: ябълка!?") { t.Fatalf("gemini attribution missing exact processed text: %q", attribution) } + if !strings.Contains(attribution, "Speak at a natural pace.") { + t.Fatalf("gemini attribution missing speed hint semantics: %q", attribution) + } + if !strings.Contains(attribution, "voice named Kore") { + t.Fatalf("gemini attribution missing voice semantics: %q", attribution) + } } func TestGenerateAudioUsesGeminiModelDefaultWhenVoiceNotSet(t *testing.T) { @@ -608,7 +614,7 @@ func TestGenerateAudioUsesGeminiModelDefaultWhenVoiceNotSet(t *testing.T) { flags.AudioProvider = "gemini" p := NewProcessor(flags) - if err := p.generateAudio("ябълка"); err != nil { + if err := p.generateAudio("ябълка!?"); err != nil { t.Fatalf("generateAudio() unexpected error: %v", err) } @@ -625,7 +631,7 @@ func TestGenerateAudioUsesGeminiModelDefaultWhenVoiceNotSet(t *testing.T) { t.Fatalf("GenerateAudio() output file = %q, want wav output", fakeProvider.lastOutputFile) } - wordDir := p.findCardDirectory("ябълка") + wordDir := p.findCardDirectory("ябълка!?") if wordDir == "" { t.Fatal("expected generated word directory") } @@ -651,9 +657,12 @@ func TestGenerateAudioUsesGeminiModelDefaultWhenVoiceNotSet(t *testing.T) { t.Fatalf("expected attribution file %q: %v", attrPath, err) } attribution := string(attributionData) - if !strings.Contains(attribution, "Processed text sent to TTS: ябълка") { + if !strings.Contains(attribution, "Processed text sent to TTS: ябълка!?") { t.Fatalf("gemini attribution missing exact processed text: %q", attribution) } + if !strings.Contains(attribution, "Speak at a natural pace.") { + t.Fatalf("gemini attribution missing speed hint semantics: %q", attribution) + } } func TestGenerateAudioBgBgUsesGeminiModelDefaultWhenVoiceNotSet(t *testing.T) { @@ -684,7 +693,7 @@ func TestGenerateAudioBgBgUsesGeminiModelDefaultWhenVoiceNotSet(t *testing.T) { flags.AudioProvider = "gemini" p := NewProcessor(flags) - if err := p.generateAudioBgBg("ябълка", "круша"); err != nil { + if err := p.generateAudioBgBg("ябълка!?", "круша."); err != nil { t.Fatalf("generateAudioBgBg() unexpected error: %v", err) } @@ -706,10 +715,13 @@ func TestGenerateAudioBgBgUsesGeminiModelDefaultWhenVoiceNotSet(t *testing.T) { t.Fatalf("expected attribution file %q: %v", attrPath, err) } attribution := string(attributionData) - wantText := []string{"ябълка", "круша"}[i] + wantText := []string{"ябълка!?", "круша."}[i] if !strings.Contains(attribution, "Processed text sent to TTS: "+wantText) { t.Fatalf("bg-bg attribution missing exact processed text %q: %q", wantText, attribution) } + if !strings.Contains(attribution, "Speak at a natural pace.") { + t.Fatalf("bg-bg attribution missing speed hint semantics: %q", attribution) + } } } @@ -742,8 +754,8 @@ func TestGenerateAudioUsesConfiguredAudioFormatWhenOpenAIConfigIsSetOnly(t *test flags.AudioFormat = "wav" p := NewProcessor(flags) - wordDir := p.findOrCreateWordDirectory("ябълка") - if err := p.generateAudioWithVoiceAndFilenameInDir("ябълка", "alloy", "audio", wordDir); err != nil { + wordDir := p.findOrCreateWordDirectory("ябълка!?") + if err := p.generateAudioWithVoiceAndFilenameInDir("ябълка!?", "alloy", "audio", wordDir); err != nil { t.Fatalf("generateAudioWithVoiceAndFilenameInDir() unexpected error: %v", err) } @@ -786,6 +798,9 @@ func TestGenerateAudioUsesConfiguredAudioFormatWhenOpenAIConfigIsSetOnly(t *test if !strings.Contains(attribution, "Processed text sent to TTS: ябълка") { t.Fatalf("openai attribution missing exact processed text: %q", attribution) } + if strings.Contains(attribution, "Voice instructions:") { + t.Fatalf("openai attribution unexpectedly recorded instructions: %q", attribution) + } } func TestGenerateAudioUsesConfiguredOpenAIVoiceFromConfig(t *testing.T) { @@ -863,6 +878,64 @@ func TestGenerateAudioUsesConfiguredOpenAIVoiceFromConfig(t *testing.T) { } } +func TestGenerateAudioOmitsOpenAIInstructionsForUnsupportedModel(t *testing.T) { + originalFactory := newAudioProvider + t.Cleanup(func() { + newAudioProvider = originalFactory + }) + + fakeProvider := &fakeAudioProvider{} + var capturedConfig *audio.Config + newAudioProvider = func(config *audio.Config) (audio.Provider, error) { + copyConfig := *config + capturedConfig = ©Config + return fakeProvider, nil + } + + originalConfig := viper.New() + *originalConfig = *viper.GetViper() + defer func() { + *viper.GetViper() = *originalConfig + }() + viper.Reset() + viper.Set("audio.provider", "openai") + viper.Set("audio.openai_instruction", "Speak clearly.") + + flags := cli.NewFlags() + flags.OutputDir = t.TempDir() + flags.AudioProvider = "openai" + flags.AudioFormat = "mp3" + flags.OpenAIModel = "tts-1" + + p := NewProcessor(flags) + if err := p.generateAudio("ябълка!?"); err != nil { + t.Fatalf("generateAudio() unexpected error: %v", err) + } + + if capturedConfig == nil { + t.Fatal("expected audio provider config to be captured") + } + if capturedConfig.OpenAIModel != "tts-1" { + t.Fatalf("captured OpenAIModel = %q, want %q", capturedConfig.OpenAIModel, "tts-1") + } + if capturedConfig.OpenAIInstruction != "Speak clearly." { + t.Fatalf("captured OpenAIInstruction = %q, want %q", capturedConfig.OpenAIInstruction, "Speak clearly.") + } + + attrPath := audio.AttributionPath(fakeProvider.lastOutputFile) + attributionData, err := os.ReadFile(attrPath) + if err != nil { + t.Fatalf("expected attribution file %q: %v", attrPath, err) + } + attribution := string(attributionData) + if strings.Contains(attribution, "Voice instructions:") { + t.Fatalf("openai attribution unexpectedly recorded unsupported instructions: %q", attribution) + } + if !strings.Contains(attribution, "Processed text sent to TTS: ябълка") { + t.Fatalf("openai attribution missing cleaned processed text: %q", attribution) + } +} + func TestGenerateAnkiFileUsesEffectiveAudioFormatForGemini(t *testing.T) { originalConfig := viper.New() *originalConfig = *viper.GetViper() -- cgit v1.2.3