musicbrainz-provider #2

Merged

Mrixs merged 72 commits from musicbrainz-provider into master 2026-08-05 20:19:16 +00:00

Conversation 0 Commits 72 Files Changed 69 +10029 -127

Mrixs commented 2026-08-05 19:57:22 +00:00

Owner

Copy Link

No description provided.

Mrixs added 72 commits 2026-08-05 19:57:23 +00:00

Add MusicBrainz provider plan and ignore coverage output 60ecc3f904

feat: create MusicBrainz client and data models ... 674daed93b

Add internal/musicbrainz/ package with:
- client.go: MusicBrainzClient struct wrapping net/http.Client with
  channel-based rate limiter (1 req/sec), doGet method with proper
  User-Agent header, and Close for cleanup
- model.go: ReleaseGroup, Artist, ExternalRelease, and Parsed* structs
- XML parsing functions for release-group list and artist responses
- Comprehensive tests: XML parsing (success, empty, malformed),
  client constructor, doGet (success, non-200, unreachable server),
  rate limiter behavior

feat: implement rate limiting and caching layer for MusicBrainz provider ... b0f69d3a4f

- Add golang.org/x/time/rate dependency for token-bucket rate limiting
- Replace custom channel-based rate limiter with rate.NewLimiter(1, 1)
- Add context.Context support to doGet for cancellation
- Add cached_at column to external_releases via migration 005
- Implement cache hit/miss queries with TTL-based filtering
- Add CacheStats type for tracking cached RGIDs
- Update ExternalRelease struct with CachedAt field
- Add rate limiting tests (1 req/sec enforcement, burst behavior)
- Add cache tests (hit, miss, expired, mixed, empty artist)
- Update migration count test for new migration

feat: implement MusicBrainz API endpoints, filtering, and normalization ... e624bb0eaf

- Add GetArtistReleaseGroups method with pagination support
- Implement status filtering (exclude Bootleg/Promotion/Pseudo-Release)
- Implement type filtering (include Album/Single/EP/Compilation)
- Add per-artist type filtering hooks (ArtistTypeFilter) for Web UI
- Add NormalizeString and NormalizeArtistName for fuzzy matching prep
- Add ReleaseGroup.ToExternalRelease conversion method
- Write comprehensive table-driven tests for filtering logic
- Write tests for normalization functions (18 cases)
- Write tests for GetArtistReleaseGroups (success, pagination, empty, errors)
- All tests pass (47 total across project), go vet clean

feat: implement MusicBrainz sync orchestration with caching and upsert ... 15b05b57fb

Creates internal/musicbrainz/sync.go with SyncArtistDiscography function
that orchestrates the full flow: cache check via GetCachedReleases, fetch
from MusicBrainz API on cache miss, filter via FilterReleaseGroups, and
upsert into external_releases via database.SaveExternalRelease. Includes
SyncArtistDiscographyWithFilter variant for per-artist type filtering. All
functions support context cancellation. 14 new tests cover cache hit/miss,
status/type filtering, context cancellation, idempotency, API errors, and
full XML pipeline integration.

feat: wire up MusicBrainz provider in application entry point ... da49d12bb6

- Add App struct with Config, DB, and MusicBrainz client dependencies
- NewApp() initializes database and MusicBrainz client from config
- App.Close() cleans up resources (MB client + DB) on shutdown
- App.run() blocks until context cancelled (goroutine-ready for future tasks)
- Config validation already requires musicbrainz.user_agent
- Add 6 tests: graceful shutdown, config integration, NewApp creation,
  shutdown cleanup, app.run shutdown, UserAgent validation
- All tests pass (6/6 in cmd, full suite green)

feat: verify MusicBrainz provider acceptance criteria ... 34ea84fc77

- All 6 tasks complete: client/models, rate limiting/caching, API
  endpoints/filtering, sync orchestration, app wiring, verification
- Full test suite passes (all packages green)
- go vet clean, no issues
- Coverage: musicbrainz 92.5%, config 100%, navidrome 79.5%, database 53.1%
- Edge cases verified: HTTP errors, rate limit blocking, context
  cancellation, NULL handling, XML parse errors

fix: address code review findings ... a5911c257c

