Merge pull request 'ci: build the image once instead of twice' (#7) from ci/one-build-not-two into main

Reviewed-on: #7

.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

@@ -109,10 +109,18 @@ matcher. The line to watch is:
 unmatched by an artist the library holds: N pairs, M plays
 ```
 
-That is a track that was played, sitting beside a file it should have matched.
+That is a track that was played by an artist the library holds. It is then
-Those are the matcher's real misses, and every one is a candidate for being
+split again, because owning an artist is a weak proxy for owning a track:
-wrongly called cold in stage four. The report lists the worst fifteen by play
+
-count so they can be eyeballed.
+- **the title exists under another artist** — an attribution disagreement, a
+  remixer or a guest billed as the artist. These are the genuine misses, and
+  each is a candidate for being wrongly called cold in stage four. The report
+  names the artist the library files them under.
+- **the title is nowhere in the library** — never bought. No amount of matching
+  conjures a file that does not exist.
+
+Counting both as matcher failures overstates the problem and would over-block
+the cull. The report lists the worst of each by play count.
 
 ## How the ingest works
 

music_curator.py

@@ -1119,12 +1119,55 @@ def coverage_report(store):
         "   AND EXISTS (SELECT 1 FROM lidarr_artist a WHERE a.norm_name = k.norm_artist)"
     ).fetchone()
     logger.info(
-        "unmatched by an artist the library holds: %d pairs, %d plays -- these are the"
+        "unmatched by an artist the library holds: %d pairs, %d plays",
-        " matcher's misses, not music you do not own",
         suspect["pairs"],
         suspect["plays"],
     )
 
+    # Owning an artist is a weak proxy for owning a track, so that figure alone
+    # overstates the matcher's failings. Split it. A title the library holds
+    # under some other artist is an attribution disagreement -- a remixer
+    # credited as the artist, a guest billed as one -- and is a real miss. A
+    # title the library does not hold at all was simply never bought, and no
+    # amount of matching will conjure it.
+    attribution = store.connection.execute(
+        "SELECT COUNT(*) AS pairs, COALESCE(SUM(plays), 0) AS plays FROM scrobble_key k"
+        " WHERE k.track_id IS NULL"
+        "   AND EXISTS (SELECT 1 FROM lidarr_artist a WHERE a.norm_name = k.norm_artist)"
+        "   AND EXISTS (SELECT 1 FROM lidarr_track t WHERE t.norm_title = k.norm_track)"
+    ).fetchone()
+    logger.info(
+        "  of those, the title exists under another artist: %d pairs, %d plays"
+        " -- attribution disagreements, and the genuine misses",
+        attribution["pairs"],
+        attribution["plays"],
+    )
+    logger.info(
+        "  the rest, %d pairs, %d plays: you own the artist but not the track",
+        suspect["pairs"] - attribution["pairs"],
+        suspect["plays"] - attribution["plays"],
+    )
+
+    mismatched = store.connection.execute(
+        "SELECT k.artist, k.track, k.plays,"
+        "       (SELECT a.name FROM lidarr_track t"
+        "          JOIN lidarr_artist a ON a.id = t.artist_id"
+        "         WHERE t.norm_title = k.norm_track LIMIT 1) AS filed_under"
+        "  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)"
+        " ORDER BY k.plays DESC, k.artist LIMIT 10"
+    ).fetchall()
+    for position, row in enumerate(mismatched, start=1):
+        logger.info(
+            "  attribution %2d: %-45s %4d plays, filed under %s",
+            position,
+            f"{row['artist']} - {row['track']}"[:45],
+            row["plays"],
+            row["filed_under"],
+        )
+
     with_files = store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE has_file = 1")
     played = store.scalar(
         "SELECT COUNT(DISTINCT k.track_id) FROM scrobble_key k"

pyproject.toml

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

tests/test_music_curator.py

@@ -732,3 +732,34 @@ def test_a_bracketed_guest_credit_matches_the_bare_tag(tmp_path):
     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)