feat(v3 library): "Fix metadata" popup — per-song override + lock, cover picker, MusicBrainz + AcoustID - #777
Merged
Merged
Conversation
Contributor
|
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 (7)
📝 WalkthroughWalkthroughAdds per-song metadata overrides and field locking, wires the new state into library display, matching, and gap-fill flows, and updates the single-song review UI to edit and save those overrides. ChangesField overrides and locking feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_field_overrides.py (1)
15-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFactor the shared
serverfixture intotests/conftest.py. The reload/env/teardown pattern is repeated across multiple test modules; centralizing it would reduce drift if the cleanup logic changes.🤖 Prompt for 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. In `@tests/test_field_overrides.py` around lines 15 - 33, The shared server setup/teardown logic in the `server` fixture is duplicated across test modules and should be centralized. Move the reload/env/cleanup fixture into `tests/conftest.py` so all tests reuse the same `server` setup, including the `importlib.import_module("server")` reload, `CONFIG_DIR`/`FEEDBACK_SKIP_STARTUP_TASKS` monkeypatching, and the `meta_db` connection cleanup with `_join_background_db_threads`. Keep the `client` fixture local if needed, but have it depend on the shared `server` fixture.
🤖 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.
Nitpick comments:
In `@tests/test_field_overrides.py`:
- Around line 15-33: The shared server setup/teardown logic in the `server`
fixture is duplicated across test modules and should be centralized. Move the
reload/env/cleanup fixture into `tests/conftest.py` so all tests reuse the same
`server` setup, including the `importlib.import_module("server")` reload,
`CONFIG_DIR`/`FEEDBACK_SKIP_STARTUP_TASKS` monkeypatching, and the `meta_db`
connection cleanup with `_join_background_db_threads`. Keep the `client` fixture
local if needed, but have it depend on the shared `server` fixture.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 87c8a3a3-99e7-49b9-912f-03474f7924cb
📒 Files selected for processing (7)
server.pystatic/tailwind.min.cssstatic/v3/match-review.jsstatic/v3/songs.jstests/test_field_overrides.pytests/test_gap_fill.pytests/test_mb_enrichment.py
This was referenced Jul 5, 2026
Contributor
Author
📋 Merge order — metadata-curation epic (7 PRs)
#778–#780 are stacked (each based on the branch above), so GitHub re-points each to
|
This was referenced Jul 5, 2026
… (popup slices 1–2)
Backend foundation for the Fix-metadata popup. Not yet surfaced in the UI (the
display + 3-tab popup are the next slices); no PR until it's user-visible.
Slice 1 — the store:
- `song_field_override(filename, field, value, locked)` table: a reversible
DISPLAY overlay (never written to the pack), filename-keyed so it survives a
rescan (never purged by delete_missing) and is dropped only with the song.
- DB methods (partial upsert that drops empty+unlocked rows; batch map) +
`GET`/`PUT /api/song/{fn}/overrides` (field allowlist title/artist/album/
year/genre; clearing rides PUT since DELETE /api/song/{path} shadows sub-
routes; PUT demo-blocked).
Slice 2 — locks respected by enrichment:
- The auto-matcher composes a per-song `_compose_lock_filter` onto the global
apply-filter, so a match still applies IDENTITY (mbid/release → art) but never
re-canonicalizes a LOCKED display field.
- Gap-fill (write-to-file) skips locked album/year/genre — writing the matched
value would be exactly the clobber the lock exists to prevent.
- Review/manual picks bypass the filter (an explicit confirm overrides a lock).
Tests: store semantics + rescan-survival + API; the lock filter + reader; an
auto-match leaving a locked field un-canonicalized; gap-fill excluding locked
keys.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The grid now displays the user's per-song title/artist/album/year override
in place of the pack value ("grid shows only overrides") — a matched
MusicBrainz canon never silently re-titles a card; canon stays in the
Details drawer + art. Overlaid in Python over the visible window, keyset-safe
like the P4 artist-alias re-label: the seek still runs on the raw column, and
the one overridable keyset column (title) stashes its raw value for the cursor
so paging never skips/dupes. The private stash is dropped from the payload.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (slice 4)
Turns the thin single-song fix-match modal into the Plex-style metadata
editor reached from a card's "Fix metadata…" menu:
- Details tab: type + lock the displayed title/artist/album/year. Values
ride the reversible override store (GET/PUT /api/song/{fn}/overrides); each
field sits on its pack value (Yours/Pack provenance + revert-to-pack), a lock
pins it against auto-match, and Save repaints the grid via library:changed
(slice-3 overlay). This is the real tool for the blank-artist city-pop pile
MusicBrainz can't surface — you just type the right title.
- Cover art tab: hands off to the shared image picker (its own modal); the
pick refreshes the thumbnail everywhere.
- Match tab: the existing MusicBrainz search + candidate/pick flow, refactored
into shared body/footer helpers (the queue-review flow is untouched).
Backend: GET /overrides now also returns the pack baseline so the Details tab
can pre-fill + show provenance. tailwind.min.css regenerated (build-tailwind.sh)
for the popup's new utility classes.
Identify-by-audio (AcoustID) is deferred: it lives in unmerged PR #759, off
main — the Match tab gains the button once #759 lands and this branch rebases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The AcoustID Identify button (#759) merged in referencing an out-of-scope `panel` in the wiring — a leftover from the pre-popup fix-match modal that my tab refactor renamed to `root`. Under strict mode that threw, so the handler never attached and the button did nothing. Scope it to `root` (the tab body), which is where the search-results area it renders into lives. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An empty AcoustID result read the same as a broken button. Each state now says plainly which outcome it is — ✓ fingerprinted-but-no-match vs no-audio vs off vs unavailable — and, in the popup, points at the manual fallback (Search, or set the album in Details + cover in Cover art by hand). A ✓ marks the states that actually ran, so "worked, found nothing" no longer looks like a failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos
force-pushed
the
feat/v3-fix-metadata-popup
branch
from
July 5, 2026 19:09
d96627e to
fcb3b41
Compare
4 tasks
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.
A Plex-style "Fix metadata" editor for the v3 library, reached from a card's right-click / ⋮ → Fix metadata…. It's the manual curation tool for packs whose metadata is wrong or blank (a large slice of community/CDLC charts) — the ones automatic matching can't crack.
Everything here is a reversible display layer — pack/
.sloppakfiles are never modified.Details tab — type + lock the shown fields
song_field_override): filename-keyed so it survives a rescan, dropped only with the song.GET/PUT /api/song/{fn}/overrides(GET also returns thepackbaseline for provenance/revert)._compose_lock_filter) so an auto-match still fixes identity/art but won't overwrite a locked display field, and gap-fill skips locked fields. An explicit user pick always wins.Grid shows your overrides
query_pageoverlays the override (over the pack value, and over the P4 artist-alias re-label) for the visible window — matched MusicBrainz canon never silently re-titles a card (canon stays in the drawer + art). Keyset-safe: paging still seeks on the raw column (the one overridable keyset column, title, stashes its raw value for the cursor; stripped from the payload).Cover art tab
Match tab — MusicBrainz + Identify by audio
Notes
tailwind.min.cssregenerated for the popup's new utility classes.mainin (fix(enrichment): rank the canonical studio take over live/comp versions #758/feat(enrichment): AcoustID audio-fingerprint identification (opt-in) #759/v3 library: "Refresh Metadata" button + per-view re-match + filename-artist seed #764 etc.).Testing
tests/test_field_overrides.py(+test_mb_enrichment/test_gap_fillfor lock/gap-fill). Full library/enrichment suites green; both JS modulesnode --check.🤖 Generated with Claude Code
Summary by CodeRabbit