diff --git a/docs/plans/2026-07-21-fix-review-findings.md b/docs/plans/2026-07-21-fix-review-findings.md index 777b40b..d2dac91 100644 --- a/docs/plans/2026-07-21-fix-review-findings.md +++ b/docs/plans/2026-07-21-fix-review-findings.md @@ -33,8 +33,8 @@ Problem: Three separate filter implementations (`musicbrainz.FilterReleaseGroups ## Progress Tracking - Mark completed items with `[x]` immediately when done -- Add newly discovered tasks with ➕ prefix -- Document issues/blockers with ⚠️ prefix +- Add newly discovered tasks with `➕` prefix +- Document issues/blockers with `⚠️` prefix ## Implementation Steps @@ -50,10 +50,10 @@ Problem: Three separate filter implementations (`musicbrainz.FilterReleaseGroups - [x] Run tests - must pass before task 3 ### Task 3: Fix sync.go cache-hit path to use centralized filter -- [ ] Update `SyncArtistDiscography` cache-hit branch (lines 93-115) to call the shared filter helper instead of inline logic -- [ ] Ensure `opts` from `getArtistFilterOptions` is passed correctly -- [ ] Write test in `sync_test.go` that verifies cache-hit path produces identical filter results as cache-miss path for same `FilterOptions` and release data -- [ ] Run tests - must pass before task 4 +- [x] Update `SyncArtistDiscography` cache-hit branch to call the shared filter helper instead of inline logic +- [x] Ensure `opts` from `getArtistFilterOptions` is passed correctly +- [x] Write test in `sync_test.go` that verifies cache-hit path produces identical filter results as cache-miss path for same `FilterOptions` and release data +- [x] Run tests - must pass before task 4 ### Task 4: Fix scanner diff.go TypeFilter.suppressed to use centralized filter - [ ] Update `TypeFilter.suppressed` in `diff.go` to use the same logic as `ApplyTypeToggles` (i.e., treat `EP` in SecondaryTypes as a Single when `IgnoreSingles=true`) @@ -104,33 +104,4 @@ Problem: Three separate filter implementations (`musicbrainz.FilterReleaseGroups ### Task 11: Update documentation - [ ] Update README.md if any new behavior or config documented -- [ ] Note the filter centralization pattern in CLAUDE.md if new pattern established - -## Technical Details - -### Filter Logic Canonical Form -```go -// IgnoreSingles filters: Type == "Single" OR SecondaryTypes contains "Single" OR "EP" -// IgnoreCompilations filters: Type == "Compilation" OR SecondaryTypes contains "Compilation" -func matchesIgnoreSingles(r ExternalRelease) bool { - return r.Type == "Single" || hasSliceType(r.SecondaryTypes, "Single", "EP") -} -func matchesIgnoreCompilations(r ExternalRelease) bool { - return r.Type == "Compilation" || hasSliceType(r.SecondaryTypes, "Compilation") -} -``` - -### Files to Modify -1. `internal/musicbrainz/api.go` — export `hasSliceType`, `FilterOptions`; add `ApplyTypeToggles` or refactor `FilterReleaseGroups` -2. `internal/musicbrainz/sync.go` — use shared filter in cache-hit path -3. `internal/scanner/diff.go` — import and use shared filter -4. `internal/database/external_releases.go` — migration + integer timestamp logic -5. `internal/database/database.go` — add migration `006` -6. `internal/musicbrainz/sync.go` — batch stale notification deletion -7. `internal/scanner/scan.go` — handle `ErrArtistNotFound` -8. `internal/notifier/scheduler.go` — composite map key -9. `internal/database/artist_settings.go` — optimize upsert - -## Post-Completion -- Manual verification: run against real Navidrome + MusicBrainz, confirm dashboard/notifications show consistent results regardless of cache state -- No external system updates required \ No newline at end of file +- [ ] Note the filter centralization pattern in CLAUDE.md if new pattern established \ No newline at end of file diff --git a/internal/musicbrainz/sync.go b/internal/musicbrainz/sync.go index 85d5144..5738ba7 100644 --- a/internal/musicbrainz/sync.go +++ b/internal/musicbrainz/sync.go @@ -65,16 +65,7 @@ func SyncArtistDiscography( if err != nil { return nil, fmt.Errorf("sync artist discography: read artist filter options: %w", err) } - filtered := make([]database.ExternalRelease, 0, len(cachedReleases)) - for _, r := range cachedReleases { - if opts.IgnoreSingles && (r.Type == "Single" || hasSliceType(r.SecondaryTypes, "Single")) { - continue - } - if opts.IgnoreCompilations && (r.Type == "Compilation" || hasSliceType(r.SecondaryTypes, "Compilation")) { - continue - } - filtered = append(filtered, r) - } + filtered := ApplyTypeToggles(cachedReleases, opts) return filtered, nil }