diff options
| author | Paul Buetow <paul@buetow.org> | 2026-04-02 21:47:12 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-04-02 21:47:12 +0300 |
| commit | 993b2efe63e221cee550756770894c9c58474d25 (patch) | |
| tree | 126b0dad9e1225c575f2521f29c6ad68556f4056 | |
| parent | dac35c77721c97f093a44d98164b38534452de9f (diff) | |
task 00g/00k/00h/008: gofmt, remove ProviderWithFallback, stdlib helpers, shared prompt
- task 00g: fix gofmt violations (trailing whitespace, missing newlines,
indentation) in 8 files; all pass gofmt -l now
- task 00k: remove unused ProviderWithFallback and its tests (YAGNI — no
production caller existed; voice-level fallback via RunWithVoiceFallbacks
already covers the real use case)
- task 00h: replace private splitLines/trimSpace/isSpace helpers in
internal/batch/processor.go with strings.Split+ReplaceAll and
strings.TrimSpace from the stdlib; remove the now-redundant tests
- task 008: extract buildEducationalPrompt into internal/image/prompt.go so
the prompt-assembly policy (scene truncation cascade, char limit) lives in
one place; both OpenAIClient and NanoBananaClient delegate to it
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| -rw-r--r-- | go_best_practices_audit.md | 87 | ||||
| -rw-r--r-- | internal/anki/doc.go | 2 | ||||
| -rw-r--r-- | internal/audio/provider.go | 51 | ||||
| -rw-r--r-- | internal/audio/provider_test.go | 92 | ||||
| -rw-r--r-- | internal/audio/validate.go | 8 | ||||
| -rw-r--r-- | internal/batch/processor.go | 49 | ||||
| -rw-r--r-- | internal/batch/processor_test.go | 134 | ||||
| -rw-r--r-- | internal/config/doc.go | 2 | ||||
| -rw-r--r-- | internal/gui/icon.go | 2 | ||||
| -rw-r--r-- | internal/image/doc.go | 2 | ||||
| -rw-r--r-- | internal/image/nanobanana.go | 62 | ||||
| -rw-r--r-- | internal/image/openai.go | 74 | ||||
| -rw-r--r-- | internal/image/prompt.go | 73 | ||||
| -rw-r--r-- | internal/image/search.go | 1 | ||||
| -rw-r--r-- | internal/image/search_test.go | 36 | ||||
| -rw-r--r-- | internal/utils.go | 12 | ||||
| -rw-r--r-- | lint-output.txt | 14 |
17 files changed, 226 insertions, 475 deletions
diff --git a/go_best_practices_audit.md b/go_best_practices_audit.md new file mode 100644 index 0000000..d27ee7b --- /dev/null +++ b/go_best_practices_audit.md @@ -0,0 +1,87 @@ +# Go Best Practices Audit Report + +## Overview +This report summarizes the findings from a Go best practices audit of the totalrecall project conducted on 2026-03-22. The audit focused on project structure, style, conventions, and potential improvements based on Go best practices. + +## Findings + +### 1. Style Issues (Low Severity) +Several minor style issues were identified by staticcheck that can be improved for code clarity and consistency. + +#### a. Use tagged switch instead of if-else chain +- **File**: `internal/gui/app.go:529` +- **Description**: The code uses an if-else chain to check the `translationDirection` string variable. Since there are only two possible values ("en-to-bg" and "bg-to-en"), a tagged switch statement would be more appropriate and readable. +- **Recommendation**: Replace the if-else chain with a switch statement on `translationDirection`. +- **Example**: + ```go + switch translationDirection { + case "en-to-bg": + // handle English to Bulgarian translation + case "bg-to-en": + // handle Bulgarian to English translation + } + ``` + +#### b. Unnecessary fmt.Sprintf on string arguments +- **Files**: + - `internal/gui/app.go:2730` + - `internal/phonetic/fetcher.go:46` + - `internal/image/openai.go:211` +- **Description**: In several places, `fmt.Sprintf("%s", arg)` is used where `arg` is already a string. This is unnecessary and less efficient than direct assignment. +- **Recommendation**: Replace `fmt.Sprintf("%s", arg)` with just `arg`. +- **Examples**: + ```go + // Before + Content: fmt.Sprintf("%s", word), + // After + Content: word, + + // Before + attribution := fmt.Sprintf("Image generated by OpenAI DALL-E\n\n") + // After + attribution := "Image generated by OpenAI DALL-E\n\n" + ``` + +### 2. Project Structure (Good Practices) +The project follows recommended Go project structure conventions: +- **cmd/**: Contains the main application (`cmd/totalrecall/main.go`) +- **internal/**: Contains private application code, properly organized by functionality +- **No pkg/ directory**: Appropriate for an application (as opposed to a library) +- **assets/**: Contains non-Go resources (icons, configuration examples, etc.) + +### 3. Dependency Management (Good Practices) +- Uses Go modules correctly with a `go.mod` file +- Dependencies are properly versioned +- Standard library and popular third-party packages are used appropriately + +### 4. Error Handling (Good Practices) +- Errors are properly checked and handled throughout the codebase +- Error wrapping with `%w` is used where appropriate +- Context propagation is observed in API calls + +### 5. Testing (Good Practices) +- Comprehensive test suite exists for internal packages +- Tests cover various functions and edge cases +- Mocking is used where appropriate to avoid external API calls in unit tests + +### 6. Configuration Management (Good Practices) +- Uses Viper for configuration management +- Supports configuration files, environment variables, and command-line flags +- Default values are provided for configuration options + +## Conclusion +The totalrecall project demonstrates good adherence to Go best practices overall. The codebase is well-structured, follows conventional Go project layout, and implements proper error handling and testing. + +The identified issues are primarily minor style improvements that would enhance code readability and maintainability but do not affect correctness or security. Addressing these staticcheck suggestions would bring the code in line with idiomatic Go practices. + +## Recommendations +1. Address the four staticcheck issues mentioned above to improve code quality. +2. Consider adding more detailed comments to complex functions, particularly in the processor and GUI packages. +3. Ensure that all public functions have appropriate godoc comments. +4. Continue to maintain the existing testing practices as the project evolves. + +## Audit Details +- **Audit Date**: 2026-03-22 +- **Auditor**: Coding Assistant +- **Tools Used**: golangci-lint (v2.11.3), go vet, manual code review +- **Scope**: All Go files in the repository
\ No newline at end of file diff --git a/internal/anki/doc.go b/internal/anki/doc.go index 5fd3d5f..196bb26 100644 --- a/internal/anki/doc.go +++ b/internal/anki/doc.go @@ -1,3 +1,3 @@ // Package anki provides functionality to generate Anki-compatible // flashcard formats from Bulgarian words, audio, and images. -package anki
\ No newline at end of file +package anki diff --git a/internal/audio/provider.go b/internal/audio/provider.go index 31b1180..0e863bf 100644 --- a/internal/audio/provider.go +++ b/internal/audio/provider.go @@ -75,54 +75,3 @@ func NewProvider(config *Config) (Provider, error) { } } -// Compile-time check that ProviderWithFallback implements the Provider interface. -var _ Provider = (*ProviderWithFallback)(nil) - -// ProviderWithFallback wraps a primary provider with a fallback option -type ProviderWithFallback struct { - primary Provider - fallback Provider -} - -// NewProviderWithFallback creates a provider that falls back to secondary if primary fails -func NewProviderWithFallback(primary, fallback Provider) Provider { - return &ProviderWithFallback{ - primary: primary, - fallback: fallback, - } -} - -// GenerateAudio tries primary provider first, falls back to secondary on error -func (p *ProviderWithFallback) GenerateAudio(ctx context.Context, text string, outputFile string) error { - err := p.primary.GenerateAudio(ctx, text, outputFile) - if err != nil { - // Log the primary error - fmt.Printf("Primary provider (%s) failed: %v. Falling back to %s\n", - p.primary.Name(), err, p.fallback.Name()) - - // Try fallback - return p.fallback.GenerateAudio(ctx, text, outputFile) - } - return nil -} - -// Name returns the provider name -func (p *ProviderWithFallback) Name() string { - return fmt.Sprintf("%s (fallback: %s)", p.primary.Name(), p.fallback.Name()) -} - -// IsAvailable checks if at least one provider is available -func (p *ProviderWithFallback) IsAvailable() error { - primaryErr := p.primary.IsAvailable() - if primaryErr == nil { - return nil - } - - fallbackErr := p.fallback.IsAvailable() - if fallbackErr == nil { - return nil - } - - return fmt.Errorf("both providers unavailable: primary=%v, fallback=%v", - primaryErr, fallbackErr) -} diff --git a/internal/audio/provider_test.go b/internal/audio/provider_test.go index a08b7a6..5702016 100644 --- a/internal/audio/provider_test.go +++ b/internal/audio/provider_test.go @@ -2,7 +2,6 @@ package audio import ( "context" - "errors" "path/filepath" "strings" "testing" @@ -153,94 +152,3 @@ func TestNewProvider(t *testing.T) { } } -func TestProviderWithFallback(t *testing.T) { - primary := &mockProvider{name: "primary"} - fallback := &mockProvider{name: "fallback"} - - provider := NewProviderWithFallback(primary, fallback) - - // Test successful primary - ctx := context.Background() - err := provider.GenerateAudio(ctx, "test", "output.mp3") - if err != nil { - t.Errorf("GenerateAudio() unexpected error: %v", err) - } - if primary.generateCalls != 1 { - t.Errorf("Expected 1 primary call, got %d", primary.generateCalls) - } - if fallback.generateCalls != 0 { - t.Errorf("Expected 0 fallback calls, got %d", fallback.generateCalls) - } - - // Test primary failure, fallback success - primary.generateErr = errors.New("primary failed") - primary.generateCalls = 0 - - err = provider.GenerateAudio(ctx, "test", "output.mp3") - if err != nil { - t.Errorf("GenerateAudio() unexpected error: %v", err) - } - if primary.generateCalls != 1 { - t.Errorf("Expected 1 primary call, got %d", primary.generateCalls) - } - if fallback.generateCalls != 1 { - t.Errorf("Expected 1 fallback call, got %d", fallback.generateCalls) - } - - // Test both fail - fallback.generateErr = errors.New("fallback failed") - primary.generateCalls = 0 - fallback.generateCalls = 0 - - err = provider.GenerateAudio(ctx, "test", "output.mp3") - if err == nil { - t.Error("GenerateAudio() expected error when both providers fail") - } -} - -func TestProviderWithFallbackName(t *testing.T) { - primary := &mockProvider{name: "primary"} - fallback := &mockProvider{name: "fallback"} - - provider := NewProviderWithFallback(primary, fallback) - - expected := "primary (fallback: fallback)" - if provider.Name() != expected { - t.Errorf("Name() = %v, want %v", provider.Name(), expected) - } -} - -func TestProviderWithFallbackIsAvailable(t *testing.T) { - primary := &mockProvider{name: "primary"} - fallback := &mockProvider{name: "fallback"} - - provider := NewProviderWithFallback(primary, fallback) - - // Both available - err := provider.IsAvailable() - if err != nil { - t.Errorf("IsAvailable() unexpected error: %v", err) - } - - // Primary unavailable, fallback available - primary.availableErr = errors.New("primary unavailable") - err = provider.IsAvailable() - if err != nil { - t.Errorf("IsAvailable() unexpected error when fallback available: %v", err) - } - - // Primary available, fallback unavailable - primary.availableErr = nil - fallback.availableErr = errors.New("fallback unavailable") - err = provider.IsAvailable() - if err != nil { - t.Errorf("IsAvailable() unexpected error when primary available: %v", err) - } - - // Both unavailable - primary.availableErr = errors.New("primary unavailable") - err = provider.IsAvailable() - if err == nil { - t.Error("IsAvailable() expected error when both providers unavailable") - } -} diff --git a/internal/audio/validate.go b/internal/audio/validate.go index db042bd..e200cfa 100644 --- a/internal/audio/validate.go +++ b/internal/audio/validate.go @@ -11,7 +11,7 @@ func ValidateBulgarianText(text string) error { if strings.TrimSpace(text) == "" { return fmt.Errorf("text cannot be empty") } - + hasCyrillic := false for _, r := range text { if unicode.In(r, unicode.Cyrillic) { @@ -19,10 +19,10 @@ func ValidateBulgarianText(text string) error { break } } - + if !hasCyrillic { return fmt.Errorf("text must contain Cyrillic characters") } - + return nil -}
\ No newline at end of file +} diff --git a/internal/batch/processor.go b/internal/batch/processor.go index 314d67f..2b002a9 100644 --- a/internal/batch/processor.go +++ b/internal/batch/processor.go @@ -31,10 +31,11 @@ func ReadBatchFile(filename string) ([]WordEntry, error) { } var entries []WordEntry - lines := string(content) - - for _, line := range splitLines(lines) { - if line = trimSpace(line); line != "" { + // Normalize \r\n to \n before splitting so both Windows and Unix line + // endings are handled uniformly by strings.Split. + normalized := strings.ReplaceAll(string(content), "\r\n", "\n") + for _, line := range strings.Split(normalized, "\n") { + if line = strings.TrimSpace(line); line != "" { entry := parseBatchLine(line) if entry != nil { entries = append(entries, *entry) @@ -103,43 +104,3 @@ func parseBatchLine(line string) *WordEntry { } } -// splitLines splits a string by newlines, handling both \n and \r\n line endings. -// Uses strings.Builder to avoid per-character heap allocations from += concatenation. -func splitLines(s string) []string { - var lines []string - var current strings.Builder - for _, r := range s { - if r == '\n' { - lines = append(lines, current.String()) - current.Reset() - } else if r != '\r' { - current.WriteRune(r) - } - } - if current.Len() > 0 { - lines = append(lines, current.String()) - } - return lines -} - -// trimSpace trims whitespace from string -func trimSpace(s string) string { - start := 0 - end := len(s) - - // Trim from start - for start < end && isSpace(rune(s[start])) { - start++ - } - - // Trim from end - for end > start && isSpace(rune(s[end-1])) { - end-- - } - - return s[start:end] -} - -func isSpace(r rune) bool { - return r == ' ' || r == '\t' || r == '\n' || r == '\r' -} diff --git a/internal/batch/processor_test.go b/internal/batch/processor_test.go index fc4f7b0..8bc8079 100644 --- a/internal/batch/processor_test.go +++ b/internal/batch/processor_test.go @@ -163,137 +163,3 @@ func TestReadBatchFile_FileNotFound(t *testing.T) { } } -func TestSplitLines(t *testing.T) { - tests := []struct { - name string - input string - want []string - }{ - { - name: "unix line endings", - input: "line1\nline2\nline3", - want: []string{"line1", "line2", "line3"}, - }, - { - name: "windows line endings", - input: "line1\r\nline2\r\nline3", - want: []string{"line1", "line2", "line3"}, - }, - { - name: "mixed line endings", - input: "line1\nline2\r\nline3", - want: []string{"line1", "line2", "line3"}, - }, - { - name: "empty string", - input: "", - want: nil, - }, - { - name: "single line no ending", - input: "single line", - want: []string{"single line"}, - }, - { - name: "trailing newline", - input: "line1\nline2\n", - want: []string{"line1", "line2"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := splitLines(tt.input) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("splitLines() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestTrimSpace(t *testing.T) { - tests := []struct { - name string - input string - want string - }{ - { - name: "no whitespace", - input: "hello", - want: "hello", - }, - { - name: "leading spaces", - input: " hello", - want: "hello", - }, - { - name: "trailing spaces", - input: "hello ", - want: "hello", - }, - { - name: "both sides", - input: " hello ", - want: "hello", - }, - { - name: "tabs and spaces", - input: "\t hello \t", - want: "hello", - }, - { - name: "newlines", - input: "\nhello\n", - want: "hello", - }, - { - name: "all whitespace types", - input: " \t\n\rhello \t\n\r", - want: "hello", - }, - { - name: "empty string", - input: "", - want: "", - }, - { - name: "only whitespace", - input: " \t\n\r ", - want: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := trimSpace(tt.input) - if got != tt.want { - t.Errorf("trimSpace() = %q, want %q", got, tt.want) - } - }) - } -} - -func TestIsSpace(t *testing.T) { - tests := []struct { - r rune - want bool - }{ - {' ', true}, - {'\t', true}, - {'\n', true}, - {'\r', true}, - {'a', false}, - {'1', false}, - {'!', false}, - {0, false}, - } - - for _, tt := range tests { - t.Run(string(tt.r), func(t *testing.T) { - if got := isSpace(tt.r); got != tt.want { - t.Errorf("isSpace(%q) = %v, want %v", tt.r, got, tt.want) - } - }) - } -} diff --git a/internal/config/doc.go b/internal/config/doc.go index 1e67a8a..2469660 100644 --- a/internal/config/doc.go +++ b/internal/config/doc.go @@ -1,3 +1,3 @@ // Package config provides configuration management for the totalrecall // application using viper for flexible configuration options. -package config
\ No newline at end of file +package config diff --git a/internal/gui/icon.go b/internal/gui/icon.go index f778225..8085cc9 100644 --- a/internal/gui/icon.go +++ b/internal/gui/icon.go @@ -14,4 +14,4 @@ func GetAppIcon() fyne.Resource { StaticName: "totalrecall.png", StaticContent: iconData, } -}
\ No newline at end of file +} diff --git a/internal/image/doc.go b/internal/image/doc.go index 2fb3723..c1b7ca3 100644 --- a/internal/image/doc.go +++ b/internal/image/doc.go @@ -1,3 +1,3 @@ // Package image provides image search functionality to find // representative images for Bulgarian words from various APIs. -package image
\ No newline at end of file +package image diff --git a/internal/image/nanobanana.go b/internal/image/nanobanana.go index a35e4cf..1f3ac57 100644 --- a/internal/image/nanobanana.go +++ b/internal/image/nanobanana.go @@ -301,8 +301,13 @@ func (c *NanoBananaClient) buildPrompt(ctx context.Context, opts *SearchOptions) return prompt, translatedWord, nil } +// createEducationalPrompt generates a prompt optimized for language learning. +// Scene generation and style selection are handled here; the shared +// buildEducationalPrompt helper assembles the actual prompt text so that the +// same policy is used by both NanoBananaClient and OpenAIClient. func (c *NanoBananaClient) createEducationalPrompt(ctx context.Context, bulgarianWord, englishTranslation string) string { subject := promptSubject(englishTranslation, bulgarianWord) + scene, err := c.generateSceneDescription(ctx, bulgarianWord, englishTranslation) if err != nil { fmt.Printf(" Failed to generate scene: %v, using basic prompt\n", err) @@ -316,66 +321,15 @@ func (c *NanoBananaClient) createEducationalPrompt(ctx context.Context, bulgaria } } + // Select a random style from the shared pool. Fall back to a generic style + // if the pool has been exhausted by tests or other callers. selectedStyle := chooseArtisticStyle() if selectedStyle == defaultArtisticStyle { fmt.Printf(" No artistic styles available, using generic prompt\n") } fmt.Printf(" Using image style: %s\n", selectedStyle) - var prompt string - - if scene != "" { - fullPrompt := fmt.Sprintf( - "Generate a %s educational flashcard image illustrating \"%s\". Scene: %s "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, easily recognizable, and prominent in the image. It should occupy the central area with sharp focus and proper lighting. Ensure the scene makes \"%s\" immediately identifiable. "+ - "IMPORTANT: No text whatsoever. Do not include any words, letters, typography, labels, captions, or writing of any kind. Image only, without any text elements.", - selectedStyle, subject, withTerminalPunctuation(scene), subject, - ) - - if len(fullPrompt) > maxImagePromptChars { - prompt = fmt.Sprintf( - "Generate a %s flashcard image illustrating \"%s\". Scene: %s "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", - selectedStyle, subject, withTerminalPunctuation(scene), - ) - - if len(prompt) > maxImagePromptChars { - maxSceneLen := maxImagePromptChars - len(fmt.Sprintf( - "Generate a %s flashcard image illustrating \"%s\". Scene: "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", - selectedStyle, subject, - )) - if maxSceneLen > 3 && len(scene) > maxSceneLen { - scene = scene[:maxSceneLen] + "..." - } - prompt = fmt.Sprintf( - "Generate a %s flashcard image illustrating \"%s\". Scene: %s "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", - selectedStyle, subject, withTerminalPunctuation(scene), - ) - } - } else { - prompt = fullPrompt - } - } else { - prompt = fmt.Sprintf( - "Generate a %s educational flashcard image illustrating \"%s\". %s "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, easily recognizable, and prominent in the image. Show it prominently centered with excellent lighting and sharp focus. "+ - "IMPORTANT: No text whatsoever. Do not include any words, letters, typography, labels, captions, or writing of any kind. Image only, without any text elements.", - selectedStyle, subject, fallbackVisualDirection(subject), - ) - } - - if len(prompt) > maxImagePromptChars { - prompt = prompt[:997] + "..." - } - - return prompt + return buildEducationalPrompt(selectedStyle, scene, subject) } func (c *NanoBananaClient) translateBulgarianToEnglish(ctx context.Context, word string) (string, error) { diff --git a/internal/image/openai.go b/internal/image/openai.go index 8edb182..fdd88e4 100644 --- a/internal/image/openai.go +++ b/internal/image/openai.go @@ -243,10 +243,13 @@ func (c *OpenAIClient) SetPromptCallback(callback func(prompt string)) { c.PromptCallback = callback } -// createEducationalPrompt generates a prompt optimized for language learning +// createEducationalPrompt generates a prompt optimized for language learning. +// Scene generation and style selection are handled here; the shared +// buildEducationalPrompt helper assembles the actual prompt text so that the +// same policy is used by both OpenAIClient and NanoBananaClient. func (c *OpenAIClient) createEducationalPrompt(ctx context.Context, bulgarianWord, englishTranslation string) string { subject := promptSubject(englishTranslation, bulgarianWord) - // Generate a scene description for the word + scene, err := c.generateSceneDescription(ctx, bulgarianWord, englishTranslation) if err != nil { fmt.Printf(" Failed to generate scene: %v, using basic prompt\n", err) @@ -260,76 +263,15 @@ func (c *OpenAIClient) createEducationalPrompt(ctx context.Context, bulgarianWor } } - // Select a random style from the shared pool. Fall back to a generic style if - // the pool has been emptied by tests or future callers. + // Select a random style from the shared pool. Fall back to a generic style + // if the pool has been exhausted by tests or other callers. selectedStyle := chooseArtisticStyle() if selectedStyle == defaultArtisticStyle { fmt.Printf(" No artistic styles available, using generic prompt\n") } fmt.Printf(" Using image style: %s\n", selectedStyle) - // Define prompt components in order of importance - var prompt string - - if scene != "" { - // Full prompt with scene - fullPrompt := fmt.Sprintf( - "Generate a %s educational flashcard image illustrating \"%s\". Scene: %s "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, easily recognizable, and prominent in the image. It should occupy the central area with sharp focus and proper lighting. Ensure the scene makes \"%s\" immediately identifiable. "+ - "IMPORTANT: No text whatsoever. Do not include any words, letters, typography, labels, captions, or writing of any kind. Image only, without any text elements.", - selectedStyle, subject, withTerminalPunctuation(scene), subject, - ) - - // Check if full prompt exceeds 1000 characters - if len(fullPrompt) > maxImagePromptChars { - // Try without the IMPORTANT notice - prompt = fmt.Sprintf( - "Generate a %s flashcard image illustrating \"%s\". Scene: %s "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", - selectedStyle, subject, withTerminalPunctuation(scene), - ) - - // If still too long, truncate the scene - if len(prompt) > maxImagePromptChars { - // Truncate scene to fit within limit - maxSceneLen := maxImagePromptChars - len(fmt.Sprintf( - "Generate a %s flashcard image illustrating \"%s\". Scene: "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", - selectedStyle, subject, - )) - if maxSceneLen > 3 && len(scene) > maxSceneLen { - scene = scene[:maxSceneLen] + "..." - } - prompt = fmt.Sprintf( - "Generate a %s flashcard image illustrating \"%s\". Scene: %s "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", - selectedStyle, subject, withTerminalPunctuation(scene), - ) - } - } else { - prompt = fullPrompt - } - } else { - // Basic prompt without scene - prompt = fmt.Sprintf( - "Generate a %s educational flashcard image illustrating \"%s\". %s "+ - "The image should be educational and suitable for language learning flashcards. "+ - "Requirements: The main subject or concept must be clearly visible, easily recognizable, and prominent in the image. Show it prominently centered with excellent lighting and sharp focus. "+ - "IMPORTANT: No text whatsoever. Do not include any words, letters, typography, labels, captions, or writing of any kind. Image only, without any text elements.", - selectedStyle, subject, fallbackVisualDirection(subject), - ) - } - - // Final check to ensure prompt is within 1000 characters - if len(prompt) > maxImagePromptChars { - prompt = prompt[:997] + "..." - } - - return prompt + return buildEducationalPrompt(selectedStyle, scene, subject) } // translateBulgarianToEnglish translates a Bulgarian word to English using OpenAI diff --git a/internal/image/prompt.go b/internal/image/prompt.go index 7e69a8c..3d5f889 100644 --- a/internal/image/prompt.go +++ b/internal/image/prompt.go @@ -1,6 +1,9 @@ package image -import "strings" +import ( + "fmt" + "strings" +) const maxImagePromptChars = 1000 @@ -92,6 +95,74 @@ func withTerminalPunctuation(text string) string { } } +// buildEducationalPrompt assembles the final image-generation prompt from a +// pre-chosen artistic style, an optional scene description, and the word subject. +// Both OpenAIClient and NanoBananaClient share this logic so the prompt policy +// has a single authoritative home. The scene parameter may be empty, in which +// case a simpler fallback prompt is used. The result is always capped at +// maxImagePromptChars characters. +func buildEducationalPrompt(style, scene, subject string) string { + var prompt string + + if scene != "" { + fullPrompt := fmt.Sprintf( + "Generate a %s educational flashcard image illustrating \"%s\". Scene: %s "+ + "The image should be educational and suitable for language learning flashcards. "+ + "Requirements: The main subject or concept must be clearly visible, easily recognizable, and prominent in the image. It should occupy the central area with sharp focus and proper lighting. Ensure the scene makes \"%s\" immediately identifiable. "+ + "IMPORTANT: No text whatsoever. Do not include any words, letters, typography, labels, captions, or writing of any kind. Image only, without any text elements.", + style, subject, withTerminalPunctuation(scene), subject, + ) + + if len(fullPrompt) <= maxImagePromptChars { + prompt = fullPrompt + } else { + // Try a shorter version without the IMPORTANT notice. + prompt = fmt.Sprintf( + "Generate a %s flashcard image illustrating \"%s\". Scene: %s "+ + "The image should be educational and suitable for language learning flashcards. "+ + "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", + style, subject, withTerminalPunctuation(scene), + ) + + // If still too long, truncate the scene to fit. + if len(prompt) > maxImagePromptChars { + template := fmt.Sprintf( + "Generate a %s flashcard image illustrating \"%s\". Scene: "+ + "The image should be educational and suitable for language learning flashcards. "+ + "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", + style, subject, + ) + maxSceneLen := maxImagePromptChars - len(template) + if maxSceneLen > 3 && len(scene) > maxSceneLen { + scene = scene[:maxSceneLen] + "..." + } + prompt = fmt.Sprintf( + "Generate a %s flashcard image illustrating \"%s\". Scene: %s "+ + "The image should be educational and suitable for language learning flashcards. "+ + "Requirements: The main subject or concept must be clearly visible, centered, well lit, and easy to identify.", + style, subject, withTerminalPunctuation(scene), + ) + } + } + } else { + // No scene available — use a simpler fallback prompt. + prompt = fmt.Sprintf( + "Generate a %s educational flashcard image illustrating \"%s\". %s "+ + "The image should be educational and suitable for language learning flashcards. "+ + "Requirements: The main subject or concept must be clearly visible, easily recognizable, and prominent in the image. Show it prominently centered with excellent lighting and sharp focus. "+ + "IMPORTANT: No text whatsoever. Do not include any words, letters, typography, labels, captions, or writing of any kind. Image only, without any text elements.", + style, subject, fallbackVisualDirection(subject), + ) + } + + // Hard cap at maxImagePromptChars. + if len(prompt) > maxImagePromptChars { + prompt = prompt[:997] + "..." + } + + return prompt +} + func fallbackVisualDirection(subject string) string { subject = normalizePromptText(subject) lower := strings.ToLower(subject) diff --git a/internal/image/search.go b/internal/image/search.go index 80274eb..9be72ed 100644 --- a/internal/image/search.go +++ b/internal/image/search.go @@ -80,4 +80,3 @@ type RateLimitError struct { func (e *RateLimitError) Error() string { return e.Provider + ": rate limit exceeded" } - diff --git a/internal/image/search_test.go b/internal/image/search_test.go index b7018d9..fa3d770 100644 --- a/internal/image/search_test.go +++ b/internal/image/search_test.go @@ -39,27 +39,27 @@ func (m *mockSearcher) Name() string { func TestDefaultSearchOptions(t *testing.T) { opts := DefaultSearchOptions("ябълка") - + if opts.Query != "ябълка" { t.Errorf("Expected query 'ябълка', got '%s'", opts.Query) } - + if opts.Language != "bg" { t.Errorf("Expected language 'bg', got '%s'", opts.Language) } - + if !opts.SafeSearch { t.Error("Expected SafeSearch to be true") } - + if opts.PerPage != 10 { t.Errorf("Expected PerPage 10, got %d", opts.PerPage) } - + if opts.Page != 1 { t.Errorf("Expected Page 1, got %d", opts.Page) } - + if opts.ImageType != "photo" { t.Errorf("Expected ImageType 'photo', got '%s'", opts.ImageType) } @@ -71,7 +71,7 @@ func TestSearchError(t *testing.T) { Code: "404", Message: " |