Remove dead code: duplicate ExternalRelease/Artist/ParsedArtist structs in
model.go, ParseArtist/mbArtist/mbArtistData in client.go, ArtistTypeFilter
and related filtering functions in api.go, SyncArtistDiscographyWithFilter
in sync.go, and CacheStats/IsArtistCacheValid in cache.go.

Fix bugs: SaveExternalRelease now stores NULL instead of empty string for
zero CachedAt; sync upserts are now transactional with stale release cleanup;
getCachedReleases returns int instead of *CacheStats; doGet uses url.Values
for proper query encoding of MBID.

Fix tests: removed duplicate TestRun_GracefulShutdown, removed dead code
(_ = dbPath) from TestNewApp, fixed assertions in httptest handler goroutine
to avoid data race, increased rate limiter timing tolerance, removed
Client.Close() calls (no-op removed), fixed sync test cache expiry to use
UPDATE instead of 0 TTL races.

Fix formatting: cancel()}() formatting in main.go, error format string in sync.go.

fix: address second code review findings ... 2baf586607

- Add CacheTTL default (24h) in config applyDefaults — without this,
  omitting cache_ttl from config silently defeats the entire caching
  mechanism (TTL=0 means cached data is never served)
- Fix pagination to use total Count instead of checking if last page
  was short — avoids wasting a rate-limit token when total count is
  an exact multiple of 100
- Preserve user-set IsIgnored flags across re-syncs — previously,
  DELETE+INSERT in the sync transaction reset all ignore flags to
  false, losing user preferences on every cache-expiry re-sync
- Check context cancellation on cache-hit code path — previously,
  ctx.Err() was not checked between cache check and returning cached
  data, violating the cancellation contract

fix: address second code review findings ... 582202bb86

- Add context cancellation check between pagination pages in GetArtistReleaseGroups
  for responsive graceful shutdown during large discography fetches.
- Eliminate double DB query on cache hit by having GetCachedReleases return
  []ExternalRelease directly instead of just a count, avoiding a redundant
  second query in SyncArtistDiscography.
- Update cache_test.go to match new GetCachedReleases return type.
- Format main.go (pre-existing whitespace issue).

fix: address third code review findings ... 5d52a09868

- Prevent infinite pagination loop when API returns empty release groups page
- Move NormalizeString regexes to package level to avoid recompilation on every call
- Use UTC consistently for cached_at timestamps to avoid DST-related TTL skew

fix: address fourth code review findings ... 424be1efc4

- Fix FK constraint violation in SyncArtistDiscography: delete
  notifications_sent rows before external_releases to prevent
  constraint failure when re-syncing artists with prior notifications.
- Implement per-artist type filtering: FilterReleaseGroups now accepts
  FilterOptions with IgnoreSingles/IgnoreCompilations flags, read from
  artist_settings table via getArtistFilterOptions.
- Fix inconsistent error wrapping: GetExternalRelease now wraps errors
  with fmt.Errorf like all other functions in the package; updated test
  to use errors.Is for sql.ErrNoRows check.
- Add tests: FilterReleaseGroups ignore singles/compilations,
  SyncArtistDiscography per-artist type filtering, and FK-safe resync.

feat: add fuzzysearch dependency for scanner engine c95c740cd5

feat: extract normalization into internal/normalize package edf9c1d1f8

feat: implement scanner similarity scoring 92aafaab05

feat: implement scanner diff engine (missing-release detection) ... a4f0460664

Add FindMissingReleases and MissingRelease to internal/scanner: skips
IsIgnored external releases, scopes matches per ArtistID, and reports
external releases with no local album above the fuzzy threshold.

feat: add DB-backed scanner entrypoint and compute-only main hook ... c2da8c2095

Implements ScanArtist/ScanAll in internal/scanner loading local/external
releases via the database layer with ctx-cancellation checks, plus a
compute-only run() hook that logs missing-release counts. Add table-driven
tests using in-memory SQLite fixtures.

feat: verify scanner engine acceptance criteria ... 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%.

feat: document Scanner Engine implementation status and normalize convention 06e09220ae

fix: address code review findings ... 8a5b58a817

