diff options
| author | Paul Buetow <paul@buetow.org> | 2025-07-22 08:15:07 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2025-07-22 08:15:07 +0300 |
| commit | 35ff0f508cf9cb52048f2ed67bd07f4aec17f2bf (patch) | |
| tree | 3c3aef2d478e3edde300311137df43b2353af5f2 | |
| parent | 1cc15cfbd68d45ae6d561e5659422e72bf9ecd1d (diff) | |
Fix GUI background image generation race conditions
- Add file.Sync() after image download to ensure data is flushed to disk
- Add double-checking in UI updates to prevent wrong card updates
- Fix background job completion to reload files when user navigates back
- Add file size validation in image display widget
- Improve error messages for image loading failures
This fixes two issues:
1. 'png: invalid format: not enough pixel data' error when navigating during generation
2. Images not updating when navigating back to a card after background generation completes
| -rw-r--r-- | CLAUDE.md | 104 | ||||
| -rw-r--r-- | internal/gui/app.go | 77 | ||||
| -rw-r--r-- | internal/gui/widgets.go | 19 | ||||
| -rw-r--r-- | internal/image/download.go | 82 |
4 files changed, 128 insertions, 154 deletions
diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 1ce4d5f..0000000 --- a/CLAUDE.md +++ /dev/null @@ -1,104 +0,0 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## Project Overview -**totalrecall** - Bulgarian Anki Flashcard Generator - -A Go CLI tool that generates Anki flashcard materials from Bulgarian words: -- Generates audio pronunciation using OpenAI TTS -- Generates images using OpenAI DALL-E -- Creates Anki-compatible output files - -## Important: Task Tracking -**Always check TODO.md for the current implementation status and pending tasks.** The TODO.md file contains a comprehensive breakdown of all features and their completion status. - -## Build and Development Commands - -### Available Tasks (via Taskfile) -```bash -# Build the binary -task -# or -task default - -# Run the application -task run - -# Run tests -task test - -# Install to Go bin directory -task install -``` - -### Common Development Commands -```bash -# Build for current platform -go build -o totalrecall ./cmd/totalrecall - -# Run without building -go run ./cmd/totalrecall "ябълка" - -# Run tests with coverage -go test -v -cover ./... - -# Check for race conditions -go test -race ./... - -# Format code -go fmt ./... - -# Lint code (requires golangci-lint) -golangci-lint run -``` - -## Architecture Overview - -### Package Structure -``` -totalrecall/ -├── cmd/totalrecall/ # CLI entry point -├── internal/ # Private packages -│ ├── audio/ # Audio generation (OpenAI TTS) -│ ├── image/ # Image generation functionality -│ ├── anki/ # Anki format generation -│ ├── config/ # Configuration management -│ └── version.go # Version information -``` - -### Key Design Decisions -1. **OpenAI TTS**: High-quality, natural-sounding Bulgarian pronunciation -2. **Image generation**: Uses OpenAI DALL-E for AI-generated images -3. **Configuration via YAML**: User-friendly configuration with viper -4. **Cobra for CLI**: Industry-standard CLI framework - -### External Dependencies -- **OpenAI API Key**: Required for both audio generation and image creation - -## Testing Approach -1. Unit tests mock API calls -2. Integration tests use real services when available -3. Test with common Bulgarian words: ябълка, котка, куче, хляб - -## Common Issues and Solutions - - -### Package Declaration Error -If you see an error about `package main`, ensure `cmd/totalrecall/main.go` has: -```go -package main // NOT package bulg -``` - -## Development Workflow -1. Check TODO.md for next tasks -2. Create feature branch -3. Implement with tests -4. Update documentation -5. Run full test suite -6. Submit for review - -## Bulgarian Language Notes -- Input should be in Cyrillic script -- Common test words: ябълка (apple), котка (cat), куче (dog) -- OpenAI voices: nova, alloy, echo, shimmer (work well for Bulgarian)
\ No newline at end of file diff --git a/internal/gui/app.go b/internal/gui/app.go index 6a58286..685c913 100644 --- a/internal/gui/app.go +++ b/internal/gui/app.go @@ -716,11 +716,19 @@ func (a *Application) generateMaterials(word string) { a.mu.Lock() if a.currentWord == word { a.currentAudioFile = audioRes.file + audioFile := audioRes.file + a.mu.Unlock() fyne.Do(func() { - a.audioPlayer.SetAudioFile(audioRes.file) + // Double-check inside the UI update that we're still on the same word + a.mu.Lock() + if a.currentWord == word { + a.audioPlayer.SetAudioFile(audioFile) + } + a.mu.Unlock() }) + } else { + a.mu.Unlock() } - a.mu.Unlock() } // Collect image result @@ -735,11 +743,19 @@ func (a *Application) generateMaterials(word string) { a.mu.Lock() if a.currentWord == word { a.currentImage = imageRes.file + imageFile := imageRes.file + a.mu.Unlock() fyne.Do(func() { - a.imageDisplay.SetImages([]string{imageRes.file}) + // Double-check inside the UI update that we're still on the same word + a.mu.Lock() + if a.currentWord == word { + a.imageDisplay.SetImages([]string{imageFile}) + } + a.mu.Unlock() }) + } else { + a.mu.Unlock() } - a.mu.Unlock() } // Collect phonetic result (UI already updated in the goroutine) @@ -967,19 +983,34 @@ func (a *Application) onRegenerateAudio() { defer a.wg.Done() defer a.decrementProcessing() // Audio processing ends + // Store the word we're generating for + wordForGeneration := a.currentWord + // Get or create context for this card - cardCtx, _ := a.getOrCreateCardContext(a.currentWord) + cardCtx, _ := a.getOrCreateCardContext(wordForGeneration) - audioFile, err := a.generateAudio(cardCtx, a.currentWord) + audioFile, err := a.generateAudio(cardCtx, wordForGeneration) if err != nil { fyne.Do(func() { a.showError(fmt.Errorf("Audio regeneration failed: %w", err)) }) } else { - a.currentAudioFile = audioFile - fyne.Do(func() { - a.audioPlayer.SetAudioFile(audioFile) - }) + // Only update if we're still on the same word + a.mu.Lock() + if a.currentWord == wordForGeneration { + a.currentAudioFile = audioFile + a.mu.Unlock() + fyne.Do(func() { + // Double-check inside the UI update that we're still on the same word + a.mu.Lock() + if a.currentWord == wordForGeneration { + a.audioPlayer.SetAudioFile(audioFile) + } + a.mu.Unlock() + }) + } else { + a.mu.Unlock() + } } fyne.Do(func() { @@ -1897,6 +1928,14 @@ func (a *Application) processWordJob(job *WordJob) { if isCurrentJob { fyne.Do(func() { + // Double-check that we're still on the same job before updating UI + a.mu.Lock() + if a.currentJobID != job.ID { + a.mu.Unlock() + return + } + a.mu.Unlock() + a.audioPlayer.SetAudioFile(audioFile) // Enable audio-related actions a.regenerateAudioBtn.Enable() @@ -1948,6 +1987,14 @@ func (a *Application) processWordJob(job *WordJob) { if isCurrentJob { fyne.Do(func() { + // Double-check that we're still on the same job before updating UI + a.mu.Lock() + if a.currentJobID != job.ID { + a.mu.Unlock() + return + } + a.mu.Unlock() + a.translationEntry.SetText(translation) if imageFile != "" { a.imageDisplay.SetImages([]string{imageFile}) @@ -2022,6 +2069,16 @@ func (a *Application) onJobComplete(job *WordJob) { } else { // This is a background job that completed a.updateStatus(fmt.Sprintf("Background processing completed: %s", job.Word)) + + // Check if user has navigated back to this word + a.mu.Lock() + currentWord := a.currentWord + a.mu.Unlock() + + if currentWord == job.Word { + // User is currently viewing this word, reload the files + a.loadExistingFiles(job.Word) + } } } }) diff --git a/internal/gui/widgets.go b/internal/gui/widgets.go index 6ad428c..df74027 100644 --- a/internal/gui/widgets.go +++ b/internal/gui/widgets.go @@ -73,7 +73,20 @@ func (d *ImageDisplay) SetImage(imagePath string) { } defer file.Close() - img, _, err := image.Decode(file) + // Get file info to ensure it's fully written + stat, err := file.Stat() + if err != nil { + d.imageLabel.SetText(fmt.Sprintf("Error getting file info: %v", err)) + return + } + + // If file size is 0, it might still be writing + if stat.Size() == 0 { + d.imageLabel.SetText("Image file is empty") + return + } + + img, format, err := image.Decode(file) if err != nil { d.imageLabel.SetText(fmt.Sprintf("Error decoding image: %v", err)) return @@ -83,8 +96,8 @@ func (d *ImageDisplay) SetImage(imagePath string) { d.imageCanvas.Image = img d.imageCanvas.Refresh() - // Update label - d.imageLabel.SetText(filepath.Base(imagePath)) + // Update label with format info + d.imageLabel.SetText(fmt.Sprintf("%s (%s)", filepath.Base(imagePath), format)) } // SetImages sets multiple images but only displays the first one diff --git a/internal/image/download.go b/internal/image/download.go index fabb1bb..264ad1a 100644 --- a/internal/image/download.go +++ b/internal/image/download.go @@ -11,21 +11,21 @@ import ( // DownloadOptions configures image download behavior type DownloadOptions struct { - OutputDir string // Directory to save images + OutputDir string // Directory to save images OverwriteExisting bool // Whether to overwrite existing files - CreateDir bool // Create output directory if it doesn't exist - FileNamePattern string // Pattern for file naming (e.g., "{word}_{source}") - MaxSizeBytes int64 // Maximum file size to download (0 = no limit) + CreateDir bool // Create output directory if it doesn't exist + FileNamePattern string // Pattern for file naming (e.g., "{word}_{source}") + MaxSizeBytes int64 // Maximum file size to download (0 = no limit) } // DefaultDownloadOptions returns sensible defaults for image downloads func DefaultDownloadOptions() *DownloadOptions { return &DownloadOptions{ - OutputDir: "./images", + OutputDir: "./images", OverwriteExisting: false, - CreateDir: true, - FileNamePattern: "{word}_{source}", - MaxSizeBytes: 10 * 1024 * 1024, // 10MB + CreateDir: true, + FileNamePattern: "{word}_{source}", + MaxSizeBytes: 10 * 1024 * 1024, // 10MB } } @@ -55,28 +55,28 @@ func (d *Downloader) DownloadImage(ctx context.Context, result *SearchResult, ou return fmt.Errorf("failed to create directory: %w", err) } } - + // Check if file already exists if !d.options.OverwriteExisting { if _, err := os.Stat(outputPath); err == nil { return fmt.Errorf("file already exists: %s", outputPath) } } - + // Download the image reader, err := d.searcher.Download(ctx, result.URL) if err != nil { return fmt.Errorf("failed to download image: %w", err) } defer reader.Close() - + // Create output file file, err := os.Create(outputPath) if err != nil { return fmt.Errorf("failed to create file: %w", err) } defer file.Close() - + // Copy with size limit if specified var written int64 if d.options.MaxSizeBytes > 0 { @@ -85,7 +85,7 @@ func (d *Downloader) DownloadImage(ctx context.Context, result *SearchResult, ou os.Remove(outputPath) // Clean up on error return fmt.Errorf("failed to write file: %w", err) } - + // Check if we hit the size limit if written == d.options.MaxSizeBytes { // Try to read one more byte to see if file is larger @@ -94,6 +94,11 @@ func (d *Downloader) DownloadImage(ctx context.Context, result *SearchResult, ou return fmt.Errorf("image exceeds maximum size of %d bytes", d.options.MaxSizeBytes) } } + + // Ensure the file is fully written to disk before returning + if err := file.Sync(); err != nil { + return fmt.Errorf("failed to sync file to disk: %w", err) + } } else { written, err = io.Copy(file, reader) if err != nil { @@ -101,7 +106,12 @@ func (d *Downloader) DownloadImage(ctx context.Context, result *SearchResult, ou return fmt.Errorf("failed to write file: %w", err) } } - + + // Ensure the file is fully written to disk before returning + if err := file.Sync(); err != nil { + return fmt.Errorf("failed to sync file to disk: %w", err) + } + // Save attribution if required if attribution := d.searcher.GetAttribution(result); attribution != "" { attrPath := strings.TrimSuffix(outputPath, filepath.Ext(outputPath)) + "_attribution.txt" @@ -110,7 +120,7 @@ func (d *Downloader) DownloadImage(ctx context.Context, result *SearchResult, ou fmt.Fprintf(os.Stderr, "Warning: failed to save attribution: %v\n", err) } } - + return nil } @@ -119,32 +129,32 @@ func (d *Downloader) DownloadBestMatch(ctx context.Context, query string) (*Sear // Search for images opts := DefaultSearchOptions(query) opts.PerPage = 5 // Get top 5 results - + results, err := d.searcher.Search(ctx, opts) if err != nil { return nil, "", fmt.Errorf("search failed: %w", err) } - + if len(results) == 0 { return nil, "", fmt.Errorf("no images found for query: %s", query) } - + // Try to download the first available image for i, result := range results { // Generate filename filename := d.generateFileName(query, &result, i) outputPath := filepath.Join(d.options.OutputDir, filename) - + // Try to download err := d.DownloadImage(ctx, &result, outputPath) if err == nil { return &result, outputPath, nil } - + // Log error and try next fmt.Fprintf(os.Stderr, "Warning: failed to download image %d: %v\n", i+1, err) } - + return nil, "", fmt.Errorf("failed to download any images for query: %s", query) } @@ -152,24 +162,24 @@ func (d *Downloader) DownloadBestMatch(ctx context.Context, query string) (*Sear func (d *Downloader) generateFileName(word string, result *SearchResult, index int) string { // Start with the pattern filename := d.options.FileNamePattern - + // Replace placeholders filename = strings.ReplaceAll(filename, "{word}", sanitizeFileName(word)) filename = strings.ReplaceAll(filename, "{source}", result.Source) filename = strings.ReplaceAll(filename, "{id}", result.ID) filename = strings.ReplaceAll(filename, "{index}", fmt.Sprintf("%d", index)) - + // Determine extension from URL ext := filepath.Ext(result.URL) if ext == "" || len(ext) > 5 { // Probably not a real extension ext = ".jpg" // Default to jpg } - + // Add extension if not present if filepath.Ext(filename) == "" { filename += ext } - + return filename } @@ -189,49 +199,47 @@ func sanitizeFileName(name string) string { " ", "_", ".", "_", ) - + sanitized := replacer.Replace(name) - + // Ensure the filename is not too long if len(sanitized) > 50 { sanitized = sanitized[:50] } - + return sanitized } - // DownloadBestMatchWithOptions downloads the best matching image for given search options func (d *Downloader) DownloadBestMatchWithOptions(ctx context.Context, opts *SearchOptions) (*SearchResult, string, error) { // Search for images - searchOpts := *opts // Copy to avoid modifying original + searchOpts := *opts // Copy to avoid modifying original searchOpts.PerPage = 5 // Get top 5 results - + results, err := d.searcher.Search(ctx, &searchOpts) if err != nil { return nil, "", fmt.Errorf("search failed: %w", err) } - + if len(results) == 0 { return nil, "", fmt.Errorf("no images found for query: %s", opts.Query) } - + // Try to download the first available image for i, result := range results { // Generate filename filename := d.generateFileName(opts.Query, &result, i) outputPath := filepath.Join(d.options.OutputDir, filename) - + // Try to download err := d.DownloadImage(ctx, &result, outputPath) if err == nil { return &result, outputPath, nil } - + // Log error and try next fmt.Fprintf(os.Stderr, "Warning: failed to download image %d: %v\n", i+1, err) } - + return nil, "", fmt.Errorf("failed to download any images for query: %s", opts.Query) } - |
