feat(tags): folksonomy tag enricher — pluggable provider chain (#1490 Step 2)

Milestone #160 Option 1, Step 2. New internal/tags package that enriches
the taste profile's tag facet beyond raw ID3 genre, built so adding a
source later is "implement TrackTagProvider + Register()" — no enricher,
settings, or schema change (operator directive).

Mirrors the internal/coverart pattern:
- Provider interface + package registry (Register/AllProviders/ByID);
  TrackTagProvider fetch capability + TestableProvider for the admin test.
- DB-backed SettingsService over new tag_provider_settings +
  tag_sources_meta (migration 0043) — enable/key/version, boot
  reconciliation, and a provider-hash bump that re-opens 'none' rows when
  the compiled-in provider set changes.
- Slim self-contained httpClient (rate-limit + retry + User-Agent), kept
  local so tag enrichment never depends on coverart internals.

Providers:
- MusicBrainz: keyless, default-ON, recording tags by MBID (rule #26 baseline).
- Last.fm: keyed, default-OFF, track.getTopTags by artist+track — opt-in
  once a key is supplied.

Enricher uses MERGE semantics (differs from coverart's first-success-wins
for a single image): unions tags across every enabled provider, caps to
top-K by weight, and stamps tag_source musicbrainz|lastfm|mixed|none.
Writes are transactional (atomic replace of track_tags).

Unit-tested without a DB: registry mechanics, provider JSON parsing +
weight normalization via httptest, and the pure merge/top-K/source-label
helpers. Wiring (startup + on-scan), integration tests, and the taste
union (Step 3) + Settings UI (Step 4) come next.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

internal/db/dbq/models.go

@@ -532,6 +532,21 @@ type SystemPlaylistRun struct {
 	LastError   *string
 }
 
+type TagProviderSetting struct {
+	ProviderID   string
+	Enabled      bool
+	ApiKey       *string
+	DisplayOrder int32
+	CreatedAt    pgtype.Timestamptz
+	UpdatedAt    pgtype.Timestamptz
+}
+
+type TagSourcesMetum struct {
+	ID                          bool
+	CurrentVersion              int32
+	LastRegisteredProvidersHash string
+}
+
 type TasteProfileArtist struct {
 	UserID    pgtype.UUID
 	ArtistID  pgtype.UUID

internal/db/dbq/tag_settings.sql.go

@@ -0,0 +1,157 @@
+// Code generated by sqlc. DO NOT EDIT.
+// versions:
+//   sqlc v1.31.1
+// source: tag_settings.sql
+
+package dbq
+
+import (
+	"context"
+)
+
+const bumpTagSourcesVersion = `-- name: BumpTagSourcesVersion :one
+UPDATE tag_sources_meta
+   SET current_version = current_version + 1
+ WHERE id = true
+RETURNING current_version
+`
+
+// Increment + return the new version. Called only when the enabled set
+// changes (not on key edits).
+func (q *Queries) BumpTagSourcesVersion(ctx context.Context) (int32, error) {
+	row := q.db.QueryRow(ctx, bumpTagSourcesVersion)
+	var current_version int32
+	err := row.Scan(&current_version)
+	return current_version, err
+}
+
+const bumpTagVersionAndSetProvidersHash = `-- name: BumpTagVersionAndSetProvidersHash :one
+UPDATE tag_sources_meta
+   SET current_version = current_version + 1,
+       last_registered_providers_hash = $1
+ WHERE id = true
+RETURNING current_version
+`
+
+// Atomically increment the version and store the new registered-provider
+// hash. Called at boot when the compiled-in provider set changed, so
+// 'none' rows become eligible for a retry through the new chain.
+func (q *Queries) BumpTagVersionAndSetProvidersHash(ctx context.Context, lastRegisteredProvidersHash string) (int32, error) {
+	row := q.db.QueryRow(ctx, bumpTagVersionAndSetProvidersHash, lastRegisteredProvidersHash)
+	var current_version int32
+	err := row.Scan(&current_version)
+	return current_version, err
+}
+
+const getCurrentTagSourcesVersion = `-- name: GetCurrentTagSourcesVersion :one
+SELECT current_version FROM tag_sources_meta WHERE id = true
+`
+
+func (q *Queries) GetCurrentTagSourcesVersion(ctx context.Context) (int32, error) {
+	row := q.db.QueryRow(ctx, getCurrentTagSourcesVersion)
+	var current_version int32
+	err := row.Scan(&current_version)
+	return current_version, err
+}
+
+const getTagProvidersHash = `-- name: GetTagProvidersHash :one
+SELECT last_registered_providers_hash
+  FROM tag_sources_meta
+ WHERE id = true
+`
+
+func (q *Queries) GetTagProvidersHash(ctx context.Context) (string, error) {
+	row := q.db.QueryRow(ctx, getTagProvidersHash)
+	var last_registered_providers_hash string
+	err := row.Scan(&last_registered_providers_hash)
+	return last_registered_providers_hash, err
+}
+
+const listTagProviderSettings = `-- name: ListTagProviderSettings :many
+
+SELECT provider_id, enabled, api_key, display_order, created_at, updated_at
+  FROM tag_provider_settings
+ ORDER BY display_order, provider_id
+`
+
+// Tag-enrichment provider settings queries (milestone #160, #1490 Step 2).
+// The tags.SettingsService consumes these — parallel to the cover-art
+// SettingsService (coverart_settings.sql). Keeping the two independent
+// means a new tag source is added without touching the art settings.
+// All rows, for boot reconciliation and the admin GET handler.
+func (q *Queries) ListTagProviderSettings(ctx context.Context) ([]TagProviderSetting, error) {
+	rows, err := q.db.Query(ctx, listTagProviderSettings)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	var items []TagProviderSetting
+	for rows.Next() {
+		var i TagProviderSetting
+		if err := rows.Scan(
+			&i.ProviderID,
+			&i.Enabled,
+			&i.ApiKey,
+			&i.DisplayOrder,
+			&i.CreatedAt,
+			&i.UpdatedAt,
+		); err != nil {
+			return nil, err
+		}
+		items = append(items, i)
+	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+	return items, nil
+}
+
+const updateTagProviderSettings = `-- name: UpdateTagProviderSettings :exec
+UPDATE tag_provider_settings
+   SET enabled    = COALESCE($2, enabled),
+       api_key    = CASE WHEN $3::boolean THEN $4 ELSE api_key END,
+       updated_at = now()
+ WHERE provider_id = $1
+`
+
+type UpdateTagProviderSettingsParams struct {
+	ProviderID string
+	Enabled    bool
+	Column3    bool
+	ApiKey     *string
+}
+
+// Admin PATCH. $2 sets enabled. Pass FALSE for the $3 apiKeyChanged flag
+// to leave api_key untouched; TRUE with empty string clears it, TRUE with
+// a value sets it.
+func (q *Queries) UpdateTagProviderSettings(ctx context.Context, arg UpdateTagProviderSettingsParams) error {
+	_, err := q.db.Exec(ctx, updateTagProviderSettings,
+		arg.ProviderID,
+		arg.Enabled,
+		arg.Column3,
+		arg.ApiKey,
+	)
+	return err
+}
+
+const upsertTagProviderSettings = `-- name: UpsertTagProviderSettings :exec
+INSERT INTO tag_provider_settings (provider_id, enabled, display_order)
+VALUES ($1, $2, $3)
+ON CONFLICT (provider_id) DO UPDATE
+   SET display_order = EXCLUDED.display_order,
+       updated_at    = now()
+`
+
+type UpsertTagProviderSettingsParams struct {
+	ProviderID   string
+	Enabled      bool
+	DisplayOrder int32
+}
+
+// INSERT a default row for a newly-registered provider, or refresh an
+// existing row's display_order on boot. Does NOT overwrite enabled /
+// api_key on conflict — the operator's settings persist across restarts.
+func (q *Queries) UpsertTagProviderSettings(ctx context.Context, arg UpsertTagProviderSettingsParams) error {
+	_, err := q.db.Exec(ctx, upsertTagProviderSettings, arg.ProviderID, arg.Enabled, arg.DisplayOrder)
+	return err
+}

internal/db/migrations/0043_tag_provider_settings.down.sql

@@ -0,0 +1,3 @@
+-- Reverse 0043_tag_provider_settings.up.sql.
+DROP TABLE IF EXISTS tag_sources_meta;
+DROP TABLE IF EXISTS tag_provider_settings;

internal/db/migrations/0043_tag_provider_settings.up.sql

@@ -0,0 +1,37 @@
+-- Tag-enrichment provider settings (milestone #160, #1490 Step 2).
+--
+-- Mirrors the cover-art provider-settings substrate (migration 0018) so
+-- adding a new tag source later is "register a provider + it auto-upserts
+-- a row here" — never a schema change. The tag enricher reads the enabled
+-- set + api keys from tag_provider_settings and stamps the current version
+-- from tag_sources_meta onto tracks.tag_sources_version (added in 0042).
+--
+-- current_version starts at 1 while tracks.tag_sources_version defaults to
+-- 0 (migration 0042), so every existing track is "stale" relative to
+-- current and gets enriched on the first pass after this migrates.
+
+CREATE TABLE tag_provider_settings (
+    provider_id   text PRIMARY KEY,
+    enabled       boolean     NOT NULL DEFAULT true,
+    api_key       text,
+    display_order int         NOT NULL DEFAULT 0,
+    created_at    timestamptz NOT NULL DEFAULT now(),
+    updated_at    timestamptz NOT NULL DEFAULT now()
+);
+
+CREATE TABLE tag_sources_meta (
+    id                              boolean PRIMARY KEY DEFAULT true,
+    current_version                 int     NOT NULL DEFAULT 1,
+    last_registered_providers_hash  text    NOT NULL DEFAULT '',
+    CONSTRAINT tag_sources_meta_singleton CHECK (id = true)
+);
+INSERT INTO tag_sources_meta (id, current_version) VALUES (true, 1);
+
+-- Seed the v1 providers. MusicBrainz is the keyless always-on default;
+-- Last.fm ships disabled and is opt-in once the operator supplies a key
+-- (Step 4 Settings UI), per the no-coercive-settings rule (#26). The
+-- SettingsService also upserts these at boot, so the seed is belt-and-
+-- suspenders / documents the intended defaults.
+INSERT INTO tag_provider_settings (provider_id, enabled, display_order) VALUES
+    ('musicbrainz', true,  0),
+    ('lastfm',      false, 1);

internal/db/queries/tag_settings.sql

@@ -0,0 +1,56 @@
+-- Tag-enrichment provider settings queries (milestone #160, #1490 Step 2).
+-- The tags.SettingsService consumes these — parallel to the cover-art
+-- SettingsService (coverart_settings.sql). Keeping the two independent
+-- means a new tag source is added without touching the art settings.
+
+-- name: ListTagProviderSettings :many
+-- All rows, for boot reconciliation and the admin GET handler.
+SELECT provider_id, enabled, api_key, display_order, created_at, updated_at
+  FROM tag_provider_settings
+ ORDER BY display_order, provider_id;
+
+-- name: UpsertTagProviderSettings :exec
+-- INSERT a default row for a newly-registered provider, or refresh an
+-- existing row's display_order on boot. Does NOT overwrite enabled /
+-- api_key on conflict — the operator's settings persist across restarts.
+INSERT INTO tag_provider_settings (provider_id, enabled, display_order)
+VALUES ($1, $2, $3)
+ON CONFLICT (provider_id) DO UPDATE
+   SET display_order = EXCLUDED.display_order,
+       updated_at    = now();
+
+-- name: UpdateTagProviderSettings :exec
+-- Admin PATCH. $2 sets enabled. Pass FALSE for the $3 apiKeyChanged flag
+-- to leave api_key untouched; TRUE with empty string clears it, TRUE with
+-- a value sets it.
+UPDATE tag_provider_settings
+   SET enabled    = COALESCE($2, enabled),
+       api_key    = CASE WHEN $3::boolean THEN $4 ELSE api_key END,
+       updated_at = now()
+ WHERE provider_id = $1;
+
+-- name: GetCurrentTagSourcesVersion :one
+SELECT current_version FROM tag_sources_meta WHERE id = true;
+
+-- name: BumpTagSourcesVersion :one
+-- Increment + return the new version. Called only when the enabled set
+-- changes (not on key edits).
+UPDATE tag_sources_meta
+   SET current_version = current_version + 1
+ WHERE id = true
+RETURNING current_version;
+
+-- name: GetTagProvidersHash :one
+SELECT last_registered_providers_hash
+  FROM tag_sources_meta
+ WHERE id = true;
+
+-- name: BumpTagVersionAndSetProvidersHash :one
+-- Atomically increment the version and store the new registered-provider
+-- hash. Called at boot when the compiled-in provider set changed, so
+-- 'none' rows become eligible for a retry through the new chain.
+UPDATE tag_sources_meta
+   SET current_version = current_version + 1,
+       last_registered_providers_hash = $1
+ WHERE id = true
+RETURNING current_version;

internal/tags/enricher.go

@@ -0,0 +1,258 @@
+package tags
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"log/slog"
+	"sort"
+
+	"github.com/jackc/pgx/v5/pgtype"
+	"github.com/jackc/pgx/v5/pgxpool"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+)
+
+// defaultTopK caps how many tags are cached per track. Enough for a rich
+// facet, small enough that a heavily-tagged track can't swamp the taste
+// profile when the recompute unions them in.
+const defaultTopK = 8
+
+// Source labels stamped onto tracks.tag_source. A single contributing
+// provider stamps its own ID; two or more stamp "mixed". Kept in sync with
+// the SetTrackTagSource query's documented whitelist.
+const (
+	sourceNone  = "none"
+	sourceMixed = "mixed"
+)
+
+// outcome classifies one track's enrichment for the batch tally.
+type outcome int
+
+const (
+	outcomeEnriched outcome = iota // wrote ≥1 tag
+	outcomeNone                    // every provider had nothing → settled 'none'
+	outcomeLeftNull                // a transient failure → left NULL for retry
+)
+
+// Enricher drains tracks needing tags, fetches from the enabled provider
+// chain, MERGES the results (union across providers, unlike coverart's
+// first-success-wins for a single image), and caches the top-K into
+// track_tags. Owns no pool; receives one via the constructor so the boot
+// backfill, post-scan trigger, and admin endpoints share state.
+type Enricher struct {
+	pool     *pgxpool.Pool
+	logger   *slog.Logger
+	settings *SettingsService
+	topK     int
+}
+
+// NewEnricher constructs an Enricher.
+func NewEnricher(pool *pgxpool.Pool, logger *slog.Logger, settings *SettingsService) *Enricher {
+	return &Enricher{pool: pool, logger: logger, settings: settings, topK: defaultTopK}
+}
+
+// EnrichTrack runs the chain for one track and persists the merged result.
+//
+// Every enabled TrackTagProvider is queried and its tags unioned (max
+// weight wins on overlap). If any tags surface they're written with the
+// source stamped by which providers contributed (musicbrainz / lastfm /
+// mixed). If every provider cleanly reports ErrNotFound (or none are
+// enabled) the row settles to 'none'. If a provider fails transiently and
+// no tags surfaced, the row is left NULL for a next-pass retry.
+func (e *Enricher) EnrichTrack(ctx context.Context, trackID pgtype.UUID, ref TrackRef) (outcome, error) {
+	merged := map[string]float64{}
+	contributors := map[string]bool{}
+	anyTransient := false
+
+	for _, provider := range e.settings.EnabledTrackTagProviders() {
+		tags, perr := provider.FetchTrackTags(ctx, ref)
+		switch {
+		case perr == nil:
+			for _, t := range tags {
+				if t.Weight > merged[t.Name] {
+					merged[t.Name] = t.Weight
+				}
+			}
+			if len(tags) > 0 {
+				contributors[provider.ID()] = true
+			}
+		case errors.Is(perr, ErrNotFound):
+			// Clean "no data from this source" — try the next provider.
+		default:
+			anyTransient = true
+			e.logger.Warn("tags: provider fetch failed; continuing",
+				"track_id", uuidString(trackID), "provider", provider.ID(), "err", perr)
+		}
+	}
+
+	if len(merged) > 0 {
+		top := topKByWeight(merged, e.topK)
+		source := sourceLabel(contributorIDs(contributors))
+		if err := e.writeTags(ctx, trackID, top, source, e.settings.CurrentVersion()); err != nil {
+			return outcomeLeftNull, err
+		}
+		return outcomeEnriched, nil
+	}
+	if anyTransient {
+		// Nothing landed but a source may recover — leave NULL for retry.
+		return outcomeLeftNull, nil
+	}
+	// Every provider (if any) had nothing: settle 'none' so we don't
+	// re-process until the version bumps (a provider enabled/added).
+	if err := e.writeTags(ctx, trackID, nil, sourceNone, e.settings.CurrentVersion()); err != nil {
+		return outcomeNone, err
+	}
+	return outcomeNone, nil
+}
+
+// writeTags atomically replaces a track's cached tags and stamps the source
+// + version. tags may be empty (the 'none' settle path), which just clears
+// any prior tags and records the outcome.
+func (e *Enricher) writeTags(ctx context.Context, trackID pgtype.UUID, tags []Tag, source string, version int32) error {
+	tx, err := e.pool.Begin(ctx)
+	if err != nil {
+		return fmt.Errorf("begin tx: %w", err)
+	}
+	defer func() { _ = tx.Rollback(ctx) }()
+	q := dbq.New(tx)
+	if err := q.DeleteTrackTags(ctx, trackID); err != nil {
+		return fmt.Errorf("delete track tags: %w", err)
+	}
+	for _, t := range tags {
+		if err := q.InsertTrackTag(ctx, dbq.InsertTrackTagParams{
+			TrackID: trackID, Tag: t.Name, Weight: t.Weight,
+		}); err != nil {
+			return fmt.Errorf("insert track tag: %w", err)
+		}
+	}
+	src := source
+	if err := q.SetTrackTagSource(ctx, dbq.SetTrackTagSourceParams{
+		ID: trackID, TagSource: &src, TagSourcesVersion: version,
+	}); err != nil {
+		return fmt.Errorf("set track tag source: %w", err)
+	}
+	if err := tx.Commit(ctx); err != nil {
+		return fmt.Errorf("commit: %w", err)
+	}
+	return nil
+}
+
+// EnrichTrackBatch drains up to limit tracks needing tags and enriches each
+// serially. limit: 0 = disabled; >0 = bounded; <0 = unbounded (remote
+// providers self-throttle via their httpClient MinInterval). Returns the
+// processed/succeeded/failed tally for the scan-run record.
+func (e *Enricher) EnrichTrackBatch(ctx context.Context, limit int,
+	progressCb func(processed, succeeded, failed int)) (processed, succeeded, failed int, err error) {
+	if limit == 0 {
+		return 0, 0, 0, nil
+	}
+	queryLimit := int32(limit)
+	if limit < 0 {
+		queryLimit = 1<<31 - 1
+	}
+	q := dbq.New(e.pool)
+	rows, qerr := q.ListTracksMissingTags(ctx, dbq.ListTracksMissingTagsParams{
+		TagSourcesVersion: e.settings.CurrentVersion(),
+		Limit:             queryLimit,
+	})
+	if qerr != nil {
+		return 0, 0, 0, fmt.Errorf("list tracks missing tags: %w", qerr)
+	}
+
+	var enriched, settledNone, leftNull, errored int
+	for _, r := range rows {
+		if ctx.Err() != nil {
+			e.logBatchSummary(len(rows), processed, enriched, settledNone, leftNull, errored)
+			return processed, enriched, settledNone + leftNull + errored, ctx.Err()
+		}
+		processed++
+		ref := TrackRef{ArtistName: r.ArtistName, Title: r.Title}
+		if r.Mbid != nil {
+			ref.MBID = *r.Mbid
+		}
+		oc, eerr := e.EnrichTrack(ctx, r.ID, ref)
+		if eerr != nil {
+			e.logger.Warn("tags: batch entry failed", "track_id", uuidString(r.ID), "err", eerr)
+			errored++
+		} else {
+			switch oc {
+			case outcomeEnriched:
+				enriched++
+			case outcomeNone:
+				settledNone++
+			case outcomeLeftNull:
+				leftNull++
+			}
+		}
+		if progressCb != nil {
+			progressCb(processed, enriched, settledNone+leftNull+errored)
+		}
+	}
+	e.logBatchSummary(len(rows), processed, enriched, settledNone, leftNull, errored)
+	return processed, enriched, settledNone + leftNull + errored, nil
+}
+
+// logBatchSummary emits one Info line with the category breakdown — the
+// enriched/settled/left-null split is the operator's 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",
+		"eligible", eligible,
+		"processed", processed,
+		"enriched", enriched,
+		"settled_none", settledNone,
+		"left_null", leftNull,
+		"errored", errored)
+}
+
+// topKByWeight returns the highest-weighted tags, tie-broken by name for
+// determinism, capped at k (k<=0 means no cap).
+func topKByWeight(merged map[string]float64, k int) []Tag {
+	tags := make([]Tag, 0, len(merged))
+	for name, w := range merged {
+		tags = append(tags, Tag{Name: name, Weight: w})
+	}
+	sort.Slice(tags, func(i, j int) bool {
+		if tags[i].Weight != tags[j].Weight {
+			return tags[i].Weight > tags[j].Weight
+		}
+		return tags[i].Name < tags[j].Name
+	})
+	if k > 0 && len(tags) > k {
+		tags = tags[:k]
+	}
+	return tags
+}
+
+// contributorIDs returns the sorted set of provider IDs that returned tags.
+func contributorIDs(set map[string]bool) []string {
+	ids := make([]string, 0, len(set))
+	for id := range set {
+		ids = append(ids, id)
+	}
+	sort.Strings(ids)
+	return ids
+}
+
+// sourceLabel maps the contributing providers onto the tag_source value:
+// none / a single provider's own ID / "mixed" for two or more.
+func sourceLabel(ids []string) string {
+	switch len(ids) {
+	case 0:
+		return sourceNone
+	case 1:
+		return ids[0]
+	default:
+		return sourceMixed
+	}
+}
+
+// uuidString renders a pgtype.UUID for logs.
+func uuidString(u pgtype.UUID) string {
+	if !u.Valid {
+		return "<nil>"
+	}
+	return fmt.Sprintf("%x-%x-%x-%x-%x",
+		u.Bytes[0:4], u.Bytes[4:6], u.Bytes[6:8], u.Bytes[8:10], u.Bytes[10:16])
+}

internal/tags/enricher_test.go

@@ -0,0 +1,53 @@
+package tags
+
+import "testing"
+
+func TestTopKByWeight(t *testing.T) {
+	merged := map[string]float64{
+		"a": 0.9,
+		"b": 0.5,
+		"c": 0.5, // ties with b → name asc breaks it (b before c)
+		"d": 0.1,
+	}
+	got := topKByWeight(merged, 3)
+	if len(got) != 3 {
+		t.Fatalf("cap not applied: got %d", len(got))
+	}
+	wantOrder := []string{"a", "b", "c"}
+	for i, w := range wantOrder {
+		if got[i].Name != w {
+			t.Errorf("position %d = %q, want %q (full: %v)", i, got[i].Name, w, got)
+		}
+	}
+	// k<=0 means no cap.
+	if len(topKByWeight(merged, 0)) != 4 {
+		t.Error("k=0 should not cap")
+	}
+	if len(topKByWeight(nil, 5)) != 0 {
+		t.Error("empty input yields empty output")
+	}
+}
+
+func TestSourceLabel(t *testing.T) {
+	cases := []struct {
+		ids  []string
+		want string
+	}{
+		{nil, sourceNone},
+		{[]string{"musicbrainz"}, "musicbrainz"},
+		{[]string{"lastfm"}, "lastfm"},
+		{[]string{"lastfm", "musicbrainz"}, sourceMixed},
+	}
+	for _, c := range cases {
+		if got := sourceLabel(c.ids); got != c.want {
+			t.Errorf("sourceLabel(%v) = %q, want %q", c.ids, got, c.want)
+		}
+	}
+}
+
+func TestContributorIDs_Sorted(t *testing.T) {
+	got := contributorIDs(map[string]bool{"lastfm": true, "musicbrainz": true})
+	if len(got) != 2 || got[0] != "lastfm" || got[1] != "musicbrainz" {
+		t.Errorf("contributorIDs sorted = %v, want [lastfm musicbrainz]", got)
+	}
+}

internal/tags/httpclient.go

@@ -0,0 +1,184 @@
+package tags
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"math/rand"
+	"net/http"
+	"sync"
+	"time"
+)
+
+// httpClientOptions configures a httpClient. A trimmed sibling of the
+// coverart HTTP helper — JSON-only (tag APIs return no binary), plus a
+// User-Agent field because MusicBrainz rejects requests without one.
+// Kept local to this package so tag enrichment never depends on
+// coverart's internals.
+type httpClientOptions struct {
+	HTTPClient *http.Client
+	// Name prefixes wrapped error messages (e.g. "tags lastfm: ...").
+	Name string
+	// UserAgent is sent on every request. MusicBrainz mandates a
+	// descriptive one; empty omits the header.
+	UserAgent string
+	// MinInterval throttles requests; 0 disables rate limiting.
+	MinInterval time.Duration
+	// MaxRetries is the number of retry attempts on 429 / 5xx.
+	MaxRetries int
+	// BaseBackoff is the base unit for exponential backoff (2^attempt × base ± 20%).
+	BaseBackoff time.Duration
+	// TreatAuthAsTransient maps 401/403 to ErrTransient (keyed providers
+	// want this so a bad/expired key retries rather than settling 'none').
+	TreatAuthAsTransient bool
+	// JSONBodyLimit caps the response body read.
+	JSONBodyLimit int64
+}
+
+// httpClient performs rate-limited, retrying JSON GETs. Each provider
+// owns one, configured with provider-specific intervals + auth semantics.
+type httpClient struct {
+	opts httpClientOptions
+
+	mu       sync.Mutex
+	lastCall time.Time
+}
+
+func newHTTPClient(opts httpClientOptions) *httpClient {
+	if opts.HTTPClient == nil {
+		opts.HTTPClient = &http.Client{Timeout: 30 * time.Second}
+	}
+	if opts.MaxRetries == 0 {
+		opts.MaxRetries = 3
+	}
+	if opts.BaseBackoff == 0 {
+		opts.BaseBackoff = 250 * time.Millisecond
+	}
+	if opts.JSONBodyLimit == 0 {
+		opts.JSONBodyLimit = 1 * 1024 * 1024
+	}
+	return &httpClient{opts: opts}
+}
+
+// getJSON sends GET fullURL and decodes the 200 body into out (capped at
+// JSONBodyLimit). Retries 429/5xx with exponential backoff up to
+// MaxRetries. 404 → ErrNotFound; other non-2xx → wrapped ErrTransient.
+func (c *httpClient) getJSON(ctx context.Context, fullURL string, out any) error {
+	for attempt := 0; attempt <= c.opts.MaxRetries; attempt++ {
+		if err := c.rateLimitWait(ctx); err != nil {
+			return err
+		}
+		req, err := http.NewRequestWithContext(ctx, http.MethodGet, fullURL, nil)
+		if err != nil {
+			return fmt.Errorf("tags %s: build request: %w", c.opts.Name, err)
+		}
+		req.Header.Set("Accept", "application/json")
+		if c.opts.UserAgent != "" {
+			req.Header.Set("User-Agent", c.opts.UserAgent)
+		}
+
+		resp, err := c.opts.HTTPClient.Do(req)
+		if err != nil {
+			if attempt < c.opts.MaxRetries {
+				if waitErr := c.backoff(ctx, attempt); waitErr != nil {
+					return waitErr
+				}
+				continue
+			}
+			return fmt.Errorf("%w: %v", ErrTransient, err)
+		}
+
+		retry, ferr := c.handleStatus(ctx, resp, attempt)
+		if retry {
+			continue
+		}
+		if ferr != nil {
+			return ferr
+		}
+
+		body, rerr := io.ReadAll(io.LimitReader(resp.Body, c.opts.JSONBodyLimit))
+		_ = resp.Body.Close()
+		if rerr != nil {
+			return fmt.Errorf("%w: read body: %v", ErrTransient, rerr)
+		}
+		if uerr := json.Unmarshal(body, out); uerr != nil {
+			return fmt.Errorf("%w: decode body: %v", ErrTransient, uerr)
+		}
+		return nil
+	}
+	return fmt.Errorf("%w: exhausted retries", ErrTransient)
+}
+
+// handleStatus classifies a response. Returns retry=true to loop again
+// (after backing off), or a terminal error, or (false, nil) to proceed to
+// body read. Closes the body on every path except the proceed case.
+func (c *httpClient) handleStatus(ctx context.Context, resp *http.Response, attempt int) (retry bool, err error) {
+	switch {
+	case resp.StatusCode == http.StatusNotFound:
+		_ = resp.Body.Close()
+		return false, ErrNotFound
+	case c.opts.TreatAuthAsTransient &&
+		(resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden):
+		_ = resp.Body.Close()
+		return false, fmt.Errorf("%w: status %d (auth)", ErrTransient, resp.StatusCode)
+	case resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500:
+		_ = resp.Body.Close()
+		if attempt < c.opts.MaxRetries {
+			if waitErr := c.backoff(ctx, attempt); waitErr != nil {
+				return false, waitErr
+			}
+			return true, nil
+		}
+		return false, fmt.Errorf("%w: status %d", ErrTransient, resp.StatusCode)
+	case resp.StatusCode != http.StatusOK:
+		_ = resp.Body.Close()
+		return false, fmt.Errorf("%w: status %d", ErrTransient, resp.StatusCode)
+	}
+	return false, nil
+}
+
+// rateLimitWait blocks until MinInterval has elapsed since the last call,
+// then claims the slot. Disabled when MinInterval is 0.
+func (c *httpClient) rateLimitWait(ctx context.Context) error {
+	if c.opts.MinInterval <= 0 {
+		return nil
+	}
+	c.mu.Lock()
+	wait := time.Until(c.lastCall.Add(c.opts.MinInterval))
+	if wait > 0 {
+		timer := time.NewTimer(wait)
+		c.mu.Unlock()
+		select {
+		case <-ctx.Done():
+			timer.Stop()
+			return ctx.Err()
+		case <-timer.C:
+		}
+		c.mu.Lock()
+	}
+	c.lastCall = time.Now()
+	c.mu.Unlock()
+	return nil
+}
+
+// backoff sleeps for 2^attempt × BaseBackoff ± 20% jitter, or until ctx is done.
+func (c *httpClient) backoff(ctx context.Context, attempt int) error {
+	base := time.Duration(1<<attempt) * c.opts.BaseBackoff
+	jitterRange := int64(base) / 5
+	if jitterRange <= 0 {
+		jitterRange = 1
+	}
+	jitter := time.Duration(rand.Int63n(jitterRange))
+	if rand.Intn(2) == 0 {
+		jitter = -jitter
+	}
+	timer := time.NewTimer(base + jitter)
+	defer timer.Stop()
+	select {
+	case <-ctx.Done():
+		return ctx.Err()
+	case <-timer.C:
+		return nil
+	}
+}

internal/tags/provider.go

@@ -0,0 +1,158 @@
+// Package tags enriches the taste profile's tag facet beyond raw ID3
+// genre (milestone #160, task #1490). 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.
+//
+// The provider model mirrors internal/coverart deliberately: a source is
+// added by implementing TrackTagProvider and calling Register() in the
+// provider file's init() — no enricher, settings, or schema change. The
+// DB-backed SettingsService then auto-upserts a settings row for it, and
+// the boot-time provider-hash bump makes previously-settled rows eligible
+// for a retry through the widened chain.
+package tags
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"sync"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
+)
+
+// Tag is one folksonomy tag with a normalized strength in [0,1]. Each
+// provider normalizes its own upstream weighting (Last.fm's 0-100 count,
+// MusicBrainz's vote count) into this range so the enricher can merge
+// across providers on a common scale.
+type Tag struct {
+	Name   string
+	Weight float64
+}
+
+// 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).
+type TrackRef struct {
+	MBID       string
+	ArtistName string
+	Title      string
+}
+
+// Provider is the base shape every tag source implements. Providers
+// register at init() via Register(). The SettingsService reads the
+// per-provider row (enabled, api_key) and calls Configure().
+type Provider interface {
+	// ID is a stable lowercase token persisted in tracks.tag_source and
+	// keyed in tag_provider_settings.provider_id, and shown in the admin UI.
+	ID() string
+
+	// DisplayName is the human-readable label for the admin Settings card.
+	DisplayName() string
+
+	// 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).
+	RequiresAPIKey() bool
+
+	// DefaultEnabled is whether a freshly-installed Minstrel ships with
+	// this provider on. MusicBrainz (keyless) returns true; Last.fm
+	// (keyed) returns false — opt-in once a key is supplied.
+	DefaultEnabled() bool
+
+	// Configure applies the operator's settings. Called at boot and on
+	// every admin PATCH; must be idempotent and goroutine-safe (the
+	// enricher may read concurrently).
+	Configure(ProviderSettings) error
+}
+
+// ProviderSettings is the runtime config a provider receives on Configure.
+type ProviderSettings struct {
+	Enabled bool
+	APIKey  string
+}
+
+// TrackTagProvider is the tag-fetch capability. Split from Provider so a
+// future source with a different capability (e.g. artist-level tags) can
+// be added without widening this one.
+type TrackTagProvider interface {
+	Provider
+	// FetchTrackTags returns the folksonomy tags for the track, or
+	// ErrNotFound (terminal — nothing upstream) / ErrTransient (retry).
+	// A disabled provider, or one missing a required key, returns
+	// ErrNotFound so the chain simply skips it.
+	FetchTrackTags(ctx context.Context, ref TrackRef) ([]Tag, error)
+}
+
+// TestableProvider is an opt-in capability for the admin Test-Connection
+// button: answer "is my config working?" without a full enrichment cycle.
+type TestableProvider interface {
+	Provider
+	TestConnection(ctx context.Context) error
+}
+
+// ErrNotFound: the provider confirmed it has no tags for this track — a
+// terminal-this-pass result. The enricher moves to the next provider; if
+// every provider returns ErrNotFound the row settles to tag_source='none'.
+// Aliased to apierror.ErrNotFound for consistency with the rest of the app.
+var ErrNotFound = apierror.ErrNotFound
+
+// ErrTransient: a temporary failure (5xx, network, timeout, auth). The
+// enricher leaves the row's tag_source NULL so the next pass retries.
+var ErrTransient = errors.New("tags: transient error")
+
+// ErrProviderNotFound is returned when a named provider is not registered.
+var ErrProviderNotFound = errors.New("tags: provider not registered")
+
+// ErrNotTestable is returned when a provider doesn't implement
+// TestableProvider.
+var ErrNotTestable = errors.New("tags: provider does not support test connection")
+
+// registry holds all compiled-in providers. Providers register at init()
+// via Register(); the enricher iterates this list filtered by settings.
+var registry = struct {
+	mu        sync.Mutex
+	providers []Provider
+}{}
+
+// Register adds a provider. Must be called from a provider file's init().
+// Duplicate IDs panic — providers are compiled-in, so it's a build bug.
+func Register(p Provider) {
+	registry.mu.Lock()
+	defer registry.mu.Unlock()
+	for _, existing := range registry.providers {
+		if existing.ID() == p.ID() {
+			panic(fmt.Sprintf("tags: duplicate provider ID %q", p.ID()))
+		}
+	}
+	registry.providers = append(registry.providers, p)
+}
+
+// AllProviders returns a snapshot in registration order. Do not mutate.
+func AllProviders() []Provider {
+	registry.mu.Lock()
+	defer registry.mu.Unlock()
+	out := make([]Provider, len(registry.providers))
+	copy(out, registry.providers)
+	return out
+}
+
+// ProviderByID looks up a registered provider. Returns ErrProviderNotFound
+// if none matches.
+func ProviderByID(id string) (Provider, error) {
+	registry.mu.Lock()
+	defer registry.mu.Unlock()
+	for _, p := range registry.providers {
+		if p.ID() == id {
+			return p, nil
+		}
+	}
+	return nil, ErrProviderNotFound
+}
+
+// ResetRegistryForTests clears the registry. Test-only.
+func ResetRegistryForTests() {
+	registry.mu.Lock()
+	defer registry.mu.Unlock()
+	registry.providers = nil
+}

internal/tags/provider_lastfm.go

@@ -0,0 +1,168 @@
+package tags
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/url"
+	"sync/atomic"
+	"time"
+)
+
+// lastfmBaseURL is a var (not const) so tests can override it.
+var lastfmBaseURL = "https://ws.audioscrobbler.com/2.0/"
+
+const (
+	// lastfmMinPeriod throttles to 4 req/s under the 5/s per-key ceiling.
+	lastfmMinPeriod = 250 * time.Millisecond
+	// lastfmTagMaxCount is Last.fm's top-tag count scale (0-100 popularity).
+	lastfmTagMaxCount = 100.0
+)
+
+// lastfmTransientErrors are Last.fm JSON-level error codes that mean
+// "try again" rather than "no data": 8 operation failed, 11/16 service
+// temporarily unavailable, 29 rate limit. Everything else non-zero
+// (6 not found, 10 invalid key, …) is treated as ErrNotFound so the
+// chain simply skips this provider for the track.
+var lastfmTransientErrors = map[int]bool{8: true, 11: true, 16: true, 29: true}
+
+// lastfmProvider fetches track-level folksonomy tags via Last.fm's
+// track.getTopTags. Requires an operator-supplied API key; stays inert
+// (ErrNotFound on every call) when disabled or unkeyed. Name-based, so it
+// covers the many local tracks that lack a recording MBID.
+type lastfmProvider struct {
+	enabled atomic.Bool
+	apiKey  atomic.Pointer[string]
+	client  *httpClient
+}
+
+func init() {
+	Register(&lastfmProvider{
+		client: newHTTPClient(httpClientOptions{
+			Name:                 "lastfm",
+			HTTPClient:           &http.Client{Timeout: 30 * time.Second},
+			MinInterval:          lastfmMinPeriod,
+			MaxRetries:           3,
+			TreatAuthAsTransient: true,
+		}),
+	})
+}
+
+func (p *lastfmProvider) ID() string           { return "lastfm" }
+func (p *lastfmProvider) DisplayName() string  { return "Last.fm" }
+func (p *lastfmProvider) RequiresAPIKey() bool { return true }
+func (p *lastfmProvider) DefaultEnabled() bool { return false }
+
+func (p *lastfmProvider) Configure(s ProviderSettings) error {
+	p.enabled.Store(s.Enabled)
+	key := s.APIKey
+	p.apiKey.Store(&key)
+	return nil
+}
+
+func (p *lastfmProvider) currentKey() string {
+	if k := p.apiKey.Load(); k != nil {
+		return *k
+	}
+	return ""
+}
+
+// lastfmTopTags models the subset of track.getTopTags we read. Last.fm
+// returns `error` on failure responses instead of the toptags block.
+type lastfmTopTags struct {
+	TopTags struct {
+		Tag []lastfmTag `json:"tag"`
+	} `json:"toptags"`
+	Error int `json:"error"`
+}
+
+type lastfmTag struct {
+	Name  string `json:"name"`
+	Count int    `json:"count"`
+}
+
+// FetchTrackTags looks up the track's top tags by artist + title (with the
+// recording MBID as an extra hint when present). Returns ErrNotFound when
+// disabled, unkeyed, missing artist/title, or when Last.fm has no tags.
+func (p *lastfmProvider) FetchTrackTags(ctx context.Context, ref TrackRef) ([]Tag, error) {
+	if !p.enabled.Load() || p.currentKey() == "" {
+		return nil, ErrNotFound
+	}
+	if ref.ArtistName == "" || ref.Title == "" {
+		return nil, ErrNotFound
+	}
+
+	q := url.Values{
+		"method":      {"track.gettoptags"},
+		"api_key":     {p.currentKey()},
+		"format":      {"json"},
+		"artist":      {ref.ArtistName},
+		"track":       {ref.Title},
+		"autocorrect": {"1"},
+	}
+	if ref.MBID != "" {
+		q.Set("mbid", ref.MBID)
+	}
+
+	var resp lastfmTopTags
+	if err := p.client.getJSON(ctx, lastfmBaseURL+"?"+q.Encode(), &resp); err != nil {
+		return nil, err
+	}
+	if resp.Error != 0 {
+		if lastfmTransientErrors[resp.Error] {
+			return nil, ErrTransient
+		}
+		return nil, ErrNotFound
+	}
+	out := normalizeLastfmTags(resp.TopTags.Tag)
+	if len(out) == 0 {
+		return nil, ErrNotFound
+	}
+	return out, nil
+}
+
+// TestConnection verifies the key against a well-known track.
+func (p *lastfmProvider) TestConnection(ctx context.Context) error {
+	if p.currentKey() == "" {
+		return errors.New("tags: lastfm api key not set")
+	}
+	q := url.Values{
+		"method":  {"track.gettoptags"},
+		"api_key": {p.currentKey()},
+		"format":  {"json"},
+		"artist":  {"Radiohead"},
+		"track":   {"Creep"},
+	}
+	var resp lastfmTopTags
+	err := p.client.getJSON(ctx, lastfmBaseURL+"?"+q.Encode(), &resp)
+	if err != nil && !errors.Is(err, ErrNotFound) {
+		return err
+	}
+	if resp.Error == 10 { // invalid API key
+		return errors.New("tags: lastfm rejected the api key")
+	}
+	return nil
+}
+
+// normalizeLastfmTags scales the 0-100 popularity counts into [0,1] and
+// drops zero-count / unnamed tags.
+func normalizeLastfmTags(raw []lastfmTag) []Tag {
+	out := make([]Tag, 0, len(raw))
+	for _, t := range raw {
+		if t.Count <= 0 || t.Name == "" {
+			continue
+		}
+		w := float64(t.Count) / lastfmTagMaxCount
+		if w > 1 {
+			w = 1
+		}
+		out = append(out, Tag{Name: t.Name, Weight: w})
+	}
+	return out
+}
+
+// Compile-time capability checks.
+var (
+	_ TrackTagProvider = (*lastfmProvider)(nil)
+	_ TestableProvider = (*lastfmProvider)(nil)
+)

internal/tags/provider_lastfm_test.go

@@ -0,0 +1,101 @@
+package tags
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"testing"
+)
+
+func newLastfmProvider(enabled bool, key string) *lastfmProvider {
+	p := &lastfmProvider{client: newHTTPClient(httpClientOptions{
+		Name: "lastfm", MaxRetries: 1, TreatAuthAsTransient: true,
+	})}
+	_ = p.Configure(ProviderSettings{Enabled: enabled, APIKey: key})
+	return p
+}
+
+func TestNormalizeLastfmTags(t *testing.T) {
+	got := tagsByName(normalizeLastfmTags([]lastfmTag{
+		{Name: "indie", Count: 100},
+		{Name: "dream pop", Count: 40},
+		{Name: "zero-drop", Count: 0},
+		{Name: "", Count: 50},
+		{Name: "clamp", Count: 150}, // >100 clamps to 1.0
+	}))
+	if len(got) != 3 {
+		t.Fatalf("want 3 tags, got %v", got)
+	}
+	if got["indie"] != 1.0 || got["dream pop"] != 0.4 || got["clamp"] != 1.0 {
+		t.Errorf("weights = %v", got)
+	}
+}
+
+func TestLastfmFetch_GatedOff(t *testing.T) {
+	// Enabled but unkeyed → ErrNotFound.
+	if _, err := newLastfmProvider(true, "").FetchTrackTags(context.Background(),
+		TrackRef{ArtistName: "A", Title: "B"}); !errors.Is(err, ErrNotFound) {
+		t.Errorf("unkeyed: err = %v, want ErrNotFound", err)
+	}
+	// Keyed but disabled → ErrNotFound.
+	if _, err := newLastfmProvider(false, "k").FetchTrackTags(context.Background(),
+		TrackRef{ArtistName: "A", Title: "B"}); !errors.Is(err, ErrNotFound) {
+		t.Errorf("disabled: err = %v, want ErrNotFound", err)
+	}
+	// Keyed + enabled but missing artist/title → ErrNotFound.
+	if _, err := newLastfmProvider(true, "k").FetchTrackTags(context.Background(),
+		TrackRef{MBID: "x"}); !errors.Is(err, ErrNotFound) {
+		t.Errorf("no artist/title: err = %v, want ErrNotFound", err)
+	}
+}
+
+func TestLastfmFetch_ParsesTags(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte(`{"toptags":{"tag":[{"name":"shoegaze","count":100},{"name":"90s","count":30}]}}`))
+	}))
+	defer srv.Close()
+	old := lastfmBaseURL
+	lastfmBaseURL = srv.URL + "/"
+	defer func() { lastfmBaseURL = old }()
+
+	tags, err := newLastfmProvider(true, "k").FetchTrackTags(context.Background(),
+		TrackRef{ArtistName: "Slowdive", Title: "Alison"})
+	if err != nil {
+		t.Fatalf("fetch: %v", err)
+	}
+	m := tagsByName(tags)
+	if m["shoegaze"] != 1.0 || m["90s"] != 0.3 {
+		t.Errorf("weights = %v", m)
+	}
+}
+
+func TestLastfmFetch_ErrorCodes(t *testing.T) {
+	cases := []struct {
+		name      string
+		errorCode int
+		want      error
+	}{
+		{"not-found", 6, ErrNotFound},
+		{"transient-rate-limit", 29, ErrTransient},
+		{"transient-unavailable", 16, ErrTransient},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+				_, _ = w.Write([]byte(`{"error":` + strconv.Itoa(c.errorCode) + `,"message":"x"}`))
+			}))
+			defer srv.Close()
+			old := lastfmBaseURL
+			lastfmBaseURL = srv.URL + "/"
+			defer func() { lastfmBaseURL = old }()
+
+			_, err := newLastfmProvider(true, "k").FetchTrackTags(context.Background(),
+				TrackRef{ArtistName: "A", Title: "B"})
+			if !errors.Is(err, c.want) {
+				t.Errorf("error %d: got %v, want %v", c.errorCode, err, c.want)
+			}
+		})
+	}
+}

