diff --git a/internal/library/acoustid_lookup.go b/internal/library/acoustid_lookup.go index dda1fa87..ce881a0f 100644 --- a/internal/library/acoustid_lookup.go +++ b/internal/library/acoustid_lookup.go @@ -18,17 +18,17 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// AcoustID lookup worker (M401 #3921). +// AcoustID lookup worker. // // A track whose tags carry no recording MBID is invisible to the ListenBrainz // similarity arm: it is never a seed and never comes back as a result. This // worker fingerprints such tracks and asks AcoustID which recording they are, -// writing an id only when the answer is unambiguous (D5). The file's own tag -// always outranks it (D4). +// writing an id only when the answer is unambiguous (chooseRecording). The +// file's own tag always outranks it. // -// It never gates anything else (rule 164). With no key, switched off or -// AcoustID unreachable, it idles and says why through Status; scans and -// playback do not notice. +// It never gates anything else. With no key, switched off or AcoustID +// unreachable, it idles and says why through Status; scans and playback do +// not notice. // acoustIDLookupTick is how often the worker looks for work. A settings save // starts a pass at once, so this only paces new tracks and retries after an @@ -187,7 +187,7 @@ func (w *AcoustIDLookupWorker) Run(ctx context.Context) { } // runOnce contains a pass so that nothing it does (an error, a panic) can stop -// the next one from starting (rule 157). +// the next one from starting. func (w *AcoustIDLookupWorker) runOnce(ctx context.Context) { w.setStatus(func(s *AcoustIDLookupStatus) { s.Running = true }) problem := "" @@ -216,8 +216,8 @@ func (w *AcoustIDLookupWorker) runOnce(ctx context.Context) { "no_match", res.NoMatch, "failed", res.Failed, "inconclusive", res.Inconclusive) } // A recording MBID AcoustID just gave a replacement may be the one its - // missing predecessor carried (M485): most of the operator's stale rows - // became pairable exactly this way. + // missing predecessor carried, which makes the pair foldable + // (PairMissingTracks). if res.Matched > 0 && ctx.Err() == nil { if _, err := PairMissingTracks(ctx, w.pool, w.logger); err != nil && ctx.Err() == nil { w.logger.Warn("acoustid lookup: missing-pair pass failed", "err", err) @@ -322,7 +322,8 @@ func (w *AcoustIDLookupWorker) lookupTrack( if isInconclusive(err) { return "", nil } - // fpcalc ran and rejected the file: a verdict, settled until it changes. + // fpcalc ran and rejected the file: a verdict, settled until it + // changes. return w.store(ctx, row, lookupDecision{state: lookupFailed, detail: err.Error()}), nil } cands, err := w.client.Lookup(ctx, cfg.APIKey, fp.fingerprint, fp.durationSec) @@ -351,7 +352,7 @@ func (w *AcoustIDLookupWorker) lookupTrack( var ErrIdentifyUnavailable = errors.New("acoustid: cannot be asked now") // IdentifyRecordings asks AcoustID which recordings a file may be, for the -// duplicate resolver (#6169): every candidate at or above the minimum score, +// duplicate resolver: every candidate at or above the minimum score, // best first. None is an answer: AcoustID does not know the audio. Nothing is // stored; the file's own lookup is the worker's. func (w *AcoustIDLookupWorker) IdentifyRecordings(ctx context.Context, path string) ([]string, error) { @@ -432,7 +433,8 @@ type lookupDecision struct { detail string } -// chooseRecording applies D5 to AcoustID's candidates (best score first): +// chooseRecording picks a recording from AcoustID's candidates (best score +// first): // only recordings at or above minScore count, one of them is a match, and // several are told apart by the track's title and length or not at all. A // wrong MBID would feed similarity the wrong neighbours, which is worse than diff --git a/internal/library/acoustid_lookup_test.go b/internal/library/acoustid_lookup_test.go index be51fd9d..aa0fdc5a 100644 --- a/internal/library/acoustid_lookup_test.go +++ b/internal/library/acoustid_lookup_test.go @@ -24,8 +24,8 @@ func cand(id, title string, sec int, score float64) acoustid.Candidate { return acoustid.Candidate{Recording: acoustid.Recording{ID: id, Title: title, DurationSec: sec}, Score: score} } -// D5, as a table over what decides it: how many candidates clear the -// threshold, and whether title and length leave exactly one of them. +// chooseRecording, as a table over what decides it: how many candidates clear +// the threshold, and whether title and length leave exactly one of them. func TestChooseRecording(t *testing.T) { const ( minScore = 0.85 @@ -154,7 +154,7 @@ func addTrack(t *testing.T, q *dbq.Queries, album dbq.Album, artist dbq.Artist, return tr } -// D4: the file's tag outranks a lookup, in both directions. Falsified by +// The file's tag outranks a lookup, in both directions. Falsified by // restoring `mbid = EXCLUDED.mbid` in UpsertTrack: the first re-read below // then erases the looked-up id. func TestUpsertTrack_KeepsALookedUpMbidUntilATagCarriesOne_Integration(t *testing.T) { @@ -302,7 +302,7 @@ func TestAcoustIDLookup_StopsWhenTheServiceCannotAnswer_Integration(t *testing.T } } -// The point of the milestone (#3921): once a lookup fills an MBID, the +// The point of the lookup: once it fills an MBID, the // similarity worker picks the track as a seed and can map ListenBrainz's // answers back to it. func TestAcoustIDLookup_FilledTrackReachesSimilarity_Integration(t *testing.T) { @@ -352,7 +352,7 @@ func TestAcoustIDLookup_FilledTrackReachesSimilarity_Integration(t *testing.T) { // And it is reachable as a target: an answer cached for another seed // before the lookup filled the MBID now resolves to it, with no new - // fetch (#5296). + // fetch. other, _, _ := seedTrack(t, pool, filepath.Join(dir, "other.mp3")) if _, err := pool.Exec(ctx, `INSERT INTO listenbrainz_similar_recordings (seed_track_id, recording_mbid, score) VALUES ($1, 'rec-played', 0.8)`, other.ID); err != nil { diff --git a/internal/library/acoustid_settings.go b/internal/library/acoustid_settings.go index 8ee310a6..f504219d 100644 --- a/internal/library/acoustid_settings.go +++ b/internal/library/acoustid_settings.go @@ -12,13 +12,13 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// AcoustID lookup settings (M401 #3922). Rule 25: a database row, changed -// without a restart, shared by the worker and the admin API. +// AcoustID lookup settings: a database row, changed without a restart, shared +// by the worker and the admin API. // AcoustIDSettings mirrors the acoustid_settings row. type AcoustIDSettings struct { Enabled bool - // APIKey is the operator's registered application key. Never sent back + // APIKey is the registered AcoustID application key. Never sent back // to the browser; the API reports only whether one is set. APIKey string // MinScore is the AcoustID score a recording needs to be considered. diff --git a/internal/library/album_loudness.go b/internal/library/album_loudness.go index 7d029e0c..17e67fae 100644 --- a/internal/library/album_loudness.go +++ b/internal/library/album_loudness.go @@ -12,7 +12,7 @@ import ( syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) -// Album loudness (M464 #4996). +// Album loudness. // // Album-mode normalization plays a whole album at one gain, so the quiet // interlude stays quieter than the single it sits between. That gain comes @@ -168,7 +168,7 @@ func (w *LoudnessBackfillWorker) storeAlbumLoudness( return fmt.Errorf("read inputs: %w", err) } a := computeAlbumLoudness(inputs) - // Logged before the write, for the reason storeLoudness gives (#2704). A + // Logged before the write, for the reason storeLoudness gives. A // failed log stores nothing, so the digest still differs and the next // pass tries again. if albumVisibleChange(before, a) { diff --git a/internal/library/album_loudness_test.go b/internal/library/album_loudness_test.go index a943e024..93f2cf98 100644 --- a/internal/library/album_loudness_test.go +++ b/internal/library/album_loudness_test.go @@ -185,7 +185,7 @@ func TestAlbumLoudness_Integration(t *testing.T) { t.Fatalf("waiting album = lufs %v, %d/%d settled; want nil, 1/2", lufs, settled, total) } - // Still no album value: clients have nothing new to read (#4997). + // Still no album value: clients have nothing new to read. if n := albumChanges(); n != 0 { t.Fatalf("a waiting album logged %d sync changes, want 0", n) } diff --git a/internal/library/delete.go b/internal/library/delete.go index a12aec3c..8e65edf2 100644 --- a/internal/library/delete.go +++ b/internal/library/delete.go @@ -30,12 +30,12 @@ var ErrTrackNotFound = errors.New("library: track not found") 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. +// 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. + // permission problem an admin cannot see from the host side. UID, GID int Err error } @@ -44,15 +44,15 @@ func (e *FileRemoveError) Error() string { return fmt.Sprintf("remove track file 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. +// 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 an admin 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. +// case an admin 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) } @@ -69,36 +69,38 @@ func (e *FileRemoveError) Reason() string { // 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 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 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). +// file both come through here. // // Order is the whole contract. The file goes first, and if it cannot go — a // read-only mount, a permission denial, an I/O error — nothing else happens and -// a *FileRemoveError comes back. Proceeding past that failure is how #3918 lost +// a *FileRemoveError comes back. Proceeding past that failure would lose // 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. +// hanging off it would be destroyed while the file survived, and the next scan +// would re-import it as a brand-new track with none of it. // // A file that is already gone (fs.ErrNotExist) is not a failure; the row is // removed as asked. // // This is NOT the missing-file path. That lifecycle is deliberately -// non-destructive: reconcile stamps missing_since (#2523), selection paths -// filter on it, and a returning file is un-marked or adopted (#2528). This is the -// explicit, irreversible "remove this recording", never the way to tidy up a row -// whose file merely went away. +// non-destructive: reconcile stamps missing_since, selection paths filter on +// it, and a returning file is un-marked or adopted. This is the explicit, +// irreversible "remove this recording", never the way to tidy up a row whose +// file merely went away. // -// dataDir, when set, also clears the cached art of an artist the delete removed. -// logger may be nil. +// dataDir, when set, also clears the cached art of an artist the delete +// removed. logger may be nil. func DeleteTrackFile( ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, trackID pgtype.UUID, ) (DeletedTrack, error) { @@ -163,9 +165,9 @@ func DeleteTrackFile( } // removeTrackFileOnDisk is the one rule for removing a track's file, shared by -// DeleteTrackFile and the duplicate merge. A file already gone is fine; anything -// else comes back as a *FileRemoveError, and the caller must then change nothing -// in the database (#3918). +// DeleteTrackFile and the duplicate merge. A file already gone is fine; +// anything else comes back as a *FileRemoveError, and the caller must then +// change nothing in the database. func removeTrackFileOnDisk(path string) error { if err := removeFile(path); err != nil && !errors.Is(err, fs.ErrNotExist) { return &FileRemoveError{Path: path, UID: os.Getuid(), GID: os.Getgid(), Err: err} diff --git a/internal/library/delete_test.go b/internal/library/delete_test.go index 76cf9ae2..f51af5c3 100644 --- a/internal/library/delete_test.go +++ b/internal/library/delete_test.go @@ -141,7 +141,7 @@ func TestDeleteTrackFile_EmptiedAlbumAndArtistGoToo(t *testing.T) { } } -// The #3918 proof. A file that cannot be removed must leave EVERYTHING in place: +// 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) { diff --git a/internal/library/duplicate_classify.go b/internal/library/duplicate_classify.go index 88acad97..b4b19817 100644 --- a/internal/library/duplicate_classify.go +++ b/internal/library/duplicate_classify.go @@ -8,7 +8,7 @@ import ( "unicode" ) -// Duplicate classification (M498 #5435; settled further 2026-10-19). +// Duplicate classification. // // A duplicate group is one recording found more than once. What should happen // to it depends on where the copies sit: @@ -19,15 +19,15 @@ import ( // album it came from, a deluxe edition and the standard one, or one // recording a later release retitled ("The Ragtime Cat", "F.O.D. (live)", // a romanized title): each copy belongs to its own release, so none is -// deleted (unless it is a video rip; the resolver decides). The measured -// library (2026-10-19) had 451 such groups waiting on the operator under -// different titles, and they were retitles, not different songs. +// deleted (unless it is a video rip; the resolver decides). Copies under +// different titles on different releases are retitles, not different +// songs. // - distinct: one album, two versions of a song that sound alike, named as // such ("Lovesick", "Lovesick (Instrumental)"). Not duplicates. // - mismatch: one album holds the same audio under different song titles: // one file filling several track slots, a broken download. Which slot is // right is the resolver's to decide from MusicBrainz's track lengths. -// - review: what none of these fits. The operator decides. +// - review: what none of these fits. An admin decides. // DuplicateClass is a group's shape. The values are duplicate_groups.class. type DuplicateClass string @@ -54,8 +54,8 @@ type ClassifyMember struct { // adds to a title (featuring credits, remaster notes, video markers) and keeps // what names a different recording (live, demo, remix, instrumental). On one // album, titles that differ only by such a word are two versions (distinct) -// unless the bytes are identical, which is one file twice: the #3885 case of -// "WWW" and "WWW (instrumental)" holding the same file is a wrong slot. +// unless the bytes are identical, which is one file twice: "WWW" and "WWW +// (instrumental)" holding the same file is a wrong slot. func ClassifyDuplicateGroup(tier string, members []ClassifyMember) DuplicateClass { if len(members) < 2 { return ClassReview @@ -136,7 +136,8 @@ func songKey(title, artist string) string { var ( // A bracketed credit: "(feat. Panda Bear)", "[ft. X]", "(featuring Y)". featBracket = regexp.MustCompile(`(?i)[(\[][^)\]]*\b(feat\.?|ft\.?|featuring)\s[^)\]]*[)\]]`) - // A credit trailing the title without brackets: "Doin' It Right ft. Panda Bear". + // A credit trailing the title without brackets: "Doin' It Right ft. Panda + // Bear". featTrailing = regexp.MustCompile(`(?i)\s(feat\.?|ft\.?|featuring)\s.*$`) // "(2011 Remaster)", "[Remastered]", "- Remastered 2009". remasterBracket = regexp.MustCompile(`(?i)[(\[][^)\]]*remaster[^)\]]*[)\]]`) @@ -149,10 +150,11 @@ var ( // whether two copies carry the same one. // // Dropped: a leading "Artist - " (a video upload's title), bracketed featuring -// credits and remaster notes, and bracketed segments carrying a video-rip marker -// ("(Official Video)", "[HD]"). Kept: every other bracketed word, so "(live)", -// "(demo)", "(acoustic)", "(remix)" and "(instrumental)" still tell recordings -// apart. Then case, "&" against "and", and punctuation and spacing are ignored. +// credits and remaster notes, and bracketed segments carrying a video-rip +// marker ("(Official Video)", "[HD]"). Kept: every other bracketed word, so +// "(live)", "(demo)", "(acoustic)", "(remix)" and "(instrumental)" still tell +// recordings apart. Then case, "&" against "and", and punctuation and spacing +// are ignored. func MatchTitleKey(title, artist string) string { t := strings.TrimSpace(title) if a := strings.TrimSpace(artist); a != "" { diff --git a/internal/library/duplicate_classify_test.go b/internal/library/duplicate_classify_test.go index b19102f2..9aea59c3 100644 --- a/internal/library/duplicate_classify_test.go +++ b/internal/library/duplicate_classify_test.go @@ -7,7 +7,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// The shapes measured on the operator's library (milestone 498's body). +// The group shapes the classifier tells apart. func TestClassifyDuplicateGroup(t *testing.T) { m := func(album, title string) ClassifyMember { return ClassifyMember{AlbumID: album, Title: title, ArtistName: "Gorillaz"} @@ -63,7 +63,8 @@ func TestMatchTitleKey(t *testing.T) { {"Song (live)", "", "songlive"}, {"Song (demo)", "", "songdemo"}, {"Song (Remix)", "", "songremix"}, - // A title that is nothing but a marker keeps its words rather than vanishing. + // A title that is nothing but a marker keeps its words rather than + // vanishing. {"(Official Video)", "", "officialvideo"}, } for _, tc := range cases { @@ -73,8 +74,8 @@ func TestMatchTitleKey(t *testing.T) { } } -// Copies the fold of 2026-10-20 merged although their file names said they -// were different versions; each pair must disagree. And copies of one version +// Copies whose file names say they are different versions must disagree, so +// the fold never merges them. And copies of one version // named differently must agree. func TestVersionWords(t *testing.T) { differ := [][2][2]string{ @@ -149,7 +150,7 @@ func TestWrongSlots(t *testing.T) { } // The lengths cannot place a recording filed as several songs (Savant's -// Mortals, cut to each slot's length); what AcoustID calls it can (#6169). +// Mortals, cut to each slot's length); what AcoustID calls it can. func TestNamedCopies(t *testing.T) { track := func(title string, ms int32, rec string) dbq.MbReleaseTrack { return dbq.MbReleaseTrack{Title: title, LengthMs: ms, RecordingMbid: rec} diff --git a/internal/library/duplicate_match.go b/internal/library/duplicate_match.go index d45b83f4..c3dd29fb 100644 --- a/internal/library/duplicate_match.go +++ b/internal/library/duplicate_match.go @@ -6,16 +6,16 @@ import ( "sort" ) -// Duplicate matching (M400 #3909). +// Duplicate matching. // -// Pure functions over fingerprints: no database, no files. This is the part that -// decides whether two tracks in the operator's library are proposed as one -// recording, so every rule in it has to be falsifiable in a unit test. +// Pure functions over fingerprints: no database, no files. This is the part +// that decides whether two tracks in the library are proposed as one recording, +// so every rule in it has to be falsifiable in a unit test. // // Two tiers, answering different questions: // // exact equal audio_stream_sha256 — the same encoded audio bytes. No score, -// no threshold, no false positives (the #3885 pair). +// no threshold, no false positives. // acoustic chromaprint fingerprints that agree closely once aligned — the // same recording at another bitrate or in another codec. // @@ -52,23 +52,22 @@ const minDistinctFraction = 0.3 // fingerprints may show and still be proposed as one recording. Unrelated audio // sits near 0.5; the same recording re-encoded lands well under 0.1. // -// Deliberately conservative. The operator's stated worry is the opposite of a -// missed duplicate: "the same song can appear in different albums, usually it's -// a different recording", and an instrumental shares its vocal version's -// harmony, which chroma features capture. A false merge is the failure that -// matters, and the report is reviewed anyway. This is an unmeasured default: -// calibrate it against real pairs once the backfill (#3908) has populated the -// library, then expose it in Settings (#3913). +// Deliberately conservative. A false merge is the failure that matters, worse +// than a missed duplicate: the same song on different albums is usually a +// different recording, and an instrumental shares its vocal version's harmony, +// which chroma features capture. An admin can change it in Settings +// (FingerprintSettings). const defaultAcousticMaxBitErrorRate = 0.15 -// durationToleranceMs is how far apart two tracks' durations may be and still be -// compared. Encoders pad and trim a little; different edits differ by more. +// durationToleranceMs is how far apart two tracks' durations may be and still +// be compared. Encoders pad and trim a little; different edits differ by more. const durationToleranceMs = 3000 -// maxAcousticGroupSize caps an acoustic group. A cluster bigger than this is far -// more likely a shared jingle, a skit or a low-information pattern than eight -// copies of one recording, and proposing it would bury the real duplicates. -// Exact-tier groups are not capped: identical bytes are identical however many. +// maxAcousticGroupSize caps an acoustic group. A cluster bigger than this is +// far more likely a shared jingle, a skit or a low-information pattern than +// eight copies of one recording, and proposing it would bury the real +// duplicates. Exact-tier groups are not capped: identical bytes are identical +// however many. const maxAcousticGroupSize = 8 // acousticScore is the result of comparing two fingerprints. @@ -84,8 +83,8 @@ type acousticScore struct { // preparedPrint is a fingerprint with the parts every comparison needs worked // out once. The sweep compares each track with every other track within a few -// seconds of its duration, so rebuilding the alignment index for each pair would -// dominate its cost. +// seconds of its duration, so rebuilding the alignment index for each pair +// would dominate its cost. type preparedPrint struct { items []int32 index map[uint32][]int @@ -208,13 +207,13 @@ type duplicateGroup struct { type groupingResult struct { Groups []duplicateGroup // OversizeClusters counts acoustic clusters discarded for exceeding - // maxAcousticGroupSize. Reported rather than silent: a sudden rise means the - // cap or the information floor needs attention. + // maxAcousticGroupSize. Reported rather than silent: a sudden rise means + // the cap or the information floor needs attention. OversizeClusters int } -// groupUnit is one thing the acoustic pass compares: a single track, or an exact -// group standing in for all its byte-identical copies. +// groupUnit is one thing the acoustic pass compares: a single track, or an +// exact group standing in for all its byte-identical copies. type groupUnit struct { ids []string // every member, sorted durationMs int32 @@ -224,11 +223,11 @@ type groupUnit struct { assigned bool } -// streamGrouper is the acoustic pass over units arriving in (durationMs, sortKey) -// order. It holds only the units within durationToleranceMs of the oldest one -// not yet settled, so memory is bounded by the densest few seconds of the -// library rather than by its size — the whole library's fingerprints would be -// hundreds of megabytes. +// streamGrouper is the acoustic pass over units arriving in (durationMs, +// sortKey) order. It holds only the units within durationToleranceMs of the +// oldest one not yet settled, so memory is bounded by the densest few seconds +// of the library rather than by its size — the whole library's fingerprints +// would be hundreds of megabytes. // // A seed can be settled as soon as a unit arrives beyond its window: everything // it could group with has already arrived, and no later seed can reach back to @@ -237,9 +236,9 @@ type groupUnit struct { // // Grouping is COMPLETE-LINKAGE: a unit joins a group only if it matches every // unit already in it, within the duration tolerance and the bit-error limit. -// Single-linkage would let a chain of near-misses — A close to B, B close to C — -// drag A and C, which are not close, into one proposed merge. Complete linkage -// also means any member can be chosen as the survivor (#3911). +// Single-linkage would let a chain of near-misses — A close to B, B close to C +// — drag A and C, which are not close, into one proposed merge. Complete +// linkage also means any member can be chosen as the survivor. // // When an acoustic group absorbs an exact group, the result is tier acoustic: a // group is only as certain as its weakest link. @@ -320,7 +319,8 @@ func (g *streamGrouper) settleOldest() { for _, member := range group { member.assigned = true // The acoustic evidence is untrustworthy; identical bytes are not. - // An exact group caught inside an oversize cluster is still proposed. + // An exact group caught inside an oversize cluster is still + // proposed. if member.exact { g.res.Groups = append(g.res.Groups, duplicateGroup{Tier: tierExact, Members: member.ids}) } diff --git a/internal/library/duplicate_merge.go b/internal/library/duplicate_merge.go index 2ba26079..54f4cd40 100644 --- a/internal/library/duplicate_merge.go +++ b/internal/library/duplicate_merge.go @@ -16,13 +16,14 @@ import ( syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) -// Duplicate merge (M400 #3911). +// Duplicate merge. // ErrDuplicateGroupNotPending means the group was already merged or dismissed, // no longer exists, or no longer has two members to merge. var ErrDuplicateGroupNotPending = errors.New("library: duplicate group is not pending") -// ErrSurvivorNotInGroup means the copy chosen to keep is not a member of the group. +// ErrSurvivorNotInGroup means the copy chosen to keep is not a member of the +// group. var ErrSurvivorNotInGroup = errors.New("library: survivor is not a member of the group") // MergedCopy is one copy a merge kept or removed. @@ -37,7 +38,7 @@ type MergeResult struct { Survivor MergedCopy Removed []MergedCopy - // What moved onto the survivor — reported so the operator, and the audit + // What moved onto the survivor — reported so an admin, and the audit // log, can see that the history was kept rather than take it on trust. PlayEvents int64 SkipEvents int64 @@ -60,16 +61,16 @@ type MergeResult struct { // survivor first, and only then deletes the now-empty row. // // It deletes the removed copies' FILES too, and first, before any row changes -// (#3918, note #3926). A merge that left the file behind would be undone by the -// next scan, which re-imports it as a new track with no history. If a file cannot -// be removed, the *FileRemoveError comes back and nothing in the database changes. -// With several copies to remove, one file may already be gone when a later one -// fails; that copy's row keeps all its history and is marked missing by the next -// scan, and retrying the merge picks up where it stopped. +// (DeleteTrackFile's order). A merge that left the file behind would be undone +// by the next scan, which re-imports it as a new track with no history. If a +// file cannot be removed, the *FileRemoveError comes back and nothing in the +// database changes. With several copies to remove, one file may already be gone +// when a later one fails; that copy's row keeps all its history and is marked +// missing by the next scan, and retrying the merge picks up where it stopped. // -// Everything else happens in one transaction, which holds a lock on the group so -// two merges of it cannot run at once. Sync changes for clients' caches are logged -// inside it, the way the playlists service logs its own. +// Everything else happens in one transaction, which holds a lock on the group +// so two merges of it cannot run at once. Sync changes for clients' caches are +// logged inside it, the way the playlists service logs its own. func MergeDuplicateGroup( ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, groupID, survivorID pgtype.UUID, @@ -152,7 +153,8 @@ func MergeDuplicateGroup( return MergeResult{}, fmt.Errorf("commit merge: %w", err) } - // After commit, like DeleteTrackFile: a leftover art directory is only disk. + // After commit, like DeleteTrackFile: a leftover art directory is only + // disk. if dataDir != "" { for _, artistID := range res.DeletedArtistIDs { if err := coverart.CleanupArtistArt(dataDir, artistID); err != nil { @@ -176,11 +178,11 @@ type foldResult struct { } // foldTrackInto moves everything the loser row carries onto the survivor, then -// deletes the loser row and tidies an album or artist it leaves empty. It is the -// row half of a merge, shared by the duplicate merge and the missing-pair pass -// (M485): neither touches a file here, and both run it inside their own -// transaction. The sync changes it owes clients go into changes, which the -// caller logs once before committing. +// deletes the loser row and tidies an album or artist it leaves empty. It is +// the row half of a merge, shared by the duplicate merge and the missing-pair +// pass (PairMissingTracks): neither touches a file here, and both run it inside +// their own transaction. The sync changes it owes clients go into changes, +// which the caller logs once before committing. func foldTrackInto( ctx context.Context, tq *dbq.Queries, survivorID, loserID pgtype.UUID, changes *mergeChanges, ) (foldResult, error) { diff --git a/internal/library/duplicate_merge_test.go b/internal/library/duplicate_merge_test.go index 3a6781fe..3d9def22 100644 --- a/internal/library/duplicate_merge_test.go +++ b/internal/library/duplicate_merge_test.go @@ -83,7 +83,8 @@ func newMergeFixture(t *testing.T) mergeFixture { } f.alice, f.bob = user("merge-alice"), user("merge-bob") - // Alice liked both copies, the removed one first; Bob liked only the removed one. + // Alice liked both copies, the removed one first; Bob liked only the + // removed one. f.aliceEarlierLikeOnRem = time.Now().Add(-72 * time.Hour).UTC().Truncate(time.Microsecond) mustExec(`INSERT INTO general_likes (user_id, track_id, liked_at) VALUES ($1, $2, $3), ($1, $4, now()), ($5, $2, now())`, f.alice.ID, f.remove.ID, f.aliceEarlierLikeOnRem, f.keep.ID, f.bob.ID) @@ -144,9 +145,8 @@ func (f mergeFixture) count(t *testing.T, sql string, args ...any) int { return n } -// The #3911 proof: after a merge, every piece of history the removed copy held -// is on the copy kept, nothing is doubled, and the removed copy — row and file — -// is gone. +// After a merge, every piece of history the removed copy held is on the copy +// kept, nothing is doubled, and the removed copy — row and file — is gone. func TestMergeDuplicateGroup_Integration(t *testing.T) { f := newMergeFixture(t) ctx := context.Background() @@ -208,8 +208,8 @@ func TestMergeDuplicateGroup_Integration(t *testing.T) { t.Errorf("the kept copy's own house weight was overwritten") } - // Similarity: rewritten onto the kept copy, no duplicate edge, no self-edge, - // nothing left pointing at the removed copy. + // Similarity: rewritten onto the kept copy, no duplicate edge, no + // self-edge, nothing left pointing at the removed copy. if n := f.count(t, `SELECT count(*) FROM track_similarity WHERE track_a_id = $1 AND track_b_id = $2 AND source = 'listenbrainz'`, f.keep.ID, f.other.ID); n != 1 { t.Errorf("listenbrainz edge keep→other = %d rows, want exactly 1", n) diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go index 2fc39e05..a41c0480 100644 --- a/internal/library/duplicate_resolve.go +++ b/internal/library/duplicate_resolve.go @@ -17,7 +17,7 @@ import ( syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) -// Duplicate resolver (M498 #5435, #5437; M502 #5517). +// Duplicate resolver. // // The sweep proposes duplicate groups; the resolver decides what each one is // and settles the ones that are safe to settle without asking. Minstrel is the @@ -29,8 +29,7 @@ import ( // - Across releases, each copy belongs to its own release (a single and its // album): nothing is removed, and the copies are linked as one song. An // album holding the song twice among them still holds it once: its extra -// copies are merged into its best (Humanz held Saturnz Barz twice beside -// the single, #5401). +// copies are merged into its best. // - Except that a video rip is no release's copy. When every copy but one is // a rip, across releases or under another title, the rips are merged into // the one that is not. @@ -40,7 +39,7 @@ import ( // lengths say which slot it belongs to, or AcoustID which song it is when // each copy was cut to its own slot's length, and the copies in the other // slots are removed so the album is fetched again (duplicate_settle.go). -// - Anything else waits for the operator. +// - Anything else waits for an admin. const ( // resolveMergeCap bounds the merges one pass makes. The first pass after @@ -50,7 +49,7 @@ const ( // duplicateResolveTick is how often the worker runs a pass. duplicateResolveTick = time.Hour // scanWaitRetry is how soon a pass put off for a running scan is tried - // again: a boot scan takes under a minute on the operator's library. + // again; a boot scan is short. scanWaitRetry = time.Minute ) @@ -83,7 +82,7 @@ type resolveGroup struct { } // isRip says whether a copy is a video rip: held back as one, or named like -// one and not kept by the operator. The name is read as well as the verdict +// one and not kept by an admin. The name is read as well as the verdict // because a pass reads the groups before it flags new rips. func isRip(m dbq.ListDuplicateGroupsForResolveRow) bool { if m.SourceVerdict != nil { @@ -141,7 +140,7 @@ type ResolveSources struct { Identify RecordingIdentifier } -// ResolveDuplicates runs one pass. act is the operator's auto-resolve setting; +// ResolveDuplicates runs one pass. act is the auto-resolve setting; // with it off, the pass only classifies. It decides from what the library and // catalogue hold; ResolveDuplicatesWith may also ask MusicBrainz and AcoustID. func ResolveDuplicates( @@ -238,14 +237,13 @@ func ResolveDuplicatesWith( } // foldAlbumCopies merges, in each cross-release group, the extra copies one -// album holds of the song into the best of that album's copies: the one that -// is not a video rip when only one is not, else ProposeSurvivor's choice. -// Copies whose title or file name name different versions (versionWords) are -// not folded: "Suite-Pee" and "Suite-Pee (Clean Version)" carried one title tag -// on 2026-10-20, and the explicit copy went. The -// copies on other releases stay and are linked as one song. At most budget -// copies go. It returns how many went and the songs they were, and drops them -// from their groups. +// album holds of the song into the best of that album's copies: the one that is +// not a video rip when only one is not, else ProposeSurvivor's choice. Copies +// whose title or file name name different versions (versionWords) are not +// folded, even when they carry one title tag: a song and its clean version are +// both kept. The copies on other releases stay and are linked as one song. At +// most budget copies go. It returns how many went and the songs they were, and +// drops them from their groups. func foldAlbumCopies( ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, groups []*resolveGroup, merged map[pgtype.UUID]bool, budget int, @@ -358,9 +356,10 @@ func foldAlbumCopy( return nil } -// linkCrossRelease links each cross-release group's copies as one song -// (#5438): a like on one is a like on all, and the Liked list, shuffle and the -// mixes count the song once. A group the pass merged has nothing left to link. Linking an already-linked group changes nothing, +// linkCrossRelease links each cross-release group's copies as one song: a +// like on one is a like on all, and the Liked list, shuffle and the +// mixes count the song once. A group the pass merged has nothing left to link. +// Linking an already-linked group changes nothing, // so this runs every pass; likes are shared only when a link is new, since a // like made after it reaches every copy as it is made. func linkCrossRelease( @@ -597,8 +596,8 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) (waitForScan bool return false } // A scan in flight has listed the files it will reconcile: a copy removed - // now is marked present again when it does (7 of the 13 removed at boot - // on 2026-10-20, #6169). A run past StuckScanThreshold is a dead one. + // now is marked present again when it does. A run past StuckScanThreshold + // is a dead one. run, err := q.GetInFlightScanRun(ctx) switch { case err == nil && time.Since(run.StartedAt.Time) <= StuckScanThreshold: diff --git a/internal/library/duplicate_resolve_test.go b/internal/library/duplicate_resolve_test.go index 58ccd0db..5cd41a74 100644 --- a/internal/library/duplicate_resolve_test.go +++ b/internal/library/duplicate_resolve_test.go @@ -95,8 +95,8 @@ func autoMergeAudit(t *testing.T, pool *pgxpool.Pool, groupID pgtype.UUID) (n in } // One album holding a song twice: the extra copy goes, the copy whose name and -// tags fit the album stays, and the merge is recorded as the resolver's -// (M502 #5517): Minstrel is the only program writing the library. +// tags fit the album stays, and the merge is recorded as the resolver's: +// Minstrel is the only program writing the library. func TestResolveDuplicates_MergesASameReleaseCopy_Integration(t *testing.T) { f := newResolveFixture(t) res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true) @@ -193,7 +193,7 @@ func newCrossReleaseRipFixture(t *testing.T) crossReleaseRipFixture { } // A video rip on one release with the clean copy on another: the rip is no -// release's copy, so it goes and the album cut stays (#5439). +// release's copy, so it goes and the album cut stays. func TestResolveDuplicates_MergesARipAcrossReleases_Integration(t *testing.T) { f := newCrossReleaseRipFixture(t) res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true) @@ -212,7 +212,7 @@ func TestResolveDuplicates_MergesARipAcrossReleases_Integration(t *testing.T) { } // Copies on two releases that are both clean each belong to their release: -// nothing is removed, and they are linked as one song. A rip the operator +// nothing is removed, and they are linked as one song. A rip an admin // marked fine counts as clean. func TestResolveDuplicates_KeepsCleanCopiesOnTwoReleases_Integration(t *testing.T) { for name, prepare := range map[string]func(f crossReleaseRipFixture) (string, []any){ @@ -392,8 +392,8 @@ func TestResolveDuplicates_RemovesACopyFromAnotherSongsSlot_Integration(t *testi if err != nil { t.Fatal(err) } - // Twelve slots were lost this way on 2026-10-20: folded by that MBID into - // the copy kept, and so never wanted back. + // Folded by that MBID into the copy kept, the slot would never be wanted + // back. if pairs, err := PairMissingTracks(context.Background(), f.pool, nil); err != nil || pairs.Folded() != 0 { t.Fatalf("missing-pair pass = %+v, %v; want the emptied slot left alone", pairs, err) } @@ -419,9 +419,8 @@ func TestResolveDuplicates_RemovesACopyFromAnotherSongsSlot_Integration(t *testi // A slot emptied of another song's file folds into a copy of its own song the // album already holds (Savant's VOID DLC "Weeper" beside Void's). A copy under -// the title that is not as long as the song is another song (the emptied "Open -// the Gateway" folded into a file holding Onward, 2026-10-20): the slot waits -// for a copy fetched again, and folds into that. +// the title that is not as long as the song is another song: the slot waits for +// a copy fetched again, and folds into that. func TestResolveDuplicates_EmptiedSlotFoldsOnlyIntoItsOwnSong_Integration(t *testing.T) { for _, tc := range []struct { name string @@ -522,7 +521,7 @@ func releaseBlocked(t *testing.T, pool *pgxpool.Pool, hash string) bool { } // AcoustID matched one copy to Tormentor's recording: that copy stays, Record -// Shop's goes, and the download they came from is set aside (#6169). +// Shop's goes, and the download they came from is set aside. func TestResolveDuplicates_KeepsTheCopyAcoustIDNames_Integration(t *testing.T) { f := seedTrimmedAlbum(t) if _, err := f.pool.Exec(context.Background(), @@ -551,9 +550,8 @@ func TestResolveDuplicates_KeepsTheCopyAcoustIDNames_Integration(t *testing.T) { // Copies whose own lookups all name Record Shop's recording are that // recording, though their fingerprints differ a little and one is six seconds -// from Tormentor's length, too near for the lengths to decide (the operator's -// Wombats and Monday Chill groups, 2026-10-20). With no lookups, copies that -// far apart are not asked about. +// from Tormentor's length, too near for the lengths to decide. With no +// lookups, copies that far apart are not asked about. func TestResolveDuplicates_AgreeingLookupsDecideAnAcousticGroup_Integration(t *testing.T) { setup := func(t *testing.T) oneAlbumFixture { f := seedTrimmedAlbum(t) @@ -706,7 +704,7 @@ func TestResolveDuplicates_LeavesAnUndecidedSlotForTheOperator_Integration(t *te } } -// A rip is held back from radio and the mixes; the operator's "fine" sticks +// A rip is held back from radio and the mixes; an admin's "fine" sticks // through later passes; a renamed file is released. func TestResolveDuplicates_HoldsBackRips_Integration(t *testing.T) { f := newCrossReleaseRipFixture(t) diff --git a/internal/library/duplicate_settle.go b/internal/library/duplicate_settle.go index f6812b5d..d44b0c4f 100644 --- a/internal/library/duplicate_settle.go +++ b/internal/library/duplicate_settle.go @@ -19,22 +19,21 @@ import ( syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) -// Duplicates that settle without a merge (M498, operator 2026-10-19: "this -// needs to be automated away"). The measured library had 536 groups waiting -// on the operator; most were retitles across releases, now linked like any -// cross-release group. What is left on one album is two shapes: +// Duplicates that settle without a merge, so they do not wait on an admin. +// Retitles across releases are linked like any cross-release group. What is +// left on one album is three shapes: // // - distinct: two versions of a song that sound alike ("Lovesick" and // "Lovesick (Instrumental)"). Not duplicates, so the group is dismissed. -// - mismatch: one file filling several track slots (Savant's Void held one -// recording as tracks 15, 17 and 20). The album's MusicBrainz track list +// - mismatch: one file filling several track slots (one recording as +// three of an album's tracks). The album's MusicBrainz track list // says how long each slot's song is. When exactly one slot's song is as // long as the file and every other slot's song is clearly not, the copies // in the other slots are removed and their tracks marked missing, so the // reacquisition sweeper wants the album back and a good copy fills them. // When every slot's song is clearly not the file, every copy goes. -// - named: the lengths cannot say (Savant's Mortals held one recording as -// 16 of its 50 tracks, each cut to its own slot's length), but AcoustID +// - named: the lengths cannot say (one recording filed as many tracks, each +// cut to its own slot's length), but AcoustID // can. When every copy's lookup names one recording, or the copies are // the same audio and a lookup names any, the track list says whose slot // that recording is: that copy is kept and the rest go, or every copy @@ -42,9 +41,9 @@ import ( // know at all is no copy's song, and every copy goes. // // Whenever copies go, the release the files came from is set aside, so the -// album fetched again is not the same files (#6169). +// album fetched again is not the same files. // -// Anything less certain stays with the operator, who settles it the same way +// Anything less certain stays with an admin, who settles it the same way // with their own answer (SettleWrongSlot). const ( @@ -63,7 +62,7 @@ const ( // sameAudioBER is the worst bit error rate at which an acoustic group's // copies are one recording cut differently rather than takes that sound // alike: the groups measured as one recording trimmed to several slots - // read 0.000, and two takes of one song read far above this (#6169). + // read 0.000, and two takes of one song read far above this. sameAudioBER = 0.02 ) @@ -268,7 +267,7 @@ type WrongSlotResult struct { } // SettleWrongSlot settles a one-album mismatch the resolver left for the -// operator (#6184), with their answer in place of MusicBrainz's lengths and +// admin, with their answer in place of MusicBrainz's lengths and // AcoustID's. keepID is the copy whose title they say the recording is; an // invalid id means they cannot say, and every copy goes. Either way it is the // resolver's own path: the release the files came from is set aside, the @@ -522,7 +521,7 @@ func neverRead(ctx context.Context, q *dbq.Queries, rg string) bool { // are stamped to wait for a copy fetched again (emptiedSlots). blocked names // the releases set aside, for the record. A file that cannot be removed stops // the group; the ones already gone are still marked, and the group waits for -// the next pass. actorID is the operator settling it by hand, invalid for the +// the next pass. actorID is the admin settling it by hand, invalid for the // resolver. func removeWrongSlots( ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, g *resolveGroup, keep int, losers []int, @@ -597,8 +596,7 @@ func removeWrongSlots( // notifyNewForReview tells admins about groups this pass classified for the // first time and left for them. A group the resolver settled, linked or will -// link is not news: telling admins about those was most of what the notice -// used to say. +// link is not news, and would bury the groups that need them. func notifyNewForReview(ctx context.Context, groups []*resolveGroup, settled map[pgtype.UUID]bool) { var count int64 var items []string diff --git a/internal/library/duplicate_settle_test.go b/internal/library/duplicate_settle_test.go index 8c623ff4..bf14ffc3 100644 --- a/internal/library/duplicate_settle_test.go +++ b/internal/library/duplicate_settle_test.go @@ -16,7 +16,7 @@ import ( // undecidedSlot is one recording filed as Void and as Reach, two songs // MusicBrainz lists at lengths that cannot tell them apart, from a download -// Minstrel imported: the resolver leaves it pending for the operator (#6184). +// Minstrel imported: the resolver leaves it pending for an admin. func undecidedSlot(t *testing.T) (oneAlbumFixture, dbq.User) { t.Helper() f := newOneAlbumFixture(t, "acoustic", "Void", "Reach") @@ -53,7 +53,7 @@ func emptiedTrack(t *testing.T, f oneAlbumFixture, id pgtype.UUID) bool { return emptied } -// settledByHand checks the group closed as the operator's and the audit row +// settledByHand checks the group closed as the admin's and the audit row // names them. func settledByHand(t *testing.T, f oneAlbumFixture, actor dbq.User) { t.Helper() @@ -72,7 +72,7 @@ func settledByHand(t *testing.T, f oneAlbumFixture, actor dbq.User) { } } -// "Get it again": the operator cannot say which song the recording is, so +// "Get it again": the admin cannot say which song the recording is, so // every copy goes, the download they came from is set aside, and both slots // wait for the album fetched again. A merge would have kept one title. func TestSettleWrongSlot_GetItAgainRemovesEveryCopy_Integration(t *testing.T) { @@ -101,7 +101,7 @@ func TestSettleWrongSlot_GetItAgainRemovesEveryCopy_Integration(t *testing.T) { } } -// "This one is Reach": the copy the operator names stays, and only Void's slot +// "This one is Reach": the copy the admin names stays, and only Void's slot // is emptied and wanted again. func TestSettleWrongSlot_KeepsTheCopyTheOperatorNames_Integration(t *testing.T) { f, op := undecidedSlot(t) diff --git a/internal/library/duplicate_survivor.go b/internal/library/duplicate_survivor.go index ea060f80..bfb8a021 100644 --- a/internal/library/duplicate_survivor.go +++ b/internal/library/duplicate_survivor.go @@ -18,15 +18,15 @@ type SurvivorCandidate struct { TagFit int } -// TagFitScore counts the signs that a copy belongs where it is filed (M498 -// #5436), one point each: +// TagFitScore counts the signs that a copy belongs where it is filed, one +// point each: // - it has a track number that no other track on its album also claims // - its file name carries no video-rip marker (SourceMarkersFor) // - it has a MusicBrainz recording id // // Two copies of one song differ in these far more often than in anything a -// listener hears: the Humanz rips differ by a few bytes, and file size picked -// the wrong one in 6 of 21 groups. +// listener hears: rips of one song can differ by a few bytes, which makes file +// size a poor guide. func TagFitScore(hasTrackNumber, positionClash bool, filePath string, hasMBID bool) int { score := 0 if hasTrackNumber && !positionClash { @@ -48,13 +48,12 @@ func TagFitScore(hasTrackNumber, positionClash bool, filePath string, hasMBID bo var losslessFormats = map[string]bool{"flac": true, "wav": true} // ProposeSurvivor picks which copy of a duplicate group to keep, and gives the -// reason in words the operator reads beside it. It is a default, not a verdict: -// the report shows it and the merge (#3911) lets the operator choose another. +// reason in words an admin reads beside it. It is a default, not a verdict: +// the report shows it and the merge lets an admin choose another. // // In order: // 1. lossless over lossy — the one difference no later step can recover. It -// comes before tag fit: tags can be fixed, a deleted FLAC cannot, and on -// 2026-10-20 five FLACs went for mp3s that only carried a recording MBID +// comes before tag fit: tags can be fixed, a deleted FLAC cannot // 2. the copy whose tags fit the album best (TagFitScore). It is zero for // every copy when the caller does not know it, and the order falls through // to the rules below diff --git a/internal/library/duplicate_survivor_test.go b/internal/library/duplicate_survivor_test.go index 5d11f56b..ad0020f9 100644 --- a/internal/library/duplicate_survivor_test.go +++ b/internal/library/duplicate_survivor_test.go @@ -42,7 +42,7 @@ func TestProposeSurvivor(t *testing.T) { wantID: "320k", wantReason: "largest file", }, { - // The #3885 pair: identical audio, sizes equal but for the tags. + // Identical audio, sizes equal but for the tags. name: "the longest-standing copy wins when size ties", cands: []SurvivorCandidate{ {TrackID: "www-02", FileFormat: "mp3", FileSize: 6_900_000, AddedAt: newer}, @@ -90,9 +90,9 @@ func TestProposeSurvivor_ReasonIsTheDecidingRule(t *testing.T) { } } -// M498 #5436: the copy whose tags fit the album outranks every quality rule but -// lossless, which decides first: a deleted FLAC cannot be recovered, and on -// 2026-10-20 five went for mp3s whose only edge was a recording MBID. Each tag +// The copy whose tags fit the album outranks every quality rule but lossless, +// which decides first: a deleted FLAC cannot be recovered, so it never goes +// for an mp3 whose only edge is a recording MBID. Each tag // fit case is built so file size would choose the other copy. func TestProposeSurvivor_LosslessThenTagFit(t *testing.T) { at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) diff --git a/internal/library/duplicate_sweep.go b/internal/library/duplicate_sweep.go index 34dd2cb8..df54351e 100644 --- a/internal/library/duplicate_sweep.go +++ b/internal/library/duplicate_sweep.go @@ -17,37 +17,38 @@ import ( syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) -// Duplicate sweep (M400 #3910). +// Duplicate sweep. // -// Reads fingerprints, runs them through the matcher and records what it proposes -// in duplicate_groups. It never merges or deletes anything the operator has not -// asked for: a group is a proposal, reviewed in the admin report (#3912). +// Reads fingerprints, runs them through the matcher and records what it +// proposes in duplicate_groups. It never merges or deletes anything itself: a +// group is a proposal, for the resolver and the admin report. // -// Blocked on duration, deliberately not on title: the #3885 pair are titled -// "WWW" and "WWW (instrumental)", so a title block would have missed the case -// that started the milestone. Candidates stream in (duration_ms, id) order and -// the grouper holds only a few seconds of durations at a time. +// Blocked on duration, deliberately not on title: one file filed as "WWW" and +// "WWW (instrumental)" would escape a title block. Candidates stream in +// (duration_ms, id) order and the grouper holds only a few seconds of durations +// at a time. // duplicateCandidatePage is how many candidates one query returns. Each row // carries a ~4 KB fingerprint, so a page is about 2 MB. const duplicateCandidatePage = 500 // duplicateSweepTick is how often the worker checks whether a sweep is due. The -// operator's sweep interval (#3913) is the least time between sweeps; the tick -// only bounds how late past it one starts. With nothing due, a tick is two cheap -// aggregate queries. +// sweep interval setting is the least time between sweeps; the tick only bounds +// how late past it one starts. With nothing due, a tick is two cheap aggregate +// queries. const duplicateSweepTick = time.Hour // sweepIntervalSlack absorbs the moment between a tick and the sweep it starts -// stamping started_at. Without it a one-hour interval checked on a one-hour tick -// would find the last sweep a moment under an hour old, and skip every other tick. +// stamping started_at. Without it a one-hour interval checked on a one-hour +// tick would find the last sweep a moment under an hour old, and skip every +// other tick. const sweepIntervalSlack = 5 * time.Minute // staleDuplicateSweepThreshold is the age past which an in-flight sweep is -// assumed dead — a crash mid-sweep leaves finished_at NULL for ever — and another -// may start. Twice the library scan's threshold, because a sweep compares -// fingerprints across the whole library and can legitimately run long on a big -// one. +// assumed dead — a crash mid-sweep leaves finished_at NULL for ever — and +// another may start. Twice the library scan's threshold, because a sweep +// compares fingerprints across the whole library and can legitimately run long +// on a big one. const staleDuplicateSweepThreshold = 2 * time.Hour // duplicateSweepFinishTimeout bounds recording that a sweep ended. It runs on a @@ -60,7 +61,7 @@ type DuplicateSweepResult struct { Candidates int // tracks with a chromaprint that were streamed Groups int // groups the matcher found Proposed int // written as pending, new or refreshed - Suppressed int // not proposed: already dismissed or resolved by the operator + Suppressed int // not proposed: already dismissed or resolved Retired int // pending proposals this sweep did not find again, removed Oversize int // acoustic clusters too large to propose } @@ -128,15 +129,17 @@ func sweepDuplicates( exactSeen := make([]bool, len(exactRows)) // Acoustic tier, streamed in duration order. The first member of an exact - // group the stream meets stands in for the whole group; the rest are skipped. - // Only prints at the current length are streamed: a print at another length - // cannot be compared, and is waiting on the backfill to be re-derived. + // group the stream meets stands in for the whole group; the rest are + // skipped. Only prints at the current length are streamed: a print at + // another length cannot be compared, and is waiting on the backfill to be + // re-derived. grouper := newStreamGrouper(cfg.AcousticMaxBitErrorRate) params := dbq.ListDuplicateCandidatesParams{ CurrentVersion: fingerprintVersion, ChromaprintLengthSec: cfg.ChromaprintLengthSec, - // Durations are never negative, and the all-zero uuid sorts first: every - // row is after this cursor. Valid must be true, or "> NULL" matches nothing. + // Durations are never negative, and the all-zero uuid sorts first: + // every row is after this cursor. Valid must be true, or "> NULL" + // matches nothing. AfterDurationMs: -1, AfterID: pgtype.UUID{Valid: true}, PageLimit: pageSize, @@ -242,7 +245,7 @@ func sweepDuplicates( } // coveredByDismissal reports whether every member of a proposal sat together in -// one group the operator dismissed. A subset counts: dismissing {A, B, C} said +// one group an admin dismissed. A subset counts: dismissing {A, B, C} said // none of them are copies of each other, so proposing {A, B} again would be // asking the same question twice. A superset does not count: a new copy joining // is new evidence, and worth asking about. @@ -305,7 +308,7 @@ func TryStartDuplicateSweep( } // DuplicateSweepWorker sweeps whenever its input has changed, at most once per -// the operator's sweep interval. +// sweep interval. type DuplicateSweepWorker struct { pool *pgxpool.Pool logger *slog.Logger @@ -336,7 +339,7 @@ func (w *DuplicateSweepWorker) Run(ctx context.Context) { } } -// tickOnce contains one check so nothing it does can stop the next tick (rule 157). +// tickOnce contains one check so nothing it does can stop the next tick. func (w *DuplicateSweepWorker) tickOnce(ctx context.Context) { defer func() { if r := recover(); r != nil { @@ -377,12 +380,12 @@ func duplicateSweepDue(ctx context.Context, q *dbq.Queries, cfg FingerprintSetti } // sweepIsDue reports whether a sweep should start: something it reads has -// changed since the last sweep started, and the operator's interval has passed. +// changed since the last sweep started, and the sweep interval has passed. // // Two things can change its answer. Fingerprints are its input, so any written // after the last sweep began count; while the backfill runs that is true every -// tick, which is what the interval is for. And a settings save counts, because a -// new threshold or length changes what the same fingerprints group into. +// tick, which is what the interval is for. And a settings save counts, because +// a new threshold or length changes what the same fingerprints group into. func sweepIsDue(latestFingerprint, lastSweepStart pgtype.Timestamptz, cfg FingerprintSettings, now time.Time) bool { if !latestFingerprint.Valid { return false // nothing fingerprinted yet diff --git a/internal/library/duplicate_sweep_test.go b/internal/library/duplicate_sweep_test.go index 15bc478b..676425ff 100644 --- a/internal/library/duplicate_sweep_test.go +++ b/internal/library/duplicate_sweep_test.go @@ -37,8 +37,9 @@ func TestCoveredByDismissal(t *testing.T) { } } -// TestDuplicateSweep_Integration pins what the sweep proposes, what it leaves out, -// and how re-sweeping treats a dismissal and a proposal that no longer holds. +// TestDuplicateSweep_Integration pins what the sweep proposes, what it leaves +// out, and how re-sweeping treats a dismissal and a proposal that no longer +// holds. func TestDuplicateSweep_Integration(t *testing.T) { pool := newPool(t) ctx := context.Background() @@ -175,9 +176,10 @@ func TestDuplicateSweep_Integration(t *testing.T) { t.Fatalf("latest sweep = %+v, want finished without error", last) } - // 5. Prints taken at another length are never compared (#3913). Every print - // here was taken at the default length, so a sweep at 60s has nothing to read, - // rather than scoring 120s prints against each other as if they were 60s ones. + // 5. Prints taken at another length are never compared. Every print + // here was taken at the default length, so a sweep at 60s has nothing to + // read, rather than scoring 120s prints against each other as if they were + // 60s ones. atOtherLength := DefaultFingerprintSettings atOtherLength.ChromaprintLengthSec = 60 res, err = runDuplicateSweep(ctx, pool, logger, atOtherLength, duplicateCandidatePage) diff --git a/internal/library/eventbus.go b/internal/library/eventbus.go index 6a1c805a..aacb603d 100644 --- a/internal/library/eventbus.go +++ b/internal/library/eventbus.go @@ -8,7 +8,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/eventbus" ) -// Package-level bus pointer for scan lifecycle events (#392). Set once +// Package-level bus pointer for scan lifecycle events. Set once // at startup via SetEventBus; reads are nil-safe so test contexts that // never call SetEventBus simply skip publishing. // diff --git a/internal/library/fingerprint.go b/internal/library/fingerprint.go index c9f5be57..ff35fdb5 100644 --- a/internal/library/fingerprint.go +++ b/internal/library/fingerprint.go @@ -18,7 +18,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// Acoustic identity (M400). +// Acoustic identity. // // Two values per track, because they answer different questions: // @@ -26,7 +26,7 @@ import ( // 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). +// their ID3. // // chromaprint fpcalc's raw fingerprint. Close means the same // recording, even at another bitrate or in another codec @@ -39,9 +39,9 @@ import ( // 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. +// 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 @@ -52,9 +52,10 @@ const fingerprintWaitDelay = 5 * time.Second // fingerprintVersion stamps how a track_fingerprints row was derived. Bump it // whenever the derivation changes — the hash arguments or fpcalc's flags — 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. +// are not comparable, and nothing else would reveal that the library held a +// mix. // -// The length is deliberately not part of it: it is an operator setting (#3913), +// The length is deliberately not part of it: it is an admin setting, // so each row records the length it was taken at and readers compare only rows // at the current one. See fingerprint_settings.go. const fingerprintVersion int16 = 1 @@ -63,7 +64,7 @@ const fingerprintVersion int16 = 1 // 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 the shipped value of the length setting (#3913): +// defaultChromaprintLengthSec is the shipped value of the length setting: // how many seconds of audio fpcalc fingerprints. 120 is fpcalc's own default. const defaultChromaprintLengthSec = 120 @@ -99,10 +100,10 @@ func streamHashArgs(path string) []string { // 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. +// 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", @@ -112,7 +113,7 @@ func fpcalcArgs(path string, lengthSec int) []string { } // lookupFingerprintLengthSec is how much audio the AcoustID lookup print -// covers (M401). Fixed at fpcalc's default rather than following the +// covers. Fixed at fpcalc's default rather than following the // duplicate-detection length setting: AcoustID's database is built from // prints that taggers such as Picard take at the default, and a lookup print // of a different span matches it less well. @@ -120,7 +121,7 @@ const lookupFingerprintLengthSec = 120 // fpcalcLookupArgs asks for the COMPRESSED fingerprint, fpcalc's default // output and the string AcoustID's lookup takes. A second decode beside the -// raw print, chosen over re-encoding the stored raw array in Go (M401 D1): +// raw print, chosen over re-encoding the stored raw array in Go: // fpcalc is the reference encoder, while a subtle bug in a port would still // produce a well-formed string that AcoustID answers with "no match". func fpcalcLookupArgs(path string) []string { @@ -147,8 +148,8 @@ type fingerprintResult struct { } // fingerprintOptions is what the settings decide for one attempt. Captured once -// per file, so the length a chromaprint was taken at is the length stored with it -// even if the setting changes mid-attempt. +// per file, so the length a chromaprint was taken at is the length stored with +// it even if the setting changes mid-attempt. type fingerprintOptions struct { lengthSec int32 // chromaprint false takes the stream hash alone: a demux, no decode. @@ -208,11 +209,11 @@ const ( ) // storeFingerprint records one attempt, for the scan (new or changed bytes) and -// the backfill (#3908) alike, so there is one rule for what gets written. It +// the backfill alike, so there is one rule for what gets written. It // never fails its caller: a missing fingerprint only keeps a track out of // duplicate detection, which is not worth dropping a scan or a pass over. // -// lengthSec is the length fp's chromaprint was taken at, stored with it (#3913). +// lengthSec is the length fp's chromaprint was taken at, stored with it. func storeFingerprint( ctx context.Context, q *dbq.Queries, logger *slog.Logger, trackID pgtype.UUID, path string, fp fingerprintResult, lengthSec int32, @@ -284,8 +285,8 @@ func computeLookupFingerprint(ctx context.Context, path string) (lookupFingerpri // 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 +// 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. @@ -336,11 +337,10 @@ func (e *toolExitError) Error() string { const fpcalcExitPartial = 3 // fpcalcReadToEnd reports whether fpcalc's partial exit only means it decoded -// the whole file (#6178). The image's fpcalc 1.5.1 reports the decoder's end of -// file as "Error decoding audio frame (End of file)" and exits 3 whenever it -// reaches the end before -length, so it failed every file shorter than the -// span asked for: every track under two minutes, at any length setting. That -// print covers the whole file, so it is the file's identity. +// the whole file. The image's fpcalc 1.5.1 reports the decoder's end of file +// as "Error decoding audio frame (End of file)" and exits 3 whenever it +// reaches the end before -length, which is every file shorter than the span +// asked for. That print covers the whole file, so it is the file's identity. // // A damaged file can stop at an end of file too. Its DURATION, the length the // container declares, is then longer than the span asked for, and it stays a @@ -475,7 +475,7 @@ func notURLBase64(r rune) bool { // LookupFingerprint is the compressed fingerprint AcoustID's lookup takes and // the file's whole length in seconds, for a file not in the library yet: the -// import checks a download with it (M502 #5522). IsInconclusive tells a +// import checks a download with it. IsInconclusive tells a // missing or stalled fpcalc from a file it rejected. func LookupFingerprint(ctx context.Context, path string) (fingerprint string, durationSec int, err error) { fp, err := computeLookupFingerprint(ctx, path) diff --git a/internal/library/fingerprint_backfill.go b/internal/library/fingerprint_backfill.go index eb19c1c8..7f25ab9c 100644 --- a/internal/library/fingerprint_backfill.go +++ b/internal/library/fingerprint_backfill.go @@ -13,11 +13,11 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// Fingerprint backfill (M400 #3908). +// Fingerprint backfill. // -// The scan fingerprints only bytes it has not seen (see scanFile), so every track -// imported before fingerprinting existed — and every row derived by an older -// fingerprintVersion — needs a pass of its own. That pass is this worker. +// The scan fingerprints only bytes it has not seen (see scanFile), so every +// track imported before fingerprinting existed — and every row derived by an +// older fingerprintVersion — needs a pass of its own. That pass is this worker. // // Its own worker rather than a stage in RunScan, for two reasons, both about // time: @@ -28,20 +28,23 @@ import ( // - A long stage holds the scan run in flight, and a manual rescan answers 409 // for as long as it runs. // -// Progress is read live (FingerprintCoverage, the admin gauge) rather than from a -// scan_runs tally: the work spans many passes with no single run to attach to. +// Progress is read live (FingerprintCoverage, the admin gauge) rather than from +// a scan_runs tally: the work spans many passes with no single run to attach +// to. // fingerprintBackfillTick is how often the worker looks for work. Once the -// library has caught up, a tick is one indexed query; mostly the hour bounds how -// long a file that timed out on a slow mount waits before it is tried again. +// library has caught up, a tick is one indexed query; mostly the hour bounds +// how long a file that timed out on a slow mount waits before it is tried +// again. const fingerprintBackfillTick = time.Hour -// fingerprintBackfillBatch is how many tracks one query hands the worker. Small, -// so tracks the scan adds mid-pass are not stuck behind one enormous page. +// fingerprintBackfillBatch is how many tracks one query hands the worker. +// Small, so tracks the scan adds mid-pass are not stuck behind one enormous +// page. const fingerprintBackfillBatch = 50 -// fingerprintBackfillConcurrency is the shipped value of the concurrency setting -// (#3913): how many files are decoded at once. Two is deliberately low: fpcalc +// fingerprintBackfillConcurrency is the shipped value of the concurrency +// setting: how many files are decoded at once. Two is deliberately low: fpcalc // and the stream hash compete with playback transcoding for CPU and with // streaming for the mount, and a backfill that makes playback stutter is worse // than one that takes longer. @@ -75,7 +78,8 @@ type FingerprintBackfillWorker struct { tick time.Duration batch int32 // fingerprint is a field for the same reason as Scanner.fingerprint: an - // integration test pins which tracks a pass touches, not what the tools print. + // integration test pins which tracks a pass touches, not what the tools + // print. fingerprint func(ctx context.Context, path string, opts fingerprintOptions) fingerprintResult } @@ -110,8 +114,8 @@ func (w *FingerprintBackfillWorker) Run(ctx context.Context) { } } -// runOnce contains a pass so that nothing it does — an error, a panic — can stop -// the next tick from firing (rule 157). +// runOnce contains a pass so that nothing it does — an error, a panic — can +// stop the next tick from firing. func (w *FingerprintBackfillWorker) runOnce(ctx context.Context) { defer func() { if r := recover(); r != nil { @@ -135,10 +139,10 @@ func (w *FingerprintBackfillWorker) runOnce(ctx context.Context) { // retried forever within the pass. // // Settings are read before every batch, so a save takes effect within a batch -// rather than an hour (#3913): switching fingerprinting off ends the pass, a new -// concurrency applies to the next batch, and a new length restarts the walk from -// the top at that length, because every row written at the old one went stale -// the moment it changed. +// rather than an hour: switching fingerprinting off ends the pass, a new +// concurrency applies to the next batch, and a new length restarts the walk +// from the top at that length, because every row written at the old one went +// stale the moment it changed. func (w *FingerprintBackfillWorker) pass(ctx context.Context) (BackfillFingerprintsResult, error) { q := dbq.New(w.pool) var ( @@ -146,7 +150,8 @@ func (w *FingerprintBackfillWorker) pass(ctx context.Context) (BackfillFingerpri mu sync.Mutex ) // The all-zero uuid sorts before every real id. Valid must be true: a NULL - // cursor would make "id > NULL" match nothing and every pass a silent no-op. + // cursor would make "id > NULL" match nothing and every pass a silent + // no-op. start := pgtype.UUID{Valid: true} after := start lengthSec := w.settings.Get().ChromaprintLengthSec diff --git a/internal/library/fingerprint_backfill_test.go b/internal/library/fingerprint_backfill_test.go index fb3a166a..ae37c662 100644 --- a/internal/library/fingerprint_backfill_test.go +++ b/internal/library/fingerprint_backfill_test.go @@ -151,11 +151,11 @@ func TestFingerprintBackfill_Integration(t *testing.T) { t.Errorf("coverage buckets %+v do not sum to the total", cov) } - // 5. Changing the length (#3913) makes every stored row stale at once — the + // 5. Changing the length makes every stored row stale at once — the // gauge shows the whole library pending before the backfill has touched a // file — and the next pass re-derives each at the new length. The failure - // this prevents is invisible from the UI: prints at two lengths that silently - // never match. + // this prevents is invisible from the UI: prints at two lengths that + // silently never match. shorter := DefaultFingerprintSettings shorter.ChromaprintLengthSec = 60 if _, err := settings.Set(ctx, shorter); err != nil { diff --git a/internal/library/fingerprint_scan_test.go b/internal/library/fingerprint_scan_test.go index 1e4c1d0a..8d505438 100644 --- a/internal/library/fingerprint_scan_test.go +++ b/internal/library/fingerprint_scan_test.go @@ -193,9 +193,10 @@ func TestScanner_FingerprintsOnlyNewOrChangedBytes_Integration(t *testing.T) { t.Fatalf("rejected file stored %+v, want both halves NULL at version %d", got, fingerprintVersion) } - // 6. With fingerprinting off (#3913) the scan decodes nothing. It still asks - // for the stream hash — a demux, and what recognises a moved file — but stores - // no row, and a changed file's old row goes: it describes bytes that are gone. + // 6. With fingerprinting off the scan decodes nothing. It still asks + // for the stream hash — a demux, and what recognises a moved file — but + // stores no row, and a changed file's old row goes: it describes bytes that + // are gone. result = fingerprintResult{streamSHA256: sum, chromaprint: chroma} off := DefaultFingerprintSettings off.Enabled = false diff --git a/internal/library/fingerprint_settings.go b/internal/library/fingerprint_settings.go index 69f94c7d..ff13c24b 100644 --- a/internal/library/fingerprint_settings.go +++ b/internal/library/fingerprint_settings.go @@ -12,10 +12,10 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// Fingerprinting settings (M400 #3913). +// Fingerprinting settings. // -// Rule 25: what an operator might tune lives in the database and changes without -// a restart. One service instance is shared by the scanner, the backfill, the +// What an admin might tune lives in the database and changes without a +// restart. One service instance is shared by the scanner, the backfill, the // duplicate sweep and the admin API, so a save reaches all of them at once. // // The length is the dangerous knob: chromaprints taken at two lengths cannot be @@ -29,8 +29,9 @@ import ( // Bounds for each setting, mirrored by migration 0061's CHECKs. const ( // Below 30s a print has too few items to align at the matcher's larger - // offsets and still overlap by minOverlapItems. Above 600s each print passes - // 20 KB and a candidate page of them grows past what one sweep should hold. + // offsets and still overlap by minOverlapItems. Above 600s each print + // passes 20 KB and a candidate page of them grows past what one sweep + // should hold. minChromaprintLengthSec = 30 maxChromaprintLengthSec = 600 // Unrelated audio sits near 0.5; a ceiling of 0.35 keeps a loosened @@ -55,9 +56,9 @@ type FingerprintSettings struct { AcousticMaxBitErrorRate float64 BackfillConcurrency int32 SweepIntervalHours int32 - // AutoResolve lets the duplicate resolver (M498) act on its own: merge - // copies no release needs. Off, it only classifies, and every group waits - // for the operator. + // AutoResolve lets the duplicate resolver act on its own: merge copies no + // release needs. Off, it only classifies, and every group waits for an + // admin. AutoResolve bool // UpdatedAt is when the settings were last saved. Set by the database; // ignored by Set. @@ -143,7 +144,8 @@ func validateFingerprintSettings(in FingerprintSettings) error { case in.ChromaprintLengthSec < minChromaprintLengthSec || in.ChromaprintLengthSec > maxChromaprintLengthSec: return fmt.Errorf("%w: chromaprint_length_sec must be %d-%d", ErrFingerprintSettingOutOfRange, minChromaprintLengthSec, maxChromaprintLengthSec) - // Written as a negated range so NaN, which fails every comparison, is refused. + // Written as a negated range so NaN, which fails every comparison, is + // refused. case !(in.AcousticMaxBitErrorRate >= minAcousticMaxBitErrorRate && in.AcousticMaxBitErrorRate <= maxAcousticMaxBitErrorRate): return fmt.Errorf("%w: acoustic_max_bit_error_rate must be %.2f-%.2f", ErrFingerprintSettingOutOfRange, minAcousticMaxBitErrorRate, maxAcousticMaxBitErrorRate) diff --git a/internal/library/fingerprint_settings_test.go b/internal/library/fingerprint_settings_test.go index 65c33d4f..d77b9b59 100644 --- a/internal/library/fingerprint_settings_test.go +++ b/internal/library/fingerprint_settings_test.go @@ -84,8 +84,8 @@ func TestFingerprintSettingsService_Integration(t *testing.T) { t.Fatalf("load: %v", err) } // ResetDB puts every column back to its migration default, so this pins the - // Go defaults to migration 0061's. Were they to drift, a database that could - // not be read would fingerprint differently from one that could. + // Go defaults to migration 0061's. Were they to drift, a database that + // could not be read would fingerprint differently from one that could. loaded := svc.Get() if loaded.UpdatedAt.IsZero() { t.Fatal("loaded settings carry no updated_at") diff --git a/internal/library/fingerprint_test.go b/internal/library/fingerprint_test.go index a8ec170b..1e9d3fe8 100644 --- a/internal/library/fingerprint_test.go +++ b/internal/library/fingerprint_test.go @@ -122,7 +122,8 @@ func TestFpcalcLookupArgs_RequestCompressedOutput(t *testing.T) { } func TestParseStreamHash(t *testing.T) { - // The real value ffmpeg printed for both files of the #3885 pair. + // The real value ffmpeg printed for two copies of one MP3 differing only + // in their ID3. const www = "24e2daa3b4a534ff1a8d1a76f67810205869daf89f728d83a16625da4d28a18e" got, err := parseStreamHash([]byte("SHA256=" + www + "\n")) @@ -146,7 +147,8 @@ func TestParseStreamHash(t *testing.T) { } } -// followedBy reports whether flag appears in args immediately followed by value. +// 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 { @@ -255,7 +257,7 @@ func TestFingerprintResult_InconclusiveIfEitherHalfIs(t *testing.T) { } // fpcalc 1.5.1 exits 3 with "(End of file)" on every file shorter than the -// span asked for (#6178). That print covers the whole file and is kept; a file +// span asked for. That print covers the whole file and is kept; a file // whose declared length runs past the span stopped early, and is not. func TestFpcalcReadToEnd(t *testing.T) { eof := []byte("ERROR: Error decoding audio frame (End of file)") diff --git a/internal/library/genre.go b/internal/library/genre.go index 7981e026..e84b549f 100644 --- a/internal/library/genre.go +++ b/internal/library/genre.go @@ -33,7 +33,7 @@ func extractGenres(meta tag.Metadata, rs io.ReadSeeker) (genres []string, fellBa // dhowden/tag will have nothing either — not worth flagging. fellBack = meta.Genre() != "" case tag.VORBIS, tag.MP4: - // dhowden/tag keeps only the last of a repeated field here (#2500): + // dhowden/tag keeps only the last of a repeated field here: // see vorbisgenre.go and mp4genre.go. var values []string var err error @@ -84,9 +84,8 @@ func normaliseGenres(values []string) []string { // A value may be: // - plain text ("Alternative Rock") — passed through // - a bare ID3v1 index ("17") — resolved to "Rock". This is what the spec -// says a numeric TCON means, and what ffmpeg does. It is why the operator's -// library showed genres like "4017" and "526617": several numeric values -// welded together by the old reader. +// says a numeric TCON means, and what ffmpeg does. Several of them +// welded together read as genres like "4017". // - ID3v2.3 refinement syntax ("(17)", "(51)(39)", "(17)Hard Rock", "(RX)") // — each parenthesised index becomes its own genre, and trailing text // becomes one more. @@ -134,7 +133,8 @@ func normaliseGenreValue(v string) []string { } if n, err := strconv.Atoi(v); err == nil { if name, ok := id3v1GenreName(n); ok { - // Canonical table name, deliberately NOT re-cased — see id3v1Genres. + // Canonical table name, deliberately NOT re-cased — see + // id3v1Genres. return append(out, name) } return out @@ -143,8 +143,7 @@ func normaliseGenreValue(v string) []string { } // genreAcronyms are tokens that belong in caps. Tag editors that title-case the -// whole genre field turn "EDM" into "Edm" and "UK Garage" into "Uk Garage", and -// the operator's library carries all of these (#2468). +// whole genre field turn "EDM" into "Edm" and "UK Garage" into "Uk Garage". // // Matched case-INSENSITIVELY, so "edm", "Edm" and "EDM" all land on "EDM". That // is a deliberate, narrow exception to the project's rule that genre case is @@ -169,8 +168,8 @@ var genreAcronyms = map[string]string{ // how "Children's Music" became "Children'S Music". // // Deliberately a fixed list rather than "lowercase whatever follows an -// apostrophe": that broader rule would break "O'Brien" and "D'Angelo", which are -// correctly capitalised after the apostrophe. +// apostrophe": that broader rule would break "O'Brien" and "D'Angelo", which +// are correctly capitalised after the apostrophe. var contractionSuffixes = map[string]bool{ "s": true, "t": true, "re": true, "ll": true, "ve": true, "d": true, "m": true, } @@ -178,7 +177,7 @@ var contractionSuffixes = map[string]bool{ // trueUpCasing repairs casing damage done by tag editors that title-case the // genre field. It only ever changes case, never the letters — so it cannot // silently turn one genre into a different one, which is what separates it from -// the label-remapping idea #2468 rejected. +// remapping labels. func trueUpCasing(s string) string { if s == "" { return s diff --git a/internal/library/genre_test.go b/internal/library/genre_test.go index 9be5da86..a3055058 100644 --- a/internal/library/genre_test.go +++ b/internal/library/genre_test.go @@ -67,7 +67,7 @@ func utf8Frame(values ...string) []byte { return append([]byte{0x03}, []byte(strings.Join(values, "\x00"))...) } -// TestReadID3v2GenreValues_MultiValue is the #2499 regression. dhowden/tag +// TestReadID3v2GenreValues_MultiValue: dhowden/tag // rejoins these values with the empty string, producing "Alternative RockRock"; // the whole point of our own reader is that they stay separate. func TestReadID3v2GenreValues_MultiValue(t *testing.T) { @@ -88,7 +88,7 @@ func TestReadID3v2GenreValues_MultiValue(t *testing.T) { } } -// The operator's worst case: eight values welded into one 70-character token. +// Eight values that a joining reader would weld into one 70-character token. func TestReadID3v2GenreValues_ManyValues(t *testing.T) { values := []string{ "Boom Bap", "Downtempo", "Hip Hop", "Instrumental", @@ -268,7 +268,7 @@ func TestNormaliseGenreValue(t *testing.T) { {"empty", "", nil}, {"whitespace only", " ", nil}, - // The operator's digit soup, one value at a time. + // Numeric references, one value at a time. {"bare numeric", "17", []string{"Rock"}}, {"bare numeric pop", "13", []string{"Pop"}}, {"bare numeric electronic", "52", []string{"Electronic"}}, @@ -426,7 +426,7 @@ func TestTrueUpCasing(t *testing.T) { in string want string }{ - // The damage actually present in the operator's library (#2468). + // Casing damage tag editors leave. {"edm acronym", "Edm", "EDM"}, {"idm acronym", "Idm", "IDM"}, {"aor acronym", "Aor", "AOR"}, diff --git a/internal/library/id3v2genre.go b/internal/library/id3v2genre.go index 7dbb15ce..49de328d 100644 --- a/internal/library/id3v2genre.go +++ b/internal/library/id3v2genre.go @@ -16,8 +16,8 @@ import ( // // — it splits on the ID3v2 null separator and rejoins with the EMPTY string, so // a file tagged "Alternative Rock" + "Rock" comes back as the single token -// "Alternative RockRock" (#2499). We stored that verbatim, which corrupted the -// genre browse axis and polluted the taste profile's tag vocabulary. +// "Alternative RockRock". Stored verbatim, that would corrupt the genre +// browse axis and pollute the taste profile's tag vocabulary. // // ffprobe is not an escape hatch either: ffmpeg's read_ttag calls decode_str // exactly once with no loop, so it keeps only the FIRST value and silently diff --git a/internal/library/leveled.go b/internal/library/leveled.go index b7abdd11..d13360f7 100644 --- a/internal/library/leveled.go +++ b/internal/library/leveled.go @@ -21,7 +21,7 @@ import ( "golang.org/x/sync/singleflight" ) -// Leveled streams (M464 #5001). +// Leveled streams. // // Sonos and other UPnP renderers fetch a track's URL themselves, so the phone // that sent them there cannot change what they play. For them the server diff --git a/internal/library/library_paths.go b/internal/library/library_paths.go index 70e34e77..fe0a9a5f 100644 --- a/internal/library/library_paths.go +++ b/internal/library/library_paths.go @@ -15,7 +15,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// The library's folders (#6129). They are set in Settings and changed without +// The library's folders. They are set in Settings and changed without // a restart; MINSTREL_LIBRARY_SCAN_PATHS names them until the first save. The // Scanner holds the live list, and the watcher and the importer read it there. // @@ -51,7 +51,7 @@ type RerootResult struct { } // RootsError is a save the folders refuse. Its text says why, in a sentence -// for the operator. +// for an admin. type RootsError struct{ msg string } func (e *RootsError) Error() string { return e.msg } diff --git a/internal/library/loudness.go b/internal/library/loudness.go index 78893553..ac6c8daa 100644 --- a/internal/library/loudness.go +++ b/internal/library/loudness.go @@ -20,7 +20,7 @@ import ( syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) -// Loudness analysis (M464 #4995). +// Loudness analysis. // // Every track is measured with ffmpeg's EBU R128 filter, which reports the // ITU-R BS.1770 values loudness normalization works from: @@ -39,7 +39,7 @@ import ( // 100 ms). Those are kept as a histogram, because an album's loudness is the // gated loudness of every block on the album, not an average of its tracks' // values. With the histograms stored, album loudness is exact and needs no -// second decode (#4996). +// second decode. // loudnessVersion stamps how a track_loudness row was measured. Bump it when // the measurement changes (the filter's options, the histogram's bins) and the @@ -136,7 +136,7 @@ func (h blockHistogram) empty() bool { return len(h.counts) == 0 } // loudness of the blocks that passed it. ok is false when nothing passes. // // The same function measures an album, over the sum of its tracks' -// histograms (#4996). +// histograms. func (h blockHistogram) gatedLoudness() (lufs float64, ok bool) { energy := func(i int) float64 { l := loudnessHistFloor + float64(int(h.start)+i)/loudnessHistPerLU @@ -354,7 +354,7 @@ const ( // value from an older method is a better gain than none until it is redone. // // A stored measurement is logged to the sync feed as a track upsert, so -// clients that cache the library (Android) pick up the new gain (#4997). If +// clients that cache the library (Android) pick up the new gain. If // the log fails nothing is stored, and the backfill tries the track again. func storeLoudness( ctx context.Context, db dbq.DBTX, logger *slog.Logger, @@ -394,7 +394,7 @@ func storeLoudness( } } } - // Logged before the write, as reconcile does (#2704): a log that then + // Logged before the write, as reconcile does: a log that then // finds the write failed costs clients one wasted re-read, but a write // that lands with no log is never retried, since the track is settled, and // clients would never learn its gain. diff --git a/internal/library/loudness_backfill.go b/internal/library/loudness_backfill.go index 3379c973..afcae46f 100644 --- a/internal/library/loudness_backfill.go +++ b/internal/library/loudness_backfill.go @@ -13,15 +13,14 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// Loudness backfill (M464 #4995). +// Loudness backfill. // // Every track is measured here, the new ones included: the scan only deletes a // changed file's measurement (see scanFile), and this worker measures it again. -// Measuring inline in the scan was the first plan and was dropped, because the -// analysis decodes the whole file. Added to the scan, a large import would run -// several times longer and could pass StuckScanThreshold (1h), at which point -// the run is reaped and a second scan started beside it. The fingerprint -// backfill is a worker of its own for the same reason. +// Not inline in the scan, because the analysis decodes the whole file: a large +// import would run several times longer and could pass StuckScanThreshold +// (1h), at which point the run is reaped and a second scan started beside it. +// The fingerprint backfill is a worker of its own for the same reason. // // Until a track is measured it plays with no gain adjustment, which is how // every track played before normalization existed. @@ -107,7 +106,7 @@ func (w *LoudnessBackfillWorker) Run(ctx context.Context) { } // runOnce contains a pass so that nothing it does (an error, a panic) can stop -// the next tick from firing (rule 157). +// the next tick from firing. func (w *LoudnessBackfillWorker) runOnce(ctx context.Context) { defer func() { if r := recover(); r != nil { @@ -123,7 +122,7 @@ func (w *LoudnessBackfillWorker) runOnce(ctx context.Context) { "processed", res.Processed, "measured", res.Measured, "silent", res.Silent, "unreadable", res.Unreadable, "inconclusive", res.Inconclusive) } - // Album loudness follows the tracks (#4996). It runs even with analysis + // Album loudness follows the tracks. It runs even with analysis // switched off: it decodes nothing, and membership still changes as the // library does. albums, err := w.albumPass(ctx) diff --git a/internal/library/loudness_backfill_test.go b/internal/library/loudness_backfill_test.go index 88451475..b650f733 100644 --- a/internal/library/loudness_backfill_test.go +++ b/internal/library/loudness_backfill_test.go @@ -132,7 +132,7 @@ func TestLoudnessBackfill_Integration(t *testing.T) { } // Each stored measurement told the sync feed, so caching clients re-read - // the track and pick up its gain (#4997). + // the track and pick up its gain. var logged int if err := pool.QueryRow(ctx, `SELECT count(*) FROM library_changes WHERE entity_type = 'track' AND op = 'upsert' AND entity_id = $1`, diff --git a/internal/library/loudness_settings.go b/internal/library/loudness_settings.go index 7f6f73d8..21fe1bad 100644 --- a/internal/library/loudness_settings.go +++ b/internal/library/loudness_settings.go @@ -12,9 +12,9 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// Loudness analysis settings (M464 #4995). Rule 25: an operator setting is a -// database row, changed without a restart. One instance is shared by the -// backfill and the admin API, so a save reaches the worker at once. +// Loudness analysis settings: a database row, changed without a restart. One +// instance is shared by the backfill and the admin API, so a save reaches the +// worker at once. // LoudnessSettings mirrors the loudness_settings row. type LoudnessSettings struct { @@ -23,14 +23,15 @@ type LoudnessSettings struct { Enabled bool BackfillConcurrency int32 // LeveledCacheMB caps the disk the leveled-stream renders for Sonos and - // UPnP speakers may use (#5001); the least recently played go first. + // UPnP speakers may use; the least recently played go first. LeveledCacheMB int32 // UpdatedAt is set by the database; ignored by Set. UpdatedAt time.Time } -// DefaultLoudnessSettings mirrors migrations 0065 and 0068's column defaults, so a -// database that cannot be read still analyzes the way a fresh install does. +// DefaultLoudnessSettings mirrors migrations 0065 and 0068's column defaults, +// so a database that cannot be read still analyzes the way a fresh install +// does. var DefaultLoudnessSettings = LoudnessSettings{ Enabled: true, BackfillConcurrency: loudnessBackfillConcurrency, diff --git a/internal/library/loudness_test.go b/internal/library/loudness_test.go index 6d4ac4da..8e2c9c03 100644 --- a/internal/library/loudness_test.go +++ b/internal/library/loudness_test.go @@ -65,7 +65,7 @@ func TestEbur128Parser_RealOutput(t *testing.T) { t.Errorf("histogram not trimmed to its occupied bins: %v", r.hist.counts) } - // Album loudness is computed from these histograms (#4996), so the + // Album loudness is computed from these histograms, so the // histogram has to reproduce ffmpeg's own figure. got, ok := r.hist.gatedLoudness() if !ok || math.Abs(got-(-10.7)) > 0.05 { diff --git a/internal/library/mbidbackfill.go b/internal/library/mbidbackfill.go index e804f167..2ebffced 100644 --- a/internal/library/mbidbackfill.go +++ b/internal/library/mbidbackfill.go @@ -18,9 +18,9 @@ import ( const pgUniqueViolation = "23505" // isUniqueViolation reports whether err is a Postgres unique-constraint -// conflict. Used to distinguish "another row already owns this MBID" -// (a duplicate album/artist row in our DB, the operator should merge -// it) from genuine write failures. +// conflict. Used to distinguish "another row already owns this MBID" (a +// duplicate album/artist row in our DB, for an admin to merge) from genuine +// write failures. func isUniqueViolation(err error) bool { var pgErr *pgconn.PgError return errors.As(err, &pgErr) && pgErr.Code == pgUniqueViolation @@ -31,7 +31,7 @@ type BackfillMBIDsResult struct { Processed int // Albums considered (mbid IS NULL). Healed int // Albums whose mbid we set. Skipped int // Albums where the file was missing or had no MBID tag. - Duplicates int // Albums whose mbid is already held by another row (operator-resolvable). + Duplicates int // Albums whose mbid another row already holds; an admin merges. } // BackfillMBIDs walks albums with NULL mbid, opens one track per album, diff --git a/internal/library/mbidbackfill_test.go b/internal/library/mbidbackfill_test.go index a645abe4..990d6e45 100644 --- a/internal/library/mbidbackfill_test.go +++ b/internal/library/mbidbackfill_test.go @@ -13,8 +13,8 @@ import ( ) // The MBID backfills read tags from files, so a track whose file is gone has -// nothing to give them. Before #5139 they opened it anyway, and every scan -// logged an "open failed" warning per missing track. +// nothing to give them: they skip it rather than log an "open failed" warning +// per missing track on every scan. func TestMBIDBackfill_SkipsMissingTracks_Integration(t *testing.T) { pool := newPool(t) ctx := context.Background() diff --git a/internal/library/mbids.go b/internal/library/mbids.go index 1a9c61bb..85bad411 100644 --- a/internal/library/mbids.go +++ b/internal/library/mbids.go @@ -14,8 +14,7 @@ type TagMBIDs struct { Release string Artist string // ReleaseGroup is musicbrainz_releasegroupid. Wants name albums by - // release group, so re-acquisition and request completion key on it - // (#5241). + // release group, so re-acquisition and request completion key on it. ReleaseGroup string // Recording is what tracks.mbid stores and the ListenBrainz Labs // similar-recordings API keys on. @@ -35,7 +34,7 @@ type TagMBIDs struct { // in Vorbis: MUSICBRAINZ_TRACKID there is the recording (the release track // is MUSICBRAINZ_RELEASETRACKID), so mbz files it under mbz.Track, its name // for the release track. Read mbz.Recording alone and every FLAC has no -// recording id (#5637). +// recording id. func ReadTagMBIDs(m tag.Metadata) TagMBIDs { info := mbz.Extract(m) ids := TagMBIDs{ diff --git a/internal/library/mbids_test.go b/internal/library/mbids_test.go index 95acc2cf..510ec05a 100644 --- a/internal/library/mbids_test.go +++ b/internal/library/mbids_test.go @@ -170,7 +170,7 @@ func TestCleanMBID_OnlyNULReturnsEmpty(t *testing.T) { } } -// #5241: the release-group id is read beside the release id, from each +// The release-group id is read beside the release id, from each // format's own key, and the two are never confused. func TestExtractReleaseGroupMBID(t *testing.T) { cases := []struct { @@ -207,7 +207,7 @@ func TestExtractReleaseGroupMBID(t *testing.T) { } } -// #5637: each format's recording id lands in Recording, and the release-track +// Each format's recording id lands in Recording, and the release-track // id never does. Picard calls the recording MUSICBRAINZ_TRACKID in Vorbis // comments, the name mbz gives the release track. func TestReadTagMBIDs_Recording(t *testing.T) { @@ -243,8 +243,7 @@ func TestReadTagMBIDs_Recording(t *testing.T) { } } -// A Vorbis TRACKNUMBER of "3/20", as Minstrel's importer wrote it, is track 3 -// (#6171): dhowden/tag reads it as 0. +// A Vorbis TRACKNUMBER of "3/20" is track 3: dhowden/tag reads it as 0. func TestReadTrackAndDisc(t *testing.T) { for _, tc := range []struct { name string diff --git a/internal/library/missing_pairs.go b/internal/library/missing_pairs.go index 0764572c..bcde5643 100644 --- a/internal/library/missing_pairs.go +++ b/internal/library/missing_pairs.go @@ -13,30 +13,29 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// Missing-pair folding (M485). +// Missing-pair folding. // // A track marked missing whose replacement already sits on disk under another // row is folded into that replacement: its likes, plays, playlist entries and -// tags move across and the missing row goes. On the operator's library this was -// 355 of 451 missing tracks (#5274), nearly all quality upgrades — the mp3 -// removed, a flac imported at a new path. +// tags move across and the missing row goes. The common case is a quality +// upgrade — the mp3 removed, a flac imported at a new path. // // Move adoption (moved.go) can't catch these. It links a new file to a missing // row when the file is first inserted, matching on the recording MBID or the -// audio hash. A re-encode never shares the hash, and the replacements arrived -// untagged, so there was nothing to match; AcoustID filled many of the MBIDs -// weeks later, by which time both rows existed. Adoption keeps the old row and -// re-points its path, which only works while the new file has no row of its own. -// Here both do, so this folds instead, with the duplicate merge's row half. +// audio hash. A re-encode never shares the hash, and an untagged replacement +// has nothing to match until AcoustID fills its MBID, by which time both rows +// exist. Adoption keeps the old row and re-points its path, which only works +// while the new file has no row of its own. Here both do, so this folds +// instead, with the duplicate merge's row half. // -// The rules are in ListMissingTrackPairs. Only a pair where each side has exactly -// one candidate is folded: a wrong fold can't be undone, a skipped one is only a -// row left in the missing list. +// The rules are in ListMissingTrackPairs. Only a pair where each side has +// exactly one candidate is folded: a wrong fold can't be undone, a skipped one +// is only a row left in the missing list. // -// Unlike the duplicate merge this runs without review (operator, 2026-10-07): -// no file is touched, the missing row has nothing to play, and adoption, its -// closest relative, is automatic too. Each fold is logged with both paths so a -// wrong pair can be read back. +// Unlike the duplicate merge this runs without review: no file is touched, the +// missing row has nothing to play, and adoption, its closest relative, is +// automatic too. Each fold is logged with both paths so a wrong pair can be +// read back. // PairResult tallies one pass. type PairResult struct { @@ -112,8 +111,8 @@ func PairMissingTracks(ctx context.Context, pool *pgxpool.Pool, logger *slog.Log return res, nil } -// foldMissingPair folds one pair in its own transaction, after locking both rows -// and checking the pair still holds. +// foldMissingPair folds one pair in its own transaction, after locking both +// rows and checking the pair still holds. func foldMissingPair(ctx context.Context, pool *pgxpool.Pool, missingID, presentID pgtype.UUID) (foldResult, error) { var f foldResult err := pgx.BeginFunc(ctx, pool, func(tx pgx.Tx) error { diff --git a/internal/library/missing_pairs_test.go b/internal/library/missing_pairs_test.go index a356f995..7e2b24fc 100644 --- a/internal/library/missing_pairs_test.go +++ b/internal/library/missing_pairs_test.go @@ -73,7 +73,8 @@ func (f pairFixture) album(title, releaseGroup string) dbq.Album { return a } -// track adds a row; mbid "" leaves it without one, and missing marks it missing. +// track adds a row; mbid "" leaves it without one, and missing marks it +// missing. func (f pairFixture) track(album dbq.Album, title, path, mbid string, missing bool) dbq.Track { f.t.Helper() tr, err := f.q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{ @@ -110,7 +111,7 @@ func (f pairFixture) count(sql string, args ...any) int { return n } -// The M485 proof on rows: each rule folds the pair it should, history moves onto +// On rows: each rule folds the pair it should, history moves onto // the replacement, and every shape the pass must leave alone is left alone. func TestPairMissingTracks_Integration(t *testing.T) { pool := newPool(t) @@ -126,13 +127,13 @@ func TestPairMissingTracks_Integration(t *testing.T) { edition := f.album("Pair Album (Deluxe)", "rg-pair") // same release group as albumA other := f.album("Other Album", "rg-other") - // Rule title: the mp3 went, the flac came, no MBID on either, and the title's - // case differs as it does between old and new tags. + // Rule title: the mp3 went, the flac came, no MBID on either, and the + // title's case differs as it does between old and new tags. oldMp3 := f.track(albumA, "Song One", "/m/a/01 - Song One.mp3", "", true) newFlac := f.track(albumA, "song one", "/m/a/01 - song one.flac", "", false) - // Rule mbid: the same recording, retitled and filed under another edition of - // the same release group. + // Rule mbid: the same recording, retitled and filed under another edition + // of the same release group. oldByMbid := f.track(albumA, "Old Name", "/m/a/02 - Old Name.mp3", "rec-2", true) newByMbid := f.track(edition, "New Name", "/m/d/02 - New Name.flac", "rec-2", false) @@ -231,10 +232,9 @@ func TestPairMissingTracks_Integration(t *testing.T) { } } -// The case that made M485: a quality upgrade. The mp3 is removed and a -// re-encode of it arrives at a new path with no recording MBID, so move adoption -// has nothing to match. The scan must still end with one row, carrying the old -// row's history. +// The common case: a quality upgrade. The mp3 is removed and a re-encode of it +// arrives at a new path with no recording MBID, so move adoption has nothing to +// match. The scan must still end with one row, carrying the old row's history. func TestScanner_FoldsUpgradedFile_Integration(t *testing.T) { pool := newPool(t) ctx := context.Background() diff --git a/internal/library/moved.go b/internal/library/moved.go index eb453ccd..55b5eb99 100644 --- a/internal/library/moved.go +++ b/internal/library/moved.go @@ -9,48 +9,46 @@ import ( syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) -// Move detection (#2528). +// Move detection. // // Track identity is file_path: UpsertTrack conflicts on it, and the reconcile // pass in reconcile.go clears a missing mark when the walk sees that same path // again. So a file that comes back exactly where it was restores cleanly, but a -// file that comes back RENAMED or in a different directory looked, to the +// file that comes back RENAMED or in a different directory looks, to the // scanner, like a deletion plus an unrelated new track: // -// - the old row stayed marked missing, holding the like and every play_event -// - a fresh row appeared with no history -// - nothing connected them +// - the old row stays marked missing, holding the like and every play_event +// - a fresh row appears with no history +// - nothing connects them // -// A liked song read as unliked after a retag, its play count reset to zero, and -// Rediscover could offer it as a discovery. All silently. Renumbering an album -// was enough to do it — which is exactly what happened on the operator's copy of -// Minutes to Midnight. +// A liked song would read as unliked after a retag, its play count reset to +// zero, and Rediscover could offer it as a discovery. Renumbering an album is +// enough to do it. // -// The fix adopts the existing row rather than inserting: re-point its file_path -// at the new location and clear the mark. The caller's normal UpsertTrack then -// conflicts on file_path and updates THAT row, so the track id survives and -// likes, plays and playlist memberships travel with it. Clients see an update -// rather than a delete-and-create, so no cache churn either. +// So the scan adopts the existing row rather than inserting: re-point its +// file_path at the new location and clear the mark. The caller's normal +// UpsertTrack then conflicts on file_path and updates THAT row, so the track id +// survives and likes, plays and playlist memberships travel with it. Clients +// see an update rather than a delete-and-create, so no cache churn either. // // Only rows already marked missing are eligible. A row whose file is present // elsewhere is a duplicate, not a move, and re-pointing it would corrupt the // copy that still exists. That constraint is what makes this safe, and the -// marking added in #2523 is what makes it expressible. +// missing mark is what makes it expressible. -// trackAdopter is the slice of dbq.Queries move detection needs, narrowed so the -// match/ambiguity logic can be tested against a fake. +// trackAdopter is the slice of dbq.Queries move detection needs, narrowed so +// the match/ambiguity logic can be tested against a fake. type trackAdopter interface { FindMissingTrackByMbid(ctx context.Context, mbid string) ([]dbq.FindMissingTrackByMbidRow, error) FindMissingTrackByAudioHash(ctx context.Context, audioStreamSha256 []byte) ([]dbq.FindMissingTrackByAudioHashRow, error) AdoptTrackPath(ctx context.Context, arg dbq.AdoptTrackPathParams) (int64, error) } -// adoptMovedTrack looks for a missing row that is the same recording as the file -// at newPath and re-points it there. Reports whether a row was adopted. +// adoptMovedTrack looks for a missing row that is the same recording as the +// file at newPath and re-points it there. Reports whether a row was adopted. // -// Never returns an error: failing to detect a move is a missed optimisation, not -// a broken scan. The caller carries on and inserts a fresh row, which is the -// pre-#2528 behaviour. +// Never returns an error: failing to detect a move is a missed optimisation, +// not a broken scan. The caller carries on and inserts a fresh row. func (s *Scanner) adoptMovedTrack( ctx context.Context, q trackAdopter, newPath string, audioHash []byte, recordingMBID string, @@ -67,10 +65,10 @@ func (s *Scanner) adoptMovedTrack( } } - // Audio-hash fallback for untagged files (#3914): the encoded audio itself, - // which a rename, a move or a retag leaves unchanged. Absent when the hash - // could not be taken, and then nothing is matched — an empty hash must never - // be looked up, or every unhashable file would pair with every other. + // Audio-hash fallback for untagged files: the encoded audio itself, which a + // rename, a move or a retag leaves unchanged. Absent when the hash could + // not be taken, and then nothing is matched — an empty hash must never be + // looked up, or every unhashable file would pair with every other. if len(audioHash) > 0 { rows, err := q.FindMissingTrackByAudioHash(ctx, audioHash) if err != nil { @@ -84,7 +82,8 @@ func (s *Scanner) adoptMovedTrack( return false } -// candidate is the shared shape of both lookups, so uniqueMatch is written once. +// candidate is the shared shape of both lookups, so uniqueMatch is written +// once. type candidate struct { id pgtype.UUID filePath string @@ -107,9 +106,10 @@ func rowsFromAudioHash(rows []dbq.FindMissingTrackByAudioHashRow) []candidate { } // uniqueMatch requires exactly one candidate. Adopting an arbitrary row out of -// several would attach this file's future history to a coin flip, which is worse -// than starting a fresh row — a fork is recoverable later, a wrong merge isn't. -// Libraries with genuine duplicates hit this, so it's logged rather than silent. +// several would attach this file's future history to a coin flip, which is +// worse than starting a fresh row — a fork is recoverable later, a wrong merge +// isn't. Libraries with genuine duplicates hit this, so it's logged rather than +// silent. func (s *Scanner) uniqueMatch( cands []candidate, newPath, via string, ) (candidate, bool) { @@ -130,8 +130,9 @@ func (s *Scanner) adopt( ) bool { n, err := q.AdoptTrackPath(ctx, dbq.AdoptTrackPathParams{ID: c.id, FilePath: newPath}) if err != nil { - // A unique violation on file_path means something else claimed this path - // first. Fall through to a normal insert rather than failing the file. + // A unique violation on file_path means something else claimed this + // path first. Fall through to a normal insert rather than failing the + // file. s.logger.Warn("library scan: adopting moved track failed", "path", newPath, "via", via, "err", err) return false @@ -141,7 +142,7 @@ func (s *Scanner) adopt( // update, so its mark was already cleared. return false } - // Logged with both paths: this is the operator's only window onto a + // Logged with both paths: this is an admin's only window onto a // reorganisation being understood as a move rather than a new track. s.logger.Info("library scan: track moved, history preserved", "from", c.filePath, "to", newPath, "via", via, "track_id", syncpkg.FormatUUID(c.id)) diff --git a/internal/library/moved_test.go b/internal/library/moved_test.go index 561dfff2..0c32383a 100644 --- a/internal/library/moved_test.go +++ b/internal/library/moved_test.go @@ -12,8 +12,8 @@ import ( ) // fakeAdopter answers the audio-hash lookup only for the hash it holds, the way -// the real query does. A fake that returned its rows for any hash could not tell -// adoption by identity apart from adoption by coincidence. +// the real query does. A fake that returned its rows for any hash could not +// tell adoption by identity apart from adoption by coincidence. type fakeAdopter struct { byMbid []dbq.FindMissingTrackByMbidRow hash []byte @@ -107,10 +107,9 @@ func TestAdoptMovedTrack_FallsBackToAudioHash(t *testing.T) { } } -// The case the old (file_size, duration_ms) pair got wrong: an unrelated file -// that happens to share a size and a duration with a missing track. Size and -// duration are no longer inputs at all; only the audio itself can match, and a -// different recording has a different hash. +// An unrelated file that happens to share a size and a duration with a missing +// track is not adopted. Size and duration are not inputs at all; only the +// audio itself can match, and a different recording has a different hash. func TestAdoptMovedTrack_DifferentAudioIsNotAdopted(t *testing.T) { s := testScanner(t) q := &fakeAdopter{hash: audioHash(0xAA), byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(1, oldPath)}, adoptRows: 1} @@ -209,8 +208,9 @@ func TestAdoptMovedTrack_NoCandidates(t *testing.T) { } } -// The row's mark was cleared between lookup and update — another file adopted it -// first. AdoptTrackPath's `missing_since IS NOT NULL` predicate reports 0 rows. +// The row's mark was cleared between lookup and update — another file adopted +// it first. AdoptTrackPath's `missing_since IS NOT NULL` predicate reports 0 +// rows. func TestAdoptMovedTrack_LostRaceReportsNotAdopted(t *testing.T) { s := testScanner(t) q := &fakeAdopter{ @@ -224,7 +224,7 @@ func TestAdoptMovedTrack_LostRaceReportsNotAdopted(t *testing.T) { } // Failing to detect a move must never fail the file: the caller falls back to -// inserting a fresh row, which is the pre-#2528 behaviour. +// inserting a fresh row. func TestAdoptMovedTrack_ToleratesQueryErrors(t *testing.T) { sentinel := errors.New("db down") tests := []struct { diff --git a/internal/library/mp4genre.go b/internal/library/mp4genre.go index d2096e12..94204997 100644 --- a/internal/library/mp4genre.go +++ b/internal/library/mp4genre.go @@ -7,15 +7,15 @@ import ( "strings" ) -// MP4 has the same gap as Vorbis comments (#2500). A tag writer stores several -// genres as several "data" atoms inside one ©gen atom (mutagen and Picard do), -// or occasionally as repeated ©gen atoms. dhowden/tag keeps atoms in a map keyed +// MP4 has the same gap as Vorbis comments. A tag writer stores several genres +// as several "data" atoms inside one ©gen atom (mutagen and Picard do), or +// occasionally as repeated ©gen atoms. dhowden/tag keeps atoms in a map keyed // by name (mp4.go: m.data[name] = data), so one value survives either way. // // Path to the values: moov > udta > meta > ilst > ©gen > data. A file that -// carries only the numeric "gnre" atom has no ©gen; this reader returns -// (nil, nil) for it, and extractGenres falls back to dhowden/tag, which resolves -// gnre to a name. +// carries only the numeric "gnre" atom has no ©gen; this reader returns (nil, +// nil) for it, and extractGenres falls back to dhowden/tag, which resolves gnre +// to a name. // maxMP4GenreAtomSize caps how much of one ©gen atom is buffered. Genre text is // bytes; the cap only stops a corrupt size from making the scanner allocate. diff --git a/internal/library/normalization_prefs.go b/internal/library/normalization_prefs.go index 22e68cc3..2a1b1585 100644 --- a/internal/library/normalization_prefs.go +++ b/internal/library/normalization_prefs.go @@ -12,10 +12,9 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// NormalizationPrefs is one user's loudness-normalization preference (M464 -// #4998). It lives on the server so every player applies the same one: the -// web player and the Android app read it, and the leveled stream for -// UPnP/Sonos (#5001) renders with it. +// NormalizationPrefs is one user's loudness-normalization preference. It lives +// on the server so every player applies the same one: the web player and the +// Android app read it, and the leveled stream for UPnP/Sonos renders with it. type NormalizationPrefs struct { // Mode is off, auto, track or album. Auto uses album gain while an album // plays in order and track gain otherwise. diff --git a/internal/library/notify.go b/internal/library/notify.go index b66c3afe..cee533dd 100644 --- a/internal/library/notify.go +++ b/internal/library/notify.go @@ -10,7 +10,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/notifications" ) -// Package-level notifier for library-health notices to admins (M489): a +// Package-level notifier for library-health notices to admins: a // scan that failed, tracks gone missing, new duplicates. Set once at startup // beside SetEventBus, for the same reason the bus is package-level. Nil, as in // tests that never set it, means nobody is told. @@ -49,9 +49,9 @@ func TrackLabel(artist, title string) string { } // notifyStillMissing tells admins which of the tracks a scan marked missing -// are still gone after it adopted moves and folded replacements (M542). A -// marked track the scan healed is not news: reporting it is what made a -// quality upgrade read as lost music. +// are still gone after it adopted moves and folded replacements. A marked +// track the scan healed is not news: reporting it would make a quality +// upgrade read as lost music. func (s *Scanner) notifyStillMissing(ctx context.Context, marked []pgtype.UUID) { if len(marked) == 0 || ctx.Err() != nil { return diff --git a/internal/library/notify_test.go b/internal/library/notify_test.go index 9308f95f..1885f878 100644 --- a/internal/library/notify_test.go +++ b/internal/library/notify_test.go @@ -87,7 +87,7 @@ func TestNotifyScanFinished_FailuresCoalesceAndShutdownIsSilent(t *testing.T) { } } -// M542: reconcile only records what it marked. The scan tells admins after +// Reconcile only records what it marked. The scan tells admins after // it has adopted moves and folded replacements, so a file it marked and then // healed is never reported as lost. func TestReconcileMissing_RecordsTheMarksWithoutNotifying(t *testing.T) { diff --git a/internal/library/reconcile.go b/internal/library/reconcile.go index 6cc273ef..18e05b9b 100644 --- a/internal/library/reconcile.go +++ b/internal/library/reconcile.go @@ -22,14 +22,13 @@ type trackReconciler interface { InsertLibraryChanges(ctx context.Context, arg dbq.InsertLibraryChangesParams) error } -// Reconcile marks tracks whose files have disappeared (#2523). +// Reconcile marks tracks whose files have disappeared. // -// Why this exists: nothing in Minstrel used to notice a deleted file. The walk -// only visits paths that exist, so a row whose file is gone was never scanned, -// never errored, never counted — permanently invisible. The watcher ignores -// removals by design (see classifyEvent), and the safety-net scan is the same -// walk, so it covers additions only. Rows accumulated forever, kept being -// offered to recommendations, and failed at playback. +// The walk only visits paths that exist, so without this a row whose file is +// gone would never be scanned, never error, never be counted. The watcher +// ignores removals by design (see classifyEvent), and the safety-net scan is +// the same walk, so it covers additions only. Unmarked, such rows would keep +// being offered to recommendations and fail at playback. // // Why it MARKS rather than deletes: a missing file is a claim about the // filesystem, and the filesystem lies transiently — an unmounted volume, a @@ -62,9 +61,9 @@ func (s *Scanner) reconcileMissing( if err := s.verifyRootsPresent(); err != nil { return err } - // Roots resolved but the walk found nothing. Either the library is genuinely - // empty — in which case there is nothing to reconcile — or the mount is - // hollow. Both mean: don't act. + // Roots resolved but the walk found nothing. Either the library is + // genuinely empty — in which case there is nothing to reconcile — or the + // mount is hollow. Both mean: don't act. if len(seen) == 0 { return errors.New("walk saw no audio files; refusing to reconcile") } @@ -123,9 +122,8 @@ func (s *Scanner) reconcileMissing( return fmt.Errorf("mark tracks missing: %w", err) } stats.Missing = int(n) - // Warn, not Info: every one of these is a library entry the operator - // probably didn't intend to lose, and the only place it surfaces today is - // this line. + // Warn, not Info: every one of these is a library entry an admin probably + // didn't intend to lose. s.logger.Warn("library scan: tracks marked missing (files not found)", "count", n, "library_total", len(rows)) // Admins are told after the scan has adopted moves and folded @@ -135,13 +133,13 @@ func (s *Scanner) reconcileMissing( } // logTrackChanges tells the delta sync that these tracks changed, so clients -// pick up the missing mark (#2704). +// pick up the missing mark. // -// Without this the mark was invisible to every client: MarkTracksMissing and -// ClearTracksMissing are plain UPDATEs, and /api/library/sync is a change-log -// feed — a row that never produces a change is never re-sent, so a client -// would keep its stale copy until an unrelated edit touched the track or the -// cursor fell out of the retention window. +// Without this the mark would be invisible to every client: MarkTracksMissing +// and ClearTracksMissing are plain UPDATEs, and /api/library/sync is a +// change-log feed — a row that never produces a change is never re-sent, so a +// client would keep its stale copy until an unrelated edit touched the track or +// the cursor fell out of the retention window. // // Logged BEFORE the mutation, deliberately, which is the opposite of the // scanner's log-after-success pattern. The two failure modes are not @@ -161,7 +159,7 @@ func logTrackChanges(ctx context.Context, q trackReconciler, ids []pgtype.UUID) } // OpUpsert, not OpDelete: the track still exists and keeps its history — // only its playability changed. A delete would tell clients to drop the - // row, which is the behaviour #2704 deliberately rejected. + // row, and a missing file is expected to come back. return q.InsertLibraryChanges(ctx, dbq.InsertLibraryChangesParams{ EntityType: string(syncpkg.EntityTrack), EntityIds: strIDs, @@ -169,11 +167,11 @@ func logTrackChanges(ctx context.Context, q trackReconciler, ids []pgtype.UUID) }) } -// verifyRootsPresent is the first and most important guard. If a configured root -// doesn't resolve to a readable directory, the walk beneath it found nothing and -// every row under it would look deleted. An unmounted media volume is the -// obvious case, and it is common enough — a container restart racing its volume -// mount does exactly this. +// verifyRootsPresent is the first and most important guard. If a configured +// root doesn't resolve to a readable directory, the walk beneath it found +// nothing and every row under it would look deleted. An unmounted media volume +// is the obvious case, and it is common enough — a container restart racing its +// volume mount does exactly this. func (s *Scanner) verifyRootsPresent() error { roots := s.Roots() if len(roots) == 0 { diff --git a/internal/library/reconcile_test.go b/internal/library/reconcile_test.go index 55cb8e45..5d043565 100644 --- a/internal/library/reconcile_test.go +++ b/internal/library/reconcile_test.go @@ -26,7 +26,7 @@ type fakeReconciler struct { listErr error markErr error clearErr error - // Change rows the reconcile pass asked the delta sync to emit (#2704). + // Change rows the reconcile pass asked the delta sync to emit. loggedChanges []dbq.InsertLibraryChangesParams } @@ -186,8 +186,8 @@ func TestReconcileMissing_DoesNotRemarkAlreadyMissingRows(t *testing.T) { } // The guard that matters most. A half-attached mount makes the walk succeed -// while seeing only part of the library — evidence indistinguishable from a mass -// deletion, so reconcile must refuse rather than guess. +// while seeing only part of the library — evidence indistinguishable from a +// mass deletion, so reconcile must refuse rather than guess. func TestReconcileMissing_RefusesWhenTooMuchWouldBeMarked(t *testing.T) { root := populatedRoot(t) s := testScanner(t, root) @@ -288,8 +288,8 @@ func TestReconcileMissing_RefusesWhenRootEmpty(t *testing.T) { } } -// Several roots, one detached. Marking must not proceed on partial evidence just -// because the other roots looked fine. +// Several roots, one detached. Marking must not proceed on partial evidence +// just because the other roots looked fine. func TestReconcileMissing_RefusesWhenAnyRootMissing(t *testing.T) { good := populatedRoot(t) s := testScanner(t, good, filepath.Join(t.TempDir(), "detached")) @@ -336,8 +336,8 @@ func TestVerifyRootsPresent_NoRootsConfigured(t *testing.T) { // Without a change row the mark is invisible to every client: // /api/library/sync is a change-log feed, so a plain UPDATE never reaches -// anyone. This was the gap that made #2704's wire field inert — the flag -// existed and nothing ever told a client to re-read the track. +// anyone, and the wire field would exist with nothing ever telling a client to +// re-read the track. func TestReconcileMissing_MarkingEmitsSyncChanges(t *testing.T) { s := testScanner(t, populatedRoot(t)) @@ -366,8 +366,8 @@ func TestReconcileMissing_MarkingEmitsSyncChanges(t *testing.T) { t.Errorf("entity_type = %q, want track", got.EntityType) } // Upsert, not delete: the track still exists and keeps its history. A - // delete would tell clients to drop the row, which is the behaviour - // #2704 deliberately rejected. + // delete would tell clients to drop the row, and a missing file is + // expected to come back. if got.Op != "upsert" { t.Errorf("op = %q, want upsert — the row survives, only its state changed", got.Op) } diff --git a/internal/library/replace.go b/internal/library/replace.go index 80fd2a1f..344b03d6 100644 --- a/internal/library/replace.go +++ b/internal/library/replace.go @@ -15,13 +15,13 @@ import ( syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) -// Replacing a track with a better copy (M502 #5524). +// Replacing a track with a better copy. // // An upgrade files the better copy first, as a track of its own, and only // then replaces the old one: its file is removed, then its likes, plays, // playlist entries and tags move onto the new row and the old row goes. It // is the duplicate merge with the survivor chosen by the import, and it -// keeps the merge's order (#3918): if the old file cannot be removed nothing +// keeps the merge's order: if the old file cannot be removed nothing // in the database changes, and the next scan finds the two copies as a // duplicate pair. diff --git a/internal/library/replaygain.go b/internal/library/replaygain.go index 8c98ae51..04e7af35 100644 --- a/internal/library/replaygain.go +++ b/internal/library/replaygain.go @@ -9,7 +9,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// ReplayGain delivery (M464 #4997). +// ReplayGain delivery. // // Clients are handed loudness in the ReplayGain 2.0 form every player already // understands: a gain in dB that brings the track (or album) to the RG2 diff --git a/internal/library/scanner.go b/internal/library/scanner.go index 170d74b1..f2cff895 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -28,8 +28,7 @@ import ( // `internal/api/media.go` MIME detection — the stream handler must be // able to serve every extension the scanner indexes, and there is no // point in adding extensions to the stream handler that the scanner -// will silently skip. Drift #571 caught the divergence after .opus, -// .aac, and .wav were added to media.go but not here. +// will silently skip. var audioExtensions = map[string]bool{ ".mp3": true, ".m4a": true, @@ -43,7 +42,7 @@ var audioExtensions = map[string]bool{ // tagReadVersion is the version of this package's tag-extraction logic. Rows // whose tracks.tag_read_version is lower get their tags re-read on the next // scan even when the file itself hasn't changed, so a fix reaches an existing -// library without the operator rebuilding it (migration 0054). +// library without anyone rebuilding it (migration 0054). // // Bump this whenever a change to tag extraction should reach already-indexed // files, and say why below. @@ -52,35 +51,33 @@ var audioExtensions = map[string]bool{ // dhowden/tag welds null-separated multi-values into one token // ("Alternative Rock" + "Rock" -> "Alternative RockRock"), which corrupted // the genre browse axis and polluted the taste profile's tag vocabulary, -// and left bare ID3v1 numeric references unresolved (#2499). -// 2: acronym and apostrophe casing repaired on genre values (#2468) — "Edm" -> +// and left bare ID3v1 numeric references unresolved. +// 2: acronym and apostrophe casing repaired on genre values — "Edm" -> // "EDM", "Children'S Music" -> "Children's Music". Bumped rather than left // to new files only because taste_profile.sql reads tracks.genre directly, // so a half-repaired library would carry both spellings as separate tags. -// 3: the MusicBrainz release-group id is read into albums.release_group_mbid -// (#5241). Wants name albums by release group, not by the release id +// 3: the MusicBrainz release-group id is read into albums.release_group_mbid. +// Wants name albums by release group, not by the release id // albums.mbid holds, so re-acquisition and request completion need it for // the albums already in the library, not only for files added from now on. // 4: every value of a repeated genre field is read for FLAC, Ogg Vorbis, Opus -// and MP4 (#2500). dhowden/tag kept only the last, and 3,658 of the -// operator's 4,092 FLACs declare more than one, so most of their genres -// were never stored. -// 5: the recording id is read from FLAC, Ogg Vorbis and Opus (#5637). Picard +// and MP4. dhowden/tag kept only the last, and most FLACs that carry +// genres declare more than one. +// 5: the recording id is read from FLAC, Ogg Vorbis and Opus. Picard // calls it MUSICBRAINZ_TRACKID there and dhowden/tag files that under the // release-track name, so every Vorbis-tagged track came in without its // tag id and waited on an AcoustID lookup. A re-read stores the tag id, -// over a looked-up one (D4). -// 6: a Vorbis TRACKNUMBER or DISCNUMBER written as "3/20" is read (#6171). +// over a looked-up one. +// 6: a Vorbis TRACKNUMBER or DISCNUMBER written as "3/20" is read. // dhowden/tag reads it as 0, and Minstrel's own importer wrote it that -// way, so all 245 FLAC tracks it imported came in with no track or disc -// number and their albums lost their order. +// way, so the tracks it imported came in with no track or disc number. // 7: an album of the same name in another release group gets its own row -// (resolveAlbum). Weezer's six self-titled albums were one 118-track row, -// so their merges, wants and upgrades all counted as one album; a re-read -// files each existing track under its own album. +// (resolveAlbum). Self-titled albums by one artist shared a row, so their +// merges, wants and upgrades all counted as one album; a re-read files +// each existing track under its own album. // 8: a file tagged with no release group joins the same-named album that // already holds a track from its folder. Under 7 it took the oldest row, -// which left three of Weezer's 1994 bonus tracks on the 2016 album. +// which could file a bonus track on a later album of the same name. const tagReadVersion int16 = 8 type Stats struct { @@ -89,31 +86,31 @@ type Stats struct { Updated int `json:"updated"` Skipped int `json:"skipped"` Errored int `json:"errored"` - // Missing / Restored come from the reconcile pass, not the walk (#2523): + // Missing / Restored come from the reconcile pass, not the walk: // rows whose file the walk didn't find, and rows whose file came back. // Only a full Scan sets these — see reconcileMissing. Missing int `json:"missing"` Restored int `json:"restored"` // marked are the tracks this scan's reconcile marked missing, so the scan // can tell admins which of them are still gone once it has adopted moves - // and folded replacements (M542). + // and folded replacements. marked []pgtype.UUID } type Scanner struct { pool *pgxpool.Pool logger *slog.Logger - // paths are the library's folders, changed by SetRoots (#6129). Read them + // paths are the library's folders, changed by SetRoots. Read them // through Roots. paths []string rootsMu sync.RWMutex rootsChanged chan struct{} - // fingerprint derives a file's acoustic identity (M400). A field so an + // fingerprint derives a file's acoustic identity. 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, opts fingerprintOptions) fingerprintResult - // settings is the operator's fingerprinting policy (#3913), shared with the + // settings is the fingerprinting policy, shared with the // workers and the admin API. Nil fingerprints with the defaults. settings *FingerprintSettingsService } @@ -128,7 +125,7 @@ func New(pool *pgxpool.Pool, logger *slog.Logger, paths []string, settings *Fing // are logged + counted; the scan keeps going. // // It then reconciles: rows whose file the walk never saw get marked missing, -// and rows whose file has come back get un-marked (#2523). Only a FULL scan may +// and rows whose file has come back get un-marked. Only a FULL scan may // do this — the walk's set of seen paths is the evidence, and a partial // (watcher-driven) scan has no basis for concluding anything about files it // didn't look at. That's why ScanFiles does not reconcile. @@ -141,23 +138,24 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro start := time.Now() // PHASE 1 — enumerate. Collect every audio path without touching tags or - // ffprobe. Cheap: WalkDir already stats each entry, so this adds a directory - // traversal and nothing else. + // ffprobe. Cheap: WalkDir already stats each entry, so this adds a + // directory traversal and nothing else. // // The order matters and is the whole reason enumeration is separate. // Reconcile has to mark disappeared rows BEFORE any file is processed, - // because move detection (#2528) can only adopt a row that is already marked - // missing. A rename performed while the server was down surfaces the deletion - // and the addition in the SAME scan — so if reconcile ran at the end, the new - // path would insert a fresh row first and the fork would be permanent. + // because move detection can only adopt a row that is already marked + // missing. A rename performed while the server was down surfaces the + // deletion and the addition in the SAME scan — so if reconcile ran at the + // end, the new path would insert a fresh row first and the fork would be + // permanent. paths, walkErrs := s.enumerate(ctx, progressCb, &stats) stats.Errored += walkErrs if err := ctx.Err(); err != nil { return stats, err } - // PHASE 2 — reconcile. Only ever on a COMPLETE enumeration: a cancelled walk - // has a partial view and would mark everything it hadn't reached. + // PHASE 2 — reconcile. Only ever on a COMPLETE enumeration: a cancelled + // walk has a partial view and would mark everything it hadn't reached. seen := make(map[string]struct{}, len(paths)) for _, p := range paths { seen[p] = struct{}{} @@ -167,10 +165,9 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro // evidence, and that refusal arrives here as an error. // // The consequence is named explicitly because it is not obvious: move - // detection (#2528) can only adopt a row that is already marked missing, - // so a refused reconcile also means renamed files insert fresh rows and - // fork their history. That's the pre-#2528 behaviour rather than a new - // failure, but it's worth knowing which scan it happened on. It bites + // detection can only adopt a row that is already marked missing, so a + // refused reconcile also means renamed files insert fresh rows and fork + // their history. It's worth knowing which scan it happened on. It bites // hardest when a large fraction of a small library is reorganised at // once, which trips the mark cap. s.logger.Warn("library scan: reconcile skipped — moved files will fork rather than adopt", @@ -207,7 +204,7 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro } // PHASE 4 — fold missing rows into replacements this scan brought in or - // that were already here (M485). After processing, so both this scan's + // that were already here. After processing, so both this scan's // marks and its arrivals are in. Not fatal: an unfolded pair is only a row // left in the missing list, and the next scan tries again. if _, err := PairMissingTracks(ctx, s.pool, s.logger); err != nil && ctx.Err() == nil { @@ -222,18 +219,18 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro // enumerate walks every configured root and returns the audio paths found, in // walk order, plus a count of walk errors. // -// A path is recorded even if it will later fail to parse: an unreadable file is a -// broken file, not a missing one, and letting reconcile mark it missing would -// hide it from the operator behind the wrong explanation. +// A path is recorded even if it will later fail to parse: an unreadable file is +// a broken file, not a missing one, and letting reconcile mark it missing would +// hide it from an admin behind the wrong explanation. func (s *Scanner) enumerate( ctx context.Context, progressCb func(Stats), stats *Stats, ) ([]string, int) { paths := make([]string, 0, 8192) errs := 0 for _, root := range s.Roots() { - // WalkDir's own error return is folded into the per-entry handler below, - // so a bad root is counted rather than aborting the whole scan — one - // unreadable root shouldn't discard the others' results. + // WalkDir's own error return is folded into the per-entry handler + // below, so a bad root is counted rather than aborting the whole scan — + // one unreadable root shouldn't discard the others' results. _ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { if ctx.Err() != nil { return fs.SkipAll @@ -280,11 +277,10 @@ func (s *Scanner) scanFile( } // Incremental skip: only when the file hasn't changed AND we already have a // real duration AND the row's tag-derived columns were written by the - // current extraction logic. The duration clause lets older scans that - // recorded duration_ms=0 (before ffprobe was wired) get backfilled without - // forcing the operator to wipe the library; the tag-version clause does the - // same job for tag-extraction fixes (#2499). Once both are current, - // subsequent scans short-circuit as before. + // current extraction logic. The duration clause backfills rows that + // recorded duration_ms=0 without anyone wiping the library; the + // tag-version clause does the same job for tag-extraction fixes. Once both + // are current, subsequent scans short-circuit. unchanged := knownTrack && !existing.UpdatedAt.Time.Before(mtime) if unchanged && existing.DurationMs > 0 && existing.TagReadVersion >= tagReadVersion { stats.Skipped++ @@ -354,15 +350,15 @@ func (s *Scanner) scanFile( // 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), + // An unchanged file with NO fingerprint yet is the backfill's job, // 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. + // Computed before move adoption so adoption can match on the audio hash; + // stored after the upsert, once the row id is known. // - // With fingerprinting switched off (#3913) the scan decodes nothing, but it + // With fingerprinting switched off the scan decodes nothing, but it // still takes the stream hash: a demux rather than a decode, and the only // thing that recognises a moved file with no MBID. var fp fingerprintResult @@ -376,13 +372,13 @@ func (s *Scanner) scanFile( } // 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 + // moved or was renamed. Adopting re-points the existing row at this // path and clears its missing mark, so the UpsertTrack below conflicts on // file_path and updates THAT row: same track id, likes and play history // intact. Without this, renumbering an album forks every track on it. // - // Runs after fingerprinting because, for a file with no MBID, adoption matches - // on its audio hash (#3914), and only for genuinely unknown paths — a known + // Runs after fingerprinting because, for a file with no MBID, adoption + // matches on its audio hash, and only for genuinely unknown paths — a known // path is already the row we're going to update. if !knownTrack { if s.adoptMovedTrack(ctx, q, path, fp.streamSHA256, recordingMBID) { @@ -444,7 +440,8 @@ func (s *Scanner) scanFile( storeFingerprint(ctx, q, s.logger, track.ID, path, fp, fpCfg.ChromaprintLengthSec) } else if err := q.DeleteTrackFingerprint(ctx, track.ID); err != nil { // Off, nothing is stored — but a row describing the previous bytes - // must not outlive them, or the sweep would compare audio that is gone. + // must not outlive them, or the sweep would compare audio that is + // gone. s.logger.Warn("fingerprint: clearing stale fingerprint failed", "path", path, "err", err) } // Loudness is measured by its own worker, never inline: it decodes the @@ -454,7 +451,7 @@ func (s *Scanner) scanFile( if err := q.DeleteTrackLoudness(ctx, track.ID); err != nil { s.logger.Warn("loudness: clearing stale measurement failed", "path", path, "err", err) } - // Likewise an AcoustID lookup (M401): the worker looks the new bytes + // Likewise an AcoustID lookup: the worker looks the new bytes // up again. A looked-up MBID stays meanwhile and is taken back only if // the new lookup does not match it. if err := q.DeleteAcoustIDLookup(ctx, track.ID); err != nil { @@ -507,7 +504,7 @@ func (s *Scanner) ScanFiles(ctx context.Context, paths []string) ([]pgtype.UUID, "errored", stats.Errored, ) // A file the watcher just brought in may be the replacement for a row - // an earlier scan marked missing (M485). + // an earlier scan marked missing. if _, err := PairMissingTracks(ctx, s.pool, s.logger); err != nil && ctx.Err() == nil { s.logger.Warn("library watch: missing-pair pass failed", "err", err) } @@ -528,12 +525,11 @@ func (s *Scanner) resolveArtist(ctx context.Context, q *dbq.Queries, name, mbid if isUniqueViolation(uerr) { // Another artist row already owns this MBID — two rows that // should be merged (usually two spellings of one name). - // Expected, not a fault: leave NULL and let the operator - // merge. Mirrors resolveAlbum, which has always handled it - // this way — without this branch the identical benign - // condition logged a generic warning plus a Postgres ERROR - // line on every scan, which teaches an operator to ignore - // database errors (#2524). + // Expected, not a fault: leave NULL and let an admin + // merge. Mirrors resolveAlbum. Logged at Info, because a + // warning plus a Postgres ERROR line on every scan for a + // benign condition teaches an admin to ignore database + // errors. s.logger.Info("library scan: duplicate artist mbid (canonical row already owns it)", "artist_id", existing.ID, "artist", name, "mbid", mbid) } else { diff --git a/internal/library/scanner_test.go b/internal/library/scanner_test.go index 18155eb8..fbceb4dc 100644 --- a/internal/library/scanner_test.go +++ b/internal/library/scanner_test.go @@ -148,8 +148,8 @@ func TestScanner_Integration(t *testing.T) { t.Fatalf("GetTrackByPath after ScanFiles: %v", err) } - // Re-running ScanFiles on the now-unchanged file (duration probed) skips it, - // so no album is reported as changed. + // Re-running ScanFiles on the now-unchanged file (duration probed) skips + // it, so no album is reported as changed. if _, err := pool.Exec(ctx, "UPDATE tracks SET duration_ms = 1000 WHERE duration_ms = 0"); err != nil { t.Fatalf("seed durations (2): %v", err) } @@ -206,14 +206,14 @@ func writeTestMP3(t *testing.T, path string, frames map[string]string) { } } -// TestScanner_AdoptsMovedFile_Integration is the #2528 proof: a renamed file +// TestScanner_AdoptsMovedFile_Integration: a renamed file // must keep its existing tracks row — same id, so likes, play history and // playlist memberships travel with it — rather than forking into a marked ghost // plus a fresh zero-history row. // -// Uses the MBID path. The synthetic MP3s here carry no real audio, so their audio -// stream hash cannot be relied on — which is why the recording MBID is the signal -// under test. The audio-hash path is covered by moved_test.go. +// Uses the MBID path. The synthetic MP3s here carry no real audio, so their +// audio stream hash cannot be relied on — which is why the recording MBID is +// the signal under test. The audio-hash path is covered by moved_test.go. // // Eight tracks with one rename keeps the marked fraction at 12.5%, under // missingMarkMaxFraction. That is load-bearing: if the rename exceeded the cap, @@ -309,7 +309,7 @@ func TestScanner_AdoptsMovedFile_Integration(t *testing.T) { } } -// TestScanner_ReleaseGroupMBID_Integration is #5241's scan half: a new album +// TestScanner_ReleaseGroupMBID_Integration: a new album // takes its release-group id from the tag, and an album indexed before this // read existed gets it on the next scan, through the tagReadVersion bump, // without its file changing. @@ -387,7 +387,7 @@ func TestScanner_ReleaseGroupMBID_Integration(t *testing.T) { } // Two albums of one name by one artist, in different release groups, are two -// albums: Weezer's six self-titled albums shared one row until 2026-10-20. A +// albums, as Weezer's self-titled albums are. A // track of the same group joins its album, and a track filed under the other // album's row by the old lookup moves to its own on the tagReadVersion re-read. func TestScanner_SameTitledAlbumsOfTwoGroupsAreTwoAlbums_Integration(t *testing.T) { diff --git a/internal/library/scanrun.go b/internal/library/scanrun.go index c56c5f9c..752b8bd2 100644 --- a/internal/library/scanrun.go +++ b/internal/library/scanrun.go @@ -24,8 +24,8 @@ type LibraryStageTallies struct { Updated int `json:"updated"` Skipped int `json:"skipped"` Errored int `json:"errored"` - // Reconcile results (#2523). Surfaced in the scan record because a track - // disappearing from the library is something the operator should be able to + // Reconcile results. Surfaced in the scan record because a track + // disappearing from the library is something an admin should be able to // see happened, rather than discovering it when a mix comes up short. Missing int `json:"missing"` Restored int `json:"restored"` @@ -38,7 +38,8 @@ func (st Stats) tallies() LibraryStageTallies { } } -// MBIDBackfillStageTallies wires BackfillMBIDsResult into the scan_runs jsonb column. +// MBIDBackfillStageTallies wires BackfillMBIDsResult into the scan_runs jsonb +// column. type MBIDBackfillStageTallies struct { Processed int `json:"processed"` Healed int `json:"healed"` @@ -46,7 +47,8 @@ type MBIDBackfillStageTallies struct { Duplicates int `json:"duplicates"` } -// CoverEnrichStageTallies wires EnrichBatch tallies into the scan_runs jsonb column. +// CoverEnrichStageTallies wires EnrichBatch tallies into the scan_runs jsonb +// column. type CoverEnrichStageTallies struct { Processed int `json:"processed"` Succeeded int `json:"succeeded"` @@ -64,13 +66,12 @@ type ArtistArtEnrichStageTallies struct { // RunScanConfig assembles the knobs for the orchestrator. BackfillCap is // the album/artist-MBID-backfill batch cap (5000); EnrichCap / // ArtistEnrichCap gate cover/artist-art enrichment (0 = off, <0 = -// unbounded — #388 removed the global cover-art cap; remote providers -// self-throttle per provider). +// unbounded; remote providers pace themselves). type RunScanConfig struct { BackfillCap int EnrichCap int - ArtistEnrichCap int // M7 cover-sources: cap for the artist-art enrich stage - DataDir string // M7 cover-sources: where to write artist-art files + ArtistEnrichCap int // cap for the artist-art enrich stage + DataDir string // where to write artist-art files } // RunScan is the unified scan-chain orchestrator. Creates a scan_runs row, @@ -235,7 +236,8 @@ func RunScan( } } - // Finalize: set finished_at + error message (or empty string if all stages succeeded). + // Finalize: set finished_at + error message (or empty string if all stages + // succeeded). errMsg := "" if firstErr != nil { errMsg = firstErr.Error() @@ -243,7 +245,8 @@ func RunScan( if ferr := q.FinishScanRun(ctx, dbq.FinishScanRunParams{ ID: row.ID, Column2: errMsg, }); ferr != nil { - // Even the finalize failed; log loudly, return the original error if any. + // Even the finalize failed; log loudly, return the original error if + // any. logger.Error("scan run finalize failed", "id", row.ID, "err", ferr) if firstErr != nil { return row.ID, firstErr diff --git a/internal/library/song_link_test.go b/internal/library/song_link_test.go index ac3f630e..1c75cbe2 100644 --- a/internal/library/song_link_test.go +++ b/internal/library/song_link_test.go @@ -151,7 +151,7 @@ func TestCrossReleaseCopiesCountAsOneSong_Integration(t *testing.T) { } } -// Without the operator's permission the resolver links nothing. +// With auto-resolve off the resolver links nothing. func TestCrossReleaseLinkNeedsAutoResolve_Integration(t *testing.T) { pool := newPool(t) ctx := context.Background() @@ -222,7 +222,7 @@ func songKeys(t *testing.T, q *dbq.Queries, ids ...pgtype.UUID) []pgtype.UUID { } // A cross-release group whose album holds the song twice (Humanz: Saturnz -// Barz as a FLAC and a YouTube mp3, beside the single, #5401): the album's +// Barz as a FLAC and a YouTube mp3, beside the single): the album's // extra copy is merged into its FLAC, the single stays, and the two copies // left are linked as one song. func TestResolveDuplicates_FoldsAnAlbumsExtraCopyInACrossReleaseGroup_Integration(t *testing.T) { diff --git a/internal/library/source_markers.go b/internal/library/source_markers.go index 0bc5a825..dec5ecf1 100644 --- a/internal/library/source_markers.go +++ b/internal/library/source_markers.go @@ -8,7 +8,7 @@ import ( // sourceMarker is one sign that a file was ripped from a video rather than // taken from a release: words a video title carries and an album track does -// not (#5410). +// not. // // Each pattern is used twice, so it is written in the subset that Postgres's // regex flavour and Go's RE2 read the same way: no \b (Postgres spells word @@ -21,9 +21,9 @@ type sourceMarker struct { pattern string } -// suspectSourceMarkers was calibrated against the operator's library on -// 2026-10-08. Two candidates were left out on purpose: -// - "live in"/"live at": 229 files, nearly all from real live albums. +// suspectSourceMarkers was calibrated against a real library. Two candidates +// were left out on purpose: +// - "live in"/"live at": nearly all from real live albums. // - a bare "reaction": it caught Beck's "Chain Reaction". The marker below // wants the phrases a reaction video actually uses. var suspectSourceMarkers = []sourceMarker{ diff --git a/internal/library/source_markers_test.go b/internal/library/source_markers_test.go index 2282a696..6eb5086d 100644 --- a/internal/library/source_markers_test.go +++ b/internal/library/source_markers_test.go @@ -5,7 +5,7 @@ import ( "testing" ) -// Real basenames from the operator's library (2026-10-08), each with the +// Real basenames, each with the // labels it should carry. The negatives are the near misses that shaped the // list: a song called "Chain Reaction", a live album, an ordinary track. func TestSourceMarkersFor(t *testing.T) { diff --git a/internal/library/stage_publisher_test.go b/internal/library/stage_publisher_test.go index ffbe969d..f04e651e 100644 --- a/internal/library/stage_publisher_test.go +++ b/internal/library/stage_publisher_test.go @@ -81,7 +81,8 @@ func TestStagePublisher_Tick_SwallowsPersistErrors(t *testing.T) { }, ) - // Ticks should not panic, should keep firing the persist (which keeps erroring). + // Ticks should not panic, should keep firing the persist (which keeps + // erroring). for i := 0; i < 5; i++ { pub.Tick() } diff --git a/internal/library/tracknumber.go b/internal/library/tracknumber.go index 3b99240a..51d27b80 100644 --- a/internal/library/tracknumber.go +++ b/internal/library/tracknumber.go @@ -9,7 +9,7 @@ import ( // ReadTrackAndDisc reads a file's track and disc numbers. dhowden/tag reads a // Vorbis TRACKNUMBER or DISCNUMBER only as a bare number, but taggers also -// write "3/20" there (Minstrel's own importer did until #6171), and that +// write "3/20" there (as Minstrel's own importer once did), and that // reads as 0. For Vorbis the number before the slash is taken instead. func ReadTrackAndDisc(m tag.Metadata) (track, disc int) { track, _ = m.Track() diff --git a/internal/library/trigger_test.go b/internal/library/trigger_test.go index fbbec894..24f20049 100644 --- a/internal/library/trigger_test.go +++ b/internal/library/trigger_test.go @@ -115,7 +115,8 @@ func TestTryStartScan_StaleInFlight_Reaps(t *testing.T) { t.Errorf("existing = %v, want nil", existing) } - // Verify the stale row was reaped: finished_at non-NULL, error_message contains "reaped". + // Verify the stale row was reaped: finished_at non-NULL, error_message + // contains "reaped". var finishedAt pgtype.Timestamptz var errMsg *string if err := pool.QueryRow(context.Background(), diff --git a/internal/library/vorbisgenre.go b/internal/library/vorbisgenre.go index fa3027ff..590ba10c 100644 --- a/internal/library/vorbisgenre.go +++ b/internal/library/vorbisgenre.go @@ -17,10 +17,9 @@ import ( // // dhowden/tag stores comments in a map keyed by field name // (vorbis.go: m.c[strings.ToLower(k)] = v), so each repeat overwrites the last -// and only the final value survives. On the operator's library that was 3,658 -// of 4,092 FLAC files: Kupla's Life Forms declares eight genres and was stored -// as "Instrumental Hip Hop" alone (#2500). Nothing looked corrupt; the other -// seven were simply never seen by browse or the taste profile. +// and only the final value survives. An album that declares eight genres +// would be stored under the last alone; nothing would look corrupt, and the +// other seven would never be seen by browse or the taste profile. // // The comment block is read here directly, as the ID3v2 genre frame is in // id3v2genre.go. Every other field still comes from dhowden/tag. @@ -43,7 +42,8 @@ func readFLACGenreValues(rs io.ReadSeeker) ([]string, error) { if _, err := io.ReadFull(rs, magic[:]); err != nil { return nil, errNoGenreFrame } - // Some taggers prepend an ID3v2 tag to a FLAC. The FLAC stream starts after it. + // Some taggers prepend an ID3v2 tag to a FLAC. The FLAC stream starts after + // it. if string(magic[:3]) == "ID3" { var rest [6]byte if _, err := io.ReadFull(rs, rest[:]); err != nil { @@ -172,7 +172,8 @@ func oggCommentGenres(packet []byte) ([]string, error) { // vorbisCommentGenres reads a comment block (vendor string, then a count of // length-prefixed "KEY=value" entries, little-endian lengths throughout) and // returns the GENRE values in file order. Field names are case-insensitive by -// spec. Anything after the last entry, such as Vorbis's framing bit, is ignored. +// spec. Anything after the last entry, such as Vorbis's framing bit, is +// ignored. func vorbisCommentGenres(b []byte) ([]string, error) { next := func() ([]byte, bool) { if len(b) < 4 { diff --git a/internal/library/vorbisgenre_test.go b/internal/library/vorbisgenre_test.go index 1fe0f0be..1349123f 100644 --- a/internal/library/vorbisgenre_test.go +++ b/internal/library/vorbisgenre_test.go @@ -161,7 +161,7 @@ func genreFields(values ...string) []string { return out } -// The #2500 regression, shaped like the operator's Kupla - Life Forms files: +// Shaped like a real multi-genre FLAC: // eight GENRE fields, of which dhowden/tag keeps the last. func TestExtractGenres_FLACRepeatedFields(t *testing.T) { data := buildFLAC(false, append([]string{"TITLE=Eons"}, genreFields(lifeForms...)...)...) diff --git a/internal/library/watcher.go b/internal/library/watcher.go index f2b42d18..c22a4c87 100644 --- a/internal/library/watcher.go +++ b/internal/library/watcher.go @@ -60,7 +60,7 @@ func classifyEvent(op fsnotify.Op, isDir, isAudio bool) watchAction { // because the walk alone only ever gave one of them: the walk re-visits every // path that EXISTS, which catches additions and edits, and the reconcile pass // that runs with it compares the whole tracks table against what the walk saw, -// which is what catches removals (#2523). classifyEvent still ignores fsnotify +// which is what catches removals. classifyEvent still ignores fsnotify // removals by design, so a deleted file surfaces at the next scan, not the // next inotify event. type Watcher struct { @@ -71,7 +71,7 @@ type Watcher struct { } // NewWatcher watches the scanner's folders, and moves to the new ones when -// they change in Settings (#6129). +// they change in Settings. func NewWatcher(scanner *Scanner, enricher *coverart.Enricher, logger *slog.Logger) *Watcher { return &Watcher{ scanner: scanner,