Enrichment: MusicBrainz + Cover Art Archive by default, Discogs opt-in (ADR-0016) - #45
Merged
Merged
Conversation
`crates/enrichment` was a ten-line stub and Find Tags & Album Art was the last large `missing` row. Neither of Lexicon's own sources was available to copy — SonoVault is not a public API, and Spotify's `audio-features` is deprecated and 403s for applications registered since 2024-11-27 — so the providers had to be chosen. ADR-0016 records the choice. MusicBrainz is the default because it needs no account, key or registration; a default source the user must register for is a dead feature with a settings page. Cover Art Archive pairs with it, keyed by the release MBID a metadata match already carries. Discogs is opt-in with a keychain token, earning its slot on label, catalogue and year for dance records. This is the first feature where the library leaves the machine, so the CLAUDE.md privacy rule is structural rather than aspirational: - every request goes through one `Http` trait, so grepping its implementors enumerates the whole network surface - the cache is consulted before any provider, asserted on the request log rather than the returned value - only an artist and a title are sent; a test pins that no path, library id or volume name reaches a URL - a no-match is cached too, so a library of bootlegs does not pay the full rate-limited round trip on every re-run Rate limits are enforced in the crate: MusicBrainz's one-per-second and its User-Agent requirement are conditions of use, not courtesies. The limiter holds its lock across the sleep, or every waiter wakes at once and bursts. Enrichment backfills and never overwrites — including over whitespace, which real libraries are full of. Every proposal names its provider, and that reaches the staged change's reason (ADR-0008). The genre split follows the manual: main genre to the Genre field, the rest to Custom Tags; zero-vote MusicBrainz tags dropped so a typo cannot become somebody's Genre, and Discogs styles preferred over its broad genres. Album art fetches but does not embed, and no UI option offers it: `audio-tags` has no picture support, so a checkbox would download an image and discard it. Asserted absent by test. Reachable from the track context menu and from `library_find_tags` in chat, MCP and the CLI. The agent-tools path runs the async lookup on a fresh thread rather than `block_on`, since the MCP HTTP server calls `execute` from inside a runtime. Not verified against live responses: the container's network policy denies `musicbrainz.org`. Parsing is tolerant by construction, so schema drift costs proposals rather than wrong values — recorded in GAPS.md with the check that closes it. Parity: 61 done / 21 partial / 12 missing / 2 blocked / 16 deferred. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gn43w2xFL3JRBRkMv3vRo
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughAdds a complete metadata enrichment workflow. MusicBrainz is the default provider, Discogs is optional, and Cover Art Archive supports art retrieval. Results are cached, reviewed in the desktop UI or agent tool, and staged as attributed metadata changes. ChangesMetadata enrichment
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TrackMenu
participant EnrichPanel
participant Tauri
participant EnrichmentService
participant CacheDb
participant RekordboxLibrary
TrackMenu->>EnrichPanel: Open enrichment for selected tracks
EnrichPanel->>Tauri: Call enrichPreview
Tauri->>RekordboxLibrary: Load selected track metadata
Tauri->>EnrichmentService: Lookup provider candidates
EnrichmentService->>CacheDb: Read or update cached responses
EnrichmentService-->>Tauri: Return merged proposals
Tauri-->>EnrichPanel: Display proposals and provider errors
EnrichPanel->>Tauri: Call enrichStage with accepted proposals
Tauri-->>EnrichPanel: Return staged change IDs
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`strip_version`'s loop-with-let-else is a `while let`, and current stable clippy says so. Pure syntax; the tests are unchanged and still pass. Worth recording *why this got past me*: `rust-toolchain.toml` pins `stable`, which is 1.97.1 on CI, while this container's image had 1.94.1. So the local clippy is systematically an older lint set than the gate, and "clippy is clean here" was never the same claim as "clippy is clean on CI". Installed the current stable and re-ran the whole definition of done against it — fmt, the workspace tests and `clippy --workspace --all-targets -D warnings` are all clean on the toolchain CI actually uses, which is what makes this a fix for the class rather than for the one line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gn43w2xFL3JRBRkMv3vRo
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Turns
crates/enrichmentfrom a ten-line stub into a real crate, closing the tag half ofdocs/lexicon/07-health.md §Find Tags & Album Art— the last largemissingrow.Why these providers
Neither of Lexicon's own sources was available to copy: SonoVault is not a public API, and Spotify's
audio-featureswas deprecated on 2024-11-27 and returns 403 to applications registered since. So the sources had to be chosen. ADR-0016 records it.discogs_token, via the existingget_api_keyplumbing). It earns its slot where MusicBrainz is weakest — label, catalogue and year for dance records, which is most of a Rekordbox library.The privacy rule, made structural
This is the first feature where the library leaves the machine, so
CLAUDE.md's rule had to stop being aspirational:enrichment::http::Httptrait — grep its implementors and you have the entire network surface. Today that's one type, in one file, behind an optionalreqwestfeature.Service::lookupconsults the cache before any provider. The test asserts on the request log, not the returned value — returning the same answer twice would not prove the network was skipped.Rate limits are terms of use
MusicBrainz documents one request per second and a User-Agent identifying the app with contact info; ignoring either gets you blocked, and a blocked client means the feature stops working with no visible cause. Both are enforced in the crate rather than at call sites, because a limit each call site must remember is one a call site will forget.
The limiter holds its lock across the sleep on purpose: releasing it first would let every waiter compute the same wake time and fire together, which is exactly the burst the limit forbids. Tested with a paused clock — five callers take four intervals, and a naturally-spaced caller is never delayed.
Behaviour worth calling out
" "as curation would make the feature do nothing on a real collection.Artist - Titleis the normal state of a downloaded library, and searching that whole string as a title matches nothing.Two things deliberately not shipped
No album-art option in the UI. The downloader works — MIME sniffed from magic bytes rather than a header (an ID3 picture frame with a wrong MIME is one players silently refuse to show), WAV refused up front per the manual's own caveat. But
crates/audio-tagshas no picture support, so a checkbox would download an image and discard it: exactly the stub logicCLAUDE.mdforbids in production paths. A test asserts the option is absent, with the reason.Not verified against live responses. The container's network policy denies
musicbrainz.org(403 on CONNECT). Both providers' field paths are written against documented schemas. Every parse is tolerant by construction, so schema drift costs proposals, never a wrong value written into a library — which is why this ships unverified and ANLZ writing does not: there the failure mode is a corrupted user file.GAPS.mdcarries the onecurlthat closes it.While writing that up I found the existing GAPS bullet claiming these providers could not be written here either. That was wrong, and the design is the disproof — making the transport a seam left everything but the wire testable. Corrected in place.
Reach
Track context menu → Find tags & album art…, and
library_find_tagsin chat, MCP and the CLI. The agent-tools path needed care:AgentToolService::executeis synchronous and the lookup is not, and the MCP HTTP server callsexecutefrom inside a Tokio runtime whereblock_onpanics — so the future runs on a fresh thread that has no ambient runtime to nest in.Verification
85 new tests in
crates/enrichment(query escaping, original-release re-ranking, tolerant parsing of every optional field, cache-before-network on the request log, provider isolation on failure, rate-limit queueing under a paused clock, WAV refusal, MIME sniffing), 7 incrates/cache, 10 inenrich.rs, 9 inEnrichPanel.test.tsx.Parity: 61 done / 21 partial / 12 missing / 2 blocked / 16 deferred.
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation