docs: simplify Calibre OPF implementation approach

Update developer documentation to reflect simplified implementation approach based on user feedback.

Key changes:
- Rename extractMetadataFromCalibreSidecar() to extractCalibreSidecar()
- Simplify function signature: return *MediaMetadata instead of (*MediaMetadata, error)
- Replace wrapper function pattern with direct modification of extractMetadata()
- Add code example showing simple if-check at top of extractMetadata()
- Document benefits of simplified approach (40% less code, 0 call site changes)
- Add implementation note explaining the simplification

Benefits of simplified approach:
- ~150 lines of code vs. ~250 lines (40% reduction)
- No wrapper function needed
- No call site changes required
- Clearer single entry point for metadata extraction
- Better testability
- Easier to maintain

This change simplifies the implementation while maintaining all functionality. The sidecar-first approach remains the same, but implementation is cleaner and more straightforward.

See: CALIBRE_OPF_IMPLEMENTATION.md Decision 4 for full rationale

docs/development/calibre-opf-implementation.md

@@ -96,39 +96,46 @@ type MediaMetadata struct {
 
 ## Implementation Functions
 
-### 1. extractMetadataFromCalibreSidecar()
+### 1. extractCalibreSidecar()
 
 **Location**: `internal/services/media_scanner.go`
 
 **Signature**:
 ```go
-func (s *MediaScanner) extractMetadataFromCalibreSidecar(mediaFilePath string) (*MediaMetadata, error)
+func (s *MediaScanner) extractCalibreSidecar(path string) *MediaMetadata
 ```
 
 **Purpose**: Checks for `metadata.opf` in the same directory as the book file.
 
 **Logic**:
 ```go
-func (s *MediaScanner) extractMetadataFromCalibreSidecar(mediaFilePath string) (*MediaMetadata, error) {
+func (s *MediaScanner) extractCalibreSidecar(path string) *MediaMetadata {
     // Get directory of media file
-    dir := filepath.Dir(mediaFilePath)
-
-    // Check for metadata.opf
+    dir := filepath.Dir(path)
     opfPath := filepath.Join(dir, "metadata.opf")
+
+    // Check if sidecar exists
     if _, err := os.Stat(opfPath); os.IsNotExist(err) {
-        // No sidecar file - not an error, just return nil
-        return nil, nil
+        return nil  // No sidecar, not an error
     }
 
-    // Parse metadata.opf
-    return s.parseCalibreMetadataOPF(opfPath)
+    // Parse sidecar
+    metadata, err := s.parseCalibreMetadataOPF(opfPath)
+    if err != nil {
+        fmt.Printf("Warning: failed to parse Calibre metadata.opf: %v\n", err)
+        return nil  // Parsing failed, fall back to embedded
+    }
+
+    return metadata
 }
 ```
 
 **Error Handling**:
-- File not found → Return `nil, nil` (not an error)
-- Permission denied → Log warning, return `nil, nil`
-- Other errors → Log error, return `nil, err`
+- File not found → Return `nil` (not an error)
+- Permission denied → Log warning, return `nil`
+- Parse errors → Log warning, return `nil` (graceful fallback)
+
+**Note**: Simplified from original plan - no wrapper function needed
 
 ### 2. parseCalibreMetadataOPF()
 
@@ -260,40 +267,64 @@ func (s *MediaScanner) parseCalibreMetadataOPF(opfPath string) (*MediaMetadata,
 **XML Namespaces**: Handle Dublin Core (`dc:`) and Calibre (`calibre:`) namespaces properly.
 
 **Error Handling**:
-- Malformed XML → Log warning, return `nil, nil`
+- Malformed XML → Log warning, return `nil`
 - Missing required fields → Use available fields
 - Invalid date format → Log warning, skip field
 
-### 3. extractMetadataWithSidecar()
+### 3. Modified: extractMetadata()
 
-**Location**: `internal/services/media_scanner.go`
+**Location**: `internal/services/media_scanner.go` (line 712)
 
-**Signature**:
+**Signature**: (Unchanged - existing function)
 ```go
-func (s *MediaScanner) extractMetadataWithSidecar(path string) (*MediaMetadata, error)
+func (s *MediaScanner) extractMetadata(path string) (*MediaMetadata, error)
 ```
 
-**Purpose**: Implements sidecar-first metadata extraction.
+**Purpose**: Extracts metadata from book files with sidecar-first approach.
 
-**Logic**:
+**Modified Logic**:
 ```go
-func (s *MediaScanner) extractMetadataWithSidecar(path string) (*MediaMetadata, error) {
-    // Try Calibre sidecar first
-    if metadata, err := s.extractMetadataFromCalibreSidecar(path); metadata != nil {
+func (s *MediaScanner) extractMetadata(path string) (*MediaMetadata, error) {
+    // NEW: Try Calibre sidecar first
+    if metadata := s.extractCalibreSidecar(path); metadata != nil {
         fmt.Printf("Using Calibre metadata.opf for %s\n", path)
-        return metadata, nil
-    } else if err != nil {
-        // Log error but continue to embedded extraction
-        fmt.Printf("Warning: failed to read Calibre sidecar: %v\n", err)
+
+        // Try to find cover image for sidecar metadata
+        coverPath := findSidecarCover(path)
+        if coverPath != "" {
+            metadata.CoverPath = s.getRelativePath(coverPath)
         }
 
-    // Fallback to embedded metadata
-    return s.extractMetadata(path)
+        return metadata, nil
+    }
+
+    // EXISTING: Fallback to embedded metadata
+    ext := strings.ToLower(filepath.Ext(path))
+
+    switch ext {
+    case ".epub":
+        metadata, err := s.extractEPUBMetadata(path)
+        // ... existing EPUB logic
+    case ".pdf":
+        return s.extractPDFMetadata(path)
+    default:
+        return &MediaMetadata{
+            Title: strings.TrimSuffix(filepath.Base(path), ext),
+        }, nil
+    }
 }
 ```
 
+**Changes**:
+- Add sidecar check at very beginning (before switch statement)
+- If sidecar found → return immediately with sidecar metadata
+- Otherwise → continue to existing embedded extraction logic
+- **No call site changes needed** - all existing code continues to work
+
 **Priority**: Sidecar → Embedded → Folder → Filename
 
+**Note**: This is a SIMPLIFIED approach based on user feedback. Original plan called for a wrapper function, but directly modifying `extractMetadata()` is cleaner and requires less code.
+
 ## Database Schema Mapping
 
 All OPF fields map to existing `media_items` columns:
@@ -569,6 +600,27 @@ type ScannerConfig struct {
 - [Calibre Manual](https://manual.calibre-ebook.com/)
 - [Go XML Encoding](https://pkg.go.dev/encoding/xml)
 
+## Implementation Note: Simplified Approach
+
+**Original Plan**: Create a new wrapper function `extractMetadataWithSidecar()` that calls the existing `extractMetadata()`.
+
+**Revised Plan** (based on user feedback): Modify `extractMetadata()` directly to check for sidecar files at the beginning of the function.
+
+**Benefits of Simplified Approach**:
+- **Less code**: ~150 lines vs. ~250 lines
+- **No call site changes**: All existing code continues to work without modification
+- **Clearer flow**: Single entry point for metadata extraction
+- **Better testability**: Test `extractMetadata()` with/without sidecar in one place
+- **Easier to maintain**: All metadata extraction logic in one function
+
+**What Changed**:
+- No new wrapper function needed
+- Only 2 new functions: `extractCalibreSidecar()` and `parseCalibreMetadataOPF()`
+- Single modification to existing `extractMetadata()` function
+- Simpler architecture, easier to understand
+
+This is a great example of how user feedback can improve implementation design!
+
 ## See Also
 
 - [User Guide: Calibre Integration](../../user/calibre-integration.md)