- Fix artist-ID namespace mismatch in MusicBrainz provider: SyncArtistDiscography
  now stores the canonical Navidrome artist ID (artist_settings.id) as
  external_releases.artist_id instead of the MusicBrainz MBID. Previously the
  MBID was stored, which violated the FK to artist_settings and broke the
  scanner join (local_albums.artist_id is the Navidrome ID), causing every
  external release to be falsely reported as missing and the sync insert to
  fail at runtime. getArtistFilterOptions now also resolves by the Navidrome ID.
- Resolve threshold in FindMissingReleases so the exported primitive honors the
  same zero-means-default contract as ScanArtist/ScanAll.
- Remove dead maxLen==0 guard in scanner.Similarity.
- Inline trivial buildPath helper; drop unused url import in client.go.
- Replace hand-rolled itoa with strconv.Itoa in tests.
- Rewrite SyncArtistDiscography tests to seed artist_settings with the Navidrome
  ID (tests previously seeded the MBID to mask the FK mismatch).
- Fix TestFuzzySmoke to exercise the real dependency (fuzzy.LevenshteinDistance /
  scanner.Similarity) instead of an unused API.
- Fix TestAppRun_ScanLogsMissingReleases to run the scan against a live context
  and assert the missing release is found.
- Document cached_at column in Specification.md and note startup scan / required
  musicbrainz.user_agent in README.
- Stop tracking .serena/ tooling config; add it to .gitignore.

fix: address code review findings a4c426f640

fix: address code review findings c70f46af27

fix: address code review findings f1839ad9e7

fix: address code review findings 401c1218b6

fix: address code review findings beef81d598

fix: address code review findings a7803615cd

fix: address code review findings 0bee9b9b27

fix: address code review findings ... b21bf07208

- Store cached_at in canonical UTC layout so the cache TTL cutoff comparison is a valid time ordering (previously go-sqlite3 serialized time.Time as RFC3339, making the space-separated cutoff match only by ASCII accident; same-day expired entries were falsely served as fresh).

- Remove stale no-op scanner config keys (ignore_bootlegs, include_compilations) from config.yaml.example and docs; these fields were removed from ScannerConfig but left in configs, silently doing nothing.

fix: address code review findings 4da5ee5f8c

fix: address code review findings (consolidate slice helpers, correct bootleg comment) da8b8aa944

fix: address code review findings 2468859435

fix: guard against whitespace-only type attribute panic in ParseReleaseGroups ab2af62cb0

fix: address code review findings ... e2de91c5d3

- Fix pagination to not trust MusicBrainz count for cutoff (prevents
  silent discography truncation / hidden missing releases)
- Add idx_external_releases_artist_id index for hot per-artist queries
- Dedupe cached_at layout constant in external_releases.go

fix: address code review findings ... 79a376a127

- Strip standalone reissue keywords (remaster/remastered/remix/deluxe/
  expanded/edition/reissue/anniversary/bonus) regardless of brackets so
  non-parenthesized remasters still match the plain local title above the
  0.85 threshold (was falsely reported missing).
- Fix TestAppRun_ScanLogsMissingReleases to verify run() performs the
  scan itself (capture its log output) instead of re-running ScanAll
  independently, which passed even if run() were a no-op.
- Fix TestSimilarity misleading cases that downgraded to a <0.85 range
  check with dead want/epsilon fields; assert actual computed scores.

move completed plan: 2026-07-19-scanner-engine-fuzzy-diff.md 35d6a5234f

feat: add mbid column to artist_settings ... 85c42ec858

Additive migration 008 adds an mbid column to artist_settings, extend the
ArtistSettings struct and persistence functions (SaveArtistSettings,
GetArtistSettings, GetAllArtistSettings, UpdateArtistSettings) to carry the
MusicBrainz ID, and add round-trip tests covering empty and set MBID values.

feat: add MusicBrainz artist-ID resolver ... 40c4240693

Add ResolveArtistMBID to query the MB artist search endpoint and return the
first matching artist ID, with httptest-backed tests for match, no-match,
HTTP error, and invalid-JSON paths.

feat: add SyncAll periodic sync pipeline and wire Navidrome client into App dc4bdcdab0

feat: wire main loop with periodic sync+scan and sync_interval config ... 0635ca8a87

Replace compute-only run() with an immediate sync+scan followed by a
ticker-driven periodic loop, add the sync.interval config field (default
6h) with defaults and validation, and add tests for the scheduling logic.

feat: add notifier Sender interface and digest formatter 3af33bd728

