feat(library): effective genre falls back to MusicBrainz enrichment#949
Merged
Conversation
Converted packs rarely carry a genres manifest key — on Byron's real library 1188/1190 songs had no genre, starving the genre facet and the career passport rack (2 usable genres) while song_enrichment already held MB genres for 636 matched songs. The effective-genre expression now resolves: per-song override → pack genre → json_extract(enrichment.genres, '$[0]') for MATCHED rows only (review/failed candidates could carry the wrong recording's genres). Same fast-path gating as before: the plain indexed column is used unless overrides or enrichment genres actually exist; stand-in DBs without the table degrade via the OperationalError guard. Career passports pick this up automatically through _effective_genre_expr(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 (1)
📝 WalkthroughWalkthroughGenre facet and filtering now compute an effective genre using override, pack genre, and trusted MusicBrainz enrichment precedence. MetadataDB safely handles absent enrichment tables, with tests covering precedence, fallback, filtering, and robustness. ChangesGenre resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/metadata_db.py (1)
1099-1113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated override SQL fragment across the two constants.
The override-lookup subquery (lines 1100-1102 vs 1105-1107) is copy-pasted verbatim between
_EFFECTIVE_GENRE_OVERRIDE_SQLand_EFFECTIVE_GENRE_SQL. A future tweak to override matching (e.g. adding a new exclusion) risks being applied to only one constant.♻️ Extract the shared override fragment
+ _EFFECTIVE_GENRE_OVERRIDE_FRAGMENT = ( + "(SELECT o.value FROM song_field_override o " + "WHERE o.filename = songs.filename AND o.field = 'genre' " + "AND o.value IS NOT NULL AND o.value != '')" + ) _EFFECTIVE_GENRE_OVERRIDE_SQL = ( - "COALESCE((SELECT o.value FROM song_field_override o " - "WHERE o.filename = songs.filename AND o.field = 'genre' " - "AND o.value IS NOT NULL AND o.value != ''), genre)" + f"COALESCE({_EFFECTIVE_GENRE_OVERRIDE_FRAGMENT}, genre)" ) _EFFECTIVE_GENRE_SQL = ( - "COALESCE((SELECT o.value FROM song_field_override o " - "WHERE o.filename = songs.filename AND o.field = 'genre' " - "AND o.value IS NOT NULL AND o.value != ''), " + f"COALESCE({_EFFECTIVE_GENRE_OVERRIDE_FRAGMENT}, " "NULLIF(genre, ''), " "(SELECT json_extract(e.genres, '$[0]') FROM song_enrichment e " "WHERE e.filename = songs.filename AND e.match_state IN ('matched', 'manual') " "AND e.genres IS NOT NULL AND e.genres NOT IN ('', '[]')), " "'')" )🤖 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 `@lib/metadata_db.py` around lines 1099 - 1113, Extract the duplicated genre override lookup subquery into a shared SQL fragment near _EFFECTIVE_GENRE_OVERRIDE_SQL and _EFFECTIVE_GENRE_SQL, then interpolate or reuse it in both constants. Preserve the existing override matching conditions and fallback behavior while ensuring future changes to the lookup apply consistently.
🤖 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 `@CHANGELOG.md`:
- Around line 11-16: Update the CHANGELOG entry’s scope wording to explicitly
include manual rows alongside matched rows, reflecting the implementation and
manual-state test assertion. Preserve the existing genre precedence and
enrichment behavior descriptions.
---
Nitpick comments:
In `@lib/metadata_db.py`:
- Around line 1099-1113: Extract the duplicated genre override lookup subquery
into a shared SQL fragment near _EFFECTIVE_GENRE_OVERRIDE_SQL and
_EFFECTIVE_GENRE_SQL, then interpolate or reuse it in both constants. Preserve
the existing override matching conditions and fallback behavior while ensuring
future changes to the lookup apply consistently.
🪄 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: ae3a50c4-a7b5-4b42-87aa-93e6db03b897
📒 Files selected for processing (3)
CHANGELOG.mdlib/metadata_db.pytests/test_field_overrides.py
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Converted packs rarely carry a
genresmanifest key — on the real library 1188 of 1190 songs had no genre, starving the library genre facet and the career passport rack (2 usable genres) whilesong_enrichmentalready held MusicBrainz genres for 636 matched songs.The effective-genre expression now resolves: per-song override → pack genre →
json_extract(enrichment.genres, '$[0]')formatched/manualrows only (review/failedcandidates could carry the wrong recording's genres — and user-pinnedmanualmatches are the most trusted, a Codex round-2 catch). Fast-path gating preserved: the plain indexed column when neither source has data; the override-only expression when overrides exist but the enrichment table doesn't (round-3 catch — stand-in DBs must never receive SQL referencing a missing table). Career passports and the library genre filter both pick this up automatically through_effective_genre_expr().Measured on the real library: the passport rack went from 2 genres to 50 (heavy metal ×184, rock ×116, power metal, thrash, ska, prog…), with coverage growing as enrichment keeps matching.
Testing
Precedence matrix (override > pack > enrichment; review/failed never leak; manual counts), enriched-genre filtering finds the song, plain-column fast path, missing-table safety. Full suites at round 1: pytest 2485, JS 1170. Codex preflight clean (round 3).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests