From 74383d0e31fee5aebec002971ebd2cbddad32ac7 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Thu, 2 Apr 2026 21:59:36 +0300 Subject: task 009/00i: remove internal translation from image clients; consolidate provider normalization - task 009: remove translateBulgarianToEnglish from OpenAIClient and NanoBananaClient. Image clients no longer make internal translation API calls; callers must supply SearchOptions.Translation (processor and GUI already do). When translation is absent, opts.Query (the Bulgarian word) is used as fallback subject so image generation still proceeds. Update tests to pass translation via SearchOptions rather than relying on the removed fallback. - task 00i: consolidate duplicate ProviderGemini/ProviderOpenAI constants and normalizeProvider() functions from internal/translation and internal/phonetic into a shared config.NormalizeProvider(string) string in internal/config/provider.go. Both packages now delegate to the shared function so the normalization rule (lowercase, trim, default "gemini") has one authoritative home. Co-Authored-By: Claude Sonnet 4.6 --- internal/config/provider.go | 24 ++++++++++++++++ internal/image/nanobanana.go | 37 ++++++------------------- internal/image/nanobanana_test.go | 49 +++++++++++++-------------------- internal/image/openai.go | 56 +++++++------------------------------- internal/phonetic/fetcher.go | 11 ++++---- internal/translation/translator.go | 15 ++++------ 6 files changed, 72 insertions(+), 120 deletions(-) create mode 100644 internal/config/provider.go diff --git a/internal/config/provider.go b/internal/config/provider.go new file mode 100644 index 0000000..7044964 --- /dev/null +++ b/internal/config/provider.go @@ -0,0 +1,24 @@ +package config + +import "strings" + +const ( + // ProviderGemini is the canonical name for the Google Gemini provider. + // It is the default across all subsystems (audio, translation, phonetic). + ProviderGemini = "gemini" + + // ProviderOpenAI is the canonical name for the OpenAI provider. + ProviderOpenAI = "openai" +) + +// NormalizeProvider returns a canonical, lowercase provider name. +// An empty input resolves to ProviderGemini (the default). The function is +// shared by the audio, translation, and phonetic packages so the same +// normalization rule has a single authoritative home. +func NormalizeProvider(provider string) string { + normalized := strings.ToLower(strings.TrimSpace(provider)) + if normalized == "" { + return ProviderGemini + } + return normalized +} diff --git a/internal/image/nanobanana.go b/internal/image/nanobanana.go index 1f3ac57..1a3d6ad 100644 --- a/internal/image/nanobanana.go +++ b/internal/image/nanobanana.go @@ -242,19 +242,20 @@ func (c *NanoBananaClient) ensureReady() error { return nil } -func (c *NanoBananaClient) resolveTranslation(ctx context.Context, opts *SearchOptions, translation string) (string, error) { +// resolveTranslation returns the English translation for the Bulgarian query. +// Translating internally would couple the image package to the Gemini text API for a +// concern that belongs in the translation package. Callers (processor, GUI) already +// resolve the English translation before calling Search, so we simply use what was +// provided and fall back to the original query word when nothing was given. +func (c *NanoBananaClient) resolveTranslation(_ context.Context, opts *SearchOptions, translation string) (string, error) { if translation != "" { fmt.Printf("Using provided translation: %s -> %s\n", opts.Query, translation) return translation, nil } - translation, err := c.translateBulgarianToEnglish(ctx, opts.Query) - if err != nil { - fmt.Printf("Translation failed: %v, using original word\n", err) - return opts.Query, nil - } - - return translation, nil + // No translation provided — fall back to the original query word so image + // generation still proceeds, albeit potentially with lower quality. + return opts.Query, nil } func (c *NanoBananaClient) resolvePrompt(ctx context.Context, opts *SearchOptions, translatedWord string) (string, error) { @@ -332,26 +333,6 @@ func (c *NanoBananaClient) createEducationalPrompt(ctx context.Context, bulgaria return buildEducationalPrompt(selectedStyle, scene, subject) } -func (c *NanoBananaClient) translateBulgarianToEnglish(ctx context.Context, word string) (string, error) { - fmt.Printf("Nano Banana Translation: Using model '%s' to translate '%s'\n", c.textModelName(), word) - - translation, err := nanoBananaGenerateText( - ctx, - c, - c.textModelName(), - "You are a Bulgarian language expert. Translate the Bulgarian word into English. Respond with only the English translation, nothing else.", - fmt.Sprintf("Translate the Bulgarian word '%s' to English. Respond with only the English translation, nothing else.", word), - 0.3, - 50, - ) - if err != nil { - return "", fmt.Errorf("translation failed: %w", err) - } - - fmt.Printf("Translated '%s' to '%s'\n", word, translation) - return translation, nil -} - func (c *NanoBananaClient) generateSceneDescription(ctx context.Context, bulgarianWord, englishTranslation string) (string, error) { fmt.Printf("Nano Banana Scene Generation: Creating scene for '%s' (%s)\n", bulgarianWord, englishTranslation) diff --git a/internal/image/nanobanana_test.go b/internal/image/nanobanana_test.go index 6d34b83..88cc43c 100644 --- a/internal/image/nanobanana_test.go +++ b/internal/image/nanobanana_test.go @@ -114,6 +114,9 @@ func TestNanoBananaClient_Search_CustomPromptSkipsTextGeneration(t *testing.T) { } } +// TestNanoBananaClient_Search_GeneratedPromptFlow verifies the full scene-generation +// prompt flow when the caller provides a pre-translated English word via +// SearchOptions.Translation (translation is now the caller's responsibility). func TestNanoBananaClient_Search_GeneratedPromptFlow(t *testing.T) { originalText := nanoBananaGenerateText originalImage := nanoBananaGenerateImage @@ -126,23 +129,13 @@ func TestNanoBananaClient_Search_GeneratedPromptFlow(t *testing.T) { ArtisticStyles = []string{"Photorealism"} - var translationCalls int var sceneCalls int var gotPrompt string var callbackPrompt string + // Only scene generation is expected; internal translation has been removed. nanoBananaGenerateText = func(_ context.Context, _ *NanoBananaClient, _, systemPrompt, userPrompt string, temperature float32, maxOutputTokens int32) (string, error) { - switch { - case strings.Contains(systemPrompt, "Bulgarian language expert"): - translationCalls++ - if temperature != 0.3 || maxOutputTokens != 50 { - t.Fatalf("translation params = %v/%d, want 0.3/50", temperature, maxOutputTokens) - } - if !strings.Contains(userPrompt, "ябълка") { - t.Fatalf("translation prompt = %q, want Bulgarian query", userPrompt) - } - return "apple", nil - case strings.Contains(systemPrompt, "educational flashcards for language learning"): + if strings.Contains(systemPrompt, "educational flashcards for language learning") { sceneCalls++ if temperature != 0.7 || maxOutputTokens != 100 { t.Fatalf("scene params = %v/%d, want 0.7/100", temperature, maxOutputTokens) @@ -151,10 +144,9 @@ func TestNanoBananaClient_Search_GeneratedPromptFlow(t *testing.T) { t.Fatalf("scene prompt = %q, want English translation", userPrompt) } return "A bright apple sits centered on a wooden table.", nil - default: - t.Fatalf("unexpected system prompt: %q", systemPrompt) - return "", nil } + t.Fatalf("unexpected system prompt: %q", systemPrompt) + return "", nil } nanoBananaGenerateImage = func(_ context.Context, _ *NanoBananaClient, prompt string) ([]byte, string, error) { @@ -169,13 +161,13 @@ func TestNanoBananaClient_Search_GeneratedPromptFlow(t *testing.T) { callbackPrompt = prompt }) - results, err := client.Search(context.Background(), DefaultSearchOptions("ябълка")) + results, err := client.Search(context.Background(), &SearchOptions{ + Query: "ябълка", + Translation: "apple", // caller provides the English translation + }) if err != nil { t.Fatalf("Search() unexpected error: %v", err) } - if translationCalls != 1 { - t.Fatalf("translationCalls = %d, want 1", translationCalls) - } if sceneCalls != 1 { t.Fatalf("sceneCalls = %d, want 1", sceneCalls) } @@ -338,22 +330,16 @@ func TestNanoBananaClient_Search_ImageGenerationError(t *testing.T) { ArtisticStyles = []string{"Photorealism"} + // Only scene generation is expected; internal translation has been removed. nanoBananaGenerateText = func(_ context.Context, _ *NanoBananaClient, _, systemPrompt, userPrompt string, _ float32, _ int32) (string, error) { - switch { - case strings.Contains(systemPrompt, "Bulgarian language expert"): - if !strings.Contains(userPrompt, "ябълка") { - t.Fatalf("translation prompt = %q, want Bulgarian query", userPrompt) - } - return "apple", nil - case strings.Contains(systemPrompt, "educational flashcards for language learning"): + if strings.Contains(systemPrompt, "educational flashcards for language learning") { if !strings.Contains(userPrompt, "apple") { t.Fatalf("scene prompt = %q, want translated word", userPrompt) } return "A bright apple sits centered on a wooden table.", nil - default: - t.Fatalf("unexpected system prompt: %q", systemPrompt) - return "", nil } + t.Fatalf("unexpected system prompt: %q", systemPrompt) + return "", nil } nanoBananaGenerateImage = func(_ context.Context, _ *NanoBananaClient, _ string) ([]byte, string, error) { @@ -361,7 +347,10 @@ func TestNanoBananaClient_Search_ImageGenerationError(t *testing.T) { } client := NewNanoBananaClient(&NanoBananaConfig{APIKey: "test-key"}) - _, err := client.Search(context.Background(), DefaultSearchOptions("ябълка")) + _, err := client.Search(context.Background(), &SearchOptions{ + Query: "ябълка", + Translation: "apple", // caller provides the English translation + }) if err == nil { t.Fatal("expected image generation error") } diff --git a/internal/image/openai.go b/internal/image/openai.go index fdd88e4..8623fcd 100644 --- a/internal/image/openai.go +++ b/internal/image/openai.go @@ -89,21 +89,17 @@ func (c *OpenAIClient) Search(ctx context.Context, opts *SearchOptions) ([]Searc } } - // Use provided translation if available, otherwise translate Bulgarian word to English - var translatedWord string - if opts.Translation != "" { - // Use the translation that was already provided (from UI or user input) - translatedWord = opts.Translation - fmt.Printf("Using provided translation: %s -> %s\n", opts.Query, translatedWord) + // Use the caller-provided translation. Translating internally would couple + // the image package to the OpenAI chat API for a concern that belongs in + // the translation package. Callers (processor, GUI) already resolve the + // English translation before calling Search. + translatedWord := opts.Translation + if translatedWord == "" { + // No translation provided — fall back to the original query word so + // image generation still proceeds, albeit potentially with lower quality. + translatedWord = opts.Query } else { - // Translate Bulgarian word to English for better results - var err error - translatedWord, err = c.translateBulgarianToEnglish(ctx, opts.Query) - if err != nil { - // If translation fails, fall back to using the original word - fmt.Printf("Translation failed: %v, using original word\n", err) - translatedWord = opts.Query - } + fmt.Printf("Using provided translation: %s -> %s\n", opts.Query, translatedWord) } // Create prompt - use custom if provided, otherwise generate educational prompt @@ -274,38 +270,6 @@ func (c *OpenAIClient) createEducationalPrompt(ctx context.Context, bulgarianWor return buildEducationalPrompt(selectedStyle, scene, subject) } -// translateBulgarianToEnglish translates a Bulgarian word to English using OpenAI -func (c *OpenAIClient) translateBulgarianToEnglish(ctx context.Context, word string) (string, error) { - // Use OpenAI chat completion to translate - fmt.Printf("OpenAI Translation: Using model 'gpt-4o-mini' to translate '%s'\n", word) - - req := openai.ChatCompletionRequest{ - Model: openai.GPT4oMini, - Messages: []openai.ChatCompletionMessage{ - { - Role: openai.ChatMessageRoleUser, - Content: fmt.Sprintf("Translate the Bulgarian word '%s' to English. Respond with only the English translation, nothing else.", word), - }, - }, - Temperature: 0.3, // Lower temperature for more consistent translations - MaxTokens: 50, - } - - resp, err := c.client.CreateChatCompletion(ctx, req) - if err != nil { - return "", fmt.Errorf("translation failed: %w", err) - } - - if len(resp.Choices) == 0 || resp.Choices[0].Message.Content == "" { - return "", fmt.Errorf("no translation received") - } - - translation := strings.TrimSpace(resp.Choices[0].Message.Content) - fmt.Printf("Translated '%s' to '%s'\n", word, translation) - - return translation, nil -} - // generateSceneDescription generates a contextual scene description for the word func (c *OpenAIClient) generateSceneDescription(ctx context.Context, bulgarianWord, englishTranslation string) (string, error) { // Use OpenAI to generate a scene description diff --git a/internal/phonetic/fetcher.go b/internal/phonetic/fetcher.go index 0346213..d10d6c1 100644 --- a/internal/phonetic/fetcher.go +++ b/internal/phonetic/fetcher.go @@ -12,6 +12,8 @@ import ( "github.com/sashabaranov/go-openai" "google.golang.org/genai" + + appconfig "codeberg.org/snonux/totalrecall/internal/config" ) const ( @@ -265,11 +267,8 @@ func normalizeConfig(config *Config) Config { return normalized } +// normalizeProvider delegates to the shared config.NormalizeProvider so the +// normalization rule (lowercase, trim, default to "gemini") has one home. func normalizeProvider(provider Provider) Provider { - normalized := Provider(strings.ToLower(strings.TrimSpace(string(provider)))) - if normalized == "" { - return ProviderGemini - } - - return normalized + return Provider(appconfig.NormalizeProvider(string(provider))) } diff --git a/internal/translation/translator.go b/internal/translation/translator.go index 383779b..179e137 100644 --- a/internal/translation/translator.go +++ b/internal/translation/translator.go @@ -10,6 +10,8 @@ import ( "github.com/sashabaranov/go-openai" "google.golang.org/genai" + + appconfig "codeberg.org/snonux/totalrecall/internal/config" ) const ( @@ -264,15 +266,8 @@ func normalizeConfig(config *Config) *Config { return &normalized } +// normalizeProvider delegates to the shared config.NormalizeProvider so the +// normalization rule (lowercase, trim, default to "gemini") has one home. func normalizeProvider(provider Provider) Provider { - switch strings.ToLower(strings.TrimSpace(string(provider))) { - case "": - return ProviderGemini - case string(ProviderOpenAI): - return ProviderOpenAI - case string(ProviderGemini): - return ProviderGemini - default: - return Provider(strings.ToLower(strings.TrimSpace(string(provider)))) - } + return Provider(appconfig.NormalizeProvider(string(provider))) } -- cgit v1.2.3