Merge pull request 'feat: mood playlists from Last.fm crowd tags' (#10) from feat/tag-vibes into main

Reviewed-on: #10

.gitea/workflows/build-and-publish.yaml

@@ -45,7 +45,8 @@ jobs:
 
       # The suite runs inside the image, against the interpreter that ships,
       # rather than against whatever the runner happens to provide. A failing
-      # test fails the build. Layers are shared with the push build below.
+      # test fails the build. The runtime stage below is built from the same
+      # daemon afterwards, so its layers are already in cache.
       - name: Run the test suite inside the image
         run: docker build --target test -t music-curator:test .
 
@@ -124,9 +125,6 @@ jobs:
           echo "release=${release}" >> "$GITHUB_OUTPUT"
           echo "Computed bump=${bump}, release=${release}, base=${base}"
 
-      - name: Set up Buildx
-        uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4
-
       - name: Log in to the Gitea container registry
         if: github.event_name != 'pull_request'
         uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4
@@ -135,21 +133,33 @@ jobs:
           username: ${{ github.repository_owner }}
           password: ${{ secrets.PACKAGES_TOKEN }}
 
-      - name: Build and push
+      # Plain `docker build` rather than buildx. buildx boots its own buildkit
-        uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf # v7
+      # in a container with a cache of its own, so it shared nothing with the
-        with:
+      # test build above and rebuilt the image from the base image up -- two
-          context: .
+      # full builds per run. It earns that cost when building for several
-          # Without this the last stage in the Dockerfile -- the test stage --
+      # platforms; this only ever targets the amd64 NAS, so it does not.
-          # would be what gets published.
+      #
-          target: runtime
+      # `--target runtime` is a strict prefix of the test stage, so every layer
-          # The NAS is the only host this runs on. Building arm64 as well would
+      # is already in the daemon's cache and this resolves in seconds.
-          # mean emulating it under QEMU for no consumer.
+      - name: Build the runtime image
-          platforms: linux/amd64
+        run: |
-          push: ${{ github.event_name != 'pull_request' }}
+          set -euo pipefail
-          tags: ${{ steps.version.outputs.tags }}
+          tags=()
-          labels: |
+          while IFS= read -r tag; do
-            org.opencontainers.image.source=${{ github.server_url }}/${{ github.repository }}
+            [ -n "$tag" ] && tags+=(-t "$tag")
-            org.opencontainers.image.revision=${{ github.sha }}
+          done <<< "${{ steps.version.outputs.tags }}"
+          docker build --target runtime \
+            --label "org.opencontainers.image.source=${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}" \
+            --label "org.opencontainers.image.revision=${GITHUB_SHA}" \
+            "${tags[@]}" .
+
+      - name: Push
+        if: github.event_name != 'pull_request'
+        run: |
+          set -euo pipefail
+          while IFS= read -r tag; do
+            [ -n "$tag" ] && docker push "$tag"
+          done <<< "${{ steps.version.outputs.tags }}"
 
       # Record the release: write the computed version into pyproject.toml, then
       # commit and tag it, so the packaging metadata always matches the release

README.md

@@ -7,22 +7,213 @@ 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 one.** It ingests the scrobble history and nothing else. There
+**This is stage three.** It ingests the scrobble history, indexes the library
-are no playlists yet, and nothing touches Lidarr or the music library. See
+from Lidarr, matches one to the other, and writes playlists into the mirror —
-"Where this is going" below.
+by listening history and by mood.
+Nothing is written back to Lidarr — every call there is a `GET`. See "Where this
+is going" below.
 
 ## What it does today
 
 - Pulls the full Last.fm scrobble history into SQLite, then keeps it current.
 - Tracks loved tracks separately, as the protected set for later stages.
-- Reports what it holds, including the number that matters most: how many
+- Indexes every artist, album and track Lidarr knows about, with file paths and
-  scrobbles carry a MusicBrainz recording id.
+  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.
 
-That last figure decides the next stage. Lidarr exposes a `ForeignRecordingId`
+## Matching
-on every track, which is the same identifier, so scrobbles carrying one can be
+
-joined to the library exactly. The rest have to go through name matching, which
+Two tiers, and no third.
-is where a curation tool goes wrong and starts recommending the deletion of
+
-music you love. Measure the join rate before trusting the verdict.
+| Tier | Key | Notes |
+| ------ | --------------------------- | ----------------------------------------- |
+| `mbid` | MusicBrainz recording id | Exact. Last.fm's per-scrobble `mbid` against Lidarr's `ForeignRecordingId` |
+| `name` | Normalised artist and title | Everything the first tier could not carry |
+| `none` | — | Recorded as a miss, never guessed at |
+
+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
+leading `The`. Punctuation gets two different rules that pull against each
+other and are both required: apostrophes are **deleted**, so `Don't` meets
+`Dont`, while every other mark becomes a **space**, so `AC/DC`, `AC-DC` and
+`AC DC` all meet as well.
+
+It leans towards collapsing too much. A false match makes something look
+played; a missed match makes something look abandoned. Only one of those
+deletes music.
+
+### Indexing quirks
+
+Albums are fetched from the **unfiltered** `GET /api/v1/album` first: one
+request, and the only path that skips albums whose artist metadata is missing
+rather than dereferencing it.
+
+That is not enough on its own. Every album endpoint maps through a resource
+that picks the release with `SingleOrDefault(x => x.Monitored)`, which throws
+for an album with **two monitored releases** and takes the whole response with
+it:
+
+```
+HTTP 500: Sequence contains more than one element
+```
+
+When the bulk call dies that way, the indexer falls back to one request per
+artist. It cannot avoid the exception, but it confines it to whichever artist
+owns the offending album and names them in the log — which is the only
+practical way to find it in a large library. Open that artist in Lidarr and
+check the Releases tab of each album: exactly one release may be monitored.
+
+Losing an artist's albums does not cost their tracks, which come from a
+different endpoint with a different mapper, so matching is unaffected. A cull
+would not be, and the report says so.
+
+### Talking to Lidarr
+
+Indexing is two requests per artist, and more when the album fallback fires. On
+a large library that is thousands of requests in a few minutes. `urllib` opens a
+new TCP connection and performs a new DNS lookup for every one of them, which is
+enough to exhaust a container's resolver and produce `[Errno -3] Try again` on
+everything at once. The client therefore holds one connection open per host and
+resolves once.
+
+Transient failures — a dropped connection, a resolver hiccup, `429`, `502`,
+`503`, `504` — are retried with a backoff. An HTTP `500` is not: it is an
+unhandled exception inside Lidarr's own serialisation and will be raised again
+identically. That distinction also decides whether a failure is worth
+investigating; a library-wide outage is not probed artist by artist, because
+doing so multiplies the load that caused it.
+
+A local address is preferable to a public hostname here. It removes DNS, the
+reverse proxy and its timeouts from a path that needs none of them.
+
+Tracks and files have no unfiltered endpoint — Lidarr rejects a call with no
+filter — so they stay per artist. If one artist cannot be served, that artist is
+skipped and the run continues, but the count is recorded and the coverage report
+says so loudly. A missing artist makes their played music look cold, so an
+incomplete index must never be culled against.
+
+### Reading the coverage report
+
+Matched against unmatched is the wrong comparison — most unmatched listening is
+music that was never in the library, which says nothing at all about the
+matcher. The line to watch is:
+
+```
+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 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 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.
+
+Only the first is worth chasing. Counting all three as matcher failures
+overstates the problem and would over-block the cull.
+
+## Playlists
+
+Written into `<mirror>/_playlists/` as extended M3U, rebuilt every pass. Six
+rules, capped at `--playlist-limit` tracks each:
+
+| Playlist | Rule |
+| -------------------- | --------------------------------------------------------- |
+| `heavy-rotation` | Most played over the last twelve months |
+| `all-time` | Most played ever |
+| `neglected` | Played heavily once, silent for twelve months |
+| `deep-cuts` | Never played, from albums whose other tracks you play constantly |
+| `unheard-favourites` | Never played, by the artists you play most |
+| `unheard` | Never played, anywhere in the library |
+
+Ninety days was the obvious window for "recent" and is the wrong one: on a real
+history it holds a few hundred plays spread thinly across a twenty-thousand
+track rotation, so nothing ranks meaningfully. Twelve months does.
+
+The two `unheard` playlists rotate **weekly**, not per pass. A pass runs every
+few hours, and a playlist that reorders itself each time is one that has to be
+re-imported each time — the Music app imports a snapshot of a file, it does not
+track it.
+
+### Moods
+
+A second set of playlists selects by **Last.fm's crowd tags** rather than by
+listening history. MusicBrainz genres arrive free with the Lidarr index and are
+no use for this: they are sparse and formal, and will not tell you a record is
+screamo or synthwave. People typing tags will.
+
+Tags are fetched once per artist — one `artist.getTopTags` call each, by
+MusicBrainz id where Lidarr has one — and refreshed every ninety days. An
+artist Last.fm has never heard of is recorded as fetched with no tags, so it is
+not asked about again on every pass. `--tag-limit` spreads the first sweep over
+several passes.
+
+The built-in moods are chosen for this library rather than as a general
+taxonomy:
+
+| Mood | Selected on |
+| ------------------ | ------------------------------------------------- |
+| `80s-synths` | synthpop, new wave, synthwave — released 1975-1992 |
+| `high-energy-rock` | hard rock, punk, pop punk, alternative |
+| `screamo` | screamo, post-hardcore, metalcore, emo |
+| `drum-and-bass` | drum and bass, liquid funk, neurofunk, jungle |
+| `dance` | house, big room, hardstyle, trance, dubstep |
+| `classic-rock` | classic rock, prog, psychedelic, blues rock |
+
+An artist qualifies when their tag weights inside a mood sum to at least 30 out
+of Last.fm's 0-100 scale. One low-weight tag is not a genre, it is somebody's
+stray opinion.
+
+`years` filters on the album's release date, which is what separates eighties
+synth records from everything else a synthpop tag drags in.
+
+`--vibes` replaces the whole set with a JSON file of the same shape, so a new
+mood does not need a new release:
+
+```json
+[{ "name": "shoegaze", "tags": ["shoegaze", "dream pop"], "min_score": 40 }]
+```
+
+Names are validated when the file is read, not when the file is written. A bad
+one would otherwise surface as a playlist created somewhere unintended.
+
+### Paths
+
+Lidarr knows where the lossless source is; the playlists have to point at the
+MP3s music-mirror made from it. The mapping strips a library root from Lidarr's
+track paths and re-roots them under the mirror, with the suffix changed.
+
+`--library-root` is derived from the common parent of the indexed artist folders
+when unset, so it agrees with Lidarr by construction rather than by being kept
+in step by hand. Override it if that guess is wrong.
+
+Entries are written **relative to the playlist file**, so one playlist works
+from the NAS, from a Mac over SMB, and from Linux, without rewriting.
+
+A track is only listed once its mirror file has been confirmed to exist. Lidarr
+holding the FLAC says nothing about whether the MP3 has been encoded yet. If a
+large number are missing, the run says so — that is what a wrong `--library-root`
+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.
 
 ## How the ingest works
 
@@ -76,6 +267,14 @@ music-curator --report-only   # report on the store, fetch nothing
 | `--interval` | `MUSIC_CURATOR_INTERVAL` | unset | Repeat forever, e.g. `45m`, `6h`, `1d` |
 | `--request-delay` | `MUSIC_CURATOR_REQUEST_DELAY` | `0.25` | Seconds between API requests |
 | `--backfill-limit` | `MUSIC_CURATOR_BACKFILL_LIMIT` | `0` | Cap backfill requests per pass; 0 for no cap |
+| `--lidarr-url` | `MUSIC_CURATOR_LIDARR_URL` | unset | Lidarr base URL, e.g. `http://lidarr:8686` |
+| `--lidarr-api-key` | `MUSIC_CURATOR_LIDARR_API_KEY` | unset | Lidarr API key |
+| `--mirror` | `MUSIC_CURATOR_MIRROR` | unset | Root of the MP3 mirror; playlists go here |
+| `--library-root` | `MUSIC_CURATOR_LIBRARY_ROOT` | derived | Prefix to strip from Lidarr's paths |
+| `--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 |
+| `--skip-index` | — | off | Match against the index already held |
 | `--report-only` | — | off | Report without fetching |
 
 A [Last.fm API key](https://www.last.fm/api/account/create) is all that is
@@ -107,9 +306,11 @@ docker build --target test .   # what CI runs
 pytest                         # needs pytest on PATH
 ```
 
-The suite runs against a fake transport that reproduces the real service's
+The suite runs against fake transports for both services. The Last.fm one
-paging, its `from`/`to` semantics and its awkward response shapes. No network,
+reproduces its paging, its `from`/`to` semantics and its awkward response
-no credentials, no rate limit. On a Nix machine:
+shapes; the Lidarr one serves a canned library split across the same four
+endpoints the indexer calls, so the stitching is exercised rather than
+stubbed. No network, no credentials, no rate limit. On a Nix machine:
 
 ```sh
 nix shell nixpkgs#python3Packages.pytest -c pytest
@@ -120,8 +321,9 @@ nix shell nixpkgs#python3Packages.pytest -c pytest
 | Stage | Status |
 | ------------------------------------------------ | ------------ |
 | Last.fm ingest and store | done |
-| Lidarr index and the scrobble-to-track matcher | next |
+| Lidarr index and the scrobble-to-track matcher | done |
-| M3U playlists written into the mirror | after that |
+| 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 |
 
 The cull will unmonitor cold albums in Lidarr and tag their artists. It will

compose.yaml

@@ -17,6 +17,10 @@ services:
       # enough, none of the endpoints used here authenticate a user.
       MUSIC_CURATOR_LASTFM_API_KEY: set-me-in-the-truenas-ui
       MUSIC_CURATOR_DB: /data/curator.db
+      # Lidarr, for indexing the library. Read-only: every call is a GET.
+      # Leave unset to ingest scrobbles and nothing else.
+      MUSIC_CURATOR_LIDARR_URL: http://lidarr:8686
+      MUSIC_CURATOR_LIDARR_API_KEY: set-me-in-the-truenas-ui
       # How long to wait between passes. Each one catches up on new scrobbles
       # and continues the backfill if it has not finished.
       MUSIC_CURATOR_INTERVAL: 6h
@@ -26,5 +30,11 @@ services:
       # Cap the backfill at this many requests per pass. Unlimited by default,
       # which finishes a long history in one go.
       # MUSIC_CURATOR_BACKFILL_LIMIT: "0"
+      # The MP3 mirror music-mirror maintains. Playlists are written into
+      # _playlists/ inside it; leave unset to skip them.
+      MUSIC_CURATOR_MIRROR: /mirror
+      # Most tracks in any one playlist.
+      # MUSIC_CURATOR_PLAYLIST_LIMIT: "100"
     volumes:
       - /mnt/tank/apps/music-curator:/data
+      - /mnt/tank/media/music-mp3:/mirror

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "music-curator"
-version = "0.1.0"
+version = "0.4.0"
 description = "Ingest a Last.fm listening history and curate a music library from it"
 readme = "README.md"
 requires-python = ">=3.11"

tests/conftest.py

@@ -1,6 +1,10 @@
+import http.server
+import io
 import json
 import os
 import sys
+import threading
+import urllib.error
 import urllib.parse
 
 import pytest
@@ -52,7 +56,9 @@ class FakeLastfm:
     is prepended to the first page with no `date`.
     """
 
-    def __init__(self, tracks=(), loved=(), nowplaying=None, outcomes=()):
+    def __init__(self, tracks=(), loved=(), nowplaying=None, outcomes=(), tags=None):
+        # Keyed by mbid or by artist name, whichever the caller asked with.
+        self.tags = tags or {}
         self.tracks = sorted(tracks, key=lambda track: int(track["date"]["uts"]), reverse=True)
         self.loved = list(loved)
         self.nowplaying = nowplaying
@@ -78,6 +84,9 @@ class FakeLastfm:
             return json.dumps(self._recent(query))
         if method == "user.getlovedtracks":
             return json.dumps(self._loved(query))
+        if method == "artist.gettoptags":
+            key = query.get("mbid") or query.get("artist", "")
+            return json.dumps({"toptags": {"tag": self.tags.get(key, [])}})
         raise AssertionError(f"unexpected method {method}")
 
     def _recent(self, query):
@@ -120,6 +129,115 @@ class FakeLastfm:
         }
 
 
+class FakeLidarr:
+    """A transport serving a canned library over the Lidarr v1 API surface.
+
+    Built from a nested description -- artist, album, tracks -- and split back
+    out across the four endpoints the indexer actually calls, so the tests
+    exercise the same stitching the real thing does.
+    """
+
+    def __init__(self, artists=(), fail=()):
+        # (path, artistId) pairs the fake refuses to serve, standing in for the
+        # 500s Lidarr returns on data it cannot hydrate.
+        self.fail = set(fail)
+        self.artists, self.albums, self.tracks, self.files = [], [], [], []
+        for artist_index, entry in enumerate(artists, start=1):
+            artist_id = artist_index
+            self.artists.append(
+                {
+                    "id": artist_id,
+                    "artistName": entry["name"],
+                    "foreignArtistId": entry.get("mbid", f"artist-mbid-{artist_id}"),
+                    "path": f"/music/{entry['name']}",
+                    "monitored": entry.get("monitored", True),
+                }
+            )
+            for album_index, album in enumerate(entry.get("albums", []), start=1):
+                album_id = artist_id * 100 + album_index
+                self.albums.append(
+                    {
+                        "id": album_id,
+                        "artistId": artist_id,
+                        "title": album["title"],
+                        "foreignAlbumId": f"album-mbid-{album_id}",
+                        "monitored": album.get("monitored", True),
+                        "releaseDate": album.get("release_date", "2019-01-01T00:00:00Z"),
+                    }
+                )
+                for track_index, track in enumerate(album.get("tracks", []), start=1):
+                    track_id = album_id * 100 + track_index
+                    has_file = track.get("has_file", True)
+                    self.tracks.append(
+                        {
+                            "id": track_id,
+                            "artistId": artist_id,
+                            "albumId": album_id,
+                            "title": track["title"],
+                            "foreignRecordingId": track.get("recording_mbid", ""),
+                            "trackFileId": track_id if has_file else 0,
+                            "hasFile": has_file,
+                            "duration": 210000,
+                        }
+                    )
+                    if has_file:
+                        self.files.append(
+                            {
+                                "id": track_id,
+                                "artistId": artist_id,
+                                "albumId": album_id,
+                                "path": f"/music/{entry['name']}/{album['title']}/"
+                                f"{track['title']}.flac",
+                                "dateAdded": track.get("added", "2020-05-01T12:00:00Z"),
+                            }
+                        )
+        self.calls = []
+
+    def __call__(self, url, timeout=None, headers=None):
+        parsed = urllib.parse.urlparse(url)
+        assert (headers or {}).get("X-Api-Key"), "Lidarr requires the API key header"
+        path = parsed.path.rsplit("/", 1)[-1]
+        query = {
+            key: value[0] for key, value in urllib.parse.parse_qs(parsed.query).items()
+        }
+        self.calls.append((path, query))
+
+        artist_id = int(query.get("artistId", 0))
+        if (path, artist_id) in self.fail:
+            raise urllib.error.HTTPError(
+                url, 500, "Internal Server Error", {}, io.BytesIO(b'{"message": "boom"}')
+            )
+
+        album_ids = query.get("albumIds")
+        if path == "album" and album_ids:
+            album_id = int(album_ids)
+            if ("albumid", album_id) in self.fail:
+                raise urllib.error.HTTPError(
+                    url,
+                    500,
+                    "Internal Server Error",
+                    {},
+                    io.BytesIO(b'{"message": "Sequence contains more than one element"}'),
+                )
+            return json.dumps([row for row in self.albums if row["id"] == album_id])
+
+        if path == "artist":
+            return json.dumps(self.artists)
+        source = {"album": self.albums, "track": self.tracks, "trackfile": self.files}[path]
+        if path == "album" and not artist_id:
+            # Lidarr's unfiltered album endpoint returns the lot.
+            return json.dumps(source)
+        if not artist_id:
+            raise urllib.error.HTTPError(
+                url,
+                400,
+                "Bad Request",
+                {},
+                io.BytesIO(b'{"message": "artistId must be provided"}'),
+            )
+        return json.dumps([row for row in source if row["artistId"] == artist_id])
+
+
 @pytest.fixture
 def now_playing():
     """The entry Last.fm prepends for a track in progress: no `date` at all."""
@@ -130,3 +248,53 @@ def now_playing():
         "mbid": "",
         "@attr": {"nowplaying": "true"},
     }
+
+
+class _CountingServer(http.server.ThreadingHTTPServer):
+    """Counts accepted connections, which is what connection reuse is about."""
+
+    daemon_threads = True
+
+    def __init__(self, *args, **kwargs):
+        self.connections = 0
+        super().__init__(*args, **kwargs)
+
+    def process_request(self, request, client_address):
+        self.connections += 1
+        super().process_request(request, client_address)
+
+
+class _Handler(http.server.BaseHTTPRequestHandler):
+    # Without HTTP/1.1 the server closes after every response and no client
+    # could reuse anything, which would make the test prove nothing.
+    protocol_version = "HTTP/1.1"
+
+    def do_GET(self):
+        if self.path.startswith("/boom"):
+            body, status = b'{"message": "boom"}', 500
+        else:
+            body = json.dumps(
+                {"path": self.path, "key": self.headers.get("X-Api-Key")}
+            ).encode()
+            status = 200
+        self.send_response(status)
+        self.send_header("Content-Type", "application/json")
+        self.send_header("Content-Length", str(len(body)))
+        self.end_headers()
+        self.wfile.write(body)
+
+    def log_message(self, *args):
+        pass
+
+
+@pytest.fixture
+def http_server():
+    """A real local HTTP server, for the one component that talks sockets."""
+    server = _CountingServer(("127.0.0.1", 0), _Handler)
+    thread = threading.Thread(target=server.serve_forever, daemon=True)
+    thread.start()
+    try:
+        yield server, f"http://127.0.0.1:{server.server_port}"
+    finally:
+        server.shutdown()
+        server.server_close()

tests/test_music_curator.py

@@ -1,7 +1,10 @@
+import json
+import stat
 import urllib.error
+from pathlib import Path
 
 import pytest
-from conftest import FakeLastfm, make_loved, make_tracks
+from conftest import FakeLastfm, FakeLidarr, make_loved, make_tracks
 
 import music_curator
 
@@ -23,6 +26,76 @@ def ingest(store, api, backfill_limit=0):
     return music_curator.run_once(client, store, "lyra", NOW, backfill_limit)
 
 
+def scrobble_of(artist, track, uts=NOW - 3600, mbid=""):
+    """One recent-tracks entry, in the shape Last.fm actually sends."""
+    return {
+        "artist": {"#text": artist, "mbid": ""},
+        "album": {"#text": "Some Album", "mbid": ""},
+        "name": track,
+        "mbid": mbid,
+        "date": {"uts": str(uts)},
+    }
+
+
+LIBRARY = [
+    {
+        "name": "Yellowcard",
+        "albums": [
+            {
+                "title": "Lift a Sail",
+                "tracks": [
+                    {"title": "Here I Am Alive", "recording_mbid": "rec-alive"},
+                    {"title": "Transmission Home"},
+                ],
+            }
+        ],
+    },
+    {
+        "name": "AC/DC",
+        "albums": [{"title": "Back in Black", "tracks": [{"title": "Hells Bells"}]}],
+    },
+    {
+        "name": "The Prodigy",
+        "albums": [
+            {
+                "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"}]}],
+    },
+]
+
+
+def indexed(tmp_path, scrobbles):
+    """Ingest scrobbles, index the canned library, and match the two."""
+    store = store_at(tmp_path)
+    ingest(store, FakeLastfm(scrobbles))
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=FakeLidarr(LIBRARY)), store
+    )
+    music_curator.match_library(store)
+    return store
+
+
+def verdict(store, artist, track):
+    row = store.connection.execute(
+        "SELECT method, track_id FROM scrobble_key WHERE artist = ? AND track = ?",
+        (artist, track),
+    ).fetchone()
+    return (row["method"], row["track_id"]) if row else (None, None)
+
+
 def test_parse_interval_units():
     assert music_curator.parse_interval("90") == 90
     assert music_curator.parse_interval("30m") == 1800
@@ -241,3 +314,816 @@ def test_main_ingests_through_the_module_level_fetcher(tmp_path, monkeypatch):
 
     assert code == 0
     assert store_at(tmp_path).count() == 20
+
+
+def test_normalise_flattens_the_ways_tags_and_scrobbles_disagree():
+    assert music_curator.normalise("Yellowcard feat. Tay Jardine") == "yellowcard"
+    assert music_curator.normalise("BABYMETAL Feat. F.Hero") == "babymetal"
+    assert music_curator.normalise("AC/DC") == "ac dc"
+    assert music_curator.normalise("The Prodigy") == "prodigy"
+    assert music_curator.normalise("Beyoncé") == "beyonce"
+    assert music_curator.normalise("Simon & Garfunkel") == "simon and garfunkel"
+    assert music_curator.normalise("Breathe (Remastered 2011)") == "breathe"
+    assert music_curator.normalise("Breathe (Live) (Remastered)") == "breathe"
+    assert music_curator.normalise("Vice Grip - Live") == "vice grip"
+
+
+def test_normalise_keeps_a_leading_parenthetical():
+    """Stripping every bracket would destroy real titles."""
+    assert music_curator.normalise("(Don't Fear) The Reaper") == "dont fear the reaper"
+
+
+def test_normalise_treats_apostrophes_and_other_punctuation_differently():
+    """The two rules pull opposite ways: one deletes, the other separates."""
+    assert music_curator.normalise("Don't") == music_curator.normalise("Dont")
+    assert music_curator.normalise("Don’t") == music_curator.normalise("Dont")
+    assert music_curator.normalise("AC/DC") == music_curator.normalise("AC-DC")
+    assert music_curator.normalise("AC/DC") == music_curator.normalise("AC DC")
+    # ...and they must not collide with each other.
+    assert music_curator.normalise("AC/DC") != music_curator.normalise("ACDC")
+
+
+def test_normalise_leaves_with_alone():
+    """`with` appears in too many genuine titles to cut on sight."""
+    assert music_curator.normalise("Dancing with Myself") == "dancing with myself"
+
+
+def test_a_recording_mbid_matches_exactly(tmp_path):
+    store = indexed(tmp_path, [scrobble_of("Yellowcard", "Here I Am Alive", mbid="rec-alive")])
+
+    assert verdict(store, "Yellowcard", "Here I Am Alive")[0] == "mbid"
+
+
+def test_a_guest_credit_still_matches_by_name(tmp_path):
+    """Last.fm puts the guest in the artist field; the file tag does not."""
+    store = indexed(
+        tmp_path, [scrobble_of("Yellowcard feat. Tay Jardine", "Here I Am Alive")]
+    )
+
+    method, track_id = verdict(store, "Yellowcard feat. Tay Jardine", "Here I Am Alive")
+    assert method == "name"
+    assert track_id is not None
+
+
+def test_a_remaster_suffix_on_the_library_side_still_matches(tmp_path):
+    store = indexed(tmp_path, [scrobble_of("The Prodigy", "Breathe")])
+
+    assert verdict(store, "The Prodigy", "Breathe")[0] == "name"
+
+
+def test_music_that_is_not_in_the_library_stays_unmatched(tmp_path):
+    store = indexed(tmp_path, [scrobble_of("Some Band", "Some Song")])
+
+    assert verdict(store, "Some Band", "Some Song") == ("none", None)
+
+
+def test_the_mbid_tier_is_preferred_over_the_name_tier(tmp_path):
+    store = indexed(
+        tmp_path,
+        [scrobble_of("Yellowcard", "Here I Am Alive", mbid="rec-alive")],
+    )
+    method, track_id = verdict(store, "Yellowcard", "Here I Am Alive")
+
+    expected = store.connection.execute(
+        "SELECT id FROM lidarr_track WHERE recording_mbid = 'rec-alive'"
+    ).fetchone()["id"]
+    assert (method, track_id) == ("mbid", expected)
+
+
+def test_keys_carry_the_play_count_and_the_span(tmp_path):
+    store = indexed(
+        tmp_path,
+        [
+            scrobble_of("AC/DC", "Hells Bells", uts=NOW - 7200),
+            scrobble_of("AC/DC", "Hells Bells", uts=NOW - 3600),
+        ],
+    )
+
+    row = store.connection.execute(
+        "SELECT plays, first_uts, last_uts FROM scrobble_key WHERE artist = 'AC/DC'"
+    ).fetchone()
+    assert row["plays"] == 2
+    assert row["last_uts"] - row["first_uts"] == 3600
+
+
+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") == 4
+
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=FakeLidarr(LIBRARY[:1])), store
+    )
+
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_artist") == 1
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE artist_id NOT IN"
+                        " (SELECT id FROM lidarr_artist)") == 0
+
+
+def test_the_index_records_paths_and_when_a_file_landed(tmp_path):
+    store = indexed(tmp_path, [])
+
+    row = store.connection.execute(
+        "SELECT path, added, has_file FROM lidarr_track WHERE title = 'Hells Bells'"
+    ).fetchone()
+    assert row["path"].endswith("Hells Bells.flac")
+    assert row["has_file"] == 1
+    assert row["added"] == music_curator.parse_added("2020-05-01T12:00:00Z")
+
+
+def test_a_track_without_a_file_has_no_path(tmp_path):
+    library = [
+        {
+            "name": "Ghost Artist",
+            "albums": [{"title": "Unowned", "tracks": [{"title": "Missing", "has_file": False}]}],
+        }
+    ]
+    store = store_at(tmp_path)
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=FakeLidarr(library)), store
+    )
+
+    row = store.connection.execute("SELECT path, has_file FROM lidarr_track").fetchone()
+    assert row["has_file"] == 0
+    assert row["path"] is None
+
+
+def test_lidarr_requires_the_api_key_header(tmp_path):
+    """The fake asserts on it, which is the point: a missing header is a 401."""
+    client = music_curator.Lidarr("http://lidarr", "key", transport=FakeLidarr(LIBRARY))
+
+    assert client.get("artist")
+
+
+def test_a_lidarr_failure_is_reported_not_swallowed():
+    def broken(url, timeout=None, headers=None):
+        raise urllib.error.HTTPError(url, 401, "unauthorised", {}, None)
+
+    client = music_curator.Lidarr("http://lidarr", "wrong", transport=broken)
+
+    with pytest.raises(music_curator.LidarrError, match="HTTP 401"):
+        client.get("artist")
+
+
+def test_an_existing_version_one_store_is_migrated_not_discarded(tmp_path):
+    """A rebuilt history is thousands of API requests; migration is additive."""
+    store = store_at(tmp_path)
+    store.add_scrobbles([music_curator.Scrobble(uts=NOW, artist="A", track="B")])
+    store.set_state("schema_version", "1")
+    store.close()
+
+    reopened = store_at(tmp_path)
+
+    assert reopened.count() == 1
+    assert reopened.get_state("schema_version") == music_curator.SCHEMA_VERSION
+
+
+def test_an_unmatched_track_by_a_known_artist_is_a_matcher_miss(tmp_path):
+    """The split that matters: music you own and played, that failed to match."""
+    store = indexed(
+        tmp_path,
+        [
+            scrobble_of("AC/DC", "A Track Lidarr Has Never Heard Of"),
+            scrobble_of("Some Band", "Some Song"),
+        ],
+    )
+
+    misses = 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)"
+    ).fetchall()
+
+    # Only the AC/DC one: "Some Band" is not in the library, so it says nothing
+    # about the matcher.
+    assert [row["artist"] for row in misses] == ["AC/DC"]
+
+
+def test_the_report_runs_over_a_matched_store(tmp_path):
+    store = indexed(tmp_path, [scrobble_of("Yellowcard", "Here I Am Alive", mbid="rec-alive")])
+
+    music_curator.report(store, NOW)
+
+
+def test_matching_is_redone_when_new_scrobbles_arrive(tmp_path):
+    """A pass with no re-index still has to give the new plays a verdict."""
+    store = indexed(tmp_path, [scrobble_of("AC/DC", "Hells Bells")])
+    api = FakeLastfm(
+        [
+            scrobble_of("AC/DC", "Hells Bells"),
+            scrobble_of("Yellowcard", "Transmission Home", uts=NOW - 60),
+        ]
+    )
+
+    music_curator.run_once(client_for(api), store, "lyra", NOW, 0)
+
+    assert verdict(store, "Yellowcard", "Transmission Home")[0] == "name"
+
+
+def test_albums_come_from_the_unfiltered_endpoint(tmp_path):
+    """One request, and the only path that skips albums it cannot hydrate."""
+    api = FakeLidarr(LIBRARY)
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+
+    album_calls = [query for path, query in api.calls if path == "album"]
+    assert album_calls == [{}]
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_album") == 4
+
+
+def test_a_bad_album_falls_back_to_asking_per_artist(tmp_path):
+    """An album with two monitored releases throws in the resource mapper, so
+    the bulk call dies wholesale. Per artist, only its owner is lost."""
+    # The bulk call fails because artist 2 owns the offending album.
+    api = FakeLidarr(LIBRARY, fail=[("album", 0), ("album", 2)])
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+
+    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") == 3
+    assert store.get_state("index_albums_skipped") == "1"
+
+
+def test_a_bad_album_does_not_cost_that_artist_their_tracks(tmp_path):
+    """Tracks come from a different endpoint with a different mapper, so
+    matching survives an album Lidarr cannot serialise."""
+    api = FakeLidarr(LIBRARY, fail=[("album", 0), ("album", 2)])
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE artist_id = 2") == 1
+    assert store.get_state("index_skipped") == "0"
+
+
+def test_an_artist_lidarr_cannot_serve_does_not_kill_the_index(tmp_path):
+    # Artist 2 is AC/DC in LIBRARY; its track lookup fails.
+    api = FakeLidarr(LIBRARY, fail=[("track", 2)])
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+
+    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") == 5
+    assert store.get_state("index_skipped") == "1"
+
+
+def test_a_skipped_artist_is_recorded_so_the_report_can_disown_the_numbers(tmp_path):
+    """An incomplete index makes played music look cold. It has to be loud."""
+    api = FakeLidarr(LIBRARY, fail=[("trackfile", 1)])
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+    music_curator.match_library(store)
+
+    assert store.get_state("index_skipped") == "1"
+
+
+def test_a_clean_index_records_no_skips(tmp_path):
+    store = indexed(tmp_path, [])
+
+    assert store.get_state("index_skipped") == "0"
+
+
+def test_a_lidarr_error_carries_the_url_and_what_the_server_said():
+    """A bare status code sends you looking at the wrong thing entirely."""
+    api = FakeLidarr(LIBRARY, fail=[("album", 0)])
+    client = music_curator.Lidarr("http://lidarr:8686", "key", transport=api)
+
+    with pytest.raises(music_curator.LidarrError) as raised:
+        client.get("album")
+
+    message = str(raised.value)
+    assert "http://lidarr:8686/api/v1/album" in message
+    assert "HTTP 500" in message
+    assert "boom" in message
+
+
+def test_the_offending_album_is_named_not_just_its_artist(tmp_path, caplog):
+    """An artist's whole discography is too much to click through by hand."""
+    # AC/DC is artist 2; its only album is id 201, holding "Hells Bells".
+    api = FakeLidarr(LIBRARY, fail=[("album", 0), ("album", 2), ("albumid", 201)])
+    store = store_at(tmp_path)
+
+    with caplog.at_level("WARNING"):
+        music_curator.index_library(
+            music_curator.Lidarr("http://lidarr", "key", transport=api), store
+        )
+
+    assert "album id 201" in caplog.text
+    assert "Hells Bells" in caplog.text
+
+
+def test_a_transient_failure_is_not_probed_album_by_album(tmp_path, caplog):
+    """When the resolver is the problem every artist fails, and probing each of
+    them multiplies the load that caused it."""
+
+    def unresolvable(url, timeout=None, headers=None):
+        if "artistId" in url:
+            raise urllib.error.URLError("[Errno -3] Try again")
+        return json.dumps(FakeLidarr(LIBRARY).artists) if url.endswith("artist") else "[]"
+
+    store = store_at(tmp_path)
+    client = music_curator.Lidarr("http://lidarr", "key", transport=unresolvable, backoff=0)
+
+    with caplog.at_level("WARNING"):
+        music_curator.index_library(client, store)
+
+    assert "Try again" in caplog.text
+    assert "cannot serialise" not in caplog.text
+
+
+def test_a_transient_failure_is_retried():
+    attempts = []
+
+    def flaky(url, timeout=None, headers=None):
+        attempts.append(url)
+        if len(attempts) < 3:
+            raise urllib.error.URLError("[Errno -3] Try again")
+        return "[]"
+
+    client = music_curator.Lidarr("http://lidarr", "key", transport=flaky, backoff=0)
+
+    assert client.get("artist") == []
+    assert len(attempts) == 3
+
+
+def test_a_lidarr_500_is_not_retried():
+    """It is an exception inside Lidarr's serialisation, not a busy server."""
+    api = FakeLidarr(LIBRARY, fail=[("album", 0)])
+    client = music_curator.Lidarr("http://lidarr", "key", transport=api, backoff=0)
+
+    with pytest.raises(music_curator.LidarrError) as raised:
+        client.get("album")
+
+    assert raised.value.transient is False
+    assert len(api.calls) == 1
+
+
+def test_keep_alive_uses_one_connection_for_many_requests(http_server):
+    """The point of the whole class: one DNS lookup and one socket, not N."""
+    server, base = http_server
+    transport = music_curator.KeepAlive()
+
+    try:
+        for index in range(5):
+            body = transport(f"{base}/api/v1/artist?n={index}", headers={"X-Api-Key": "key"})
+            assert json.loads(body)["key"] == "key"
+    finally:
+        transport.close()
+
+    assert server.connections == 1
+
+
+def test_keep_alive_maps_an_error_status_onto_httperror(http_server):
+    _, base = http_server
+    transport = music_curator.KeepAlive()
+
+    try:
+        with pytest.raises(urllib.error.HTTPError) as raised:
+            transport(f"{base}/boom", headers={"X-Api-Key": "key"})
+        assert raised.value.code == 500
+        assert music_curator.error_detail(raised.value) == "boom"
+    finally:
+        transport.close()
+
+
+def test_lidarr_talks_to_a_real_server_through_keep_alive(http_server):
+    server, base = http_server
+    client = music_curator.Lidarr(base, "secret")
+
+    try:
+        assert client.get("artist", {"x": 1})["key"] == "secret"
+        assert client.get("album")["path"] == "/api/v1/album"
+    finally:
+        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
+
+
+def test_misses_are_split_by_whether_the_library_holds_the_title(tmp_path):
+    """Owning an artist is a weak proxy for owning a track. Counting both as
+    matcher failures overstates the problem and would over-block the cull."""
+    store = indexed(
+        tmp_path,
+        [
+            # The library holds "Hells Bells", but under AC/DC, not Yellowcard:
+            # an attribution disagreement, and a real miss.
+            scrobble_of("Yellowcard", "Hells Bells"),
+            # Yellowcard is in the library; this track is not, under any artist.
+            scrobble_of("Yellowcard", "A Single She Never Bought"),
+        ],
+    )
+
+    def count(extra):
+        return store.connection.execute(
+            "SELECT COUNT(*) 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)"
+            f"  {extra}"
+        ).fetchone()[0]
+
+    assert count("") == 2
+    assert count("AND EXISTS (SELECT 1 FROM lidarr_track t WHERE t.norm_title = k.norm_track)") == 1
+
+
+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
+
+
+def playlist_library(tmp_path):
+    """A library on disk, with mirror MP3s beside the Lidarr source paths."""
+    source = tmp_path / "music"
+    mirror = tmp_path / "mirror"
+    library = [
+        {
+            "name": "Played Band",
+            "albums": [
+                {
+                    "title": "Known",
+                    "tracks": [{"title": "Hit"}, {"title": "Album Track"}],
+                }
+            ],
+        },
+        {
+            "name": "Silent Band",
+            "albums": [{"title": "Unknown", "tracks": [{"title": "Never Heard"}]}],
+        },
+    ]
+    api = FakeLidarr(library)
+    # FakeLidarr invents /music/<artist>/<album>/<title>.flac; put the mirror
+    # MP3s at the paths music-mirror would have produced from those.
+    for handle in api.files:
+        relative = Path(handle["path"]).relative_to("/music")
+        handle["path"] = str(source / relative)
+        mp3 = (mirror / relative).with_suffix(".mp3")
+        mp3.parent.mkdir(parents=True, exist_ok=True)
+        mp3.write_bytes(b"not really an mp3")
+    for artist in api.artists:
+        artist["path"] = str(source / Path(artist["path"]).name)
+    return api, source, mirror
+
+
+def test_playlists_are_written_into_the_mirror(tmp_path):
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    ingest(store, FakeLastfm([scrobble_of("Played Band", "Hit")] * 1))
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    music_curator.match_library(store)
+
+    music_curator.build_playlists(store, mirror, str(source), 100, NOW)
+
+    written = sorted(p.name for p in (mirror / "_playlists").glob("*.m3u"))
+    assert written == [
+        "all-time.m3u",
+        "deep-cuts.m3u",
+        "heavy-rotation.m3u",
+        "neglected.m3u",
+        "unheard-favourites.m3u",
+        "unheard.m3u",
+    ]
+
+    played = (mirror / "_playlists" / "all-time.m3u").read_text().splitlines()
+    assert played[0] == "#EXTM3U"
+    assert played[1].startswith("#EXTINF:")
+    assert "Played Band - Hit" in played[1]
+    # Relative to the playlist file, so the same file works from any mount.
+    assert played[2] == "../Played Band/Known/Hit.mp3"
+    assert (mirror / "_playlists" / "all-time.m3u").parent.joinpath(played[2]).resolve().is_file()
+
+
+def test_an_unplayed_track_lands_in_the_unheard_playlist(tmp_path):
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    ingest(store, FakeLastfm([scrobble_of("Played Band", "Hit")]))
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    music_curator.match_library(store)
+
+    music_curator.build_playlists(store, mirror, str(source), 100, NOW)
+
+    unheard = (mirror / "_playlists" / "unheard.m3u").read_text()
+    assert "Never Heard" in unheard
+    assert "Album Track" in unheard
+    # The one thing that was played must not be in it.
+    assert "Played Band - Hit\n" not in unheard
+
+
+def test_a_track_with_no_mirror_file_is_left_out(tmp_path):
+    """The index knows what Lidarr holds; whether music-mirror has encoded the
+    MP3 yet is a different question, and is checked rather than assumed."""
+    api, source, mirror = playlist_library(tmp_path)
+    for mp3 in mirror.rglob("*.mp3"):
+        mp3.unlink()
+    store = store_at(tmp_path)
+    ingest(store, FakeLastfm([scrobble_of("Played Band", "Hit")]))
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    music_curator.match_library(store)
+
+    total = music_curator.build_playlists(store, mirror, str(source), 100, NOW)
+
+    assert total == 0
+    assert (mirror / "_playlists" / "all-time.m3u").read_text() == "#EXTM3U\n"
+
+
+def test_the_library_root_is_derived_from_the_artist_folders(tmp_path):
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+
+    assert music_curator.library_root_of(store) == str(source)
+
+
+def test_the_playlist_limit_is_honoured(tmp_path):
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    music_curator.match_library(store)
+
+    music_curator.build_playlists(store, mirror, str(source), 1, NOW)
+
+    unheard = (mirror / "_playlists" / "unheard.m3u").read_text().splitlines()
+    assert len([line for line in unheard if line.startswith("#EXTINF")]) == 1
+
+
+def test_the_rotation_moves_weekly_not_every_pass(tmp_path):
+    """A playlist that reorders on every pass is one that has to be re-imported
+    on every pass; the Music app imports a snapshot, it does not track a file."""
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    music_curator.match_library(store)
+
+    def unheard_at(when):
+        music_curator.build_playlists(store, mirror, str(source), 100, when)
+        return (mirror / "_playlists" / "unheard.m3u").read_text()
+
+    same_week = unheard_at(NOW), unheard_at(NOW + 3600)
+    assert same_week[0] == same_week[1]
+
+
+def test_playlists_are_group_readable(tmp_path):
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    music_curator.match_library(store)
+
+    music_curator.build_playlists(store, mirror, str(source), 100, NOW)
+
+    for playlist in (mirror / "_playlists").glob("*.m3u"):
+        assert playlist.stat().st_mode & stat.S_IRGRP, playlist
+
+
+def test_tag_weights_fall_back_to_rank_when_no_count_is_sent():
+    """The documented sample carries only a name and a URL; a live response also
+    carries a count. Neither may be relied on alone."""
+    with_count = music_curator.parse_tags(
+        {"toptags": {"tag": [{"name": "Screamo", "count": 100}, {"name": "emo", "count": 40}]}}
+    )
+    assert with_count == [("screamo", 100), ("emo", 40)]
+
+    without = music_curator.parse_tags(
+        {"toptags": {"tag": [{"name": "screamo"}, {"name": "emo"}]}}
+    )
+    assert [tag for tag, _ in without] == ["screamo", "emo"]
+    assert without[0][1] > without[1][1]
+
+
+def test_a_lone_tag_is_not_a_list():
+    assert music_curator.parse_tags({"toptags": {"tag": {"name": "dnb", "count": 90}}}) == [
+        ("dnb", 90)
+    ]
+
+
+def test_an_artist_lastfm_cannot_answer_for_is_not_asked_again(tmp_path):
+    """Recording the fetch even when it returns nothing is what stops a pass
+    spending a request per unknown artist, forever."""
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    lastfm = FakeLastfm(tags={})
+
+    assert music_curator.sync_tags(client_for(lastfm), store, NOW) == 2
+    before = len(lastfm.calls)
+
+    assert music_curator.sync_tags(client_for(lastfm), store, NOW) == 0
+    assert len(lastfm.calls) == before
+
+
+def test_tags_are_refetched_once_they_go_stale(tmp_path):
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    lastfm = FakeLastfm(tags={})
+    music_curator.sync_tags(client_for(lastfm), store, NOW)
+
+    later = NOW + music_curator.TAG_REFRESH_SECONDS + 1
+    assert music_curator.sync_tags(client_for(lastfm), store, later) == 2
+
+
+def tagged_store(tmp_path, tags):
+    api, source, mirror = playlist_library(tmp_path)
+    store = store_at(tmp_path)
+    music_curator.index_library(
+        music_curator.Lidarr("http://lidarr", "key", transport=api), store
+    )
+    music_curator.sync_tags(client_for(FakeLastfm(tags=tags)), store, NOW)
+    return store, source, mirror
+
+
+def test_a_vibe_selects_by_tag(tmp_path):
+    store, source, mirror = tagged_store(
+        tmp_path,
+        {
+            "artist-mbid-1": [{"name": "screamo", "count": 100}],
+            "artist-mbid-2": [{"name": "classic rock", "count": 100}],
+        },
+    )
+    vibes = [{"name": "screamo", "tags": ["screamo"]}]
+
+    music_curator.build_vibe_playlists(store, vibes, mirror, str(source), 100, NOW)
+
+    written = (mirror / "_playlists" / "screamo.m3u").read_text()
+    assert "Played Band" in written
+    assert "Silent Band" not in written
+
+
+def test_a_weakly_tagged_artist_is_below_the_threshold(tmp_path):
+    """A single low-weight tag is not a genre, it is somebody's stray opinion."""
+    store, source, mirror = tagged_store(
+        tmp_path, {"artist-mbid-1": [{"name": "screamo", "count": 3}]}
+    )
+    vibes = [{"name": "screamo", "tags": ["screamo"]}]
+
+    music_curator.build_vibe_playlists(store, vibes, mirror, str(source), 100, NOW)
+
+    assert (mirror / "_playlists" / "screamo.m3u").read_text() == "#EXTM3U\n"
+
+
+def test_a_vibe_can_be_restricted_by_release_year(tmp_path):
+    """What separates eighties synth records from everything else a synthpop
+    tag drags in."""
+    store, source, mirror = tagged_store(
+        tmp_path, {"artist-mbid-1": [{"name": "synthpop", "count": 100}]}
+    )
+    inside = [{"name": "eighties", "tags": ["synthpop"], "years": [1975, 1992]}]
+    outside = [{"name": "nineties", "tags": ["synthpop"], "years": [1993, 1999]}]
+
+    # FakeLidarr dates every album 2019, so neither window should catch it.
+    music_curator.build_vibe_playlists(store, inside, mirror, str(source), 100, NOW)
+    music_curator.build_vibe_playlists(store, outside, mirror, str(source), 100, NOW)
+    assert (mirror / "_playlists" / "eighties.m3u").read_text() == "#EXTM3U\n"
+    assert (mirror / "_playlists" / "nineties.m3u").read_text() == "#EXTM3U\n"
+
+    modern = [{"name": "modern", "tags": ["synthpop"], "years": [2000, 2030]}]
+    music_curator.build_vibe_playlists(store, modern, mirror, str(source), 100, NOW)
+    assert "Played Band" in (mirror / "_playlists" / "modern.m3u").read_text()
+
+
+def test_the_built_in_vibes_are_all_usable_filenames():
+    for vibe in music_curator.DEFAULT_VIBES:
+        assert music_curator.SAFE_VIBE_NAME.fullmatch(vibe["name"]), vibe["name"]
+        assert vibe["tags"]
+
+
+def test_a_vibes_file_replaces_the_built_in_set(tmp_path):
+    path = tmp_path / "vibes.json"
+    path.write_text(json.dumps([{"name": "mine", "tags": ["shoegaze"]}]))
+
+    assert music_curator.load_vibes(str(path)) == ({"name": "mine", "tags": ["shoegaze"]},)
+    assert music_curator.load_vibes(None) is music_curator.DEFAULT_VIBES
+
+
+@pytest.mark.parametrize(
+    "content",
+    [
+        '{"not": "a list"}',
+        '[{"tags": ["x"]}]',
+        '[{"name": "../escape", "tags": ["x"]}]',
+        '[{"name": "ok"}]',
+        "not json at all",
+    ],
+)
+def test_a_bad_vibes_file_is_refused_up_front(tmp_path, content):
+    """A bad name would otherwise surface as a file written somewhere
+    unintended, which is a poor way to learn about a typo."""
+    path = tmp_path / "vibes.json"
+    path.write_text(content)
+
+    with pytest.raises(ValueError):
+        music_curator.load_vibes(str(path))