mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-12 03:41:40 +00:00
library: MusicBrainz text matching + Match-Review UI (P8) (#710)
* library: MusicBrainz text matching + Match-Review UI — P8
Replaces the enrichment plumbing's no-op matcher (P7) with the real
pipeline, per the library-metadata design: a wrong match is worse than a
slow one, so medium confidence goes to a human review queue and never
straight to canonical values.
- lib/mb_match.py (new, pure — no network/DB/server imports): denoise
(author credits, (440Hz)/(Live)/(No Lead)/(v2) parentheticals,
diacritics/punctuation, ACDC / AC DC / AC/DC folding via compacted
token equality), token-set similarity, scoring with year/duration
corroboration bonuses, tier classification (auto needs combined
>= 0.95 AND per-field floors — a perfect-title cover by the wrong
artist, or a chart with no artist, can never auto-match), Lucene
query building, MusicBrainz response normalization.
- Matcher precedence in _enrich_one: content-hash cache copy (another
chart of the same recording matches with no network) -> manifest
mbid (tier 0) / isrc (tier 1) exact keys, feature-detected and
strictly shape-validated, read-only -> text search tiers
(auto / review / failed).
- Lifecycle: review rows store their ranked candidate list (JSON) and
write NO canonical fields until a human accepts; failed rows retry on
an exponential backoff (1 h doubling, 7 d cap) via the attempts
column; user-rejected rows never auto-retry; an identity edit
re-queues anything and resets the backoff; never-overwrite-manual is
enforced inside the single writer (apply_enrichment_match) so no call
path can forget it.
- Network: _mb_http_get is the one transport seam — throttled to
<= 1 req/s through P7's _enrich_throttle, identified with a real
User-Agent from VERSION, and a 503 pauses the whole pass without
burning attempts. Offline guard: no sockets under
FEEDBACK_ENRICH_OFFLINE or FEEDBACK_SKIP_STARTUP_TASKS, so pytest can
never reach MusicBrainz; the pass still stamps identity hashes
(two-phase), which is why every P7 test passes unchanged.
- Routes: GET /api/enrichment/review, POST
/api/enrichment/review/{filename}/accept|reject|pick, GET
/api/enrichment/search (throttled manual-search proxy). All four are
demo-mode blocked.
- Match facet: match= CSV accepted by /api/library AND
/api/library/stats (the A-Z rail's letter counts stay lockstep with
the grid) — review / matched (incl. manual) / unmatched / pending,
the same EXISTS idiom as the mastery facet.
- UI: static/v3/match-review.js (new, self-contained) — an ambient
"N to review" chip beside the song count (rendered only when
non-zero; silent on success, no toasts), and a review drawer on the
filter-drawer slide idiom (Escape + focus trap; row click accepts,
"Not a match" rejects, "Search instead" is the fix-match escape
hatch). songs.js gets the chip mount, a Match filter section, and
session-only match state; also fixes the latent applySavedPrefs bug
where restored filters dropped the mastery key, which made the
filter drawer throw for anyone with saved prefs.
- static/tailwind.min.css regenerated (scripts/build-tailwind.sh) for
the new utility classes; conflicts with sibling PRs resolve by
re-running the script.
Nothing is ever written to pack files — canonical values live only in
the song_enrichment display cache. Cover art caching and acoustic
fingerprinting are follow-up slices.
22 pure unit tests + 19 server tests (fake transport injected over the
_mb_http_get seam) + demo-mode route cases; full-suite failure set
A/B-identical with the change stashed vs applied.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
* library: match-review modal + configurable auto-apply confidence (P8 R0)
Follow-up to the initial P8 commit, folding in the first round of tester
feedback on the review surface and the matcher's knobs:
- Review GUI is a centred MODAL now, not a sidebar — one chart at a
time (the scraper-review model from media-server / emulation-frontend
apps): the chart's current metadata with explicit amber
"Missing: album / year / cover art" chips (art detected via the art
request failing), candidates each carrying "Adds: year - genres -
ISRC" / "Shows as: ACDC -> AC/DC" per-field chips, and Skip /
Not a match / Search instead / Use selected with prev-next + arrow-key
navigation. Chip + window API surface unchanged, so songs.js needed no
edits for the rework.
- Auto-apply confidence is a SETTING: default drops 0.95 -> 0.90
(mb_match.AUTO_MIN; classify() takes an auto_min override). The
per-field floors are untouched and threshold-independent — a
perfect-title cover by the wrong artist still can't auto-match at any
setting. New validated settings keys: enrich_enabled (bool) +
enrich_auto_threshold (0.5–1.01; >1.0 = "Always review", since a
capped score can equal exactly 1.0). Read once per pass; disabling
gates only the BACKGROUND matcher — manual search/fix stays available.
- Settings -> Library -> "Metadata matching" card: enable toggle,
confidence select (85 / 90 / 95 / Always review), a Match Now button
(new POST /api/enrichment/kick, single-flight like every other kick,
demo-mode blocked), and a live status line fed by the same fetch as
the review chip. Markup in index.html per the v3 settings pattern,
wired by match-review.js, null-guarded so v2 no-ops.
- Review queue orders missing-data charts first — confirming those has
the most to gain; complete charts only stand to be re-labelled.
Tests: threshold moves the auto/review boundary via settings; the
enable toggle gates matching but not the manual proxy; settings
validation; kick route; queue ordering; classify(auto_min=...) floors.
Full-suite failure set byte-identical to the pre-change baseline.
tailwind.min.css regenerated for the modal's utility classes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
* fix(library): lock MusicBrainz throttle across sleep + de-dup enrich queue (PR #710 review)
Hold a module-level lock across _enrich_throttle's read/sleep/write so the
background daemon and threadpooled sync search route serialize outbound MB
requests instead of bursting past the 1 req/s limit. De-dup the enrich queue
by filename so a changed-hash failed row isn't processed twice per pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
byrongamatos
parent
7c15cdda66
commit
74cd08f765
@@ -91,6 +91,13 @@ def test_demo_off_settings_post_not_blocked(tmp_path, monkeypatch):
|
||||
("GET", "/api/plugins/updates"),
|
||||
("POST", "/api/plugins/highway_3d/files"),
|
||||
("DELETE", "/api/plugins/highway_3d/files"),
|
||||
# Enrichment (P8): review writes + the MusicBrainz search proxy (the
|
||||
# proxy would spend the shared rate limit for anonymous demo visitors).
|
||||
("POST", "/api/enrichment/review/some-file/accept"),
|
||||
("POST", "/api/enrichment/review/some-file/reject"),
|
||||
("POST", "/api/enrichment/review/some-file/pick"),
|
||||
("POST", "/api/enrichment/kick"),
|
||||
("GET", "/api/enrichment/search"),
|
||||
])
|
||||
def test_demo_on_blocked_routes_return_403(tmp_path, monkeypatch, method, path):
|
||||
server, client = _make_client(tmp_path, monkeypatch, demo=True)
|
||||
|
||||
@@ -0,0 +1,512 @@
|
||||
"""Server-level tests for the P8 MusicBrainz matcher + Match-Review flow.
|
||||
|
||||
The HTTP transport is a fake installed over `server._mb_http_get` — the ONE
|
||||
seam enrichment uses to reach the network — so nothing here ever opens a
|
||||
socket. The offline default is itself under test: without explicitly
|
||||
enabling the network flag, a pass must skip matching entirely (pytest can
|
||||
never hit MusicBrainz, whatever a test triggers).
|
||||
"""
|
||||
|
||||
import importlib
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def server(tmp_path, monkeypatch, isolate_logging):
|
||||
monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "config"))
|
||||
dlc = tmp_path / "dlc"
|
||||
dlc.mkdir()
|
||||
monkeypatch.setenv("DLC_DIR", str(dlc))
|
||||
monkeypatch.setenv("FEEDBACK_SKIP_STARTUP_TASKS", "1")
|
||||
sys.modules.pop("server", None)
|
||||
srv = importlib.import_module("server")
|
||||
try:
|
||||
yield srv
|
||||
finally:
|
||||
conn = getattr(getattr(srv, "meta_db", None), "conn", None)
|
||||
if conn is not None:
|
||||
conn.close()
|
||||
sys.modules.pop("server", None)
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def client(server):
|
||||
return TestClient(server.app)
|
||||
|
||||
|
||||
class FakeMB:
|
||||
"""Canned MusicBrainz: records every call, serves per-path responses."""
|
||||
|
||||
def __init__(self):
|
||||
self.calls = []
|
||||
self.search_response = {"recordings": []}
|
||||
self.recording_lookups = {} # mbid → recording doc
|
||||
self.isrc_lookups = {} # isrc → {"recordings": [...]}
|
||||
self.raise_transport = False
|
||||
|
||||
def __call__(self, path, params):
|
||||
if self.raise_transport:
|
||||
raise self._srv.EnrichTransportError("fake network down")
|
||||
self.calls.append((path, dict(params)))
|
||||
if path == "recording":
|
||||
return self.search_response
|
||||
if path.startswith("recording/"):
|
||||
return self.recording_lookups.get(path.split("/", 1)[1])
|
||||
if path.startswith("isrc/"):
|
||||
return self.isrc_lookups.get(path.split("/", 1)[1])
|
||||
raise AssertionError(f"unexpected MB path {path!r}")
|
||||
|
||||
@property
|
||||
def search_calls(self):
|
||||
return [c for c in self.calls if c[0] == "recording"]
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def mb(server, monkeypatch):
|
||||
"""Install the fake transport AND enable the network flag (the test env
|
||||
disables it by default — see test_offline_default_skips_matching)."""
|
||||
fake = FakeMB()
|
||||
fake._srv = server
|
||||
monkeypatch.setattr(server, "_mb_http_get", fake)
|
||||
monkeypatch.setattr(server, "_enrich_network_enabled", lambda: True)
|
||||
return fake
|
||||
|
||||
|
||||
def _put(server, fn, title="Thunderstruck (v2)", artist="ACDC", album="",
|
||||
duration=292, year="1990"):
|
||||
server.meta_db.put(fn, 0, 0, {
|
||||
"title": title, "artist": artist, "album": album, "year": year,
|
||||
"duration": duration, "arrangements": [{"name": "Lead", "index": 0}],
|
||||
})
|
||||
|
||||
|
||||
def mb_doc(rid="rec-1", title="Thunderstruck", artist="AC/DC", artist_id="art-1",
|
||||
album="The Razors Edge", date="1990-09-24", length_ms=292000, score=100):
|
||||
return {
|
||||
"id": rid, "score": score, "title": title, "length": length_ms,
|
||||
"isrcs": ["AUAP09000045"],
|
||||
"artist-credit": [{"name": artist, "artist": {
|
||||
"id": artist_id, "name": artist, "sort-name": artist}}],
|
||||
"releases": [{"id": "rel-1", "title": album, "status": "Official",
|
||||
"date": date, "release-group": {"primary-type": "Album"}}],
|
||||
"tags": [{"name": "hard rock", "count": 7}],
|
||||
}
|
||||
|
||||
|
||||
# ── offline safety (the pytest-never-hits-network contract) ──────────────────
|
||||
|
||||
def test_offline_default_skips_matching(server, monkeypatch):
|
||||
"""Under the test env (FEEDBACK_SKIP_STARTUP_TASKS) a pass stamps hashes
|
||||
but never matches — even with a transport installed."""
|
||||
fake = FakeMB()
|
||||
fake._srv = server
|
||||
monkeypatch.setattr(server, "_mb_http_get", fake)
|
||||
_put(server, "a.sloppak")
|
||||
server._background_enrich()
|
||||
assert fake.calls == []
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["match_state"] == "unscanned"
|
||||
|
||||
|
||||
def test_real_transport_refuses_when_offline(server):
|
||||
"""_mb_http_get itself raises (before any socket) when the network is
|
||||
disabled — defence in depth under pytest."""
|
||||
with pytest.raises(server.EnrichTransportError):
|
||||
server._mb_http_get("recording", {"query": "x"})
|
||||
|
||||
|
||||
def test_transport_error_pauses_pass_without_burning_attempts(server, mb):
|
||||
_put(server, "a.sloppak")
|
||||
_put(server, "b.sloppak", title="Other Song")
|
||||
mb.raise_transport = True
|
||||
server._background_enrich()
|
||||
for fn in ("a.sloppak", "b.sloppak"):
|
||||
row = server.meta_db.get_enrichment(fn)
|
||||
assert row["match_state"] == "unscanned"
|
||||
assert row["attempts"] == 0
|
||||
# Network comes back → the next kick matches both.
|
||||
mb.raise_transport = False
|
||||
mb.search_response = {"recordings": [mb_doc()]}
|
||||
server._background_enrich()
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["match_state"] == "matched"
|
||||
|
||||
|
||||
# ── text tiers ────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_high_confidence_auto_matches_and_settles(server, mb):
|
||||
_put(server, "a.sloppak")
|
||||
mb.search_response = {"recordings": [mb_doc()]}
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "matched"
|
||||
assert row["match_source"] == "text"
|
||||
assert row["match_score"] >= 0.95
|
||||
assert row["mb_recording_id"] == "rec-1"
|
||||
assert row["canon_artist"] == "AC/DC"
|
||||
assert row["canon_title"] == "Thunderstruck"
|
||||
assert row["canon_album"] == "The Razors Edge"
|
||||
assert row["canon_year"] == "1990"
|
||||
assert row["genres"] == ["hard rock"]
|
||||
# Settled: another pass makes NO further network calls…
|
||||
n = len(mb.calls)
|
||||
server._background_enrich()
|
||||
assert len(mb.calls) == n
|
||||
# …until the identity changes, which re-matches.
|
||||
_put(server, "a.sloppak", title="Back in Black")
|
||||
mb.search_response = {"recordings": [mb_doc(rid="rec-2", title="Back in Black",
|
||||
album="Back in Black", date="1980-07-25")]}
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["mb_recording_id"] == "rec-2"
|
||||
|
||||
|
||||
def test_medium_confidence_goes_to_review_not_canonical(server, mb):
|
||||
# Partial artist agreement → medium confidence.
|
||||
_put(server, "a.sloppak", artist="AC/DC ft Nobody")
|
||||
mb.search_response = {"recordings": [mb_doc()]}
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "review"
|
||||
assert row["match_source"] == "text"
|
||||
# Review before auto-canonicalize: NO canonical values written yet.
|
||||
assert row["canon_artist"] is None
|
||||
assert row["mb_recording_id"] is None
|
||||
assert row["candidates"] and row["candidates"][0]["recording_id"] == "rec-1"
|
||||
# A review row is settled while its identity is unchanged — no re-query.
|
||||
n = len(mb.calls)
|
||||
server._background_enrich()
|
||||
assert len(mb.calls) == n
|
||||
|
||||
|
||||
def test_low_confidence_fails_with_backoff(server, mb):
|
||||
_put(server, "a.sloppak")
|
||||
mb.search_response = {"recordings": [mb_doc(rid="rec-x", title="Sunrise",
|
||||
artist="Norah Jones")]}
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "failed"
|
||||
assert row["attempts"] == 1
|
||||
assert row["last_attempt_at"] is not None
|
||||
# Immediately after, the backoff hasn't elapsed → no retry, no network.
|
||||
n = len(mb.calls)
|
||||
server._background_enrich()
|
||||
assert len(mb.calls) == n
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["attempts"] == 1
|
||||
# Rewind the clock two hours → eligible again, attempts increments.
|
||||
with server.meta_db._lock:
|
||||
server.meta_db.conn.execute(
|
||||
"UPDATE song_enrichment SET last_attempt_at = last_attempt_at - 7200")
|
||||
server.meta_db.conn.commit()
|
||||
server._background_enrich()
|
||||
assert len(mb.calls) == n + 1
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["attempts"] == 2
|
||||
|
||||
|
||||
def test_no_results_fails(server, mb):
|
||||
_put(server, "a.sloppak")
|
||||
mb.search_response = {"recordings": []}
|
||||
server._background_enrich()
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["match_state"] == "failed"
|
||||
|
||||
|
||||
# ── the content-hash match cache ──────────────────────────────────────────────
|
||||
|
||||
def test_cache_hit_copies_match_without_network(server, mb):
|
||||
_put(server, "a.sloppak")
|
||||
_put(server, "b.sloppak") # identical identity → same content_hash
|
||||
mb.search_response = {"recordings": [mb_doc()]}
|
||||
server._background_enrich()
|
||||
assert len(mb.search_calls) == 1 # ONE search covered both charts
|
||||
a = server.meta_db.get_enrichment("a.sloppak")
|
||||
b = server.meta_db.get_enrichment("b.sloppak")
|
||||
assert a["match_state"] == b["match_state"] == "matched"
|
||||
assert {a["match_source"], b["match_source"]} == {"text", "cache"}
|
||||
assert a["mb_recording_id"] == b["mb_recording_id"] == "rec-1"
|
||||
|
||||
|
||||
# ── exact keys from the manifest (tier 0 / tier 1) ────────────────────────────
|
||||
|
||||
def _write_sloppak_manifest(server, name, extra_yaml=""):
|
||||
d = server.DLC_DIR / name
|
||||
d.mkdir(parents=True)
|
||||
(d / "manifest.yaml").write_text(
|
||||
"title: Thunderstruck\nartist: AC/DC\nduration: 292\n"
|
||||
"arrangements: []\nstems: []\n" + extra_yaml,
|
||||
encoding="utf-8")
|
||||
|
||||
|
||||
def test_manifest_mbid_tier0(server, mb):
|
||||
mbid = "12345678-abcd-4ef0-9876-0123456789ab"
|
||||
_write_sloppak_manifest(server, "a.sloppak", f"mbid: {mbid}\n")
|
||||
_put(server, "a.sloppak")
|
||||
mb.recording_lookups[mbid] = mb_doc(rid=mbid)
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "matched"
|
||||
assert row["match_source"] == "mbid"
|
||||
assert row["match_score"] == 1.0
|
||||
assert row["mb_recording_id"] == mbid
|
||||
assert mb.search_calls == [] # trusted key — no text search
|
||||
|
||||
|
||||
def test_manifest_isrc_tier1(server, mb):
|
||||
_write_sloppak_manifest(server, "a.sloppak", "isrc: AUAP09000045\n")
|
||||
_put(server, "a.sloppak")
|
||||
mb.isrc_lookups["AUAP09000045"] = {"recordings": [mb_doc()]}
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "matched"
|
||||
assert row["match_source"] == "isrc"
|
||||
assert mb.search_calls == []
|
||||
|
||||
|
||||
def test_bad_manifest_mbid_falls_through_to_text(server, mb):
|
||||
mbid = "12345678-abcd-4ef0-9876-0123456789ab"
|
||||
_write_sloppak_manifest(server, "a.sloppak", f"mbid: {mbid}\n")
|
||||
_put(server, "a.sloppak")
|
||||
mb.recording_lookups.clear() # lookup 404s (typo'd manifest)
|
||||
mb.search_response = {"recordings": [mb_doc()]}
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "matched"
|
||||
assert row["match_source"] == "text"
|
||||
|
||||
|
||||
# ── manual is sacred ──────────────────────────────────────────────────────────
|
||||
|
||||
def test_manual_never_overwritten_by_matcher(server, mb):
|
||||
_put(server, "a.sloppak")
|
||||
server.meta_db.set_enrichment_manual(
|
||||
"a.sloppak", {"recording_id": "user-pick", "title": "Thunderstruck",
|
||||
"artist": "AC/DC"}, source="search")
|
||||
mb.search_response = {"recordings": [mb_doc(rid="machine-pick")]}
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "manual"
|
||||
assert row["mb_recording_id"] == "user-pick"
|
||||
# The writer refuses machine writes onto manual outright.
|
||||
ok = server.meta_db.apply_enrichment_match(
|
||||
"a.sloppak", row["content_hash"], "matched", source="text", score=1.0,
|
||||
cand={"recording_id": "machine-pick", "title": "X"})
|
||||
assert ok is False
|
||||
|
||||
|
||||
# ── review routes ─────────────────────────────────────────────────────────────
|
||||
|
||||
def _seed_review(server, mb, fn="a.sloppak", title="Thunderstruck (v2)"):
|
||||
# Distinct raw titles give distinct content hashes (else the match cache
|
||||
# legitimately copies an earlier row instead of running the text tiers).
|
||||
_put(server, fn, title=title, artist="AC/DC ft Nobody")
|
||||
mb.search_response = {"recordings": [mb_doc()]}
|
||||
server._background_enrich()
|
||||
assert server.meta_db.get_enrichment(fn)["match_state"] == "review"
|
||||
|
||||
|
||||
def test_review_queue_route(server, mb, client):
|
||||
_seed_review(server, mb)
|
||||
body = client.get("/api/enrichment/review").json()
|
||||
assert body["total_review"] == 1
|
||||
assert body["songs"][0]["filename"] == "a.sloppak"
|
||||
assert body["songs"][0]["candidates"][0]["recording_id"] == "rec-1"
|
||||
|
||||
|
||||
def test_review_accept_route(server, mb, client):
|
||||
_seed_review(server, mb)
|
||||
r = client.post("/api/enrichment/review/a.sloppak/accept",
|
||||
json={"recording_id": "rec-1"})
|
||||
assert r.status_code == 200
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "manual"
|
||||
assert row["match_source"] == "review"
|
||||
assert row["canon_artist"] == "AC/DC"
|
||||
assert client.get("/api/enrichment/review").json()["total_review"] == 0
|
||||
# Accepting a candidate that isn't in the stored list → 404.
|
||||
_seed_review(server, mb, fn="b.sloppak", title="Thunderstruck (Live)")
|
||||
r = client.post("/api/enrichment/review/b.sloppak/accept",
|
||||
json={"recording_id": "nope"})
|
||||
assert r.status_code == 404
|
||||
|
||||
|
||||
def test_review_reject_route_never_retries(server, mb, client):
|
||||
_seed_review(server, mb)
|
||||
r = client.post("/api/enrichment/review/a.sloppak/reject")
|
||||
assert r.status_code == 200
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "failed"
|
||||
assert row["match_source"] == "rejected"
|
||||
# Rejected rows are excluded from the retry backoff forever…
|
||||
n = len(mb.calls)
|
||||
server._background_enrich()
|
||||
assert len(mb.calls) == n
|
||||
# …but an identity edit re-queues (the user fixed the metadata).
|
||||
_put(server, "a.sloppak", artist="AC/DC")
|
||||
server._background_enrich()
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["match_state"] == "matched"
|
||||
# Rejecting a manual row is refused.
|
||||
r = client.post("/api/enrichment/review/a.sloppak/reject")
|
||||
assert r.status_code == 200 # matched → rejectable
|
||||
client.post("/api/enrichment/review/a.sloppak/pick",
|
||||
json={"candidate": {"recording_id": "rec-9", "title": "T"}})
|
||||
r = client.post("/api/enrichment/review/a.sloppak/reject")
|
||||
assert r.status_code == 404
|
||||
|
||||
|
||||
def test_pick_route_fix_match(server, mb, client):
|
||||
_put(server, "a.sloppak")
|
||||
r = client.post("/api/enrichment/review/a.sloppak/pick", json={"candidate": {
|
||||
"recording_id": "rec-77", "title": "Thunderstruck", "artist": "AC/DC",
|
||||
"album": "The Razors Edge", "year": "1990", "genres": ["hard rock"],
|
||||
"junk_key": "dropped"}})
|
||||
assert r.status_code == 200
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "manual"
|
||||
assert row["match_source"] == "search"
|
||||
assert row["mb_recording_id"] == "rec-77"
|
||||
# Malformed candidate → 400; unknown song → 404.
|
||||
assert client.post("/api/enrichment/review/a.sloppak/pick",
|
||||
json={"candidate": {"title": "no id"}}).status_code == 400
|
||||
assert client.post("/api/enrichment/review/ghost.sloppak/pick",
|
||||
json={"candidate": {"recording_id": "x", "title": "t"}}
|
||||
).status_code == 404
|
||||
|
||||
|
||||
def test_search_proxy(server, mb, client, monkeypatch):
|
||||
_put(server, "a.sloppak")
|
||||
mb.search_response = {"recordings": [mb_doc()]}
|
||||
assert client.get("/api/enrichment/search").status_code == 400
|
||||
body = client.get("/api/enrichment/search",
|
||||
params={"title": "Thunderstruck", "artist": "AC/DC",
|
||||
"filename": "a.sloppak"}).json()
|
||||
assert body["candidates"][0]["recording_id"] == "rec-1"
|
||||
assert body["candidates"][0]["score"] > 0.9
|
||||
# Transport failure surfaces as 503, not a 500.
|
||||
def _down(path, params):
|
||||
raise server.EnrichTransportError("down")
|
||||
monkeypatch.setattr(server, "_mb_http_get", _down)
|
||||
r = client.get("/api/enrichment/search", params={"title": "x"})
|
||||
assert r.status_code == 503
|
||||
|
||||
|
||||
# ── the match facet on /api/library + /api/library/stats ─────────────────────
|
||||
|
||||
def test_match_facet_filters_grid_and_stats(server, mb, client, monkeypatch):
|
||||
_put(server, "auto.sloppak") # → matched
|
||||
_put(server, "rev.sloppak", title="Revsong", artist="AC/DC ft Nobody") # → review
|
||||
_put(server, "fail.sloppak", title="Failsong", artist="Zzz") # → failed
|
||||
_put(server, "pend.sloppak", title="Pendsong", artist="Yyy") # stays unscanned
|
||||
|
||||
def _routed(path, params):
|
||||
q = params.get("query", "")
|
||||
if "thunderstruck" in q:
|
||||
return {"recordings": [mb_doc()]}
|
||||
if "revsong" in q:
|
||||
return {"recordings": [mb_doc(rid="rec-r", title="Revsong")]}
|
||||
return {"recordings": []}
|
||||
monkeypatch.setattr(server, "_mb_http_get", _routed)
|
||||
server._background_enrich()
|
||||
# Pendsong got failed by the pass (no results); reset it to unscanned to
|
||||
# represent the not-yet-scanned band.
|
||||
with server.meta_db._lock:
|
||||
server.meta_db.conn.execute(
|
||||
"UPDATE song_enrichment SET match_state='unscanned', attempts=0, "
|
||||
"last_attempt_at=NULL WHERE filename='pend.sloppak'")
|
||||
server.meta_db.conn.commit()
|
||||
|
||||
def names(match):
|
||||
return sorted(s["filename"] for s in client.get(
|
||||
"/api/library", params={"match": match, "size": 50}).json()["songs"])
|
||||
|
||||
assert names("review") == ["rev.sloppak"]
|
||||
assert names("matched") == ["auto.sloppak"]
|
||||
assert names("unmatched") == ["fail.sloppak"]
|
||||
assert names("pending") == ["pend.sloppak"]
|
||||
assert names("review,matched") == ["auto.sloppak", "rev.sloppak"]
|
||||
# Stats agree with the grid (the rail's lockstep contract).
|
||||
total = client.get("/api/library/stats",
|
||||
params={"match": "review"}).json()["total_songs"]
|
||||
assert total == 1
|
||||
# Unknown values are ignored → unfiltered.
|
||||
assert len(names("bogus")) == 4
|
||||
|
||||
|
||||
def test_status_counts_by_state(server, mb, client):
|
||||
_seed_review(server, mb)
|
||||
states = client.get("/api/enrichment/status").json()["states"]
|
||||
assert states.get("review") == 1
|
||||
|
||||
|
||||
# ── settings: enable toggle + auto-apply confidence ───────────────────────────
|
||||
|
||||
def test_auto_threshold_setting_moves_the_auto_review_boundary(server, mb, client):
|
||||
# artist exact (1.0) + title 4/5 token overlap (0.8) and NO year/duration
|
||||
# corroboration → combined exactly 0.90.
|
||||
mb.search_response = {"recordings": [
|
||||
mb_doc(title="Highway Hell", artist="AC/DC", date="", length_ms=None)]}
|
||||
client.post("/api/settings", json={"enrich_auto_threshold": 0.95})
|
||||
_put(server, "a.sloppak", title="Highway to Hell", artist="AC/DC",
|
||||
year="", duration=0)
|
||||
server._background_enrich()
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["match_state"] == "review"
|
||||
# Lower the bar to the default 0.90 → an identity edit re-queues, and the
|
||||
# same 0.90-scored candidate now auto-applies.
|
||||
client.post("/api/settings", json={"enrich_auto_threshold": 0.9})
|
||||
_put(server, "a.sloppak", title="Highway to Hell", artist="AC/DC",
|
||||
year="", duration=0, album="Different Album")
|
||||
server._background_enrich()
|
||||
row = server.meta_db.get_enrichment("a.sloppak")
|
||||
assert row["match_state"] == "matched"
|
||||
assert abs(row["match_score"] - 0.9) < 1e-6
|
||||
|
||||
|
||||
def test_enrich_enabled_setting_gates_background_matching(server, mb, client):
|
||||
client.post("/api/settings", json={"enrich_enabled": False})
|
||||
_put(server, "a.sloppak")
|
||||
mb.search_response = {"recordings": [mb_doc()]}
|
||||
server._background_enrich()
|
||||
assert mb.calls == []
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["match_state"] == "unscanned"
|
||||
# Manual search/fix stays available while the background matcher is off.
|
||||
r = client.get("/api/enrichment/search", params={"title": "Thunderstruck"})
|
||||
assert r.status_code == 200
|
||||
# Re-enable → the next pass matches.
|
||||
client.post("/api/settings", json={"enrich_enabled": True})
|
||||
server._background_enrich()
|
||||
assert server.meta_db.get_enrichment("a.sloppak")["match_state"] == "matched"
|
||||
|
||||
|
||||
def test_settings_validation(server, client):
|
||||
assert "error" in client.post(
|
||||
"/api/settings", json={"enrich_enabled": "yes"}).json()
|
||||
assert "error" in client.post(
|
||||
"/api/settings", json={"enrich_auto_threshold": "high"}).json()
|
||||
assert "error" in client.post(
|
||||
"/api/settings", json={"enrich_auto_threshold": 2.5}).json()
|
||||
ok = client.post("/api/settings", json={"enrich_auto_threshold": 1.01}).json()
|
||||
assert "error" not in ok
|
||||
assert client.get("/api/settings").json()["enrich_auto_threshold"] == 1.01
|
||||
|
||||
|
||||
def test_kick_route(server, mb, client):
|
||||
import time as _t
|
||||
body = client.post("/api/enrichment/kick").json()
|
||||
assert "started" in body
|
||||
# Let the kicked pass settle so its daemon thread can't bleed into the
|
||||
# fixture teardown (the DB connection closes there).
|
||||
for _ in range(200):
|
||||
if not client.get("/api/enrichment/status").json()["running"]:
|
||||
break
|
||||
_t.sleep(0.02)
|
||||
|
||||
|
||||
def test_review_queue_orders_missing_data_first(server, mb, client):
|
||||
# Complete chart first alphabetically, incomplete second — the queue must
|
||||
# surface the incomplete (missing album + year) one first anyway.
|
||||
_seed_review(server, mb, fn="aa.sloppak", title="Thunderstruck (v2)")
|
||||
_put(server, "zz.sloppak", title="Thunderstruck (Live)",
|
||||
artist="AC/DC ft Nobody", album="", year="")
|
||||
server._background_enrich()
|
||||
assert server.meta_db.get_enrichment("zz.sloppak")["match_state"] == "review"
|
||||
songs = client.get("/api/enrichment/review").json()["songs"]
|
||||
assert [s["filename"] for s in songs] == ["zz.sloppak", "aa.sloppak"]
|
||||
@@ -0,0 +1,227 @@
|
||||
"""Unit tests for lib/mb_match.py — the pure text-matching engine (P8).
|
||||
|
||||
No network, no database, no server import: denoise/tokenize, similarity,
|
||||
scoring + tier classification, Lucene query building, and MusicBrainz
|
||||
response parsing are all exercised as plain functions.
|
||||
"""
|
||||
|
||||
import mb_match as m
|
||||
|
||||
|
||||
# ── denoise / tokenize ────────────────────────────────────────────────────────
|
||||
|
||||
def test_denoise_lowercases_and_strips_punct_and_diacritics():
|
||||
assert m.denoise("Motörhead") == "motorhead"
|
||||
assert m.denoise("Beyoncé!!") == "beyonce"
|
||||
assert m.denoise("Guns N' Roses") == "guns n roses"
|
||||
assert m.denoise(" Weird spacing ") == "weird spacing"
|
||||
|
||||
|
||||
def test_denoise_strips_noise_parentheticals():
|
||||
# The design's explicit list: author suffixes + (440Hz)/(Live)/(No Lead)/(v2).
|
||||
assert m.denoise("Thunderstruck (440Hz)") == "thunderstruck"
|
||||
assert m.denoise("Thunderstruck (Live)") == "thunderstruck"
|
||||
assert m.denoise("Thunderstruck (No Lead)") == "thunderstruck"
|
||||
assert m.denoise("Thunderstruck (v2)") == "thunderstruck"
|
||||
assert m.denoise("Thunderstruck [Remastered 2012]") == "thunderstruck"
|
||||
assert m.denoise("One (Live at Wembley)") == "one"
|
||||
|
||||
|
||||
def test_denoise_strips_author_credits():
|
||||
assert m.denoise("Back in Black (by SomeCharter)") == "back in black"
|
||||
assert m.denoise("Back in Black (charted by X99)") == "back in black"
|
||||
assert m.denoise("Back in Black - by SomeCharter") == "back in black"
|
||||
|
||||
|
||||
def test_denoise_keeps_meaningful_parentheticals():
|
||||
# A parenthetical with no noise term survives (both sides get the same
|
||||
# treatment, so symmetric content still matches).
|
||||
assert m.denoise("Doin' It (All for My Baby)") == "doin it all for my baby"
|
||||
|
||||
|
||||
def test_denoise_leading_the_is_artist_only():
|
||||
assert m.denoise("The Beatles", strip_leading_the=True) == "beatles"
|
||||
# Titles keep their "The" — never strip it there.
|
||||
assert m.denoise("The Trooper") == "the trooper"
|
||||
|
||||
|
||||
def test_ampersand_folds_to_and():
|
||||
assert m.similarity("Angus & Julia Stone", "Angus and Julia Stone", artist=True) == 1.0
|
||||
|
||||
|
||||
# ── similarity ────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_similarity_exact_and_empty():
|
||||
assert m.similarity("Back in Black", "Back In Black!") == 1.0
|
||||
assert m.similarity("", "Anything") == 0.0
|
||||
assert m.similarity(None, None) == 0.0
|
||||
|
||||
|
||||
def test_similarity_folds_spelling_drift_via_compaction():
|
||||
# The headline case: ACDC / AC DC / AC/DC all name the same artist.
|
||||
assert m.similarity("ACDC", "AC/DC", artist=True) == 1.0
|
||||
assert m.similarity("AC DC", "ACDC", artist=True) == 1.0
|
||||
assert m.similarity("Greenday", "Green Day", artist=True) == 1.0
|
||||
|
||||
|
||||
def test_similarity_partial_overlap():
|
||||
s = m.similarity("Highway to Hell", "Highway Hell")
|
||||
assert 0.7 < s < 1.0
|
||||
assert m.similarity("Back in Black", "Paint It Black") < 0.5
|
||||
|
||||
|
||||
# ── scoring + tiers ───────────────────────────────────────────────────────────
|
||||
|
||||
SONG = {"artist": "ACDC", "title": "Thunderstruck (v2)", "album": "The Razors Edge",
|
||||
"year": "1990", "duration": 292}
|
||||
|
||||
|
||||
def test_score_exact_match_is_high():
|
||||
cand = {"artist": "AC/DC", "title": "Thunderstruck", "year": "1990", "duration": 292}
|
||||
s = m.score_candidate(SONG, cand)
|
||||
assert s == 1.0
|
||||
assert m.classify(SONG, cand, s) == "auto"
|
||||
|
||||
|
||||
def test_score_cover_never_auto():
|
||||
# Perfect title, wrong artist (a cover) — must not auto-match.
|
||||
cand = {"artist": "Some Cover Band", "title": "Thunderstruck"}
|
||||
s = m.score_candidate(SONG, cand)
|
||||
assert m.classify(SONG, cand, s) != "auto"
|
||||
|
||||
|
||||
def test_missing_artist_caps_at_review():
|
||||
song = {"artist": "", "title": "Thunderstruck", "duration": 292}
|
||||
cand = {"artist": "AC/DC", "title": "Thunderstruck", "duration": 292}
|
||||
s = m.score_candidate(song, cand)
|
||||
# artist half scores 0 → combined ≤ 0.55 + bonuses → review at best.
|
||||
assert m.classify(song, cand, s) != "auto"
|
||||
|
||||
|
||||
def test_year_and_duration_corroborate():
|
||||
# Fuzzy title so the base sits below the 1.0 cap and bonuses are visible.
|
||||
base = {"artist": "AC/DC", "title": "Thunderstruck Thunder"}
|
||||
plain = m.score_candidate(SONG, base)
|
||||
with_year = m.score_candidate(SONG, dict(base, year="1990"))
|
||||
with_dur = m.score_candidate(SONG, dict(base, duration=290))
|
||||
assert with_year > plain
|
||||
assert with_dur > plain
|
||||
|
||||
|
||||
def test_fuzzy_title_with_corroboration_lands_review_or_auto():
|
||||
cand = {"artist": "AC/DC", "title": "Thunderstruck Thunder"}
|
||||
s = m.score_candidate(SONG, cand)
|
||||
assert m.classify(SONG, cand, s) in ("review", "auto")
|
||||
|
||||
|
||||
def test_unrelated_is_none():
|
||||
cand = {"artist": "Norah Jones", "title": "Sunrise"}
|
||||
s = m.score_candidate(SONG, cand)
|
||||
assert m.classify(SONG, cand, s) == "none"
|
||||
|
||||
|
||||
def test_classify_auto_min_override():
|
||||
# The host's "auto-apply confidence" setting: a perfect match autos at
|
||||
# any real threshold, and "Always review" (>1.0) sends even it to review.
|
||||
cand = {"artist": "AC/DC", "title": "Thunderstruck", "year": "1990", "duration": 292}
|
||||
s = m.score_candidate(SONG, cand)
|
||||
assert s == 1.0
|
||||
assert m.classify(SONG, cand, s, auto_min=0.9) == "auto"
|
||||
assert m.classify(SONG, cand, s, auto_min=1.01) == "review"
|
||||
# The per-field floors are independent of the threshold: a wrong-artist
|
||||
# cover stays non-auto even at a permissive auto_min.
|
||||
cover = {"artist": "Some Cover Band", "title": "Thunderstruck"}
|
||||
cs = m.score_candidate(SONG, cover)
|
||||
assert m.classify(SONG, cover, cs, auto_min=0.5) != "auto"
|
||||
|
||||
|
||||
def test_rank_candidates_orders_by_our_score():
|
||||
cands = [
|
||||
{"recording_id": "b", "artist": "Someone Else", "title": "Thunderstruck", "mb_score": 100},
|
||||
{"recording_id": "a", "artist": "AC/DC", "title": "Thunderstruck", "mb_score": 90},
|
||||
]
|
||||
ranked = m.rank_candidates(SONG, cands)
|
||||
assert [c["recording_id"] for c in ranked] == ["a", "b"]
|
||||
assert all("score" in c for c in ranked)
|
||||
|
||||
|
||||
# ── query building ────────────────────────────────────────────────────────────
|
||||
|
||||
def test_build_recording_query_denoises_and_quotes():
|
||||
q = m.build_recording_query("ACDC", 'Thunderstruck (v2)')
|
||||
assert q == 'recording:"thunderstruck" AND artist:"acdc"'
|
||||
|
||||
|
||||
def test_build_recording_query_escapes_and_handles_missing_artist():
|
||||
q = m.build_recording_query("", 'Say "Hello"')
|
||||
# Quotes are punct-stripped by denoise, so nothing to escape here — but
|
||||
# the artist clause must be absent entirely.
|
||||
assert q.startswith('recording:"')
|
||||
assert "artist:" not in q
|
||||
|
||||
|
||||
# ── MusicBrainz response parsing ──────────────────────────────────────────────
|
||||
|
||||
MB_DOC = {
|
||||
"id": "rec-123",
|
||||
"score": 98,
|
||||
"title": "Thunderstruck",
|
||||
"length": 292773,
|
||||
"isrcs": ["AUAP09000045"],
|
||||
"artist-credit": [
|
||||
{"name": "AC/DC", "joinphrase": "",
|
||||
"artist": {"id": "art-1", "name": "AC/DC", "sort-name": "AC/DC"}},
|
||||
],
|
||||
"releases": [
|
||||
{"id": "rel-compilation", "title": "Greatest Hits", "status": "Official",
|
||||
"date": "2005-01-01", "release-group": {"primary-type": "Compilation"}},
|
||||
{"id": "rel-album", "title": "The Razors Edge", "status": "Official",
|
||||
"date": "1990-09-24", "release-group": {"primary-type": "Album"}},
|
||||
{"id": "rel-boot", "title": "Bootleg", "status": "Bootleg",
|
||||
"date": "1989-01-01", "release-group": {"primary-type": "Album"}},
|
||||
],
|
||||
"tags": [{"name": "hard rock", "count": 10}, {"name": "rock", "count": 4}],
|
||||
}
|
||||
|
||||
|
||||
def test_parse_recording_doc_normalizes():
|
||||
c = m.parse_recording_doc(MB_DOC)
|
||||
assert c["recording_id"] == "rec-123"
|
||||
assert c["title"] == "Thunderstruck"
|
||||
assert c["artist"] == "AC/DC"
|
||||
assert c["artist_id"] == "art-1"
|
||||
# Official Album beats the compilation and the bootleg.
|
||||
assert c["album"] == "The Razors Edge"
|
||||
assert c["release_id"] == "rel-album"
|
||||
assert c["year"] == "1990"
|
||||
assert c["duration"] == 293
|
||||
assert c["isrc"] == "AUAP09000045"
|
||||
assert c["genres"] == ["hard rock", "rock"]
|
||||
assert c["mb_score"] == 98
|
||||
|
||||
|
||||
def test_parse_recording_doc_joined_artist_credit():
|
||||
doc = dict(MB_DOC)
|
||||
doc["artist-credit"] = [
|
||||
{"name": "Queen", "joinphrase": " & ",
|
||||
"artist": {"id": "q", "name": "Queen", "sort-name": "Queen"}},
|
||||
{"name": "David Bowie",
|
||||
"artist": {"id": "b", "name": "David Bowie", "sort-name": "Bowie, David"}},
|
||||
]
|
||||
c = m.parse_recording_doc(doc)
|
||||
assert c["artist"] == "Queen & David Bowie"
|
||||
assert c["artist_id"] == "q"
|
||||
|
||||
|
||||
def test_parse_recording_doc_rejects_malformed():
|
||||
assert m.parse_recording_doc({}) is None
|
||||
assert m.parse_recording_doc({"id": "x"}) is None
|
||||
assert m.parse_recording_doc(None) is None
|
||||
|
||||
|
||||
def test_parse_search_response():
|
||||
body = {"recordings": [MB_DOC, {"bogus": True}]}
|
||||
cands = m.parse_search_response(body)
|
||||
assert len(cands) == 1
|
||||
assert m.parse_search_response({}) == []
|
||||
assert m.parse_search_response(None) == []
|
||||
Reference in New Issue
Block a user