From d191a28217936e900d32280345b2b0038f12f98c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 23 Oct 2026 12:15:05 -0400 Subject: [PATCH] docs: server comments say what the code does, not how it came to be (#6277) Comment blocks in cmd/minstrel and 30 smaller internal packages, rewritten to rule 32's standard with each citation read in context: task, milestone and rule ids go, and so do the stories they were attached to ("used to", "Before #N", measurements from one install, quotes, pointers to Fabled Librarian's records and to docs/superpowers files that are not in the repo). Where a story wrapped a real constraint, the constraint stays as a present-tense fact. Comments that had gone stale are corrected: - taste.Config: the tuned values are overlaid by internal/recsettings; it is no longer "a later follow-up". - StorageConfig: the data dir holds art, caches and the stream secret, not only playlist covers. - the Subsonic album list and getUser comments describe what they do today rather than a milestone that would change it. Checked: the non-comment token stream of every changed file matches HEAD, and gofmt -l is empty. Co-Authored-By: Claude Opus 5.5 --- cmd/minstrel/admin.go | 12 +- cmd/minstrel/main.go | 121 ++++++++---------- internal/acoustid/client.go | 11 +- internal/acquire/fanout.go | 3 +- internal/acquire/grab.go | 15 +-- internal/acquire/memory.go | 6 +- internal/acquire/pace.go | 5 +- internal/acquire/search.go | 8 +- internal/acquire/search_test.go | 2 +- internal/acquire/settings.go | 9 +- internal/acquire/store.go | 2 +- internal/audit/audit.go | 12 +- internal/auth/clientip.go | 25 ++-- internal/auth/clientip_test.go | 6 +- internal/auth/session.go | 6 +- internal/auth/setup.go | 8 +- internal/buildinfo/buildinfo.go | 10 +- internal/catalogue/art.go | 6 +- internal/catalogue/catalogue.go | 2 +- internal/config/config.go | 34 ++--- internal/coplay/worker.go | 15 +-- internal/coverart/artist_enricher.go | 10 +- internal/coverart/enricher.go | 18 ++- internal/coverart/provider.go | 7 +- internal/coverart/provider_mbcaa.go | 4 +- internal/coverart/settings.go | 9 +- internal/downloader/driver.go | 7 +- internal/exchange/exchange.go | 5 +- internal/gc/worker.go | 28 ++-- internal/importer/importer.go | 6 +- internal/importer/inspect.go | 2 +- internal/importer/name.go | 2 +- internal/importer/place.go | 8 +- internal/importer/upgrade.go | 2 +- internal/importer/verify.go | 12 +- internal/importer/verify_test.go | 2 +- internal/indexer/indexer.go | 9 +- internal/indexer/torznab.go | 5 +- internal/lifecycle/lifecycle.go | 12 +- internal/lifecycle/lifecycle_test.go | 2 +- internal/logging/logging.go | 2 +- internal/logging/recent.go | 2 +- internal/mood/mood.go | 4 +- internal/musicbrainz/client.go | 5 +- internal/musicbrainz/pace.go | 2 +- internal/netsettings/service.go | 10 +- internal/notifications/digest.go | 6 +- internal/notifications/email_settings.go | 2 +- internal/notifications/kinds.go | 24 ++-- internal/notifications/notifier.go | 2 +- internal/notifications/notifier_db_test.go | 12 +- internal/notifications/render.go | 2 +- internal/notifications/render_test.go | 2 +- internal/playevents/writer.go | 55 +++----- internal/playevents/writer_test.go | 8 +- internal/playsessions/service.go | 3 +- internal/prowlarr/prowlarr.go | 5 +- internal/quarantine/service.go | 2 +- internal/reacquisition/settings.go | 9 +- internal/reacquisition/sweeper.go | 8 +- internal/reacquisition/sweeper_test.go | 11 +- internal/recsettings/patch.go | 2 +- internal/recsettings/service.go | 80 ++++++------ internal/recsettings/service_test.go | 15 +-- .../recsettings/songs_like_weights_test.go | 9 +- internal/release/agree.go | 2 +- internal/release/parse.go | 2 +- internal/release/quality.go | 6 +- internal/release/release_test.go | 2 +- internal/release/torrent.go | 2 +- internal/release/version.go | 2 +- internal/release/version_test.go | 2 +- internal/requests/reconciler.go | 4 +- internal/requests/service.go | 4 +- internal/scrobble/threshold.go | 5 +- internal/scrobble/worker.go | 2 +- internal/server/hygiene.go | 4 +- internal/server/hygiene_test.go | 2 +- internal/server/release_gate_test.go | 2 +- internal/server/release_version_test.go | 42 +++--- internal/server/requestlog.go | 16 +-- internal/server/requestlog_test.go | 7 +- internal/server/server.go | 34 +++-- internal/server/server_test.go | 12 +- internal/server/version.go | 2 +- internal/similarity/worker.go | 16 +-- .../similarity/worker_integration_test.go | 22 ++-- internal/subsonic/browse.go | 18 ++- internal/subsonic/replaygain_test.go | 2 +- internal/subsonic/star.go | 4 +- internal/subsonic/types.go | 2 +- internal/subsonic/user.go | 5 +- internal/sync/changes.go | 6 +- internal/sync/compactor.go | 19 ++- internal/sync/types.go | 2 +- internal/tags/enricher.go | 6 +- internal/tags/enricher_chain_test.go | 2 +- internal/tags/provider.go | 13 +- internal/tags/provider_artist_tags_test.go | 6 +- internal/tags/provider_lastfm.go | 2 +- internal/tags/provider_musicbrainz.go | 12 +- internal/tags/provider_musicbrainz_test.go | 2 +- internal/tags/worker.go | 2 +- internal/taste/engagement.go | 2 +- internal/taste/profile.go | 25 ++-- internal/tracks/service.go | 8 +- internal/words/words.go | 2 +- 107 files changed, 485 insertions(+), 597 deletions(-) diff --git a/cmd/minstrel/admin.go b/cmd/minstrel/admin.go index c2dcd3eb..26103e30 100644 --- a/cmd/minstrel/admin.go +++ b/cmd/minstrel/admin.go @@ -55,14 +55,12 @@ func runAdmin(args []string) error { } } -// adminResetPassword resets a user's login password (password_hash). It -// recovers a locked-out operator when the bootstrap password was missed or -// the DB volume was recreated (Fable #321) without DB surgery. +// adminResetPassword resets a user's login password (password_hash), so an +// admin who is locked out can get back in without editing the database. // -// It deliberately leaves subsonic_password alone. That column is stored in -// plain text, and it used to receive the new login password here, so any -// account recovered this way had its login password readable in the database -// (#5026). The Subsonic password is generated separately in Settings. +// It leaves subsonic_password alone: that column is stored in plain text, so +// writing the login password there would make it readable in the database. +// The Subsonic password is generated separately in Settings. func adminResetPassword(args []string) error { fs := flag.NewFlagSet("admin reset-password", flag.ContinueOnError) configPath := fs.String("config", os.Getenv("MINSTREL_CONFIG"), "path to YAML config file") diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 96fddca7..7768650d 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -128,7 +128,7 @@ func run() error { } defer pool.Close() - // Fingerprinting settings (M400 #3913): one instance, shared by the scanner, + // Fingerprinting settings: one instance, shared by the scanner, // the fingerprint backfill, the duplicate sweep and the admin API, so a save // reaches all of them without a restart. A load failure is logged, not fatal: // the service falls back to the shipped defaults. @@ -136,9 +136,9 @@ func run() error { if fpErr != nil { logger.Warn("fingerprint settings: using defaults", "err", fpErr) } - // The library's folders (#6129): the ones saved in Settings, or + // The library's folders: the ones saved in Settings, or // MINSTREL_LIBRARY_SCAN_PATHS until any are. A load failure falls back to - // env, which is what the server read before the setting existed. + // the env value. roots, rootsSaved, rootsErr := library.LoadRoots(ctx, pool, cfg.Library.ScanPaths) switch { case rootsErr != nil: @@ -149,14 +149,14 @@ func run() error { } scanner := library.New(pool, logger, roots, fpSettings) - // Loudness analysis settings (M464 #4995): shared by the loudness backfill + // Loudness analysis settings: shared by the loudness backfill // and the admin API, and falls back to the defaults like the above. loudSettings, loudErr := library.NewLoudnessSettingsService(ctx, pool) if loudErr != nil { logger.Warn("loudness settings: using defaults", "err", loudErr) } - // AcoustID lookup settings (M401): shared by the lookup worker and the + // AcoustID lookup settings: shared by the lookup worker and the // admin API. A load failure leaves the lookup off. acoustIDSettings, acoustIDErr := library.NewAcoustIDSettingsService(ctx, pool) if acoustIDErr != nil { @@ -194,10 +194,9 @@ func run() error { coverEnricher := coverart.NewEnricher(pool, logger.With("component", "coverart"), coverSettings) coverEnricher.DataDir = cfg.Storage.DataDir - // Recommendation tuning lab (#1250): seeds shipped defaults on first - // boot and pushes the daily_mix weights + taste config into package - // playlists — must precede the scheduler so the first builds score - // with the operator's tuned values, not the pre-push literals. + // Recommendation tuning: seeds the shipped defaults on first boot and + // pushes the daily-mix weights and taste config into package playlists. + // It must precede the scheduler, so the first builds use the tuned values. recSettings, err := recsettings.New(ctx, pool, logger.With("component", "recsettings")) if err != nil { logger.Error("recommendation settings service init failed", "err", err) @@ -213,7 +212,7 @@ func run() error { logger.With("component", "scan_run"), library.RunScanConfig{ BackfillCap: 5000, - EnrichCap: -1, // #388: no global cap; per-provider rate limits + EnrichCap: -1, // no global cap; each provider paces itself ArtistEnrichCap: -1, DataDir: cfg.Storage.DataDir, }, @@ -223,14 +222,13 @@ func run() error { }() } else { // No startup file walk, but still run backfill + enrich so cover - // progress doesn't stall just because the operator disabled - // scan-on-startup. + // art keeps arriving with scan-on-startup off. go func() { if _, err := library.RunScan(ctx, pool, nil, coverEnricher, logger.With("component", "scan_run"), library.RunScanConfig{ BackfillCap: 5000, - EnrichCap: -1, // #388: no global cap; per-provider rate limits + EnrichCap: -1, // no global cap; each provider paces itself ArtistEnrichCap: -1, DataDir: cfg.Storage.DataDir, }, @@ -240,46 +238,44 @@ func run() error { }() } - // Start the ListenBrainz scrobble worker. Per spec §M4a, runs every 30s - // and drains up to 50 pending rows per tick. Per-user gating happens - // inside the worker (rows from disabled users are skipped). + // ListenBrainz scrobble worker: every 30s, drains up to 50 pending rows. + // Rows from users with scrobbling off are skipped inside the worker. scrobbleWorker := scrobble.NewWorker(pool, listenbrainz.NewClient(), logger.With("component", "scrobble")) go scrobbleWorker.Run(ctx) - // Start the similarity ingest worker. Per spec §M4b, runs every 1h, drains - // up to 5 played tracks + 5 played artists per tick, weekly re-fetch cap - // per row. Public LB endpoints — no token required. + // Similarity ingest worker: hourly, fetches ListenBrainz similarity for up + // to 5 played tracks and 5 played artists, each re-fetched at most weekly. + // The endpoints are public, so no token is needed. similarityWorker := similarity.NewWorker(pool, listenbrainz.NewClient(), logger.With("component", "similarity")) go similarityWorker.Run(ctx) - // Start the household co-play worker (#1533). Recomputes artist–artist + // Household co-play worker: recomputes artist–artist // co-occurrence edges (source='user_cooccurrence') from play_events every // 6h — a collaborative candidate arm for the radio/mix pools. Pure local // SQL, no external calls; empty on single-user servers. go coplay.NewWorker(pool, logger.With("component", "coplay")).Run(ctx) - // Fingerprint backfill (M400 #3908): fingerprints the tracks the scan never - // will — everything imported before fingerprinting existed, and rows derived - // by an older method. A worker of its own rather than a scan stage; see - // internal/library/fingerprint_backfill.go for why. + // Fingerprint backfill: fingerprints the tracks the scan skips (unchanged + // files, and rows fingerprinted by an older method). A worker of its own + // rather than a scan stage; see internal/library/fingerprint_backfill.go. go library.NewFingerprintBackfillWorker(pool, logger.With("component", "fingerprint_backfill"), fpSettings).Run(ctx) - // Loudness backfill (M464 #4995): measures every track's loudness for + // Loudness backfill: measures every track's loudness for // normalization, new tracks included; the scan only drops a changed file's // measurement. See internal/library/loudness_backfill.go. go library.NewLoudnessBackfillWorker(pool, logger.With("component", "loudness_backfill"), loudSettings).Run(ctx) - // AcoustID lookup (M401): fills the recording MBIDs untagged tracks lack, - // so the similarity arms can see them. Idle until the operator adds a key - // and switches it on. See internal/library/acoustid_lookup.go. + // AcoustID lookup: fills the recording MBIDs untagged tracks lack, so the + // similarity arms can see them. Idle until an admin adds a key and + // switches it on. See internal/library/acoustid_lookup.go. go acoustIDLookup.Run(ctx) - // Duplicate sweep (M400 #3910): proposes groups of tracks holding one + // Duplicate sweep: proposes groups of tracks holding one // recording, from the fingerprints above. Sweeps only when fingerprints have // changed since the last sweep. go library.NewDuplicateSweepWorker(pool, logger.With("component", "duplicate_sweep"), fpSettings).Run(ctx) - // Start the tag-enrichment worker (#1490). Reconciles the compiled-in + // Tag-enrichment worker: reconciles the compiled-in // tag providers with tag_provider_settings, bumps the sources version if // the provider set changed (re-opening settled rows), then drains tracks // needing folksonomy tags on a periodic tick. Standalone (not in the file @@ -298,54 +294,50 @@ func run() error { tagEnricher := tags.NewEnricher(pool, logger.With("component", "tags"), tagSettings) go tags.NewWorker(tagEnricher, logger.With("component", "tags")).Run(ctx) - // Start the GC worker. Runs every 1h and sweeps lifecycle tables - // that have no writer-side close path or retention policy: - // orphan play_events, stale play_sessions, expired - // scrobble_queue failures, stuck system_playlist_runs, expired - // password_resets. Each sweep is idempotent — a row that's - // already clean is a no-op. Addresses drift audit findings - // #565 #566 #567 #574 #575 (Scribe parent #552). + // GC worker: hourly, sweeps the tables that have no writer-side close + // path or retention policy: orphan play_events, stale play_sessions, + // expired scrobble_queue failures, stuck system_playlist_runs and expired + // password_resets. Each sweep is idempotent. gcWorker := gc.NewWorker(pool, logger.With("component", "gc")) go gcWorker.Run(ctx) - // The request reconciler completes approved requests once the library - // holds what they asked for (M502 #5520). - // Live-event bus shared between SSE subscribers (api.Mount) and - // background workers that publish (reconciler today; scanner later). - // Constructed before any service that publishes so they all share the - // same instance. + // Live-event bus shared between SSE subscribers (api.Mount) and the + // background workers that publish. Built before any of them, so they all + // share one instance. bus := eventbus.New() // Scan-run lifecycle events use a package-level setter rather than // threading the bus through RunScan + TryStartScan + the Scheduler // + every test caller. Per-process singleton, set once at startup. library.SetEventBus(bus) - // The notifications inbox's one writer (M489), shared by every + // The notifications inbox's one writer, shared by every // background producer started here. The API builds its own over the // same pool and bus. notifier := notifications.New(pool, bus, logger.With("component", "notifications")) + // The request reconciler completes approved requests once the library + // holds what they asked for. requestReconciler := requests.NewReconciler(pool, logger.With("component", "requests"), bus) requestReconciler.SetNotifier(notifier) library.SetNotifier(notifier) go requestReconciler.Run(ctx) - // Release catalogue (M502 #5519): re-reads discographies older than a + // Release catalogue: re-reads discographies older than a // week from MusicBrainz, a batch an hour, through the process-wide // MusicBrainz pacer it shares with tag enrichment. cat := catalogue.New(pool, nil, logger.With("component", "catalogue")) go cat.Run(ctx) - // The wanted list (M502 #5520): reads followed artists' discographies + // The wanted list: reads followed artists' discographies // daily for new releases and marks wants held once the library has them. go wanted.New(pool, cat, logger.With("component", "wanted")).Run(ctx) - // Search and grab (M502 #5521): every ten minutes, searches the + // Search and grab: every ten minutes, searches the // configured indexers for wants due, paced by each source's // searches-per-hour, and hands the best release to the download client // when auto-grab is on. go wanted.NewSearcher(pool, cat, logger.With("component", "search")).Run(ctx) - // Import with verification (M502 #5522): every minute, finished + // Import with verification: every minute, finished // downloads are checked against their album (AcoustID, lengths, tags) // and filed into the library with MusicBrainz tags, or refused. var printer importer.Printer @@ -356,13 +348,13 @@ func run() error { imp.DataDir = cfg.Storage.DataDir go imp.Run(ctx) - // Download lifecycle (M502 #5523): every ten minutes, a download that + // Download lifecycle: every ten minutes, a download that // failed or that nobody shares is retried, then given up and its album // searched again; an imported one seeds, then is removed. go lifecycle.New(pool, logger.With("component", "lifecycle")).Run(ctx) - // Duplicate resolver (M498, M502): classifies the sweep's groups and, with - // the operator's auto-resolve on, merges the copies that are safe to merge. + // Duplicate resolver: classifies the sweep's groups and, with auto-resolve + // on, merges the copies that are safe to merge. // Started after SetNotifier so its summaries reach admins. It asks the // catalogue for the track list of an album whose wrong slot it must // decide, a few a pass. See internal/library/duplicate_resolve.go. @@ -371,15 +363,12 @@ func run() error { TrackLists: cat.RefreshReleaseGroup, Identify: acoustIDLookup.IdentifyRecordings, }).Run(ctx) - // Missing-file re-acquisition (milestone #290, M502 #5520). Wants back - // albums whose files have been gone longer than the grace window; the - // search schedule looks for them. Hourly tick — the grace window is - // measured in hours. + // Missing-file re-acquisition: wants back albums whose files have been + // gone longer than the grace window, for the search schedule to find. + // Hourly, because the grace window is measured in hours. // // A settings-load failure is logged, not fatal: NewSettingsService always - // returns a usable service holding the defaults, and running on defaults - // is far better than dropping the feature because the database hiccuped - // during boot. + // returns a usable service holding the defaults. reacqSettings, reacqErr := reacquisition.NewSettingsService( ctx, pool, logger.With("component", "reacquisition")) if reacqErr != nil { @@ -387,18 +376,18 @@ func run() error { } go reacquisition.NewSweeper(pool, reacqSettings, logger.With("component", "reacquisition")).Run(ctx) - // library_changes compactor (#357 follow-up). Daily tick; deletes + // library_changes compactor: daily, deletes // rows older than the configured retention so the change-log table // doesn't grow unbounded. Clients that drop offline longer than // retention hit the /api/library/sync 410 fallback and resync. libraryChangesCompactor := syncpkg.NewCompactor(pool, logger.With("component", "library_changes_compactor")) go libraryChangesCompactor.Run(ctx) - // Notifications inbox retention (M489). Daily: read rows go after 90 + // Notifications inbox retention, daily: read rows go after 90 // days, anything at all after a year. go notifications.NewRetention(pool, logger.With("component", "notifications_retention")).Run(ctx) - // Notifications by email (M489 #5346), grouped, never one per event: new + // Notifications by email, grouped, never one per event: new // music as a daily summary at a local hour, everything else batched an // hour (admin-configurable) after the first item. go notifications.NewDigest( @@ -407,10 +396,8 @@ func run() error { logger.With("component", "notification_digest"), ).Run(ctx) - // Per-user system-playlist scheduler (#392 Half B). Fires each - // active user's daily build at 03:00 in their stored timezone. - // Replaces the 24h-anchored cron loop (removed in the next commit - // of this arc). + // Per-user system-playlist scheduler: runs each active user's daily + // build at 03:00 in their stored timezone. playlistScheduler, err := playlists.NewScheduler( pool, logger.With("component", "playlist_scheduler"), @@ -439,7 +426,7 @@ func run() error { scanCfg := library.RunScanConfig{ BackfillCap: 5000, - EnrichCap: -1, // #388: no global cap; per-provider rate limits + EnrichCap: -1, // no global cap; each provider paces itself ArtistEnrichCap: -1, DataDir: cfg.Storage.DataDir, } @@ -468,7 +455,7 @@ func run() error { srv.LoudnessSettings = loudSettings srv.AcoustIDLookup = acoustIDLookup // The sweeper above holds this same instance, so a save from the admin - // card changes what it does on its next tick (#3936). + // card changes what it does on its next tick. srv.ReacqSettings = reacqSettings srv.StreamSecret = cfg.StreamSecret httpServer := &http.Server{ diff --git a/internal/acoustid/client.go b/internal/acoustid/client.go index d6153dea..dbf1f42d 100644 --- a/internal/acoustid/client.go +++ b/internal/acoustid/client.go @@ -1,13 +1,13 @@ // Package acoustid looks up MusicBrainz recording ids by acoustic fingerprint -// through the AcoustID web service (M401). +// through the AcoustID web service. // -// What it relies on, from https://acoustid.org/webservice (read 2026-10-06): +// What it relies on, from https://acoustid.org/webservice: // - POST https://api.acoustid.org/v2/lookup with client, duration (whole // seconds), fingerprint (fpcalc's compressed string) and meta. A gzip body // with Content-Encoding: gzip is accepted and preferred, because // fingerprints are long. // - meta=recordings returns each recording's title, duration and artists, -// which the worker needs to tell several candidates apart (M401 D5). +// which the worker needs to tell several candidates apart. // - "Do not make more than 3 requests per second." // - The service is free for non-commercial use only. // @@ -39,8 +39,7 @@ const ( // minInterval keeps to AcoustID's 3 requests a second with headroom: one // every 400ms is 2.5 a second. minInterval = 400 * time.Millisecond - // lookupTimeout bounds one request, connection included (rule 156). A - // lookup answers in well under a second; 20s is the line past which slow + // lookupTimeout bounds one request, connection included. A lookup answers in well under a second; 20s is the line past which slow // has become never. lookupTimeout = 20 * time.Second // bodyLimit caps the response read. A lookup with meta=recordings for a @@ -59,7 +58,7 @@ const ( ) var ( - // ErrInvalidKey: the operator's application key was refused. No lookup + // ErrInvalidKey: the configured application key was refused. No lookup // can succeed until it is changed, so the worker stops and says so. ErrInvalidKey = errors.New("acoustid: the API key was refused") // ErrInvalidFingerprint: AcoustID rejected this track's fingerprint. A diff --git a/internal/acquire/fanout.go b/internal/acquire/fanout.go index 3d0594b3..35260078 100644 --- a/internal/acquire/fanout.go +++ b/internal/acquire/fanout.go @@ -10,8 +10,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/indexer" ) -// Searching every source at once (M502 #5521), ported from Fabled Librarian -// (#4182). A source that is down narrows a search rather than failing it: the +// Searching every source at once. A source that is down narrows a search rather than failing it: the // results come back with the list of who did not answer. // SourceFailure is one source that could not be reached. diff --git a/internal/acquire/grab.go b/internal/acquire/grab.go index 20b106af..001814bd 100644 --- a/internal/acquire/grab.go +++ b/internal/acquire/grab.go @@ -16,12 +16,11 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/release" ) -// Taking a release (M502 #5521), ported from Fabled Librarian's grab.go and -// rivals.go. One live grab per want: a second is tie-broken against the -// download already running (FL #4470). +// Taking a release. One live grab per want: a second is tie-broken against +// the download already running. -// Tag goes on every torrent Minstrel adds, so the operator can tell them -// from their own in the client. +// Tag goes on every torrent Minstrel adds, so they can be told apart from +// anything else in the client. const Tag = "minstrel" // MissingClient is Taken.Missing when no download client is set up. @@ -211,9 +210,9 @@ func (s *Service) Grab(ctx context.Context, want Want, chosen *Candidate, all [] } // heldElsewhere reports whether a release the client says it already holds is -// one it does not list for Minstrel: held under another category, from before -// Minstrel took the downloads over (#6176). Recorded as a grab, it would read -// as gone half an hour later and be taken again, every pass. A client that +// one it does not list for Minstrel: held under another category. Recorded +// as a grab, it would read as gone half an hour later and be taken again, +// every pass. A client that // cannot answer now is given the benefit of the doubt, as the add was. func (s *Service) heldElsewhere(ctx context.Context, hash string) bool { if hash == "" { diff --git a/internal/acquire/memory.go b/internal/acquire/memory.go index 7d2eac24..8e553e9f 100644 --- a/internal/acquire/memory.go +++ b/internal/acquire/memory.go @@ -20,13 +20,13 @@ import ( const ( BlockOperator = "operator" BlockFiles = "files" - // BlockImport: the import found its files were not the album (#5522). + // BlockImport: the import found its files were not the album. BlockImport = "import" - // BlockStalled: given up on because it would not finish (#5523); held + // BlockStalled: given up on because it would not finish; held // back until the block's expires_at. BlockStalled = "stalled" // BlockGone: its download client would not let Minstrel follow it, held - // outside Minstrel's category or leaving the client twice (#6176); held + // outside Minstrel's category or leaving the client twice; held // back for GoneBlock. BlockGone = "gone" ) diff --git a/internal/acquire/pace.go b/internal/acquire/pace.go index 53900e85..8b8bb575 100644 --- a/internal/acquire/pace.go +++ b/internal/acquire/pace.go @@ -8,8 +8,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/indexer" ) -// SearchPace is the one Pacer every search shares, ported from Fabled -// Librarian (#4448). Process-wide because a Fanout is built fresh for each +// SearchPace is the one Pacer every search shares. Process-wide because a Fanout is built fresh for each // search; a Pacer per Fanout would start empty and pace nothing. var SearchPace = &Pacer{} @@ -19,7 +18,7 @@ const holdWithoutRetryAfter = 10 * time.Minute // Pacer spaces the searches sent to each source by its searches_per_hour: a // spacing, not a burst bucket, because Prowlarr answers 429 to a few requests -// in quick succession (FL #4304). Every search spends, the schedule's and a +// in quick succession. Every search spends, the schedule's and a // person's alike; only the schedule waits. type Pacer struct { // Now and Sleep are the clock, replaceable in tests. diff --git a/internal/acquire/search.go b/internal/acquire/search.go index 0610b04d..73960b3a 100644 --- a/internal/acquire/search.go +++ b/internal/acquire/search.go @@ -12,8 +12,8 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/release" ) -// One search for one want, and what could be done about each result (M502 -// #5521), ported from Fabled Librarian's internal/acquire. internal/release +// One search for one want, and what could be done about each result. +// internal/release // says whether a name is the album and what quality it is; this package // asks in the right order and says why in words a person can act on. @@ -51,7 +51,7 @@ const ( CodeNoSeeders = "no_seeders" CodeDoubt = "doubt" // CodeNotBetter: the album, but no better than the copy the library - // holds, for a want that is an upgrade (#5524). + // holds, for a want that is an upgrade. CodeNotBetter = "not_better" ) @@ -68,7 +68,7 @@ type Want struct { ID pgtype.UUID release.Want // Floor is the tier of the copy the library holds, for an album wanted - // for a better one (#5524): a release must rank above it. "" for an + // for a better one: a release must rank above it. "" for an // album not held, and for a video rip, which any clean copy beats. Floor release.Tier // Held says the floor in words: "MP3, about 192 kbps". diff --git a/internal/acquire/search_test.go b/internal/acquire/search_test.go index 85ea3cf5..63022d0e 100644 --- a/internal/acquire/search_test.go +++ b/internal/acquire/search_test.go @@ -219,7 +219,7 @@ func TestGrab_AClientThatAlreadyHasItCounts(t *testing.T) { } // A duplicate the client does not list for Minstrel sits under another -// category, where it would read as gone half an hour later (#6176). It is set +// category, where it would read as gone half an hour later. It is set // aside rather than taken; forced, it is refused with the client's reason. func TestGrab_ADuplicateHeldOutsideMinstrelIsNotTaken(t *testing.T) { client := &fakeClient{addErr: fmt.Errorf("%w: already downloading Ohio Players (lidarr)", downloader.ErrDuplicate), elsewhere: true} diff --git a/internal/acquire/settings.go b/internal/acquire/settings.go index 01d4b92a..e71284a3 100644 --- a/internal/acquire/settings.go +++ b/internal/acquire/settings.go @@ -1,6 +1,5 @@ -// Package acquire turns the operator's configured indexers and download -// clients into working clients (M502 #5518). Ported from Fabled Librarian's -// internal/acquire. +// Package acquire turns the configured indexers and download clients into +// working clients. // // ONE BUILDER PER KIND. SourceFor makes a search source from a row and // DownloadClient makes a download client, path mapping included. The grab, @@ -60,7 +59,7 @@ type addable struct { perHour int32 } -// catalogue is every protocol the operator can add. Exchange starts unpaced +// catalogue is every protocol an admin can add. Exchange starts unpaced // (it searches its own database); the others at 120 searches an hour, which // keeps a Prowlarr's indexers under the rate most trackers allow. var catalogue = map[string]addable{ @@ -70,7 +69,7 @@ var catalogue = map[string]addable{ ProtocolExchange: {kind: KindDownload, label: "FabledExchange", category: DefaultCategory, timeoutMs: 30000, perHour: 0}, } -// Addable says whether protocol is one the operator can add. +// Addable says whether protocol is one an admin can add. func Addable(protocol string) bool { _, ok := catalogue[protocol] return ok diff --git a/internal/acquire/store.go b/internal/acquire/store.go index 833435b9..0245be01 100644 --- a/internal/acquire/store.go +++ b/internal/acquire/store.go @@ -19,7 +19,7 @@ var ErrNotFound = errors.New("no such service") // ErrTooMany is a twenty-first row of one protocol. var ErrTooMany = errors.New("there are already twenty of these") -// InvalidError is a setting the operator has to change, in words for the +// InvalidError is a setting an admin has to change, in words for the // settings card. type InvalidError struct { Field string diff --git a/internal/audit/audit.go b/internal/audit/audit.go index ff870fe8..c88b2048 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -49,25 +49,23 @@ const ( ActionForgotPasswordInit Action = "forgot_password_initiated" ActionPasswordResetByEmail Action = "password_reset_via_email" - // Active-sessions surface (#370). Worth auditing rather than silent: - // revoking sessions is what a user does when they think an account is - // compromised, so the audit trail is most useful precisely when it's - // exercised. + // Revoking sessions is what a user does when they think an account is + // compromised, so it is audited: the trail matters most exactly then. ActionSessionRevoke Action = "session_revoke" ActionSessionRevokeOthers Action = "session_revoke_others" - // Duplicate merge (#3911). Irreversible: a copy's file and row are removed + // Duplicate merge. Irreversible: a copy's file and row are removed // and its history moved onto the copy kept. The metadata names both, so the // log can answer "where did that file go" long after the report is gone. ActionDuplicateMerge Action = "duplicate_merge" - // The resolver settling a group without a merge (M498, 2026-10-19): + // The resolver settling a group without a merge: // dismissing two versions of a song that are not duplicates, and removing // copies that filled another song's track slot (their tracks are kept, // marked missing, and fetched again). ActionDuplicateDismiss Action = "duplicate_dismiss" ActionDuplicateWrongFile Action = "duplicate_wrong_file" - // Subsonic password (#5026): generated in Settings for t/s-only clients. + // Subsonic password: generated in Settings for t/s-only clients. ActionSubsonicPasswordSet Action = "subsonic_password_set" ActionSubsonicPasswordClear Action = "subsonic_password_clear" ) diff --git a/internal/auth/clientip.go b/internal/auth/clientip.go index 3b741dc4..be7b450d 100644 --- a/internal/auth/clientip.go +++ b/internal/auth/clientip.go @@ -7,7 +7,7 @@ import ( ) // ClientIP returns the caller's address, reading through trustedProxyHops -// reverse proxies (#2453). +// reverse proxies. // // X-Forwarded-For grows left-to-right: every proxy APPENDS the peer it // received the request from. For client -> CDN -> own-proxy -> Minstrel the @@ -20,13 +20,12 @@ import ( // honestly claim is the address it told us about // hops 2 -> XFF[0] = client // -// This replaces an earlier heuristic that ignored XFF whenever RemoteAddr was -// public. That was safe but useless in the deployment that matters: a proxy -// on a public address (separate host, or a CDN) meant every session recorded -// the proxy, so the active-sessions surface could never show an address -// change (#370). +// Ignoring XFF whenever RemoteAddr is public would be safe but useless: behind +// a proxy on a public address (a separate host, or a CDN) every session would +// record the proxy, and the active-sessions list could never show an address +// change. // -// # What the operator is asserting +// # What the hop count asserts // // hops >= 1 is a DECLARATION that a proxy sits in front. Two ways to get it // wrong, both worth understanding rather than papering over: @@ -36,9 +35,9 @@ import ( // - Set HIGHER than the real chain: the index runs past the proxy-written // entries into attacker-supplied ones, same result. // -// Both are inherent to the trusted-hop model — Rails, Caddy, Traefik and -// nginx all behave this way — which is why 0 is a first-class value and the -// admin card tells the operator to count their proxies. +// Both are inherent to the trusted-hop model (Rails, Caddy, Traefik and nginx +// behave the same way), which is why 0 is the default and the admin card asks +// for the number of proxies in front. func ClientIP(r *http.Request, trustedProxyHops int) string { remote := hostOf(r.RemoteAddr) if trustedProxyHops <= 0 { @@ -51,7 +50,7 @@ func ClientIP(r *http.Request, trustedProxyHops int) string { return remote } // Clamp rather than reject: a chain shorter than the configured depth - // means the operator over-counted, and the leftmost entry is the closest + // means the hops were over-counted, and the leftmost entry is the closest // thing to a client on offer. The caveat above covers the risk. idx := len(chain) - trustedProxyHops if idx < 0 { @@ -68,8 +67,8 @@ func ClientIP(r *http.Request, trustedProxyHops int) string { // IsHTTPS reports whether the client reached us over HTTPS, reading through // trustedProxyHops reverse proxies by the same rule as ClientIP. // -// TLS terminates at the operator's proxy (rule 94), so r.TLS is nil in every -// real deployment and only X-Forwarded-Proto can say what the client used. +// TLS terminates at the reverse proxy, so r.TLS is nil in every real +// deployment and only X-Forwarded-Proto can say what the client used. // That header is believed only when hops >= 1: with no trusted proxy it is // just something any client can send. Proxies that append rather than // overwrite produce a comma list, read positionally like X-Forwarded-For. diff --git a/internal/auth/clientip_test.go b/internal/auth/clientip_test.go index 50a330d5..f13f52f3 100644 --- a/internal/auth/clientip_test.go +++ b/internal/auth/clientip_test.go @@ -68,8 +68,8 @@ func TestClientIP(t *testing.T) { want: "198.51.100.7", }, { - // A client prepending a lie is only reachable if the operator - // over-counts their proxies; at the correct depth it's skipped. + // A client prepending a lie is only reachable if the hops are + // over-counted; at the correct depth it's skipped. name: "forged prefix is not reached at the correct depth", hops: 1, remoteAddr: "172.18.0.1:40000", @@ -178,7 +178,7 @@ func TestIsHTTPS(t *testing.T) { tls bool want bool }{ - // The three cases rule 94 names. + // No trusted proxy, a trusted proxy saying https, and one saying http. {name: "hops 0 ignores a forwarded https", hops: 0, proto: "https", want: false}, {name: "hops 1 believes a forwarded https", hops: 1, proto: "https", want: true}, {name: "plain request is not https", hops: 1, want: false}, diff --git a/internal/auth/session.go b/internal/auth/session.go index 8f2ecdfa..2d4c45df 100644 --- a/internal/auth/session.go +++ b/internal/auth/session.go @@ -68,8 +68,8 @@ const SessionCookieName = "minstrel_session" // leak whether the username existed (matches the /rest/* auth posture). // // trustedHops supplies the reverse-proxy depth used to record the session's -// current address (#2453). It's a func rather than an int because the value -// is operator-editable at runtime and this middleware is constructed once at +// current address. It's a func rather than an int because the value +// is editable at runtime and this middleware is constructed once at // boot — reading it per request is what makes an admin change take effect // without a restart. Passing nil means "trust nothing", i.e. the socket peer. func RequireUser(pool *pgxpool.Pool, trustedHops func() int) func(http.Handler) http.Handler { @@ -155,7 +155,7 @@ func SessionIDCtxKeyForTest() any { return sessionIDCtxKey } // // Database lookup failures fall through too — a transient DB blip should not // 5xx a stream request that may have a perfectly valid signed token. The -// error is logged so the operator can correlate. +// error is logged so it can be correlated. func OptionalUser(pool *pgxpool.Pool, logger *slog.Logger) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/auth/setup.go b/internal/auth/setup.go index 9d576be7..cb0b6848 100644 --- a/internal/auth/setup.go +++ b/internal/auth/setup.go @@ -7,10 +7,10 @@ import ( ) // SetupToken guards the first-admin registration. Until the first account -// exists, register makes whoever calls it the admin, so a fresh instance on a -// public address belonged to whoever found it first. Now that call also has -// to carry this token, which is generated at startup and written only to the -// server log: proof that the caller can read the operator's logs. +// exists, register makes whoever calls it the admin, so without this a fresh +// instance on a public address would belong to whoever found it first. The +// call must carry this token, which is generated at startup and written only +// to the server log: proof that the caller can read the server's logs. // // The token lives in memory. A restart mints a new one and logs it again, // which is the behaviour wanted: an old token from a log line someone else diff --git a/internal/buildinfo/buildinfo.go b/internal/buildinfo/buildinfo.go index 04461efc..67bc4d8b 100644 --- a/internal/buildinfo/buildinfo.go +++ b/internal/buildinfo/buildinfo.go @@ -14,20 +14,18 @@ import "time" // release.yml passes it through the MINSTREL_VERSION build-arg → Dockerfile // ldflag. // -// This carried the literal strings "main" and "dev" until 2026-09-10, which -// made every image on a channel report the same thing forever. It stopped -// being cosmetic when :vYYYY.MM.DD.HHMM image tags were retired (family rule -// 145): this is now the ONLY thing that says which build is running. +// Image tags carry no version, so this is the only thing that says which +// build is running. var Version = "dev" // Channel is which line this build came off — "stable" or "dev", or "local" // for a plain `docker build`. // -// A SIBLING FIELD, never a suffix inside Version (family rule 149). The same +// A sibling field, never a suffix inside Version. The same // commit built on both lanes reports the same version and differs only here; // folding the two together is what makes a version string stop being // comparable. var Channel = "local" -// Started is when this process started, for the Overview's uptime (M504). +// Started is when this process started, for the admin Overview's uptime. var Started = time.Now() diff --git a/internal/catalogue/art.go b/internal/catalogue/art.go index ca60066d..0d1a1b63 100644 --- a/internal/catalogue/art.go +++ b/internal/catalogue/art.go @@ -17,7 +17,7 @@ import ( // the sources are asked again. const NoArtRetry = 7 * 24 * time.Hour -// fetchTimeout bounds one look-up across every source (rule 156). +// fetchTimeout bounds one look-up across every source. const fetchTimeout = 30 * time.Second // ErrNoArt: no enabled source has an image, or the sources could not be @@ -32,7 +32,7 @@ func ValidMBID(s string) bool { return mbidPattern.MatchString(s) } // Art serves images for artists and release groups that are not in the // library (search results, Discover suggestions, wanted albums), from the -// operator's enabled cover-art sources. Each image is fetched once and kept +// enabled cover-art sources. Each image is fetched once and kept // under the data directory, so the browser only ever talks to Minstrel. type Art struct { dir string @@ -51,7 +51,7 @@ type artCall struct { } // ArtSources lists the image sources to ask, in order. The cover-art -// settings service is one: it answers with what the operator has enabled. +// settings service is one: it answers with the sources an admin enabled. type ArtSources interface { EnabledArtistProviders() []coverart.ArtistArtProvider EnabledAlbumProviders() []coverart.AlbumCoverProvider diff --git a/internal/catalogue/catalogue.go b/internal/catalogue/catalogue.go index 92af3b3d..ada4fe47 100644 --- a/internal/catalogue/catalogue.go +++ b/internal/catalogue/catalogue.go @@ -1,5 +1,5 @@ // Package catalogue is Minstrel's own record of what artists have released, -// read from MusicBrainz and kept in the database (M502 #5519). The wanted +// read from MusicBrainz and kept in the database. The wanted // list, search and import read it; MusicBrainz is asked only when what is // held is missing or old. package catalogue diff --git a/internal/config/config.go b/internal/config/config.go index aec1f643..a7d978fa 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -24,12 +24,10 @@ type Config struct { Recommendation RecommendationConfig `yaml:"recommendation"` Branding BrandingConfig `yaml:"branding"` // StreamSecret is the base64-decoded HMAC key used to sign UPnP / - // Sonos stream URLs (see internal/api/stream_token.go and - // docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md). - // Sourced from MINSTREL_STREAM_SECRET (base64-encoded), with a - // per-machine fallback persisted at /stream_secret. - // Not serialized to YAML — operator-machine-scoped runtime state, not - // a user-facing setting. Never log the contents. + // Sonos stream URLs (see internal/api/stream_token.go). Sourced from + // MINSTREL_STREAM_SECRET (base64-encoded), with a per-machine fallback + // persisted at /stream_secret. Not read from YAML: it + // is per-machine state, not a setting. Never log the contents. StreamSecret []byte `yaml:"-"` } @@ -37,12 +35,8 @@ type ServerConfig struct { Address string `yaml:"address"` } -// StorageConfig points at on-disk locations the server uses for cached -// runtime artifacts. Currently: -// - DataDir hosts /playlist_covers/{playlist_id}.jpg (M7 #352). -// -// More entries may grow here (e.g., transcode cache) without breaking -// existing layouts. +// StorageConfig points at the on-disk root for generated files: playlist +// cover collages, artwork, caches and the stream secret. type StorageConfig struct { // DataDir is the root for cached artifacts. Default: "./data" (relative // to the working directory). Operators in production should set an @@ -88,10 +82,9 @@ type EventsConfig struct { SkipMaxDurationPlayedMs int `yaml:"skip_max_duration_played_ms"` } -// RecommendationConfig holds the radio path's operational knobs. The -// scoring WEIGHTS moved to DB-backed admin settings (#1250, -// internal/recsettings) — YAML is bootstrap-only; anything an operator -// tunes lives in the UI with live effect. +// RecommendationConfig holds the radio path's operational knobs. The scoring +// weights are DB-backed admin settings (internal/recsettings), tuned in the +// UI with live effect. type RecommendationConfig struct { RecentlyPlayedHours int `yaml:"recently_played_hours"` RadioSize int `yaml:"radio_size"` @@ -120,8 +113,7 @@ func Default() Config { }, Library: LibraryConfig{ // On by default so a fresh install populates its library on - // first boot without the operator hunting for a "scan now" - // button. Scans are incremental (unchanged files skipped by + // first boot with no manual step. Scans are incremental (unchanged files skipped by // mtime), so every later restart is just a cheap directory walk. ScanOnStartup: true, CoverArtFromMBCAA: true, @@ -159,15 +151,13 @@ const streamSecretBytes = 64 const streamSecretFile = "stream_secret" // resolveStreamSecret populates cfg.StreamSecret using, in order: -// 1. MINSTREL_STREAM_SECRET env var (operator-supplied, base64-url -// encoded — accepts either padded or raw form). +// 1. MINSTREL_STREAM_SECRET env var (base64-url encoded — accepts either padded or raw form). // 2. /stream_secret if present (auto-generated on a // previous boot; survives restarts so signed URLs stay valid). // 3. Auto-generate streamSecretBytes random bytes, persist them to that // file at 0600, then use them. // -// Logs (only) when an auto-generation happens so the operator can spot -// it during first-boot. Never logs the contents. +// Logs (only) when an auto-generation happens, so it shows on first boot. Never logs the contents. // // The loader runs before slog is initialized in cmd/minstrel/main.go; // log.Printf is the project's pre-logger convention. diff --git a/internal/coplay/worker.go b/internal/coplay/worker.go index ed2c0435..732508e9 100644 --- a/internal/coplay/worker.go +++ b/internal/coplay/worker.go @@ -1,11 +1,10 @@ -// Package coplay builds household co-play similarity edges (#1533, milestone -// #160 Opt 5). Minstrel is a single shared-library, multi-user server (no -// per-user library ACL), so the "household" is the whole instance's user set. -// A periodic worker recomputes artist–artist co-occurrence from play_events -// entirely in SQL (no external API) and stores the edges in artist_similarity -// under source='user_cooccurrence' — the pre-provisioned co-occurrence source -// from the 0009 schema — which the radio/mix candidate pool consumes as a -// collaborative arm. Single-user servers produce no edges (the >= 2 co-player +// Package coplay builds household co-play similarity edges. Minstrel is a +// single shared-library, multi-user server (no per-user library ACL), so the +// "household" is the whole instance's user set. A periodic worker recomputes +// artist–artist co-occurrence from play_events entirely in SQL (no external +// API) and stores the edges in artist_similarity under +// source='user_cooccurrence', which the radio/mix candidate pool consumes as +// a collaborative arm. Single-user servers produce no edges (the >= 2 co-player // gate), so the feature is a graceful no-op there. package coplay diff --git a/internal/coverart/artist_enricher.go b/internal/coverart/artist_enricher.go index 48187a42..2af0a052 100644 --- a/internal/coverart/artist_enricher.go +++ b/internal/coverart/artist_enricher.go @@ -141,11 +141,11 @@ func (e *Enricher) recordArtistArt(ctx context.Context, artistID pgtype.UUID, th // // Emits a single Info log line at end-of-batch with a category // breakdown (succeeded / settled-none / no-MBID / left-NULL / -// errored). The JSON tally written to scan_runs keeps the existing -// processed/succeeded/failed shape for backwards compat with the -// admin overview UI; the log line is the operator's diagnostic. -// limit: 0 = stage disabled; >0 = bounded; <0 = unbounded (#388 — -// no global cap; remote providers self-throttle per provider). +// errored). The JSON tally written to scan_runs keeps the +// processed/succeeded/failed shape the admin overview reads; the log line +// is the diagnostic. +// limit: 0 = stage disabled; >0 = bounded; <0 = unbounded (remote +// providers pace themselves). func (e *Enricher) EnrichArtistBatch(ctx context.Context, limit int, dataDir string, progressCb func(processed, succeeded, failed int)) (processed, succeeded, failed int, err error) { if limit == 0 { diff --git a/internal/coverart/enricher.go b/internal/coverart/enricher.go index 09e4e3d2..34f6d9f9 100644 --- a/internal/coverart/enricher.go +++ b/internal/coverart/enricher.go @@ -210,13 +210,12 @@ func (e *Enricher) recordAlbumCover(ctx context.Context, albumID pgtype.UUID, pa // // Emits a single Info log line at end-of-batch with a category // breakdown (succeeded / settled-none / left-NULL / errored). The -// JSON tally written to scan_runs keeps the existing -// processed/succeeded/failed shape for backwards compat with the -// admin overview UI; the log line is the operator's diagnostic. +// JSON tally written to scan_runs keeps the processed/succeeded/failed +// shape the admin overview reads; the log line is the diagnostic. // limit semantics: 0 = stage disabled (skip); >0 = bounded batch; -// <0 = unbounded — walk every album needing art. The global cap was -// removed (#388); external providers self-throttle via their -// per-provider httpClient MinInterval, local sources run at disk speed. +// <0 = unbounded — walk every album needing art. External providers pace +// themselves via their httpClient MinInterval; local sources run at disk +// speed. func (e *Enricher) EnrichBatch(ctx context.Context, limit int, progressCb func(processed, succeeded, failed int)) (processed, succeeded, failed int, err error) { if limit == 0 { @@ -302,7 +301,7 @@ func (e *Enricher) logBatchSummary(stage string, eligible, processed, succeeded, // admin bulk-retry endpoint. Clears 'none' to NULL first so // EnrichAlbum picks them up. // limit: 0 = no-op; >0 = bounded; <0 = unbounded (retry every -// missing/none album). Admin bulk-retry passes <0 post-#388. +// missing/none album). Admin bulk-retry passes <0. func (e *Enricher) EnrichRetryMissing(ctx context.Context, limit int) (int, error) { if limit == 0 { return 0, nil @@ -346,9 +345,8 @@ func (e *Enricher) RetryAlbum(ctx context.Context, albumID pgtype.UUID) error { } return fmt.Errorf("get album: %w", err) } - // Delete the file only if we previously wrote it (mbcaa or - // theaudiodb). Operator sidecars stay; embedded extracts (future) - // live in a managed cache with its own lifecycle. + // Delete the file only if we wrote it (mbcaa or theaudiodb). Sidecar + // images placed beside the music stay. if row.CoverArtSource != nil && (*row.CoverArtSource == "mbcaa" || *row.CoverArtSource == "theaudiodb") && row.CoverArtPath != nil && *row.CoverArtPath != "" { diff --git a/internal/coverart/provider.go b/internal/coverart/provider.go index 122f02dd..29068f61 100644 --- a/internal/coverart/provider.go +++ b/internal/coverart/provider.go @@ -16,8 +16,7 @@ import ( type Provider interface { // ID is a stable lowercase token persisted in DB columns // (cover_art_source / artist_art_source) and the - // cover_art_provider_settings.provider_id key, plus referenced - // in the operator-facing UI. + // cover_art_provider_settings.provider_id key, and shown in the admin UI. ID() string // DisplayName is the human-readable label shown in the admin @@ -36,7 +35,7 @@ type Provider interface { // with this provider on. Both v1 providers return true. DefaultEnabled() bool - // Configure receives the operator's settings (api_key, enabled + // Configure receives the admin's settings (api_key, enabled // flag) and applies them. Called at boot during reconciliation // and on every admin PATCH. Implementations must be idempotent // and goroutine-safe (the enricher may be reading concurrently). @@ -84,7 +83,7 @@ type AlbumCoverProvider interface { // ReleaseGroupCoverProvider is an opt-in capability for sources that can // answer by MusicBrainz release group, which is how the release catalogue -// names an album before any edition of it is held (M502 #5519). +// names an album before any edition of it is held. type ReleaseGroupCoverProvider interface { AlbumCoverProvider FetchReleaseGroupCover(ctx context.Context, groupMBID string) ([]byte, error) diff --git a/internal/coverart/provider_mbcaa.go b/internal/coverart/provider_mbcaa.go index 35975f5d..0bcc1f25 100644 --- a/internal/coverart/provider_mbcaa.go +++ b/internal/coverart/provider_mbcaa.go @@ -11,7 +11,7 @@ import ( "time" ) -// FetcherConfig assembles the operator-tunable knobs for the MBCAA +// FetcherConfig assembles the tunable knobs for the MBCAA // HTTP client. Kept exported because main.go's boot wiring constructs // one to pass into NewMBCAAProviderFromConfig. type FetcherConfig struct { @@ -97,7 +97,7 @@ func (p *mbcaaProvider) FetchAlbumCover(ctx context.Context, ref AlbumRef) ([]by // FetchReleaseGroupCover retrieves the 500px front cover the Cover Art // Archive chose for a release group: what a search result or a wanted album -// shows before any edition of it is held (M502 #5519). +// shows before any edition of it is held. func (p *mbcaaProvider) FetchReleaseGroupCover(ctx context.Context, groupMBID string) ([]byte, error) { return p.fetchFront(ctx, "release-group", groupMBID) } diff --git a/internal/coverart/settings.go b/internal/coverart/settings.go index ab025cb7..d2e51b14 100644 --- a/internal/coverart/settings.go +++ b/internal/coverart/settings.go @@ -270,10 +270,9 @@ func enabledSetSignature(enabledIDs map[string]bool) string { // unchanged, returns (currentVersion, false, nil) without touching the DB. // // Used at boot to ensure rows previously settled to 'none' get re-tried -// once after a provider chain change (e.g. operator updates the binary -// and a new provider is registered, or one is removed). The version -// bump triggers eligibility for 'none' rows on the next enrichment pass -// without requiring a manual operator action. +// once after a provider chain change (an update registers a new provider, +// or removes one). The version bump makes 'none' rows eligible on the next +// enrichment pass with no manual step. func (s *SettingsService) BumpVersionIfProvidersChanged(ctx context.Context) (version int32, bumped bool, err error) { hash := registeredProvidersHash() q := dbq.New(s.pool) @@ -313,7 +312,7 @@ func registeredProvidersHash() string { // BumpVersion unconditionally increments cover_art_sources_meta.current_version // and returns the new value. Used by the manual "Re-search missing art" -// admin endpoint when the operator wants every 'none' row re-attempted +// admin endpoint, to re-attempt every 'none' row // on the next enrichment pass. // // Distinct from BumpVersionIfProvidersChanged: that method is the boot- diff --git a/internal/downloader/driver.go b/internal/downloader/driver.go index 90e3bf9d..2b1d8d99 100644 --- a/internal/downloader/driver.go +++ b/internal/downloader/driver.go @@ -1,6 +1,5 @@ -// Package downloader drives the download clients Minstrel hands a grab to -// (M502 #5518): qBittorrent's WebUI v2, and FabledExchange's native -// /api/downloads. Ported from Fabled Librarian's internal/downloader. +// Package downloader drives the download clients Minstrel hands a grab to: +// qBittorrent's WebUI v2, and FabledExchange's native /api/downloads. // // Both answer through one Driver, and Torrent is qBittorrent's shape: the // Exchange driver maps its own downloads onto it, so the grab, the importer @@ -58,7 +57,7 @@ type Config struct { // APIKey is Exchange's, sent as a bearer only when set. APIKey string // Category files everything Minstrel adds, so its torrents can be told - // from the operator's own. + // from anything else in the client. Category string Timeout time.Duration } diff --git a/internal/exchange/exchange.go b/internal/exchange/exchange.go index a5417b73..987c90bc 100644 --- a/internal/exchange/exchange.go +++ b/internal/exchange/exchange.go @@ -1,7 +1,6 @@ // Package exchange talks to FabledExchange's native API for the two jobs that -// are not downloading (M502 #5518): searching its DHT index (/api/search) and -// describing one torrent (/api/torrents/{infohash}). Downloading is -// internal/downloader's. Ported from Fabled Librarian's internal/exchange. +// are not downloading: searching its DHT index (/api/search) and describing +// one torrent (/api/torrents/{infohash}). Downloading is internal/downloader's. // // The native search rather than Exchange's Torznab feed, because it reports // whether a swarm was measured and how many files a torrent holds, which the diff --git a/internal/gc/worker.go b/internal/gc/worker.go index e0d31068..c88ad965 100644 --- a/internal/gc/worker.go +++ b/internal/gc/worker.go @@ -1,8 +1,6 @@ -// Package gc runs periodic garbage-collection / lifecycle sweeps -// against tables that have NO writer-side close path or NO retention -// policy. Each sweep addresses a drift finding from the 2026-06-02 -// audit (Scribe parent #552) and is idempotent — re-running it on -// already-clean rows is a no-op. +// Package gc runs periodic garbage-collection / lifecycle sweeps against +// tables that have no writer-side close path or no retention policy. Each +// sweep is idempotent: re-running it on already-clean rows is a no-op. // // One Worker handles all sweeps so a single long-tick goroutine // amortises the per-tick fixed cost. Each individual sweep is small @@ -11,16 +9,16 @@ // visible in the application log without an explicit metrics layer. // // Sweeps: -// - GcCloseStalePlayEvents (#566) -// - GcClosePlaySessionsWithNoRecentEvents (#565) -// - GcExpireScrobbleQueueFailedRows (#567) -// - GcResetStuckSystemPlaylistRuns (#574) -// - GcDeleteExpiredPasswordResets (#575) -// - GcDeleteExpiredSessions (M462 #4978 — idle 30d / absolute 1y) -// - GcPruneDiagnostics (M9 — diagnostics 30d retention) -// - GcDeleteExpiredSuggestionSnoozes (#2374 — snoozes expire, then go) -// - GcDeleteOrphanedCandidateArtistTags(+State) (#2376 — the similarity -// feed churns, so cached candidate tags outlive their candidates) +// - GcCloseStalePlayEvents +// - GcClosePlaySessionsWithNoRecentEvents +// - GcExpireScrobbleQueueFailedRows +// - GcResetStuckSystemPlaylistRuns +// - GcDeleteExpiredPasswordResets +// - GcDeleteExpiredSessions (idle 30d / absolute 1y) +// - GcPruneDiagnostics (30d retention) +// - GcDeleteExpiredSuggestionSnoozes (snoozes expire, then go) +// - GcDeleteOrphanedCandidateArtistTags(+State) (the similarity feed +// churns, so cached candidate tags outlive their candidates) package gc import ( diff --git a/internal/importer/importer.go b/internal/importer/importer.go index d075311f..024c96c4 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -64,11 +64,11 @@ type Importer struct { Catalogue Catalogue Printer Printer // Roots are the library's folders (the scan paths), read on every use - // because they change in Settings (#6129). New music is filed under the + // because they change in Settings. New music is filed under the // one the settings name, or the first. Roots func() []string // DataDir is where cached art lives, for tidying an artist an upgrade - // leaves with nothing (#5524). Empty skips it. + // leaves with nothing. Empty skips it. DataDir string // ClientFor builds the download client a grab went to. Defaults to // acquire.DownloadClientFor. @@ -195,7 +195,7 @@ func (im *Importer) passClient(ctx context.Context, q *dbq.Queries, code string, } t, ok := byHash[indexer.NormalizeHash(g.InfoHash)] if !ok { - continue // gone from the client: the lifecycle settles it (#5523) + continue // gone from the client: the lifecycle settles it } if !g.WantID.Valid { wait(g, "Its album is no longer wanted.") diff --git a/internal/importer/inspect.go b/internal/importer/inspect.go index e13359a2..e727b4cb 100644 --- a/internal/importer/inspect.go +++ b/internal/importer/inspect.go @@ -26,7 +26,7 @@ type Printer interface { var ErrNoPrints = errors.New("importer: fingerprints are not available") // AcoustIDPrinter fingerprints with fpcalc and asks AcoustID, under the -// operator's settings (key, minimum score). Each call reads the settings, so +// AcoustID settings (key, minimum score). Each call reads the settings, so // a change takes effect on the next file. type AcoustIDPrinter struct { Settings interface { diff --git a/internal/importer/name.go b/internal/importer/name.go index 60bc9f4d..b6a15a5b 100644 --- a/internal/importer/name.go +++ b/internal/importer/name.go @@ -21,7 +21,7 @@ type Placement struct { } // Plan names every file's destination under root the way the library is -// already named (note #5527), so new arrivals sit beside what is there: +// conventionally named, so new arrivals sit beside what is there: // // {Artist}/{Album} ({Year})/{Artist} - {Album} - {track:00} - {Title}.ext // diff --git a/internal/importer/place.go b/internal/importer/place.go index 03d6a2a9..090091ea 100644 --- a/internal/importer/place.go +++ b/internal/importer/place.go @@ -40,9 +40,9 @@ type Tags struct { // gets the plain names only. // // A Vorbis track or disc number is the number alone, its count in -// TRACKTOTAL and DISCTOTAL, as Picard writes them: dhowden/tag reads -// TRACKNUMBER "3/20" as 0, and every album imported before #6171 lost its -// order that way. ID3 and MP4 take "3/20", which their readers parse. +// TRACKTOTAL and DISCTOTAL, as Picard writes them: dhowden/tag reads a +// TRACKNUMBER of "3/20" as 0, which would lose the album's order. ID3 and MP4 +// take "3/20", which their readers parse. func (t Tags) args(ext string) []string { vorbis := ext == ".flac" || ext == ".ogg" || ext == ".opus" track, disc := ofN(t.Track, t.Tracks), ofN(t.Disc, t.Discs) @@ -111,7 +111,7 @@ func Place(ctx context.Context, src, dst string, t Tags) (tagged bool, err error return placeFile(ctx, src, dst, t, false) } -// Overwrite is Place onto a file an upgrade replaces (#5524): the new copy +// Overwrite is Place onto a file an upgrade replaces: the new copy // takes the old one's path in one rename, so the library never sees the // path empty, and the track row at that path, history and all, carries on // with the new file. diff --git a/internal/importer/upgrade.go b/internal/importer/upgrade.go index 8a75272f..d40f9c08 100644 --- a/internal/importer/upgrade.go +++ b/internal/importer/upgrade.go @@ -17,7 +17,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/release" ) -// Importing an upgrade (M502 #5524): a better copy of an album the library +// Importing an upgrade: a better copy of an album the library // holds. Each file is matched to the track it would replace, by recording // and then by disc and track. It replaces that track when the track is a // video rip or the new file is a better tier; otherwise the old track stays. diff --git a/internal/importer/verify.go b/internal/importer/verify.go index a1f50166..9181d46c 100644 --- a/internal/importer/verify.go +++ b/internal/importer/verify.go @@ -1,13 +1,11 @@ // Package importer checks a finished download against the album it was -// grabbed for and files it into the library (M502 #5522). Nothing reaches +// grabbed for and files it into the library. Nothing reaches // the library before the check says the files are the album: a wrong album, // another version of it, a video rip or an incomplete copy is refused, its // release is blocked and the album is searched for again. // -// Ported from Fabled Librarian's importer and relocate packages, with the -// check rewritten for music: the files are matched to one of the album's -// MusicBrainz editions by AcoustID fingerprints, track lengths, titles and -// the tags they carry. +// The files are matched to one of the album's MusicBrainz editions by +// AcoustID fingerprints, track lengths, titles and the tags they carry. package importer import ( @@ -67,7 +65,7 @@ type File struct { RecordingMBID string LengthMs int // Recordings are the MusicBrainz recordings AcoustID says the audio is, - // at or above the operator's minimum score. + // at or above the configured minimum score. Recordings []string // Video: the file name says it is a music video, a lyric video or the // like (library.SourceMarkersFor). @@ -113,7 +111,7 @@ type Want struct { Year int PrimaryType string SecondaryTypes []string - // Upgrade: a held album wanted for a better copy (#5524), quality or + // Upgrade: a held album wanted for a better copy, quality or // rip; AlbumID is the library album it holds, whose tracks the import // replaces. Upgrade string diff --git a/internal/importer/verify_test.go b/internal/importer/verify_test.go index 21909098..e3208fb7 100644 --- a/internal/importer/verify_test.go +++ b/internal/importer/verify_test.go @@ -220,7 +220,7 @@ func TestTagsArgs(t *testing.T) { t.Errorf("flac args lack %q: %s", s, flac) } } - // dhowden/tag reads a Vorbis TRACKNUMBER of "8/13" as 0 (#6171). + // dhowden/tag reads a Vorbis TRACKNUMBER of "8/13" as 0. if strings.Contains(flac, "/13") { t.Errorf("flac track number carries its count: %s", flac) } diff --git a/internal/indexer/indexer.go b/internal/indexer/indexer.go index df554a94..d6a8af17 100644 --- a/internal/indexer/indexer.go +++ b/internal/indexer/indexer.go @@ -1,5 +1,5 @@ // Package indexer is what every search source answers with, and the Torznab -// client (M502 #5518). Ported from Fabled Librarian's internal/indexer. +// client. // // A client here holds ONE endpoint and knows nothing about the others. The // fan-out across sources, the merge and what a source being down means belong @@ -7,9 +7,8 @@ // about one source. // // Prowlarr is not served from here: it has no aggregate Torznab feed (`all` -// 404s like any id it does not have, measured by Fabled Librarian, #4304), so -// asking it everything at once means its native API, which internal/prowlarr -// speaks. +// 404s like any id it does not have), so asking it everything at once means +// its native API, which internal/prowlarr speaks. package indexer import ( @@ -34,7 +33,7 @@ var ErrUnreachable = errors.New("indexer unreachable") // field in Settings, an outage is something to wait out. var ErrAuth = errors.New("the API key was refused") -// UserAgent names Minstrel in the operator's own Prowlarr and indexer logs. +// UserAgent names Minstrel in Prowlarr's and each indexer's logs. const UserAgent = "Minstrel (self-hosted music server; release search)" // MusicCategories are the Newznab categories a music search asks for: 3000 diff --git a/internal/indexer/torznab.go b/internal/indexer/torznab.go index 03c9a5f4..8a458b1b 100644 --- a/internal/indexer/torznab.go +++ b/internal/indexer/torznab.go @@ -23,9 +23,8 @@ type TorznabConfig struct { // FeedURL is the feed's whole address up to and including /api, as // Jackett or a Prowlarr indexer shows it — e.g. // http://prowlarr:9696/12/api. Query parameters are appended to it. - // One field rather than Fabled Librarian's base + path + indexer id: a - // pasted URL is more reliable than Minstrel being right about a third - // party's routes. + // One field rather than base + path + indexer id: a pasted URL is more + // reliable than Minstrel being right about a third party's routes. FeedURL string // APIKey is optional: Torznab makes it so. A feed that needs one says so // with Newznab error 100, which is ErrAuth. diff --git a/internal/lifecycle/lifecycle.go b/internal/lifecycle/lifecycle.go index 4e173b52..23102f4d 100644 --- a/internal/lifecycle/lifecycle.go +++ b/internal/lifecycle/lifecycle.go @@ -1,6 +1,4 @@ -// Package lifecycle looks after a download from the grab to its removal -// (M502 #5523), ported from Fabled Librarian's internal/health and its seed -// release (System 104). +// Package lifecycle looks after a download from the grab to its removal. // // - A download the client has failed is given up at once. // - One nobody has shared, or still fetching its file list, after the @@ -75,7 +73,7 @@ const ( // goneAfter: a client restarting can answer without a download for a // while; half an hour missing is gone. goneAfter = 30 * time.Minute - // goneWindow: a release gone twice within it is held back (#6176). + // goneWindow: a release gone twice within it is held back. goneWindow = 7 * 24 * time.Hour ) @@ -335,7 +333,7 @@ func (s *Service) judge(ctx context.Context, q *dbq.Queries, d downloader.Driver } // The client's own verdict, given up on at once: it has already made - // the decision a retry would ask about (FL #4749). + // the decision a retry would ask about. if t.Failed() { why := t.FailReason if why == "" { @@ -445,8 +443,8 @@ func (s *Service) giveUp(ctx context.Context, q *dbq.Queries, d downloader.Drive // endGone ends a grab whose download left the client, and searches again. // The first time, the release is not held back: nothing was found wrong with // it. A release that leaves its client again within goneWindow is held back for -// acquire.GoneBlock (#6176): whatever keeps the client from listing it, taking -// it again on every pass found 38 grabs of one release in a day. +// acquire.GoneBlock: whatever keeps the client from listing it would otherwise +// have it taken again on every pass. func (s *Service) endGone(ctx context.Context, q *dbq.Queries, g dbq.ListLiveGrabsRow, rep *Report) { detail := "The download client no longer lists it: removed outside Minstrel. The album is searched for again." before, err := q.CountGoneGrabs(ctx, dbq.CountGoneGrabsParams{ diff --git a/internal/lifecycle/lifecycle_test.go b/internal/lifecycle/lifecycle_test.go index 61dc617e..b5e9277b 100644 --- a/internal/lifecycle/lifecycle_test.go +++ b/internal/lifecycle/lifecycle_test.go @@ -279,7 +279,7 @@ func TestLifecycle_ADownloadGoneFromTheClientEndsAfterHalfAnHour_Integration(t * } // A release that leaves its client a second time is held back, whatever the -// cause (#6176): one release was taken 40 times in a day, each ending gone. +// cause, so it is not taken again on every pass. func TestLifecycle_AReleaseGoneTwiceIsHeldBack_Integration(t *testing.T) { st := newStack(t, downloader.Torrent{Name: "Daft Punk - RAM [FLAC]", Progress: 0.3, State: "downloading", NumSeeds: 4}) ctx := context.Background() diff --git a/internal/logging/logging.go b/internal/logging/logging.go index e0dc48b0..80ca1041 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -23,7 +23,7 @@ func New(w io.Writer, level, format string) (*slog.Logger, error) { default: return nil, fmt.Errorf("unknown log format %q (want text|json)", format) } - // Errors are also kept for the admin Overview (M504). + // Errors are also kept for the admin Overview. return slog.New(newRingHandler(h, Errors)), nil } diff --git a/internal/logging/recent.go b/internal/logging/recent.go index 0aee1681..55c5f8a6 100644 --- a/internal/logging/recent.go +++ b/internal/logging/recent.go @@ -9,7 +9,7 @@ import ( "time" ) -// The server's recent errors, kept in memory for the admin Overview (M504): +// The server's recent errors, kept in memory for the admin Overview: // what has gone wrong since the process started, without reading the // container's logs. Only ERROR and above, and only the newest few; the full // record is still the log stream. diff --git a/internal/mood/mood.go b/internal/mood/mood.go index 7e855643..032a2743 100644 --- a/internal/mood/mood.go +++ b/internal/mood/mood.go @@ -1,5 +1,5 @@ -// Package mood maps folksonomy tags (from the track_tags cache, #1490) to a -// small set of canonical mood buckets (#1534, milestone #160 Opt 2b). Both the +// Package mood maps folksonomy tags (from the track_tags cache) to a small +// set of canonical mood buckets. Both the // taste-profile builder and the recommendation scorer classify a track's tags // through Of() so a track's mood is derived the same way on both sides. // diff --git a/internal/musicbrainz/client.go b/internal/musicbrainz/client.go index 2d906a76..86721968 100644 --- a/internal/musicbrainz/client.go +++ b/internal/musicbrainz/client.go @@ -20,7 +20,7 @@ const DefaultBaseURL = "https://musicbrainz.org/ws/2" // descriptive one. const UserAgent = "Minstrel/1.0 ( https://git.fabledsword.com/bvandeusen/minstrel )" -// DefaultTimeout bounds one request (rule 156). A browse page with track +// DefaultTimeout bounds one request. A browse page with track // lists is the slowest thing asked for. const DefaultTimeout = 20 * time.Second @@ -33,7 +33,8 @@ const maxPages = 25 var ( // ErrUnreachable: MusicBrainz could not be asked, or kept answering with - // its throttle or a server error. Callers degrade visibly (rule 164). + // its throttle or a server error. Callers degrade visibly: they say + // MusicBrainz is unreachable rather than show an empty answer. ErrUnreachable = errors.New("musicbrainz: unreachable") // ErrNotFound: MusicBrainz has nothing under that id. ErrNotFound = errors.New("musicbrainz: not found") diff --git a/internal/musicbrainz/pace.go b/internal/musicbrainz/pace.go index 4a0b3ed1..c000c75d 100644 --- a/internal/musicbrainz/pace.go +++ b/internal/musicbrainz/pace.go @@ -1,5 +1,5 @@ // Package musicbrainz is Minstrel's client for the MusicBrainz web service: -// searches and browses for the release catalogue (M502 #5519), and the one +// searches and browses for the release catalogue, and the one // pacer every MusicBrainz request in the process waits on. package musicbrainz diff --git a/internal/netsettings/service.go b/internal/netsettings/service.go index 49d23d1b..8fe4d31a 100644 --- a/internal/netsettings/service.go +++ b/internal/netsettings/service.go @@ -1,6 +1,6 @@ // Package netsettings holds the DB-backed network settings the request path -// needs. Today that's the trusted reverse-proxy depth used to pull a real -// client address out of X-Forwarded-For (#2453). +// needs: the trusted reverse-proxy depth used to pull a real client address +// out of X-Forwarded-For, and the public address links are built from. // // Values are cached under an RWMutex and refreshed on write. That isn't an // optimisation: auth.ClientIP runs in the RequireUser middleware for every @@ -53,7 +53,7 @@ type Service struct { // It ALWAYS returns a usable Service, even alongside a non-nil error. The // value it holds sits on the authenticated request path, so a boot-time // database hiccup must degrade to the default rather than take every request -// down with it (rule #131). The error is returned so the caller can log that +// down with it. The error is returned so the caller can log that // the cache holds a default rather than stored state. func New(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*Service, error) { s := &Service{pool: pool, logger: logger, hops: DefaultTrustedProxyHops} @@ -84,7 +84,7 @@ func (s *Service) Hops() int { } // SetHops persists a new depth and refreshes the cache, so an admin change -// takes effect on the next request with no restart (rule #25). +// takes effect on the next request with no restart. func (s *Service) SetHops(ctx context.Context, hops int) error { // Range first, availability second. The argument is wrong regardless of // whether the database is reachable, and the distinction is user-visible: @@ -115,7 +115,7 @@ func (s *Service) SetHops(ctx context.Context, hops int) error { return nil } -// PublicURL returns the operator-set address users reach Minstrel at, with no +// PublicURL returns the admin-set address users reach Minstrel at, with no // trailing slash, or "" when it hasn't been set. Links that leave the app (a // password-reset email) are built from this and never from the request's // Host header, which the requester controls. Nil-safe like Hops. diff --git a/internal/notifications/digest.go b/internal/notifications/digest.go index a1cd60c2..94b44211 100644 --- a/internal/notifications/digest.go +++ b/internal/notifications/digest.go @@ -15,10 +15,8 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/mailer" ) -// The email digest (#5346). Nothing is emailed per event. Operator, -// 2026-10-08: "music and stuff coming in should land as a summary email and -// not everytime same for a approvals the emails should be grouped by a -// reasonable amount of time like an hour or so." +// The email digest. Nothing is emailed per event: a steady stream of new +// music or approvals would otherwise be a stream of emails. // // - EmailSummary kinds (new music) go out at most once a day, at the summary // hour in the user's own timezone, listing everything since the last one. diff --git a/internal/notifications/email_settings.go b/internal/notifications/email_settings.go index 01903567..52cafe00 100644 --- a/internal/notifications/email_settings.go +++ b/internal/notifications/email_settings.go @@ -9,7 +9,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// EmailSettings are the digest's two knobs, in admin Settings (rule 25). +// EmailSettings are the digest's two knobs, edited in admin Settings. type EmailSettings struct { // SummaryHour is the local hour (0-23, in each user's own timezone) the // daily new-music summary goes out. diff --git a/internal/notifications/kinds.go b/internal/notifications/kinds.go index b7cfe1b1..1c985363 100644 --- a/internal/notifications/kinds.go +++ b/internal/notifications/kinds.go @@ -1,17 +1,16 @@ -// Package notifications is the one writer of the per-user notifications inbox -// (M489, #726). Producers across the server call Notifier.Notify; nothing else +// Package notifications is the one writer of the per-user notifications +// inbox. Producers across the server call Notifier.Notify; nothing else // inserts into user_notifications. // -// The event bus alone was fire-and-forget: a client that was not connected -// when a request completed, or when tracks went missing, never heard of it. A -// row here is the durable record. The bus only nudges open clients to come -// and read it. +// The event bus is fire-and-forget: a client that is not connected when a +// request completes, or when tracks go missing, never hears of it. A row here +// is the durable record. The bus only nudges open clients to come and read +// it. package notifications -// Kind names one sort of notification. The set is CHECK-gated in migration -// 0073, swapped since by 0075 (rule 36): a new kind adds its value in a -// migration of the same change, and TestEveryKindPassesTheSchemaChecks -// fails until it does. +// Kind names one sort of notification. The set is CHECK-gated in the schema: +// a new kind adds its value (DROP + ADD of the CHECK) in a migration of the +// same change, and TestEveryKindPassesTheSchemaChecks fails until it does. type Kind string const ( @@ -23,9 +22,8 @@ const ( KindScanFailed Kind = "scan_failed" KindTracksMissing Kind = "tracks_missing" KindDuplicatesFound Kind = "duplicates_found" - // KindDuplicatesResolved: the duplicate resolver (M498) merged copies on - // its own. The operator asked for that to - // happen automatically; this is how it stays visible. + // KindDuplicatesResolved: the duplicate resolver merged copies on its + // own. This is how an automatic merge stays visible. KindDuplicatesResolved Kind = "duplicates_resolved" KindPlaybackErrors Kind = "playback_errors" ) diff --git a/internal/notifications/notifier.go b/internal/notifications/notifier.go index faad7c3d..a8ea66bb 100644 --- a/internal/notifications/notifier.go +++ b/internal/notifications/notifier.go @@ -17,7 +17,7 @@ import ( // row lands for them. It is a nudge with no content: the client fetches the // inbox. A frame that carried the notification could not be told apart from // a repeat after a reconnect, and a frame nobody received is then one nobody -// needed, because the client pulls on reconnect anyway (Roundtable #2535). +// needed, because the client pulls on reconnect anyway. const EventCreated = "notification.created" // Recipients names who an event is for. diff --git a/internal/notifications/notifier_db_test.go b/internal/notifications/notifier_db_test.go index c8a782c6..e4c55081 100644 --- a/internal/notifications/notifier_db_test.go +++ b/internal/notifications/notifier_db_test.go @@ -146,9 +146,9 @@ func TestNotify_CoalescedCountAddsUpWhileUnread_AndStartsAfreshOnceRead(t *testi require.True(t, rows[1].ReadAt.Valid) } -// M542: an update says more, newest names first, without arriving again. A -// moved created_at was announced again on the phone, and a cleared emailed_at -// was emailed again. +// An update says more, newest names first, without arriving again: a moved +// created_at would announce it again on the phone, and a cleared emailed_at +// would email it again. func TestNotify_ACoalescedUpdateMergesNamesAndKeepsItsArrival(t *testing.T) { pool := testPool(t) q := dbq.New(pool) @@ -238,9 +238,9 @@ func TestMarkRead_ScopedToTheOwner_AndIdempotent(t *testing.T) { require.Zero(t, unread) } -// Every kind the code knows must pass both CHECKs in the real schema. This is -// rule 36's guard: a kind added in Go without the migration fails here, not in -// production at INSERT time. +// Every kind the code knows must pass both CHECKs in the real schema: a kind +// added in Go without its migration fails here, not in production at INSERT +// time. func TestEveryKindPassesTheSchemaChecks(t *testing.T) { pool := testPool(t) q := dbq.New(pool) diff --git a/internal/notifications/render.go b/internal/notifications/render.go index 7e618bf5..3f3eafb7 100644 --- a/internal/notifications/render.go +++ b/internal/notifications/render.go @@ -42,7 +42,7 @@ type Payload struct { // read. Count int64 `json:"count,omitempty"` // Items name what Count counts, one each, newest first: "Weezer – Buddy - // Holly". A coalesced update puts its own in front and keeps ten (M542), + // Holly". A coalesced update puts its own in front and keeps ten, // so the body can say which, not only how many. Items []string `json:"items,omitempty"` // Detail is free text: scan_failed's error message. Rows from before diff --git a/internal/notifications/render_test.go b/internal/notifications/render_test.go index 5ecd299b..c8376e85 100644 --- a/internal/notifications/render_test.go +++ b/internal/notifications/render_test.go @@ -45,7 +45,7 @@ func TestRender_CoalescedCountsReadNaturally(t *testing.T) { require.Equal(t, "Some tracks went missing", Render(KindTracksMissing, []byte(`{}`)).Title) } -// M542: a notice says which, not only how many. +// A notice says which, not only how many. func TestRender_NamesWhatChanged(t *testing.T) { require.Equal(t, "No longer on disk: Weezer – Buddy Holly.", render(KindTracksMissing, Payload{Count: 1, Items: []string{"Weezer – Buddy Holly"}}).Body) diff --git a/internal/playevents/writer.go b/internal/playevents/writer.go index f83fb099..5fbb9eb6 100644 --- a/internal/playevents/writer.go +++ b/internal/playevents/writer.go @@ -33,7 +33,7 @@ type Writer struct { } // NewWriter constructs a writer. timeout is the play-session window; the two -// remaining args are the skip rule thresholds from spec §6. +// remaining args are the skip rule's thresholds. func NewWriter( pool *pgxpool.Pool, logger *slog.Logger, @@ -78,23 +78,12 @@ func strPtrOrNil(s string) *string { } // systemPlaylistSources are the play_events.source values that count -// against a system playlist's rotation (Fable #415). Mirrors the -// `playlists_kind_variant_consistent` CHECK in migration -// 0028_discovery_mix_variants.up.sql — every variant in that CHECK -// list that ships as a refreshable system mix needs to be here so -// the rotation reporter sees Android + web plays from that surface. -// -// Drift #563: this map drifted behind the migrations. It had only -// for_you + discover but migrations 0021 + 0028 added 6 more -// variants. Plays from Rediscover / Deep Cuts / Songs Like X / -// New for You / On This Day / First Listens didn't advance the -// per-user rotation, so "unplayed first" ordering staled on those -// mixes — same tracks kept surfacing. -// -// Future variant additions should update this map AND the CHECK in -// the matching migration in the same change; the -// `db_check_constraint_for_new_variants` standing rule covers the -// CHECK half. +// against a system playlist's rotation. Every variant in the +// `playlists_kind_variant_consistent` CHECK that ships as a refreshable +// system mix must be here: a variant missing from this map never advances +// its per-user rotation, so "unplayed first" goes stale and the same tracks +// keep surfacing. A new variant updates this map and that CHECK (in its +// migration) in the same change. var systemPlaylistSources = map[string]bool{ "for_you": true, "discover": true, @@ -107,10 +96,10 @@ var systemPlaylistSources = map[string]bool{ } // lookupSystemPickKind resolves a system-playlist play's pick_kind -// (taste/fresh, Discover's bucket, or a rule-#131 tier) from the user's +// (taste/fresh, Discover's bucket, or an eligibility tier) from the user's // live snapshot of that variant at ingestion time. Attribution must be // frozen now — snapshots rebuild daily, so it can't be reconstructed at -// metrics-read time (#1249, generalized in #1270). Source strings +// metrics-read time. Source strings // double as system_variant values (see systemPlaylistSources). Returns // nil (unattributed) when the track isn't in the current snapshot — // plays predating the feature, or an offline replay arriving after the @@ -140,8 +129,8 @@ func lookupSystemPickKind( // tag identifying which surface the play came from. When source is a // known system-playlist kind the track is appended to that user's // rotation set so shuffle-on-play can prefer the unplayed tail on a -// re-play within the day. Empty source → behaves exactly like the -// pre-#415 path (NULL column, no rotation write). +// re-play within the day. An empty source behaves exactly like +// RecordPlayStarted (NULL column, no rotation write). func (w *Writer) RecordPlayStartedWithSource( ctx context.Context, userID, trackID pgtype.UUID, @@ -209,7 +198,7 @@ func (w *Writer) RecordPlayStartedWithSource( } // RecordPlayEnded closes a play_event with duration + computed completion -// ratio. Applies the spec §6 skip rule (AND of completion < threshold AND +// ratio. Applies the skip rule (AND of completion < threshold AND // duration_played < threshold) — both conditions must hold for the row to // be marked was_skipped=true. If skipped, also writes a skip_events row. func (w *Writer) RecordPlayEnded( @@ -232,7 +221,7 @@ func (w *Writer) RecordPlayEnded( // row is already closed, re-applying the close is harmless (the // UPDATE is last-write-wins) but a second skip_events insert must // NOT fire — a lost-response retry would otherwise double the - // skip signal. See the 2026-06-11 contract audit. + // skip signal. alreadyEnded := ev.EndedAt.Valid ratio := 0.0 if track.DurationMs > 0 { @@ -382,17 +371,15 @@ func (w *Writer) RecordSyntheticCompletedPlay( } // RecordOfflinePlay writes a complete start+end play in one call from -// a caller-supplied timestamp + duration. This is the replay path for -// the Flutter offline mutation queue (#426 part B): a play that -// happened with no/flaky connectivity is captured locally as a -// completed unit and replayed here once back online, preserving the -// original `at`. +// a caller-supplied timestamp + duration. This is the replay path for a +// client's offline queue: a play that happened with no or flaky +// connectivity is captured locally as a completed unit and replayed here +// once back online, preserving the original `at`. // // Generalizes RecordSyntheticCompletedPlay: instead of assuming full -// completion it applies the spec §6 skip rule from the real -// durationPlayed vs track length (same AND-of-thresholds as -// RecordPlayEnded), and threads `source` so #415 rotation advances -// for system-playlist plays just like the live path. +// completion it applies the skip rule from the real durationPlayed vs +// track length (same AND-of-thresholds as RecordPlayEnded), and threads +// `source` so system-playlist rotation advances just like the live path. // // durationPlayedMs is clamped to [0, track duration] to absorb // client clock / position drift. @@ -414,7 +401,7 @@ func (w *Writer) RecordOfflinePlay( // after commit makes the client re-fire the same play. The replayed // payload carries the original `at`, so an exact (user, track, // started_at) match means we already recorded it — no-op instead of a - // duplicate history row. See the 2026-06-11 contract audit. + // duplicate history row. var dupID pgtype.UUID err = tx.QueryRow(ctx, `SELECT id FROM play_events diff --git a/internal/playevents/writer_test.go b/internal/playevents/writer_test.go index 24893d50..49d7136f 100644 --- a/internal/playevents/writer_test.go +++ b/internal/playevents/writer_test.go @@ -220,8 +220,8 @@ func TestRecordPlayEnded_DurationOver50Percent_NotSkip(t *testing.T) { } // A lost-response retry of an offline play (same user/track/started_at) -// must not create a second history row. Guards the at-least-once replay -// path added in the 2026-06-11 audit. +// must not create a second history row: the offline replay is +// at-least-once. func TestRecordOfflinePlay_DedupsByUserTrackStartedAt(t *testing.T) { f := newFixture(t, 200_000) ctx := context.Background() @@ -491,7 +491,7 @@ func TestRecordPlayStarted_NoRotationWithoutSource(t *testing.T) { // seedSystemSnapshot creates a system playlist of the given variant // containing the fixture track with the given pick_kind, mimicking -// what the builder persists (#1249, generalized #1270). +// what the builder persists. func seedSystemSnapshot(t *testing.T, f fixture, variant, pickKind string) { t.Helper() pl, err := f.q.CreateSystemPlaylist(context.Background(), dbq.CreateSystemPlaylistParams{ @@ -567,7 +567,7 @@ func TestRecordPlayStartedWithSource_CrossVariant_PickKindNull(t *testing.T) { } func TestRecordPlayStartedWithSource_StampsDiscoverBucket(t *testing.T) { - // Discover stamps its candidate bucket (#1270); a discover play whose + // Discover stamps its candidate bucket; a discover play whose // track is in the live snapshot freezes that bucket onto the play. f := newFixture(t, 200_000) seedSystemSnapshot(t, f, "discover", "dormant") diff --git a/internal/playsessions/service.go b/internal/playsessions/service.go index edb14898..3b3d2d4b 100644 --- a/internal/playsessions/service.go +++ b/internal/playsessions/service.go @@ -1,5 +1,4 @@ -// Package playsessions implements the listening-session lifecycle from -// spec §6: a session is a contiguous sequence of plays by a user with no +// Package playsessions implements the listening-session lifecycle: a session is a contiguous sequence of plays by a user with no // gap longer than the configured timeout (default 30 minutes) between // events. package playsessions diff --git a/internal/prowlarr/prowlarr.go b/internal/prowlarr/prowlarr.go index 17b3e639..3ca4975b 100644 --- a/internal/prowlarr/prowlarr.go +++ b/internal/prowlarr/prowlarr.go @@ -1,9 +1,8 @@ -// Package prowlarr searches through Prowlarr's native API (M502 #5518). -// Ported from Fabled Librarian's internal/prowlarr. +// Package prowlarr searches through Prowlarr's native API. // // Native rather than Torznab because Prowlarr has no aggregate Torznab feed: // `/api/v1/indexer/all/newznab` 404s exactly like an indexer id that does not -// exist (measured by Fabled Librarian against a real Prowlarr, its #4304). +// exist. // `/api/v1/search` is the one place Prowlarr asks every indexer at once. package prowlarr diff --git a/internal/quarantine/service.go b/internal/quarantine/service.go index f7a4661a..1e676508 100644 --- a/internal/quarantine/service.go +++ b/internal/quarantine/service.go @@ -234,7 +234,7 @@ func (s *Service) snapshot(ctx context.Context, q *dbq.Queries, track dbq.Track) // // If the file cannot be removed, nothing is deleted and the per-user rows stay // so the admin can retry: the error wraps a *library.FileRemoveError, which the -// handler turns into an answer naming the cause (#3918). No partial state. +// handler turns into an answer naming the cause. No partial state. func (s *Service) DeleteFile(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.QuarantineAction, error) { q := dbq.New(s.pool) track, err := q.GetTrackByID(ctx, trackID) diff --git a/internal/reacquisition/settings.go b/internal/reacquisition/settings.go index 466ec07f..29bf05ba 100644 --- a/internal/reacquisition/settings.go +++ b/internal/reacquisition/settings.go @@ -1,12 +1,11 @@ // Package reacquisition wants an album back when its files go missing, -// without anyone pressing anything (milestone #290; a want since M502 -// #5520). +// without anyone pressing anything. // // The unit of work is the ALBUM, not the track. Music is acquired as // releases; there is no meaningful "fetch me one track", and a recording // MBID is something plenty of files lack. Grouping also does most of the -// safety work: the loss that produced #2523 — three reorganised albums, ~40 -// missing files — becomes three wants rather than forty. +// safety work: a reorganised album's forty missing files become one want, +// not forty. package reacquisition import ( @@ -21,7 +20,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// Settings is the operator-tunable policy, mirroring the columns and CHECK +// Settings is the admin-tunable policy, mirroring the columns and CHECK // ranges in migration 0056. type Settings struct { Enabled bool diff --git a/internal/reacquisition/sweeper.go b/internal/reacquisition/sweeper.go index 5ad73cc8..b5c4118a 100644 --- a/internal/reacquisition/sweeper.go +++ b/internal/reacquisition/sweeper.go @@ -15,8 +15,8 @@ import ( ) // Sweeper periodically wants back albums whose files have been missing past -// the grace window (milestone #290, M502 #5520). Searching for them is the -// search schedule's (#5521), which retries per want. +// the grace window. Searching for them is the search schedule's, which +// retries per want. // // Deliberately a periodic worker rather than a hook inside the scanner's // reconcile pass: reconcile runs inside a scan and re-runs often, and a @@ -28,7 +28,7 @@ type Sweeper struct { logger *slog.Logger tick time.Duration // releaseGroup names the MusicBrainz release group of a release id, for an - // album whose tags never carried one (#5241). Wants are kept by release + // album whose tags never carried one. Wants are kept by release // group. A field so tests can stand in for MusicBrainz. releaseGroup func(ctx context.Context, releaseMBID string) (string, error) } @@ -79,7 +79,7 @@ func (s *Sweeper) SweepOnce(ctx context.Context) error { q := dbq.New(s.pool) var res PassResult // Counted, not acted on: an album MusicBrainz cannot name is not a - // failure to retry, it is a permanent gap the operator should see. + // failure to retry, it is a permanent gap an admin should see. if n, cerr := q.CountAlbumsMissingWithoutMbid(ctx); cerr == nil { res.Unnameable = n } diff --git a/internal/reacquisition/sweeper_test.go b/internal/reacquisition/sweeper_test.go index fbd4b3ff..8d8a0899 100644 --- a/internal/reacquisition/sweeper_test.go +++ b/internal/reacquisition/sweeper_test.go @@ -83,9 +83,9 @@ func heldWant(t *testing.T, pool *pgxpool.Pool, group string, album pgtype.UUID, } } -// Only an album on the wanted list is fetched back (#5567): one taken off -// it, or dropped into the library by hand, is not. An album on the list is -// wanted by its release group, never its release id (#5241): a tag-supplied +// Only an album on the wanted list is fetched back: one taken off it, or +// dropped into the library by hand, is not. An album on the list is wanted +// by its release group, never its release id: a tag-supplied // group is used as is; otherwise MusicBrainz names it and the answer is // cached onto the album; an album MusicBrainz cannot name is skipped and // tried again next pass. @@ -103,8 +103,7 @@ func TestSweep_WantsBackOnlyAlbumsOnTheList_Integration(t *testing.T) { resolved := lostAlbum(t, pool, "Resolved", "release-resolved", "") unknown := lostAlbum(t, pool, "Unknown", "release-unknown", "") _ = lostAlbum(t, pool, "Off The List", "release-off", "group-off") - // Matched to a group with no release id: 238 such albums on the operator's - // library were never wanted back (#6167). + // Matched to a group with no release id: still wanted back. groupOnly := lostAlbum(t, pool, "Group Only", "", "group-only") heldWant(t, pool, "group-tagged", tagged, "3 days") heldWant(t, pool, "group-only", groupOnly, "3 days") @@ -165,7 +164,7 @@ func TestSweep_WantsBackOnlyAlbumsOnTheList_Integration(t *testing.T) { } // A want held after the files went missing means the library has a full copy -// despite the missing rows (#5245): the album is not wanted again. One held +// despite the missing rows: the album is not wanted again. One held // before they went missing is reopened. func TestSweep_HeldWantsAreReopenedOnlyForANewLoss_Integration(t *testing.T) { pool := newSweepPool(t) diff --git a/internal/recsettings/patch.go b/internal/recsettings/patch.go index f7139cdf..179c1e3b 100644 --- a/internal/recsettings/patch.go +++ b/internal/recsettings/patch.go @@ -171,7 +171,7 @@ const ( tagOverlapWeightMax = 10.0 // Snooze duration: at least a day (anything less isn't a snooze, it's a // flicker), at most a year — past that it's a permanent dismissal wearing a - // snooze's clothes, which is exactly the shape rule #101 rules out. + // snooze's clothes, and suggestions have no permanent "never show" control. snoozeDaysMin = 1.0 snoozeDaysMax = 365.0 ) diff --git a/internal/recsettings/service.go b/internal/recsettings/service.go index 7841ddb7..e333ff46 100644 --- a/internal/recsettings/service.go +++ b/internal/recsettings/service.go @@ -1,16 +1,15 @@ // Package recsettings is the DB-backed home of the recommendation -// tuning knobs (#1250): the two scoring-weight profiles (radio / -// daily_mix) and the taste-profile build settings (engagement -// half-life + completion curve). It follows the coverart +// tuning knobs: the scoring-weight profiles (radio, daily_mix, +// songs_like), Discover's settings, and the taste-profile build +// settings (engagement half-life + completion curve). It follows the coverart // SettingsService pattern — boot reconcile seeds shipped defaults for // missing rows, values are cached under a RWMutex, and every change // takes effect live (the daily_mix profile + taste config are pushed // into package playlists; radio reads the cache per request). // -// Framing (decision #1247): this is the defaults-discovery lab. The -// operator turns knobs to FIND good values; found-good values get -// baked back into the Shipped* functions below as new shipped -// defaults. End users and other operators should never need the card. +// This is a lab for finding good defaults: an admin turns knobs to find +// good values, and those get baked back into the Shipped* functions below +// as new shipped defaults. A typical install never needs to touch it. package recsettings import ( @@ -35,24 +34,23 @@ const ( ScopeRadio = "radio" ScopeDailyMix = "daily_mix" ScopeTaste = "taste" - // ScopeDiscover is the Discover request surface (#2377). Its own scope - // rather than columns on taste: SnoozeDays lives here, and a snooze must - // never be read as taste signal (#2374) — filing it under taste would put - // it one careless join from the leak that design forbids. + // ScopeDiscover is the Discover request surface. Its own scope rather + // than columns on taste: SnoozeDays lives here, and a snooze must never + // be read as taste signal — filing it under taste would put it one + // careless join from that leak. ScopeDiscover = "discover" - // ScopeSongsLike is the "Songs like {X}" surface (#3881). It shared - // daily_mix with For-You until 2026-09-10, and that sharing WAS the bug: - // the two surfaces want opposite things. For-You answers "what will they - // enjoy today" and is supposed to roam; Songs-like answers "what sounds - // like THIS" and is the tightest surface in the product. One set of - // weights cannot serve both, and the broad answer was winning. + // ScopeSongsLike is the "Songs like {X}" surface. It is separate from + // daily_mix because the two want opposite things: For-You answers "what + // will they enjoy today" and is supposed to roam; Songs-like answers + // "what sounds like THIS" and is the tightest surface in the product. One + // set of weights cannot serve both. ScopeSongsLike = "songs_like" ) // TasteTuning is the tunable subset of taste.Config: the engagement // half-life, the completion→engagement curve points, the enriched-tag -// weight (how much folksonomy tags count vs raw ID3 genre, #1520), and the -// era-facet weight (how strongly a decade-play imprints, #1530). +// weight (how much folksonomy tags count vs raw ID3 genre), and the +// era-facet weight (how strongly a decade-play imprints). type TasteTuning struct { HalfLifeDays float64 EngagementHardSkip float64 @@ -63,12 +61,10 @@ type TasteTuning struct { MoodScale float64 } -// ShippedRadioWeights are the shipped radio-profile defaults (moved -// here from config.RecommendationConfig — YAML is bootstrap-only, -// rule: config in UI). Radio is seed-directed (the user picked a -// direction), so taste is a lighter nudge than in the daily mixes. -// ContextTimeWeight starts uniform (1.0) across both profiles pending -// trend data (#1531); split them once the metrics view justifies it. +// ShippedRadioWeights are the shipped radio-profile defaults. Radio is +// seed-directed (the user picked a direction), so taste is a lighter nudge +// than in the daily mixes. ContextTimeWeight is 1.0 in both profiles; the +// metrics view is what would justify splitting them. func ShippedRadioWeights() recommendation.ScoringWeights { return recommendation.ScoringWeights{ BaseWeight: 1.0, @@ -99,12 +95,11 @@ func ShippedDailyMixWeights() recommendation.ScoringWeights { } } -// ShippedSongsLikeWeights are the shipped songs_like-profile defaults -// (#3881). The whole point is that SIMILARITY DOMINATES; every other -// profile balances it against taste and engagement, and this one must not. +// ShippedSongsLikeWeights are the shipped songs_like-profile defaults. The +// whole point is that SIMILARITY DOMINATES; every other profile balances it +// against taste and engagement, and this one must not. // -// The failure being corrected, arithmetic from the daily_mix profile that -// this surface used to share: +// Under the daily_mix weights, for example: // // unrelated track, liked, not played recently → 1.0 + 2.0 + 1.0 = 4.0 // PERFECT similarity match, not liked → 1.0 + 1.5 = 2.5 @@ -132,12 +127,10 @@ func ShippedDailyMixWeights() recommendation.ScoringWeights { // stays 2.0 because a track the user skips is still unwanted no matter how // similar it is. // -// These are DEFAULTS, not settings (rule 25) — the operator turns them in the -// admin tuning card and good values get baked back here. They are a -// defensible starting point rather than a measured optimum: the per-arm fill -// rates and the real sim_score distribution are still unknown (#3879), and -// `likes_overlap` contributes a flat 0.3 that a high SimilarityWeight -// amplifies. Expect to move these once that lands. +// These are DEFAULTS, not settings: an admin turns them in the tuning card +// and good values get baked back here. They are a defensible starting point +// rather than a measured optimum, and `likes_overlap` contributes a flat 0.3 +// that a high SimilarityWeight amplifies. func ShippedSongsLikeWeights() recommendation.ScoringWeights { return recommendation.ScoringWeights{ BaseWeight: 1.0, // same for all candidates; differentiates nothing @@ -169,12 +162,12 @@ func shippedWeightsFor(scope string) (recommendation.ScoringWeights, bool) { return recommendation.ScoringWeights{}, false } -// DiscoverTuning is the tunable set for the Discover request surface (#2377). +// DiscoverTuning is the tunable set for the Discover request surface. type DiscoverTuning struct { // TagOverlapWeight scales the taste-tag term: score × (1 + w × overlap). // 0 disables it and restores pure similarity ranking. TagOverlapWeight float64 - // SnoozeDays is the default "not right now" duration (#2374). + // SnoozeDays is the default "not right now" duration. SnoozeDays float64 } @@ -183,12 +176,11 @@ type DiscoverTuning struct { // TagOverlapWeight 1.0 lets a perfect tag match at most double a candidate's // similarity score — enough to reorder the deck meaningfully, not enough for a // popular-tag coincidence to beat a genuinely strong similarity match. It is a -// starting point for the tuning lab, not a tuned value: the honest way to pick -// it is the metrics trend view after some real use. +// starting point, not a tuned value: the way to pick it is the metrics trend +// view after some real use. // -// SnoozeDays 90 matches the operator's approved shape: long enough that a -// parked suggestion stops nagging, short enough that a taste shift brings it -// back on its own. +// SnoozeDays 90: long enough that a parked suggestion stops nagging, short +// enough that a taste shift brings it back on its own. func ShippedDiscoverTuning() DiscoverTuning { return DiscoverTuning{ TagOverlapWeight: 1.0, @@ -337,7 +329,7 @@ func (s *Service) Taste() TasteTuning { // Discover returns the cached Discover-tuning values. Read per request by the // suggestions handler, so an admin change takes effect on the next refresh -// with no restart (rule #25). +// with no restart. func (s *Service) Discover() DiscoverTuning { s.mu.RLock() defer s.mu.RUnlock() diff --git a/internal/recsettings/service_test.go b/internal/recsettings/service_test.go index 083a38bd..37bc3958 100644 --- a/internal/recsettings/service_test.go +++ b/internal/recsettings/service_test.go @@ -282,7 +282,7 @@ func TestUpdate_NoOpWritesNoAudit(t *testing.T) { } } -// --- Discover scope (#2377, milestone #268 slice 6) --- +// --- Discover scope --- func TestNew_SeedsDiscoverDefaults(t *testing.T) { pool := newPool(t) @@ -310,9 +310,8 @@ func TestUpdateDiscover_PersistsAndAudits(t *testing.T) { t.Errorf("SnoozeDays = %v, want 30", got) } - // The audit row must land under the new scope. This is the assertion that - // would have caught a missing rule-#36 CHECK migration: without expanding - // recommendation_tuning_audit's whitelist, this INSERT fails at runtime. + // The audit row must land under the new scope: a scope missing from + // recommendation_tuning_audit's CHECK fails this INSERT at runtime. rows := auditRows(t, pool) if len(rows) != 1 { t.Fatalf("audit rows = %d, want 1", len(rows)) @@ -343,8 +342,7 @@ func TestUpdateDiscover_Validation(t *testing.T) { {"weight past the typo bound", map[string]float64{"tag_overlap_weight": 100}}, // A sub-day snooze isn't a snooze, it's a flicker. {"snooze under a day", map[string]float64{"snooze_days": 0.5}}, - // Past a year it's a permanent dismissal wearing a snooze's clothes — - // the shape rule #101 rules out. + // Past a year it's a permanent dismissal wearing a snooze's clothes. {"snooze past a year", map[string]float64{"snooze_days": 400}}, } for _, c := range cases { @@ -360,9 +358,8 @@ func TestUpdateDiscover_Validation(t *testing.T) { } } -// Weight 0 must be accepted — it's the operator's off switch for the whole -// tag term, so a "must be positive" bound would remove their ability to -// disable the feature. +// Weight 0 must be accepted: it's the off switch for the whole tag term, so +// a "must be positive" bound would leave no way to disable it. func TestUpdateDiscover_ZeroWeightIsAllowed(t *testing.T) { pool := newPool(t) s := newService(t, pool) diff --git a/internal/recsettings/songs_like_weights_test.go b/internal/recsettings/songs_like_weights_test.go index 805b940b..c4830d49 100644 --- a/internal/recsettings/songs_like_weights_test.go +++ b/internal/recsettings/songs_like_weights_test.go @@ -7,10 +7,9 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/recommendation" ) -// The bug, reproduced as a ranking: a track that sounds nothing like the seed -// but which the user liked and has not played lately used to OUTRANK a perfect -// similarity match. Operator, 2026-09-10: "I was getting a seeming wide variety -// of music from each one when I was hoping to stay in a certain neighborhood." +// A track that sounds nothing like the seed, but which the user liked and has +// not played lately, must not outrank a perfect similarity match: Songs-like +// should stay in the seed's neighbourhood. // // This is the whole point of the songs_like profile, so it is asserted as // BEHAVIOUR — two candidates, which one wins — rather than by checking the @@ -85,7 +84,7 @@ func TestSongsLikeWeights_SimilarityOutrangesEverySeedIndependentTerm(t *testing } // A scope that is seedable but not resettable is the half-wired shape this -// guards: the profile appears in the admin card, the operator turns a knob, +// guards: the profile appears in the admin card, an admin turns a knob, // and Reset then 404s on a scope the rest of the service knows about. func TestShippedWeightsFor_CoversEveryWeightProfile(t *testing.T) { for _, scope := range []string{ScopeRadio, ScopeDailyMix, ScopeSongsLike} { diff --git a/internal/release/agree.go b/internal/release/agree.go index cd2b9822..d220408e 100644 --- a/internal/release/agree.go +++ b/internal/release/agree.go @@ -3,7 +3,7 @@ package release import "slices" // Comparing a file against a MusicBrainz track: shared by the importer's -// check of a download and the library listing's match of an album (#5569). +// check of a download and the library listing's match of an album. // LengthFits allows five seconds or 3%, whichever is more: remasters and // rips differ by a second or two, a different recording by far more. A diff --git a/internal/release/parse.go b/internal/release/parse.go index 956d548d..9acce161 100644 --- a/internal/release/parse.go +++ b/internal/release/parse.go @@ -1,5 +1,5 @@ // Package release reads music release names and judges them against a -// wanted album (M502 #5521): what a name says it is (format, bitrate, bit +// wanted album: what a name says it is (format, bitrate, bit // depth, source, year, edition), whether it is the album wanted, and which // quality tier it sits in. Nothing here touches the network or the database, // so every rule is testable on a string. diff --git a/internal/release/quality.go b/internal/release/quality.go index 055cc773..456a3950 100644 --- a/internal/release/quality.go +++ b/internal/release/quality.go @@ -44,7 +44,7 @@ var Tiers = []TierInfo{ } // DefaultTiers is what a new install allows: every format from trash lossy -// to lossless (note #5527). Unknown is left out; a +// to lossless. Unknown is left out; a // release that names no format has its .torrent read to find out. var DefaultTiers = []Tier{ TierLosslessHiRes, TierLossless, TierLossyHigh, TierLossyStandard, @@ -151,7 +151,7 @@ func khz(f float64) string { return strings.TrimSuffix(strconv.FormatFloat(f, 'f', 1, 64), ".0") } -// Profile is which tiers the operator takes. +// Profile is which tiers an install takes. type Profile struct { Allowed map[Tier]bool } @@ -216,7 +216,7 @@ func FileKbps(bytes, ms int64) int { // mp3High is the average bitrate from which an MP3 counts as high quality // when only its size is known. A V0 encode averages 220 to 260 kbps, so the // line sits below it: an estimate errs toward the copy held, and an album is -// never fetched again on a guess (#5524). +// never fetched again on a guess. const mp3High = 220 // TierOfFile places files of one format on the ladder from their average diff --git a/internal/release/release_test.go b/internal/release/release_test.go index 5f7ae12c..46ab4456 100644 --- a/internal/release/release_test.go +++ b/internal/release/release_test.go @@ -16,7 +16,7 @@ func TestParse_ReadsWhatTheNameSays(t *testing.T) { rate float64 tier Tier }{ - // The wrong edition grabbed on 2026-10-09 (note #5527). + // A bracketed edition with its own year: not the original album. {"Daft Punk Random Access Memories (Drumless Edition 2023) [Flac 24-88]", FormatFLAC, "", "", 0, 24, 2023, 88.2, TierLosslessHiRes}, {"Gorillaz - Humanz - (Japanese Edition) - [CD-FLAC] 2017", FormatFLAC, "", SourceCD, 0, 0, 2017, 0, TierLossless}, {"Radiohead - OK Computer (1997) [MP3 320]", FormatMP3, "", "", 320, 0, 1997, 0, TierLossyHigh}, diff --git a/internal/release/torrent.go b/internal/release/torrent.go index 578528de..15c844d4 100644 --- a/internal/release/torrent.go +++ b/internal/release/torrent.go @@ -10,7 +10,7 @@ import ( "strings" ) -// Reading a .torrent file, ported from Fabled Librarian (#4463). A .torrent +// Reading a .torrent file. A .torrent // is the metadata: its info dictionary names every file, and its SHA-1 is the // infohash. So a release offered as a .torrent link can be judged on what it // holds before any of it is downloaded. Bencode is four shapes; walking them diff --git a/internal/release/version.go b/internal/release/version.go index c90dbf50..6a849fe2 100644 --- a/internal/release/version.go +++ b/internal/release/version.go @@ -5,7 +5,7 @@ import "strings" // TitleVersion is the version of a recording a release group's title names // in its qualifier: the words in brackets, or after a dash. MusicBrainz // does not always type such a group: My Chemical Romance's "Common People -// (Recorded For BBC Radio 1)" is a plain Single (#5573). Typ is the +// (Recorded For BBC Radio 1)" is a plain Single. Typ is the // MusicBrainz secondary type the version is, or "" for one no type names // (an acoustic or instrumental version); Word is the word that said so. type TitleVersion struct { diff --git a/internal/release/version_test.go b/internal/release/version_test.go index c6a877c9..7f9b4fc3 100644 --- a/internal/release/version_test.go +++ b/internal/release/version_test.go @@ -4,7 +4,7 @@ import "testing" func TestVersionInTitle(t *testing.T) { for title, want := range map[string]string{ - // My Chemical Romance's groups MusicBrainz types as plain Singles (#5573). + // Groups MusicBrainz types as plain Singles, named by their title. "Common People (Recorded For BBC Radio 1)": "Live", "Welcome to the Black Parade (Steve Aoki remix)": "Remix", "The Five of Us Are Dying (Rough mix)": "Demo", diff --git a/internal/requests/reconciler.go b/internal/requests/reconciler.go index 72fed3b8..06593805 100644 --- a/internal/requests/reconciler.go +++ b/internal/requests/reconciler.go @@ -36,8 +36,8 @@ func NewReconciler(pool *pgxpool.Pool, logger *slog.Logger, bus *eventbus.Bus) * return &Reconciler{pool: pool, logger: logger, bus: bus, tick: 5 * time.Minute, batch: 50} } -// SetNotifier makes completions land in the requester's notifications inbox -// (M489). Without one, completions are only broadcast on the bus. +// SetNotifier makes completions land in the requester's notifications inbox. +// Without one, completions are only broadcast on the bus. func (r *Reconciler) SetNotifier(n *notifications.Notifier) { r.notifier = n } // publishCompleted tells the requester their request arrived: a diff --git a/internal/requests/service.go b/internal/requests/service.go index 747b922b..203a7f53 100644 --- a/internal/requests/service.go +++ b/internal/requests/service.go @@ -1,6 +1,6 @@ // Package requests owns the lifecycle of what users ask to have added. The // synchronous Service handles Create/List/Approve/Reject/Cancel; approving -// records the request on the wanted list (M502 #5520): an album or a track +// records the request on the wanted list: an album or a track // wants its album's release group, an artist is followed. The async // Reconciler (separate file) completes approved requests once the library // holds what they asked for. @@ -60,7 +60,7 @@ func (s *Service) Create(ctx context.Context, userID pgtype.UUID, p CreateParams // CreateTracked is Create, also reporting whether a row was inserted. A // request that dedupes into one already in flight is not new, so callers -// that announce new requests (M489) say nothing for it. +// that announce new requests say nothing for it. func (s *Service) CreateTracked(ctx context.Context, userID pgtype.UUID, p CreateParams) (dbq.Request, bool, error) { if err := validateKindFields(p); err != nil { return dbq.Request{}, false, err diff --git a/internal/scrobble/threshold.go b/internal/scrobble/threshold.go index a33ecd7e..1b564057 100644 --- a/internal/scrobble/threshold.go +++ b/internal/scrobble/threshold.go @@ -7,9 +7,8 @@ package scrobble // recommended scrobble threshold: ≥240 seconds played OR ≥50% of the // track length played. // -// The two thresholds are deliberately separate from M2's skip-detection -// thresholds (which serve a different purpose — internal engine signal -// for the scoring formula's skip penalty). +// The two thresholds are deliberately separate from the skip-detection +// thresholds, which feed the scoring formula's skip penalty instead. func Qualifies(durationPlayedMs int, completionRatio float64) bool { return durationPlayedMs >= 240_000 || completionRatio >= 0.5 } diff --git a/internal/scrobble/worker.go b/internal/scrobble/worker.go index 3a436158..05c2ad27 100644 --- a/internal/scrobble/worker.go +++ b/internal/scrobble/worker.go @@ -14,7 +14,7 @@ import ( ) // backoffSchedule maps the failure count (1-indexed) to the delay before -// the next attempt. Per spec §M4a: 1m → 5m → 30m → 2h → 6h, then give up. +// the next attempt: 1m → 5m → 30m → 2h → 6h, then give up. var backoffSchedule = []time.Duration{ 1 * time.Minute, 5 * time.Minute, diff --git a/internal/server/hygiene.go b/internal/server/hygiene.go index 649f392f..854e85d3 100644 --- a/internal/server/hygiene.go +++ b/internal/server/hygiene.go @@ -120,8 +120,8 @@ func requireJSONForCookieWrites(next http.Handler) http.Handler { // by the SPA handler, which knows the inline-script hashes. // // HSTS goes out only when the request reached us over HTTPS as the trusted -// proxy reports it (rule 94): sending it over plain HTTP is ignored by -// browsers at best, and the app never forces HTTPS. +// proxy reports it: sent over plain HTTP it is ignored by browsers at best, +// and the app never forces HTTPS. func securityHeaders(hops func() int) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/server/hygiene_test.go b/internal/server/hygiene_test.go index ae9a1c4f..2c31dbf6 100644 --- a/internal/server/hygiene_test.go +++ b/internal/server/hygiene_test.go @@ -115,7 +115,7 @@ func TestSecurityHeaders(t *testing.T) { t.Errorf("Permissions-Policy = %q", base.Get("Permissions-Policy")) } - // Rule 94's three cases for HSTS. + // HSTS with no trusted proxy, and through one saying https or http. if got := serve(0, "https").Get("Strict-Transport-Security"); got != "" { t.Errorf("hops 0 + forwarded https: HSTS = %q, want none", got) } diff --git a/internal/server/release_gate_test.go b/internal/server/release_gate_test.go index 345d84de..c62b1a17 100644 --- a/internal/server/release_gate_test.go +++ b/internal/server/release_gate_test.go @@ -10,7 +10,7 @@ import ( "gopkg.in/yaml.v3" ) -// The publish gate (rule 177, M462 #4984), pinned. release.yml holds every +// The publish gate, pinned. release.yml holds every // verifying lane and every publishing job in one graph so that a publish can // depend on the lanes; these tests keep that dependency from eroding. // diff --git a/internal/server/release_version_test.go b/internal/server/release_version_test.go index fa508d71..d4be48b6 100644 --- a/internal/server/release_version_test.go +++ b/internal/server/release_version_test.go @@ -10,17 +10,10 @@ import ( "testing" ) -// Guards the version derivation that stamps every build. -// -// These assertions used to be impossible to run. The derivation lived inline -// in release.yml, which triggers only on `main` and on tags — so a mistake in -// it could not surface until a release was already under way, and its failure -// mode is silence: a version nobody can compare looks exactly like being up to -// date, and nobody reports an update they were never offered. -// -// Moving it to ci/version.sh made it executable, so this runs on every push -// that touches the release machinery. That is the whole point of the file; the -// specific assertions below matter less than the fact that they run at all. +// Guards the version derivation that stamps every build, in ci/version.sh. +// Its failure mode is silence: a version nobody can compare looks exactly +// like being up to date, and nobody reports an update they were never +// offered. Keeping it in a script lets these tests run it on every push. // // The tests EXECUTE the script rather than asserting on its text, so they // break when the behaviour changes rather than when the wording does. @@ -188,8 +181,7 @@ var jobBoundary = regexp.MustCompile(`\n\n [^ \n]`) // // The naive cut — "up to the next `- name:`" — silently returns an EMPTY body // for the last step in a job, and every assertion over it then passes -// vacuously. That is the failure rule 167 names: a check that reads as -// coverage while asserting on nothing. Cutting at a blank line followed by +// vacuously: a check that reads as coverage while asserting on nothing. Cutting at a blank line followed by // job-level indentation handles the last-step case, which is exactly where // `Build and push` sits. func stepBody(t *testing.T, yaml, name string) string { @@ -274,7 +266,7 @@ func TestDevChannel_PublishesDevAloneAndNeverLatest(t *testing.T) { if strings.Contains(arm, ":latest") { t.Errorf("dev arm moves :latest — that ships dev code to every stable operator\n%s", arm) } - // Rule 145: a rolling channel gets no commit-addressable tag. + // A rolling channel gets no commit-addressable tag. if strings.Contains(arm, "GITHUB_SHA") { t.Errorf("dev arm publishes a per-commit tag; a rolling channel should not\n%s", arm) } @@ -367,21 +359,19 @@ func TestMainChannel_PublishesLatestAndTheRollbackUnit(t *testing.T) { if !strings.Contains(arm, "${IMAGE}:${GITHUB_SHA}") { t.Errorf("main arm publishes no commit-addressable image; there is no rollback target for production commits\n%s", arm) } - // Rule 147: :latest tracks main's tip, and a second name for the same + // :latest tracks main's tip, and a second name for the same // image sends readers looking for a distinction that does not exist. if strings.Contains(arm, "${IMAGE}:main") { t.Errorf("main arm publishes :main — rule 147 says that tag should not exist\n%s", arm) } } -// A release refreshes the CHANNEL and mints nothing else (rules 145 + 146). +// A release refreshes the CHANNEL and mints nothing else. // -// The specific regression: re-adding : here. The tag build rebuilds the -// SAME SOURCE as main's build minutes earlier, differing only in which APK is -// baked in — so a : minted here would overwrite main's immutable rollback -// target with different contents, under the same name. That is the exact thing -// rule 145's immutability clause exists to prevent, and it is the half with -// the incidents behind it. +// The regression to guard: a : tag here. The tag build rebuilds the SAME +// SOURCE as main's build minutes earlier, differing only in which APK is baked +// in — so a : minted here would overwrite main's immutable rollback +// target with different contents, under the same name. func TestReleaseBuild_RefreshesTheChannelAndMintsNothingElse(t *testing.T) { arm, _, _ := imageTagArms(t, releaseYAML(t)) @@ -393,8 +383,8 @@ func TestReleaseBuild_RefreshesTheChannelAndMintsNothingElse(t *testing.T) { } } -// No version-numbered image tags anywhere, on any arm (rule 145, and the -// operator's 2026-09-10 decision to drop them across every project). +// No version-numbered image tags anywhere, on any arm: the build's version is +// self-reported, and the tag map is :latest, : and :dev. // // Asserted across the whole step rather than per-arm because the mistake this // catches is re-adding one ANYWHERE, and the tag arm is only the likeliest @@ -441,9 +431,9 @@ func TestVerifyJob_ChecksTheRollbackImageNotAVersionTag(t *testing.T) { } } -// The server's self-reported version is now the ONLY thing that identifies a +// The server's self-reported version is the ONLY thing that identifies a // build, so a lane that stamps a channel word instead of a version silently -// removes that ability. It used to stamp the literal "main"/"dev". +// removes that ability. func TestImageBuild_StampsADerivedVersionAndAChannel(t *testing.T) { yaml := releaseYAML(t) diff --git a/internal/server/requestlog.go b/internal/server/requestlog.go index fe597149..b4587626 100644 --- a/internal/server/requestlog.go +++ b/internal/server/requestlog.go @@ -19,18 +19,16 @@ import ( // hits it every 5s and would otherwise drown real signals (~17k lines/day). // // Severity is keyed off the response status so 4xx/5xx surface even when -// the operator's logger level is set above Info. +// the logger level is set above Info. // -// The `remote` attribute holds the address resolved through the operator's -// configured reverse-proxy depth, NOT the raw socket peer (#2453). Behind a -// proxy — the normal deployment for anything public — the socket peer is the -// proxy, so every line would have carried the same useless address, and the -// access log would have disagreed with the Active-sessions surface about who -// connected. The attribute key is unchanged so existing log greps keep -// working; only its accuracy improved. +// The `remote` attribute holds the address resolved through the configured +// reverse-proxy depth, NOT the raw socket peer. Behind a proxy — the normal +// deployment for anything public — the socket peer is the proxy, so every +// line would carry the same useless address, and the access log would +// disagree with the Active-sessions list about who connected. // // trustedHops is a func because this middleware is constructed at boot while -// the value is operator-editable at runtime, and — since Router() registers +// the value is editable at runtime, and — since Router() registers // this before it builds the settings service — because it lets the accessor // be wired before the thing it reads exists. auth.ClientIP tolerates a depth // of 0, which is what a nil service reports. diff --git a/internal/server/requestlog_test.go b/internal/server/requestlog_test.go index b5b552c5..24ecf3b4 100644 --- a/internal/server/requestlog_test.go +++ b/internal/server/requestlog_test.go @@ -109,8 +109,7 @@ func TestRequestLog_AttributesPresent(t *testing.T) { } } -// The point of routing #2453 through the access log: behind a proxy, `remote` -// must be the client rather than the proxy, and must agree with what the +// Behind a proxy, the access log's `remote` must be the client rather than the proxy, and must agree with what the // Active-sessions surface records for the same request. Logs and UI // disagreeing about who connected is worse than either being wrong alone. func TestRequestLog_RemoteHonoursTrustedProxyDepth(t *testing.T) { @@ -136,8 +135,8 @@ func TestRequestLog_RemoteHonoursTrustedProxyDepth(t *testing.T) { want: "203.0.113.200", }, { - // The case that motivated the change: proxy on a PUBLIC address, - // which the pre-#2453 heuristic logged as the proxy forever. + // A proxy on a PUBLIC address: a "trust XFF only from private + // peers" heuristic would log the proxy forever. name: "depth 1 through a public-addressed proxy logs the client", hops: func() int { return 1 }, remoteAddr: "203.0.113.200:40000", diff --git a/internal/server/server.go b/internal/server/server.go index 17d51891..47b045ce 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -65,34 +65,34 @@ type Server struct { // When nil, Router() constructs a local fallback (test contexts). Bus *eventbus.Bus // PlaylistScheduler fires per-user daily system-playlist builds at - // 03:00 in each user's stored timezone (#392 Half B). Constructed + // 03:00 in each user's stored timezone. Constructed // in cmd/minstrel/main.go and threaded into the API handlers so // PUT /api/me/timezone and POST /api/auth/register can call // Refresh synchronously. PlaylistScheduler *playlists.Scheduler - // RecSettings is the DB-backed recommendation tuning lab (#1250): + // RecSettings is the DB-backed recommendation tuning: // scoring-weight profiles + taste-build knobs. Constructed in // cmd/minstrel/main.go (it pushes daily-mix weights into package // playlists at boot); the API layer reads radio weights per request // and serves the admin tuning endpoints from it. Router() constructs // a fallback when nil (tests). RecSettings *recsettings.Service - // FingerprintSettings is the DB-backed fingerprinting policy (M400 #3913). + // FingerprintSettings is the DB-backed fingerprinting policy. // Constructed in cmd/minstrel/main.go and shared with the scanner and the // fingerprint workers, so a save from the admin card reaches them without a // restart. Router() constructs a fallback when nil (tests). FingerprintSettings *library.FingerprintSettingsService - // LoudnessSettings is the DB-backed loudness analysis policy (M464 #4995), + // LoudnessSettings is the DB-backed loudness analysis policy, // shared with the loudness backfill. Nil makes the router load its own. LoudnessSettings *library.LoudnessSettingsService - // AcoustIDLookup is the AcoustID lookup worker (M401), whose settings, + // AcoustIDLookup is the AcoustID lookup worker, whose settings, // status and "look up now" the admin card drives. Nil in test contexts; // one is built (and never run) so the admin routes still answer. AcoustIDLookup *library.AcoustIDLookupWorker - // ReacqSettings is the DB-backed missing-file re-acquisition policy - // (milestone #290) — the same instance the sweeper in cmd/minstrel/main.go - // reads, so a save from the admin card reaches it without a restart - // (#3936). Router() constructs a fallback when nil (tests). + // ReacqSettings is the DB-backed missing-file re-acquisition policy — the + // same instance the sweeper in cmd/minstrel/main.go reads, so a save from + // the admin card reaches it without a restart. Router() constructs a + // fallback when nil (tests). ReacqSettings *reacquisition.SettingsService // StreamSecret is the HMAC key used by /api/cast/stream-token to // mint signed UPnP / Sonos stream URLs and by /api/tracks/{id}/stream @@ -154,13 +154,11 @@ func (s *Server) Router() http.Handler { tracksSvc := tracks.NewService(s.Pool, s.Logger, s.DataDir) playlistsSvc := playlists.NewService(s.Pool, s.Logger, s.DataDir) smtpSender := mailer.NewSMTPSender(s.Pool, s.Logger.With("component", "mailer")) - // Live-event bus for SSE subscribers (#392). Constructed per-process; - // producers in playevents / requests / scanner publish into - // the same instance. Background workers (request reconciler, scan - // scheduler) live in cmd/minstrel/main.go where they're constructed - // before Router() runs; they read s.Bus directly so they share this - // process's bus. When Router() runs before main set the bus (tests, - // or future contexts) we fall back to a fresh local instance. + // Live-event bus for SSE subscribers, one per process: producers in + // playevents / requests / scanner publish into the same instance. + // Background workers are built in cmd/minstrel/main.go before Router() + // runs and share s.Bus. With no bus set (tests), a fresh local one is + // used. bus := s.Bus if bus == nil { bus = eventbus.New() @@ -240,8 +238,8 @@ func (s *Server) handleHealthz(w http.ResponseWriter, _ *http.Request) { } // handleAdminScan runs a scan synchronously and returns the resulting stats. -// Kept synchronous for v1: libraries are small and the operator can tell when -// it's done. Async jobs with status polling are a later-milestone concern. +// Synchronous, so the caller can tell when it's done; the scan's progress is +// also published as scan-run events. func (s *Server) handleAdminScan(w http.ResponseWriter, r *http.Request) { stats, err := s.Scanner.Scan(r.Context(), nil) w.Header().Set("Content-Type", "application/json") diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 2e689688..7a82d9dc 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -244,13 +244,11 @@ func TestRouter_AdminSubtreeNotShadowed(t *testing.T) { } } -// TestRouter_ReacquisitionSettingsSavedThroughTheAPIReachTheSweeper is a -// regression test for #3936. Router() used to construct a -// reacquisition.SettingsService of its own, so a save from the admin card -// refreshed THAT instance's cache while the sweeper in cmd/minstrel/main.go -// kept serving what it had loaded at boot. The card showed the new policy, the -// feature kept running the old one, and only a restart reconciled them — the -// exact thing rule 25 says a setting must not need. +// TestRouter_ReacquisitionSettingsSavedThroughTheAPIReachTheSweeper: a +// Router() that built a reacquisition.SettingsService of its own would refresh +// THAT instance's cache on a save, while the sweeper in cmd/minstrel/main.go +// kept serving what it loaded at boot. The card would show the new policy and +// the feature run the old one until a restart. // // The assertion is made against the instance main.go hands the sweeper: save // through the router, then read that instance. A second service leaves it stale. diff --git a/internal/server/version.go b/internal/server/version.go index 5c120a0c..2a65d8b6 100644 --- a/internal/server/version.go +++ b/internal/server/version.go @@ -6,4 +6,4 @@ package server const MinClientVersion = "0.1.0" // The build's version and channel live in internal/buildinfo, shared with -// the Subsonic envelope and the admin Overview (M504). +// the Subsonic envelope and the admin Overview. diff --git a/internal/similarity/worker.go b/internal/similarity/worker.go index dab420f1..461a4c79 100644 --- a/internal/similarity/worker.go +++ b/internal/similarity/worker.go @@ -5,14 +5,14 @@ // // For recordings it keeps LB's whole answer in listenbrainz_similar_recordings // and derives the track_similarity edges from it in SQL, against whatever is -// in the library at the time (#5296). Matching once at fetch time missed every -// recording that reached the library afterwards, through an import or an MBID -// the AcoustID lookup filled in. For artists it stores the in-library matches in -// artist_similarity and the rest in artist_similarity_unmatched, as before. +// in the library at the time, so a recording that reaches the library later +// (an import, or an MBID the AcoustID lookup fills in) gains its edges without +// asking LB again. For artists it stores the in-library matches in +// artist_similarity and the rest in artist_similarity_unmatched. // // Every answer is recorded as a fetch, an empty one included, and the queue -// reads those records: freshness used to be read from the edges, so a seed -// that matched nothing was never fresh and held the head of the queue. +// reads those records rather than the edges: a seed that matched nothing is +// still fresh and does not hold the head of the queue. package similarity import ( @@ -200,8 +200,8 @@ func (w *Worker) upsertArtistSimilar(ctx context.Context, q *dbq.Queries, artist } sort.Slice(results, func(i, j int) bool { return results[i].Score > results[j].Score }) // Scores are stored relative to the seed's closest artist, as track edges - // are (#6177): LB's raw scores are counts (16 to 10,875 on the operator's - // library), and both the candidate pool and the suggestions read 0 to 1. + // are: LB's raw scores are unbounded counts, and both the candidate pool + // and the suggestions read 0 to 1. best := results[0].Score mbids := make([]string, 0, len(results)) diff --git a/internal/similarity/worker_integration_test.go b/internal/similarity/worker_integration_test.go index 229e8131..dea811b2 100644 --- a/internal/similarity/worker_integration_test.go +++ b/internal/similarity/worker_integration_test.go @@ -209,9 +209,9 @@ func TestTickOnce_MapsLBResponseToLocalLibrary(t *testing.T) { } } -// LB's scores are counts, not 0 to 1 (16 to 1445 on the operator's library). -// An edge is stored over the seed's best score in the whole answer, in the -// library or not, so the scorer reads it as 0 to 1 (#6177). +// LB's scores are counts, not 0 to 1. An edge is stored over the seed's best +// score in the whole answer, in the library or not, so the scorer reads it as +// 0 to 1. func TestTickOnce_EdgeIsRelativeToTheSeedsBest(t *testing.T) { f := newFixture(t) mbidA := "11111111-1111-1111-1111-111111111111" @@ -241,8 +241,8 @@ func TestTickOnce_EdgeIsRelativeToTheSeedsBest(t *testing.T) { } } -// The 20-per-seed cap is gone with the cache (#5296): LB's answer is at most -// 50, and every recording of it that is in the library becomes an edge. +// LB's answer is at most 50, and every recording of it that is in the library +// becomes an edge: no per-seed cap. func TestTickOnce_KeepsEveryInLibraryMatch(t *testing.T) { f := newFixture(t) mbidSeed := "11111111-1111-1111-1111-111111111111" @@ -311,7 +311,7 @@ func TestTickOnce_RefreshesStaleRow(t *testing.T) { _ = f.pool.QueryRow(context.Background(), `SELECT score, fetched_at FROM track_similarity WHERE track_a_id = $1 AND track_b_id = $2 AND source = 'listenbrainz'`, trackA.ID, other.ID).Scan(&score, &fetchedAt) - // The seed's only answer is its best, so the refreshed edge is 1.0 (#6177). + // The seed's only answer is its best, so the refreshed edge is 1.0. if score != 1.0 { t.Errorf("score = %v, want 1.0 (refreshed, relative to the seed's best)", score) } @@ -452,7 +452,7 @@ func TestUpsertArtistSimilar_PersistsUnmatchedToTable(t *testing.T) { w := newTestWorker(f, "") similars := []listenbrainz.SimilarArtist{ - // LB's scores are counts; rows are stored over the seed's best (#6177). + // LB's scores are counts; rows are stored over the seed's best. {MBID: inLibMBID, Name: "InLib Artist", Score: 1000}, {MBID: "out-mbid-1", Name: "Outsider One", Score: 850}, {MBID: "out-mbid-2", Name: "Outsider Two", Score: 800}, @@ -543,9 +543,9 @@ func recordingLB(answer func(mbid string) string) (*httptest.Server, func() []st } } -// The starvation #3879 measured: a seed whose answer matched nothing in the -// library wrote nothing, stayed due, and was asked again every tick ahead of -// everything behind it. Recording the fetch moves the queue on. +// A seed whose answer matches nothing in the library writes no edges, and +// must still leave the head of the queue: recording the fetch moves it on, +// so it is not asked again every tick ahead of everything behind it. func TestTickOnce_UnmatchedAnswerDoesNotHoldTheQueue(t *testing.T) { f := newFixture(t) mbids := []string{ @@ -583,7 +583,7 @@ func TestTickOnce_UnmatchedAnswerDoesNotHoldTheQueue(t *testing.T) { } } -// The point of the cache (#5296): a recording LB named while it was not in the +// The point of the cache: a recording LB named while it was not in the // library becomes an edge once it arrives, on the next tick's resolve, without // asking LB again. One edge per recording when the library holds two copies, // and none to a copy that is missing. diff --git a/internal/subsonic/browse.go b/internal/subsonic/browse.go index 2afb0ed8..de51ae01 100644 --- a/internal/subsonic/browse.go +++ b/internal/subsonic/browse.go @@ -26,8 +26,8 @@ type browseHandlers struct { // for backwards compatibility with clients that regroup client-side. const ignoredArticles = "The El La Los Las Le Les" -// getMusicFolders always returns a single folder. Minstrel has no multi-library -// concept in M1; clients use the folder id as an opaque cookie so we return 1. +// getMusicFolders always returns a single folder: Minstrel serves one library, +// and clients use the folder id as an opaque cookie, so it is 1. func (b *browseHandlers) getMusicFolders(w http.ResponseWriter, r *http.Request) { Write(w, r, MusicFoldersResponse{ Envelope: NewEnvelope("ok"), @@ -236,9 +236,8 @@ func (b *browseHandlers) getSong(w http.ResponseWriter, r *http.Request) { } // getAlbumList2 returns a paged album list filtered by type. Supported types -// match the Subsonic spec § getAlbumList2. "recent"/"frequent" require play -// history (M2) and currently return an empty list rather than an error so -// clients that poll them don't break. +// match the Subsonic spec § getAlbumList2. "recent" and "frequent" return an +// empty list rather than an error, so clients that poll them don't break. func (b *browseHandlers) getAlbumList2(w http.ResponseWriter, r *http.Request) { params := r.URL.Query() listType := params.Get("type") @@ -285,15 +284,14 @@ func (b *browseHandlers) getAlbumList2(w http.ResponseWriter, r *http.Request) { WriteFail(w, r, ErrMissingParameter, "Missing required parameter: genre") return } - // Genre is a plain string as of #367 — the query now splits - // tracks.genre on [;,] instead of comparing the whole column, so a - // client asking for "Rock" also reaches tracks tagged "Rock;Pop". - // Previously those were unreachable from either of their genres. + // The query splits tracks.genre on [;,] rather than comparing the + // whole column, so a client asking for "Rock" also reaches tracks + // tagged "Rock;Pop". albums, err = q.ListAlbumsByGenre(r.Context(), dbq.ListAlbumsByGenreParams{ Genre: genre, Lim: int32(size), Off: int32(offset), }) case "recent", "frequent": - // Play history lands in M2; return empty to keep clients happy. + // Not served from play history: empty keeps clients working. albums = nil default: WriteFail(w, r, ErrGeneric, "Invalid list type: "+listType) diff --git a/internal/subsonic/replaygain_test.go b/internal/subsonic/replaygain_test.go index 774093c5..bbc08e81 100644 --- a/internal/subsonic/replaygain_test.go +++ b/internal/subsonic/replaygain_test.go @@ -11,7 +11,7 @@ import ( ) // OpenSubsonic's replayGain is an object in JSON and an element with -// attributes in XML. Clients parse both, so both are pinned (#4997). +// attributes in XML. Clients parse both, so both are pinned. func TestSongRef_ReplayGainEncodesForBothFormats(t *testing.T) { gain, peak := float32(-4.5), float32(0.8913) tr := dbq.Track{Title: "Song", FilePath: "/m/a.flac", FileFormat: "flac"} diff --git a/internal/subsonic/star.go b/internal/subsonic/star.go index 2a9fcc12..0c85d806 100644 --- a/internal/subsonic/star.go +++ b/internal/subsonic/star.go @@ -40,7 +40,7 @@ func (m *mediaHandlers) handleStar(w http.ResponseWriter, r *http.Request) { return } if trackID.Valid { - // Stars the song: every copy of it (M498 #5438). + // Stars the song: every copy of it. liked, err := q.LikeTrack(r.Context(), dbq.LikeTrackParams{UserID: user.ID, TrackID: trackID}) if err != nil { WriteFail(w, r, ErrGeneric, "Could not star track") @@ -182,7 +182,7 @@ func (m *mediaHandlers) handleGetStarred2(w http.ResponseWriter, r *http.Request // loadStarred fetches the user's full starred set in liked_at DESC order, // projects to Subsonic refs (Song / Album / Artist). func loadStarred(ctx context.Context, q *dbq.Queries, userID pgtype.UUID) ([]SongRef, []AlbumRef, []ArtistRef, error) { - const cap = 500 // bounded for safety; M2 doesn't paginate getStarred + const cap = 500 // bounded for safety; getStarred is not paginated songs := []SongRef{} albums := []AlbumRef{} artists := []ArtistRef{} diff --git a/internal/subsonic/types.go b/internal/subsonic/types.go index 2414fe60..bbbc8f34 100644 --- a/internal/subsonic/types.go +++ b/internal/subsonic/types.go @@ -181,7 +181,7 @@ type SongRef struct { BitRate int `json:"bitRate,omitempty" xml:"bitRate,attr,omitempty"` IsDir bool `json:"isDir" xml:"isDir,attr"` Type string `json:"type" xml:"type,attr"` - // ReplayGain is OpenSubsonic's replayGain (M464 #4997): Minstrel's own + // ReplayGain is OpenSubsonic's replayGain: Minstrel's own // measurements, so third-party clients that level by ReplayGain do it // from the same numbers the Minstrel apps use. Omitted until measured. ReplayGain *ReplayGain `json:"replayGain,omitempty" xml:"replayGain,omitempty"` diff --git a/internal/subsonic/user.go b/internal/subsonic/user.go index 2f41e6b6..6195333e 100644 --- a/internal/subsonic/user.go +++ b/internal/subsonic/user.go @@ -52,9 +52,8 @@ func handleGetUser(w http.ResponseWriter, r *http.Request) { WriteFail(w, r, ErrNotAuthorized, "Not authorized to view other users") return } - // We don't have a per-user lookup wired up yet; until then admins asking - // for another username see the caller's roles. Acceptable while M1 has a - // single user; revisit once user management lands. + // No per-user lookup: an admin asking for another username is answered + // with the caller's own roles. Write(w, r, UserResponse{ Envelope: NewEnvelope("ok"), User: userPayload(caller), diff --git a/internal/sync/changes.go b/internal/sync/changes.go index a14b617d..ec03db41 100644 --- a/internal/sync/changes.go +++ b/internal/sync/changes.go @@ -38,9 +38,9 @@ func LogChange(ctx context.Context, dbtx dbq.DBTX, entityType EntityType, entity // LogChanges is LogChange for a set of entities of one type sharing one op. // -// Exists because the scan's reconcile pass can mark a quarter of a library -// missing in a single sweep (#2704), and a per-row loop there would be -// thousands of round-trips inside an operation that is already slow. Every +// Exists because the scan's reconcile pass can mark a large part of a library +// missing in a single sweep, and a per-row loop there would be thousands of +// round-trips inside an operation that is already slow. Every // other caller mutates one entity and should keep using LogChange. // // A no-op on an empty set, so callers don't have to guard. diff --git a/internal/sync/compactor.go b/internal/sync/compactor.go index e26943f1..93829b2b 100644 --- a/internal/sync/compactor.go +++ b/internal/sync/compactor.go @@ -1,15 +1,12 @@ -// Periodic compactor for the library_changes log (#357 deferred -// follow-up). Every mutation in the library writes one row, so the -// table grows unbounded without a retention policy. This worker -// trims rows older than CompactorRetention on a daily tick. +// Periodic compactor for the library_changes log. Every mutation in the +// library writes one row, so the table grows unbounded without a retention +// policy. This worker trims rows older than CompactorRetention on a daily +// tick. // -// Why daily / 30 days: Flutter clients track a per-app cursor and -// pull deltas from the last seen id. As long as a client checks in -// at least once per retention window, it never sees the cursor go -// stale. Clients that drop offline longer than that hit the /api/ -// library/sync 410 fallback and do a full resync. 30 days is the -// established norm for similar offline-cache delta logs and matches -// the spec at docs/superpowers/specs/2026-05-09-flutter-offline-mode-design.md. +// Why 30 days: clients keep a cursor and pull deltas from the last seen id. +// A client that checks in at least once per retention window never sees its +// cursor go stale; one offline longer than that gets the /api/library/sync +// 410 and does a full resync. package sync diff --git a/internal/sync/types.go b/internal/sync/types.go index a95baba5..1865caa4 100644 --- a/internal/sync/types.go +++ b/internal/sync/types.go @@ -1,4 +1,4 @@ -// Package sync supports the Flutter delta-sync endpoint by writing an +// Package sync supports the delta-sync endpoint by writing an // append-only library_changes log alongside every mutation on tracked // entities (artists, albums, tracks, likes, playlists, playlist_tracks). // diff --git a/internal/tags/enricher.go b/internal/tags/enricher.go index b41ac4cb..a17ef139 100644 --- a/internal/tags/enricher.go +++ b/internal/tags/enricher.go @@ -93,7 +93,7 @@ func (e *Enricher) EnrichTrack(ctx context.Context, trackID pgtype.UUID, ref Tra // tagFetch pairs a provider ID with a bound fetch call, so the merge-and- // classify loop below is shared between the track chain and the candidate- -// artist chain (#2376) instead of being written twice with one word changed. +// artist chain instead of being written twice with one word changed. type tagFetch struct { providerID string fetch func(context.Context) ([]Tag, error) @@ -241,7 +241,7 @@ func (e *Enricher) EnrichTrackBatch(ctx context.Context, limit int, } // EnrichCandidateArtist runs the artist-tag chain for one out-of-library -// Discover candidate and caches the merged result (#2376). +// Discover candidate and caches the merged result. // // Mirrors EnrichTrack, with one deliberate difference in the transient case: // there is no row to "leave NULL", because eligibility is the ABSENCE of a @@ -381,7 +381,7 @@ func (e *Enricher) logCandidateBatchSummary(eligible, processed, enriched, settl } // logBatchSummary emits one Info line with the category breakdown — the -// enriched/settled/left-null split is the operator's diagnostic for a +// enriched/settled/left-null split is the diagnostic for a // "0 enriched" symptom the collapsed tally can't explain. func (e *Enricher) logBatchSummary(eligible, processed, enriched, settledNone, leftNull, errored int) { e.logger.Info("tags: enrichment batch complete", diff --git a/internal/tags/enricher_chain_test.go b/internal/tags/enricher_chain_test.go index 23b73fb4..171dc9ab 100644 --- a/internal/tags/enricher_chain_test.go +++ b/internal/tags/enricher_chain_test.go @@ -9,7 +9,7 @@ import ( ) // runChain is the merge-and-classify loop shared by the track and -// candidate-artist drains (#2376). It touches no DB, so it is testable with a +// candidate-artist drains. It touches no DB, so it is testable with a // bare Enricher. // // The classification is the part that matters: "every source cleanly had diff --git a/internal/tags/provider.go b/internal/tags/provider.go index 900825f3..863d86aa 100644 --- a/internal/tags/provider.go +++ b/internal/tags/provider.go @@ -1,5 +1,5 @@ // Package tags enriches the taste profile's tag facet beyond raw ID3 -// genre (milestone #160, task #1490). It fetches folksonomy style/mood +// genre. It fetches folksonomy style/mood // tags for tracks from a chain of pluggable providers (MusicBrainz, // Last.fm, …), caches them in track_tags, and the taste recompute unions // them alongside tracks.genre. @@ -33,7 +33,7 @@ type Tag struct { // TrackRef is the lookup key passed to a provider. MBID is the track's // recording MBID (preferred by MBID-keyed providers); ArtistName + Title // are the always-populated fallback for name-based providers (Last.fm); -// ArtistMBID (nullable) enables MusicBrainz's artist-tag fallback (#1519) +// ArtistMBID (nullable) enables MusicBrainz's artist-tag fallback // when the recording itself is untagged or MBID-less. type TrackRef struct { MBID string @@ -55,7 +55,7 @@ type Provider interface { // RequiresAPIKey reports whether the provider needs a key to function. // The admin UI shows/hides the key field on this; it does NOT gate the - // enabled toggle (rule #26 — providers with usable defaults ship working). + // enabled toggle: a provider with usable defaults ships working. RequiresAPIKey() bool // DefaultEnabled is whether a freshly-installed Minstrel ships with @@ -63,7 +63,7 @@ type Provider interface { // (keyed) returns false — opt-in once a key is supplied. DefaultEnabled() bool - // Configure applies the operator's settings. Called at boot and on + // Configure applies the admin's settings. Called at boot and on // every admin PATCH; must be idempotent and goroutine-safe (the // enricher may read concurrently). Configure(ProviderSettings) error @@ -96,9 +96,8 @@ type ArtistRef struct { Name string } -// ArtistTagProvider is the artist-level tag capability, added for tag-space -// Discover (#2376). Separate from TrackTagProvider — exactly the split that -// interface's doc comment anticipated — so a source can implement either +// ArtistTagProvider is the artist-level tag capability, used by tag-space +// Discover. Separate from TrackTagProvider, so a source can implement either // without the other, and so the enricher can ask for the capability it needs // rather than checking at the call site. // diff --git a/internal/tags/provider_artist_tags_test.go b/internal/tags/provider_artist_tags_test.go index 489be798..8d99c401 100644 --- a/internal/tags/provider_artist_tags_test.go +++ b/internal/tags/provider_artist_tags_test.go @@ -8,7 +8,7 @@ import ( "testing" ) -// Artist-level tag fetching for out-of-library Discover candidates (#2376). +// Artist-level tag fetching for out-of-library Discover candidates. // Reuses the mbEntityServer / newMBProvider / newLastfmProvider helpers from // the per-provider test files. @@ -16,8 +16,8 @@ func TestMusicBrainzFetchArtistTags_UsesFullWeightNotTheFallbackDiscount(t *test // THE distinction worth a test. artistTagWeightFactor (0.6) exists because // FetchTrackTags uses artist tags as a *proxy* for a track's tags. Here the // artist IS the subject, so weights must land unscaled — otherwise these - // are not comparable with track_tags, which is the exact comparison slice 6 - // is built on. + // are not comparable with track_tags, which is the comparison tag-space + // Discover is built on. srv := mbEntityServer(``, `{"tags":[{"count":4,"name":"shoegaze"},{"count":2,"name":"dream pop"}]}`) defer srv.Close() old := mbBaseURL diff --git a/internal/tags/provider_lastfm.go b/internal/tags/provider_lastfm.go index 12917333..acca1e0f 100644 --- a/internal/tags/provider_lastfm.go +++ b/internal/tags/provider_lastfm.go @@ -122,7 +122,7 @@ func (p *lastfmProvider) FetchTrackTags(ctx context.Context, ref TrackRef) ([]Ta } // FetchArtistTags looks up an artist's top tags by name, with the MBID as an -// extra hint when present (#2376). Name-first is deliberate and the opposite +// extra hint when present. Name-first is deliberate and the opposite // emphasis from MusicBrainz: Last.fm's tag data is keyed on its own artist // pages, and autocorrect resolves most spelling drift from the similarity feed. // diff --git a/internal/tags/provider_musicbrainz.go b/internal/tags/provider_musicbrainz.go index c066c7b5..d477494d 100644 --- a/internal/tags/provider_musicbrainz.go +++ b/internal/tags/provider_musicbrainz.go @@ -20,7 +20,7 @@ const ( mbUserAgent = musicbrainz.UserAgent // artistTagWeightFactor down-weights artist-level fallback tags relative // to recording-specific tags — the artist's overall character is a coarser - // signal than a tag on the exact recording (#1519). + // signal than a tag on the exact recording. artistTagWeightFactor = 0.6 ) @@ -28,7 +28,7 @@ const ( // service. Keyless (the public endpoint needs no auth) and on by default — // the always-available baseline source. Prefers recording-level tags by the // track's recording MBID; when the recording is untagged or MBID-less it -// falls back to the artist's tags by artist MBID (#1519), down-weighted as a +// falls back to the artist's tags by artist MBID, down-weighted as a // coarser signal. Name-based lookup is deferred to providers like Last.fm. type musicbrainzProvider struct { enabled atomic.Bool @@ -42,7 +42,7 @@ func init() { UserAgent: mbUserAgent, HTTPClient: &http.Client{Timeout: 30 * time.Second}, // MusicBrainz's 1 request/second is per address: share it with - // the release catalogue (M502 #5519). + // the release catalogue. Pacer: musicbrainz.Shared(), MaxRetries: 3, }), @@ -99,7 +99,7 @@ func (p *musicbrainzProvider) FetchTrackTags(ctx context.Context, ref TrackRef) return nil, ErrNotFound } -// FetchArtistTags looks up an artist's own tags by MBID (#2376). MBID-only: +// FetchArtistTags looks up an artist's own tags by MBID. MBID-only: // MusicBrainz has no name-based tag lookup worth trusting for this, and a // wrong-artist match would poison the tag cache silently. // @@ -108,7 +108,7 @@ func (p *musicbrainzProvider) FetchTrackTags(ctx context.Context, ref TrackRef) // the artist's overall character is the coarser signal of the two. Here the // artist IS the subject, so there is nothing to discount relative to — and // applying it would make these weights incomparable with track_tags, which is -// exactly the comparison slice 6 depends on. +// the comparison tag-space Discover depends on. func (p *musicbrainzProvider) FetchArtistTags(ctx context.Context, ref ArtistRef) ([]Tag, error) { if !p.enabled.Load() || ref.MBID == "" { return nil, ErrNotFound @@ -159,7 +159,7 @@ func (p *musicbrainzProvider) TestConnection(ctx context.Context) error { } // ReleaseGroupForRelease names the MusicBrainz release group a release -// belongs to (#5241). Minstrel's albums.mbid is the release id from the tags, +// belongs to. Minstrel's albums.mbid is the release id from the tags, // while wants know albums only by release group, so re-acquiring a lost // album needs this translation when its files (and their release-group tag) // are already gone. diff --git a/internal/tags/provider_musicbrainz_test.go b/internal/tags/provider_musicbrainz_test.go index 9de04629..042887a1 100644 --- a/internal/tags/provider_musicbrainz_test.go +++ b/internal/tags/provider_musicbrainz_test.go @@ -177,7 +177,7 @@ func TestMusicBrainzFetch_EmptyTags(t *testing.T) { } } -// #5241: a release id resolves to its release group through +// A release id resolves to its release group through // /release/?inc=release-groups; an unknown id (or one that is already a // group) and a switched-off source both say ErrNotFound. func TestMusicBrainzReleaseGroupForRelease(t *testing.T) { diff --git a/internal/tags/worker.go b/internal/tags/worker.go index 35ab883c..0706a7ff 100644 --- a/internal/tags/worker.go +++ b/internal/tags/worker.go @@ -28,7 +28,7 @@ type Worker struct { // // candidateBatch is smaller than the track batch on purpose. Library tracks are // a finite set that drains to completion and then costs nothing; out-of-library -// candidates (#2376) are effectively unbounded — every library artist's +// candidates are effectively unbounded — every library artist's // neighbours — so this arm would otherwise monopolise every tick forever and // starve the track arm. 50/tick at ~1 req/s is roughly a minute of work, and // the query hands back the highest-similarity candidates first so the ones that diff --git a/internal/taste/engagement.go b/internal/taste/engagement.go index 40bcab7d..70613352 100644 --- a/internal/taste/engagement.go +++ b/internal/taste/engagement.go @@ -1,4 +1,4 @@ -// Package taste builds a persistent, decaying per-user taste profile (#796). +// Package taste builds a persistent, decaying per-user taste profile. // engagement.go is the pure core: it turns a single play's completion ratio // into a graded taste signal in [-1, +1], replacing the binary was_skipped // for learning purposes (was_skipped stays as-is for History display). diff --git a/internal/taste/profile.go b/internal/taste/profile.go index 65fd187a..2e43005f 100644 --- a/internal/taste/profile.go +++ b/internal/taste/profile.go @@ -1,7 +1,7 @@ // profile.go assembles the per-user taste profile from graded play // engagement (decayed over time) plus explicit-like bonuses, and -// atomic-replaces the taste_profile_* tables. Phase 1 only builds the -// object; candidate sourcing + scoring consume it in phase 2. +// atomic-replaces the taste_profile_* tables, which candidate sourcing and +// scoring read. package taste import ( @@ -19,9 +19,8 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/mood" ) -// Config holds the operator-tunable knobs. DefaultConfig supplies tuned -// starting values; wiring these into the server RecommendationConfig is a -// later follow-up (phase 1 uses the defaults). +// Config holds the tunable knobs. DefaultConfig supplies the shipped values; +// internal/recsettings overlays what an admin has tuned. type Config struct { Engagement EngagementParams @@ -37,8 +36,8 @@ type Config struct { TrackLikeBonus float64 TagLikeBonus float64 - // EnrichedTagScale weights folksonomy tags (from the track_tags cache, - // #1490) relative to a track's raw ID3 genre when both fold into the tag + // EnrichedTagScale weights folksonomy tags (from the track_tags cache) + // relative to a track's raw ID3 genre when both fold into the tag // facet. Each enriched tag contributes base × tag.weight × this scale, // where base is the play's decayed engagement (or the tag-like bonus for // a liked track) and tag.weight is the normalized folksonomy strength in @@ -47,19 +46,19 @@ type Config struct { // contribution entirely (falls back to genre-only). EnrichedTagScale float64 - // EraScale weights the decade/era facet (#1530): each play adds + // EraScale weights the decade/era facet: each play adds // base × this scale to its album's decade bucket ("1990s"), where base // is the decayed engagement (or the tag-like bonus for a liked track). // Era is a coarse signal, so it's kept ≤ 1 to imprint at most as strongly - // as the raw engagement; 0 disables the era facet entirely. Tunable in the - // lab (mirrors EnrichedTagScale, #1520). + // as the raw engagement; 0 disables the era facet entirely. Tunable in + // Admin › Tuning. EraScale float64 - // MoodScale weights the mood facet (#1534): each play adds base × this + // MoodScale weights the mood facet: each play adds base × this // scale to the canonical mood buckets (melancholic / chill / …) derived // from its enriched folksonomy tags via internal/mood. Coverage is partial // (depends on tag enrichment), so it's a supplement kept ≤ 1; 0 disables - // the mood facet entirely. Tunable in the lab (mirrors EraScale). + // the mood facet entirely. Tunable in Admin › Tuning. MoodScale float64 // ArtistFloor / TagFloor clamp how negative a single entity's weight may @@ -166,7 +165,7 @@ func accumulate( if err != nil { return nil, nil, nil, nil, fmt.Errorf("taste: load play engagement: %w", err) } - // Enriched folksonomy tags keyed by track (#1490) — folded into the tag + // Enriched folksonomy tags keyed by track, folded into the tag // facet alongside raw ID3 genre so a coarse "Rock" gains the cached // "post-punk / shoegaze / melancholic" vocabulary. Window matches the // play window; a track with no cached tags simply contributes genre only. diff --git a/internal/tracks/service.go b/internal/tracks/service.go index ba669460..f01e9ce7 100644 --- a/internal/tracks/service.go +++ b/internal/tracks/service.go @@ -55,13 +55,9 @@ func NewService(pool *pgxpool.Pool, logger *slog.Logger, dataDir string) *Servic // empty (only set if deletedAlbumID is also set). // - err: ErrNotFound, or a failure before anything was deleted. When the // file cannot be removed it is a *library.FileRemoveError and NOTHING was -// deleted — see library.DeleteTrackFile for why that order is the contract -// (#3918). +// deleted — see library.DeleteTrackFile for why that order is the contract. // -// adminID is currently unused — the cascade audit-log line that would -// reference it isn't wired in this slice. It's threaded through the -// signature so the upcoming admin_tracks handler doesn't have to -// re-plumb when audit logging lands. +// adminID is accepted for the audit log but not yet written to it. func (s *Service) RemoveTrack( ctx context.Context, trackID, adminID pgtype.UUID, //nolint:revive // adminID reserved for audit-log wiring in a follow-up diff --git a/internal/words/words.go b/internal/words/words.go index 4f890cfe..d4bbbd96 100644 --- a/internal/words/words.go +++ b/internal/words/words.go @@ -6,7 +6,7 @@ import "strconv" // Count says n with the noun that agrees with it: "1 track", "3 tracks". The // noun may carry its verb ("track went missing" / "tracks went missing"). -// The web app's plural() is the same thing on the other side (M505). +// The web app's plural() is the same thing on the other side. func Count[N ~int | ~int32 | ~int64](n N, one, many string) string { if n == 1 { return "1 " + one