fix: distinguish a title collision from an attribution miss, and index for it
Build and publish container / build (pull_request) Successful in 5m16s
Build and publish container / build (pull_request) Successful in 5m16s
Two faults in the split added last change, both visible in the first real run. It reported 700 pairs as attribution disagreements on the strength of the library holding the same title under a different artist. The examples show what that actually caught: "Everyday" matched Def Leppard, "Kaleidoscope" matched Chappell Roan, "Fight for Your Right" matched Motley Crue. Different songs that happen to share a name. Across fifty thousand tracks that is not an edge case, it is the common case, and presenting it as a matcher failure argues for exactly the title-only matching tier that would produce this rubbish on purpose. The signal for a real attribution miss is narrower: the library's own title credits the artist the play is filed under, as in "Voodoo People (Pendulum Remix)" against a scrobble credited to Pendulum. The normalised title has that suffix stripped -- which is what let the two meet in the first place -- so the raw title is searched for the name. Collisions are now counted and named separately, as what they are. The same query also took forty-seven seconds. lidarr_track was indexed on (norm_artist, norm_title), which a lookup by title alone cannot use because its leading column is the artist, so every unmatched key scanned all eighty-four thousand tracks. Add the index on the title by itself; the query plan changes from an automatic partial index to a covering one.
This commit is contained in:
@@ -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
|
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
|
- **the library's own title credits the scrobbled artist** — `Voodoo People
|
||||||
remixer or a guest billed as the artist. These are the genuine misses, and
|
(Pendulum Remix)` against a play credited to Pendulum. Same song, filed
|
||||||
each is a candidate for being wrongly called cold in stage four. The report
|
under the original artist. These are the genuine misses.
|
||||||
names the artist the library files them under.
|
- **the same title under an unrelated artist** — a collision, not a miss.
|
||||||
- **the title is nowhere in the library** — never bought. No amount of matching
|
Across fifty thousand tracks these are constant: `Everyday` is Rusko and
|
||||||
conjures a file that does not exist.
|
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
|
Only the first is worth chasing. Counting all three as matcher failures
|
||||||
the cull. The report lists the worst of each by play count.
|
overstates the problem and would over-block the cull.
|
||||||
|
|
||||||
## How the ingest works
|
## How the ingest works
|
||||||
|
|
||||||
|
|||||||
+41
-15
@@ -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_recording ON lidarr_track (recording_mbid);
|
||||||
CREATE INDEX IF NOT EXISTS lidarr_track_norm ON lidarr_track (norm_artist, norm_title);
|
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);
|
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
|
-- 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
|
# 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
|
# overstates the matcher's failings. Split it -- but not on the title alone.
|
||||||
# under some other artist is an attribution disagreement -- a remixer
|
# Across fifty thousand tracks, titles collide constantly: "Everyday" is
|
||||||
# credited as the artist, a guest billed as one -- and is a real miss. A
|
# Rusko and also Def Leppard, "Kaleidoscope" is Delta Heavy and also
|
||||||
# title the library does not hold at all was simply never bought, and no
|
# Chappell Roan. Matching those would be worse than missing them.
|
||||||
# amount of matching will conjure it.
|
#
|
||||||
|
# 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(
|
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"
|
"SELECT COUNT(*) AS pairs, COALESCE(SUM(plays), 0) AS plays FROM scrobble_key k"
|
||||||
" WHERE k.track_id IS NULL"
|
" 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_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)"
|
||||||
).fetchone()
|
).fetchone()
|
||||||
logger.info(
|
logger.info(
|
||||||
" of those, the title exists under another artist: %d pairs, %d plays"
|
" the library's title credits the scrobbled artist: %d pairs, %d plays"
|
||||||
" -- attribution disagreements, and the genuine misses",
|
" -- remixes and guest spots, and the genuine misses",
|
||||||
attribution["pairs"],
|
attribution["pairs"],
|
||||||
attribution["plays"],
|
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(
|
logger.info(
|
||||||
" the rest, %d pairs, %d plays: you own the artist but not the track",
|
" the rest, %d pairs, %d plays: you own the artist but not the track",
|
||||||
suspect["pairs"] - attribution["pairs"],
|
suspect["pairs"] - collision["pairs"],
|
||||||
suspect["plays"] - attribution["plays"],
|
suspect["plays"] - collision["plays"],
|
||||||
)
|
)
|
||||||
|
|
||||||
mismatched = store.connection.execute(
|
mismatched = store.connection.execute(
|
||||||
"SELECT k.artist, k.track, k.plays,"
|
"SELECT k.artist, k.track, k.plays,"
|
||||||
" (SELECT a.name FROM lidarr_track t"
|
" (SELECT t.title FROM lidarr_track t"
|
||||||
" JOIN lidarr_artist a ON a.id = t.artist_id"
|
" WHERE t.norm_title = k.norm_track"
|
||||||
" WHERE t.norm_title = k.norm_track LIMIT 1) AS filed_under"
|
" AND instr(lower(t.title), lower(k.artist)) > 0 LIMIT 1) AS library_title"
|
||||||
" FROM scrobble_key k"
|
" FROM scrobble_key k"
|
||||||
" WHERE k.track_id IS NULL"
|
" 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_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"
|
" ORDER BY k.plays DESC, k.artist LIMIT 10"
|
||||||
).fetchall()
|
).fetchall()
|
||||||
for position, row in enumerate(mismatched, start=1):
|
for position, row in enumerate(mismatched, start=1):
|
||||||
logger.info(
|
logger.info(
|
||||||
" attribution %2d: %-45s %4d plays, filed under %s",
|
" attribution %2d: %-45s %4d plays, library has %r",
|
||||||
position,
|
position,
|
||||||
f"{row['artist']} - {row['track']}"[:45],
|
f"{row['artist']} - {row['track']}"[:45],
|
||||||
row["plays"],
|
row["plays"],
|
||||||
row["filed_under"],
|
row["library_title"],
|
||||||
)
|
)
|
||||||
|
|
||||||
with_files = store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE has_file = 1")
|
with_files = store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE has_file = 1")
|
||||||
|
|||||||
@@ -55,8 +55,22 @@ LIBRARY = [
|
|||||||
{
|
{
|
||||||
"name": "The Prodigy",
|
"name": "The Prodigy",
|
||||||
"albums": [
|
"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):
|
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."""
|
"""The index is Lidarr's mirror, not an accumulation of everything ever seen."""
|
||||||
store = indexed(tmp_path, [scrobble_of("AC/DC", "Hells Bells")])
|
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.index_library(
|
||||||
music_curator.Lidarr("http://lidarr", "key", transport=FakeLidarr(LIBRARY[:1])), store
|
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"]
|
album_calls = [query for path, query in api.calls if path == "album"]
|
||||||
assert album_calls == [{}]
|
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):
|
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
|
assert store.scalar("SELECT COUNT(*) FROM lidarr_album WHERE artist_id = 2") == 0
|
||||||
# The other two artists keep their albums.
|
# 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"
|
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)
|
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
|
assert store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE artist_id = 2") == 0
|
||||||
# The other two artists are indexed in full.
|
# 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"
|
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")])
|
store = indexed(tmp_path, [scrobble_of("Yellowcard", "Hells Bells")])
|
||||||
|
|
||||||
music_curator.report(store, NOW)
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user