diff --git a/README.md b/README.md index eebe9c4..852ccb9 100644 --- a/README.md +++ b/README.md @@ -110,17 +110,21 @@ unmatched by an artist the library holds: N pairs, M plays ``` That is a track that was played by an artist the library holds. It is then -split again, because owning an artist is a weak proxy for owning a track: +split three ways, because owning an artist is a weak proxy for owning a track +and a shared title is a weak proxy for a shared song: -- **the title exists under another artist** — an attribution disagreement, a - remixer or a guest billed as the artist. These are the genuine misses, and - each is a candidate for being wrongly called cold in stage four. The report - names the artist the library files them under. -- **the title is nowhere in the library** — never bought. No amount of matching - conjures a file that does not exist. +- **the library's own title credits the scrobbled artist** — `Voodoo People + (Pendulum Remix)` against a play credited to Pendulum. Same song, filed + under the original artist. These are the genuine misses. +- **the same title under an unrelated artist** — a collision, not a miss. + Across fifty thousand tracks these are constant: `Everyday` is Rusko and + also Def Leppard, `Kaleidoscope` is Delta Heavy and also Chappell Roan. + Matching on title alone would be far worse than missing them, which is why + there is no such tier. +- **the title is nowhere in the library** — never bought. -Counting both as matcher failures overstates the problem and would over-block -the cull. The report lists the worst of each by play count. +Only the first is worth chasing. Counting all three as matcher failures +overstates the problem and would over-block the cull. ## How the ingest works diff --git a/music_curator.py b/music_curator.py index 6186694..55546f4 100644 --- a/music_curator.py +++ b/music_curator.py @@ -141,6 +141,11 @@ CREATE TABLE IF NOT EXISTS lidarr_track ( ); CREATE INDEX IF NOT EXISTS lidarr_track_recording ON lidarr_track (recording_mbid); CREATE INDEX IF NOT EXISTS lidarr_track_norm ON lidarr_track (norm_artist, norm_title); +-- Separate from the composite above, which a lookup by title alone cannot use: +-- its leading column is the artist. The report searches by title on its own, and +-- without this it scans every track for every unmatched key -- forty-seven +-- seconds on a library of eighty-four thousand. +CREATE INDEX IF NOT EXISTS lidarr_track_title ON lidarr_track (norm_title); CREATE INDEX IF NOT EXISTS lidarr_track_album ON lidarr_track (album_id); -- One row per distinct thing listened to, with the verdict on whether it could @@ -1125,47 +1130,68 @@ def coverage_report(store): ) # Owning an artist is a weak proxy for owning a track, so that figure alone - # overstates the matcher's failings. Split it. A title the library holds - # under some other artist is an attribution disagreement -- a remixer - # credited as the artist, a guest billed as one -- and is a real miss. A - # title the library does not hold at all was simply never bought, and no - # amount of matching will conjure it. + # overstates the matcher's failings. Split it -- but not on the title alone. + # Across fifty thousand tracks, titles collide constantly: "Everyday" is + # Rusko and also Def Leppard, "Kaleidoscope" is Delta Heavy and also + # Chappell Roan. Matching those would be worse than missing them. + # + # The signal for a genuine attribution miss is that the library's own title + # credits the artist the scrobble is filed under -- "Voodoo People (Pendulum + # Remix)" against a play credited to Pendulum. The normalised title has that + # suffix stripped, which is exactly what let them meet, so the raw one has to + # be searched for the name. attribution = store.connection.execute( + "SELECT COUNT(*) AS pairs, COALESCE(SUM(plays), 0) AS plays FROM scrobble_key k" + " WHERE k.track_id IS NULL" + " AND EXISTS (SELECT 1 FROM lidarr_artist a WHERE a.norm_name = k.norm_artist)" + " AND EXISTS (SELECT 1 FROM lidarr_track t" + " WHERE t.norm_title = k.norm_track" + " AND instr(lower(t.title), lower(k.artist)) > 0)" + ).fetchone() + collision = store.connection.execute( "SELECT COUNT(*) AS pairs, COALESCE(SUM(plays), 0) AS plays FROM scrobble_key k" " WHERE k.track_id IS NULL" " AND EXISTS (SELECT 1 FROM lidarr_artist a WHERE a.norm_name = k.norm_artist)" " AND EXISTS (SELECT 1 FROM lidarr_track t WHERE t.norm_title = k.norm_track)" ).fetchone() logger.info( - " of those, the title exists under another artist: %d pairs, %d plays" - " -- attribution disagreements, and the genuine misses", + " the library's title credits the scrobbled artist: %d pairs, %d plays" + " -- remixes and guest spots, and the genuine misses", attribution["pairs"], attribution["plays"], ) + logger.info( + " same title under an unrelated artist: %d pairs, %d plays" + " -- title collisions, not misses; matching these would be a mistake", + collision["pairs"] - attribution["pairs"], + collision["plays"] - attribution["plays"], + ) logger.info( " the rest, %d pairs, %d plays: you own the artist but not the track", - suspect["pairs"] - attribution["pairs"], - suspect["plays"] - attribution["plays"], + suspect["pairs"] - collision["pairs"], + suspect["plays"] - collision["plays"], ) mismatched = store.connection.execute( "SELECT k.artist, k.track, k.plays," - " (SELECT a.name FROM lidarr_track t" - " JOIN lidarr_artist a ON a.id = t.artist_id" - " WHERE t.norm_title = k.norm_track LIMIT 1) AS filed_under" + " (SELECT t.title FROM lidarr_track t" + " WHERE t.norm_title = k.norm_track" + " AND instr(lower(t.title), lower(k.artist)) > 0 LIMIT 1) AS library_title" " FROM scrobble_key k" " WHERE k.track_id IS NULL" " AND EXISTS (SELECT 1 FROM lidarr_artist a WHERE a.norm_name = k.norm_artist)" - " AND EXISTS (SELECT 1 FROM lidarr_track t WHERE t.norm_title = k.norm_track)" + " AND EXISTS (SELECT 1 FROM lidarr_track t" + " WHERE t.norm_title = k.norm_track" + " AND instr(lower(t.title), lower(k.artist)) > 0)" " ORDER BY k.plays DESC, k.artist LIMIT 10" ).fetchall() for position, row in enumerate(mismatched, start=1): logger.info( - " attribution %2d: %-45s %4d plays, filed under %s", + " attribution %2d: %-45s %4d plays, library has %r", position, f"{row['artist']} - {row['track']}"[:45], row["plays"], - row["filed_under"], + row["library_title"], ) with_files = store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE has_file = 1") diff --git a/tests/test_music_curator.py b/tests/test_music_curator.py index 0a2e776..4ecfa85 100644 --- a/tests/test_music_curator.py +++ b/tests/test_music_curator.py @@ -55,9 +55,23 @@ LIBRARY = [ { "name": "The Prodigy", "albums": [ - {"title": "The Fat of the Land", "tracks": [{"title": "Breathe (Remastered)"}]} + { + "title": "The Fat of the Land", + "tracks": [ + {"title": "Breathe (Remastered)"}, + # Credits its remixer in the title, which is the only signal + # separating a real attribution miss from a title collision. + {"title": "Voodoo People (Pendulum Remix)"}, + ], + } ], }, + # Held by the library in its own right, which is what puts its scrobbles + # inside the "artist the library holds" filter at all. + { + "name": "Pendulum", + "albums": [{"title": "Immersion", "tracks": [{"title": "Watercolour"}]}], + }, ] @@ -393,7 +407,7 @@ def test_keys_carry_the_play_count_and_the_span(tmp_path): def test_re_indexing_drops_what_lidarr_no_longer_has(tmp_path): """The index is Lidarr's mirror, not an accumulation of everything ever seen.""" store = indexed(tmp_path, [scrobble_of("AC/DC", "Hells Bells")]) - assert store.scalar("SELECT COUNT(*) FROM lidarr_artist") == 3 + assert store.scalar("SELECT COUNT(*) FROM lidarr_artist") == 4 music_curator.index_library( music_curator.Lidarr("http://lidarr", "key", transport=FakeLidarr(LIBRARY[:1])), store @@ -512,7 +526,7 @@ def test_albums_come_from_the_unfiltered_endpoint(tmp_path): album_calls = [query for path, query in api.calls if path == "album"] assert album_calls == [{}] - assert store.scalar("SELECT COUNT(*) FROM lidarr_album") == 3 + assert store.scalar("SELECT COUNT(*) FROM lidarr_album") == 4 def test_a_bad_album_falls_back_to_asking_per_artist(tmp_path): @@ -526,7 +540,7 @@ def test_a_bad_album_falls_back_to_asking_per_artist(tmp_path): assert store.scalar("SELECT COUNT(*) FROM lidarr_album WHERE artist_id = 2") == 0 # The other two artists keep their albums. - assert store.scalar("SELECT COUNT(*) FROM lidarr_album") == 2 + assert store.scalar("SELECT COUNT(*) FROM lidarr_album") == 3 assert store.get_state("index_albums_skipped") == "1" @@ -549,10 +563,10 @@ def test_an_artist_lidarr_cannot_serve_does_not_kill_the_index(tmp_path): music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store) - assert store.scalar("SELECT COUNT(*) FROM lidarr_artist") == 3 + assert store.scalar("SELECT COUNT(*) FROM lidarr_artist") == 4 assert store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE artist_id = 2") == 0 # The other two artists are indexed in full. - assert store.scalar("SELECT COUNT(*) FROM lidarr_track") == 3 + assert store.scalar("SELECT COUNT(*) FROM lidarr_track") == 5 assert store.get_state("index_skipped") == "1" @@ -763,3 +777,51 @@ def test_the_report_survives_the_attribution_split(tmp_path): store = indexed(tmp_path, [scrobble_of("Yellowcard", "Hells Bells")]) music_curator.report(store, NOW) + + +def attribution_pairs(store): + """Unmatched pairs where the library's own title credits the scrobbled artist.""" + return [ + row["artist"] + for row in store.connection.execute( + "SELECT k.artist FROM scrobble_key k WHERE k.track_id IS NULL" + " AND EXISTS (SELECT 1 FROM lidarr_artist a WHERE a.norm_name = k.norm_artist)" + " AND EXISTS (SELECT 1 FROM lidarr_track t" + " WHERE t.norm_title = k.norm_track" + " AND instr(lower(t.title), lower(k.artist)) > 0)" + ) + ] + + +def test_a_shared_title_is_not_an_attribution_miss(tmp_path): + """Across fifty thousand tracks, titles collide constantly: "Everyday" is + Rusko and also Def Leppard. Matching those would be worse than missing.""" + store = indexed(tmp_path, [scrobble_of("Yellowcard", "Hells Bells")]) + + # The library holds "Hells Bells", by AC/DC, and its title says nothing + # about Yellowcard. A collision, not a miss. + assert verdict(store, "Yellowcard", "Hells Bells") == ("none", None) + assert attribution_pairs(store) == [] + + +def test_a_remix_credited_in_the_library_title_is_an_attribution_miss(tmp_path): + """The library has "Voodoo People (Pendulum Remix)" under The Prodigy; the + scrobble credits Pendulum. Same song, different filing.""" + store = indexed(tmp_path, [scrobble_of("Pendulum", "Voodoo People")]) + + assert attribution_pairs(store) == ["Pendulum"] + + +def test_the_title_index_is_used_for_the_report_lookup(tmp_path): + """Without it the report scans every track for every unmatched key: forty- + seven seconds on a real library.""" + store = indexed(tmp_path, []) + + plan = "\n".join( + row[-1] + for row in store.connection.execute( + "EXPLAIN QUERY PLAN SELECT 1 FROM scrobble_key k WHERE k.track_id IS NULL" + " AND EXISTS (SELECT 1 FROM lidarr_track t WHERE t.norm_title = k.norm_track)" + ) + ) + assert "lidarr_track_title" in plan, plan