internal/tags/provider_musicbrainz.go

@@ -0,0 +1,123 @@
+package tags
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/url"
+	"sync/atomic"
+	"time"
+)
+
+// mbBaseURL is a var (not const) so tests can point it at an httptest server.
+var mbBaseURL = "https://musicbrainz.org/ws/2"
+
+const (
+	// mbMinPeriod honors MusicBrainz's 1 req/s anonymous ceiling (with headroom).
+	mbMinPeriod = 1100 * time.Millisecond
+	// mbUserAgent identifies the app — MusicBrainz rejects requests without one.
+	mbUserAgent = "Minstrel/1.0 ( https://git.fabledsword.com/bvandeusen/minstrel )"
+)
+
+// musicbrainzProvider fetches recording-level folksonomy tags from the
+// MusicBrainz web service. Keyless (the public endpoint needs no auth) and
+// on by default — the always-available baseline source. MBID-only: a track
+// with no recording MBID yields ErrNotFound (name search is ambiguous for
+// recordings and deferred to name-based providers like Last.fm).
+type musicbrainzProvider struct {
+	enabled atomic.Bool
+	client  *httpClient
+}
+
+func init() {
+	Register(&musicbrainzProvider{
+		client: newHTTPClient(httpClientOptions{
+			Name:        "musicbrainz",
+			UserAgent:   mbUserAgent,
+			HTTPClient:  &http.Client{Timeout: 30 * time.Second},
+			MinInterval: mbMinPeriod,
+			MaxRetries:  3,
+		}),
+	})
+}
+
+func (p *musicbrainzProvider) ID() string           { return "musicbrainz" }
+func (p *musicbrainzProvider) DisplayName() string  { return "MusicBrainz" }
+func (p *musicbrainzProvider) RequiresAPIKey() bool { return false }
+func (p *musicbrainzProvider) DefaultEnabled() bool { return true }
+
+func (p *musicbrainzProvider) Configure(s ProviderSettings) error {
+	p.enabled.Store(s.Enabled)
+	return nil
+}
+
+// mbTagsResponse models the subset of /ws/2/recording/{mbid}?inc=tags we read.
+type mbTagsResponse struct {
+	Tags []mbTag `json:"tags"`
+}
+
+type mbTag struct {
+	Count int    `json:"count"`
+	Name  string `json:"name"`
+}
+
+// FetchTrackTags looks up the recording's tags by MBID. Returns ErrNotFound
+// when disabled, when the track has no MBID, or when MusicBrainz has no tags.
+func (p *musicbrainzProvider) FetchTrackTags(ctx context.Context, ref TrackRef) ([]Tag, error) {
+	if !p.enabled.Load() || ref.MBID == "" {
+		return nil, ErrNotFound
+	}
+	q := url.Values{"inc": {"tags"}, "fmt": {"json"}}
+	full := mbBaseURL + "/recording/" + url.PathEscape(ref.MBID) + "?" + q.Encode()
+
+	var resp mbTagsResponse
+	if err := p.client.getJSON(ctx, full, &resp); err != nil {
+		return nil, err
+	}
+	out := normalizeMBTags(resp.Tags)
+	if len(out) == 0 {
+		return nil, ErrNotFound
+	}
+	return out, nil
+}
+
+// TestConnection issues a tiny search request; any non-transient response
+// means MusicBrainz is reachable.
+func (p *musicbrainzProvider) TestConnection(ctx context.Context) error {
+	full := mbBaseURL + "/recording?query=*&limit=1&fmt=json"
+	var discard map[string]any
+	err := p.client.getJSON(ctx, full, &discard)
+	if err == nil || errors.Is(err, ErrNotFound) {
+		return nil
+	}
+	return err
+}
+
+// normalizeMBTags drops non-positive-vote tags (net-downvoted or zero) and
+// scales the remaining vote counts into [0,1] relative to the strongest tag
+// on this recording, so one recording's raw counts don't dominate another's.
+func normalizeMBTags(raw []mbTag) []Tag {
+	maxCount := 0
+	for _, t := range raw {
+		if t.Count > maxCount {
+			maxCount = t.Count
+		}
+	}
+	if maxCount == 0 {
+		return nil
+	}
+	out := make([]Tag, 0, len(raw))
+	for _, t := range raw {
+		if t.Count <= 0 || t.Name == "" {
+			continue
+		}
+		out = append(out, Tag{Name: t.Name, Weight: float64(t.Count) / float64(maxCount)})
+	}
+	return out
+}
+
+// Compile-time capability checks.
+var (
+	_ TrackTagProvider = (*musicbrainzProvider)(nil)
+	_ TestableProvider = (*musicbrainzProvider)(nil)
+)

