fix(enrichment): rank the canonical studio take over live/comp versions - #758
Merged
Conversation
A flat MusicBrainz /recording text search ties every take of a song at the same score, so "AC/DC — Highway to Hell" returns a wall of live bootlegs and compilations with the 1979 studio version buried (or below the fetch limit). - build_recording_query: drop live-ONLY recordings (`-secondarytype:Live`). Compilations are deliberately kept — they REUSE the studio recording, so filtering them cuts the very recording we want (verified against MB). - _best_release / parse_recording_doc: pick the canonical studio album (primary Album, no Live/Compilation/Remix/... secondary type) for the displayed album/year, and expose a `studio` flag. - rank_candidates: since the combined score caps at 1.0 (perfect text match ties), break ties on the studio flag and — when the caller knows the audio length — on duration proximity, so the studio take wins over live/extended cuts. The studio distinction is intentionally NOT scored (a live take is still the right SONG), only re-ordered. - /api/enrichment/search: accept an optional `duration` param so a caller that has the audio but no library row (the editor's create modal) can pass the master-track length for the duration tiebreak. Verified end-to-end against live MusicBrainz: AC/DC "Highway to Hell" now returns the 1979 studio recording at #1 with the correct album + year. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMusicBrainz matching now prefers clean studio albums and studio takes, excludes live-only recordings for non-live charts, and lets enrichment search use an optional duration hint with a larger candidate limit. ChangesStudio vs non-studio recording preference
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server.py (1)
6238-6247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace ambiguous en dash flagged by Ruff (RUF002).
Line 6239's docstring uses
–(EN DASH) instead of a plain hyphen.🔤 Suggested fix
- """Text search (tier 2–4): denoised Lucene query over /recording. The query + """Text search (tier 2-4): denoised Lucene query over /recording. The query🤖 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 `@server.py` around lines 6238 - 6247, The docstring in _mb_search_recordings contains an ambiguous en dash that Ruff flags as RUF002; replace the EN DASH in the phrase describing the tier range with a plain hyphen while keeping the rest of the docstring unchanged. Locate the change in _mb_search_recordings and ensure any similar punctuation in nearby docstrings is normalized to ASCII-compatible hyphens.Source: Linters/SAST tools
lib/mb_match.py (1)
273-286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocstring understates the priority ordering between "clean studio" and "official".
The sort key treats
cleanas strictly higher priority thanstatus_ok— a non-official clean studio release will beat an official but non-clean (e.g. compilation) release. The docstring's phrasing ("prefer an OFFICIAL studio Album ... then the earliest date") reads as if official-and-clean is one combined criterion, obscuring that clean strictly dominates official. Worth tightening the wording so future edits to this ordering don't accidentally couple the two criteria.✏️ Suggested docstring clarification
def _best_release(doc: dict) -> dict: - """Pick the release used for canon album/year: prefer an OFFICIAL studio - Album (primary Album with no Live/Compilation/… secondary type), then the - earliest date. Falls back to any release when none is clean. {} if none.""" + """Pick the release used for canon album/year. Priority, in order: + (1) a clean studio Album (primary Album, no Live/Compilation/… secondary + type) over anything else, (2) OFFICIAL status, (3) earliest date. + Falls back to any release when none is clean. {} if none."""🤖 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/mb_match.py` around lines 273 - 286, Clarify the _best_release docstring so it matches the actual sort_key ordering: clean studio status is checked before official status, meaning a clean non-official release can outrank an official non-clean one. Update the wording near _best_release and sort_key to state the priority as clean studio first, then official, then earliest date, so the criteria are not read as a combined condition.
🤖 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 `@lib/mb_match.py`:
- Around line 193-217: The new studio tie-break in rank_candidates only affects
freshly ranked rows, so older song_enrichment.candidates entries still use the
old stored shape and won’t benefit from the studio field. Update the read path
that uses rank_candidates (or the surrounding review-candidate loading logic) to
re-rank persisted candidates on fetch, or add a backfill/migration to populate
studio for existing review rows so they can be sorted with the new tie-break.
---
Nitpick comments:
In `@lib/mb_match.py`:
- Around line 273-286: Clarify the _best_release docstring so it matches the
actual sort_key ordering: clean studio status is checked before official status,
meaning a clean non-official release can outrank an official non-clean one.
Update the wording near _best_release and sort_key to state the priority as
clean studio first, then official, then earliest date, so the criteria are not
read as a combined condition.
In `@server.py`:
- Around line 6238-6247: The docstring in _mb_search_recordings contains an
ambiguous en dash that Ruff flags as RUF002; replace the EN DASH in the phrase
describing the tier range with a plain hyphen while keeping the rest of the
docstring unchanged. Locate the change in _mb_search_recordings and ensure any
similar punctuation in nearby docstrings is normalized to ASCII-compatible
hyphens.
🪄 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: b7d4f974-6021-4fe5-b207-fbdda8a39a80
📒 Files selected for processing (3)
lib/mb_match.pyserver.pytests/test_mb_match.py
Comment on lines
193
to
217
| def rank_candidates(song: dict, candidates: list[dict]) -> list[dict]: | ||
| """Score every candidate against the song and return them sorted by our | ||
| score (MusicBrainz's own search score is only a tiebreak). Each returned | ||
| dict is a copy carrying `score` (rounded — it's displayed and stored).""" | ||
| """Score every candidate against the song and return them sorted best-first. | ||
| The combined `score` caps at 1.0, so a perfect-text-match query (every "AC/DC | ||
| Highway to Hell" recording) ties at the top — there the studio flag and, when | ||
| the caller knows the audio length, the duration match break the tie so the | ||
| canonical studio take wins over live/promo/extended cuts. Each returned dict | ||
| is a copy carrying `score` (rounded — it's displayed and stored).""" | ||
| sd = _duration_int(song.get("duration")) | ||
|
|
||
| def _dur_diff(c): | ||
| cd = _duration_int(c.get("duration")) | ||
| return abs(sd - cd) if (sd and cd) else 10 ** 6 | ||
|
|
||
| ranked = [] | ||
| for cand in candidates or []: | ||
| c = dict(cand) | ||
| c["score"] = round(score_candidate(song, cand), 4) | ||
| ranked.append(c) | ||
| ranked.sort(key=lambda c: (c["score"], c.get("mb_score") or 0), reverse=True) | ||
| ranked.sort( | ||
| key=lambda c: (c["score"], | ||
| 1 if c.get("studio") else 0, # canonical studio take | ||
| -_dur_diff(c), # closest to the audio length | ||
| c.get("mb_score") or 0), | ||
| reverse=True) | ||
| return ranked |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all rank_candidates call sites and see what "candidates" argument they use
rg -nP -C4 '\brank_candidates\(' server.py lib/mb_match.py
# Check if there is any persistence/backfill logic touching song_enrichment.candidates
rg -nP -C3 'song_enrichment' server.py | rg -n 'candidates'Repository: got-feedBack/feedBack
Length of output: 2727
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the parser/docstring and any persistence paths around `candidates`
rg -n -C4 'parse_recording_doc|candidates|song_enrichment' lib server.py
# Read the relevant code around the API endpoint that returns ranked candidates
sed -n '7390,7460p' server.py
# Read the code around the MusicBrainz search / lookup helpers that feed rank_candidates
sed -n '6580,6660p' server.pyRepository: got-feedBack/feedBack
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every place candidates are written to or read from storage
rg -n -C3 'apply_enrichment_match|candidates\s*=|\.candidates\b|song_enrichment.*candidates|UPDATE .*candidates|INSERT .*candidates|SELECT .*candidates' server.py libRepository: got-feedBack/feedBack
Length of output: 8188
Backfill stored review candidates for the new studio tie-break
Rows already persisted in song_enrichment.candidates keep the old shape, so review entries created before this change still rank without studio until refreshed. Re-rank on read or add a backfill for existing review rows.
🤖 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/mb_match.py` around lines 193 - 217, The new studio tie-break in
rank_candidates only affects freshly ranked rows, so older
song_enrichment.candidates entries still use the old stored shape and won’t
benefit from the studio field. Update the read path that uses rank_candidates
(or the surrounding review-candidate loading logic) to re-rank persisted
candidates on fetch, or add a backfill/migration to populate studio for existing
review rows so they can be sorted with the new tie-break.
_best_release sorted (clean, status_ok, date), so an UNofficial bootleg Album outranked an official Single/EP/comp — regressing canonical album/year and seeding cover-art from a bootleg for single-only songs. Order status_ok before clean: official first, then prefer a clean studio album among the official releases (still surfaces the studio album over an official live/comp album). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
build_recording_query unconditionally added -secondarytype:Live, but denoise()
strips a '(Live at …)' qualifier from the query — so a chart that IS a live take
had its only correct recording filtered out (both background enrichment and
manual search). Skip the live filter when the source title carries a
parenthetical live marker; a bare title word ('Live and Let Die') still filters.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-through on keeping live recordings for live charts: rank_candidates still ranked the studio take ahead of a tied live one, so a live chart would auto-match the studio recording. Skip the studio tiebreak when the source title has a live marker — duration proximity + score then pick the right live version. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Review + fix pass (Claude Code). The ranking logic is sound; pushed 3 fixes closing the live-vs-studio loop (26 mb_match tests green). CI red is the unrelated main-level starter-content bug.
|
This was referenced Jul 4, 2026
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
3 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.
Problem
A flat MusicBrainz
/recordingtext search ties every take of a song at the same score. Searching AC/DC — "Highway to Hell" returns ~10 live bootlegs and compilations with the canonical 1979 studio version buried below the fetch limit. This hits the editor's new "Match metadata" popup and the background library enrichment (both sharelib/mb_match.py+/api/enrichment/search).Fix (verified against live MusicBrainz)
build_recording_query— drop live-only recordings (-secondarytype:Live). Compilations are deliberately kept: they reuse the studio recording, so filtering them cut the very recording we want (empirically confirmed)._best_release/parse_recording_doc— pick the canonical studio album (primaryAlbum, noLive/Compilation/Remix/… secondary type) for the displayed album/year, and expose astudioflag.rank_candidates— the combined score caps at 1.0, so perfect-text-match candidates tie; break ties on the studio flag and, when the caller knows the audio length, on duration proximity. The studio distinction is not scored (a live take is still the right song) — only re-ordered./api/enrichment/search— accept an optionaldurationparam so a caller that has the audio but no library row (the editor's create modal, which holds the master track) can pass the length for the duration tiebreak.Result: AC/DC "Highway to Hell" now returns the 1979 studio recording at #1 with the correct album + year.
Tests
tests/test_mb_match.py+tests/test_mb_enrichment.py— 48 passing (query-string assertion updated for the new-secondarytype:Live).🤖 Generated with Claude Code
Summary by CodeRabbit