Mrixs/NaviWatcher
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: complete task 3 - fix sync.go cache-hit path to use centralized filter

Browse Source
This commit is contained in:
Vladimir Zagainov
2026-07-25 23:06:56 +03:00
parent 9e4d2385ff
commit 41e5e079d4
2 changed files with 8 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
docs/plans/2026-07-21-fix-review-findings.md
View File

@@ -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
- [ ] Note the filter centralization pattern in CLAUDE.md if new pattern established