Recommendation relevance, the rollback unit, and a version that names what shipped (#131)

Dockerfile

@@ -33,14 +33,8 @@ 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 libchromaprint-tools \
+    && apt-get install -y --no-install-recommends ca-certificates ffmpeg \
     && rm -rf /var/lib/apt/lists/*
 
 RUN groupadd --system --gid 1000 minstrel \

internal/api/radio.go

@@ -101,10 +101,6 @@ 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

@@ -1,59 +0,0 @@
-// 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,14 +667,6 @@ 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, md5(t.id::text || $12::text)
+    ORDER BY asim.score DESC, random()
     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 md5(gl.track_id::text || $12::text)
+    ORDER BY random()
     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, md5(t.id::text || $12::text)
+    ORDER BY asim.score DESC, random()
     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 md5(t.id::text || $12::text)
+    ORDER BY random()
     LIMIT $9
 )
 SELECT
@@ -938,18 +938,17 @@ 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 {
@@ -972,22 +971,8 @@ 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,
@@ -1002,7 +987,6 @@ 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

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

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

@@ -1,38 +0,0 @@
--- 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

@@ -1,22 +0,0 @@
--- 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,22 +45,7 @@ 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
@@ -102,7 +87,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, md5(t.id::text || $12::text)
+    ORDER BY asim.score DESC, random()
     LIMIT $6
 ),
 tag_overlap AS (
@@ -130,7 +115,7 @@ likes_overlap AS (
           WHERE t.id = gl.track_id
             AND trim(g_overlap.g) IN (SELECT tag FROM seed_tags)
       )
-    ORDER BY md5(gl.track_id::text || $12::text)
+    ORDER BY random()
     LIMIT $8
 ),
 taste_overlap AS (
@@ -157,7 +142,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, md5(t.id::text || $12::text)
+    ORDER BY asim.score DESC, random()
     LIMIT $11
 ),
 random_fill AS (
@@ -173,7 +158,7 @@ random_fill AS (
           UNION SELECT track_id FROM taste_overlap
           UNION SELECT track_id FROM coplay_artists
       )
-    ORDER BY md5(t.id::text || $12::text)
+    ORDER BY random()
     LIMIT $9
 )
 SELECT

internal/dbtest/reset.go

@@ -87,7 +87,6 @@ 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/fingerprint.go

@@ -1,296 +0,0 @@
-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

@@ -1,174 +0,0 @@
-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

@@ -1,194 +0,0 @@
-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,15 +75,10 @@ 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, fingerprint: computeFingerprint}
+	return &Scanner{pool: pool, logger: logger, paths: paths}
 }
 
 // Scan walks every configured root and upserts any audio file whose mtime is
@@ -300,25 +295,6 @@ 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
@@ -383,9 +359,6 @@ 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/playlists/system.go

@@ -276,17 +276,6 @@ 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()
@@ -640,7 +629,6 @@ func produceForYou(
 			zeroVec,
 			seeds,
 			systemForYouSourceLimits(),
-			dailyOrderSeed(userID, dateStr),
 		)
 		if cerr != nil {
 			logger.Warn("system playlist: for-you candidates load failed for seed; continuing",
@@ -728,7 +716,6 @@ 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,7 +102,6 @@ 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,41 +139,46 @@ 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".
 //
-// The seed-independent arms are trimmed hardest, because on this surface they
+// DO NOT SHRINK AN ARM ORDERED BY UNSEEDED random(). This is the constraint
-// are noise: `taste_overlap` (tracks by the user's top taste artists) and
+// that shapes the numbers below, and it is not obvious from reading them.
-// `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.
 //
-// THESE TRIMS WERE BLOCKED UNTIL #3889. `likes_overlap` and `random_fill`
+// `likes_overlap` and `random_fill` both end in a bare `ORDER BY random()`
-// used to end in a bare `ORDER BY random()`, which made their output a stable
+// (recommendation.sql:118, :161) with no daily seed. Such an arm returns a
-// SET only while the limit exceeded the eligible rows — so SHRINKING them
+// STABLE set only while its LIMIT exceeds the rows eligible for it — at that
-// changed pool membership between same-day rebuilds and broke daily
+// point it returns all of them and the random order is irrelevant, because
-// determinism. Those arms now order by md5(id || seed), so a smaller limit
+// the caller sorts by id before scoring. Drop the limit below the eligible
-// takes a smaller but REPRODUCIBLE slice, and the trim is safe.
+// count and the arm starts returning a random SUBSET, which differs between
+// two builds on the same day.
 //
-// Reduced, never removed. Rule 131: the two seed-independent arms are the
+// That is a real defect (#3889) rather than a quirk of this function, and it
-// tier-3 floor, and zeroing them would leave a seed with thin ListenBrainz
+// bit here: cutting RandomFill to 10 broke
-// coverage producing a short mix or none at all. The weights (SimilarityWeight
+// TestBuildSystemPlaylists_DailyNonceDeterminism, whose library is smaller
-// 4.0, everything seed-independent demoted) keep them ranked last, so they
+// than the default limit and whose determinism was therefore accidental.
-// surface only when the closer tiers cannot fill the mix.
+// Growing an arm is always safe; only shrinking one is.
 //
-// likes_overlap is cut hardest of the tier-2 arms for a specific reason: its
+// So the seed-independent arms are trimmed only where the ordering is
-// SQL assigns a FLAT 0.6 sim_score (recommendation.sql) rather than measuring
+// deterministic: `taste_overlap` sorts by `tpa.weight DESC, t.id` and can be
-// anything. It is a collaborative signal wearing similarity's clothes, and a
+// cut, `random_fill` cannot. The reduction is consequently modest — and it
-// raised SimilarityWeight amplifies it — if real ListenBrainz scores commonly
+// matters less than it looks, because the WEIGHTS are what demote sim_score-0
-// land below 0.6 it would outrank genuine matches. Halved pending the
+// candidates now. The pool change biases the draw; the songs_like profile is
-// fill-rate measurement in #3879; the honest fix is to stop it claiming a
+// what actually keeps unrelated tracks out of the result.
-// 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:  10, // tier 2, halved — flat 0.6 sim_score, see above
+		LikesOverlap:  20, // tier 2 — NOT trimmed: unseeded random(), see above
-		TasteOverlap:  10, // tier 3 floor — halved, not removed
+		TasteOverlap:  10, // tier 3 floor — halved; deterministic ordering, safe
-		RandomFill:    10, // tier 3 floor — cut hard, never to zero
+		RandomFill:    30, // tier 3 floor — NOT trimmed: unseeded random(), see above
 	}
 }
 
@@ -182,10 +187,6 @@ 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,
@@ -195,7 +196,6 @@ func LoadCandidatesFromSimilarity(
 	currentVector SessionVector,
 	exclude []pgtype.UUID,
 	limits CandidateSourceLimits,
-	orderSeed string,
 ) ([]Candidate, error) {
 	if exclude == nil {
 		exclude = []pgtype.UUID{}
@@ -212,12 +212,6 @@ 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,10 +2,6 @@ package recommendation
 
 import (
 	"context"
-	"fmt"
-	"reflect"
-	"sort"
-	"strings"
 	"testing"
 
 	"github.com/jackc/pgx/v5/pgtype"
@@ -54,7 +50,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -86,7 +82,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -110,7 +106,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -137,7 +133,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -159,7 +155,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -180,7 +176,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(), "test-seed",
+		[]pgtype.UUID{excluded}, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -196,7 +192,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -226,7 +222,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -246,7 +242,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -299,7 +295,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, "test-seed",
+		ctx, f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, limits,
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -325,7 +321,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(), "test-seed",
+		context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
 	)
 	if err != nil {
 		t.Fatalf("load: %v", err)
@@ -335,97 +331,3 @@ 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,3 +59,41 @@ 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)
+		}
+	}
+}