Acoustic fingerprints at ingest, a delete that can't lose history, and a reproducible candidate draw #132

Merged

bvandeusen merged 5 commits from dev into main 2026-09-11 14:43:00 -04:00

Conversation 0 Commits 5 Files Changed 39

+1702 -259

bvandeusen commented 2026-09-11 14:42:51 -04:00

Owner

Copy Link

Copy Source

Five commits from dev. CI green on the head 71d43355: runs 6503 (signed APK + :dev image) and 6504 (Go short + integration). Web and Android lanes were green on d7a8e5f3, and nothing they build changed after it.

Fingerprinting (M400 #3905–#3907) — cba77a51

Every new or changed file now gets two identities, stored in a new track_fingerprints table (migration 0058):

• audio_stream_sha256: a SHA-256 of the encoded audio packets (ffmpeg -map 0:a -c:a copy -f hash). Equal means the same audio, whatever the tags say. Checked against the #3885 pair: the two WWW (2020) files hash identically; their whole-file hashes differ.
• chromaprint: fpcalc -raw -signed, for spotting the same recording at another bitrate or in another codec.

The image gains libchromaprint-tools (fpcalc).

Nothing reads the fingerprints yet. The duplicate matcher, sweep and report are later M400 steps. Existing unchanged tracks are fingerprinted by the backfill (#3908), not by this scan: folding them into the scan would re-decode the whole library on the first scan after upgrade.

Operational effect: importing new files costs one ffmpeg and one fpcalc invocation per file.

A track delete that cannot remove its file deletes nothing (#3918) — d7a8e5f3, 702b48ce

• One delete path. library.DeleteTrackFile: the file first, then the row plus any emptied album or artist in one transaction, then the sync change and artist-art cleanup. The admin remove-track endpoint and quarantine's Delete file both use it.
• Before this, RemoveTrack deleted the row even when the file wouldn't go. That cascaded away likes, plays and playlist memberships, and the surviving file came back on the next scan as a new track.
• An unwritable library (EROFS, EACCES, EPERM) now answers 409 library_not_writable, naming the directory, the uid:gid and that nothing was deleted. Other failures return 500 file_delete_failed with the path.
• Quarantine Delete file used to fail silently. Web and Android had no copy for its error, and Android swallowed it. Both clients now show the reason, and Android uses a snackbar.
• Behaviour change: quarantine Delete file now also removes an album or artist the delete empties.

README — 71d43355

The quickstart's music mount changes from :ro to writable, stating that Minstrel deletes a file only when an admin asks and never moves, renames or retags. :ro still works: deletes then refuse and explain why.

Reproducible candidate draw (#3889) — eff3d889

The recommendation arms that used ORDER BY random() now order by md5(id || seed). System mixes seed per user per day, so a same-day rebuild draws the same set; radio seeds per request.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH

Five commits from `dev`. CI green on the head `71d43355`: runs 6503 (signed APK + `:dev` image) and 6504 (Go short + integration). Web and Android lanes were green on `d7a8e5f3`, and nothing they build changed after it. ## Fingerprinting (M400 #3905–#3907) — `cba77a51` Every **new or changed** file now gets two identities, stored in a new `track_fingerprints` table (migration **0058**): - **`audio_stream_sha256`**: a SHA-256 of the encoded audio packets (`ffmpeg -map 0:a -c:a copy -f hash`). Equal means the same audio, whatever the tags say. Checked against the #3885 pair: the two `WWW (2020)` files hash identically; their whole-file hashes differ. - **`chromaprint`**: `fpcalc -raw -signed`, for spotting the same recording at another bitrate or in another codec. The image gains `libchromaprint-tools` (fpcalc). **Nothing reads the fingerprints yet.** The duplicate matcher, sweep and report are later M400 steps. Existing unchanged tracks are fingerprinted by the backfill (#3908), not by this scan: folding them into the scan would re-decode the whole library on the first scan after upgrade. **Operational effect:** importing new files costs one ffmpeg and one fpcalc invocation per file. ## A track delete that cannot remove its file deletes nothing (#3918) — `d7a8e5f3`, `702b48ce` - **One delete path.** `library.DeleteTrackFile`: the file first, then the row plus any emptied album or artist in one transaction, then the sync change and artist-art cleanup. The admin remove-track endpoint and quarantine's Delete file both use it. - **Before this**, `RemoveTrack` deleted the row even when the file wouldn't go. That cascaded away likes, plays and playlist memberships, and the surviving file came back on the next scan as a new track. - **An unwritable library** (EROFS, EACCES, EPERM) now answers **409 `library_not_writable`**, naming the directory, the uid:gid and that nothing was deleted. Other failures return 500 `file_delete_failed` with the path. - **Quarantine Delete file used to fail silently.** Web and Android had no copy for its error, and Android swallowed it. Both clients now show the reason, and Android uses a snackbar. - **Behaviour change:** quarantine Delete file now also removes an album or artist the delete empties. ## README — `71d43355` The quickstart's music mount changes from `:ro` to writable, stating that Minstrel deletes a file only when an admin asks and never moves, renames or retags. `:ro` still works: deletes then refuse and explain why. ## Reproducible candidate draw (#3889) — `eff3d889` The recommendation arms that used `ORDER BY random()` now order by `md5(id || seed)`. System mixes seed per user per day, so a same-day rebuild draws the same set; radio seeds per request. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH

bvandeusen added 5 commits 2026-09-11 14:42:51 -04:00

fix(recommendation): make the candidate draw reproducible, not accidentally so ... eff3d88931

Four arms of the candidate query ended in a bare `ORDER BY random()` with no
seed: similar_artists, likes_overlap, coplay_artists and random_fill.

Such an arm returns a STABLE set only while its LIMIT exceeds the rows
eligible for it — at that point it returns all of them and the order stops
mattering, because scoreAndSortCandidates sorts by track id before drawing
jitter. Below that threshold it returns a random SUBSET, and two builds on
the same day draw different ones.

So daily determinism held BY ACCIDENT, and only for libraries smaller than
the limits. Any real library is larger, which means same-day rebuilds have
been producing different mixes since those arms were written — invisible,
because a mix that changes after a refresh looks like a feature rather than
a broken promise.

Found by breaking it: cutting RandomFill to 10 while tuning Songs-like
turned TestBuildSystemPlaylists_DailyNonceDeterminism red. That test seeds
~20 tracks against a default RandomFill of 30, so its determinism came from
the limit exceeding the library, not from the code being right. It is now a
real guard.

The arms order by md5(id || $12) instead. The CALLER decides what that
means, which is the point: system mixes pass a per-(user, day) seed and get
the determinism they promise, radio passes a fresh value per request and
keeps varying, which is what a radio should do. Same shape the browse
queries in this file already use (`md5(id::text || current_date::text)`) —
existing idiom, not a new one.

This also unblocks the trim that #3881 wanted and could not have. Shrinking
a randomly-ordered arm was what broke membership; a seeded one takes a
smaller but REPRODUCIBLE slice. Songs-like's seed-independent share drops
from 29% to 12%, which was the original intent before determinism forced it
back to 20%.

TestSongsLikeLimits_DoNotShrinkTheUnseededRandomArms is DELETED rather than
kept passing. It existed to stop anyone trimming those arms while the
ordering was broken; the ordering is fixed, so the constraint is gone and a
guard enforcing it would now forbid correct code.

Was filed as blocked on tooling. It was not: `make generate-go` runs sqlc as
a pinned Go tool and is the same path CI takes.

One thing worth knowing for next time: three files in internal/db/dbq are
owned by root, left by `make generate` running sqlc in Docker. sqlc errored
on the first it could not write. They are untouched by this change and the
regeneration of recommendation.sql.go completed, but `make generate` will
keep failing until they are chowned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH

feat(library): fingerprint every new or changed file — M400 #3905-#3907 ... cba77a5187

Two identities per track, because they answer different questions:

- audio_stream_sha256: SHA-256 of the ENCODED audio packets
  (ffmpeg -map 0:a -c:a copy -f hash). Equal means identical audio
  whatever the tags say. Measured against the #3885 pair: the two WWW
  files hash identically here and differently as whole files. Packets
  rather than decoded samples, so an ffmpeg upgrade cannot silently
  change every stored hash, and nothing is decoded.
- chromaprint: fpcalc -raw -signed. The same recording at another
  bitrate or codec, for the acoustic tier.

fpcalc ships in the image (libchromaprint-tools); shelled out because
CGO_ENABLED=0 rules out bindings.

Stored in a track_fingerprints table rather than on tracks: eight
queries read tracks with SELECT *, including album pages, search and
the Subsonic surface, and a ~4 KB array there would be de-TOASTed on
every one of them.

The scan fingerprints only bytes it has not seen (a new path, or mtime
past the row's). A tag-repair pass leaves fingerprints alone, and
unchanged files with no fingerprint are the backfill's job (#3908).
Folding that into the skip check would re-decode the whole library on
the first scan after upgrade and push a sync change per track.

A failure that says nothing about the file (timeout, cancelled scan,
tool not installed) is never stored, and on changed bytes it removes
the old row. A tool that rejects the file stores NULL at the current
version, so the backfill does not retry it every boot.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH

fix(library): a track delete that cannot remove its file deletes nothing — #3918 ... d7a8e5f300

Two delete paths had opposite failure policies. tracks.RemoveTrack
logged a failed os.Remove and deleted the row anyway, which CASCADEs
likes, plays, playlist memberships and tags, while the file survived
for the next scan to re-import as a stranger. library.DeleteTrackFile
stopped correctly but reported it as a bare 500 nobody could read.

One path now: library.DeleteTrackFile removes the file first and, on
anything but ErrNotExist, returns *FileRemoveError with nothing
deleted. Only then does it delete the row and tidy an emptied album
and artist in one transaction, log the sync change and clear orphaned
artist art. RemoveTrack calls it, which also fixes RemoveTrack never
logging a sync change. Quarantine Delete file now tidies emptied
albums and artists too.

Both endpoints answer an unwritable library (EROFS, EACCES, EPERM) with
409 library_not_writable. The message names the directory (removal
writes to the parent), the uid:gid the server runs as, and that
nothing was deleted. Other remove errors are 500 file_delete_failed
with the path.

The reachable surface is quarantine Delete file, which failed
silently: no copy for the code on either client, and Android swallowed
the exception so the row just reappeared. Web and Android now have
copy for both codes and append the server message for exactly those
two. Android's quarantine screen shows it in a snackbar.

DELETE /api/admin/tracks/{id} has had no client since f7278f24, which
kept it on purpose for a safer admin surface, so its history loss was
latent. Fixed rather than removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH

fix(lidarrquarantine): pass dataDir at the four stub-client test constructors ... 702b48ce36

d7a8e5f3 added a dataDir parameter to NewService and updated the 13
call sites spelled NewService(pool, lidarrconfig.New(pool), nil). Four
more build their client from a Lidarr stub, NewService(pool, cfg,
clientFn), and were missed, so the package's tests did not compile and
run 6495 failed both go vet and the integration build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH

docs(readme): the music mount is writable — Minstrel deletes when asked ... 71d4335584

The quickstart mounted the library :ro and promised "Minstrel never
writes to your library". That stopped being true long before #3918:
quarantine's Delete file removes files, and under :ro it failed. The
operator has accepted delete ownership (Scribe note #3926).

The quickstart now mounts it writable and says exactly what Minstrel
writes: it deletes a file when an admin asks, and never moves, renames
or retags. It notes that uid 1000 needs write access, and that :ro
still works, with deletes refusing and explaining why.

Reorganising and tag writes stay out, pending whether Minstrel absorbs
Lidarr's role.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH

bvandeusen merged commit d2985f3841 into main 2026-09-11 14:43:00 -04:00

bvandeusen referenced this pull request 2026-09-11 15:34:11 -04:00

Backfill fingerprints for the existing library (M400 #3908) #133

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

deps

Dependency update (Renovate)

deps:major

Major dependency bump — held for manual review

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate-bot bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/minstrel#132