diff --git a/README.md b/README.md index 615bf2a..6fb41da 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,14 @@ Two tiers, and no third. | `name` | Normalised artist and title | Everything the first tier could not carry | | `none` | — | Recorded as a miss, never guessed at | -The normalisation is the load-bearing part, because the two sides disagree in +The name tier does almost all of the work. A recording MBID is exact when it +lands, but MusicBrainz holds a separate recording per release, and Last.fm and +Lidarr rarely pick the same one: on a real library, two thirds of scrobbles +carry a recording id and barely a twentieth of them join on it. The report +counts how many carried an id and matched on name anyway, which is the measure +of that disagreement. + +So the normalisation is the load-bearing part, because the two sides disagree in predictable ways. It folds case and accents, drops guest credits (`Yellowcard feat. Tay Jardine` against a tag of `Yellowcard`), strips a trailing version suffix (`(Remastered 2011)`, `- Live`), expands `&`, and removes a diff --git a/music_curator.py b/music_curator.py index 21677fe..4a27a30 100644 --- a/music_curator.py +++ b/music_curator.py @@ -187,18 +187,28 @@ VERSION_WORDS = ( "anniversary", "reissue", "instrumental", + # Drum and bass and its neighbours mark versions their own way. + "vip", + "bootleg", + "rework", + "extended", ) _VERSIONS = "|".join(VERSION_WORDS) BRACKETED_VERSION = re.compile( rf"\s*[\(\[][^\)\]]*\b(?:{_VERSIONS})\b[^\)\]]*[\)\]]\s*$", re.IGNORECASE ) -TRAILING_VERSION = re.compile(rf"\s+-\s+[^-]*\b(?:{_VERSIONS})\b.*$", re.IGNORECASE) +# The suffix is matched lazily rather than as a run of non-hyphens, because the +# thing being stripped frequently contains hyphens of its own -- "Gold Dust - +# Shy FX Re-Edit", "Back To Your Roots - Friction & K-Tee Remix". +TRAILING_VERSION = re.compile(rf"\s+-\s+.*?\b(?:{_VERSIONS})\b.*$", re.IGNORECASE) -# Last.fm routinely carries the guest credit in the artist field where the file -# tag holds only the primary artist -- "Yellowcard feat. Tay Jardine" against a -# tag of "Yellowcard". `with` is deliberately absent: it appears in far too many -# real titles to cut on sight. -GUEST_CREDIT = re.compile(r"\s+(?:feat|ft|featuring)\b.*$", re.IGNORECASE) +# Last.fm routinely carries the guest credit where the file tag holds only the +# primary artist -- "Yellowcard feat. Tay Jardine" against a tag of +# "Yellowcard", or "Self vs Self (feat. In Flames)" against "Self vs Self". The +# opening bracket has to be allowed for: requiring whitespace immediately before +# the word misses every bracketed credit, which is most of them. `with` is +# deliberately absent -- it appears in far too many real titles to cut on sight. +GUEST_CREDIT = re.compile(r"[\s(\[]+(?:feat|ft|featuring)\b.*$", re.IGNORECASE) LEADING_ARTICLE = re.compile(r"^the\s+") # Deleted rather than spaced, so "Don't" and "Dont" agree. Every other mark @@ -1078,6 +1088,31 @@ def coverage_report(store): row["plays"], ) + # Why the mbid tier performs the way it does. A recording id on both sides + # that still fails to join means the two disagree about which recording the + # song is -- MusicBrainz holds a separate recording per release, and Last.fm + # and Lidarr need not have picked the same one. That is not a fault to fix + # in the matcher; it is the reason the name tier has to carry the load. + library_with_mbid = store.scalar( + "SELECT COUNT(*) FROM lidarr_track WHERE recording_mbid IS NOT NULL" + ) + logger.info( + "library tracks carrying a recording MBID: %d of %d (%.1f%%)", + library_with_mbid, + tracks, + 100 * library_with_mbid / tracks, + ) + disagreed = store.connection.execute( + "SELECT COUNT(*) AS pairs, COALESCE(SUM(plays), 0) AS plays FROM scrobble_key" + " WHERE track_mbid IS NOT NULL AND method = 'name'" + ).fetchone() + logger.info( + "carried a recording MBID, joined on name instead: %d pairs, %d plays" + " -- both sides know the song, they disagree on which recording it is", + disagreed["pairs"], + disagreed["plays"], + ) + suspect = store.connection.execute( "SELECT COUNT(*) AS pairs, COALESCE(SUM(plays), 0) AS plays FROM scrobble_key k" " WHERE k.track_id IS NULL" diff --git a/tests/test_music_curator.py b/tests/test_music_curator.py index 3c679e9..e376fa9 100644 --- a/tests/test_music_curator.py +++ b/tests/test_music_curator.py @@ -687,3 +687,48 @@ def test_lidarr_talks_to_a_real_server_through_keep_alive(http_server): client.transport.close() assert server.connections == 1 + + +# Titles taken verbatim from a real coverage report's unmatched list. Each one +# was a genuine miss before the normaliser handled it. +@pytest.mark.parametrize( + ("scrobbled", "tagged"), + [ + ("Self vs Self (feat. In Flames)", "Self vs Self"), + ("Grime Battle of Hastings (feat. The Town Crier)", "Grime Battle of Hastings"), + ("Gold Dust - Shy FX Re-Edit", "Gold Dust"), + ("Back To Your Roots - Friction & K-Tee Remix", "Back To Your Roots"), + ("Constellations - Forza Horizon 3 VIP", "Constellations"), + ("Everyday (Netsky Remix)", "Everyday"), + ("Voodoo People [Pendulum Remix] [Live At Brixton Academy]", "Voodoo People"), + ], +) +def test_real_unmatched_titles_now_agree_with_their_tags(scrobbled, tagged): + assert music_curator.normalise(scrobbled) == music_curator.normalise(tagged) + + +@pytest.mark.parametrize( + "title", + [ + "Dancing with Myself", + "(Don't Fear) The Reaper", + "Live and Let Die", + "Radio Ga Ga", + "Editors", + "Mixed Emotions", + "Vipassana", + ], +) +def test_the_version_words_do_not_eat_ordinary_titles(title): + """Every one of these contains a version word and must survive intact.""" + assert music_curator.normalise(title) == music_curator.normalise(title.lower()) + assert len(music_curator.normalise(title).split()) == len(title.split()) + + +def test_a_bracketed_guest_credit_matches_the_bare_tag(tmp_path): + """Whitespace-then-feat misses the bracketed form, which is most of them.""" + store = indexed(tmp_path, [scrobble_of("Yellowcard", "Here I Am Alive (feat. Someone)")]) + + method, track_id = verdict(store, "Yellowcard", "Here I Am Alive (feat. Someone)") + assert method == "name" + assert track_id is not None