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

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

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"