Dockerfile

@@ -33,8 +33,14 @@ RUN go build -trimpath \
     -o /out/minstrel ./cmd/minstrel
 
 FROM debian:bookworm-slim
+# ffmpeg: duration probes and the exact-tier audio hash (a SHA-256 of the
+# encoded audio packets, so no decode). libchromaprint-tools: fpcalc, the
+# acoustic fingerprint that tells the same recording at two bitrates apart
+# from two different recordings (M400). Both are baked in at build time so a
+# deployed instance never fetches either (rule 164); fpcalc is shelled out
+# rather than bound because CGO_ENABLED=0 above rules out cgo.
 RUN apt-get update \
-    && apt-get install -y --no-install-recommends ca-certificates ffmpeg \
+    && apt-get install -y --no-install-recommends ca-certificates ffmpeg libchromaprint-tools \
     && rm -rf /var/lib/apt/lists/*
 
 RUN groupadd --system --gid 1000 minstrel \

README.md

@@ -37,8 +37,12 @@ services:
     ports: ['4533:4533']
     volumes:
       # Your music library. Point ./music at wherever your audio files
-      # live. Mounted read-only — Minstrel never writes to your library.
+      # live. Writable, because Minstrel deletes a file when an admin asks
-      - ./music:/music:ro
+      # it to (for example, quarantine's "Delete file"). It never moves,
+      # renames or retags anything. The container runs as uid 1000, so that
+      # user needs write access to the folders. Mount it :ro to forbid even
+      # deletes: those actions then refuse, say why, and delete nothing.
+      - ./music:/music
       # Generated data: playlist cover collages, artist art, caches.
       # The path must match MINSTREL_STORAGE_DATA_DIR, which the image
       # sets to /app/data — keep this mount on /app/data or your cache
@@ -47,7 +51,7 @@ services:
     environment:
       MINSTREL_DATABASE_URL: postgres://minstrel:minstrel@db:5432/minstrel?sslmode=disable
       # Colon-separated library roots to scan; must match the container
-      # path of the read-only music mount above (/music here).
+      # path of the music mount above (/music here).
       MINSTREL_LIBRARY_SCAN_PATHS: /music
     depends_on: [db]
 

android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminQuarantineScreen.kt

@@ -15,10 +15,14 @@ import androidx.compose.material3.HorizontalDivider
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.OutlinedButton
 import androidx.compose.material3.Scaffold
+import androidx.compose.material3.SnackbarHost
+import androidx.compose.material3.SnackbarHostState
 import androidx.compose.material3.Text
 import androidx.compose.material3.TextButton
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.text.style.TextOverflow
@@ -42,6 +46,12 @@ fun AdminQuarantineScreen(
     viewModel: AdminQuarantineViewModel = hiltViewModel(),
 ) {
     val state by viewModel.uiState.collectAsStateWithLifecycle()
+    val snackbarHostState = remember { SnackbarHostState() }
+    LaunchedEffect(Unit) {
+        viewModel.transientMessages.collect { msg ->
+            snackbarHostState.showSnackbar(msg)
+        }
+    }
     Scaffold(
         contentWindowInsets = ShellContentWindowInsets,
         modifier = Modifier.fillMaxSize(),
@@ -53,6 +63,7 @@ fun AdminQuarantineScreen(
                 onBack = { navController.popBackStack() },
             )
         },
+        snackbarHost = { SnackbarHost(snackbarHostState) },
     ) { inner ->
         PullToRefreshScaffold(
             onRefresh = { viewModel.refresh().join() },

android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminQuarantineViewModel.kt

@@ -10,10 +10,13 @@ import com.fabledsword.minstrel.events.EventsStream
 import com.fabledsword.minstrel.models.AdminQuarantineItemRef
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.Job
+import kotlinx.coroutines.channels.Channel
+import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.filter
+import kotlinx.coroutines.flow.receiveAsFlow
 import kotlinx.coroutines.launch
 import javax.inject.Inject
 
@@ -34,6 +37,15 @@ class AdminQuarantineViewModel @Inject constructor(
     private val internal = MutableStateFlow<AdminQuarantineUiState>(AdminQuarantineUiState.Loading)
     val uiState: StateFlow<AdminQuarantineUiState> = internal.asStateFlow()
 
+    /**
+     * One-shot messages for the screen's snackbar. A failed action has to say
+     * why: the row quietly reappearing reads as a glitch, and for a Delete
+     * file refused by a read-only library it hides the one thing the
+     * operator can fix (#3918).
+     */
+    private val transientMessagesChannel = Channel<String>(Channel.BUFFERED)
+    val transientMessages: Flow<String> = transientMessagesChannel.receiveAsFlow()
+
     init {
         refresh()
         viewModelScope.launch {
@@ -86,8 +98,9 @@ class AdminQuarantineViewModel @Inject constructor(
             try {
                 action(trackId)
             } catch (
-                @Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable,
+                @Suppress("TooGenericExceptionCaught") e: Throwable,
             ) {
+                transientMessagesChannel.trySend(ErrorCopy.fromThrowable(e))
                 refresh()
             }
         }

android/app/src/main/java/com/fabledsword/minstrel/api/ErrorCopy.kt

@@ -37,18 +37,35 @@ object ErrorCopy {
      * as connection failures.
      */
     fun fromThrowable(t: Throwable): String = when (t) {
-        is HttpException -> messageFor(codeFromHttp(t))
+        is HttpException -> fromHttp(t)
         is IOException -> messageFor("connection_refused")
         else -> TABLE.getValue("unknown")
     }
 
-    private fun codeFromHttp(e: HttpException): String {
+    /**
+     * Codes whose server message carries specifics the operator needs in
+     * order to act — which directory, which uid — that fixed copy cannot say.
+     * For these the message follows the copy (#3918). Kept to a named set on
+     * purpose: most server messages are internal detail. Mirrors web's
+     * errors.ts.
+     */
+    private val DETAIL_CODES = setOf("library_not_writable", "file_delete_failed")
+
+    private fun fromHttp(e: HttpException): String {
+        val body = bodyFromHttp(e)
+        val copy = messageFor(body.code.ifEmpty { "unknown" })
+        return if (body.code in DETAIL_CODES && body.message.isNotBlank()) {
+            "$copy ${body.message}"
+        } else {
+            copy
+        }
+    }
+
+    private fun bodyFromHttp(e: HttpException): Body {
         val raw = runCatching { e.response()?.errorBody()?.string() }.getOrNull()
-            ?: return "unknown"
+            ?: return Body()
-        val code = runCatching { json.decodeFromString<Envelope>(raw).error?.code }
+        return runCatching { json.decodeFromString<Envelope>(raw).error }
-            .getOrNull()
+            .getOrNull() ?: Body()
-            .orEmpty()
-        return code.ifEmpty { "unknown" }
     }
 
     private val TABLE: Map<String, String> = mapOf(
@@ -99,6 +116,8 @@ object ErrorCopy {
         "request_not_pending" to "This request is no longer pending.",
         "request_not_found" to "That request no longer exists.",
         "track_not_found" to "That track no longer exists.",
+        "library_not_writable" to "The music library isn't writable by the server.",
+        "file_delete_failed" to "The file couldn't be deleted.",
         "album_not_found" to "That album no longer exists.",
         "artist_not_found" to "That artist no longer exists.",
         "playlist_not_found" to "That playlist no longer exists.",

android/app/src/test/java/com/fabledsword/minstrel/api/ErrorCopyTest.kt

@@ -0,0 +1,60 @@
+package com.fabledsword.minstrel.api
+
+import okhttp3.MediaType.Companion.toMediaType
+import okhttp3.ResponseBody.Companion.toResponseBody
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Test
+import retrofit2.HttpException
+import retrofit2.Response
+import java.io.IOException
+
+class ErrorCopyTest {
+    private fun httpError(status: Int, body: String): HttpException =
+        HttpException(
+            Response.error<Unit>(status, body.toResponseBody("application/json".toMediaType())),
+        )
+
+    @Test
+    fun libraryNotWritableAppendsTheServerDetail() {
+        val detail = "Minstrel runs as uid 1000, gid 1000 and cannot delete from /music/A " +
+            "(read-only file system). The library mount must be writable by that user. " +
+            "Nothing was deleted."
+        val e = httpError(409, """{"error":{"code":"library_not_writable","message":"$detail"}}""")
+
+        assertEquals(
+            "${ErrorCopy.messageFor("library_not_writable")} $detail",
+            ErrorCopy.fromThrowable(e),
+        )
+    }
+
+    @Test
+    fun detailCodeWithoutAMessageShowsTheCopyAlone() {
+        val e = httpError(409, """{"error":{"code":"library_not_writable","message":""}}""")
+
+        assertEquals(ErrorCopy.messageFor("library_not_writable"), ErrorCopy.fromThrowable(e))
+    }
+
+    // Server messages are usually internal detail; appending them for every
+    // code would leak driver errors into snackbars. This pins the scope.
+    @Test
+    fun otherCodesNeverCarryTheServerMessage() {
+        val e = httpError(404, """{"error":{"code":"track_not_found","message":"pgx: no rows"}}""")
+
+        assertEquals(ErrorCopy.messageFor("track_not_found"), ErrorCopy.fromThrowable(e))
+    }
+
+    @Test
+    fun anUnparseableBodyFallsBackToUnknown() {
+        val e = httpError(500, "not json")
+
+        assertEquals(ErrorCopy.messageFor("unknown"), ErrorCopy.fromThrowable(e))
+    }
+
+    @Test
+    fun transportFailureMapsToConnectionRefused() {
+        assertEquals(
+            ErrorCopy.messageFor("connection_refused"),
+            ErrorCopy.fromThrowable(IOException("refused")),
+        )
+    }
+}

internal/api/admin_quarantine.go

@@ -133,6 +133,13 @@ func (h *handlers) handleDeleteQuarantineFile(w http.ResponseWriter, r *http.Req
 	}
 	action, err := h.lidarrQuarantine.DeleteFile(r.Context(), id, admin.ID)
 	if err != nil {
+		// Written in the enveloped shape, not writeAdminJSONErr's bare code: the
+		// message is the part that tells the operator which directory and uid.
+		if apiErr, ok := fileRemoveAPIError(err); ok {
+			logFileRemoveFailure(h.logger, apiErr, "track_id", uuidToString(id))
+			writeErr(w, apiErr)
+			return
+		}
 		switch {
 		case errors.Is(err, lidarrquarantine.ErrTrackNotFound):
 			writeAdminJSONErr(w, http.StatusNotFound, "track_not_found")

internal/api/admin_quarantine_test.go

@@ -69,7 +69,7 @@ func installQuarantineClientFn(t *testing.T, h *handlers) {
 		}
 		return lidarr.NewClient(c.BaseURL, c.APIKey)
 	}
-	h.lidarrQuarantine = lidarrquarantine.NewService(h.pool, cfg, clientFn)
+	h.lidarrQuarantine = lidarrquarantine.NewService(h.pool, cfg, clientFn, h.dataDir)
 }
 
 // flagDirect bypasses the HTTP handler to seed a quarantine row via the

internal/api/admin_tracks.go

@@ -23,15 +23,17 @@ type removeTrackResponse struct {
 
 // handleRemoveTrack implements DELETE /api/admin/tracks/{id}?unmonitor=true|false.
 //
-// Admin-only (gated by auth.RequireAdmin on the /admin route group). Always
+// Admin-only (gated by auth.RequireAdmin on the /admin route group). Deletes the
-// deletes the file + DB row and runs the album/artist cascade tidy-up. When
+// file, then the DB row, and runs the album/artist cascade tidy-up — and deletes
+// nothing at all when the file cannot be removed (#3918). When
 // unmonitor=true and the track has an mbid, also calls Lidarr.UnmonitorTrack
 // — failure there is non-fatal (the destructive part already completed) and
 // surfaces as `lidarr_unmonitor_failed: true` in the success envelope.
 //
 // Per spec §5, Lidarr-side errors during the unmonitor step do NOT map to
-// wire error codes; the only error codes this handler emits are not_found,
+// wire error codes. The codes this handler emits are not_found,
-// server_error, plus the auth codes the middleware emits upstream.
+// library_not_writable (409) and file_delete_failed when the file could not be
+// removed, server_error, plus the auth codes the middleware emits upstream.
 func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
 	idStr := chi.URLParam(r, "id")
 	trackID, ok := parseUUID(idStr)
@@ -66,6 +68,11 @@ func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
 			writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "track not found"})
 			return
 		}
+		if apiErr, ok := fileRemoveAPIError(err); ok {
+			logFileRemoveFailure(h.logger, apiErr, "track_id", idStr)
+			writeErr(w, apiErr)
+			return
+		}
 		h.logger.Error("api: remove track failed", "err", err, "track_id", idStr)
 		writeErr(w, apierror.InternalMsg("remove failed", err))
 		return

internal/api/auth_test.go

@@ -65,7 +65,7 @@ func testHandlers(t *testing.T) (*handlers, *pgxpool.Pool) {
 	}
 	lidarrCfg := lidarrconfig.New(pool)
 	lidarrReqs := lidarrrequests.NewService(pool, lidarrCfg, nil, nil)
-	lidarrQuar := lidarrquarantine.NewService(pool, lidarrCfg, nil)
+	lidarrQuar := lidarrquarantine.NewService(pool, lidarrCfg, nil, "")
 	// tracks.Service has no Lidarr unmonitorer in tests by default; the
 	// admin-tracks tests below override h.tracks via installTracksLidarrStub
 	// when they need a stubbed Lidarr.

internal/api/file_remove_error.go

@@ -0,0 +1,58 @@
+package api
+
+import (
+	"errors"
+	"fmt"
+	"log/slog"
+	"net/http"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/library"
+)
+
+// fileRemoveAPIError answers a delete that could not reach the track's file
+// (#3918). Both delete endpoints use it, so the operator gets the same
+// explanation from the admin remove-track action and from quarantine's Delete
+// file.
+//
+// The unwritable case is a 409 rather than a 500 because nothing is broken: the
+// request conflicts with how the library is mounted, and the fix is the
+// operator's. The message names the directory — removal writes to the parent,
+// not the file — and the uid/gid the process runs as, which is the half of a
+// permission problem invisible from the host. Every case says nothing was
+// deleted, because that is exactly what the operator will be worried about.
+func fileRemoveAPIError(err error) (*apierror.Error, bool) {
+	var fre *library.FileRemoveError
+	if !errors.As(err, &fre) {
+		return nil, false
+	}
+	if fre.NotWritable() {
+		return &apierror.Error{
+			Status: http.StatusConflict,
+			Code:   "library_not_writable",
+			Message: fmt.Sprintf(
+				"Minstrel runs as uid %d, gid %d and cannot delete from %s (%s). "+
+					"The library mount must be writable by that user. Nothing was deleted.",
+				fre.UID, fre.GID, fre.Dir(), fre.Reason()),
+			Cause: err,
+		}, true
+	}
+	return &apierror.Error{
+		Status:  http.StatusInternalServerError,
+		Code:    "file_delete_failed",
+		Message: fmt.Sprintf("Could not delete %s (%s). Nothing was deleted.", fre.Path, fre.Reason()),
+		Cause:   err,
+	}, true
+}
+
+// logFileRemoveFailure records a delete that could not reach its file. An
+// unwritable library is an environment fact the operator can fix, so it is a
+// Warn; anything else is a real fault.
+func logFileRemoveFailure(logger *slog.Logger, apiErr *apierror.Error, attrs ...any) {
+	attrs = append(attrs, "code", apiErr.Code, "err", apiErr.Cause)
+	if apiErr.Status == http.StatusConflict {
+		logger.Warn("api: track file could not be deleted", attrs...)
+		return
+	}
+	logger.Error("api: track file could not be deleted", attrs...)
+}

internal/api/file_remove_error_test.go

@@ -0,0 +1,96 @@
+package api
+
+import (
+	"errors"
+	"fmt"
+	"io/fs"
+	"net/http"
+	"strings"
+	"syscall"
+	"testing"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/library"
+)
+
+const removeTestPath = "/music/Moe Shop/WWW (2020)/01 - WWW.mp3"
+
+// removeFailure builds the error a delete service returns when the file would
+// not go, wrapped the way lidarrquarantine.DeleteFile and tracks.RemoveTrack
+// wrap it — the mapping has to see through that.
+func removeFailure(errno syscall.Errno) error {
+	return fmt.Errorf("delete file: %w", &library.FileRemoveError{
+		Path: removeTestPath, UID: 1000, GID: 1000,
+		Err: &fs.PathError{Op: "remove", Path: removeTestPath, Err: errno},
+	})
+}
+
+func TestFileRemoveAPIError(t *testing.T) {
+	cases := []struct {
+		name       string
+		errno      syscall.Errno
+		wantStatus int
+		wantCode   string
+		wantIn     []string
+	}{
+		{
+			name: "read-only mount", errno: syscall.EROFS,
+			wantStatus: http.StatusConflict, wantCode: "library_not_writable",
+			wantIn: []string{"uid 1000, gid 1000", "/music/Moe Shop/WWW (2020)", "read-only file system", "Nothing was deleted"},
+		},
+		{
+			name: "permission denied", errno: syscall.EACCES,
+			wantStatus: http.StatusConflict, wantCode: "library_not_writable",
+			wantIn: []string{"permission denied", "Nothing was deleted"},
+		},
+		{
+			name: "operation not permitted", errno: syscall.EPERM,
+			wantStatus: http.StatusConflict, wantCode: "library_not_writable",
+			wantIn: []string{"operation not permitted"},
+		},
+		{
+			name: "i/o error", errno: syscall.EIO,
+			wantStatus: http.StatusInternalServerError, wantCode: "file_delete_failed",
+			wantIn: []string{removeTestPath, "input/output error", "Nothing was deleted"},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			apiErr, ok := fileRemoveAPIError(removeFailure(tc.errno))
+			if !ok {
+				t.Fatal("a wrapped *library.FileRemoveError was not recognised")
+			}
+			if apiErr.Status != tc.wantStatus || apiErr.Code != tc.wantCode {
+				t.Fatalf("got %d %s, want %d %s", apiErr.Status, apiErr.Code, tc.wantStatus, tc.wantCode)
+			}
+			for _, want := range tc.wantIn {
+				if !strings.Contains(apiErr.Message, want) {
+					t.Errorf("message %q lacks %q", apiErr.Message, want)
+				}
+			}
+		})
+	}
+}
+
+// The unwritable answer must name the DIRECTORY. Removal needs write access to
+// the parent, so a message naming the file would send the operator to fix the
+// wrong permissions. The directory is a prefix of the file path, which is why a
+// plain "contains the directory" check could never catch that regression.
+func TestFileRemoveAPIError_NotWritableNamesTheDirectoryNotTheFile(t *testing.T) {
+	apiErr, _ := fileRemoveAPIError(removeFailure(syscall.EROFS))
+	if strings.Contains(apiErr.Message, "01 - WWW.mp3") {
+		t.Fatalf("message names the file rather than its directory: %q", apiErr.Message)
+	}
+}
+
+func TestFileRemoveAPIError_IgnoresOtherErrors(t *testing.T) {
+	for name, err := range map[string]error{
+		"nil":         nil,
+		"plain error": errors.New("delete track: connection reset"),
+		"path error":  &fs.PathError{Op: "remove", Path: removeTestPath, Err: syscall.EROFS},
+		"not found":   library.ErrTrackNotFound,
+	} {
+		if _, ok := fileRemoveAPIError(err); ok {
+			t.Errorf("%s: mapped as a file-remove failure", name)
+		}
+	}
+}

internal/api/radio.go

@@ -101,6 +101,10 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) {
 	candidates, err := recommendation.LoadCandidatesFromSimilarity(
 		r.Context(), q, user.ID, seedID,
 		h.recCfg.RecentlyPlayedHours, currentVec, exclude, limits,
+		// A fresh seed per request (#3889): radio is a new session each time
+		// and SHOULD draw differently. The system mixes are the surfaces that
+		// promise repeatability; this is not one of them.
+		strconv.FormatInt(time.Now().UnixNano(), 36),
 	)
 	if err != nil {
 		h.logger.Warn("api: radio: similarity-pool failed; falling back to whole-library", "err", err)

internal/db/dbq/fingerprints.sql.go

@@ -0,0 +1,59 @@
+// Code generated by sqlc. DO NOT EDIT.
+// versions:
+//   sqlc v1.31.1
+// source: fingerprints.sql
+
+package dbq
+
+import (
+	"context"
+
+	"github.com/jackc/pgx/v5/pgtype"
+)
+
+const deleteTrackFingerprint = `-- name: DeleteTrackFingerprint :exec
+DELETE FROM track_fingerprints WHERE track_id = $1
+`
+
+// A file changed but could not be fingerprinted, for a reason unrelated to the
+// file. The stored row describes the OLD bytes, so it goes and the backfill
+// re-derives it — nothing may keep trusting a stale identity.
+func (q *Queries) DeleteTrackFingerprint(ctx context.Context, trackID pgtype.UUID) error {
+	_, err := q.db.Exec(ctx, deleteTrackFingerprint, trackID)
+	return err
+}
+
+const upsertTrackFingerprint = `-- name: UpsertTrackFingerprint :exec
+INSERT INTO track_fingerprints (
+    track_id, audio_stream_sha256, chromaprint, fingerprint_version
+) VALUES (
+    $1, $2, $3,
+    $4
+)
+ON CONFLICT (track_id) DO UPDATE SET
+    audio_stream_sha256 = EXCLUDED.audio_stream_sha256,
+    chromaprint         = EXCLUDED.chromaprint,
+    fingerprint_version = EXCLUDED.fingerprint_version,
+    computed_at         = now()
+`
+
+type UpsertTrackFingerprintParams struct {
+	TrackID            pgtype.UUID
+	AudioStreamSha256  []byte
+	Chromaprint        []int32
+	FingerprintVersion int16
+}
+
+// Written whenever a track's fingerprint is derived: by the scan when a file is
+// new or its bytes changed, and by the backfill (#3908) for rows derived by an
+// older method. Replaces the row wholesale — a fingerprint of the old bytes has
+// no standing once the file has changed.
+func (q *Queries) UpsertTrackFingerprint(ctx context.Context, arg UpsertTrackFingerprintParams) error {
+	_, err := q.db.Exec(ctx, upsertTrackFingerprint,
+		arg.TrackID,
+		arg.AudioStreamSha256,
+		arg.Chromaprint,
+		arg.FingerprintVersion,
+	)
+	return err
+}

internal/db/dbq/models.go

@@ -667,6 +667,14 @@ type Track struct {
 	MissingSince      pgtype.Timestamptz
 }
 
+type TrackFingerprint struct {
+	TrackID            pgtype.UUID
+	AudioStreamSha256  []byte
+	Chromaprint        []int32
+	FingerprintVersion int16
+	ComputedAt         pgtype.Timestamptz
+}
+
 type TrackSimilarity struct {
 	TrackAID  pgtype.UUID
 	TrackBID  pgtype.UUID

internal/db/dbq/recommendation.sql.go

@@ -829,7 +829,7 @@ similar_artists AS (
     JOIN seed_artist sa ON asim.artist_a_id = sa.artist_id
     WHERE asim.source = 'listenbrainz'
       AND t.id NOT IN (SELECT id FROM excluded_ids)
-    ORDER BY asim.score DESC, random()
+    ORDER BY asim.score DESC, md5(t.id::text || $12::text)
     LIMIT $6
 ),
 tag_overlap AS (
@@ -857,7 +857,7 @@ likes_overlap AS (
           WHERE t.id = gl.track_id
             AND trim(g_overlap.g) IN (SELECT tag FROM seed_tags)
       )
-    ORDER BY random()
+    ORDER BY md5(gl.track_id::text || $12::text)
     LIMIT $8
 ),
 taste_overlap AS (
@@ -884,7 +884,7 @@ coplay_artists AS (
     WHERE asim.source = 'user_cooccurrence'
       AND t.id NOT IN (SELECT id FROM excluded_ids)
       AND t.id <> $2
-    ORDER BY asim.score DESC, random()
+    ORDER BY asim.score DESC, md5(t.id::text || $12::text)
     LIMIT $11
 ),
 random_fill AS (
@@ -900,7 +900,7 @@ random_fill AS (
           UNION SELECT track_id FROM taste_overlap
           UNION SELECT track_id FROM coplay_artists
       )
-    ORDER BY random()
+    ORDER BY md5(t.id::text || $12::text)
     LIMIT $9
 )
 SELECT
@@ -938,17 +938,18 @@ GROUP BY t.id, t.title, t.album_id, t.artist_id, t.duration_ms, t.file_path,
 `
 
 type LoadRadioCandidatesV2Params struct {
-	UserID  pgtype.UUID
+	UserID   pgtype.UUID
-	ID      pgtype.UUID
+	ID       pgtype.UUID
-	Column3 interface{}
+	Column3  interface{}
-	Column4 []pgtype.UUID
+	Column4  []pgtype.UUID
-	Limit   int32
+	Limit    int32
-	Limit_2 int32
+	Limit_2  int32
-	Limit_3 int32
+	Limit_3  int32
-	Limit_4 int32
+	Limit_4  int32
-	Limit_5 int32
+	Limit_5  int32
-	Limit_6 int32
+	Limit_6  int32
-	Limit_7 int32
+	Limit_7  int32
+	Column12 string
 }
 
 type LoadRadioCandidatesV2Row struct {
@@ -971,8 +972,22 @@ type LoadRadioCandidatesV2Row struct {
 //	enter the pool even when the similarity/random arms miss them; scored
 //	in Go via TasteMatch, so sim_score here is 0 pool-inclusion),
 //	$11 coplay_artists K (#1533 — tracks by artists co-played across the
-//	instance with the seed's artist; source='user_cooccurrence').
+//	instance with the seed's artist; source='user_cooccurrence'),
+//	$12 order_seed (text) — see below.
 //
+// $12 REPLACES `ORDER BY random()` IN FOUR ARMS (#3889). Those arms returned
+// a stable set only while their LIMIT exceeded the rows eligible for them: at
+// that point they returned all of them and the order stopped mattering,
+// because the caller sorts by track id before scoring. Below that threshold
+// they returned a random SUBSET, and two builds on the same day drew
+// different ones — so "daily determinism" held by accident, and only for
+// libraries smaller than the limits.
+//
+// md5(id || seed) keeps the intent — an arbitrary spread that changes when
+// the seed does — while making it reproducible for a given seed. The CALLER
+// decides what that means: system mixes pass a per-(user, day) string and get
+// the determinism they promise; radio passes a fresh value per request and
+// keeps varying, which is what a radio should do.
 // Returns same shape as LoadRadioCandidates plus similarity_score column.
 func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandidatesV2Params) ([]LoadRadioCandidatesV2Row, error) {
 	rows, err := q.db.Query(ctx, loadRadioCandidatesV2,
@@ -987,6 +1002,7 @@ func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandid
 		arg.Limit_5,
 		arg.Limit_6,
 		arg.Limit_7,
+		arg.Column12,
 	)
 	if err != nil {
 		return nil, err

internal/db/migrations/0058_track_fingerprints.down.sql

@@ -0,0 +1 @@
+DROP TABLE track_fingerprints;

internal/db/migrations/0058_track_fingerprints.up.sql

@@ -0,0 +1,38 @@
+-- 0058_track_fingerprints.up.sql — an acoustic identity per track (Scribe
+-- milestone #400: #3905, #3906).
+--
+-- A table of its own rather than columns on tracks, for the hot path's sake:
+-- tracks is read with SELECT * by eight queries, among them ListTracksByAlbum,
+-- SearchTracks and GetTracksByIDs — album pages, search, the Subsonic surface.
+-- A ~4 KB chromaprint column on tracks would be de-TOASTed on every one of
+-- those reads to carry a value only the duplicate sweep ever looks at.
+--
+-- What a row means, which the backfill depends on:
+--   no row                          never fingerprinted
+--   fingerprint_version < current   derived by an older method; re-derive it
+--   fingerprint_version = current   attempted; a NULL value means that tool
+--                                   failed on this file, and it is not retried
+--                                   until the file changes
+-- A failure that says nothing about the file — a timeout, a cancelled scan, a
+-- missing binary — writes no row at all, so the backfill tries again.
+CREATE TABLE track_fingerprints (
+    -- CASCADE is right here, unlike for the likes and play history M400's
+    -- merge has to carry across: a fingerprint describes one file's bytes and
+    -- means nothing once that file's row is gone.
+    track_id            uuid        PRIMARY KEY REFERENCES tracks (id) ON DELETE CASCADE,
+    -- SHA-256 of the ENCODED audio packets (ffmpeg -c:a copy -f hash), not of
+    -- decoded samples. internal/library/fingerprint.go says why.
+    audio_stream_sha256 bytea
+        CHECK (audio_stream_sha256 IS NULL OR octet_length(audio_stream_sha256) = 32),
+    -- fpcalc -raw -signed: the same 32 bits per item, stored signed because
+    -- integer is.
+    chromaprint         integer[],
+    fingerprint_version smallint    NOT NULL,
+    computed_at         timestamptz NOT NULL DEFAULT now()
+);
+
+-- The exact duplicate tier is an equality match on this column. Partial
+-- because a NULL is never looked up — it only means the hash was not taken.
+CREATE INDEX track_fingerprints_audio_stream_sha256
+    ON track_fingerprints (audio_stream_sha256)
+    WHERE audio_stream_sha256 IS NOT NULL;

internal/db/queries/fingerprints.sql

@@ -0,0 +1,22 @@
+-- name: UpsertTrackFingerprint :exec
+-- Written whenever a track's fingerprint is derived: by the scan when a file is
+-- new or its bytes changed, and by the backfill (#3908) for rows derived by an
+-- older method. Replaces the row wholesale — a fingerprint of the old bytes has
+-- no standing once the file has changed.
+INSERT INTO track_fingerprints (
+    track_id, audio_stream_sha256, chromaprint, fingerprint_version
+) VALUES (
+    sqlc.arg(track_id), sqlc.narg(audio_stream_sha256), sqlc.narg(chromaprint),
+    sqlc.arg(fingerprint_version)
+)
+ON CONFLICT (track_id) DO UPDATE SET
+    audio_stream_sha256 = EXCLUDED.audio_stream_sha256,
+    chromaprint         = EXCLUDED.chromaprint,
+    fingerprint_version = EXCLUDED.fingerprint_version,
+    computed_at         = now();
+
+-- name: DeleteTrackFingerprint :exec
+-- A file changed but could not be fingerprinted, for a reason unrelated to the
+-- file. The stored row describes the OLD bytes, so it goes and the backfill
+-- re-derives it — nothing may keep trusting a stale identity.
+DELETE FROM track_fingerprints WHERE track_id = $1;

internal/db/queries/recommendation.sql

@@ -45,7 +45,22 @@ WHERE t.id <> $2
 --   enter the pool even when the similarity/random arms miss them; scored
 --   in Go via TasteMatch, so sim_score here is 0 pool-inclusion),
 --   $11 coplay_artists K (#1533 — tracks by artists co-played across the
---   instance with the seed's artist; source='user_cooccurrence').
+--   instance with the seed's artist; source='user_cooccurrence'),
+--   $12 order_seed (text) — see below.
+--
+-- $12 REPLACES `ORDER BY random()` IN FOUR ARMS (#3889). Those arms returned
+-- a stable set only while their LIMIT exceeded the rows eligible for them: at
+-- that point they returned all of them and the order stopped mattering,
+-- because the caller sorts by track id before scoring. Below that threshold
+-- they returned a random SUBSET, and two builds on the same day drew
+-- different ones — so "daily determinism" held by accident, and only for
+-- libraries smaller than the limits.
+--
+-- md5(id || seed) keeps the intent — an arbitrary spread that changes when
+-- the seed does — while making it reproducible for a given seed. The CALLER
+-- decides what that means: system mixes pass a per-(user, day) string and get
+-- the determinism they promise; radio passes a fresh value per request and
+-- keeps varying, which is what a radio should do.
 -- Returns same shape as LoadRadioCandidates plus similarity_score column.
 
 WITH
@@ -87,7 +102,7 @@ similar_artists AS (
     JOIN seed_artist sa ON asim.artist_a_id = sa.artist_id
     WHERE asim.source = 'listenbrainz'
       AND t.id NOT IN (SELECT id FROM excluded_ids)
-    ORDER BY asim.score DESC, random()
+    ORDER BY asim.score DESC, md5(t.id::text || $12::text)
     LIMIT $6
 ),
 tag_overlap AS (
@@ -115,7 +130,7 @@ likes_overlap AS (
           WHERE t.id = gl.track_id
             AND trim(g_overlap.g) IN (SELECT tag FROM seed_tags)
       )
-    ORDER BY random()
+    ORDER BY md5(gl.track_id::text || $12::text)
     LIMIT $8
 ),
 taste_overlap AS (
@@ -142,7 +157,7 @@ coplay_artists AS (
     WHERE asim.source = 'user_cooccurrence'
       AND t.id NOT IN (SELECT id FROM excluded_ids)
       AND t.id <> $2
-    ORDER BY asim.score DESC, random()
+    ORDER BY asim.score DESC, md5(t.id::text || $12::text)
     LIMIT $11
 ),
 random_fill AS (
@@ -158,7 +173,7 @@ random_fill AS (
           UNION SELECT track_id FROM taste_overlap
           UNION SELECT track_id FROM coplay_artists
       )
-    ORDER BY random()
+    ORDER BY md5(t.id::text || $12::text)
     LIMIT $9
 )
 SELECT

internal/dbtest/reset.go

@@ -87,6 +87,7 @@ var dataTables = []string{
 	// pristine Discover knobs rather than whatever a previous test tuned.
 	"discover_tuning",
 	"recommendation_tuning_audit",
+	"track_fingerprints", // M400
 	"tracks",
 	"albums",
 	"artists",

internal/library/delete.go

@@ -5,12 +5,16 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
+	"log/slog"
 	"os"
+	"path/filepath"
+	"syscall"
 
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgtype"
 	"github.com/jackc/pgx/v5/pgxpool"
 
+	"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
 	syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
 )
@@ -19,54 +23,159 @@ import (
 // that has no row in tracks.
 var ErrTrackNotFound = errors.New("library: track not found")
 
-// DeleteTrackFile removes a track file from disk and its row from the
+// removeFile is os.Remove behind a variable so a test can make removal fail the
-// tracks table. Album and artist rows are left untouched.
+// way a read-only mount or a wrongly-owned directory does. A chmod-based test
+// cannot stand in for that: root ignores permission bits, so in a CI container
+// running as root it would pass without ever exercising the failure.
+var removeFile = os.Remove
+
+// FileRemoveError reports that a track's file exists but could not be removed.
+// When DeleteTrackFile returns one, NOTHING was deleted: the row, its likes, its
+// play history and its playlist memberships are all intact.
+type FileRemoveError struct {
+	Path string
+	// UID and GID are the identity the server process runs as — the half of a
+	// permission problem the operator cannot see from the host side.
+	UID, GID int
+	Err      error
+}
+
+func (e *FileRemoveError) Error() string { return fmt.Sprintf("remove track file: %v", e.Err) }
+
+func (e *FileRemoveError) Unwrap() error { return e.Err }
+
+// Dir is the directory removal needs write access to. Unlinking a file writes to
+// its PARENT, so a world-writable file inside a read-only directory still cannot
+// be removed — naming the file's own permissions would send the operator to the
+// wrong place.
+func (e *FileRemoveError) Dir() string { return filepath.Dir(e.Path) }
+
+// NotWritable reports whether the library is unwritable for this process — a
+// read-only mount or a permission denial — rather than an I/O fault. It is the
+// case the operator can fix, so callers answer it differently.
+func (e *FileRemoveError) NotWritable() bool {
+	return errors.Is(e.Err, fs.ErrPermission) || errors.Is(e.Err, syscall.EROFS)
+}
+
+// Reason is the underlying cause without the path os.Remove already wrapped
+// around it, for messages that name the directory themselves.
+func (e *FileRemoveError) Reason() string {
+	var pathErr *fs.PathError
+	if errors.As(e.Err, &pathErr) {
+		return pathErr.Err.Error()
+	}
+	return e.Err.Error()
+}
+
+// DeletedTrack reports what a delete tidied away beyond the track itself.
+type DeletedTrack struct {
+	// AlbumID is set when the track was its album's last, so the album went too.
+	AlbumID *pgtype.UUID
+	// ArtistID is set when that album was its artist's last, so the artist went too.
+	ArtistID *pgtype.UUID
+}
+
+// DeleteTrackFile removes a track's file from disk and then its row, tidying
+// away an album or artist the delete leaves empty. It is the ONLY path that
+// deletes a track file: the admin remove-track endpoint and quarantine's Delete
+// file both come through here (#3918).
 //
-// Steps:
+// Order is the whole contract. The file goes first, and if it cannot go — a
-//  1. Look up the track to get its file_path.
+// read-only mount, a permission denial, an I/O error — nothing else happens and
-//  2. Remove the file from disk. fs.ErrNotExist is OK — already gone.
+// a *FileRemoveError comes back. Proceeding past that failure is how #3918 lost
-//  3. Delete the tracks row.
+// history: tracks CASCADEs to play_events, general_likes, contextual_likes,
+// playlist_tracks, track_tags and playback_errors, so the row and everything
+// hanging off it were destroyed while the file survived, and the next scan
+// re-imported it as a brand-new track with none of it.
 //
-// Order matters: file first, then DB. If the file delete fails (permission,
+// A file that is already gone (fs.ErrNotExist) is not a failure; the row is
-// I/O error), we leave the DB row alone so the admin can retry.
+// removed as asked.
 //
-// The reverse failure mode — file gone, DB row still present — IS reconciled
+// This is NOT the missing-file path. That lifecycle is deliberately
-// now, and not by this function: the scan's reconcile pass stamps
+// non-destructive: reconcile stamps missing_since (#2523), selection paths
-// tracks.missing_since (#2523), every selection path filters on it, and a file
+// filter on it, and a returning file is un-marked or adopted (#2528). This is the
-// that returns is un-marked or adopted at its new path (#2528). That is the
+// explicit, irreversible "remove this recording", never the way to tidy up a row
-// normal life of a vanished file and it is deliberately non-destructive: the
+// whose file merely went away.
-// row, its play history and its likes survive, because a missing file is a
-// track Minstrel still knows about (#2527).
 //
-// So this function is NOT the missing-file path. It is the explicit admin
+// dataDir, when set, also clears the cached art of an artist the delete removed.
-// action "remove this recording from disk and from the library", and it is
+// logger may be nil.
-// irreversible: tracks CASCADEs to play_events, general_likes_tracks,
+func DeleteTrackFile(
-// contextual_likes, track_tags and playback_errors. Reach for it when the
+	ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, trackID pgtype.UUID,
-// operator means to destroy the record, never to tidy up a row whose file
+) (DeletedTrack, error) {
-// merely went away.
+	if logger == nil {
-func DeleteTrackFile(ctx context.Context, pool *pgxpool.Pool, trackID pgtype.UUID) error {
+		logger = slog.Default()
+	}
 	q := dbq.New(pool)
 	track, err := q.GetTrackByID(ctx, trackID)
 	if err != nil {
 		if errors.Is(err, pgx.ErrNoRows) {
-			return ErrTrackNotFound
+			return DeletedTrack{}, ErrTrackNotFound
 		}
-		return fmt.Errorf("get track: %w", err)
+		return DeletedTrack{}, fmt.Errorf("get track: %w", err)
 	}
 
-	if err := os.Remove(track.FilePath); err != nil && !errors.Is(err, fs.ErrNotExist) {
+	if err := removeFile(track.FilePath); err != nil && !errors.Is(err, fs.ErrNotExist) {
-		return fmt.Errorf("remove file: %w", err)
+		return DeletedTrack{}, &FileRemoveError{
+			Path: track.FilePath, UID: os.Getuid(), GID: os.Getgid(), Err: err,
+		}
 	}
 
-	if _, err := pool.Exec(ctx, "DELETE FROM tracks WHERE id = $1", trackID); err != nil {
+	// The row and any album or artist it empties go together, so a failure
-		return fmt.Errorf("delete row: %w", err)
+	// partway cannot leave a deleted track with a ghost album behind it.
+	tx, err := pool.Begin(ctx)
+	if err != nil {
+		return DeletedTrack{}, fmt.Errorf("begin tx: %w", err)
 	}
-	// Log the change after the delete succeeds. Best-effort: a Warn-level
+	defer func() { _ = tx.Rollback(ctx) }()
-	// failure here would leave the cache index orphaned on offline clients
+	tq := dbq.New(tx)
-	// until the next scan touches the surrounding album.
+
+	deleted, err := tq.DeleteTrack(ctx, trackID)
+	if err != nil {
+		if errors.Is(err, pgx.ErrNoRows) {
+			// Removed by someone else between the lookup and here.
+			return DeletedTrack{}, ErrTrackNotFound
+		}
+		return DeletedTrack{}, fmt.Errorf("delete track: %w", err)
+	}
+
+	var out DeletedTrack
+	album, err := tq.DeleteAlbumIfEmpty(ctx, deleted.AlbumID)
+	switch {
+	case err == nil:
+		albumID := album.ID
+		out.AlbumID = &albumID
+		artistID, aerr := tq.DeleteArtistIfEmpty(ctx, album.ArtistID)
+		switch {
+		case aerr == nil:
+			out.ArtistID = &artistID
+		case errors.Is(aerr, pgx.ErrNoRows):
+			// The artist still has other albums or stray tracks.
+		default:
+			return DeletedTrack{}, fmt.Errorf("delete artist if empty: %w", aerr)
+		}
+	case errors.Is(err, pgx.ErrNoRows):
+		// The album still has other tracks.
+	default:
+		return DeletedTrack{}, fmt.Errorf("delete album if empty: %w", err)
+	}
+
+	if err := tx.Commit(ctx); err != nil {
+		return DeletedTrack{}, fmt.Errorf("commit: %w", err)
+	}
+
+	// Both of these run after the delete has committed, so neither may fail
+	// it: the recording is gone either way. An unlogged change leaves the track
+	// in offline clients' caches until the next scan touches its album; a
+	// leftover art directory is only disk.
 	if err := syncpkg.LogChange(ctx, pool, syncpkg.EntityTrack,
 		syncpkg.FormatUUID(trackID), syncpkg.OpDelete); err != nil {
-		return fmt.Errorf("log change: %w", err)
+		logger.Warn("track delete: LogChange failed", "track_id", syncpkg.FormatUUID(trackID), "err", err)
 	}
-	return nil
+	if out.ArtistID != nil && dataDir != "" {
+		if err := coverart.CleanupArtistArt(dataDir, *out.ArtistID); err != nil {
+			logger.Warn("track delete: artist-art cleanup failed",
+				"artist_id", syncpkg.FormatUUID(*out.ArtistID), "err", err)
+		}
+	}
+	return out, nil
 }

internal/library/delete_test.go

@@ -4,9 +4,11 @@ import (
 	"context"
 	"errors"
 	"io"
+	"io/fs"
 	"log/slog"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
 
 	"github.com/jackc/pgx/v5/pgtype"
@@ -64,6 +66,15 @@ func seedTrack(t *testing.T, pool *pgxpool.Pool, filePath string) (dbq.Track, db
 	return track, album, artist
 }
 
+// stubRemoveFile makes file removal fail (or succeed) on demand for one test.
+// See removeFile for why this is a seam rather than a chmod.
+func stubRemoveFile(t *testing.T, fn func(string) error) {
+	t.Helper()
+	orig := removeFile
+	removeFile = fn
+	t.Cleanup(func() { removeFile = orig })
+}
+
 func TestDeleteTrackFile_HappyPath(t *testing.T) {
 	pool := newPool(t)
 	q := dbq.New(pool)
@@ -73,9 +84,18 @@ func TestDeleteTrackFile_HappyPath(t *testing.T) {
 	if err := os.WriteFile(path, []byte("payload"), 0o644); err != nil {
 		t.Fatalf("write file: %v", err)
 	}
-	track, album, _ := seedTrack(t, pool, path)
+	track, album, artist := seedTrack(t, pool, path)
+	// A sibling keeps the album non-empty, so this case pins that the tidy-up
+	// only removes an album the delete actually emptied.
+	if _, err := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{
+		Title: "Sibling", AlbumID: album.ID, ArtistID: artist.ID,
+		DurationMs: 1000, FilePath: filepath.Join(dir, "sibling.mp3"), FileSize: 100, FileFormat: "mp3",
+	}); err != nil {
+		t.Fatalf("sibling: %v", err)
+	}
 
-	if err := DeleteTrackFile(context.Background(), pool, track.ID); err != nil {
+	got, err := DeleteTrackFile(context.Background(), pool, nil, "", track.ID)
+	if err != nil {
 		t.Fatalf("DeleteTrackFile: %v", err)
 	}
 
@@ -85,9 +105,99 @@ func TestDeleteTrackFile_HappyPath(t *testing.T) {
 	if _, err := q.GetTrackByID(context.Background(), track.ID); err == nil {
 		t.Errorf("track row still exists")
 	}
-	// Album row preserved (other tracks may reference it).
 	if _, err := q.GetAlbumByID(context.Background(), album.ID); err != nil {
-		t.Errorf("album row vanished: %v", err)
+		t.Errorf("album with a remaining track vanished: %v", err)
+	}
+	if got.AlbumID != nil || got.ArtistID != nil {
+		t.Errorf("reported tidy-up %+v for an album that still has a track", got)
+	}
+}
+
+func TestDeleteTrackFile_EmptiedAlbumAndArtistGoToo(t *testing.T) {
+	pool := newPool(t)
+	q := dbq.New(pool)
+
+	path := filepath.Join(t.TempDir(), "lone.mp3")
+	if err := os.WriteFile(path, []byte("payload"), 0o644); err != nil {
+		t.Fatalf("write file: %v", err)
+	}
+	track, album, artist := seedTrack(t, pool, path)
+
+	got, err := DeleteTrackFile(context.Background(), pool, nil, "", track.ID)
+	if err != nil {
+		t.Fatalf("DeleteTrackFile: %v", err)
+	}
+	if got.AlbumID == nil || *got.AlbumID != album.ID {
+		t.Errorf("AlbumID = %v, want %v", got.AlbumID, album.ID)
+	}
+	if got.ArtistID == nil || *got.ArtistID != artist.ID {
+		t.Errorf("ArtistID = %v, want %v", got.ArtistID, artist.ID)
+	}
+	if _, err := q.GetAlbumByID(context.Background(), album.ID); err == nil {
+		t.Errorf("emptied album row still exists")
+	}
+	if _, err := q.GetArtistByID(context.Background(), artist.ID); err == nil {
+		t.Errorf("emptied artist row still exists")
+	}
+}
+
+// The #3918 proof. A file that cannot be removed must leave EVERYTHING in place:
+// the row is what carries likes, plays and playlist memberships, and the file
+// surviving means the next scan would re-import it as a stranger.
+func TestDeleteTrackFile_UnremovableFileDeletesNothing(t *testing.T) {
+	cases := []struct {
+		name        string
+		errno       syscall.Errno
+		notWritable bool
+	}{
+		{"read-only mount", syscall.EROFS, true},
+		{"permission denied", syscall.EACCES, true},
+		{"operation not permitted", syscall.EPERM, true},
+		{"i/o error", syscall.EIO, false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			pool := newPool(t)
+			q := dbq.New(pool)
+
+			dir := t.TempDir()
+			path := filepath.Join(dir, "track.mp3")
+			if err := os.WriteFile(path, []byte("payload"), 0o644); err != nil {
+				t.Fatalf("write file: %v", err)
+			}
+			track, album, _ := seedTrack(t, pool, path)
+			stubRemoveFile(t, func(name string) error {
+				return &fs.PathError{Op: "remove", Path: name, Err: tc.errno}
+			})
+
+			_, err := DeleteTrackFile(context.Background(), pool, nil, "", track.ID)
+
+			var fre *FileRemoveError
+			if !errors.As(err, &fre) {
+				t.Fatalf("err = %v, want a *FileRemoveError", err)
+			}
+			if fre.NotWritable() != tc.notWritable {
+				t.Errorf("NotWritable = %v, want %v", fre.NotWritable(), tc.notWritable)
+			}
+			if fre.Dir() != dir {
+				t.Errorf("Dir = %q, want the parent directory %q", fre.Dir(), dir)
+			}
+			if fre.Reason() != tc.errno.Error() {
+				t.Errorf("Reason = %q, want %q", fre.Reason(), tc.errno.Error())
+			}
+			if fre.UID != os.Getuid() || fre.GID != os.Getgid() {
+				t.Errorf("identity = %d:%d, want this process's %d:%d", fre.UID, fre.GID, os.Getuid(), os.Getgid())
+			}
+			if _, err := q.GetTrackByID(context.Background(), track.ID); err != nil {
+				t.Errorf("track row was deleted although its file was not: %v", err)
+			}
+			if _, err := q.GetAlbumByID(context.Background(), album.ID); err != nil {
+				t.Errorf("album row was deleted although the track's file was not: %v", err)
+			}
+			if _, err := os.Stat(path); err != nil {
+				t.Errorf("file gone although removal was refused: %v", err)
+			}
+		})
 	}
 }
 
@@ -97,7 +207,7 @@ func TestDeleteTrackFile_FileAlreadyGoneSucceeds(t *testing.T) {
 
 	track, _, _ := seedTrack(t, pool, "/no/such/file/anywhere.mp3")
 
-	if err := DeleteTrackFile(context.Background(), pool, track.ID); err != nil {
+	if _, err := DeleteTrackFile(context.Background(), pool, nil, "", track.ID); err != nil {
 		t.Fatalf("DeleteTrackFile with missing file: %v", err)
 	}
 	if _, err := q.GetTrackByID(context.Background(), track.ID); err == nil {
@@ -112,7 +222,7 @@ func TestDeleteTrackFile_NotFoundReturnsErr(t *testing.T) {
 	bogus.Bytes = [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
 	bogus.Valid = true
 
-	err := DeleteTrackFile(context.Background(), pool, bogus)
+	_, err := DeleteTrackFile(context.Background(), pool, nil, "", bogus)
 	if !errors.Is(err, ErrTrackNotFound) {
 		t.Errorf("err = %v, want ErrTrackNotFound", err)
 	}

internal/library/fingerprint.go

@@ -0,0 +1,296 @@
+package library
+
+import (
+	"bytes"
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"errors"
+	"fmt"
+	"os/exec"
+	"strconv"
+	"strings"
+	"time"
+
+	"github.com/jackc/pgx/v5/pgtype"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+)
+
+// Acoustic identity (M400).
+//
+// Two values per track, because they answer different questions:
+//
+//	audio_stream_sha256  a SHA-256 of the ENCODED audio packets. Equal means the
+//	                     same audio bytes, whatever the tags or container around
+//	                     them say. No threshold and no false positives — this is
+//	                     what catches two copies of one MP3 that differ only in
+//	                     their ID3 (#3885).
+//
+//	chromaprint          fpcalc's raw fingerprint. Close means the same
+//	                     recording, even at another bitrate or in another codec
+//	                     — the case an exact hash cannot see.
+//
+// Both shell out, in the shape probeDurationMs already set: a deadline on every
+// call, and a failure that leaves the value unset rather than failing the file.
+// A track with no fingerprint is never a duplicate candidate; it is still a
+// track.
+
+// fingerprintTimeout bounds one ffmpeg hash or fpcalc call. Longer than
+// probeTimeout because both read the audio rather than a header: the hash reads
+// every packet and fpcalc decodes up to its -length. 60s leaves room for a large
+// lossless file on a slow network mount; a call needing more is a stall, not a
+// big file.
+const fingerprintTimeout = 60 * time.Second
+
+// fingerprintWaitDelay bounds how long Output may keep waiting on the tool's
+// pipes after the deadline has killed it. Without it, a child that left a
+// descendant holding stdout open would block the scan past its own timeout.
+const fingerprintWaitDelay = 5 * time.Second
+
+// fingerprintVersion stamps how a track_fingerprints row was derived. Bump it
+// whenever the derivation changes — the hash arguments, fpcalc's flags or its
+// length — and the backfill re-derives every row below it. Fingerprints taken
+// by two methods are not comparable, and nothing else would reveal that the
+// library held a mix.
+const fingerprintVersion int16 = 1
+
+// errFingerprintTimeout marks a tool that ran out of time. Distinct from a
+// failed exit because a stall is a fact about the mount, not about the file.
+var errFingerprintTimeout = errors.New("fingerprint tool timed out")
+
+// defaultChromaprintLengthSec is how many seconds of audio fpcalc fingerprints.
+// 120 is fpcalc's own default. Fingerprints taken at different lengths are not
+// comparable, so changing this has to re-derive every stored one.
+const defaultChromaprintLengthSec = 120
+
+// fpcalcStderrTail caps how much of a failing tool's stderr reaches the log.
+const fpcalcStderrTail = 512
+
+// streamHashArgs hashes the encoded audio packets, never decoded samples.
+//
+// -c:a copy is the point, not an optimisation. A decoded hash of a lossy file
+// depends on the decoder's float maths and sample conversion, which can move
+// between ffmpeg releases — so an image upgrade could silently change every
+// stored hash, and yesterday's duplicate would stop matching today's copy.
+// Packet bytes do not move. It is also far cheaper: demux only, no decode.
+//
+// -map 0:a keeps embedded cover art (an attached-picture video stream) out of
+// the hash, so two copies of one recording carrying different art still match.
+func streamHashArgs(path string) []string {
+	return []string{
+		"-v", "error",
+		"-i", path,
+		"-map", "0:a",
+		"-c:a", "copy",
+		"-f", "hash", "-hash", "sha256",
+		"-",
+	}
+}
+
+// fpcalcArgs asks for the raw fingerprint as SIGNED integers.
+//
+// -raw because the matcher compares items bit by bit, which the compressed form
+// cannot do without being unpacked first. -signed because the column is Postgres
+// integer[], which is signed: fpcalc's default prints uint32, and half of those
+// values do not fit. Signed output is the same 32 bits with no reinterpretation
+// step left to get wrong.
+func fpcalcArgs(path string, lengthSec int) []string {
+	return []string{
+		"-raw", "-signed",
+		"-length", strconv.Itoa(lengthSec),
+		path,
+	}
+}
+
+// fingerprintResult is one attempt at both halves of a track's identity. They
+// fail independently: a file ffmpeg can demux may still defeat fpcalc.
+type fingerprintResult struct {
+	streamSHA256 []byte
+	chromaprint  []int32
+	hashErr      error
+	printErr     error
+}
+
+// computeFingerprint derives both halves for the file at path.
+func computeFingerprint(ctx context.Context, path string) fingerprintResult {
+	var r fingerprintResult
+	r.streamSHA256, r.hashErr = computeAudioStreamSHA256(ctx, path)
+	r.chromaprint, r.printErr = computeChromaprint(ctx, path, defaultChromaprintLengthSec)
+	return r
+}
+
+// inconclusive reports whether either half failed for a reason that says
+// nothing about the file. Such a result must never be stored: stamped at the
+// current version it would read as "tried, and this file cannot be
+// fingerprinted", and the backfill would never try it again.
+func (r fingerprintResult) inconclusive() bool {
+	return isInconclusive(r.hashErr) || isInconclusive(r.printErr)
+}
+
+// isInconclusive names the failures that are not a verdict on the file: a
+// stall, a cancelled scan, and a tool that is not installed. The last matters
+// outside the image — a dev binary run without fpcalc on PATH must not stamp
+// every track in the library as unfingerprintable.
+func isInconclusive(err error) bool {
+	return errors.Is(err, errFingerprintTimeout) ||
+		errors.Is(err, context.Canceled) ||
+		errors.Is(err, context.DeadlineExceeded) ||
+		errors.Is(err, exec.ErrNotFound)
+}
+
+// fingerprintFile runs the scanner's fingerprinter. A Scanner built without New
+// gets the real tools rather than a nil-func panic halfway through a scan.
+func (s *Scanner) fingerprintFile(ctx context.Context, path string) fingerprintResult {
+	if s.fingerprint == nil {
+		return computeFingerprint(ctx, path)
+	}
+	return s.fingerprint(ctx, path)
+}
+
+// storeFingerprint records one attempt for a track whose bytes are new or have
+// changed. It never fails the scan: a missing fingerprint only keeps a track
+// out of duplicate detection, which is not worth dropping the track over.
+func (s *Scanner) storeFingerprint(
+	ctx context.Context, q *dbq.Queries, trackID pgtype.UUID, path string, fp fingerprintResult,
+) {
+	if fp.hashErr != nil {
+		s.logger.Warn("library scan: audio stream hash failed", "path", path, "err", fp.hashErr)
+	}
+	if fp.printErr != nil {
+		s.logger.Warn("library scan: chromaprint failed", "path", path, "err", fp.printErr)
+	}
+	if fp.inconclusive() {
+		// Any row this track holds describes its PREVIOUS bytes. Drop it and
+		// leave the track to the backfill, rather than stamping a failure that
+		// says nothing about this file.
+		if err := q.DeleteTrackFingerprint(ctx, trackID); err != nil {
+			s.logger.Warn("library scan: clearing stale fingerprint failed", "path", path, "err", err)
+		}
+		return
+	}
+	// A NULL half here is a verdict — the tool ran and rejected this file — and
+	// is stamped at the current version so the backfill does not retry it on
+	// every boot. It is retried when the file changes.
+	if err := q.UpsertTrackFingerprint(ctx, dbq.UpsertTrackFingerprintParams{
+		TrackID:            trackID,
+		AudioStreamSha256:  fp.streamSHA256,
+		Chromaprint:        fp.chromaprint,
+		FingerprintVersion: fingerprintVersion,
+	}); err != nil {
+		s.logger.Warn("library scan: storing fingerprint failed", "path", path, "err", err)
+	}
+}
+
+// computeAudioStreamSHA256 returns the SHA-256 of the file's encoded audio.
+func computeAudioStreamSHA256(ctx context.Context, path string) ([]byte, error) {
+	out, err := runFingerprintTool(ctx, "ffmpeg", streamHashArgs(path))
+	if err != nil {
+		return nil, err
+	}
+	return parseStreamHash(out)
+}
+
+// computeChromaprint returns the raw acoustic fingerprint of the first
+// lengthSec seconds of the file.
+func computeChromaprint(ctx context.Context, path string, lengthSec int) ([]int32, error) {
+	out, err := runFingerprintTool(ctx, "fpcalc", fpcalcArgs(path, lengthSec))
+	if err != nil {
+		return nil, err
+	}
+	return parseFpcalcRaw(out)
+}
+
+// runFingerprintTool runs one tool under fingerprintTimeout.
+//
+// Any non-zero exit is an error, and that deliberately includes fpcalc's exit 3:
+// "reading failed, but here is a fingerprint of what I got". A partial
+// fingerprint of a damaged file is not that file's identity. Stored, it would
+// score against a healthy copy over whatever prefix survived, and could group
+// or fail to group either way. Absent is better than wrong.
+func runFingerprintTool(ctx context.Context, name string, args []string) ([]byte, error) {
+	runCtx, cancel := context.WithTimeout(ctx, fingerprintTimeout)
+	defer cancel()
+
+	cmd := exec.CommandContext(runCtx, name, args...)
+	cmd.WaitDelay = fingerprintWaitDelay
+	out, err := cmd.Output()
+	if err == nil {
+		return out, nil
+	}
+	// The caller gave up (a cancelled scan). Report that rather than the
+	// signal-killed exit it caused, so it is never mistaken for a verdict on
+	// the file.
+	if ctx.Err() != nil {
+		return nil, fmt.Errorf("%s: %w", name, ctx.Err())
+	}
+	// Named separately so a stall reads as a stall, not as a crash.
+	if errors.Is(runCtx.Err(), context.DeadlineExceeded) {
+		return nil, fmt.Errorf("%s: no result within %s: %w", name, fingerprintTimeout, errFingerprintTimeout)
+	}
+	var exitErr *exec.ExitError
+	if errors.As(err, &exitErr) {
+		return nil, fmt.Errorf("%s exited %d: %s", name, exitErr.ExitCode(), stderrTail(exitErr.Stderr))
+	}
+	return nil, fmt.Errorf("%s: %w", name, err)
+}
+
+// stderrTail keeps the END of a failing tool's stderr. ffmpeg and fpcalc print
+// the actual reason last, after any banner or per-frame warnings, so a cap that
+// kept the head would log the noise and drop the cause.
+func stderrTail(stderr []byte) []byte {
+	stderr = bytes.TrimSpace(stderr)
+	if len(stderr) > fpcalcStderrTail {
+		stderr = stderr[len(stderr)-fpcalcStderrTail:]
+	}
+	return stderr
+}
+
+// parseStreamHash reads the ffmpeg hash muxer's "SHA256=<hex>" line.
+func parseStreamHash(out []byte) ([]byte, error) {
+	for _, line := range strings.Split(string(out), "\n") {
+		hexed, ok := strings.CutPrefix(strings.TrimSpace(line), "SHA256=")
+		if !ok {
+			continue
+		}
+		sum, err := hex.DecodeString(hexed)
+		if err != nil {
+			return nil, fmt.Errorf("stream hash %q: %w", hexed, err)
+		}
+		if len(sum) != sha256.Size {
+			return nil, fmt.Errorf("stream hash is %d bytes, want %d", len(sum), sha256.Size)
+		}
+		return sum, nil
+	}
+	return nil, errors.New("ffmpeg printed no SHA256= line")
+}
+
+// parseFpcalcRaw reads fpcalc's text output:
+//
+//	DURATION=<seconds>
+//	FINGERPRINT=<int32>,<int32>,...
+func parseFpcalcRaw(out []byte) ([]int32, error) {
+	for _, line := range strings.Split(string(out), "\n") {
+		list, ok := strings.CutPrefix(strings.TrimSpace(line), "FINGERPRINT=")
+		if !ok {
+			continue
+		}
+		if list == "" {
+			return nil, errors.New("fpcalc returned an empty fingerprint")
+		}
+		items := strings.Split(list, ",")
+		fp := make([]int32, len(items))
+		for i, item := range items {
+			// ParseInt at 32 bits, not ParseUint: a value past int32 means the
+			// output was unsigned — -signed went missing from the invocation —
+			// and nothing downstream would reinterpret it. Refuse it here.
+			v, err := strconv.ParseInt(item, 10, 32)
+			if err != nil {
+				return nil, fmt.Errorf("fingerprint item %d %q: %w", i, item, err)
+			}
+			fp[i] = int32(v)
+		}
+		return fp, nil
+	}
+	return nil, errors.New("fpcalc printed no FINGERPRINT= line")
+}

internal/library/fingerprint_scan_test.go

@@ -0,0 +1,174 @@
+package library
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"slices"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5"
+	"github.com/jackc/pgx/v5/pgxpool"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db"
+)
+
+// TestScanner_FingerprintsOnlyNewOrChangedBytes_Integration pins WHEN the scan
+// fingerprints. The cost of getting it wrong is asymmetric and invisible: a
+// scan that re-fingerprints unchanged files still produces correct rows, just
+// by decoding the entire library on every tag-repair pass.
+//
+// The fingerprinter is stubbed. CI has no real audio, and the tools' output is
+// covered by the parser tests; this covers the scan's decisions.
+func TestScanner_FingerprintsOnlyNewOrChangedBytes_Integration(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping scanner integration in -short mode")
+	}
+	dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
+	if dsn == "" {
+		t.Skip("MINSTREL_TEST_DATABASE_URL not set")
+	}
+	ctx := context.Background()
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+
+	if err := db.Migrate(dsn, logger); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	pool, err := pgxpool.New(ctx, dsn)
+	if err != nil {
+		t.Fatalf("pool: %v", err)
+	}
+	t.Cleanup(pool.Close)
+	if _, err := pool.Exec(ctx, "TRUNCATE tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil {
+		t.Fatalf("truncate: %v", err)
+	}
+
+	root := t.TempDir()
+	a := filepath.Join(root, "artist/album/01.mp3")
+	b := filepath.Join(root, "artist/album/02.mp3")
+	writeTestMP3(t, a, map[string]string{"TIT2": "One", "TPE1": "Artist", "TALB": "Album", "TRCK": "1"})
+	writeTestMP3(t, b, map[string]string{"TIT2": "Two", "TPE1": "Artist", "TALB": "Album", "TRCK": "2"})
+
+	sum := bytes.Repeat([]byte{0xAB}, 32)
+	chroma := []int32{7, -7, 2147483647}
+	result := fingerprintResult{streamSHA256: sum, chromaprint: chroma}
+	calls := map[string]int{}
+
+	scanner := New(pool, logger, []string{root})
+	scanner.fingerprint = func(_ context.Context, path string) fingerprintResult {
+		calls[path]++
+		return result
+	}
+
+	scan := func(step string) Stats {
+		t.Helper()
+		st, err := scanner.Scan(ctx, nil)
+		if err != nil {
+			t.Fatalf("%s: scan: %v", step, err)
+		}
+		return st
+	}
+	type row struct {
+		sha     []byte
+		chroma  []int32
+		version int16
+	}
+	stored := func(path string) (row, bool) {
+		t.Helper()
+		var r row
+		err := pool.QueryRow(ctx, `
+			SELECT f.audio_stream_sha256, f.chromaprint, f.fingerprint_version
+			  FROM track_fingerprints f JOIN tracks t ON t.id = f.track_id
+			 WHERE t.file_path = $1`, path).Scan(&r.sha, &r.chroma, &r.version)
+		if errors.Is(err, pgx.ErrNoRows) {
+			return row{}, false
+		}
+		if err != nil {
+			t.Fatalf("read fingerprint for %s: %v", path, err)
+		}
+		return r, true
+	}
+	// A later step moves mtime forward past the row's updated_at, which is
+	// what the scan reads as "these bytes changed".
+	touch := func(path string, ahead time.Duration) {
+		t.Helper()
+		when := time.Now().Add(ahead)
+		if err := os.Chtimes(path, when, when); err != nil {
+			t.Fatalf("chtimes %s: %v", path, err)
+		}
+	}
+
+	// 1. New files are fingerprinted, and stored at the current version.
+	scan("first scan")
+	if calls[a] != 1 || calls[b] != 1 {
+		t.Fatalf("first scan fingerprint calls = %v, want one per file", calls)
+	}
+	got, ok := stored(a)
+	if !ok {
+		t.Fatal("first scan stored no fingerprint")
+	}
+	if !bytes.Equal(got.sha, sum) || !slices.Equal(got.chroma, chroma) || got.version != fingerprintVersion {
+		t.Fatalf("stored %+v, want sha %x chromaprint %v version %d", got, sum, chroma, fingerprintVersion)
+	}
+
+	// 2. A tag-repair pass re-reads every unchanged file and must not
+	// fingerprint any of them again.
+	//
+	// The Updated count is what makes this able to fail. Without it, a scan
+	// that simply SKIPPED both files would also leave the call counts at one,
+	// and the assertion would pass without the re-read path ever running.
+	if _, err := pool.Exec(ctx, "UPDATE tracks SET duration_ms = 1000, tag_read_version = 0"); err != nil {
+		t.Fatalf("force tag re-read: %v", err)
+	}
+	if st := scan("tag-repair scan"); st.Updated != 2 || st.Skipped != 0 {
+		t.Fatalf("tag-repair scan stats = %+v, want both files re-read (Updated=2 Skipped=0)", st)
+	}
+	if calls[a] != 1 || calls[b] != 1 {
+		t.Fatalf("tag-repair scan re-fingerprinted unchanged files: calls = %v", calls)
+	}
+	if _, ok := stored(a); !ok {
+		t.Fatal("tag-repair scan dropped a stored fingerprint")
+	}
+
+	// 3. Bytes that changed are fingerprinted again, and only those.
+	touch(a, time.Hour)
+	scan("changed-file scan")
+	if calls[a] != 2 || calls[b] != 1 {
+		t.Fatalf("changed-file scan calls = %v, want a=2 b=1", calls)
+	}
+
+	// 4. A changed file whose attempt is inconclusive loses its old row: that
+	// row describes the previous bytes, and a stall says nothing about the new
+	// ones.
+	result = fingerprintResult{streamSHA256: sum, printErr: fmt.Errorf("fpcalc: %w", errFingerprintTimeout)}
+	touch(a, 2*time.Hour)
+	scan("inconclusive scan")
+	if _, ok := stored(a); ok {
+		t.Fatal("inconclusive attempt left the previous bytes' fingerprint in place")
+	}
+	if _, ok := stored(b); !ok {
+		t.Fatal("inconclusive attempt on one file removed another file's fingerprint")
+	}
+
+	// 5. A file the tools reject gets a row at the current version with both
+	// halves NULL — a verdict, so the backfill does not retry it every boot.
+	result = fingerprintResult{
+		hashErr:  errors.New("ffmpeg exited 1"),
+		printErr: errors.New("fpcalc exited 2"),
+	}
+	touch(a, 3*time.Hour)
+	scan("rejected scan")
+	got, ok = stored(a)
+	if !ok {
+		t.Fatal("a file the tools rejected got no row, so the backfill would retry it forever")
+	}
+	if got.sha != nil || got.chroma != nil || got.version != fingerprintVersion {
+		t.Fatalf("rejected file stored %+v, want both halves NULL at version %d", got, fingerprintVersion)
+	}
+}

internal/library/fingerprint_test.go

@@ -0,0 +1,194 @@
+package library
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"os/exec"
+	"slices"
+	"strings"
+	"testing"
+)
+
+func TestParseFpcalcRaw(t *testing.T) {
+	cases := []struct {
+		name    string
+		out     string
+		want    []int32
+		wantErr string
+	}{
+		{
+			name: "signed output with negatives",
+			out:  "DURATION=213\nFINGERPRINT=-1453821711,17,0,2147483647,-2147483648\n",
+			want: []int32{-1453821711, 17, 0, 2147483647, -2147483648},
+		},
+		{
+			name: "fingerprint line need not come second",
+			out:  "FINGERPRINT=5,6\nDURATION=1\n",
+			want: []int32{5, 6},
+		},
+		{
+			// fpcalc's default is uint32. This value only appears when -signed
+			// is missing, and storing it would need a reinterpretation nothing
+			// performs.
+			name:    "unsigned output is refused",
+			out:     "DURATION=213\nFINGERPRINT=2841145585,17\n",
+			wantErr: "item 0",
+		},
+		{name: "empty fingerprint", out: "DURATION=0\nFINGERPRINT=\n", wantErr: "empty fingerprint"},
+		{name: "no fingerprint line", out: "DURATION=213\n", wantErr: "no FINGERPRINT= line"},
+		{name: "non-numeric item", out: "FINGERPRINT=1,x,3\n", wantErr: "item 1"},
+		{name: "trailing comma", out: "FINGERPRINT=1,2,\n", wantErr: "item 2"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := parseFpcalcRaw([]byte(tc.out))
+			if tc.wantErr != "" {
+				if err == nil || !strings.Contains(err.Error(), tc.wantErr) {
+					t.Fatalf("err = %v, want one containing %q", err, tc.wantErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected err: %v", err)
+			}
+			if !slices.Equal(got, tc.want) {
+				t.Fatalf("got %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestParseStreamHash(t *testing.T) {
+	// The real value ffmpeg printed for both files of the #3885 pair.
+	const www = "24e2daa3b4a534ff1a8d1a76f67810205869daf89f728d83a16625da4d28a18e"
+
+	got, err := parseStreamHash([]byte("SHA256=" + www + "\n"))
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if len(got) != 32 || got[0] != 0x24 || got[31] != 0x8e {
+		t.Fatalf("decoded %x, want %s", got, www)
+	}
+
+	for name, out := range map[string]string{
+		"no hash line":  "",
+		"other hash":    "MD5=" + www[:32] + "\n",
+		"not hex":       "SHA256=" + strings.Repeat("zz", 32) + "\n",
+		"short digest":  "SHA256=" + www[:62] + "\n",
+		"odd hex chars": "SHA256=" + www[:63] + "\n",
+	} {
+		if _, err := parseStreamHash([]byte(out)); err == nil {
+			t.Errorf("%s: parsed %q without error", name, out)
+		}
+	}
+}
+
+// followedBy reports whether flag appears in args immediately followed by value.
+func followedBy(args []string, flag, value string) bool {
+	for i := 0; i+1 < len(args); i++ {
+		if args[i] == flag && args[i+1] == value {
+			return true
+		}
+	}
+	return false
+}
+
+// The exact tier's stored hashes must stay comparable across ffmpeg upgrades,
+// which only holds while the packets are copied rather than decoded. A decoded
+// hash still matches within one ffmpeg build, so nothing else would notice the
+// change until an image upgrade silently broke every stored value.
+func TestStreamHashArgs_HashPacketsNotSamples(t *testing.T) {
+	args := streamHashArgs("/music/a.mp3")
+	for _, pair := range [][2]string{
+		{"-c:a", "copy"}, // no decode
+		{"-map", "0:a"},  // audio only: cover art stays out of the hash
+		{"-f", "hash"},   // the hash muxer, not a file
+		{"-hash", "sha256"},
+		{"-i", "/music/a.mp3"},
+	} {
+		if !followedBy(args, pair[0], pair[1]) {
+			t.Errorf("streamHashArgs lacks %s %s: %v", pair[0], pair[1], args)
+		}
+	}
+}
+
+func TestFpcalcArgs_RequestSignedRawOutput(t *testing.T) {
+	args := fpcalcArgs("/music/a.flac", 90)
+	for _, flag := range []string{"-raw", "-signed"} {
+		if !slices.Contains(args, flag) {
+			t.Errorf("fpcalcArgs lacks %s: %v", flag, args)
+		}
+	}
+	if !followedBy(args, "-length", "90") {
+		t.Errorf("fpcalcArgs does not pass the requested length: %v", args)
+	}
+	// fpcalc takes the file as its trailing positional argument.
+	if args[len(args)-1] != "/music/a.flac" {
+		t.Errorf("path is not last: %v", args)
+	}
+}
+
+func TestStderrTail_KeepsTheCauseNotTheBanner(t *testing.T) {
+	banner := bytes.Repeat([]byte("warning: skipping frame\n"), fpcalcStderrTail)
+	got := stderrTail(append(banner, []byte("ERROR: could not decode\n")...))
+	if len(got) != fpcalcStderrTail {
+		t.Fatalf("tail is %d bytes, want the %d-byte cap", len(got), fpcalcStderrTail)
+	}
+	if !bytes.HasSuffix(got, []byte("ERROR: could not decode")) {
+		t.Fatalf("tail dropped the final line: ...%q", got[len(got)-40:])
+	}
+
+	if got := stderrTail([]byte("  short  \n")); string(got) != "short" {
+		t.Fatalf("short stderr = %q, want it trimmed and whole", got)
+	}
+}
+
+// A stored failure is permanent until the file changes, so the classification
+// decides whether a track is ever retried. Every inconclusive case here would,
+// if misfiled as a verdict, silently exclude that track from duplicate
+// detection for good.
+func TestIsInconclusive(t *testing.T) {
+	notInstalled := fmt.Errorf("fpcalc: %w", &exec.Error{Name: "fpcalc", Err: exec.ErrNotFound})
+	cases := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{"timeout", fmt.Errorf("fpcalc: no result: %w", errFingerprintTimeout), true},
+		{"scan cancelled", fmt.Errorf("ffmpeg: %w", context.Canceled), true},
+		{"caller deadline", fmt.Errorf("ffmpeg: %w", context.DeadlineExceeded), true},
+		{"tool not installed", notInstalled, true},
+		{"tool rejected the file", errors.New("fpcalc exited 2: could not decode"), false},
+		{"unparseable output", errors.New("fpcalc printed no FINGERPRINT= line"), false},
+		{"success", nil, false},
+	}
+	for _, tc := range cases {
+		if got := isInconclusive(tc.err); got != tc.want {
+			t.Errorf("%s: isInconclusive = %v, want %v", tc.name, got, tc.want)
+		}
+	}
+}
+
+// Either half being inconclusive taints the whole result: storing the half that
+// succeeded would stamp the row at the current version with the other half
+// NULL, and that NULL would then read as a verdict.
+func TestFingerprintResult_InconclusiveIfEitherHalfIs(t *testing.T) {
+	stall := fmt.Errorf("fpcalc: %w", errFingerprintTimeout)
+	rejected := errors.New("fpcalc exited 2")
+	for name, tc := range map[string]struct {
+		r    fingerprintResult
+		want bool
+	}{
+		"both succeeded":            {fingerprintResult{streamSHA256: []byte{1}, chromaprint: []int32{1}}, false},
+		"hash ok, print stalled":    {fingerprintResult{streamSHA256: []byte{1}, printErr: stall}, true},
+		"hash stalled, print ok":    {fingerprintResult{hashErr: stall, chromaprint: []int32{1}}, true},
+		"hash ok, print rejected":   {fingerprintResult{streamSHA256: []byte{1}, printErr: rejected}, false},
+		"both rejected by the file": {fingerprintResult{hashErr: rejected, printErr: rejected}, false},
+	} {
+		if got := tc.r.inconclusive(); got != tc.want {
+			t.Errorf("%s: inconclusive = %v, want %v", name, got, tc.want)
+		}
+	}
+}

internal/library/scanner.go

@@ -75,10 +75,15 @@ type Scanner struct {
 	pool   *pgxpool.Pool
 	logger *slog.Logger
 	paths  []string
+	// fingerprint derives a file's acoustic identity (M400). A field so an
+	// integration test can substitute a deterministic one: CI has no real audio
+	// to fingerprint, and what the test pins is WHEN the scan fingerprints, not
+	// what the tools print. Call it through fingerprintFile.
+	fingerprint func(ctx context.Context, path string) fingerprintResult
 }
 
 func New(pool *pgxpool.Pool, logger *slog.Logger, paths []string) *Scanner {
-	return &Scanner{pool: pool, logger: logger, paths: paths}
+	return &Scanner{pool: pool, logger: logger, paths: paths, fingerprint: computeFingerprint}
 }
 
 // Scan walks every configured root and upserts any audio file whose mtime is
@@ -295,6 +300,25 @@ func (s *Scanner) scanFile(
 		durationMs = probed
 	}
 
+	// Fingerprint only bytes this row has not seen: a new path, or a file whose
+	// mtime moved past the row's. An unchanged file re-read for a tag repair
+	// keeps its stored fingerprint, for the same reason it keeps its duration
+	// above — a tagReadVersion bump must stay bound by tag reads, not become a
+	// decode of the whole library.
+	//
+	// An unchanged file with NO fingerprint yet is the backfill's job (#3908),
+	// deliberately not the scan's. Folding it into the skip check would make
+	// the first scan after an upgrade re-decode every track and push a sync
+	// change to every client for each one.
+	//
+	// Computed before move adoption so adoption can match on the audio hash
+	// (#3914); stored after the upsert, once the row id is known.
+	var fp fingerprintResult
+	fingerprinted := !unchanged
+	if fingerprinted {
+		fp = s.fingerprintFile(ctx, path)
+	}
+
 	// A path we've never seen might not be a new track — it might be one that
 	// moved or was renamed (#2528). Adopting re-points the existing row at this
 	// path and clears its missing mark, so the UpsertTrack below conflicts on
@@ -359,6 +383,9 @@ func (s *Scanner) scanFile(
 		// touches this track will re-emit the change.
 		s.logger.Warn("library scan: LogChange track upsert failed", "track_id", track.ID, "err", err)
 	}
+	if fingerprinted {
+		s.storeFingerprint(ctx, q, track.ID, path, fp)
+	}
 
 	if knownTrack {
 		stats.Updated++

internal/lidarrquarantine/service.go

@@ -35,6 +35,9 @@ var (
 // config changes in lidarrconfig take effect immediately. clientFn returns
 // nil when Lidarr is disabled.
 type Service struct {
+	// dataDir lets a Delete file that empties an artist clear that artist's
+	// cached art, the same as the admin remove-track path.
+	dataDir   string
 	pool      *pgxpool.Pool
 	lidarrCfg *lidarrconfig.Service
 	clientFn  func() *lidarr.Client
@@ -42,11 +45,11 @@ type Service struct {
 
 // NewService constructs a Service. Pass nil for clientFn to disable the
 // Lidarr-using methods (DeleteViaLidarr will return ErrLidarrDisabled).
-func NewService(pool *pgxpool.Pool, cfg *lidarrconfig.Service, clientFn func() *lidarr.Client) *Service {
+func NewService(pool *pgxpool.Pool, cfg *lidarrconfig.Service, clientFn func() *lidarr.Client, dataDir string) *Service {
 	if clientFn == nil {
 		clientFn = func() *lidarr.Client { return nil }
 	}
-	return &Service{pool: pool, lidarrCfg: cfg, clientFn: clientFn}
+	return &Service{pool: pool, lidarrCfg: cfg, clientFn: clientFn, dataDir: dataDir}
 }
 
 // Flag inserts or updates a quarantine row for the caller. Re-flagging
@@ -245,10 +248,13 @@ func (s *Service) snapshot(ctx context.Context, q *dbq.Queries, track dbq.Track)
 	}, nil
 }
 
-// DeleteFile removes the track file from disk and the tracks row, then
+// DeleteFile removes the track file from disk and the tracks row (tidying away
-// (via FK ON DELETE CASCADE) clears all per-user quarantine rows for that
+// an album or artist that leaves empty), then — via FK ON DELETE CASCADE —
-// track and writes an audit row. If the file deletion fails, the per-user
+// clears every per-user quarantine row for that track and writes an audit row.
-// rows stay so admin can retry. No partial state.
+//
+// If the file cannot be removed, nothing is deleted and the per-user rows stay
+// so the admin can retry: the error wraps a *library.FileRemoveError, which the
+// handler turns into an answer naming the cause (#3918). No partial state.
 func (s *Service) DeleteFile(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.LidarrQuarantineAction, error) {
 	q := dbq.New(s.pool)
 	track, err := q.GetTrackByID(ctx, trackID)
@@ -267,7 +273,7 @@ func (s *Service) DeleteFile(ctx context.Context, trackID, adminID pgtype.UUID)
 	if err != nil {
 		return dbq.LidarrQuarantineAction{}, fmt.Errorf("count: %w", err)
 	}
-	if err := library.DeleteTrackFile(ctx, s.pool, trackID); err != nil {
+	if _, err := library.DeleteTrackFile(ctx, s.pool, nil, s.dataDir, trackID); err != nil {
 		return dbq.LidarrQuarantineAction{}, fmt.Errorf("delete file: %w", err)
 	}
 	// tracks row is gone; ON DELETE CASCADE on lidarr_quarantine.track_id

internal/lidarrquarantine/service_test.go

@@ -87,7 +87,7 @@ func TestFlag_HappyPath(t *testing.T) {
 	user := seedUser(t, pool, "alice")
 	track, _, _ := seedTrack(t, pool, "Bad Track", "abc")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	row, err := svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "crackly")
 	if err != nil {
 		t.Fatalf("Flag: %v", err)
@@ -105,7 +105,7 @@ func TestFlag_UpsertOnSecondFlag(t *testing.T) {
 	user := seedUser(t, pool, "alice")
 	track, _, _ := seedTrack(t, pool, "T", "x")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	if _, err := svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "first"); err != nil {
 		t.Fatalf("first flag: %v", err)
 	}
@@ -129,7 +129,7 @@ func TestFlag_NonexistentTrackReturnsErrTrackNotFound(t *testing.T) {
 	bogus.Bytes = [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
 	bogus.Valid = true
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	_, err := svc.Flag(context.Background(), user.ID, bogus, "bad_rip", "")
 	if !errors.Is(err, ErrTrackNotFound) {
 		t.Errorf("err = %v, want ErrTrackNotFound", err)
@@ -141,7 +141,7 @@ func TestFlag_BadReasonRejected(t *testing.T) {
 	user := seedUser(t, pool, "alice")
 	track, _, _ := seedTrack(t, pool, "T", "x")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	_, err := svc.Flag(context.Background(), user.ID, track.ID, "garbage", "")
 	if !errors.Is(err, ErrBadReason) {
 		t.Errorf("err = %v, want ErrBadReason", err)
@@ -153,7 +153,7 @@ func TestUnflag_DeletesRow(t *testing.T) {
 	user := seedUser(t, pool, "alice")
 	track, _, _ := seedTrack(t, pool, "T", "x")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	if _, err := svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", ""); err != nil {
 		t.Fatalf("Flag: %v", err)
 	}
@@ -171,7 +171,7 @@ func TestListMine_OrderedNewestFirst(t *testing.T) {
 	t1, _, _ := seedTrack(t, pool, "T1", "x")
 	t2, _, _ := seedTrack(t, pool, "T2", "y")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	if _, err := svc.Flag(context.Background(), user.ID, t1.ID, "bad_rip", ""); err != nil {
 		t.Fatalf("Flag t1: %v", err)
 	}
@@ -199,7 +199,7 @@ func TestListAdminQueue_AggregatesByTrackWithReasonCounts(t *testing.T) {
 	carol := seedUser(t, pool, "carol")
 	track, _, _ := seedTrack(t, pool, "Hot Mess", "abc")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	if _, err := svc.Flag(context.Background(), alice.ID, track.ID, "bad_rip", ""); err != nil {
 		t.Fatalf("alice flag: %v", err)
 	}
@@ -235,7 +235,7 @@ func TestResolve_ClearsRowsAndWritesAudit(t *testing.T) {
 	bob := seedUser(t, pool, "bob")
 	track, _, _ := seedTrack(t, pool, "T", "x")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	if _, err := svc.Flag(context.Background(), alice.ID, track.ID, "bad_rip", ""); err != nil {
 		t.Fatalf("alice flag: %v", err)
 	}
@@ -267,7 +267,7 @@ func TestResolve_NoExistingRowsStillWritesAudit(t *testing.T) {
 	user := seedUser(t, pool, "alice")
 	track, _, _ := seedTrack(t, pool, "T", "x")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	// No flags applied — resolve a track with zero quarantine rows.
 	audit, err := svc.Resolve(context.Background(), track.ID, user.ID)
 	if err != nil {
@@ -291,7 +291,7 @@ func TestResolve_TrackNotFound(t *testing.T) {
 	bogus.Bytes = [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
 	bogus.Valid = true
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	_, err := svc.Resolve(context.Background(), bogus, user.ID)
 	if !errors.Is(err, ErrTrackNotFound) {
 		t.Errorf("err = %v, want ErrTrackNotFound", err)
@@ -315,7 +315,7 @@ func TestDeleteFile_RemovesFileAndAuditsAffected(t *testing.T) {
 		DurationMs: 1000, FilePath: path, FileSize: 1, FileFormat: "mp3",
 	})
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	if _, err := svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", ""); err != nil {
 		t.Fatalf("Flag: %v", err)
 	}
@@ -366,7 +366,7 @@ func TestDeleteViaLidarr_FullCascade(t *testing.T) {
 		t.Fatalf("save config: %v", err)
 	}
 	clientFn := func() *lidarr.Client { return lidarr.NewClient(stub.URL, "k") }
-	svc := NewService(pool, cfg, clientFn)
+	svc := NewService(pool, cfg, clientFn, "")
 
 	q := dbq.New(pool)
 	artist, _ := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{Name: "A", SortName: "A"})
@@ -424,7 +424,7 @@ func TestDeleteViaLidarr_LidarrDisabled(t *testing.T) {
 	user := seedUser(t, pool, "alice")
 	track, _, _ := seedTrack(t, pool, "T", "x")
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	_, _, err := svc.DeleteViaLidarr(context.Background(), track.ID, user.ID)
 	if !errors.Is(err, ErrLidarrDisabled) {
 		t.Errorf("err = %v, want ErrLidarrDisabled", err)
@@ -456,7 +456,7 @@ func TestDeleteViaLidarr_AlbumMBIDMissing(t *testing.T) {
 	cfg := lidarrconfig.New(pool)
 	_ = cfg.Save(context.Background(), lidarrconfig.Config{Enabled: true, BaseURL: stub.URL, APIKey: "k"})
 	clientFn := func() *lidarr.Client { return lidarr.NewClient(stub.URL, "k") }
-	svc := NewService(pool, cfg, clientFn)
+	svc := NewService(pool, cfg, clientFn, "")
 	_, _ = svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "")
 
 	_, _, err := svc.DeleteViaLidarr(context.Background(), track.ID, user.ID)
@@ -481,7 +481,7 @@ func TestDeleteViaLidarr_LidarrAlbumNotFound(t *testing.T) {
 	cfg := lidarrconfig.New(pool)
 	_ = cfg.Save(context.Background(), lidarrconfig.Config{Enabled: true, BaseURL: stub.URL, APIKey: "k"})
 	clientFn := func() *lidarr.Client { return lidarr.NewClient(stub.URL, "k") }
-	svc := NewService(pool, cfg, clientFn)
+	svc := NewService(pool, cfg, clientFn, "")
 
 	track, _, _ := seedTrack(t, pool, "T", "x")
 	_, _ = svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "")
@@ -499,7 +499,7 @@ func TestDeleteFile_TrackNotFound(t *testing.T) {
 	bogus.Bytes = [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
 	bogus.Valid = true
 
-	svc := NewService(pool, lidarrconfig.New(pool), nil)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, "")
 	_, err := svc.DeleteFile(context.Background(), bogus, user.ID)
 	if !errors.Is(err, ErrTrackNotFound) {
 		t.Errorf("err = %v, want ErrTrackNotFound", err)
@@ -521,7 +521,7 @@ func TestDeleteViaLidarr_TrackNotFound(t *testing.T) {
 		t.Fatalf("save config: %v", err)
 	}
 	clientFn := func() *lidarr.Client { return lidarr.NewClient(stub.URL, "k") }
-	svc := NewService(pool, cfg, clientFn)
+	svc := NewService(pool, cfg, clientFn, "")
 
 	var bogus pgtype.UUID
 	bogus.Bytes = [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}

internal/playlists/system.go

@@ -276,6 +276,17 @@ func SetTasteConfig(c taste.Config) {
 	systemTasteConfig = c
 }
 
+// dailyOrderSeed is the value the randomised candidate arms order by (#3889).
+//
+// Per (user, day) so a same-day rebuild draws the SAME set — which is what
+// TestBuildSystemPlaylists_DailyNonceDeterminism asserts and what those arms
+// only ever achieved by accident before, when their limits happened to exceed
+// the eligible rows. It changes on the day boundary, so the mixes still move
+// daily.
+func dailyOrderSeed(userID pgtype.UUID, dateStr string) string {
+	return uuidStringPL(userID) + ":" + dateStr
+}
+
 func currentSongsLikeWeights() recommendation.ScoringWeights {
 	systemTuningMu.RLock()
 	defer systemTuningMu.RUnlock()
@@ -629,6 +640,7 @@ func produceForYou(
 			zeroVec,
 			seeds,
 			systemForYouSourceLimits(),
+			dailyOrderSeed(userID, dateStr),
 		)
 		if cerr != nil {
 			logger.Warn("system playlist: for-you candidates load failed for seed; continuing",
@@ -716,6 +728,7 @@ func produceSeedMixes(
 			recommendation.ScaleForLibrary(
 				recommendation.SongsLikeCandidateSourceLimits(), librarySize,
 			),
+			dailyOrderSeed(userID, dateStr),
 		)
 		if cerr != nil {
 			logger.Warn("system playlist: seed candidates load failed; skipping",

internal/playlists/you_might_like.go

@@ -102,6 +102,7 @@ func buildYouMightLike(
 	cands, err := recommendation.LoadCandidatesFromSimilarity(
 		ctx, q, userID, seed, 1, zeroVec,
 		[]pgtype.UUID{seed}, ymlLimits,
+		dailyOrderSeed(userID, dateStr),
 	)
 	if err != nil {
 		logger.Warn("you-might-like: candidate load failed; skipping",

internal/recommendation/candidates.go

@@ -139,46 +139,41 @@ func DefaultCandidateSourceLimits() CandidateSourceLimits {
 // would produce a short mix or none at all, and "no playlist" is a worse
 // answer than "a few tracks further from the seed than we would like".
 //
-// DO NOT SHRINK AN ARM ORDERED BY UNSEEDED random(). This is the constraint
+// The seed-independent arms are trimmed hardest, because on this surface they
-// that shapes the numbers below, and it is not obvious from reading them.
+// are noise: `taste_overlap` (tracks by the user's top taste artists) and
+// `random_fill` (any track not already in the pool) both carry
+// `0.0::float8 AS sim_score`, so nearly a third of the default pool had no
+// relationship to the seed at all.
 //
-// `likes_overlap` and `random_fill` both end in a bare `ORDER BY random()`
+// THESE TRIMS WERE BLOCKED UNTIL #3889. `likes_overlap` and `random_fill`
-// (recommendation.sql:118, :161) with no daily seed. Such an arm returns a
+// used to end in a bare `ORDER BY random()`, which made their output a stable
-// STABLE set only while its LIMIT exceeds the rows eligible for it — at that
+// SET only while the limit exceeded the eligible rows — so SHRINKING them
-// point it returns all of them and the random order is irrelevant, because
+// changed pool membership between same-day rebuilds and broke daily
-// the caller sorts by id before scoring. Drop the limit below the eligible
+// determinism. Those arms now order by md5(id || seed), so a smaller limit
-// count and the arm starts returning a random SUBSET, which differs between
+// takes a smaller but REPRODUCIBLE slice, and the trim is safe.
-// two builds on the same day.
 //
-// That is a real defect (#3889) rather than a quirk of this function, and it
+// Reduced, never removed. Rule 131: the two seed-independent arms are the
-// bit here: cutting RandomFill to 10 broke
+// tier-3 floor, and zeroing them would leave a seed with thin ListenBrainz
-// TestBuildSystemPlaylists_DailyNonceDeterminism, whose library is smaller
+// coverage producing a short mix or none at all. The weights (SimilarityWeight
-// than the default limit and whose determinism was therefore accidental.
+// 4.0, everything seed-independent demoted) keep them ranked last, so they
-// Growing an arm is always safe; only shrinking one is.
+// surface only when the closer tiers cannot fill the mix.
 //
-// So the seed-independent arms are trimmed only where the ordering is
+// likes_overlap is cut hardest of the tier-2 arms for a specific reason: its
-// deterministic: `taste_overlap` sorts by `tpa.weight DESC, t.id` and can be
+// SQL assigns a FLAT 0.6 sim_score (recommendation.sql) rather than measuring
-// cut, `random_fill` cannot. The reduction is consequently modest — and it
+// anything. It is a collaborative signal wearing similarity's clothes, and a
-// matters less than it looks, because the WEIGHTS are what demote sim_score-0
+// raised SimilarityWeight amplifies it — if real ListenBrainz scores commonly
-// candidates now. The pool change biases the draw; the songs_like profile is
+// land below 0.6 it would outrank genuine matches. Halved pending the
-// what actually keeps unrelated tracks out of the result.
+// fill-rate measurement in #3879; the honest fix is to stop it claiming a
-//
+// similarity score it never computed.
-// One arm is left alone that arguably should not be: `likes_overlap` assigns
-// a FLAT 0.6 sim_score (recommendation.sql:108) rather than measuring
-// anything — a collaborative signal wearing similarity's clothes, which a
-// raised SimilarityWeight amplifies. If real ListenBrainz scores commonly
-// land below 0.6 it will outrank genuine matches. It cannot be trimmed here
-// without the determinism fix landing first; the honest repair is to stop it
-// claiming a similarity score it never computed (#3879).
 func SongsLikeCandidateSourceLimits() CandidateSourceLimits {
 	return CandidateSourceLimits{
 		LBSimilar:     60, // tier 1 — doubled; the only arm that measures the seed
 		SimilarArtist: 40, // tier 2 — raised; growing is always safe
 		TagOverlap:    20, // tier 2
 		UserCoplay:    20, // tier 2
-		LikesOverlap:  20, // tier 2 — NOT trimmed: unseeded random(), see above
+		LikesOverlap:  10, // tier 2, halved — flat 0.6 sim_score, see above
-		TasteOverlap:  10, // tier 3 floor — halved; deterministic ordering, safe
+		TasteOverlap:  10, // tier 3 floor — halved, not removed
-		RandomFill:    30, // tier 3 floor — NOT trimmed: unseeded random(), see above
+		RandomFill:    10, // tier 3 floor — cut hard, never to zero
 	}
 }
 
@@ -187,6 +182,10 @@ func SongsLikeCandidateSourceLimits() CandidateSourceLimits {
 // likes-overlap / random fill) + dedup-by-max sim_score. Returns
 // []Candidate (same shape as LoadCandidates) so Shuffle is unchanged.
 //
+// orderSeed decides whether the randomised arms repeat their draw — see
+// Column12 below and #3889. Pass a stable per-(user, day) value where the
+// selection must be reproducible, and a varying one where it should not be.
+//
 // Caller (radio handler) falls back to LoadCandidates on error.
 func LoadCandidatesFromSimilarity(
 	ctx context.Context,
@@ -196,6 +195,7 @@ func LoadCandidatesFromSimilarity(
 	currentVector SessionVector,
 	exclude []pgtype.UUID,
 	limits CandidateSourceLimits,
+	orderSeed string,
 ) ([]Candidate, error) {
 	if exclude == nil {
 		exclude = []pgtype.UUID{}
@@ -212,6 +212,12 @@ func LoadCandidatesFromSimilarity(
 		Limit_5: int32(limits.RandomFill),
 		Limit_6: int32(limits.TasteOverlap),
 		Limit_7: int32(limits.UserCoplay),
+		// #3889. Four arms used to end in a bare ORDER BY random(), which made
+		// their output a stable SET only while the limit exceeded the eligible
+		// rows. They now order by md5(id || this), so the caller decides
+		// whether the draw repeats: a per-(user, day) seed for the system
+		// mixes that promise daily determinism, a fresh one per radio request.
+		Column12: orderSeed,
 	})
 	if err != nil {
 		return nil, err

internal/recommendation/candidates_v2_test.go

@@ -2,6 +2,10 @@ package recommendation
 
 import (
 	"context"
+	"fmt"
+	"reflect"
+	"sort"
+	"strings"
 	"testing"
 
 	"github.com/jackc/pgx/v5/pgtype"
@@ -50,7 +54,7 @@ func TestLoadCandidatesFromSimilarity_LBSimilarSourceContributes(t *testing.T) {
 	target := f.tracks[1]
 	helperLBSimilarity(t, f, seed.ID, target.ID, 0.85)
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -82,7 +86,7 @@ func TestLoadCandidatesFromSimilarity_SimilarArtistTracksContribute(t *testing.T
 	})
 	helperArtistSimilarity(t, f, seed.ArtistID, otherArtist.ID, 0.8)
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -106,7 +110,7 @@ func TestLoadCandidatesFromSimilarity_TagOverlapContributes(t *testing.T) {
 	helperSetTrackGenre(t, f, seed.ID, "Rock; Pop")
 	helperSetTrackGenre(t, f, target.ID, "Rock")
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -133,7 +137,7 @@ func TestLoadCandidatesFromSimilarity_LikesOverlapContributes(t *testing.T) {
 		t.Fatalf("like: %v", err)
 	}
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -155,7 +159,7 @@ func TestLoadCandidatesFromSimilarity_RandomFillReturnsTracks(t *testing.T) {
 	f := newFixture(t, 10) // 10 tracks; no similarity data
 	seed := f.tracks[0]
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -176,7 +180,7 @@ func TestLoadCandidatesFromSimilarity_ExcludeListRespected(t *testing.T) {
 	excluded := f.tracks[1].ID
 	got, err := LoadCandidatesFromSimilarity(
 		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true},
-		[]pgtype.UUID{excluded}, defaultLimits(),
+		[]pgtype.UUID{excluded}, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -192,7 +196,7 @@ func TestLoadCandidatesFromSimilarity_SeedAlwaysExcluded(t *testing.T) {
 	f := newFixture(t, 5)
 	seed := f.tracks[0]
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -222,7 +226,7 @@ func TestLoadCandidatesFromSimilarity_RecentlyPlayedExcluded(t *testing.T) {
 		t.Fatalf("play_event: %v", err)
 	}
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -242,7 +246,7 @@ func TestLoadCandidatesFromSimilarity_DedupTakesMaxScore(t *testing.T) {
 	helperSetTrackGenre(t, f, target.ID, "Rock")      // jaccard 1/1 = 1.0 from tag-overlap
 	helperLBSimilarity(t, f, seed.ID, target.ID, 0.5) // weaker LB signal
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -295,7 +299,7 @@ func TestLoadCandidatesFromSimilarity_TasteOverlapArm(t *testing.T) {
 	// Only the taste_overlap arm is enabled.
 	limits := CandidateSourceLimits{TasteOverlap: 10}
 	got, err := LoadCandidatesFromSimilarity(
-		ctx, f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, limits,
+		ctx, f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, limits, "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -321,7 +325,7 @@ func TestLoadCandidatesFromSimilarity_EmptyLibrary_NoError(t *testing.T) {
 	f := newFixture(t, 1) // just the seed
 	seed := f.tracks[0]
 	got, err := LoadCandidatesFromSimilarity(
-		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -331,3 +335,97 @@ func TestLoadCandidatesFromSimilarity_EmptyLibrary_NoError(t *testing.T) {
 		t.Errorf("got %d candidates from seed-only library, want 0", len(got))
 	}
 }
+
+// The randomised arms must draw REPRODUCIBLY for a given seed (#3889).
+//
+// Four arms used to end in a bare `ORDER BY random()`. That returned a stable
+// set only while the arm's LIMIT exceeded the rows eligible for it — at that
+// point it returned all of them and the order stopped mattering, because the
+// caller sorts by track id before scoring. Below that threshold it returned a
+// random SUBSET, so two calls drew different candidates.
+//
+// It therefore held by ACCIDENT, and only for libraries smaller than the
+// limits. Any real library is larger, so same-day rebuilds had been drawing
+// different mixes since the arm was written — invisible, because a mix that
+// changes after a refresh looks like a feature.
+//
+// Limits deliberately smaller than the fixture, because that is the only
+// regime where the bug existed at all: with limits above the eligible count
+// the old code passes this too.
+func TestLoadCandidatesFromSimilarity_SameSeedDrawsTheSameSet(t *testing.T) {
+	f := newFixture(t, 12)
+	seed := f.tracks[0]
+
+	tight := CandidateSourceLimits{
+		LBSimilar: 2, SimilarArtist: 2, TagOverlap: 2,
+		LikesOverlap: 2, RandomFill: 3, TasteOverlap: 2, UserCoplay: 2,
+	}
+	ids := func(cs []Candidate) []string {
+		out := make([]string, 0, len(cs))
+		for _, c := range cs {
+			out = append(out, fmt.Sprintf("%x", c.Track.ID.Bytes))
+		}
+		sort.Strings(out) // membership, not order — order is settled downstream
+		return out
+	}
+
+	first, err := LoadCandidatesFromSimilarity(
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, tight, "day-one",
+	)
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	if len(first) == 0 {
+		t.Fatal("no candidates, so this test asserts nothing")
+	}
+
+	for i := 0; i < 3; i++ {
+		again, err := LoadCandidatesFromSimilarity(
+			context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, tight, "day-one",
+		)
+		if err != nil {
+			t.Fatalf("load %d: %v", i, err)
+		}
+		if !reflect.DeepEqual(ids(first), ids(again)) {
+			t.Fatalf("same seed drew a different set on call %d:\n  first %v\n  again %v",
+				i, ids(first), ids(again))
+		}
+	}
+}
+
+// ...and a different seed is free to draw differently, or the ordering would
+// be fixed rather than seeded and every day would serve the same mix.
+//
+// Asserted as "not pinned to one answer" rather than "always differs": with a
+// small fixture two seeds can legitimately collide, so requiring a difference
+// on any single pair would be flaky. Several seeds producing exactly one
+// distinct set is the real regression — that is what a constant ORDER BY
+// looks like.
+func TestLoadCandidatesFromSimilarity_DifferentSeedsCanDrawDifferently(t *testing.T) {
+	f := newFixture(t, 12)
+	seed := f.tracks[0]
+	tight := CandidateSourceLimits{
+		LBSimilar: 2, SimilarArtist: 2, TagOverlap: 2,
+		LikesOverlap: 2, RandomFill: 3, TasteOverlap: 2, UserCoplay: 2,
+	}
+
+	seen := map[string]bool{}
+	for _, orderSeed := range []string{"a", "b", "c", "d", "e", "f"} {
+		cs, err := LoadCandidatesFromSimilarity(
+			context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, tight, orderSeed,
+		)
+		if err != nil {
+			t.Fatalf("load %q: %v", orderSeed, err)
+		}
+		ids := make([]string, 0, len(cs))
+		for _, c := range cs {
+			ids = append(ids, fmt.Sprintf("%x", c.Track.ID.Bytes))
+		}
+		sort.Strings(ids)
+		seen[strings.Join(ids, ",")] = true
+	}
+	if len(seen) < 2 {
+		t.Errorf("six different seeds produced %d distinct set(s); the ordering is not "+
+			"varying with the seed at all", len(seen))
+	}
+}

internal/recommendation/songs_like_limits_test.go

@@ -59,41 +59,3 @@ func TestSongsLikeLimits_KeepThePoolRoughlyTheSameSize(t *testing.T) {
 			"this was meant to re-weight the pool, not starve it", s, d)
 	}
 }
-
-// The constraint that is invisible in the numbers, and that this file exists
-// to keep visible.
-//
-// `likes_overlap` and `random_fill` end in a bare `ORDER BY random()` with no
-// daily seed (recommendation.sql:118, :161). Such an arm returns a stable set
-// only while its LIMIT exceeds the eligible rows; below that it returns a
-// random SUBSET that differs between two builds on the same day, and the
-// daily-determinism promise quietly stops holding.
-//
-// This is not hypothetical — it is how this change first failed CI. Cutting
-// RandomFill to 10 broke TestBuildSystemPlaylists_DailyNonceDeterminism,
-// whose library is smaller than the default limit and whose determinism was
-// therefore an accident of the limit exceeding the library.
-//
-// Growing these arms is always safe. Only shrinking is, and the fix that
-// would make shrinking safe is a seeded ordering (#3889), not a smaller
-// number here.
-func TestSongsLikeLimits_DoNotShrinkTheUnseededRandomArms(t *testing.T) {
-	d := DefaultCandidateSourceLimits()
-	s := SongsLikeCandidateSourceLimits()
-
-	for _, tc := range []struct {
-		arm             string
-		songsLike, dflt int
-	}{
-		{"RandomFill", s.RandomFill, d.RandomFill},
-		{"LikesOverlap", s.LikesOverlap, d.LikesOverlap},
-	} {
-		if tc.songsLike < tc.dflt {
-			t.Errorf("%s cut from %d to %d. That arm is ordered by unseeded random(), "+
-				"so a smaller limit makes pool membership vary between same-day "+
-				"rebuilds — it breaks daily determinism rather than merely narrowing "+
-				"the mix. Fix the ordering (#3889) before trimming this.",
-				tc.arm, tc.dflt, tc.songsLike)
-		}
-	}
-}

internal/server/server.go

@@ -160,7 +160,7 @@ func (s *Server) Router() http.Handler {
 		if raErr != nil {
 			s.Logger.Warn("reacquisition settings unavailable; serving defaults", "err", raErr)
 		}
-		lidarrQuar := lidarrquarantine.NewService(s.Pool, lidarrCfg, lidarrClientFn)
+		lidarrQuar := lidarrquarantine.NewService(s.Pool, lidarrCfg, lidarrClientFn, s.DataDir)
 		tracksSvc := tracks.NewService(s.Pool, s.Logger, lidarrUnmonitorAdapter{fn: lidarrClientFn}, s.DataDir)
 		playlistsSvc := playlists.NewService(s.Pool, s.Logger, s.DataDir)
 		smtpSender := mailer.NewSMTPSender(s.Pool, s.Logger.With("component", "mailer"))

internal/tracks/service.go

@@ -1,9 +1,9 @@
-// Package tracks owns the track-level admin actions exposed by the
+// Package tracks owns the track-level admin actions behind DELETE
-// M7 #372 track-actions menu. Today that's RemoveTrack: the destructive
+// /api/admin/tracks/{id}. Today that's RemoveTrack: the destructive part goes
-// part is always handled directly by Minstrel (os.Remove + DB delete +
+// through library.DeleteTrackFile — the one path that deletes a track file —
-// cascade); when the operator opts in via `unmonitor=true` the service
+// and when the operator opts in via `unmonitor=true` the service also tells
-// also tells Lidarr to flip the track's monitored flag off so Lidarr
+// Lidarr to flip the track's monitored flag off so Lidarr doesn't search for a
-// doesn't search for a replacement.
+// replacement.
 //
 // History: an earlier shape (commit 50a231f, since rewritten) routed
 // Lidarr-managed tracks through lidarrquarantine.DeleteViaLidarr — but
@@ -12,6 +12,9 @@
 // drop sibling tracks the operator didn't ask to remove. The current
 // shape per spec revision 723eee9 is "always direct delete; opt-in
 // Lidarr unmonitor for replacement-suppression."
+//
+// The web track-kebab entry that called this was removed in f7278f24; the
+// endpoint was kept deliberately for a safer admin surface to rebind.
 package tracks
 
 import (
@@ -19,15 +22,14 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
-	"os"
 
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgtype"
 	"github.com/jackc/pgx/v5/pgxpool"
 
 	"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
-	"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/library"
 )
 
 // ErrNotFound is returned when the track id doesn't resolve. Aliased
@@ -69,10 +71,10 @@ func NewService(pool *pgxpool.Pool, logger *slog.Logger, lidarr LidarrUnmonitore
 	return &Service{pool: pool, logger: logger, lidarr: lidarr, dataDir: dataDir}
 }
 
-// RemoveTrack deletes the file from disk and the DB rows, runs the
+// RemoveTrack deletes the track's file and then its row, tidies away an album
-// album-empty / artist-empty cascade tidy-up, and (when unmonitor is
+// or artist the delete empties, and (when unmonitor is true and the track is
-// true and the track is Lidarr-managed) tells Lidarr to flip the track's
+// Lidarr-managed) tells Lidarr to flip the track's monitored flag off so it
-// monitored flag off so it won't search for a replacement.
+// won't search for a replacement.
 //
 // Returns:
 //   - deletedAlbumID: non-nil when removing the track left the album
@@ -81,9 +83,10 @@ func NewService(pool *pgxpool.Pool, logger *slog.Logger, lidarr LidarrUnmonitore
 //     empty (only set if deletedAlbumID is also set).
 //   - lidarrUnmonitorFailed: true when the operator requested unmonitor
 //     and the Lidarr call failed; the file + DB delete still succeeded.
-//   - err: only for failures *before* the destructive part completes.
+//   - err: ErrNotFound, or a failure before anything was deleted. When the
-//     A failed os.Remove is logged and tolerated. A failed Lidarr
+//     file cannot be removed it is a *library.FileRemoveError and NOTHING was
-//     unmonitor is reflected in the bool flag, not the error.
+//     deleted — see library.DeleteTrackFile for why that order is the contract
+//     (#3918). A failed Lidarr unmonitor is reflected in the bool, not here.
 //
 // adminID is currently unused — the cascade audit-log line that would
 // reference it isn't wired in this slice. It's threaded through the
@@ -104,10 +107,9 @@ func (s *Service) RemoveTrack(
 		return nil, nil, false, fmt.Errorf("get track: %w", err)
 	}
 
-	// Capture the album's mbid *before* the cascade-delete transaction.
+	// Capture the album's mbid *before* the delete. If removing this track
-	// If removing this track empties the album, DeleteAlbumIfEmpty
+	// empties the album, its row is gone afterwards and the unmonitor walk
-	// removes the row and a post-commit GetAlbumByID would return
+	// would have no album mbid to name.
-	// pgx.ErrNoRows — leaving the unmonitor walk with no album mbid.
 	var albumMbid string
 	if track.Mbid != nil && *track.Mbid != "" && unmonitor && s.lidarr != nil {
 		alb, alerr := q.GetAlbumByID(ctx, track.AlbumID)
@@ -118,64 +120,14 @@ func (s *Service) RemoveTrack(
 		// "no albumMbid → can't unmonitor → flag failure."
 	}
 
-	// Always: remove the file. Tolerate already-missing.
+	deleted, err := library.DeleteTrackFile(ctx, s.pool, s.logger, s.dataDir, trackID)
-	if track.FilePath != "" {
+	if err != nil {
-		if rerr := os.Remove(track.FilePath); rerr != nil && !errors.Is(rerr, os.ErrNotExist) {
+		if errors.Is(err, library.ErrTrackNotFound) {
-			s.logger.Warn("track delete: file remove failed",
+			return nil, nil, false, ErrNotFound
-				"path", track.FilePath, "track_id", trackID, "err", rerr)
-			// Proceed: DB consistency is the priority.
 		}
-	}
-
-	// DB cleanup in a transaction so a midway failure leaves things consistent.
-	tx, err := s.pool.Begin(ctx)
-	if err != nil {
-		return nil, nil, false, fmt.Errorf("begin tx: %w", err)
-	}
-	defer func() { _ = tx.Rollback(ctx) }()
-
-	tq := dbq.New(tx)
-
-	deleted, err := tq.DeleteTrack(ctx, trackID)
-	if err != nil {
 		return nil, nil, false, fmt.Errorf("delete track: %w", err)
 	}
 
-	albumRow, aerr := tq.DeleteAlbumIfEmpty(ctx, deleted.AlbumID)
-	switch {
-	case aerr == nil:
-		albumID := albumRow.ID
-		deletedAlbumID = &albumID
-		artistID, arerr := tq.DeleteArtistIfEmpty(ctx, albumRow.ArtistID)
-		switch {
-		case arerr == nil:
-			id := artistID
-			deletedArtistID = &id
-		case errors.Is(arerr, pgx.ErrNoRows):
-			// Artist still has other albums or stray tracks. OK.
-		default:
-			return nil, nil, false, fmt.Errorf("delete artist if empty: %w", arerr)
-		}
-	case errors.Is(aerr, pgx.ErrNoRows):
-		// Album still has other tracks. OK.
-	default:
-		return nil, nil, false, fmt.Errorf("delete album if empty: %w", aerr)
-	}
-
-	if err := tx.Commit(ctx); err != nil {
-		return nil, nil, false, fmt.Errorf("commit: %w", err)
-	}
-
-	// Cleanup artist-art filesystem cache if the delete cascade orphaned
-	// an artist. Non-fatal — the destructive part is done; we just want
-	// to keep the dataDir tidy.
-	if deletedArtistID != nil && s.dataDir != "" {
-		if err := coverart.CleanupArtistArt(s.dataDir, *deletedArtistID); err != nil {
-			s.logger.Warn("track delete: artist-art cleanup failed",
-				"artist_id", *deletedArtistID, "err", err)
-		}
-	}
-
 	// Lidarr unmonitor — non-fatal. The destructive part is done; any
 	// failure here is informational so the operator can retry manually.
 	if unmonitor && track.Mbid != nil && *track.Mbid != "" && s.lidarr != nil {
@@ -193,5 +145,5 @@ func (s *Service) RemoveTrack(
 		}
 	}
 
-	return deletedAlbumID, deletedArtistID, lidarrUnmonitorFailed, nil
+	return deleted.AlbumID, deleted.ArtistID, lidarrUnmonitorFailed, nil
 }

web/src/lib/api/errors.test.ts

@@ -49,3 +49,37 @@ describe('errMessage', () => {
     expect(result.length).toBeGreaterThan(0);
   });
 });
+
+describe('errMessage detail codes (#3918)', () => {
+  const detail =
+    'Minstrel runs as uid 1000, gid 1000 and cannot delete from /music/A (read-only file system). ' +
+    'The library mount must be writable by that user. Nothing was deleted.';
+
+  test('library_not_writable appends the server message to the copy', () => {
+    expect(errMessage({ code: 'library_not_writable', message: detail })).toBe(
+      `${ERROR_COPY.library_not_writable} ${detail}`
+    );
+  });
+
+  test('file_delete_failed appends the server message to the copy', () => {
+    const msg = 'Could not delete /music/A/01.mp3 (input/output error). Nothing was deleted.';
+    expect(errMessage({ code: 'file_delete_failed', message: msg })).toBe(
+      `${ERROR_COPY.file_delete_failed} ${msg}`
+    );
+  });
+
+  test('a detail code with no message shows the copy alone', () => {
+    expect(errMessage({ code: 'library_not_writable' })).toBe(ERROR_COPY.library_not_writable);
+    expect(errMessage({ code: 'library_not_writable', message: '  ' })).toBe(
+      ERROR_COPY.library_not_writable
+    );
+  });
+
+  // Server messages are usually internal detail. Appending them for every code
+  // would leak things like driver errors into toasts; this pins the scope.
+  test('other codes never carry the server message', () => {
+    expect(errMessage({ code: 'track_not_found', message: 'pgx: no rows in result set' })).toBe(
+      ERROR_COPY.track_not_found
+    );
+  });
+});

web/src/lib/api/errors.ts

@@ -8,11 +8,25 @@ export function errCode(err: unknown): string {
   return (err as { code?: string })?.code ?? 'unknown';
 }
 
+/**
+ * Codes whose server message carries specifics the operator needs in order to
+ * act — which directory, which uid — that fixed copy cannot say. For these the
+ * message follows the copy (#3918). Kept to a named list on purpose: most
+ * server messages are internal detail and must never reach a toast. Mirrored
+ * in Android's ErrorCopy.
+ */
+const DETAIL_CODES: ReadonlySet<string> = new Set(['library_not_writable', 'file_delete_failed']);
+
 /**
  * Returns user-facing copy for an unknown error value. Looks up the
  * error's code in the error-copy map; falls back to the supplied
  * fallback (default: "Something went wrong.") when the code is unknown.
+ * For a DETAIL_CODES code, the server's message is appended.
  */
 export function errMessage(err: unknown, fallback = 'Something went wrong.'): string {
-  return copyForCode(errCode(err)) ?? fallback;
+  const code = errCode(err);
+  const copy = copyForCode(code) ?? fallback;
+  if (!DETAIL_CODES.has(code)) return copy;
+  const detail = (err as { message?: unknown })?.message;
+  return typeof detail === 'string' && detail.trim() !== '' ? `${copy} ${detail}` : copy;
 }

web/src/lib/styles/error-copy.json

@@ -40,6 +40,8 @@
   "request_not_pending": "This request is no longer pending.",
   "request_not_found": "That request no longer exists.",
   "track_not_found": "That track no longer exists.",
+  "library_not_writable": "The music library isn't writable by the server.",
+  "file_delete_failed": "The file couldn't be deleted.",
   "album_not_found": "That album no longer exists.",
   "artist_not_found": "That artist no longer exists.",
   "playlist_not_found": "That playlist no longer exists.",