feat: add notifier scheduler and NotifyOnce sent-tracking 11f838ace9

feat: add Web UI server with basic-auth and artist dashboard cea20957e7

feat: add Web UI artist detail, archive, and ignore actions ... 44f3b0a2a7

Implements Task 8: artist detail page (local albums + found-missing
with ignore buttons), ignored-releases archive with restore, and POST
handlers toggling ignore flags and ignore_singles. Adds ErrArtistNotFound
sentinel so callers can distinguish missing artists, and wires routes via
the enhanced ServeMux path wildcard.

feat: verify acceptance criteria and document sync config ... 1a644dede7

Add a scan->notify->web end-to-end smoke test against a seeded :memory: DB,
fix .gitignore so the cmd/naviwatcher source dir is not excluded by the
root binary rule, and document the new sync.interval field in config.yaml.example.

feat: document wired sync pipeline, notifier, and web UI in README e493a4d228

fix: address code review findings ... 389d177d85

- Fix duplicate Telegram notifications: SyncArtistDiscography no longer wipes
  notifications_sent for the whole artist on every cache-miss re-sync; only
  markers for releases that disappear are pruned (FK-safe via INSERT OR REPLACE
  + rgid NOT IN (...)).
- Cache empty MusicBrainz discographies via a new artist_settings.last_synced
  column (migration 009) so zero-release artists honor the TTL instead of being
  re-fetched every cycle.
- Wire the Web UI server and Telegram notifier scheduler into main.run/NewApp.
- Guard startPeriodicSync against overlapping syncs with a done-channel slot.
- Add server.public_url config; NewServerWithConfig derives reachable links
  and no longer advertises the 0.0.0.0 bind address.
- Web handlers: use scanner.ScanArtist per artist, drop always-false
  releaseIgnored lookup and dead endsWith, thread configured threshold.
- Limit :memory: DB pool to one connection so migrations and queries share the
  same in-memory store.

fix: address code review findings ... 7cdb473d9c

- notifier: show artist display names (not internal IDs) in digest; resolve
  names from artist_settings and fall back to ID when unavailable
- notifier: skip sending an empty digest to avoid daily spam
- config: require telegram token/chat_id when enabled
- web: warn loudly when auth is disabled on a non-loopback bind; add HTTP
  server timeouts
- web: treat SetReleaseIgnored "release not found" as benign redirect (0 rows)
- musicbrainz: reject low-score/name-mismatched MBID resolutions instead of
  silently caching the wrong artist
- database: remove dead duplicate err check; harden DSN param appending
- musicbrainz: check rows.Err() after iterating existing releases

fix: address code review findings ... ce1c39e14b

- Preserve MBID and last_synced across periodic artist syncs (INSERT OR
  REPLACE was wiping them, forcing MusicBrainz re-resolution every cycle)
- Exclude unmonitored artists from unnotified-release digest query
- Require telegram.cron_schedule when telegram.enabled is true
- Add same-origin CSRF defense to state-changing web POST routes
- Skip WAL/busy_timeout pragmas for :memory: databases (no-op there)
- Scan mbid as sql.NullString in GetAllArtistSettings to tolerate NULLs

fix: address code review findings e0211343e0

fix: link dashboard artist names to detail pages ... f5b0034b4d

Task 8's ignore/restore/ignore-singles actions live on the artist detail
page, but the dashboard rendered artist names as plain text with no link,
making those actions unreachable through normal UI navigation. Wrap the
name in an anchor to /artist/{id} and assert the link in the dashboard test.

fix: address code review findings ... a8aa445d94

- Honor ignore_singles/ignore_compilations at scanner read time so toggles
  take effect immediately on the dashboard, artist page, and digest instead
  of waiting for the MusicBrainz cache to expire and prune rows.
- Run notifier notify synchronously in the scheduler loop to avoid overlapping
  read-send-mark runs double-sending the digest.
- Show artist name (with ID fallback) on the archive page instead of raw IDs.
- Select last_synced in GetAllArtistSettings for contract consistency.
- Fix stale startPeriodicSync comment and remove redundant error var.
- Remove dead ignored-branch from the artist template (never rendered).
- Add tests: CSRF sameOrigin, ArtistCacheFresh, secondary_types round-trip,
  and scanner type-toggle filtering.
