diff --git a/internal/api/admin_recommendation_tuning.go b/internal/api/admin_recommendation_tuning.go index a97276c5..918a3b55 100644 --- a/internal/api/admin_recommendation_tuning.go +++ b/internal/api/admin_recommendation_tuning.go @@ -67,15 +67,30 @@ func tasteRespFrom(t recsettings.TasteTuning) tasteTuningResp { } } +// discoverTuningResp is the Discover scope on the wire (#2377). +type discoverTuningResp struct { + TagOverlapWeight float64 `json:"tag_overlap_weight"` + SnoozeDays float64 `json:"snooze_days"` +} + +func discoverRespFrom(d recsettings.DiscoverTuning) discoverTuningResp { + return discoverTuningResp{ + TagOverlapWeight: d.TagOverlapWeight, + SnoozeDays: d.SnoozeDays, + } +} + // tuningSnapshot is both the GET response and the post-mutation echo: // current values alongside shipped defaults so the card can mark // which knobs deviate. type tuningSnapshot struct { Profiles map[string]weightsResp `json:"profiles"` Taste tasteTuningResp `json:"taste"` + Discover discoverTuningResp `json:"discover"` Shipped struct { Profiles map[string]weightsResp `json:"profiles"` Taste tasteTuningResp `json:"taste"` + Discover discoverTuningResp `json:"discover"` } `json:"shipped"` } @@ -86,11 +101,13 @@ func (h *handlers) tuningSnapshot() tuningSnapshot { recsettings.ScopeDailyMix: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeDailyMix)), } out.Taste = tasteRespFrom(h.recSettings.Taste()) + out.Discover = discoverRespFrom(h.recSettings.Discover()) out.Shipped.Profiles = map[string]weightsResp{ recsettings.ScopeRadio: weightsRespFrom(recsettings.ShippedRadioWeights()), recsettings.ScopeDailyMix: weightsRespFrom(recsettings.ShippedDailyMixWeights()), } out.Shipped.Taste = tasteRespFrom(recsettings.ShippedTasteTuning()) + out.Shipped.Discover = discoverRespFrom(recsettings.ShippedDiscoverTuning()) return out } @@ -120,9 +137,12 @@ func (h *handlers) handlePatchRecommendationTuning(w http.ResponseWriter, r *htt } var err error - if scope == recsettings.ScopeTaste { + switch scope { + case recsettings.ScopeTaste: err = h.recSettings.UpdateTaste(r.Context(), body.Values) - } else { + case recsettings.ScopeDiscover: + err = h.recSettings.UpdateDiscover(r.Context(), body.Values) + default: err = h.recSettings.UpdateProfile(r.Context(), scope, body.Values) } if err != nil { diff --git a/internal/api/suggestions.go b/internal/api/suggestions.go index da3bd18c..977d571f 100644 --- a/internal/api/suggestions.go +++ b/internal/api/suggestions.go @@ -25,6 +25,12 @@ type suggestionView struct { Name string `json:"name"` Score float64 `json:"score"` Attribution []seedContributionView `json:"attribution"` + // MatchedTags are the candidate's tags that overlap the user's taste + // profile, strongest first (#2377) — the "matches: shoegaze, melancholic" + // line. Omitted when empty, which is common: tag coverage for + // out-of-library artists is permanently partial (#2376), and the card + // falls back to the seed attribution it has always shown. + MatchedTags []string `json:"matched_tags,omitempty"` // ImageURL is resolved on-demand from Lidarr (out-of-library // artists have no local art row). Omitted when Lidarr is disabled // or has no match — the client falls back to a placeholder. Not @@ -71,7 +77,11 @@ func (h *handlers) handleListSuggestions(w http.ResponseWriter, r *http.Request) halfLife = f } - suggestions, err := recommendation.SuggestArtists(r.Context(), h.pool, user.ID, halfLife, limit) + // Read the tuned weight per request so an admin change takes effect on the + // next refresh, no restart (rule #25). + tagWeight := h.recSettings.Discover().TagOverlapWeight + suggestions, err := recommendation.SuggestArtists( + r.Context(), h.pool, user.ID, halfLife, limit, tagWeight) if err != nil { h.logger.Error("api: list suggestions", "err", err) writeErr(w, apierror.InternalMsg("failed to load suggestions", err)) @@ -92,19 +102,17 @@ func (h *handlers) handleListSuggestions(w http.ResponseWriter, r *http.Request) } out = append(out, suggestionView{ MBID: s.MBID, Name: s.Name, Score: s.Score, Attribution: attr, + MatchedTags: s.MatchedTags, }) } h.resolveSuggestionArt(r.Context(), out) writeJSON(w, http.StatusOK, out) } -// Snooze duration bounds. 90 days is long enough that a parked suggestion -// stops feeling like it's nagging, short enough that a taste shift brings it -// back on its own — the whole point of a snooze over a dismissal (#2374). -const ( - defaultSnoozeDays = 90.0 - maxSnoozeDays = 365.0 -) +// maxSnoozeDays caps a client-supplied duration. The DEFAULT is not here: it's +// a DB-backed knob on the admin tuning card (rule #25), read per request via +// recSettings.Discover().SnoozeDays. See #2377. +const maxSnoozeDays = 365.0 // snoozeRequest is the POST body. Both fields are optional in the JSON sense // (an absent body snoozes for the default), but Name is required in practice: @@ -158,7 +166,7 @@ func (h *handlers) handleSnoozeSuggestion(w http.ResponseWriter, r *http.Request } days := body.Days if days <= 0 { - days = defaultSnoozeDays + days = h.recSettings.Discover().SnoozeDays } if days > maxSnoozeDays { // Clamp rather than reject: a client asking for longer than we allow diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 524bb6a9..bfbb4ef8 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -290,6 +290,13 @@ type DiagnosticEvent struct { ReceivedAt pgtype.Timestamptz } +type DiscoverTuning struct { + Singleton bool + TagOverlapWeight float64 + SnoozeDays float64 + UpdatedAt pgtype.Timestamptz +} + type GeneralLike struct { UserID pgtype.UUID TrackID pgtype.UUID diff --git a/internal/db/dbq/recommendation_tuning.sql.go b/internal/db/dbq/recommendation_tuning.sql.go index f90aaa97..251eeed5 100644 --- a/internal/db/dbq/recommendation_tuning.sql.go +++ b/internal/db/dbq/recommendation_tuning.sql.go @@ -9,6 +9,22 @@ import ( "context" ) +const getDiscoverTuning = `-- name: GetDiscoverTuning :one +SELECT singleton, tag_overlap_weight, snooze_days, updated_at FROM discover_tuning WHERE singleton = true +` + +func (q *Queries) GetDiscoverTuning(ctx context.Context) (DiscoverTuning, error) { + row := q.db.QueryRow(ctx, getDiscoverTuning) + var i DiscoverTuning + err := row.Scan( + &i.Singleton, + &i.TagOverlapWeight, + &i.SnoozeDays, + &i.UpdatedAt, + ) + return i, err +} + const getTasteTuning = `-- name: GetTasteTuning :one SELECT singleton, half_life_days, engagement_hard_skip, engagement_neutral, engagement_full, updated_at, enriched_tag_scale, era_scale, mood_scale FROM taste_tuning WHERE singleton = true ` @@ -118,6 +134,32 @@ func (q *Queries) ListWeightProfiles(ctx context.Context) ([]RecommendationWeigh return items, nil } +const updateDiscoverTuning = `-- name: UpdateDiscoverTuning :one +UPDATE discover_tuning + SET tag_overlap_weight = $1, + snooze_days = $2, + updated_at = now() + WHERE singleton = true +RETURNING singleton, tag_overlap_weight, snooze_days, updated_at +` + +type UpdateDiscoverTuningParams struct { + TagOverlapWeight float64 + SnoozeDays float64 +} + +func (q *Queries) UpdateDiscoverTuning(ctx context.Context, arg UpdateDiscoverTuningParams) (DiscoverTuning, error) { + row := q.db.QueryRow(ctx, updateDiscoverTuning, arg.TagOverlapWeight, arg.SnoozeDays) + var i DiscoverTuning + err := row.Scan( + &i.Singleton, + &i.TagOverlapWeight, + &i.SnoozeDays, + &i.UpdatedAt, + ) + return i, err +} + const updateTasteTuning = `-- name: UpdateTasteTuning :one UPDATE taste_tuning SET half_life_days = $1, @@ -226,6 +268,24 @@ func (q *Queries) UpdateWeightProfile(ctx context.Context, arg UpdateWeightProfi return i, err } +const upsertDiscoverTuningDefaults = `-- name: UpsertDiscoverTuningDefaults :exec +INSERT INTO discover_tuning (singleton, tag_overlap_weight, snooze_days) +VALUES (true, $1, $2) +ON CONFLICT (singleton) DO NOTHING +` + +type UpsertDiscoverTuningDefaultsParams struct { + TagOverlapWeight float64 + SnoozeDays float64 +} + +// Boot reconcile for the Discover scope (#2377). Never overwrites +// operator-tuned values, same contract as the other two. +func (q *Queries) UpsertDiscoverTuningDefaults(ctx context.Context, arg UpsertDiscoverTuningDefaultsParams) error { + _, err := q.db.Exec(ctx, upsertDiscoverTuningDefaults, arg.TagOverlapWeight, arg.SnoozeDays) + return err +} + const upsertTasteTuningDefaults = `-- name: UpsertTasteTuningDefaults :exec INSERT INTO taste_tuning ( singleton, half_life_days, engagement_hard_skip, diff --git a/internal/db/migrations/0051_discover_tuning.down.sql b/internal/db/migrations/0051_discover_tuning.down.sql new file mode 100644 index 00000000..1da70de4 --- /dev/null +++ b/internal/db/migrations/0051_discover_tuning.down.sql @@ -0,0 +1,10 @@ +-- Drop any audit rows under the scope the constraint is about to forbid, +-- otherwise re-adding the narrower CHECK fails against existing data. +DELETE FROM recommendation_tuning_audit WHERE scope = 'discover'; +ALTER TABLE recommendation_tuning_audit + DROP CONSTRAINT recommendation_tuning_audit_scope_check; +ALTER TABLE recommendation_tuning_audit + ADD CONSTRAINT recommendation_tuning_audit_scope_check + CHECK (scope IN ('radio', 'daily_mix', 'taste')); + +DROP TABLE IF EXISTS discover_tuning; diff --git a/internal/db/migrations/0051_discover_tuning.up.sql b/internal/db/migrations/0051_discover_tuning.up.sql new file mode 100644 index 00000000..54bc2be0 --- /dev/null +++ b/internal/db/migrations/0051_discover_tuning.up.sql @@ -0,0 +1,38 @@ +-- 0051_discover_tuning.up.sql — tunable knobs for the Discover request +-- surface (#2377, milestone #268 slice 6). +-- +-- A FOURTH tuning scope alongside radio / daily_mix / taste. Its own scope +-- rather than extra columns on taste_tuning, for a reason that matters: +-- snooze_days lives here, and a snooze must never be read as taste signal +-- (#2374). Filing it under 'taste' would put it one careless join away from +-- exactly the leak that design forbids. +-- +-- Per rule #25 these are DB-backed and editable in the admin UI with no +-- restart — the shipped values below are defaults, not settings. +CREATE TABLE discover_tuning ( + singleton boolean PRIMARY KEY DEFAULT true + CONSTRAINT discover_tuning_singleton_check CHECK (singleton), + -- How strongly taste-tag overlap boosts a candidate's similarity score. + -- The blend is MULTIPLICATIVE: score * (1 + w * overlap), overlap in + -- [0,1]. So 0 disables the feature outright and leaves pure similarity + -- ranking, 1.0 lets a perfectly-matching candidate double its score, and + -- a candidate with no cached tags is unchanged rather than penalised + -- (rule #131 — tag coverage is permanently partial, see #2376). + tag_overlap_weight double precision NOT NULL, + -- Default snooze duration in days. Was a Go constant in + -- internal/api/suggestions.go; moved here per rule #25. + snooze_days double precision NOT NULL, + updated_at timestamptz NOT NULL DEFAULT now() +); + +INSERT INTO discover_tuning (singleton, tag_overlap_weight, snooze_days) +VALUES (true, 1.0, 90); + +-- Rule #36: a new value for a CHECK-gated column needs the constraint +-- rewritten in the SAME change, or the first audit row written under the new +-- scope fails at runtime rather than at migrate time. +ALTER TABLE recommendation_tuning_audit + DROP CONSTRAINT recommendation_tuning_audit_scope_check; +ALTER TABLE recommendation_tuning_audit + ADD CONSTRAINT recommendation_tuning_audit_scope_check + CHECK (scope IN ('radio', 'daily_mix', 'taste', 'discover')); diff --git a/internal/db/queries/recommendation_tuning.sql b/internal/db/queries/recommendation_tuning.sql index b017e542..92ae3bce 100644 --- a/internal/db/queries/recommendation_tuning.sql +++ b/internal/db/queries/recommendation_tuning.sql @@ -53,6 +53,24 @@ UPDATE taste_tuning WHERE singleton = true RETURNING *; +-- name: UpsertDiscoverTuningDefaults :exec +-- Boot reconcile for the Discover scope (#2377). Never overwrites +-- operator-tuned values, same contract as the other two. +INSERT INTO discover_tuning (singleton, tag_overlap_weight, snooze_days) +VALUES (true, $1, $2) +ON CONFLICT (singleton) DO NOTHING; + +-- name: GetDiscoverTuning :one +SELECT * FROM discover_tuning WHERE singleton = true; + +-- name: UpdateDiscoverTuning :one +UPDATE discover_tuning + SET tag_overlap_weight = $1, + snooze_days = $2, + updated_at = now() + WHERE singleton = true +RETURNING *; + -- name: InsertTuningAudit :exec -- changes is a jsonb array of {field, old, new} objects. INSERT INTO recommendation_tuning_audit (scope, action, changes) diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go index b39669cc..c71b9aae 100644 --- a/internal/dbtest/reset.go +++ b/internal/dbtest/reset.go @@ -82,6 +82,10 @@ var dataTables = []string{ // (#1250), so truncating gives each test pristine tuning values. "recommendation_weight_profiles", "taste_tuning", + // #2377. Same reasoning as taste_tuning above: recsettings.New re-seeds + // shipped defaults on every construction, so truncating gives each test + // pristine Discover knobs rather than whatever a previous test tuned. + "discover_tuning", "recommendation_tuning_audit", "tracks", "albums", diff --git a/internal/recommendation/candidate_tags_integration_test.go b/internal/recommendation/candidate_tags_integration_test.go index eda08a71..373b4134 100644 --- a/internal/recommendation/candidate_tags_integration_test.go +++ b/internal/recommendation/candidate_tags_integration_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/jackc/pgx/v5/pgtype" "github.com/jackc/pgx/v5/pgxpool" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" @@ -280,3 +281,141 @@ func TestCandidateTags_CoverageCountsProcessedAndTagged(t *testing.T) { t.Errorf("with_tags = %d, want 2 ('none' excluded)", got.WithTags) } } + +// --- Slice 6 (#2377): the taste-tag blend, end to end --- +// +// The pure tests in tagoverlap_test.go cover the scoring maths. These cover the +// wiring the pure tests cannot reach: that loadTagInputs actually reads both +// sides from the DB and that the blend reaches the returned deck. + +func seedTasteTag(t *testing.T, pool *pgxpool.Pool, userID pgtype.UUID, tag string, weight float64) { + t.Helper() + if _, err := pool.Exec(context.Background(), + `INSERT INTO taste_profile_tags (user_id, tag, weight) VALUES ($1, $2, $3) + ON CONFLICT (user_id, tag) DO UPDATE SET weight = EXCLUDED.weight`, + userID, tag, weight, + ); err != nil { + t.Fatalf("seedTasteTag: %v", err) + } +} + +func seedCandidateTag(t *testing.T, pool *pgxpool.Pool, mbid, tag string, weight float64) { + t.Helper() + if err := dbq.New(pool).InsertCandidateArtistTag(context.Background(), + dbq.InsertCandidateArtistTagParams{CandidateMbid: mbid, Tag: tag, Weight: weight}, + ); err != nil { + t.Fatalf("seedCandidateTag: %v", err) + } +} + +// twoCandidatePool wires a liked seed with two unmatched neighbours: "loud" +// scores higher on similarity, "match" lower. Skewed on purpose — with equal +// similarity the reorder assertion below could not fail. +func twoCandidatePool(t *testing.T, pool *pgxpool.Pool, userID pgtype.UUID) { + t.Helper() + seed := seedArtist(t, pool, "Seed", "") + likeArtist(t, pool, userID, seed.ID) + seedUnmatched(t, pool, seed.ID, "loud", "Loud Neighbour", 0.9) + seedUnmatched(t, pool, seed.ID, "match", "Taste Match", 0.6) +} + +func TestSuggestArtists_TasteTagMatchOvertakesStrongerSimilarity(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + twoCandidatePool(t, pool, user.ID) + + seedTasteTag(t, pool, user.ID, "shoegaze", 10) + seedCandidateTag(t, pool, "match", "shoegaze", 1.0) + seedCandidateTag(t, pool, "loud", "death metal", 1.0) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 1.0) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 2 { + t.Fatalf("len = %d, want 2", len(out)) + } + if out[0].MBID != "match" { + t.Errorf("first = %q, want match (0.6×2 beats 0.9×1)", out[0].MBID) + } + // And it explains itself. + if len(out[0].MatchedTags) != 1 || out[0].MatchedTags[0] != "shoegaze" { + t.Errorf("matched tags = %v, want [shoegaze]", out[0].MatchedTags) + } + if out[1].MatchedTags != nil { + t.Errorf("non-matching candidate got matched tags: %v", out[1].MatchedTags) + } +} + +// The same fixture with the knob at 0 must return pure similarity order — the +// operator's off switch, verified against a real DB rather than assumed. +func TestSuggestArtists_ZeroTagWeightKeepsSimilarityOrder(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + twoCandidatePool(t, pool, user.ID) + + seedTasteTag(t, pool, user.ID, "shoegaze", 10) + seedCandidateTag(t, pool, "match", "shoegaze", 1.0) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if out[0].MBID != "loud" { + t.Errorf("first = %q, want loud (tag term disabled)", out[0].MBID) + } +} + +// A user with no taste profile must still get a full deck in similarity order: +// the cold-start path, which is every user's first days (rule #131). +func TestSuggestArtists_NoTasteTagsStillReturnsSimilarityOrder(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + twoCandidatePool(t, pool, user.ID) + // Candidate tags exist, but the user has no taste tags to match them. + seedCandidateTag(t, pool, "match", "shoegaze", 1.0) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 1.0) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 2 { + t.Fatalf("len = %d, want 2 (nothing dropped)", len(out)) + } + if out[0].MBID != "loud" { + t.Errorf("first = %q, want loud", out[0].MBID) + } +} + +// An untagged candidate must never be dropped or sunk just because another +// candidate has tags — permanently-partial coverage (#2376) must not become a +// permanent ranking penalty. +func TestSuggestArtists_UntaggedCandidateSurvivesAlongsideTagged(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + twoCandidatePool(t, pool, user.ID) + + seedTasteTag(t, pool, user.ID, "shoegaze", 10) + seedCandidateTag(t, pool, "match", "shoegaze", 1.0) + // "loud" is deliberately left with NO cached tags at all. + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 1.0) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 2 { + t.Fatalf("len = %d, want 2 — the untagged candidate must still appear", len(out)) + } + var loud *ArtistSuggestion + for i := range out { + if out[i].MBID == "loud" { + loud = &out[i] + } + } + if loud == nil { + t.Fatal("untagged candidate vanished from the deck") + } + if loud.Score != 0.9 { + t.Errorf("untagged score = %v, want 0.9 unchanged", loud.Score) + } +} diff --git a/internal/recommendation/suggestions.go b/internal/recommendation/suggestions.go index 1898d35b..348ebf6d 100644 --- a/internal/recommendation/suggestions.go +++ b/internal/recommendation/suggestions.go @@ -31,6 +31,15 @@ type ArtistSuggestion struct { Name string Score float64 Attribution []SeedContribution + // MatchedTags are the candidate's own tags that overlap the user's taste + // profile, strongest first (max 3) — the "matches: shoegaze, melancholic" + // explanation (#2377). Empty when the candidate has no cached tags, which + // is common and not an error: coverage is permanently partial (#2376). + MatchedTags []string + // TagOverlap is the [0,1] share of the candidate's tag mass the user likes. + // Exposed for the admin tuning lab — seeing the term's actual distribution + // is how the operator picks a weight rather than guessing at one. + TagOverlap float64 } // SeedContribution is one of the top-3 contributing seeds for a candidate. @@ -49,7 +58,10 @@ type SeedContribution struct { // seed path only — the likes + completed-plays fallback used while the user // has no taste-profile rows yet. Once the profile is populated it seeds // instead, carrying its own decay, so this knob stops applying. -func SuggestArtists(ctx context.Context, pool *pgxpool.Pool, userID pgtype.UUID, halfLifeDays float64, limit int) ([]ArtistSuggestion, error) { +func SuggestArtists( + ctx context.Context, pool *pgxpool.Pool, userID pgtype.UUID, + halfLifeDays float64, limit int, tagOverlapWeight float64, +) ([]ArtistSuggestion, error) { if limit <= 0 || limit > 50 { limit = 12 } @@ -117,9 +129,71 @@ func SuggestArtists(ctx context.Context, pool *pgxpool.Pool, userID pgtype.UUID, Attribution: attribution, }) } + + // Taste-tag term (#2377). Applied to the whole over-fetched pool BEFORE + // selection, so the rotation and diversity rules in selectSuggestions + // operate on taste-blended scores — boosting only the twelve already + // chosen by similarity would leave the actual re-ranking undone. + // + // A query error here is returned, NOT degraded past. Graceful degradation + // is for expected absence — no taste profile yet, no cached tags for a + // candidate — and both of those are handled explicitly as empty inputs + // below. A failing query is neither: swallowing it would hide a broken DB + // behind a subtly worse ranking that nothing reports. + tasteTags, candTags, err := loadTagInputs(ctx, q, userID, out) + if err != nil { + return nil, err + } + out = applyTagOverlap(out, candTags, tasteTags, tagOverlapWeight) + return selectSuggestions(out, limit, rotationDay(time.Now())), nil } +// tasteTagLimit caps how many of the user's taste tags participate. The +// profile's long tail is near-zero weight and contributes nothing after +// normalization, so this bounds the query rather than the meaning. +const tasteTagLimit = 50 + +// loadTagInputs fetches both sides of the overlap comparison: the user's taste +// tags and the cached tags for exactly the candidates in this pool. +func loadTagInputs( + ctx context.Context, q *dbq.Queries, userID pgtype.UUID, pool []ArtistSuggestion, +) (TagWeights, map[string]TagWeights, error) { + tasteRows, err := q.ListTasteProfileTagsForUser(ctx, dbq.ListTasteProfileTagsForUserParams{ + UserID: userID, + Limit: tasteTagLimit, + }) + if err != nil { + return nil, nil, fmt.Errorf("suggest: taste tags: %w", err) + } + // No taste tags is a cold start, not a failure — return early and skip the + // candidate-tag fetch entirely, since nothing could match. + if len(tasteRows) == 0 { + return nil, nil, nil + } + taste := make(TagWeights, len(tasteRows)) + for _, r := range tasteRows { + taste[r.Tag] = r.Weight + } + + mbids := make([]string, 0, len(pool)) + for _, s := range pool { + mbids = append(mbids, s.MBID) + } + tagRows, err := q.ListCandidateArtistTagsForMbids(ctx, mbids) + if err != nil { + return nil, nil, fmt.Errorf("suggest: candidate tags: %w", err) + } + byCandidate := make(map[string]TagWeights, len(pool)) + for _, r := range tagRows { + if byCandidate[r.CandidateMbid] == nil { + byCandidate[r.CandidateMbid] = TagWeights{} + } + byCandidate[r.CandidateMbid][r.Tag] = r.Weight + } + return taste, byCandidate, nil +} + // Pool multiplier: how many scored candidates to fetch per slot shown, so the // rotation has somewhere to rotate. 4x keeps a day's deck genuinely different // from yesterday's without pulling the whole long tail (whose scores are noise) diff --git a/internal/recommendation/suggestions_integration_test.go b/internal/recommendation/suggestions_integration_test.go index 1f82da8c..7a8cd40c 100644 --- a/internal/recommendation/suggestions_integration_test.go +++ b/internal/recommendation/suggestions_integration_test.go @@ -147,7 +147,7 @@ func TestSuggestArtists_LikesAndPlaysContributeToScore(t *testing.T) { seedUnmatched(t, pool, seedA.ID, "out-mbid", "Outsider", 0.9) seedUnmatched(t, pool, seedB.ID, "out-mbid", "Outsider", 0.5) - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -174,7 +174,7 @@ func TestSuggestArtists_Top12Cap(t *testing.T) { for i := 0; i < 30; i++ { seedUnmatched(t, pool, seed.ID, fmt.Sprintf("mbid-%02d", i), fmt.Sprintf("Artist %02d", i), 0.99-float64(i)*0.01) } - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -195,7 +195,7 @@ func TestSuggestArtists_AttributionTopThree(t *testing.T) { likeArtist(t, pool, user.ID, seeds[i].ID) seedUnmatched(t, pool, seeds[i].ID, "shared-mbid", "Shared", 0.9-float64(i)*0.1) } - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -227,7 +227,7 @@ func TestSuggestArtists_RecencyDecayDownweightsOldPlays(t *testing.T) { seedUnmatched(t, pool, recentSeed.ID, "cand", "Cand", 0.5) seedUnmatched(t, pool, oldSeed.ID, "cand", "Cand", 0.5) - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -255,7 +255,7 @@ func TestSuggestArtists_FiltersInLibraryCandidates(t *testing.T) { seedArtist(t, pool, "InLib", inLibMBID) seedUnmatched(t, pool, seed.ID, inLibMBID, "InLib", 0.9) - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -279,7 +279,7 @@ func TestSuggestArtists_FiltersAlreadyRequested(t *testing.T) { t.Fatalf("CreateLidarrRequest: %v", err) } - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -310,7 +310,7 @@ func TestSuggestArtists_RejectedRequestStillShown(t *testing.T) { t.Fatalf("RejectLidarrRequest: %v", err) } - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -322,7 +322,7 @@ func TestSuggestArtists_RejectedRequestStillShown(t *testing.T) { func TestSuggestArtists_EmptyForNewUser(t *testing.T) { pool := newPool(t) user := seedUser(t, pool, "newbie") - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -374,7 +374,7 @@ func TestSuggestArtists_TasteProfileWeightSeedsTier1(t *testing.T) { setTasteWeight(t, pool, user.ID, seed.ID, 4.0) seedUnmatched(t, pool, seed.ID, "out-mbid", "Outsider", 0.9) - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -408,7 +408,7 @@ func TestSuggestArtists_TasteProfileSupersedesRawPlays(t *testing.T) { seedUnmatched(t, pool, kept.ID, "kept-cand", "Kept Candidate", 0.9) seedUnmatched(t, pool, dropped.ID, "dropped-cand", "Dropped Candidate", 0.9) - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -435,7 +435,7 @@ func TestSuggestArtists_NonPositiveTasteWeightDoesNotSeed(t *testing.T) { seedUnmatched(t, pool, positive.ID, "pos-cand", "Positive Candidate", 0.9) seedUnmatched(t, pool, abandoned.ID, "neg-cand", "Negative Candidate", 0.9) - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -464,7 +464,7 @@ func TestSuggestArtists_SkippedPlaysDoNotSeedTier2(t *testing.T) { } seedUnmatched(t, pool, skipped.ID, "skip-cand", "Skip Candidate", 0.9) - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -518,7 +518,7 @@ func TestSuggestArtists_ActiveSnoozeHidesCandidate(t *testing.T) { t.Fatalf("SnoozeSuggestion: %v", err) } - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -534,7 +534,7 @@ func TestSuggestArtists_ExpiredSnoozeShowsCandidateAgain(t *testing.T) { seedOneCandidate(t, pool, user.ID, "expired-mbid", "Returning Artist") snoozeUntil(t, pool, user.ID, "expired-mbid", "Returning Artist", time.Now().Add(-time.Hour)) - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } @@ -560,14 +560,14 @@ func TestSuggestArtists_SnoozeIsPerUser(t *testing.T) { snoozeUntil(t, pool, alice.ID, "shared-mbid", "Shared Candidate", time.Now().Add(24*time.Hour)) - aliceOut, err := SuggestArtists(context.Background(), pool, alice.ID, 30, 12) + aliceOut, err := SuggestArtists(context.Background(), pool, alice.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists(alice): %v", err) } if len(aliceOut) != 0 { t.Errorf("alice len = %d, want 0 (she snoozed it)", len(aliceOut)) } - bobOut, err := SuggestArtists(context.Background(), pool, bob.ID, 30, 12) + bobOut, err := SuggestArtists(context.Background(), pool, bob.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists(bob): %v", err) } @@ -604,7 +604,7 @@ func TestUnsnoozeSuggestion_RestoresImmediately(t *testing.T) { t.Errorf("repeat rows = %d, want 0", rows) } - out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12, 0) if err != nil { t.Fatalf("SuggestArtists: %v", err) } diff --git a/internal/recommendation/tagoverlap.go b/internal/recommendation/tagoverlap.go new file mode 100644 index 00000000..8e2d3bbd --- /dev/null +++ b/internal/recommendation/tagoverlap.go @@ -0,0 +1,156 @@ +// tagoverlap.go — the taste-tag term for the Discover request surface +// (#2377, milestone #268 slice 6). +// +// Slices 1-2 made the deck stop repeating; this is what makes it *relevant*. +// Before this, a candidate's only claim on a slot was "some artist you play is +// similar to it" — a graph-adjacency fact that says nothing about whether the +// music sounds like anything you actually like. Here the candidate's own +// folksonomy tags (cached by slice 5) are compared against the user's +// taste-profile tags, so the surface can rank on "matches the sound you like" +// and say WHY. +// +// Pure by design — no DB, no clock — so the scoring rules are unit-testable in +// the fast lane rather than behind the integration gate. +package recommendation + +import "sort" + +// maxMatchedTags caps the "matches: …" explanation. Three is what the existing +// seed attribution shows, and a longer list stops being a reason and becomes a +// tag dump. +const maxMatchedTags = 3 + +// TagWeights is a tag → weight map. Both sides of the comparison use it: +// candidate tags (normalized [0,1] by the enrichment providers) and the user's +// taste-profile tags (accumulated, unbounded — normalized here). +type TagWeights map[string]float64 + +// tagOverlap scores how much of a candidate's tag identity the user actually +// likes, in [0,1], and returns the matched tags ordered by contribution. +// +// The measure is: of this candidate's total tag mass, what share sits on tags +// the user likes — each weighted by how strongly they like it? +// +// overlap = Σ(shared) candWeight × normalizedTasteWeight ÷ Σ(all) candWeight +// +// Normalizing the taste side by the user's STRONGEST tag is what makes this +// comparable across users: taste weights accumulate with listening, so a +// heavy listener's raw numbers dwarf a new user's while meaning the same +// thing — "this is my favourite tag". Dividing by the candidate's own total +// mass makes it comparable across candidates, so a densely-tagged artist +// can't out-score a sparsely-tagged one just by having more tags. +// +// Returns (0, nil) when either side is empty. That is the load-bearing +// degradation path: tag coverage for out-of-library candidates is permanently +// partial (#2376), and a cold-start user has no taste tags at all. Both must +// leave the candidate's similarity score untouched rather than sink it — +// rule #131, tiered degradation, never vanish-or-nothing. +func tagOverlap(candidate, taste TagWeights) (float64, []string) { + if len(candidate) == 0 || len(taste) == 0 { + return 0, nil + } + + maxTaste := 0.0 + for _, w := range taste { + if w > maxTaste { + maxTaste = w + } + } + // Every taste weight <= 0 carries no preference to match against. Guarding + // here also avoids dividing by zero below. + if maxTaste <= 0 { + return 0, nil + } + + totalMass := 0.0 + for _, w := range candidate { + // Negative or zero candidate weights would let a tag subtract from the + // denominator and inflate the ratio past 1. + if w > 0 { + totalMass += w + } + } + if totalMass <= 0 { + return 0, nil + } + + type contribution struct { + tag string + score float64 + } + var matched []contribution + sum := 0.0 + for tag, candWeight := range candidate { + if candWeight <= 0 { + continue + } + tasteWeight, ok := taste[tag] + if !ok || tasteWeight <= 0 { + continue + } + c := candWeight * (tasteWeight / maxTaste) + sum += c + matched = append(matched, contribution{tag: tag, score: c}) + } + if len(matched) == 0 { + return 0, nil + } + + // Strongest contribution first; tag name breaks ties so the explanation is + // deterministic for a given input rather than map-iteration order. + sort.Slice(matched, func(i, j int) bool { + if matched[i].score != matched[j].score { + return matched[i].score > matched[j].score + } + return matched[i].tag < matched[j].tag + }) + names := make([]string, 0, min(len(matched), maxMatchedTags)) + for i := 0; i < len(matched) && i < maxMatchedTags; i++ { + names = append(names, matched[i].tag) + } + return sum / totalMass, names +} + +// applyTagOverlap re-scores and re-orders a candidate pool by taste-tag +// overlap, stamping the matched tags onto each suggestion for the UI. +// +// The blend is MULTIPLICATIVE — score × (1 + weight × overlap) — not additive, +// and the difference is the whole safety argument: +// +// - A candidate with no tags has overlap 0, so its score is EXACTLY +// unchanged. Partial tag coverage costs a candidate nothing. +// - Nothing can leapfrog on tags alone. An additive term with a large +// weight would let a near-zero-similarity artist outrank a strong match +// just for sharing a popular tag, which reads as noise to the user. +// - weight 0 disables the feature completely and restores pure similarity +// order, so the operator's knob has a real off position. +// +// Callers must pass the pool in similarity order; it is returned in blended +// order. Mutates the elements in place (they're the caller's own slice built +// per request), and re-sorts, because selectSuggestions downstream relies on +// score order for its head/tail split. +func applyTagOverlap( + pool []ArtistSuggestion, candidateTags map[string]TagWeights, taste TagWeights, weight float64, +) []ArtistSuggestion { + // A zero weight is the operator turning the feature off. Skip the work + // AND the re-sort so the ordering is bit-for-bit the pre-slice-6 result. + if weight == 0 || len(taste) == 0 { + return pool + } + for i := range pool { + overlap, matched := tagOverlap(candidateTags[pool[i].MBID], taste) + pool[i].MatchedTags = matched + pool[i].TagOverlap = overlap + pool[i].Score *= 1 + weight*overlap + } + sort.SliceStable(pool, func(i, j int) bool { + if pool[i].Score != pool[j].Score { + return pool[i].Score > pool[j].Score + } + // Stable tiebreak by MBID. Without it, two candidates on equal scores + // could swap between requests within the same day, which the daily + // rotation (#2373) exists to prevent. + return pool[i].MBID < pool[j].MBID + }) + return pool +} diff --git a/internal/recommendation/tagoverlap_test.go b/internal/recommendation/tagoverlap_test.go new file mode 100644 index 00000000..e66269e4 --- /dev/null +++ b/internal/recommendation/tagoverlap_test.go @@ -0,0 +1,244 @@ +package recommendation + +import ( + "fmt" + "testing" +) + +// tagOverlap / applyTagOverlap are pure, so slice 6's ranking rules are covered +// in the fast lane rather than behind the integration gate. +// +// Fixtures here are deliberately SKEWED — candidates that match the taste +// profile to clearly different degrees. An evenly-matching pool cannot exercise +// a re-ranking at all: every candidate gets the same multiplier and the order is +// unchanged whether the blend works or not. Slice 2's first diversity test had +// exactly that defect, so it's called out explicitly here. + +func TestTagOverlap_FullMatchScoresOne(t *testing.T) { + // Every unit of the candidate's tag mass sits on the user's single + // strongest tag → the whole mass matches at full strength. + got, matched := tagOverlap( + TagWeights{"shoegaze": 1.0}, + TagWeights{"shoegaze": 5.0}, + ) + if got != 1.0 { + t.Errorf("overlap = %v, want 1.0", got) + } + if len(matched) != 1 || matched[0] != "shoegaze" { + t.Errorf("matched = %v, want [shoegaze]", matched) + } +} + +func TestTagOverlap_NoSharedTagsScoresZero(t *testing.T) { + got, matched := tagOverlap( + TagWeights{"death metal": 1.0}, + TagWeights{"shoegaze": 5.0}, + ) + if got != 0 { + t.Errorf("overlap = %v, want 0", got) + } + if matched != nil { + t.Errorf("matched = %v, want nil", matched) + } +} + +// Half the candidate's mass is on a matching tag, and that tag is the user's +// strongest → 0.5. +func TestTagOverlap_PartialMassMatchIsProportional(t *testing.T) { + got, _ := tagOverlap( + TagWeights{"shoegaze": 1.0, "death metal": 1.0}, + TagWeights{"shoegaze": 5.0}, + ) + if got != 0.5 { + t.Errorf("overlap = %v, want 0.5", got) + } +} + +// Matching a tag the user barely likes must score below matching one they love. +func TestTagOverlap_WeakTasteTagScoresLowerThanStrong(t *testing.T) { + taste := TagWeights{"shoegaze": 10.0, "polka": 1.0} + strong, _ := tagOverlap(TagWeights{"shoegaze": 1.0}, taste) + weak, _ := tagOverlap(TagWeights{"polka": 1.0}, taste) + if !(strong > weak) { + t.Errorf("strong=%v weak=%v — a favourite tag must outscore a marginal one", strong, weak) + } + if weak != 0.1 { // 1.0 * (1/10) / 1.0 + t.Errorf("weak = %v, want 0.1", weak) + } +} + +// THE degradation path. Tag coverage for out-of-library candidates is +// permanently partial (#2376) and cold-start users have no taste tags, so both +// must be scored 0 — never negative, never dropped (rule #131). +func TestTagOverlap_EmptyEitherSideIsZeroNotNegative(t *testing.T) { + cases := []struct { + name string + candidate, taste TagWeights + }{ + {"no candidate tags", nil, TagWeights{"shoegaze": 5}}, + {"no taste tags", TagWeights{"shoegaze": 1}, nil}, + {"both empty", nil, nil}, + {"taste weights all zero", TagWeights{"shoegaze": 1}, TagWeights{"shoegaze": 0}}, + {"candidate weights all zero", TagWeights{"shoegaze": 0}, TagWeights{"shoegaze": 5}}, + } + for _, c := range cases { + got, matched := tagOverlap(c.candidate, c.taste) + if got != 0 { + t.Errorf("%s: overlap = %v, want 0", c.name, got) + } + if matched != nil { + t.Errorf("%s: matched = %v, want nil", c.name, matched) + } + } +} + +// A densely-tagged artist must not out-score a focused one merely by having +// more tags — that's what dividing by the candidate's own mass buys. +func TestTagOverlap_IsNotAPopularityContest(t *testing.T) { + taste := TagWeights{"shoegaze": 5.0} + focused, _ := tagOverlap(TagWeights{"shoegaze": 1.0}, taste) + sprawling, _ := tagOverlap(TagWeights{ + "shoegaze": 1.0, "rock": 1.0, "alternative": 1.0, "90s": 1.0, + }, taste) + if !(focused > sprawling) { + t.Errorf("focused=%v sprawling=%v — extra unmatched tags must dilute, not add", + focused, sprawling) + } +} + +// Taste weights accumulate with listening, so raw magnitudes differ wildly +// between a new user and a heavy one while meaning the same thing. Normalizing +// by the user's own strongest tag is what makes the score comparable. +func TestTagOverlap_IsInvariantToTasteMagnitude(t *testing.T) { + candidate := TagWeights{"shoegaze": 1.0, "dream pop": 1.0} + newUser, _ := tagOverlap(candidate, TagWeights{"shoegaze": 2.0, "polka": 1.0}) + heavyUser, _ := tagOverlap(candidate, TagWeights{"shoegaze": 2000.0, "polka": 1000.0}) + if newUser != heavyUser { + t.Errorf("newUser=%v heavyUser=%v — scaling all taste weights must not change the result", + newUser, heavyUser) + } +} + +func TestTagOverlap_MatchedTagsAreOrderedAndCapped(t *testing.T) { + got, matched := tagOverlap( + TagWeights{"a": 0.2, "b": 0.9, "c": 0.5, "d": 0.7}, + TagWeights{"a": 10, "b": 10, "c": 10, "d": 10}, + ) + if got <= 0 { + t.Fatalf("overlap = %v, want > 0", got) + } + // All taste weights equal, so candidate weight decides: b(.9) d(.7) c(.5). + want := []string{"b", "d", "c"} + if fmt.Sprint(matched) != fmt.Sprint(want) { + t.Errorf("matched = %v, want %v (strongest first, capped at %d)", + matched, want, maxMatchedTags) + } +} + +// --- applyTagOverlap --- + +func poolFor(specs ...struct { + mbid string + score float64 +}) []ArtistSuggestion { + out := make([]ArtistSuggestion, 0, len(specs)) + for _, s := range specs { + out = append(out, ArtistSuggestion{MBID: s.mbid, Name: s.mbid, Score: s.score}) + } + return out +} + +type spec = struct { + mbid string + score float64 +} + +// The payoff, and the reason the fixture is skewed: a taste-matching candidate +// that started BELOW another must be able to overtake it. With an evenly +// matching pool this assertion could not fail. +func TestApplyTagOverlap_TasteMatchOvertakesAStrongerNonMatch(t *testing.T) { + pool := poolFor(spec{"loud", 1.0}, spec{"match", 0.7}) + candTags := map[string]TagWeights{ + "loud": {"death metal": 1.0}, + "match": {"shoegaze": 1.0}, + } + taste := TagWeights{"shoegaze": 5.0} + + got := applyTagOverlap(pool, candTags, taste, 1.0) + if got[0].MBID != "match" { + t.Errorf("first = %q, want match (0.7×2 = 1.4 beats 1.0×1)", got[0].MBID) + } + if got[0].MatchedTags == nil { + t.Error("matched tags not stamped onto the winner") + } +} + +// An untagged candidate keeps its score EXACTLY. This is the multiplicative +// blend's whole safety argument: partial coverage costs a candidate nothing. +func TestApplyTagOverlap_UntaggedCandidateScoreIsUnchanged(t *testing.T) { + pool := poolFor(spec{"untagged", 0.9}) + got := applyTagOverlap(pool, map[string]TagWeights{}, TagWeights{"shoegaze": 5}, 1.0) + if got[0].Score != 0.9 { + t.Errorf("score = %v, want 0.9 exactly (no tags must not penalise)", got[0].Score) + } + if got[0].MatchedTags != nil { + t.Errorf("matched = %v, want nil", got[0].MatchedTags) + } +} + +// Weight 0 is the operator's off switch: the ordering must be bit-for-bit the +// pre-slice-6 result, not merely similar. +func TestApplyTagOverlap_ZeroWeightIsAnExactNoOp(t *testing.T) { + pool := poolFor(spec{"a", 1.0}, spec{"b", 0.7}) + candTags := map[string]TagWeights{"b": {"shoegaze": 1.0}} + got := applyTagOverlap(pool, candTags, TagWeights{"shoegaze": 5.0}, 0) + if got[0].MBID != "a" || got[0].Score != 1.0 || got[1].Score != 0.7 { + t.Errorf("weight 0 changed the pool: %+v", got) + } + // And it must not stamp matched tags either — the UI would otherwise + // explain a boost that never happened. + if got[1].MatchedTags != nil { + t.Errorf("matched tags stamped while the feature is off: %v", got[1].MatchedTags) + } +} + +// A user with no taste profile is the cold-start case: every candidate scores +// the same multiplier of 1, so similarity order must survive intact. +func TestApplyTagOverlap_NoTasteProfileLeavesOrderIntact(t *testing.T) { + pool := poolFor(spec{"a", 1.0}, spec{"b", 0.7}, spec{"c", 0.4}) + candTags := map[string]TagWeights{"c": {"shoegaze": 1.0}} + got := applyTagOverlap(pool, candTags, nil, 1.0) + for i, want := range []string{"a", "b", "c"} { + if got[i].MBID != want { + t.Fatalf("order = %v..., want a b c", got[i].MBID) + } + } +} + +// Bounded boost: even a perfect match at the maximum weight can only multiply +// by (1 + w), so a candidate cannot be catapulted arbitrarily far. +func TestApplyTagOverlap_BoostIsBoundedByOnePlusWeight(t *testing.T) { + pool := poolFor(spec{"perfect", 1.0}) + got := applyTagOverlap(pool, + map[string]TagWeights{"perfect": {"shoegaze": 1.0}}, + TagWeights{"shoegaze": 5.0}, 2.0) + if got[0].Score != 3.0 { + t.Errorf("score = %v, want 3.0 (1.0 × (1 + 2×1))", got[0].Score) + } + if got[0].TagOverlap != 1.0 { + t.Errorf("TagOverlap = %v, want 1.0", got[0].TagOverlap) + } +} + +// Equal blended scores must resolve deterministically, or two candidates could +// swap between requests inside one day — the exact churn the daily rotation +// (#2373) exists to prevent. +func TestApplyTagOverlap_TiesBreakDeterministically(t *testing.T) { + first := applyTagOverlap(poolFor(spec{"zzz", 1.0}, spec{"aaa", 1.0}), + map[string]TagWeights{}, TagWeights{"x": 1}, 1.0) + second := applyTagOverlap(poolFor(spec{"aaa", 1.0}, spec{"zzz", 1.0}), + map[string]TagWeights{}, TagWeights{"x": 1}, 1.0) + if first[0].MBID != "aaa" || second[0].MBID != "aaa" { + t.Errorf("tie order not deterministic: %q then %q", first[0].MBID, second[0].MBID) + } +} diff --git a/internal/recsettings/patch.go b/internal/recsettings/patch.go index 6ae86833..f7139cdf 100644 --- a/internal/recsettings/patch.go +++ b/internal/recsettings/patch.go @@ -161,6 +161,70 @@ func applyTastePatch(current TasteTuning, patch map[string]float64) (TasteTuning return next, changes, nil } +// Discover tuning bounds. +const ( + // A tag-overlap weight above this stops being a boost and becomes the + // ranking — at 10, a perfect match multiplies similarity by 11, which lets + // tag agreement swamp the similarity signal entirely. The bound is for + // typos, not to constrain exploration; the multiplicative blend keeps even + // the maximum from reordering an untagged candidate. + 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. + snoozeDaysMin = 1.0 + snoozeDaysMax = 365.0 +) + +// applyDiscoverPatch validates and applies a partial Discover update. +func applyDiscoverPatch( + current DiscoverTuning, patch map[string]float64, +) (DiscoverTuning, []fieldChange, error) { + next := current + var changes []fieldChange + for field, v := range patch { + var target *float64 + switch field { + case "tag_overlap_weight": + if v < 0 || v > tagOverlapWeightMax { + return current, nil, fmt.Errorf("%w: %s = %v (must be in [0, %v])", + ErrOutOfRange, field, v, tagOverlapWeightMax) + } + target = &next.TagOverlapWeight + case "snooze_days": + if v < snoozeDaysMin || v > snoozeDaysMax { + return current, nil, fmt.Errorf("%w: %s = %v (must be in [%v, %v])", + ErrOutOfRange, field, v, snoozeDaysMin, snoozeDaysMax) + } + target = &next.SnoozeDays + default: + return current, nil, fmt.Errorf("%w: %q", ErrUnknownField, field) + } + if *target == v { + continue + } + changes = append(changes, fieldChange{Field: field, Old: *target, New: v}) + *target = v + } + return next, changes, nil +} + +// diffDiscover returns per-field changes from a to b (empty when equal). +func diffDiscover(a, b DiscoverTuning) []fieldChange { + var out []fieldChange + if a.TagOverlapWeight != b.TagOverlapWeight { + out = append(out, fieldChange{ + Field: "tag_overlap_weight", Old: a.TagOverlapWeight, New: b.TagOverlapWeight, + }) + } + if a.SnoozeDays != b.SnoozeDays { + out = append(out, fieldChange{ + Field: "snooze_days", Old: a.SnoozeDays, New: b.SnoozeDays, + }) + } + return out +} + // diffWeights returns per-field changes from a to b (empty when equal). func diffWeights(a, b recommendation.ScoringWeights) []fieldChange { var out []fieldChange diff --git a/internal/recsettings/service.go b/internal/recsettings/service.go index 22640847..29b41d14 100644 --- a/internal/recsettings/service.go +++ b/internal/recsettings/service.go @@ -35,6 +35,11 @@ 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 = "discover" ) // TasteTuning is the tunable subset of taste.Config: the engagement @@ -87,6 +92,33 @@ func ShippedDailyMixWeights() recommendation.ScoringWeights { } } +// DiscoverTuning is the tunable set for the Discover request surface (#2377). +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 float64 +} + +// ShippedDiscoverTuning are the shipped Discover defaults. +// +// 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. +// +// 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. +func ShippedDiscoverTuning() DiscoverTuning { + return DiscoverTuning{ + TagOverlapWeight: 1.0, + SnoozeDays: 90, + } +} + // ShippedTasteTuning mirrors taste.DefaultConfig's tunable subset. func ShippedTasteTuning() TasteTuning { d := taste.DefaultConfig() @@ -110,6 +142,7 @@ type Service struct { mu sync.RWMutex profiles map[string]recommendation.ScoringWeights taste TasteTuning + discover DiscoverTuning } // New boots the service: seeds shipped defaults for missing rows, @@ -151,6 +184,13 @@ func (s *Service) reconcile(ctx context.Context) error { }); err != nil { return fmt.Errorf("seed taste tuning: %w", err) } + sd := ShippedDiscoverTuning() + if err := q.UpsertDiscoverTuningDefaults(ctx, dbq.UpsertDiscoverTuningDefaultsParams{ + TagOverlapWeight: sd.TagOverlapWeight, + SnoozeDays: sd.SnoozeDays, + }); err != nil { + return fmt.Errorf("seed discover tuning: %w", err) + } rows, err := q.ListWeightProfiles(ctx) if err != nil { @@ -160,6 +200,10 @@ func (s *Service) reconcile(ctx context.Context) error { if err != nil { return fmt.Errorf("get taste tuning: %w", err) } + dt, err := q.GetDiscoverTuning(ctx) + if err != nil { + return fmt.Errorf("get discover tuning: %w", err) + } s.mu.Lock() s.profiles = map[string]recommendation.ScoringWeights{} @@ -175,6 +219,10 @@ func (s *Service) reconcile(ctx context.Context) error { EraScale: tt.EraScale, MoodScale: tt.MoodScale, } + s.discover = DiscoverTuning{ + TagOverlapWeight: dt.TagOverlapWeight, + SnoozeDays: dt.SnoozeDays, + } s.mu.Unlock() s.push() @@ -208,6 +256,15 @@ func (s *Service) Taste() TasteTuning { return s.taste } +// 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). +func (s *Service) Discover() DiscoverTuning { + s.mu.RLock() + defer s.mu.RUnlock() + return s.discover +} + // TasteConfig assembles the full taste.Config the profile builder // consumes: shipped non-tunable knobs (like bonuses, floors, caps) // plus the tuned half-life and curve. WindowDays scales with the @@ -267,6 +324,19 @@ func (s *Service) UpdateTaste(ctx context.Context, patch map[string]float64) err return s.persistTaste(ctx, next, "update", changes) } +// UpdateDiscover applies a partial update to the Discover tuning singleton. +func (s *Service) UpdateDiscover(ctx context.Context, patch map[string]float64) error { + current := s.Discover() + next, changes, err := applyDiscoverPatch(current, patch) + if err != nil { + return err + } + if len(changes) == 0 { + return nil + } + return s.persistDiscover(ctx, next, "update", changes) +} + // Reset restores a scope to its shipped defaults, with one audit row // carrying the full diff. A scope already at defaults is a no-op. func (s *Service) Reset(ctx context.Context, scope string) error { @@ -288,6 +358,13 @@ func (s *Service) Reset(ctx context.Context, scope string) error { return nil } return s.persistTaste(ctx, shipped, "reset", changes) + case ScopeDiscover: + shipped := ShippedDiscoverTuning() + changes := diffDiscover(s.Discover(), shipped) + if len(changes) == 0 { + return nil + } + return s.persistDiscover(ctx, shipped, "reset", changes) default: return fmt.Errorf("%w: %q", ErrUnknownScope, scope) } @@ -338,6 +415,28 @@ func (s *Service) persistTaste( return nil } +// persistDiscover writes the discover row + audit entry and refreshes the +// cache. No push(): unlike taste and daily_mix, nothing precomputes from these +// — the suggestions handler reads Discover() per request. +func (s *Service) persistDiscover( + ctx context.Context, d DiscoverTuning, action string, changes []fieldChange, +) error { + q := dbq.New(s.pool) + if _, err := q.UpdateDiscoverTuning(ctx, dbq.UpdateDiscoverTuningParams{ + TagOverlapWeight: d.TagOverlapWeight, + SnoozeDays: d.SnoozeDays, + }); err != nil { + return fmt.Errorf("update discover tuning: %w", err) + } + if err := s.audit(ctx, q, ScopeDiscover, action, changes); err != nil { + return err + } + s.mu.Lock() + s.discover = d + s.mu.Unlock() + return nil +} + // audit writes one recommendation_tuning_audit row. Changes are // sorted by field so rows are deterministic and diff-friendly. func (s *Service) audit( diff --git a/internal/recsettings/service_test.go b/internal/recsettings/service_test.go index cf96f8a3..083a38bd 100644 --- a/internal/recsettings/service_test.go +++ b/internal/recsettings/service_test.go @@ -281,3 +281,118 @@ func TestUpdate_NoOpWritesNoAudit(t *testing.T) { t.Errorf("no-op update wrote %d audit rows, want 0", len(rows)) } } + +// --- Discover scope (#2377, milestone #268 slice 6) --- + +func TestNew_SeedsDiscoverDefaults(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + got := s.Discover() + want := ShippedDiscoverTuning() + if got != want { + t.Errorf("Discover() = %+v, want shipped %+v", got, want) + } +} + +func TestUpdateDiscover_PersistsAndAudits(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + if err := s.UpdateDiscover(context.Background(), map[string]float64{ + "tag_overlap_weight": 2.5, + "snooze_days": 30, + }); err != nil { + t.Fatalf("UpdateDiscover: %v", err) + } + if got := s.Discover().TagOverlapWeight; got != 2.5 { + t.Errorf("TagOverlapWeight = %v, want 2.5", got) + } + if got := s.Discover().SnoozeDays; got != 30 { + 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. + rows := auditRows(t, pool) + if len(rows) != 1 { + t.Fatalf("audit rows = %d, want 1", len(rows)) + } + if rows[0].Scope != ScopeDiscover { + t.Errorf("audit scope = %q, want %q", rows[0].Scope, ScopeDiscover) + } + if len(rows[0].Changes) != 2 { + t.Errorf("audit changes = %+v, want both fields", rows[0].Changes) + } + + // Reload from the DB to prove it persisted rather than only caching. + s2 := newService(t, pool) + if got := s2.Discover().TagOverlapWeight; got != 2.5 { + t.Errorf("after reload TagOverlapWeight = %v, want 2.5 (not re-seeded to shipped)", got) + } +} + +func TestUpdateDiscover_Validation(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + cases := []struct { + name string + patch map[string]float64 + }{ + {"unknown field", map[string]float64{"nope": 1}}, + {"negative weight", map[string]float64{"tag_overlap_weight": -1}}, + {"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. + {"snooze past a year", map[string]float64{"snooze_days": 400}}, + } + for _, c := range cases { + if err := s.UpdateDiscover(context.Background(), c.patch); err == nil { + t.Errorf("%s: expected rejection, got nil", c.name) + } + } + if got := s.Discover(); got != ShippedDiscoverTuning() { + t.Errorf("a rejected patch mutated state: %+v", got) + } + if rows := auditRows(t, pool); len(rows) != 0 { + t.Errorf("rejected patches wrote %d audit rows, want 0", len(rows)) + } +} + +// 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. +func TestUpdateDiscover_ZeroWeightIsAllowed(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + if err := s.UpdateDiscover(context.Background(), + map[string]float64{"tag_overlap_weight": 0}); err != nil { + t.Fatalf("UpdateDiscover(0): %v", err) + } + if got := s.Discover().TagOverlapWeight; got != 0 { + t.Errorf("TagOverlapWeight = %v, want 0", got) + } +} + +func TestResetDiscover_RestoresShippedDefaults(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + if err := s.UpdateDiscover(context.Background(), + map[string]float64{"tag_overlap_weight": 4}); err != nil { + t.Fatalf("UpdateDiscover: %v", err) + } + if err := s.Reset(context.Background(), ScopeDiscover); err != nil { + t.Fatalf("Reset: %v", err) + } + if got := s.Discover(); got != ShippedDiscoverTuning() { + t.Errorf("after reset = %+v, want shipped %+v", got, ShippedDiscoverTuning()) + } + // Already-at-defaults is a no-op: update + reset = 2 rows, not 3. + if err := s.Reset(context.Background(), ScopeDiscover); err != nil { + t.Fatalf("second Reset: %v", err) + } + if rows := auditRows(t, pool); len(rows) != 2 { + t.Errorf("audit rows = %d, want 2 (the no-op reset must not audit)", len(rows)) + } +}