feat: expose enriched song metadata to plugins#1045
Conversation
Signed-off-by: Jaime McGovern <jaimemcgovern@outlook.com>
Signed-off-by: Jaime McGovern <jaimemcgovern@outlook.com>
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds a versioned plugin metadata assembler backed by cached song and enrichment data, exposes it through REST and WebSocket APIs, forwards it to browser song state, and documents and tests the additive contract. ChangesPlugin Metadata API
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant REST
participant Highway
participant Metadata
participant SQLite
Client->>REST: GET /api/song/{filename}/metadata
REST->>Metadata: plugin_metadata_for(filename)
Metadata->>SQLite: Read song fields and enrichment
SQLite-->>Metadata: Cached metadata
Metadata-->>REST: Metadata object
REST-->>Client: JSON response
Client->>Highway: Request song_info
Highway->>Metadata: plugin_metadata_for(song path)
Metadata->>SQLite: Read song fields and enrichment
SQLite-->>Metadata: Cached metadata
Metadata-->>Highway: Metadata object
Highway-->>Client: song_info.metadata
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/PLUGIN_METADATA_API.md`:
- Around line 174-178: Update the old-server fallback description in the plugin
metadata documentation to state that currentSong.metadata is assigned null when
the server omits metadata, rather than being absent or undefined. Adjust the
optional-chaining example and feature-detection guidance to match this
null-valued contract.
In `@docs/PLUGIN_METADATA_FINAL_VERIFICATION.md`:
- Around line 7-13: Regenerate the verification record in the documented report,
replacing stale test counts and coverage claims with results from the current
tests/test_plugin_metadata.py, including its 23 collected cases and
base-metadata coverage. Update the diff-scope statement to accurately reflect
the current nine-file diff, and remove any unsupported harness, validation, or
readiness assertions before publishing the record.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bd05f420-f663-4396-bf69-92007027211d
📒 Files selected for processing (9)
CHANGELOG.mddocs/PLUGIN_METADATA_API.mddocs/PLUGIN_METADATA_FINAL_VERIFICATION.mdlib/plugin_metadata.pylib/routers/song.pylib/routers/ws_highway.pystatic/highway.jstests/test_plugin_metadata.pytests/test_plugin_metadata_api.py
…t undefined) Addresses CodeRabbit review comment on PR got-feedBack#1045: static/highway.js assigns metadata: msg.metadata ?? null, so an old server's song_info frame (which has no metadata key at all) still results in currentSong.metadata being null on the client, not undefined. The optional-chaining claim was still correct (null?.album also safely evaluates to undefined) — only the direct null/undefined claim was wrong. Signed-off-by: Jaime McGovern <jaimemcgovern@outlook.com>
Replace the stale staged-not-committed snapshot with the feature's actual state: three signed commits on feature/plugin-metadata-api, pushed and open as got-feedBack#1045. - Test counts: 16 -> 23 items in test_plugin_metadata.py, reflecting the 5 base_metadata test functions (7 pytest items) added since the prior pass. - Incorporate real pytest evidence (31/31 across both test files, 21/21 WS regression suite) from the contributor's own machine, superseding the manual-harness-only evidence this record previously relied on exclusively. - Diff scope: 3 files -> the current 9-file diff (adds CHANGELOG.md), matching git diff --stat dd1927e..HEAD. - Remove the stale 'READY TO COMMIT UPSTREAM PR' / 'do not commit, do not push' framing, since both have already happened. No code changes. Manual harness re-run fresh and git diff --check confirmed clean before committing. Signed-off-by: Jaime McGovern <jaimemcgovern@outlook.com>
2 participants
What
Adds a generic Plugin Metadata API: any plugin can now get album, year,
genre, and MusicBrainz identifiers (recording/release/artist MBID, ISRC)
for the currently-playing or any known song — data the server already
computes via its MusicBrainz/AcoustID enrichment pipeline, but that
never reached a plugin before.
Two delivery surfaces, one shared resolver:
song_infoframe gains one new optional key,metadata.GET /api/song/{filename}/metadataREST route serves the sameshape for a song that isn't currently playing.
Both call the same
lib/plugin_metadata.py::plugin_metadata_for()—there is exactly one place this assembly happens, so the two surfaces
can't drift apart in shape or in the rules that gate MusicBrainz data.
Why this is generic, not Song-Background-Manager-specific
This started as an investigation into what SBM (a community plugin)
would need for album art/MusicBrainz lookups. The server already had
everything — playback metadata, an enrichment pipeline with confirmed
MusicBrainz matches — but exposed none of it to any plugin. Rather than
build a one-off channel for one plugin, this exposes the same
information generically: any current or future plugin (visualizers,
scrobblers, tagging tools, alternate art sources) gets it through the
same public surface. SBM itself is not modified by this PR.
Backwards compatibility
Purely additive:
renamed, or reinterpreted.
metadatakey onsong_infoand one new REST route.
metadataobject has a stable set of documented keys and usesconsistent empty or null defaults, allowing consumers to read fields
safely without checking whether each individual key exists.
metadatabehaves identically beforeand after this change. A new plugin against an old server just sees
metadataasundefined/absent.How it works
plugin_metadata_for(filename, *, base_metadata=None)reads theexisting
songscache (MetadataDB.pack_fields) and the existingsong_enrichmenttable (MetadataDB.get_enrichment) — the existingindexed MetadataDB lookups, no new tables, no new query shapes, and
no network access anywhere in this path.
base_metadatalets a caller that already has a loadedSong(theWebSocket handler, mid-playback) pass its
album/yearstraightfrom the sloppak manifest instead of waiting on the next library
scan to populate the cache — resolved per field
(
base_metadata → songs cache → safe default), never all-or-nothing.The REST route has no loaded
Songto draw on, so it stayscache-only, exactly as before this parameter existed. (Note: this
means
album/yearcan briefly differ between the two surfaces fora song that hasn't been scanned yet — documented in
docs/PLUGIN_METADATA_API.md.)
enrichment match is
matchedormanual— areview/failed/unscannedrow's candidate data is a proposal, not a confirmedidentity, and is never leaked as if it were one. This mirrors the
same gate
routers/art.py's cover-art candidate assembly alreadyapplies.
Why no duplication
Before this PR, adding metadata to both the WS payload and a REST route
independently would mean the two could silently disagree (e.g. one
surface's gating logic for MusicBrainz identifiers drifting from the
other's over time). Routing both through one function makes that
structurally impossible rather than a matter of remembering to keep
two implementations in sync.
Testing
tests/test_plugin_metadata.py— pure unit tests against a realMetadataDB(no FastAPI, no network): unknown-song defaults, yearcoercion edge cases, enrichment gating (matched/manual/review/failed/
unscanned),
base_metadataresolution (overrides cache, blank/zerofalls through, partial base_metadata still pulls missing fields from
cache, enrichment unaffected either way), JSON round-trip.
tests/test_plugin_metadata_api.py— WS/REST integration tests:route precedence (the new
/metadataroute can't be shadowed by theexisting bare
GET /api/song/{filename}catch-all), missing-songhandling, and confirmation that every pre-existing
song_infofieldis unchanged by this addition.
test_highway_ws_authors,test_highway_ws_instrument_routing,test_highway_ws_notation,test_ws_highway_disconnect) passing with no regressions.See
docs/PLUGIN_METADATA_API.mdfor the full field reference anddocs/PLUGIN_METADATA_FINAL_VERIFICATION.mdfor the pre-mergeverification pass.
feedpak surface
(manifest keys, pack files, folder layout) — it only reads
fields the existing loader/scanner already parse.
Checklist
CHANGELOG.md[Unreleased]updatedgit commit -s)Summary by CodeRabbit