internal/tags/provider_musicbrainz_test.go

@@ -0,0 +1,92 @@
+package tags
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func newMBProvider(enabled bool) *musicbrainzProvider {
+	p := &musicbrainzProvider{client: newHTTPClient(httpClientOptions{Name: "musicbrainz", MaxRetries: 1})}
+	_ = p.Configure(ProviderSettings{Enabled: enabled})
+	return p
+}
+
+func tagsByName(ts []Tag) map[string]float64 {
+	m := make(map[string]float64, len(ts))
+	for _, t := range ts {
+		m[t.Name] = t.Weight
+	}
+	return m
+}
+
+func TestNormalizeMBTags(t *testing.T) {
+	got := tagsByName(normalizeMBTags([]mbTag{
+		{Count: 3, Name: "rock"},
+		{Count: 6, Name: "shoegaze"},
+		{Count: 0, Name: "zero-drop"},
+		{Count: -2, Name: "downvoted-drop"},
+		{Count: 5, Name: ""},
+	}))
+	if len(got) != 2 {
+		t.Fatalf("want 2 tags, got %v", got)
+	}
+	if got["shoegaze"] != 1.0 {
+		t.Errorf("shoegaze weight = %v, want 1.0", got["shoegaze"])
+	}
+	if got["rock"] != 0.5 {
+		t.Errorf("rock weight = %v, want 0.5", got["rock"])
+	}
+	if normalizeMBTags(nil) != nil {
+		t.Error("nil input should yield nil")
+	}
+}
+
+func TestMusicBrainzFetch_GatedOff(t *testing.T) {
+	// Disabled → ErrNotFound without any network call.
+	if _, err := newMBProvider(false).FetchTrackTags(context.Background(),
+		TrackRef{MBID: "abc"}); !errors.Is(err, ErrNotFound) {
+		t.Errorf("disabled: err = %v, want ErrNotFound", err)
+	}
+	// No MBID → ErrNotFound (MBID-only provider).
+	if _, err := newMBProvider(true).FetchTrackTags(context.Background(),
+		TrackRef{ArtistName: "A", Title: "B"}); !errors.Is(err, ErrNotFound) {
+		t.Errorf("no MBID: err = %v, want ErrNotFound", err)
+	}
+}
+
+func TestMusicBrainzFetch_ParsesTags(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte(`{"tags":[{"count":2,"name":"post-punk"},{"count":4,"name":"melancholic"}]}`))
+	}))
+	defer srv.Close()
+	old := mbBaseURL
+	mbBaseURL = srv.URL
+	defer func() { mbBaseURL = old }()
+
+	tags, err := newMBProvider(true).FetchTrackTags(context.Background(), TrackRef{MBID: "mbid-1"})
+	if err != nil {
+		t.Fatalf("fetch: %v", err)
+	}
+	m := tagsByName(tags)
+	if m["melancholic"] != 1.0 || m["post-punk"] != 0.5 {
+		t.Errorf("weights = %v, want melancholic=1.0 post-punk=0.5", m)
+	}
+}
+
+func TestMusicBrainzFetch_EmptyTags(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte(`{"tags":[]}`))
+	}))
+	defer srv.Close()
+	old := mbBaseURL
+	mbBaseURL = srv.URL
+	defer func() { mbBaseURL = old }()
+
+	if _, err := newMBProvider(true).FetchTrackTags(context.Background(),
+		TrackRef{MBID: "mbid-1"}); !errors.Is(err, ErrNotFound) {
+		t.Errorf("empty tags: err = %v, want ErrNotFound", err)
+	}
+}