- Update Specification.md schema/config to reflect mbid, last_synced,
  secondary_types, sync.interval, and server.public_url.

fix: address code review findings ... aee0241bb7

- Start Web UI before the blocking initial sync so the dashboard is
  reachable during the (rate-limited, potentially multi-minute) first
  sync; fold the immediate sync into startPeriodicSync's overlap guard
  so it can never race a concurrent tick over the shared DB / MB client.
- Make MarkNotificationSent idempotent: INSERT OR IGNORE for same-second
  PK collisions, and explicitly swallow FK violations when a release was
  pruned by a concurrent re-sync. Prevents a single vanished/duplicate
  release from aborting the digest mark-sent loop and re-sending.
- Do not abort NotifyOnce's mark-sent loop on a single failure; log and
  continue so every release in the batch is marked.
- NULL-safe reads: COALESCE(type,''), COALESCE(release_date,'') in the
  external_releases and unnotified readers to match the cache reader.
- Update/extend tests for the new idempotency and startup contracts.

feat: complete task 2 - update FilterReleaseGroups to use centralized filter helper 9e4d2385ff

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

feat: complete task 4 - fix scanner diff.go TypeFilter.suppressed to use centralized filter 355a12c6a6

feat: complete task 6 - fix stale notification pruning to avoid SQLite parameter limit 4b4e852fd1

feat: handle ErrArtistNotFound in ScanArtist gracefully by using empty TypeFilter 395a7f9b07

feat: fix NotifyOnce map key to use composite ArtistID+RGID f697ebf2f5

feat: complete filter centralization pattern and document in CLAUDE.md aec9d89435

feat: update documentation to note filter centralization pattern completion 6e49f436c3

move completed plan: 2026-07-21-fix-review-findings.md e95f581c80

move completed plan: 2026-07-26-fix-scanner-wiring.md 348d0648ba

feat: write unit tests for filtering logic 35b1f466ec

feat: verify wiring of ignore_singles/ignore_compilations toggles - completed Task 1 827b5405e5

feat: document and enhance scanner wiring verification - added filter flow documentation and code comments c2f3258186

move completed plan: 2026-07-27-verify-scanner-wiring.md f49ad40002

feat: complete Task 1 - update database schema and model for live/remix filtering 75326cda3a

feat: complete Task 2 - update filtering core logic for live/remix filtering a3b8aa8a74

feat: complete Task 3 - update web UI and handlers for Live/Remix filtering 0dcc86015d

feat: complete Task 4 - verify and document live/remix filtering implementation 4d93218132

move completed plan: 2026-07-30-live-remix-filtering.md ca8d503c50

feat: implement Live/Remix filtering and add CI/CD pipeline ... e73b17673e

This commit includes:

1. Live/Remix Filtering Feature:
   - Added ignore_live and ignore_remix columns to artist_settings table (migration 010)
   - Updated ArtistSettings struct with IgnoreLive and IgnoreRemix fields
   - Modified SaveArtistSettings and UpdateArtistSettings to handle new fields
   - Extended FilterOptions struct with IgnoreLive and IgnoreRemix
   - Updated ApplyTypeToggles and ApplyTypeTogglesToReleaseGroups to filter Live/Remix types
   - Added toggleIgnoreLive and toggleIgnoreRemix handlers in web layer
   - Updated ArtistData view model and artist.html template with new toggle UI
   - Comprehensive test coverage for all new functionality

2. CI/CD Pipeline with Gitea Actions:
   - Added .gitea/workflows/docker-build.yml for automated Docker builds
   - Workflow triggers on pushes to main/master and tags, plus PRs
   - Runs Go tests before building
   - Builds and pushes multi-architecture Docker images to gitea.mrixs.me
   - Includes caching for faster subsequent builds
   - Proper tagging strategy (branch, semver, SHA)
   - CI-CD-GUIDE.md documentation

3. Cleanup:
   - Removed temporary build artifacts and coverage files

Mrixs merged commit 9d24730efa into master 2026-08-05 20:19:16 +00:00

Mrixs referenced this issue from a commit 2026-08-05 20:19:18 +00:00

Merge pull request 'musicbrainz-provider' (#2) from musicbrainz-provider into master

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

Mrixs

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Mrixs/NaviWatcher#2