bvandeusen/minstrel
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases 52 Wiki Activity

docs: wanted, dbtest, recommendation and playlists comments say what the code does (#6277)

Browse Source 
The same rewrite as d191a282, for four packages whose comments carried the
most history: tuning stories, quotes from the operator, single-install
measurements in library_scale.go, and 'used to' accounts of bugs. The
constraint each story protected stays as a present-tense reason (why tier 3
is reduced not removed, why the randomised arms take an order seed, why the
seed artist is eligible in Songs-like). Comments naming system_cron.go and
config.RecommendationConfig, neither of which holds what they claimed, now
name the scheduler and internal/recsettings.

Checked: non-comment tokens identical to HEAD; gofmt clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
bvandeusen
2026-10-23 12:19:15 -04:00
co-authored by Claude Opus 5.5
parent d191a28217
commit fcf5b7e5a8
48 changed files with 391 additions and 501 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/wanted/library.go
+8 -8
View File
@@ -12,7 +12,7 @@ import (
)
// ReasonLibrary: on the list because the library held it when the list
// began (#5567).
// began.
const ReasonLibrary = "library"
// libraryMatchBudget bounds the time one pass spends matching albums to
@@ -33,17 +33,17 @@ const libraryMatchBatch = 25
// 4 an album holding every track of an edition, and more, is that group
const libraryMatcher = 4
// listLibrary puts the library's albums on the wanted list as held, once
// (operator, 2026-10-10: "make a record saying that the import has
// happened so later changes like unwanting hold and dont auto import from
// the file directory"). It runs a step each pass until done:
// listLibrary puts the library's albums on the wanted list as held, once,
// so that from then on the list is the record of what the library keeps: an
// album taken off it stays off, and files dropped in by hand are not listed.
// It runs a step each pass until done:
//
// - the first step stamps when the listing began, which is its cutoff:
// an album the library gains afterwards, dropped in by hand, is never
// listed;
// - every album whose tags name its release group and artist goes on the
// list at once;
// - every other album is matched to MusicBrainz (#5569), within a time
// - every other album is matched to MusicBrainz, within a time
// budget a pass, and listed as it is matched;
// - once none is left to match, the listing is stamped done, with a count
// of the albums no match was found for.
@@ -51,8 +51,8 @@ const libraryMatcher = 4
// Each album decided is recorded in library_album_matches, so an album taken
// off the list stays off. Once done the listing runs again only for albums
// an older matcher refused (libraryMatcher), refused albums whose tracks
// have changed since, and library tracks a re-tag filed under a new album
// (#5579). A person matches the rest by hand, or leaves them off, in the
// have changed since, and library tracks a re-tag filed under a new album.
// A person matches the rest by hand, or leaves them off, in the
// Collection's Unmatched section (MatchAlbumTo, LeaveAlbumOff).
func (s *Service) listLibrary(ctx context.Context, res *PassResult) error {
q := dbq.New(s.pool)
internal/wanted/library_test.go
+1 -1
View File
@@ -70,7 +70,7 @@ func matchOf(t *testing.T, pool *pgxpool.Pool, album pgtype.UUID) (outcome, meth
return
}
// The library's albums go on the list as held, once (#5567). Tags that name
// The library's albums go on the list as held, once. Tags that name
// the group list it at once; a release id is named through MusicBrainz;
// what nothing matches is recorded with why. Taken off the list, an album
// stays off, and one added to the library afterwards is never listed.
internal/wanted/manual.go
+4 -5
View File
@@ -15,11 +15,10 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
)
// Matching a library album by hand (#5579): the Collection's Unmatched
// section. A person names the MusicBrainz
// album an album is, by a link or a search, and it goes on the list as held;
// or leaves it off for good, as a local band's album MusicBrainz will never
// have.
// Matching a library album by hand: the Collection's Unmatched section. A
// person names the MusicBrainz album an album is, by a link or a search, and
// it goes on the list as held; or leaves it off for good, as a local band's
// album MusicBrainz will never have.
const matchManual = "manual"
internal/wanted/manual_test.go
+2 -3
View File
@@ -32,8 +32,7 @@ func TestParseMusicBrainzRef(t *testing.T) {
}
// A person names the MusicBrainz album an unmatched album is, and it goes
// on the list as held; or leaves it off, and matching never tries it again
// (#5579).
// on the list as held; or leaves it off, and matching never tries it again.
func TestMatchAlbumTo_ByLinkOrLeaveItOff_Integration(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
@@ -120,7 +119,7 @@ func TestMatchAlbumTo_ByLinkOrLeaveItOff_Integration(t *testing.T) {
// A re-tag files a library track under a new album row. That album is the
// library's, not one dropped in by hand, so the listing matches it; the
// old row, its tracks gone, is no longer unmatched (#5579).
// old row, its tracks gone, is no longer unmatched.
func TestListLibrary_MatchesARetaggedAlbum_Integration(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
internal/wanted/match.go
+4 -6
View File
@@ -16,13 +16,11 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/words"
)
// Matching a library album to its MusicBrainz release group (#5569).
// Matching a library album to its MusicBrainz release group.
//
// Everything in the library came from a service that matched it against
// MusicBrainz (operator, 2026-10-10: "they should all exist in music brainz
// we need to use their meta data or a search system to link them up"), so
// every album should have a group. The ids its tags carry are taken as
// they are. Without them, the groups called what the album is called, from
// Music from any mainstream source was matched against MusicBrainz on its
// way in, so nearly every album has a group. The ids its tags carry are
// taken as they are. Without them, the groups called what the album is called, from
// the artist's discography and then from a search, are checked against the
// album's own tracks, and the one its tracks agree with is taken.
internal/wanted/profiles.go
+1 -1
View File
@@ -40,7 +40,7 @@ var (
// which every profile passes on its primary type alone.
//
// A version its title names counts as a secondary type MusicBrainz left
// off (#5573): "Common People (Recorded For BBC Radio 1)" is live whatever
// off: "Common People (Recorded For BBC Radio 1)" is live whatever
// its types say, and a version no type names, an acoustic one, no profile
// passes.
func Passes(p dbq.ReleaseTypeProfile, title, primary string, secondary []string) bool {
internal/wanted/search.go
+4 -5
View File
@@ -18,8 +18,7 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/release"
)
// Searching for wants on a schedule (M502 #5521), ported from Fabled
// Librarian's internal/wanted. The schedule decides which wants and when;
// Searching for wants on a schedule. The schedule decides which wants and when;
// internal/acquire judges what a search finds and grabs.
// Search outcomes, matching wants_search_outcome_check.
@@ -32,7 +31,7 @@ const (
const (
// searchTick is how often the schedule looks for wants due; the
// operator's setting is how long a want waits between searches.
// search interval setting is how long a want waits between searches.
searchTick = 10 * time.Minute
// searchWindow is how long one pass searches: most of a tick, so a pass
// ends before the next begins.
@@ -194,7 +193,7 @@ type searchRow struct {
artist, title, primary, date, group string
secondary []string
// upgrade, heldTier and held: a held album wanted for a better copy
// (#5524), and the copy it has.
// and the copy it has.
upgrade, heldTier, held string
}
@@ -346,7 +345,7 @@ func (s *Searcher) Take(ctx context.Context, id pgtype.UUID, key string, force b
}
// load reads a want for a search: an open one, or a held album wanted for a
// better copy (#5524).
// better copy.
func (s *Searcher) load(ctx context.Context, id pgtype.UUID) (acquire.Want, error) {
row, err := dbq.New(s.pool).GetWantForSearch(ctx, id)
if err != nil {
internal/wanted/search_test.go
+1 -1
View File
@@ -28,7 +28,7 @@ const ramFeed = `<?xml version="1.0"?>
</channel></rss>`
// searchStack is a fake indexer feed and a fake FabledExchange download
// client, configured the way the operator would in Integrations.
// client, configured as an admin would in Admin › Acquisition.
func searchStack(t *testing.T, ctx context.Context, store *acquire.Store) *atomic.Int32 {
t.Helper()
var added atomic.Int32
internal/wanted/upgrade.go
+2 -2
View File
@@ -11,8 +11,8 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/release"
)
// Upgrades (M502 #5524): a held album stays wanted when the copy the library
// has is below the quality the operator settles for (acquisition_settings.
// Upgrades: a held album stays wanted when the copy the library has is below
// the quality the install settles for (acquisition_settings.
// upgrade_until), or when one of its tracks is a video rip. The search
// schedule then looks for a better release weekly, and the import replaces
// the old tracks with the new ones, carrying their history across.
internal/wanted/upgrade_test.go
+3 -4
View File
@@ -99,7 +99,7 @@ func TestCheckUpgrades_AHeldAlbumBelowTheCutoffIsWantedAgain_Integration(t *test
t.Fatalf("due for search: %+v, %v", due, err)
}
// Upgrades switched off: no longer wanted again.
// Upgrades switched off: the album is not wanted again.
if _, err := pool.Exec(ctx, `UPDATE acquisition_settings SET upgrade_until = ''`); err != nil {
t.Fatal(err)
}
@@ -138,9 +138,8 @@ func TestCheckUpgrades_ARipIsReplacedWhateverTheSetting_Integration(t *testing.T
}
// A missing track with no disc number is on disc 1, so a present 1/2 stands in
// for a missing "2". Air's Love 2 stayed wanted over one such row and was
// downloaded again for nothing (2026-10-20). A missing track no present track
// covers still counts.
// for a missing "2", and the album is not downloaded again for nothing. A
// missing track no present track covers still counts.
func TestWantHoldings_AMissingTrackWithNoDiscIsOnDiscOne_Integration(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
internal/wanted/wanted.go
+17 -19
View File
@@ -1,10 +1,9 @@
// Package wanted is what Minstrel is looking for (M502 #5520): followed
// artists, each on a release-type profile, and wanted release groups. A want
// comes from an approved request, a followed artist's new release, or an
// album whose files went missing; it is held once the library has a full
// copy. The library's own albums were put on the list as held once (#5567),
// so the list is the record of what the library keeps. Searching for open
// wants is #5521's.
// Package wanted is what Minstrel is looking for: followed artists, each on
// a release-type profile, and wanted release groups. A want comes from an
// approved request, a followed artist's new release, or an album whose files
// went missing; it is held once the library has a full copy. The library's
// own albums are put on the list as held once, so the list is the record of
// what the library keeps. Searching for open wants is the Searcher's.
package wanted
import (
@@ -47,11 +46,11 @@ type Service struct {
logger *slog.Logger
now func() time.Time
// releaseGroup names the group of a MusicBrainz release id, to move a
// want recorded under a release id (#5241) to its group. A field so tests
// want recorded under a release id to its group. A field so tests
// stand in for MusicBrainz.
releaseGroup func(ctx context.Context, releaseMBID string) (string, error)
// upgradesChecked is when held albums were last judged for an upgrade
// (#5524); the worker alone touches it.
// the worker alone touches it.
upgradesChecked time.Time
}
@@ -196,9 +195,9 @@ type PassResult struct {
Baselined int // of those, first reads
Wanted int // wants recorded from follows
Held int // wants the library now answers
Listed int // library albums put on the list as held (#5567)
Listed int // library albums put on the list as held
// Upgrades: held albums wanted for a better copy, when the hourly check
// ran this pass (#5524).
// ran this pass.
Upgrades int
}
@@ -211,7 +210,7 @@ func (s *Service) Pass(ctx context.Context) (PassResult, error) {
if ctx.Err() != nil {
return res, ctx.Err()
}
// The one-time listing of the library (#5567); a failure waits for the
// The one-time listing of the library; a failure waits for the
// next pass and does not hold back settling.
if err := s.listLibrary(ctx, &res); err != nil && ctx.Err() == nil {
s.logger.Warn("wanted: list the library", "err", err)
@@ -321,9 +320,9 @@ func (s *Service) applyFollow(ctx context.Context, q *dbq.Queries, f dbq.Follow)
return wanted, q.MarkFollowChecked(ctx, f.ArtistMbid)
}
// dropRefusedVersion takes off the list a group the follow wanted before
// Passes read the version its title names (#5573), while it is still only
// wanted: one the operator holds, or wanted for another reason, stays.
// dropRefusedVersion takes off the list a group a follow wants that Passes
// now refuses for the version its title names, while it is still only
// wanted: one the library holds, or wanted for another reason, stays.
func (s *Service) dropRefusedVersion(ctx context.Context, q *dbq.Queries, group, title string) error {
v, ok := release.VersionInTitle(title)
if !ok {
@@ -464,7 +463,7 @@ func Shortest(releases []dbq.MbRelease) int {
}
// moveToGroup handles a want MusicBrainz has no release group under: one
// recorded under a release id before #5241. It is moved to the release's
// recorded under a release id. It is moved to the release's
// group, with the requests that name it; when the group is already wanted,
// this copy is dropped.
func (s *Service) moveToGroup(ctx context.Context, w dbq.Want) (bool, error) {
@@ -509,9 +508,8 @@ func (s *Service) moveToGroup(ctx context.Context, w dbq.Want) (bool, error) {
// missing track is left without a track on disk in its place;
// - otherwise an album of the group with at least as many tracks on disk as
// the shortest official edition;
// - and when the catalogue cannot say how long the group is, the test from
// #5263: a track arrived since the want, or nothing missing before it is
// still missing.
// - and when the catalogue cannot say how long the group is: a track
// arrived since the want, or nothing missing before it is still missing.
//
// Only albums with a track on disk count.
func (s *Service) answeredBy(ctx context.Context, q *dbq.Queries, w dbq.Want, shortest int) (pgtype.UUID, bool, error) {
internal/wanted/wanted_test.go
+4 -4
View File
@@ -42,7 +42,7 @@ func TestPasses_PrimaryListedAndEverySecondaryAllowed(t *testing.T) {
{withLive, "Live at Koko", "Album", []string{"Live"}, true},
{withLive, "Live & Rare", "Album", []string{"Live", "Compilation"}, false},
{withLive, "Like Phantoms, Forever", "EP", nil, false},
// MusicBrainz types these plain Singles; their titles say otherwise (#5573).
// MusicBrainz types these plain Singles; their titles say otherwise.
{standard, "Common People (Recorded For BBC Radio 1)", "Single", nil, false},
{standard, "Welcome to the Black Parade (Steve Aoki remix)", "Single", nil, false},
{standard, "The Five of Us Are Dying (Rough mix)", "Single", nil, false},
@@ -185,7 +185,7 @@ const (
theBallad = "11111111-0000-4000-8000-000000000003" // 2023 album, not held
newSingle = "11111111-0000-4000-8000-000000000004" // appears after the baseline
oldSingle = "11111111-0000-4000-8000-000000000005" // 1997 single, catalogued late
bbcSingle = "11111111-0000-4000-8000-000000000006" // live, typed a plain Single (#5573)
bbcSingle = "11111111-0000-4000-8000-000000000006" // live, typed a plain Single
parkRelease = "22222222-0000-4000-8000-000000000001"
)
@@ -238,7 +238,7 @@ func newFakeMB(t *testing.T) *fakeMB {
case r.URL.Path == "/release":
_, _ = fmt.Fprint(w, `{"release-count": 0, "releases": []}`)
case r.URL.Path == "/release-group" && strings.Contains(r.URL.Query().Get("query"), "releasegroup:"):
// The library listing's search by artist and title (#5569).
// The library listing's search by artist and title.
found := ""
if strings.Contains(r.URL.Query().Get("query"), "Ballad") {
found = group(theBallad, "The Ballad of Darren", "Album", "", "2023-07-21")
@@ -338,7 +338,7 @@ func TestFollow_WantExistingWantsWhatPassesAndIsNotHeld_Integration(t *testing.T
f := newFakeMB(t)
s := newService(t, pool, f)
_ = seedHeld(t, q, "Parklife", parkRelease, parklife, 2)
// Wanted before the follow read the version a title names (#5573).
// Wanted before the follow refused the version its title names.
if _, err := WantWith(ctx, q, WantParams{GroupMBID: bbcSingle, ArtistMBID: blur, ArtistName: "Blur",
Title: "Song 2 (Recorded For BBC Radio 1)", PrimaryType: "Single", Reason: ReasonFollow}); err != nil {
t.Fatal(err)