internal/tags/provider_test.go

@@ -0,0 +1,85 @@
+package tags
+
+import "testing"
+
+// fakeProvider is a minimal Provider for registry mechanics tests.
+type fakeProvider struct {
+	id      string
+	deflt   bool
+	needKey bool
+}
+
+func (f *fakeProvider) ID() string                       { return f.id }
+func (f *fakeProvider) DisplayName() string              { return f.id }
+func (f *fakeProvider) RequiresAPIKey() bool             { return f.needKey }
+func (f *fakeProvider) DefaultEnabled() bool             { return f.deflt }
+func (f *fakeProvider) Configure(ProviderSettings) error { return nil }
+
+func TestRegisterAndLookup(t *testing.T) {
+	ResetRegistryForTests()
+	t.Cleanup(ResetRegistryForTests)
+
+	a := &fakeProvider{id: "alpha", deflt: true}
+	b := &fakeProvider{id: "beta"}
+	Register(a)
+	Register(b)
+
+	all := AllProviders()
+	if len(all) != 2 || all[0].ID() != "alpha" || all[1].ID() != "beta" {
+		t.Fatalf("AllProviders preserves registration order; got %v", all)
+	}
+
+	got, err := ProviderByID("beta")
+	if err != nil || got.ID() != "beta" {
+		t.Fatalf("ProviderByID(beta) = %v, %v", got, err)
+	}
+	if _, err := ProviderByID("missing"); err != ErrProviderNotFound {
+		t.Fatalf("ProviderByID(missing) err = %v, want ErrProviderNotFound", err)
+	}
+}
+
+func TestRegisterDuplicatePanics(t *testing.T) {
+	ResetRegistryForTests()
+	t.Cleanup(ResetRegistryForTests)
+	Register(&fakeProvider{id: "dup"})
+	defer func() {
+		if recover() == nil {
+			t.Error("expected panic on duplicate provider ID")
+		}
+	}()
+	Register(&fakeProvider{id: "dup"})
+}
+
+// TestBuiltinProvidersRegister confirms the two v1 sources register with
+// the expected identities/defaults. Runs against a fresh registry so it is
+// independent of test ordering: re-register constructed instances.
+func TestBuiltinProvidersRegister(t *testing.T) {
+	ResetRegistryForTests()
+	t.Cleanup(ResetRegistryForTests)
+	Register(&musicbrainzProvider{})
+	Register(&lastfmProvider{})
+
+	mb, err := ProviderByID("musicbrainz")
+	if err != nil {
+		t.Fatalf("musicbrainz not registered: %v", err)
+	}
+	if mb.RequiresAPIKey() || !mb.DefaultEnabled() {
+		t.Errorf("musicbrainz should be keyless + default-on; got key=%v deflt=%v",
+			mb.RequiresAPIKey(), mb.DefaultEnabled())
+	}
+	lf, err := ProviderByID("lastfm")
+	if err != nil {
+		t.Fatalf("lastfm not registered: %v", err)
+	}
+	if !lf.RequiresAPIKey() || lf.DefaultEnabled() {
+		t.Errorf("lastfm should require a key + default-off; got key=%v deflt=%v",
+			lf.RequiresAPIKey(), lf.DefaultEnabled())
+	}
+	// Both implement the tag-fetch + test capabilities.
+	if _, ok := mb.(TrackTagProvider); !ok {
+		t.Error("musicbrainz must implement TrackTagProvider")
+	}
+	if _, ok := lf.(TestableProvider); !ok {
+		t.Error("lastfm must implement TestableProvider")
+	}
+}

