Vladimir Zagainov
9fc89fd5a4
Confirm normalization + 0.85 threshold, IsIgnored skip, per-artist scoping, and zero-threshold→default contract. Added TestScanArtist_ZeroThresholdUsesDefault, fixed gofmt on two test files. Coverage: scanner 90.3%, normalize 100.0%.
119 lines
11 KiB
Markdown
119 lines
11 KiB
Markdown
# Scanner Engine: Fuzzy Diff (local vs external releases)
|
||
|
||
## Overview
|
||
- Implement the **Scanner Engine** — the missing core of NaviWatcher. It compares a user's local albums (from Navidrome, stored in `local_albums`) against an artist's external discography (from MusicBrainz, stored in `external_releases`) and returns the list of **missing releases** (external releases with no sufficiently similar local album).
|
||
- Problem solved: without this, `main.run()` is empty and the service cannot fulfil its stated purpose (find missing albums and notify). This plan delivers only the computation core; persistence, notifier, and Web UI are explicitly out of scope.
|
||
- Integrates with existing data layer: reads `database.LocalAlbum` and `database.ExternalRelease`, reuses normalization logic, and consumes `config.Scanner.FuzzyThreshold` (default 0.85).
|
||
|
||
## Context (from discovery)
|
||
- Files/components involved:
|
||
- `internal/database/local_albums.go` — `LocalAlbum{ID, ArtistID, Title}`, accessors `GetLocalAlbumsByArtist`, `GetAllLocalAlbums`, `GetLocalAlbums` (to confirm names during impl).
|
||
- `internal/database/external_releases.go` — `ExternalRelease{RGID, ArtistID, Title, Type, ReleaseDate, IsIgnored, CachedAt}`, accessors `GetExternalReleasesByArtist`, `GetIgnoredReleases`.
|
||
- `internal/database/database.go:176-191` — struct definitions.
|
||
- `internal/config/config.go:50-54` — `ScannerConfig{FuzzyThreshold, IgnoreBootlegs, IncludeCompilations}`.
|
||
- `internal/musicbrainz/api.go:132-165` — existing `NormalizeString` / `NormalizeArtistName` (regexes precompiled at init).
|
||
- `cmd/naviwatcher/main.go` — `App` struct, `NewApp`, empty `run()`.
|
||
- `go.mod` — **no fuzzy library present**; `lithammer/fuzzysearch` must be added.
|
||
- Related patterns found:
|
||
- MusicBrainz provider uses `ctx.Err()` checks before/within loops, `fmt.Errorf("...: %w", err)` wrapping, `db.Begin()`/`defer tx.Rollback()`/`tx.Commit()`, and table-driven white-box tests with `newTestDB(t, ":memory:")` + `seedArtist` fixtures.
|
||
- Existing `NormalizeString` already covers: lowercase, strip `[...]`/`(...)`, strip years `(1|2)xxx`, strip non-alphanumerics, collapse spaces. Bracket stripping removes keywords like Deluxe/Anniversary/Expanded regardless of a keyword list.
|
||
- Dependencies identified:
|
||
- New dep: `github.com/lithammer/fuzzysearch` (specified in Specification.md §2).
|
||
- New package: `internal/normalize` (extracted from `musicbrainz.NormalizeString`).
|
||
- New package: `internal/scanner` (the engine).
|
||
|
||
## Development Approach
|
||
- **Testing approach**: TDD — write/extend tests alongside every task's code.
|
||
- Complete each task fully (code + tests passing) before moving to the next.
|
||
- **CRITICAL: every task MUST include new/updated tests** for code changes in that task (success + error/edge cases).
|
||
- **CRITICAL: all tests must pass before starting next task** — no exceptions.
|
||
- Update this plan file when scope changes; mark items `[x]` immediately on completion.
|
||
- Reuse existing test helpers (`newTestDB`, `seedArtist`, table-driven style) and the same error-wrapping/context conventions.
|
||
|
||
## Testing Strategy
|
||
- **Unit tests** (required per task): normalization, similarity scoring, and the scanner diff are all pure functions — ideal for table-driven tests with no DB. Scanner diff against DB uses `:memory:` SQLite + `seedArtist` fixtures, mirroring `musicbrainz/sync_test.go`.
|
||
- No UI/e2e in this plan (Web UI is out of scope).
|
||
|
||
## Progress Tracking
|
||
- Mark completed items with `[x]` immediately when done.
|
||
- Add newly discovered tasks with ➕ prefix.
|
||
- Document blockers with ⚠️ prefix.
|
||
- Keep plan in sync with actual work done.
|
||
|
||
## What Goes Where
|
||
- **Implementation Steps** (`[ ]`): all code + test tasks below.
|
||
- **Post-Completion** (no checkboxes): manual/integration verification notes.
|
||
|
||
## Implementation Steps
|
||
|
||
### Task 1: Add fuzzysearch dependency
|
||
- [x] run `go get github.com/lithammer/fuzzysearch@latest` and confirm it appears in `go.mod`/`go.sum`
|
||
- [x] run `go mod tidy` and verify the build still compiles (`go build ./...`)
|
||
- [x] write a trivial smoke test (or rely on Task 3's first test) confirming `fuzzy.Ratio` is importable and returns expected ordering
|
||
- [x] run tests — must pass before task 2
|
||
|
||
> Note: the library exposes `fuzzy.RankMatch` (subsequence-ranked Levenshtein distance: 0=exact, -1=no match) rather than a `fuzzy.Ratio` 0-100 function assumed in the plan. Task 3 will normalize this into a 0.0-1.0 similarity score.
|
||
|
||
### Task 2: Extract normalization into `internal/normalize`
|
||
- [x] create `internal/normalize/normalize.go` with `NormalizeString(s string) string` and `NormalizeArtistName(s string) string`, moving the regexes + logic from `internal/musicbrainz/api.go:132-165`
|
||
- [x] refactor `internal/musicbrainz/api.go` to call `normalize.NormalizeString` / `normalize.NormalizeArtistName` instead of its local copies (remove duplicated regexes/functions)
|
||
- [x] write tests `internal/normalize/normalize_test.go` (table-driven): lowercase, bracket/paren strip, year strip `(20xx)`, special-char strip, space collapse, `NormalizeArtistName` prefix strip (`the `/`a `/`an `)
|
||
- [x] update existing `internal/musicbrainz/api_test.go` if it referenced the moved functions, ensuring it still passes
|
||
- [x] run tests — must pass before task 3
|
||
|
||
### Task 3: Implement similarity scoring in `internal/scanner`
|
||
- [x] create `internal/scanner/scanner.go` with `Similarity(a, b string) float64` using `normalize.NormalizeString` + `fuzzy.LevenshteinDistance` (normalized to 0.0–1.0); define `IsMatch(a, b string, threshold float64) bool`
|
||
- [x] write tests `internal/scanner/scanner_test.go` (table-driven): exact match → 1.0, `(Remastered)` / year variants still match above 0.85, clearly different titles → below threshold, empty-string handling
|
||
- [x] run tests — must pass before task 4
|
||
|
||
### Task 4: Implement the diff engine (missing-release detection)
|
||
- [x] add `type MissingRelease struct { RGID, ArtistID, Title, Type, ReleaseDate string }` in `internal/scanner`
|
||
- [x] implement `FindMissingReleases(local []database.LocalAlbum, external []database.ExternalRelease, threshold float64) []MissingRelease`:
|
||
- skip external releases where `IsIgnored == true`
|
||
- for each external release, check if any local album (same ArtistID) is a match via `IsMatch`; if none matches, it is missing
|
||
- respect context cancellation if signature uses `ctx` (decide in impl; pure slice version preferred for testability)
|
||
- [x] write tests `internal/scanner/scanner_test.go` (table-driven, using in-memory DB fixtures or hand-built slices): no local albums → all external are missing; exact title present → not missing; fuzzy title present (e.g. `The Wall` vs `The Wall (Remastered)`) → not missing; ignored external → never reported; different ArtistID → not matched across artists; threshold boundary (0.85) behaviour
|
||
- [x] run tests — must pass before task 5
|
||
|
||
### Task 5: Wire a DB-backed scanner entrypoint + `main.go` hook (compute-only)
|
||
- [x] add `func ScanArtist(ctx context.Context, db *database.DB, artistID string, threshold float64) ([]MissingRelease, error)` that loads local + external by artist via `database.GetLocalAlbumsByArtist` / `database.GetExternalReleasesByArtist` and calls `FindMissingReleases`
|
||
- [x] add `func ScanAll(ctx context.Context, db *database.DB, threshold float64) ([]MissingRelease, error)` iterating monitored artists (reuse `database.GetAllArtistSettings`) with `ctx.Err()` checks between artists
|
||
- [x] write tests `internal/scanner/scan_test.go` using `newTestDB(t)` + `seedArtist` + seeded `local_albums`/`external_releases` rows; assert missing set matches expectations; test ctx-cancellation returns early
|
||
- [x] extend `cmd/naviwatcher/main.go` `App` struct + `NewApp` to construct the scanner (or keep stateless) and add a compute-only call in `run()` (e.g. log count of missing releases for monitored artists) without starting notifier/web — keep `run()` non-blocking / goroutine-safe per spec concurrency note
|
||
- [x] write/extend `cmd/naviwatcher/main_test.go` if `App`/wiring changed
|
||
- [x] run full test suite (`go test ./...`) and `go vet ./...` — must pass before final task
|
||
|
||
### Task 6: Verify acceptance criteria
|
||
- [x] verify `FindMissingReleases`/`ScanArtist`/`ScanAll` meet spec: normalization + 0.85 fuzzy threshold, ignore `IsIgnored`, per-artist scoping (confirmed via tests in scanner_test.go / scan_test.go; grep of diff.go + scan.go shows normalizing via normalize.NormalizeString, IsIgnored skip, ArtistID grouping)
|
||
- [x] verify `config.Scanner.FuzzyThreshold` default 0.85 is used when threshold arg is zero (resolveThreshold in scanner.go returns DefaultThreshold=0.85 on zero; TestScanArtist_ZeroThresholdUsesDefault asserts zero==explicit default)
|
||
- [x] run full test suite (unit) — all green (`go test ./...` passes)
|
||
- [x] run `go vet ./...` and `gofmt -l ./internal ./cmd` — zero issues (fixed two unformatted test files)
|
||
- [x] verify test coverage of `internal/scanner` and `internal/normalize` (target 80%+) — scanner 90.3%, normalize 100.0%
|
||
|
||
### Task 7: Update documentation
|
||
- [ ] update `README.md` to note the Scanner Engine is implemented (compute-only; notifier/web pending)
|
||
- [ ] add a short note in `CLAUDE.md` or a plan-completion comment if new package conventions (e.g. `internal/normalize` is the shared normalization home) were established
|
||
|
||
## Technical Details
|
||
- **Normalization** (`internal/normalize`): port regexes from `musicbrainz/api.go`:
|
||
- `bracketRe` = `\[[^\]]*\]` , `parenRe` = `\([^)]*\)`, `yearRe` = `\b(1[0-9]{3}|2[0-9]{3})\b`, `spaceRe` = `\s+`.
|
||
- `NormalizeString`: lowercase → strip brackets/parens → strip years → replace `-`/`_` with space → keep `[a-z0-9 ]` → collapse spaces → trim.
|
||
- `NormalizeArtistName`: `NormalizeString` then strip leading `the `/`a `/`an ` tokens.
|
||
- **Similarity**: `fuzzy.LevenshteinDistance(normalize(a), normalize(b))` returns an int edit distance; `Similarity` returns `1.0 - float64(dist)/float64(maxLen)` (clamped to [0.0, 1.0]). Empty/whitespace-only inputs normalize to empty and score 0.0 (no false match). `IsMatch` returns `Similarity(a,b) >= threshold`. Note: `fuzzy.Ratio` does not exist in `lithammer/fuzzysearch` v1.1.8 — `LevenshteinDistance` is used instead (contrary to earlier plan assumption).
|
||
- **Diff algorithm**: per external release (filtered by `!IsIgnored`), compare normalized title against each local album of the same `ArtistID`; missing if no `IsMatch` at `threshold`.
|
||
- **Config contract**: `threshold` passed from `config.Scanner.FuzzyThreshold` (default 0.85). Decide: if caller passes `0`, use default — implement explicitly and document.
|
||
- **No new DB tables** in this plan (compute-only per user decision).
|
||
|
||
## Post-Completion
|
||
*Informational only — no checkboxes.*
|
||
|
||
**Manual verification** (optional, requires live Navidrome + MusicBrainz cache):
|
||
- Run the binary with a real `config.yaml`, observe `run()` log line reporting missing-release counts for monitored artists.
|
||
- Confirm no false positives for `(Remastered)` / year-suffixed local titles.
|
||
|
||
**Follow-up (out of scope, future plans)**:
|
||
- Persist `MissingRelease` into a new `missing_releases` table for notifier/Web UI.
|
||
- Implement Notifier (Telegram bot + cron) consuming scanner output.
|
||
- Implement Web UI (dashboard / artist / archive) with basic-auth and `//go:embed` templates.
|
||
- Replace the compute-only `run()` hook with full goroutine orchestration (scanner + notifier + web).
|