diff --git a/README.md b/README.md index 3ae5d07..ec8b2e3 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ which keeps an MP3 copy of a lossless library for an iPod. This one answers the question that mirror cannot: which of it is worth carrying, and which of it has not been played in years. -**This is stage three.** It ingests the scrobble history, indexes the library +**This is stage four, read-only.** It ingests the scrobble history, indexes the library from Lidarr, matches one to the other, and writes playlists into the mirror — by listening history and by mood. Nothing is written back to Lidarr — every call there is a `GET`. See "Where this @@ -20,7 +20,8 @@ is going" below. - Indexes every artist, album and track Lidarr knows about, with file paths and the date each file landed. - Ties the two together and reports how well it managed. -- Writes M3U playlists into the mirror, from the listening history. +- Writes M3U playlists into the mirror, from the listening history and by mood. +- Reports which albums have never been played. It does not act on that. ## Matching @@ -276,6 +277,40 @@ or `--mirror` looks like, since the paths then map to nothing at all. `_playlists/` survives music-mirror's prune: it only deletes `*.mp3`, and its empty-directory sweep skips a directory holding M3Us. +## The cold report + +Which albums have files, have never had a single track played in the whole +history, and have sat there long enough to have had the chance. Ranked by the +disk they occupy, because that is the point of the exercise. + +**Album-level, not track-level.** A record with two played tracks is a record +that gets played; picking the other ten off it leaves gaps rather than +reclaiming anything worth having. + +The age floor is measured from the newest file in the album, not from the +release date — what matters is how long it has been available to play, not how +old the record is. `--cold-after` sets it, defaulting to a year. + +Artists whose *every* album is cold are counted separately. That is a different +proposition from one cold record by somebody otherwise played, and Lidarr can +only tag at artist level anyway. + +### What stops it + +The report refuses to produce anything at all when: + +- the scrobble backfill is unfinished +- any artist failed to index, or any artist has no albums indexed +- the library has not been indexed, or there is no history to judge against + +Each of those makes played music look unplayed, which is the single failure that +costs a library. They are checked rather than trusted, because the report they +gate is the one that ends in deletion. + +**Nothing is written to Lidarr.** Every call there is still a `GET`. Unmonitoring +comes once the list has been looked at, because no flag protects against a list +that is wrong. + ## How the ingest works Two halves, both taking their bounds from the database rather than from a saved @@ -335,6 +370,7 @@ music-curator --report-only # report on the store, fetch nothing | `--playlist-limit` | `MUSIC_CURATOR_PLAYLIST_LIMIT` | `100` | Most tracks in any one playlist | | `--vibes` | `MUSIC_CURATOR_VIBES` | built-in | JSON file of mood definitions | | `--tag-limit` | `MUSIC_CURATOR_TAG_LIMIT` | `0` | Cap artist tag lookups per pass | +| `--cold-after` | `MUSIC_CURATOR_COLD_AFTER` | `365` | Days a file must sit unplayed to count as cold | | `--skip-index` | — | off | Match against the index already held | | `--report-only` | — | off | Report without fetching | @@ -385,7 +421,8 @@ nix shell nixpkgs#python3Packages.pytest -c pytest | Lidarr index and the scrobble-to-track matcher | done | | M3U playlists from the listening history | done | | Genre and mood playlists from Last.fm tags | done | -| Cold-music report, unmonitoring what is not played | last | +| Cold-music report | done | +| Unmonitoring what is not played | last | The cull will unmonitor cold albums in Lidarr and tag their artists. It will never delete files: Lidarr's `AlbumResource` has no tags at all, so tagging diff --git a/music_curator.py b/music_curator.py index 31e38c4..14552e9 100644 --- a/music_curator.py +++ b/music_curator.py @@ -85,12 +85,17 @@ SAFE_VIBE_NAME = re.compile(r"^[a-z0-9][a-z0-9-]*$") # back by whatever serves it, and a playlist nobody can read is not a playlist. GROUP_READ = 0o040 -SCHEMA_VERSION = "2" +SCHEMA_VERSION = "3" # Versions this build upgrades in place. Everything added since version 1 is a -# new table, and the schema script only ever creates what is missing, so running -# it is the whole migration -- an existing history is not re-downloaded. -MIGRATABLE_FROM = {"1"} +# new table or a new column, both of which are applied in place, so an existing +# history is never re-downloaded -- a full backfill is thousands of requests. +MIGRATABLE_FROM = {"1", "2"} + +# Columns added to existing tables after the fact. CREATE TABLE IF NOT EXISTS +# will not add a column to a table that already exists, so these are applied +# separately and only when missing. +ADDED_COLUMNS = (("lidarr_track", "size", "INTEGER"),) SCHEMA = """ -- One row per scrobble. The primary key collapses two plays of the same track @@ -160,7 +165,8 @@ CREATE TABLE IF NOT EXISTS lidarr_track ( has_file INTEGER NOT NULL DEFAULT 0, path TEXT, added INTEGER, - duration INTEGER + duration INTEGER, + size INTEGER ); 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); @@ -497,8 +503,18 @@ class Store: # thousand name pairs into Python to compare them one at a time. self.connection.create_function("normalise", 1, normalise, deterministic=True) self.connection.executescript(SCHEMA) + self._add_missing_columns() self._check_version() + def _add_missing_columns(self): + """Apply column additions to tables that predate them.""" + for table, column, kind in ADDED_COLUMNS: + held = {row[1] for row in self.connection.execute(f"PRAGMA table_info({table})")} + if column not in held: + logger.info("adding %s.%s to the store", table, column) + with self.connection: + self.connection.execute(f"ALTER TABLE {table} ADD COLUMN {column} {kind}") + def _check_version(self): held = self.get_state("schema_version") if held is None or held in MIGRATABLE_FROM: @@ -580,8 +596,8 @@ class Store: self.connection.executemany( "INSERT INTO lidarr_track" " (id, artist_id, album_id, recording_mbid, title, norm_artist, norm_title," - " has_file, path, added, duration)" - " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + " has_file, path, added, duration, size)" + " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", tracks, ) @@ -1040,6 +1056,7 @@ def index_library(client, store): handle.get("path"), parse_added(handle.get("dateAdded")), track.get("duration"), + handle.get("size"), ) ) @@ -1473,11 +1490,12 @@ def sync_tags(client, store, now, limit=0): def build_vibe_playlists(store, vibes, mirror_root, library_root, limit, now): """Write one playlist per mood. Returns how many tracks were listed.""" if not store.scalar("SELECT COUNT(*) FROM artist_tag"): - return 0 + return 0, [] directory = Path(mirror_root) / PLAYLIST_DIRECTORY week = now // ROTATION_PERIOD_SECONDS total = 0 + produced = [] for vibe in vibes: tags = [str(tag).strip().casefold() for tag in vibe["tags"]] @@ -1535,10 +1553,11 @@ def build_vibe_playlists(store, vibes, mirror_root, library_root, limit, now): entries.append({**dict(row), "mirror": mirror}) write_playlist(directory / f"{vibe['name']}.m3u", entries, Path(mirror_root)) + produced.append(f"{vibe['name']}.m3u") total += len(entries) logger.info("playlist %-20s %4d tracks -- by tag", vibe["name"], len(entries)) - return total + return total, produced def library_root_of(store): @@ -1641,6 +1660,22 @@ def write_playlist(path, entries, reference=None): temporary.unlink(missing_ok=True) +def prune_playlists(directory, produced, store): + """Delete playlists this build no longer produces. + + Driven by a record of what was written last time rather than by "every M3U + that is not one of ours", so a playlist put there by hand is left alone. A + renamed mood otherwise leaves its old file on the device for ever. + """ + previous = set(json.loads(store.get_state("playlist_files") or "[]")) + for name in sorted(previous - set(produced)): + stale = directory / name + if stale.is_file(): + logger.info("removing playlist %s, no longer produced", name) + stale.unlink(missing_ok=True) + store.set_state("playlist_files", json.dumps(sorted(produced))) + + def build_playlists(store, mirror_root, library_root, limit, now): """Write every playlist into the mirror. Returns how many tracks were listed. @@ -1658,6 +1693,7 @@ def build_playlists(store, mirror_root, library_root, limit, now): total = 0 missing = 0 + produced = [] for name, description, sql in PLAYLISTS: entries = [] for row in store.connection.execute(sql, parameters): @@ -1667,6 +1703,7 @@ def build_playlists(store, mirror_root, library_root, limit, now): continue entries.append({**dict(row), "mirror": mirror}) write_playlist(directory / f"{name}.m3u", entries, Path(mirror_root)) + produced.append(f"{name}.m3u") total += len(entries) logger.info("playlist %-20s %4d tracks -- %s", name, len(entries), description) @@ -1678,7 +1715,7 @@ def build_playlists(store, mirror_root, library_root, limit, now): " place, since the paths are then being mapped to nothing.", missing, ) - return total + return total, produced def coverage_report(store): @@ -1852,7 +1889,156 @@ def coverage_report(store): logger.info(" unmatched %2d: %-60s %d plays", position, label[:60], row["plays"]) -def report(store, now): +# An album has to have sat unplayed for at least this long before it counts as +# cold. Measured from the newest file in it, not from the release date: what +# matters is how long it has been available to play, not how old the record is. +COLD_AFTER_DAYS = 365 + + +def human_bytes(count): + """Return a size that can be read at a glance.""" + size = float(count or 0) + for unit in ("B", "KiB", "MiB", "GiB", "TiB"): + if size < 1024 or unit == "TiB": + return f"{size:.1f} {unit}" + size /= 1024 + + +def cull_is_safe(store): + """Return why a cull must not run, or None if it may. + + Every one of these makes played music look unplayed, which is the single + failure that costs a library. They are checked rather than trusted because + the report they gate is the one that ends in deletion. + """ + if store.get_state("backfill_complete") != "yes": + return "the scrobble history is still being backfilled" + if int(store.get_state("index_skipped") or 0): + return "some artists could not be indexed, so their tracks are missing" + if int(store.get_state("index_albums_skipped") or 0): + return "some artists have no albums indexed" + if not store.scalar("SELECT COUNT(*) FROM lidarr_track"): + return "the library has not been indexed" + if not store.scalar("SELECT COUNT(*) FROM scrobble"): + return "there is no listening history to judge against" + return None + + +COLD_ALBUMS = """ +WITH played AS ( + SELECT DISTINCT sk.track_id AS track_id + FROM scrobble_key sk + WHERE sk.track_id IS NOT NULL +), +album AS ( + SELECT t.album_id AS album_id, + COUNT(*) AS tracks, + SUM(COALESCE(t.size, 0)) AS bytes, + MAX(COALESCE(t.added, 0)) AS newest, + SUM(CASE WHEN p.track_id IS NULL THEN 0 ELSE 1 END) AS plays + FROM lidarr_track t + LEFT JOIN played p ON p.track_id = t.id + WHERE t.has_file = 1 + GROUP BY t.album_id +) +SELECT al.id AS album_id, + al.title AS title, + ar.id AS artist_id, + ar.name AS artist, + album.tracks AS tracks, + album.bytes AS bytes, + album.newest AS newest + FROM album + JOIN lidarr_album al ON al.id = album.album_id + JOIN lidarr_artist ar ON ar.id = al.artist_id + WHERE album.plays = 0 + AND album.newest > 0 + AND album.newest < :cutoff +""" + + +def cold_albums(store, cutoff): + """Return every album with files, none of them ever played, old enough to judge.""" + return store.connection.execute(COLD_ALBUMS, {"cutoff": cutoff}).fetchall() + + +def cold_report(store, now, cold_after_days=COLD_AFTER_DAYS): + """Report what has never been played. Writes nothing, anywhere. + + Album-level rather than track-level: a record with two played tracks is a + record that gets played, and picking the other ten off it leaves gaps rather + than reclaiming anything worth having. + """ + refusal = cull_is_safe(store) + if refusal is not None: + logger.warning("no cold report: %s", refusal) + return [] + + cutoff = now - cold_after_days * 86400 + rows = cold_albums(store, cutoff) + if not rows: + logger.info("--- cold albums --- none: everything with a file has been played") + return [] + + total_bytes = sum(row["bytes"] or 0 for row in rows) + total_tracks = sum(row["tracks"] or 0 for row in rows) + albums_with_files = store.scalar( + "SELECT COUNT(DISTINCT album_id) FROM lidarr_track WHERE has_file = 1" + ) + + logger.info("--- cold albums (never played, files older than %d days) ---", cold_after_days) + logger.info( + "%d of %d albums, %d tracks, %s", + len(rows), + albums_with_files, + total_tracks, + human_bytes(total_bytes), + ) + + # An artist every one of whose albums is cold is a different proposition + # from one cold record by someone otherwise played, and Lidarr can only tag + # at artist level anyway. + cold_by_artist = {} + for row in rows: + held = cold_by_artist.setdefault(row["artist_id"], {"name": row["artist"], "albums": 0, + "bytes": 0}) + held["albums"] += 1 + held["bytes"] += row["bytes"] or 0 + + owned = dict( + store.connection.execute( + "SELECT al.artist_id, COUNT(DISTINCT al.id) FROM lidarr_album al" + " JOIN lidarr_track t ON t.album_id = al.id AND t.has_file = 1" + " GROUP BY al.artist_id" + ).fetchall() + ) + entirely = [ + held for artist_id, held in cold_by_artist.items() + if held["albums"] == owned.get(artist_id) + ] + logger.info( + "%d artists are cold in their entirety (%s), %d have some cold records", + len(entirely), + human_bytes(sum(held["bytes"] for held in entirely)), + len(cold_by_artist) - len(entirely), + ) + + for position, held in enumerate( + sorted(cold_by_artist.values(), key=lambda held: -held["bytes"])[:20], start=1 + ): + logger.info( + " cold %2d: %-40s %2d albums, %s", + position, + held["name"][:40], + held["albums"], + human_bytes(held["bytes"]), + ) + + logger.info("nothing was changed in Lidarr: this report does not write") + return rows + + +def report(store, now, cold_after=COLD_AFTER_DAYS): """Log what the store holds. The MBID coverage line is the one to watch: scrobbles carrying a MusicBrainz @@ -1886,6 +2072,7 @@ def report(store, now): logger.info(" top artist %2d: %-40s %d", position, row["artist"][:40], row["plays"]) coverage_report(store) + cold_report(store, now, cold_after) recent = store.connection.execute( "SELECT artist, track, COUNT(*) AS plays FROM scrobble WHERE uts >= ?" @@ -1921,9 +2108,14 @@ def run_once( " written; set --library-root" ) else: - build_playlists(store, mirror, root, playlist_limit, now) + _, from_history = build_playlists(store, mirror, root, playlist_limit, now) sync_tags(client, store, now, limit=tag_limit) - build_vibe_playlists(store, vibes, mirror, root, playlist_limit, now) + _, from_tags = build_vibe_playlists( + store, vibes, mirror, root, playlist_limit, now + ) + prune_playlists( + Path(mirror) / PLAYLIST_DIRECTORY, from_history + from_tags, store + ) logger.info( "pass complete in %.1fs: %d scrobbles added, %d loved", @@ -2027,6 +2219,13 @@ def build_parser(): default=int(os.getenv("MUSIC_CURATOR_TAG_LIMIT", "0")), help="cap artist tag lookups per pass; 0 for no cap (env MUSIC_CURATOR_TAG_LIMIT)", ) + parser.add_argument( + "--cold-after", + type=int, + default=int(os.getenv("MUSIC_CURATOR_COLD_AFTER", str(COLD_AFTER_DAYS))), + help="days a file must sit unplayed before its album counts as cold" + " (env MUSIC_CURATOR_COLD_AFTER)", + ) parser.add_argument( "--skip-index", action="store_true", @@ -2065,7 +2264,7 @@ def main(argv=None, clock=time.time): store = Store(database) if args.report_only: - report(store, int(clock())) + report(store, int(clock()), args.cold_after) store.close() lock.close() return 0 @@ -2109,7 +2308,7 @@ def main(argv=None, clock=time.time): logger.error("%s", error) if interval is None: return 1 - report(store, now) + report(store, now, args.cold_after) if interval is None or stopping: return 0 diff --git a/tests/test_music_curator.py b/tests/test_music_curator.py index df74267..969784c 100644 --- a/tests/test_music_curator.py +++ b/tests/test_music_curator.py @@ -2,6 +2,7 @@ import json import os import stat import urllib.error +from datetime import datetime, timezone from pathlib import Path import pytest @@ -924,9 +925,10 @@ def test_a_track_with_no_mirror_file_is_left_out(tmp_path): ) music_curator.match_library(store) - total = music_curator.build_playlists(store, mirror, str(source), 100, NOW) + total, produced = music_curator.build_playlists(store, mirror, str(source), 100, NOW) assert total == 0 + assert produced, "the playlists are still written, they are simply empty" assert (mirror / "_playlists" / "all-time.m3u").read_text() == "#EXTM3U\n" @@ -1342,3 +1344,117 @@ def test_a_playlist_is_chowned_to_match_the_mirror(tmp_path, monkeypatch): assert all(tuple(owner) == (568, 568) for _, *owner in attempted) # The temporary file, before the rename, never the finished playlist. assert all(path.endswith(".part") or path.endswith("_playlists") for path, *_ in attempted) + + +def test_a_renamed_mood_does_not_leave_its_old_playlist_behind(tmp_path): + """Otherwise the dead file stays on the device for ever.""" + api, source, mirror = playlist_library(tmp_path) + store = store_at(tmp_path) + directory = mirror / "_playlists" + directory.mkdir(parents=True, exist_ok=True) + (directory / "high-energy-rock.m3u").write_text("#EXTM3U\n") + (directory / "screamo.m3u").write_text("#EXTM3U\n") + store.set_state( + "playlist_files", json.dumps(["high-energy-rock.m3u", "screamo.m3u"]) + ) + + music_curator.prune_playlists(directory, ["screamo.m3u"], store) + + assert not (directory / "high-energy-rock.m3u").exists() + assert (directory / "screamo.m3u").exists() + + +def test_pruning_leaves_a_playlist_it_never_wrote(tmp_path): + """A hand-made playlist in that directory is not ours to delete.""" + api, source, mirror = playlist_library(tmp_path) + store = store_at(tmp_path) + directory = mirror / "_playlists" + directory.mkdir(parents=True, exist_ok=True) + (directory / "lyras-own-mix.m3u").write_text("#EXTM3U\n") + store.set_state("playlist_files", json.dumps(["screamo.m3u"])) + + music_curator.prune_playlists(directory, [], store) + + assert (directory / "lyras-own-mix.m3u").exists() + + +def cold_store(tmp_path, played=(), added="2020-05-01T12:00:00Z"): + """A library where nothing is played unless named, indexed and matched.""" + library = [ + { + "name": "Played Band", + "albums": [{"title": "Known", "tracks": [{"title": "Hit", "added": added}]}], + }, + { + "name": "Silent Band", + "albums": [ + {"title": "Unknown", "tracks": [{"title": "Never Heard", "added": added}]} + ], + }, + ] + store = store_at(tmp_path) + ingest(store, FakeLastfm([scrobble_of(a, t) for a, t in played])) + music_curator.index_library( + music_curator.Lidarr("http://lidarr", "key", transport=FakeLidarr(library)), store + ) + music_curator.match_library(store) + return store + + +def test_an_album_with_no_plays_is_cold(tmp_path): + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + + cold = music_curator.cold_report(store, NOW) + + assert [row["artist"] for row in cold] == ["Silent Band"] + + +def test_an_album_with_any_play_is_not_cold(tmp_path): + """A record with one played track is a record that gets played; picking the + rest off it leaves gaps rather than reclaiming anything.""" + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + + cold = music_curator.cold_report(store, NOW) + + assert "Played Band" not in [row["artist"] for row in cold] + + +def test_a_recent_arrival_is_too_young_to_judge(tmp_path): + """It has not had a chance to be played yet.""" + recent = datetime.fromtimestamp(NOW - 30 * 86400, tz=timezone.utc).isoformat() + store = cold_store(tmp_path, played=[("Played Band", "Hit")], added=recent) + + assert music_curator.cold_report(store, NOW) == [] + + +def test_the_cull_refuses_to_run_on_an_incomplete_index(tmp_path): + """A missing artist makes their played music look unplayed, which is + precisely how this would delete something you like.""" + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + store.set_state("index_skipped", "1") + + assert music_curator.cull_is_safe(store) is not None + assert music_curator.cold_report(store, NOW) == [] + + +def test_the_cull_refuses_while_the_backfill_is_unfinished(tmp_path): + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + store.set_state("backfill_complete", "no") + + assert music_curator.cold_report(store, NOW) == [] + + +def test_the_cold_report_writes_nothing_to_lidarr(tmp_path): + """Stage four is read-only until the list it produces has been looked at.""" + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + before = store.scalar("SELECT COUNT(*) FROM lidarr_album WHERE monitored = 1") + + music_curator.cold_report(store, NOW) + + assert store.scalar("SELECT COUNT(*) FROM lidarr_album WHERE monitored = 1") == before + + +def test_human_bytes_reads_at_a_glance(): + assert music_curator.human_bytes(0) == "0.0 B" + assert music_curator.human_bytes(1536) == "1.5 KiB" + assert music_curator.human_bytes(3 * 1024**3) == "3.0 GiB"