internal/tags/settings.go

@@ -0,0 +1,305 @@
+package tags
+
+import (
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
+	"log/slog"
+	"sort"
+	"strings"
+	"sync"
+
+	"github.com/jackc/pgx/v5/pgxpool"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+)
+
+// SettingsService reconciles the compiled-in provider registry with the
+// tag_provider_settings table at boot and on every admin update, and owns
+// the tag_sources_meta current_version stamp. Parallel to (and independent
+// of) coverart's SettingsService — the two never share a row, so a new tag
+// source is added without touching art settings.
+type SettingsService struct {
+	pool   *pgxpool.Pool
+	logger *slog.Logger
+
+	mu             sync.RWMutex
+	enabledIDs     map[string]bool
+	apiKeys        map[string]string
+	currentVersion int32
+}
+
+// ProviderUpdatePatch is the admin-PATCH body. nil pointers mean "leave
+// unchanged"; non-nil means "set" (empty APIKey clears it).
+type ProviderUpdatePatch struct {
+	Enabled *bool
+	APIKey  *string
+}
+
+// ProviderInfo is the settings view of a provider for the admin GET handler.
+type ProviderInfo struct {
+	ID             string
+	DisplayName    string
+	RequiresAPIKey bool
+	Supports       []string
+	Enabled        bool
+	APIKeySet      bool
+	DisplayOrder   int32
+	Testable       bool
+}
+
+// NewSettingsService boots the service: upserts default rows for any
+// registered provider, Configures each with its row, and loads the version.
+func NewSettingsService(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*SettingsService, error) {
+	s := &SettingsService{
+		pool:       pool,
+		logger:     logger,
+		enabledIDs: map[string]bool{},
+		apiKeys:    map[string]string{},
+	}
+	if err := s.reconcile(ctx); err != nil {
+		return nil, fmt.Errorf("tag settings boot: %w", err)
+	}
+	return s, nil
+}
+
+// reconcile reads the DB, inserts defaults for missing providers, Configures
+// each registered provider, and caches the current version.
+func (s *SettingsService) reconcile(ctx context.Context) error {
+	q := dbq.New(s.pool)
+
+	registered := AllProviders()
+	for i, p := range registered {
+		if err := q.UpsertTagProviderSettings(ctx, dbq.UpsertTagProviderSettingsParams{
+			ProviderID:   p.ID(),
+			Enabled:      p.DefaultEnabled(),
+			DisplayOrder: int32(i),
+		}); err != nil {
+			return fmt.Errorf("upsert provider %q: %w", p.ID(), err)
+		}
+	}
+
+	rows, err := q.ListTagProviderSettings(ctx)
+	if err != nil {
+		return fmt.Errorf("list provider settings: %w", err)
+	}
+	settingsByID := map[string]dbq.TagProviderSetting{}
+	for _, r := range rows {
+		settingsByID[r.ProviderID] = r
+	}
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.enabledIDs = map[string]bool{}
+	s.apiKeys = map[string]string{}
+	for _, p := range registered {
+		row, ok := settingsByID[p.ID()]
+		if !ok {
+			s.logger.Warn("tag settings: missing row after upsert", "provider", p.ID())
+			continue
+		}
+		key := ""
+		if row.ApiKey != nil {
+			key = *row.ApiKey
+		}
+		s.enabledIDs[p.ID()] = row.Enabled
+		s.apiKeys[p.ID()] = key
+		if err := p.Configure(ProviderSettings{Enabled: row.Enabled, APIKey: key}); err != nil {
+			s.logger.Warn("tag settings: provider Configure failed", "provider", p.ID(), "err", err)
+		}
+	}
+
+	v, err := q.GetCurrentTagSourcesVersion(ctx)
+	if err != nil {
+		return fmt.Errorf("get current tag sources version: %w", err)
+	}
+	s.currentVersion = v
+	return nil
+}
+
+// EnabledTrackTagProviders returns the enabled providers implementing
+// TrackTagProvider, in registration order. Snapshot — do not mutate.
+func (s *SettingsService) EnabledTrackTagProviders() []TrackTagProvider {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	var out []TrackTagProvider
+	for _, p := range AllProviders() {
+		if !s.enabledIDs[p.ID()] {
+			continue
+		}
+		if tp, ok := p.(TrackTagProvider); ok {
+			out = append(out, tp)
+		}
+	}
+	return out
+}
+
+// CurrentVersion returns the version the enricher stamps onto rows.
+func (s *SettingsService) CurrentVersion() int32 {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return s.currentVersion
+}
+
+// UpdateProvider applies an admin patch. Returns versionBumped=true if the
+// enabled set changed (not on key-only changes). Re-Configures the provider.
+func (s *SettingsService) UpdateProvider(ctx context.Context, providerID string, patch ProviderUpdatePatch) (bool, error) {
+	p, err := ProviderByID(providerID)
+	if err != nil {
+		return false, err
+	}
+	q := dbq.New(s.pool)
+
+	s.mu.RLock()
+	beforeSig := enabledSetSignature(s.enabledIDs)
+	currentEnabled := s.enabledIDs[providerID]
+	s.mu.RUnlock()
+
+	enabledVal := currentEnabled
+	if patch.Enabled != nil {
+		enabledVal = *patch.Enabled
+	}
+	if err := q.UpdateTagProviderSettings(ctx, dbq.UpdateTagProviderSettingsParams{
+		ProviderID: providerID,
+		Enabled:    enabledVal,
+		Column3:    patch.APIKey != nil,
+		ApiKey:     patch.APIKey,
+	}); err != nil {
+		return false, fmt.Errorf("update provider settings: %w", err)
+	}
+
+	rows, err := q.ListTagProviderSettings(ctx)
+	if err != nil {
+		return false, fmt.Errorf("re-list provider settings: %w", err)
+	}
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	for _, r := range rows {
+		key := ""
+		if r.ApiKey != nil {
+			key = *r.ApiKey
+		}
+		s.enabledIDs[r.ProviderID] = r.Enabled
+		s.apiKeys[r.ProviderID] = key
+		if r.ProviderID == providerID {
+			if err := p.Configure(ProviderSettings{Enabled: r.Enabled, APIKey: key}); err != nil {
+				s.logger.Warn("tag settings: Configure after update failed", "provider", providerID, "err", err)
+			}
+		}
+	}
+
+	if enabledSetSignature(s.enabledIDs) == beforeSig {
+		return false, nil
+	}
+	newVersion, err := q.BumpTagSourcesVersion(ctx)
+	if err != nil {
+		return false, fmt.Errorf("bump tag sources version: %w", err)
+	}
+	s.currentVersion = newVersion
+	return true, nil
+}
+
+// enabledSetSignature is a deterministic string of the enabled provider IDs.
+func enabledSetSignature(enabledIDs map[string]bool) string {
+	ids := make([]string, 0, len(enabledIDs))
+	for id, on := range enabledIDs {
+		if on {
+			ids = append(ids, id)
+		}
+	}
+	sort.Strings(ids)
+	return strings.Join(ids, ",")
+}
+
+// BumpVersionIfProvidersChanged bumps the version + stores the new provider
+// hash when the compiled-in provider set changed since the last boot, so
+// 'none' rows become eligible for a retry through the widened chain. This
+// is what makes "add a source" a zero-touch change — a new registered
+// provider trips this on the next start.
+func (s *SettingsService) BumpVersionIfProvidersChanged(ctx context.Context) (version int32, bumped bool, err error) {
+	hash := registeredProvidersHash()
+	q := dbq.New(s.pool)
+
+	stored, qerr := q.GetTagProvidersHash(ctx)
+	if qerr != nil {
+		return 0, false, fmt.Errorf("get providers hash: %w", qerr)
+	}
+	if stored == hash {
+		return s.CurrentVersion(), false, nil
+	}
+	newVer, berr := q.BumpTagVersionAndSetProvidersHash(ctx, hash)
+	if berr != nil {
+		return 0, false, fmt.Errorf("bump version: %w", berr)
+	}
+	s.mu.Lock()
+	s.currentVersion = newVer
+	s.mu.Unlock()
+	return newVer, true, nil
+}
+
+// registeredProvidersHash is the SHA-256 hex of the sorted, colon-joined
+// registered provider IDs.
+func registeredProvidersHash() string {
+	provs := AllProviders()
+	ids := make([]string, 0, len(provs))
+	for _, p := range provs {
+		ids = append(ids, p.ID())
+	}
+	sort.Strings(ids)
+	sum := sha256.Sum256([]byte(strings.Join(ids, ":")))
+	return hex.EncodeToString(sum[:])
+}
+
+// BumpVersion unconditionally increments the version. Used by the admin
+// "re-search missing tags" action to re-attempt every 'none' row next pass.
+func (s *SettingsService) BumpVersion(ctx context.Context) (int32, error) {
+	q := dbq.New(s.pool)
+	newVer, err := q.BumpTagVersionAndSetProvidersHash(ctx, registeredProvidersHash())
+	if err != nil {
+		return 0, fmt.Errorf("bump version: %w", err)
+	}
+	s.mu.Lock()
+	s.currentVersion = newVer
+	s.mu.Unlock()
+	return newVer, nil
+}
+
+// TestProvider invokes TestConnection if the provider supports it.
+func (s *SettingsService) TestProvider(ctx context.Context, providerID string) error {
+	p, err := ProviderByID(providerID)
+	if err != nil {
+		return err
+	}
+	tp, ok := p.(TestableProvider)
+	if !ok {
+		return ErrNotTestable
+	}
+	return tp.TestConnection(ctx)
+}
+
+// ListProviderInfo builds the admin-GET view in registration order.
+func (s *SettingsService) ListProviderInfo() []ProviderInfo {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	var out []ProviderInfo
+	for i, p := range AllProviders() {
+		info := ProviderInfo{
+			ID:             p.ID(),
+			DisplayName:    p.DisplayName(),
+			RequiresAPIKey: p.RequiresAPIKey(),
+			Enabled:        s.enabledIDs[p.ID()],
+			APIKeySet:      s.apiKeys[p.ID()] != "",
+			DisplayOrder:   int32(i),
+		}
+		if _, ok := p.(TrackTagProvider); ok {
+			info.Supports = append(info.Supports, "track_tags")
+		}
+		if _, ok := p.(TestableProvider); ok {
+			info.Testable = true
+		}
+		out = append(out, info)
+	}
+	return out
+}