diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 4e887257..b9e9d00c 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -25,6 +25,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests" "git.fabledsword.com/bvandeusen/minstrel/internal/logging" "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" + "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" "git.fabledsword.com/bvandeusen/minstrel/internal/scrobble" "git.fabledsword.com/bvandeusen/minstrel/internal/scrobble/listenbrainz" "git.fabledsword.com/bvandeusen/minstrel/internal/server" @@ -146,6 +147,16 @@ func run() error { coverEnricher := coverart.NewEnricher(pool, logger.With("component", "coverart"), coverSettings) coverEnricher.DataDir = cfg.Storage.DataDir + // Recommendation tuning lab (#1250): seeds shipped defaults on first + // boot and pushes the daily_mix weights + taste config into package + // playlists — must precede the scheduler so the first builds score + // with the operator's tuned values, not the pre-push literals. + recSettings, err := recsettings.New(ctx, pool, logger.With("component", "recsettings")) + if err != nil { + logger.Error("recommendation settings service init failed", "err", err) + os.Exit(1) + } + // One unified scan chain: library walk → MBID backfill → cover enrich. // Boot-time scan and manual-trigger scans share this path; results land // in scan_runs for the admin overview. @@ -294,6 +305,7 @@ func run() error { }, cfg.Events, cfg.Recommendation, cfg.Storage.DataDir, cfg.Branding, coverEnricher, coverSettings, scanner, scanCfg) srv.Bus = bus srv.PlaylistScheduler = playlistScheduler + srv.RecSettings = recSettings srv.StreamSecret = cfg.StreamSecret httpServer := &http.Server{ Addr: cfg.Server.Address, diff --git a/internal/api/admin_recommendation_trends.go b/internal/api/admin_recommendation_trends.go new file mode 100644 index 00000000..d9e8922c --- /dev/null +++ b/internal/api/admin_recommendation_trends.go @@ -0,0 +1,202 @@ +// Admin recommendation-trends endpoint (#1251): weekly per-surface +// outcome series with tuning-audit markers — the verify half of the +// tune→verify loop the tuning lab (#1250) opens. Aggregated across +// all users because the knobs are global; rows carry rates only. +package api + +import ( + "encoding/json" + "net/http" + "sort" + "strconv" + "time" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +const ( + trendsDefaultWeeks = 12 + trendsMaxWeeks = 52 + // trendsAuditFetchCap bounds the audit fetch; markers older than + // the window are dropped in Go. Far above any real knob-turn count + // inside a year. + trendsAuditFetchCap = 500 +) + +// trendPoint is one week of one surface family's outcomes. +type trendPoint struct { + WeekStart string `json:"week_start"` // ISO date (Monday) + Plays int64 `json:"plays"` + Skips int64 `json:"skips"` + SkipRate float64 `json:"skip_rate"` + AvgCompletion float64 `json:"avg_completion"` + // TasteHitRate is the share of plays whose track's artist carries a + // positive weight in the player's current taste profile — a drifted + // but retroactive read on whether the surface feeds taste-fitting + // tracks. + TasteHitRate float64 `json:"taste_hit_rate"` + // completionN carries the completion sample count through same-week + // merges so avg_completion stays sample-weighted; not serialized. + completionN int64 +} + +// trendSeries is one surface family's weekly series. +type trendSeries struct { + Key string `json:"key"` + Label string `json:"label"` + Intent string `json:"intent"` // go_to | discovery | direct; "" for the manual baseline + Plays int64 `json:"plays"` // window total, for sorting/volume-gating + Points []trendPoint `json:"points"` +} + +// trendMarker is one tuning-audit event annotated on the timeline. +type trendMarker struct { + ChangedAt string `json:"changed_at"` + Scope string `json:"scope"` + Action string `json:"action"` + Changes json.RawMessage `json:"changes"` +} + +type trendsResp struct { + Weeks int `json:"weeks"` + Series []trendSeries `json:"series"` + Markers []trendMarker `json:"markers"` +} + +// handleGetRecommendationTrends implements +// GET /api/admin/recommendation-trends?weeks=N (default 12, cap 52). +func (h *handlers) handleGetRecommendationTrends(w http.ResponseWriter, r *http.Request) { + weeks := trendsDefaultWeeks + if v := r.URL.Query().Get("weeks"); v != "" { + n, err := strconv.Atoi(v) + if err != nil || n < 1 { + writeErr(w, apierror.BadRequest("bad_request", "invalid weeks")) + return + } + if n > trendsMaxWeeks { + n = trendsMaxWeeks + } + weeks = n + } + + q := dbq.New(h.pool) + rows, err := q.RecommendationWeeklyTrends(r.Context(), int32(weeks)) + if err != nil { + h.logger.Error("api: recommendation trends", "err", err) + writeErr(w, apierror.InternalMsg("lookup failed", err)) + return + } + audits, err := q.ListTuningAudit(r.Context(), trendsAuditFetchCap) + if err != nil { + h.logger.Error("api: recommendation trends audit", "err", err) + writeErr(w, apierror.InternalMsg("lookup failed", err)) + return + } + + writeJSON(w, http.StatusOK, buildTrendsResponse(weeks, time.Now().UTC(), rows, audits)) +} + +// buildTrendsResponse folds weekly rows into per-family series and +// windows the audit markers. Split from the handler for pure-unit +// testability. +func buildTrendsResponse( + weeks int, now time.Time, + rows []dbq.RecommendationWeeklyTrendsRow, + audits []dbq.RecommendationTuningAudit, +) trendsResp { + type accum struct { + fam recFamily + plays int64 + points []trendPoint + } + families := map[string]*accum{} + for _, row := range rows { + fam := recFamily{key: "manual", label: "Manual library plays"} + if row.Source != nil && *row.Source != "" { + fam = bucketRecSource(*row.Source) + } + acc, ok := families[fam.key] + if !ok { + acc = &accum{fam: fam} + families[fam.key] = acc + } + acc.plays += row.Plays + p := trendPoint{ + WeekStart: row.WeekStart.Time.Format("2006-01-02"), + Plays: row.Plays, + Skips: row.Skips, + AvgCompletion: row.AvgCompletion, + completionN: row.CompletionN, + } + if row.Plays > 0 { + p.SkipRate = float64(row.Skips) / float64(row.Plays) + p.TasteHitRate = float64(row.TasteHits) / float64(row.Plays) + } + // Same family can arrive as several raw sources (radio:); + // merge same-week points play-weighted. + if n := len(acc.points); n > 0 && acc.points[n-1].WeekStart == p.WeekStart { + acc.points[n-1] = mergeTrendPoints(acc.points[n-1], p) + } else { + acc.points = append(acc.points, p) + } + } + + resp := trendsResp{Weeks: weeks, Series: []trendSeries{}, Markers: []trendMarker{}} + for _, acc := range families { + resp.Series = append(resp.Series, trendSeries{ + Key: acc.fam.key, + Label: acc.fam.label, + Intent: acc.fam.intent, + Plays: acc.plays, + Points: acc.points, + }) + } + sort.Slice(resp.Series, func(i, j int) bool { + if resp.Series[i].Plays != resp.Series[j].Plays { + return resp.Series[i].Plays > resp.Series[j].Plays + } + return resp.Series[i].Key < resp.Series[j].Key + }) + + cutoff := now.Add(-time.Duration(weeks) * 7 * 24 * time.Hour) + for _, a := range audits { + if a.ChangedAt.Time.Before(cutoff) { + continue + } + resp.Markers = append(resp.Markers, trendMarker{ + ChangedAt: a.ChangedAt.Time.UTC().Format(time.RFC3339), + Scope: a.Scope, + Action: a.Action, + Changes: json.RawMessage(a.Changes), + }) + } + // ListTuningAudit returns newest-first; the timeline reads better + // oldest-first. + sort.Slice(resp.Markers, func(i, j int) bool { + return resp.Markers[i].ChangedAt < resp.Markers[j].ChangedAt + }) + return resp +} + +// mergeTrendPoints combines two same-week points of one family: +// counts add, skip/taste rates re-derive from the merged counts, and +// avg_completion is weighted by each side's completion sample count. +func mergeTrendPoints(a, b trendPoint) trendPoint { + out := trendPoint{ + WeekStart: a.WeekStart, + Plays: a.Plays + b.Plays, + Skips: a.Skips + b.Skips, + completionN: a.completionN + b.completionN, + } + if out.Plays > 0 { + out.SkipRate = float64(out.Skips) / float64(out.Plays) + out.TasteHitRate = (a.TasteHitRate*float64(a.Plays) + b.TasteHitRate*float64(b.Plays)) / + float64(out.Plays) + } + if out.completionN > 0 { + out.AvgCompletion = (a.AvgCompletion*float64(a.completionN) + b.AvgCompletion*float64(b.completionN)) / + float64(out.completionN) + } + return out +} diff --git a/internal/api/admin_recommendation_trends_test.go b/internal/api/admin_recommendation_trends_test.go new file mode 100644 index 00000000..278f9cdb --- /dev/null +++ b/internal/api/admin_recommendation_trends_test.go @@ -0,0 +1,152 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func date(s string) pgtype.Date { + t, _ := time.Parse("2006-01-02", s) + return pgtype.Date{Time: t, Valid: true} +} + +func TestBuildTrendsResponse_SeriesMergingAndMarkers(t *testing.T) { + src := func(s string) *string { return &s } + now := time.Date(2026, 7, 3, 12, 0, 0, 0, time.UTC) + rows := []dbq.RecommendationWeeklyTrendsRow{ + // Two radio session sources in the same week collapse into one + // family point; completion weighting by sample count. + {WeekStart: date("2026-06-22"), Source: src("radio:aaaa"), + Plays: 3, Skips: 1, CompletionN: 3, AvgCompletion: 0.6, TasteHits: 3}, + {WeekStart: date("2026-06-22"), Source: src("radio:bbbb"), + Plays: 1, Skips: 1, CompletionN: 1, AvgCompletion: 0.2, TasteHits: 0}, + {WeekStart: date("2026-06-29"), Source: src("radio:aaaa"), + Plays: 2, Skips: 0, CompletionN: 2, AvgCompletion: 0.9, TasteHits: 1}, + // NULL source = manual baseline family. + {WeekStart: date("2026-06-29"), Source: nil, + Plays: 5, Skips: 1, CompletionN: 5, AvgCompletion: 0.8, TasteHits: 4}, + } + audits := []dbq.RecommendationTuningAudit{ + {ID: 2, ChangedAt: pgtype.Timestamptz{Time: now.Add(-24 * time.Hour), Valid: true}, + Scope: "radio", Action: "update", Changes: []byte(`[{"field":"taste_weight","old":1,"new":2}]`)}, + // Older than the window → dropped. + {ID: 1, ChangedAt: pgtype.Timestamptz{Time: now.Add(-100 * 7 * 24 * time.Hour), Valid: true}, + Scope: "taste", Action: "reset", Changes: []byte(`[]`)}, + } + + resp := buildTrendsResponse(12, now, rows, audits) + + if len(resp.Series) != 2 { + t.Fatalf("series = %d, want 2 (radio + manual)", len(resp.Series)) + } + radio := resp.Series[0] // 6 plays > manual's 5 → sorted first + if radio.Key != "radio" || radio.Plays != 6 { + t.Fatalf("series[0] = %s/%d, want radio/6", radio.Key, radio.Plays) + } + if len(radio.Points) != 2 { + t.Fatalf("radio points = %d, want 2 weeks", len(radio.Points)) + } + wk1 := radio.Points[0] + if wk1.WeekStart != "2026-06-22" || wk1.Plays != 4 || wk1.Skips != 2 { + t.Errorf("week1 = %+v, want 2026-06-22 with 4 plays / 2 skips", wk1) + } + if wk1.SkipRate != 0.5 { + t.Errorf("week1 skip_rate = %v, want 0.5", wk1.SkipRate) + } + // Completion weighted by sample count: (0.6*3 + 0.2*1) / 4 = 0.5. + if wk1.AvgCompletion < 0.49 || wk1.AvgCompletion > 0.51 { + t.Errorf("week1 avg_completion = %v, want 0.5", wk1.AvgCompletion) + } + // Taste hits: 3 of 4 plays. + if wk1.TasteHitRate != 0.75 { + t.Errorf("week1 taste_hit_rate = %v, want 0.75", wk1.TasteHitRate) + } + if manual := resp.Series[1]; manual.Key != "manual" || manual.Intent != "" { + t.Errorf("series[1] = %+v, want the manual baseline family", manual) + } + + if len(resp.Markers) != 1 { + t.Fatalf("markers = %d, want 1 (out-of-window marker dropped)", len(resp.Markers)) + } + if resp.Markers[0].Scope != "radio" || resp.Markers[0].Action != "update" { + t.Errorf("marker = %+v, want radio/update", resp.Markers[0]) + } +} + +func newTrendsRouter(h *handlers) chi.Router { + r := chi.NewRouter() + r.Get("/api/admin/recommendation-trends", h.handleGetRecommendationTrends) + return r +} + +func TestRecommendationTrends_EndToEnd(t *testing.T) { + h, pool := testHandlers(t) + user := seedUser(t, pool, "trends", "pw", true) + artist := seedArtist(t, pool, "TrendArtist") + album := seedAlbum(t, pool, artist.ID, "TrendAlbum", 2020) + tk := seedTrack(t, pool, album.ID, artist.ID, "TrendTrack", 1, 200000) + session := seedPlaySession(t, pool, user.ID, time.Now()) + + // Positive taste weight for the artist → plays count as taste hits. + if _, err := pool.Exec(context.Background(), + `INSERT INTO taste_profile_artists (user_id, artist_id, weight) VALUES ($1, $2, 1.5)`, + user.ID, artist.ID); err != nil { + t.Fatalf("seed taste profile: %v", err) + } + radio := "radio" + seedSourcedPlay(t, h, user.ID, tk.ID, session, &radio, nil, 0.9, false) + // A knob turn to mark the timeline. + if err := h.recSettings.UpdateProfile(context.Background(), "radio", + map[string]float64{"taste_weight": 2}); err != nil { + t.Fatalf("UpdateProfile: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/api/admin/recommendation-trends", nil) + rec := httptest.NewRecorder() + newTrendsRouter(h).ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (%s)", rec.Code, rec.Body.String()) + } + var resp trendsResp + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Weeks != trendsDefaultWeeks { + t.Errorf("weeks = %d, want %d", resp.Weeks, trendsDefaultWeeks) + } + var radioSeries *trendSeries + for i := range resp.Series { + if resp.Series[i].Key == "radio" { + radioSeries = &resp.Series[i] + } + } + if radioSeries == nil || len(radioSeries.Points) != 1 { + t.Fatalf("radio series = %+v, want one point", radioSeries) + } + if radioSeries.Points[0].TasteHitRate != 1.0 { + t.Errorf("taste_hit_rate = %v, want 1.0 (positive-weight artist)", + radioSeries.Points[0].TasteHitRate) + } + if len(resp.Markers) != 1 || resp.Markers[0].Scope != "radio" { + t.Errorf("markers = %+v, want the radio knob turn", resp.Markers) + } +} + +func TestRecommendationTrends_InvalidWeeks(t *testing.T) { + h, _ := testHandlers(t) + req := httptest.NewRequest(http.MethodGet, "/api/admin/recommendation-trends?weeks=zero", nil) + rec := httptest.NewRecorder() + newTrendsRouter(h).ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rec.Code) + } +} diff --git a/internal/api/admin_recommendation_tuning.go b/internal/api/admin_recommendation_tuning.go new file mode 100644 index 00000000..1a556d9d --- /dev/null +++ b/internal/api/admin_recommendation_tuning.go @@ -0,0 +1,152 @@ +// Admin recommendation-tuning endpoints (#1250): the defaults- +// discovery lab. GET returns current values + shipped defaults for +// every scope; PATCH applies a partial update to one scope; reset +// restores a scope to shipped defaults. Every change writes an audit +// row (consumed by the metrics trend view, #1251). +package api + +import ( + "encoding/json" + "errors" + "net/http" + + "github.com/go-chi/chi/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/recommendation" + "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" +) + +// weightsResp is one weight profile on the wire, keyed by the same +// snake_case field names the PATCH body accepts. +type weightsResp struct { + BaseWeight float64 `json:"base_weight"` + LikeBoost float64 `json:"like_boost"` + RecencyWeight float64 `json:"recency_weight"` + SkipPenalty float64 `json:"skip_penalty"` + JitterMagnitude float64 `json:"jitter_magnitude"` + ContextWeight float64 `json:"context_weight"` + SimilarityWeight float64 `json:"similarity_weight"` + TasteWeight float64 `json:"taste_weight"` +} + +func weightsRespFrom(w recommendation.ScoringWeights) weightsResp { + return weightsResp{ + BaseWeight: w.BaseWeight, + LikeBoost: w.LikeBoost, + RecencyWeight: w.RecencyWeight, + SkipPenalty: w.SkipPenalty, + JitterMagnitude: w.JitterMagnitude, + ContextWeight: w.ContextWeight, + SimilarityWeight: w.SimilarityWeight, + TasteWeight: w.TasteWeight, + } +} + +type tasteTuningResp struct { + HalfLifeDays float64 `json:"half_life_days"` + EngagementHardSkip float64 `json:"engagement_hard_skip"` + EngagementNeutral float64 `json:"engagement_neutral"` + EngagementFull float64 `json:"engagement_full"` +} + +func tasteRespFrom(t recsettings.TasteTuning) tasteTuningResp { + return tasteTuningResp{ + HalfLifeDays: t.HalfLifeDays, + EngagementHardSkip: t.EngagementHardSkip, + EngagementNeutral: t.EngagementNeutral, + EngagementFull: t.EngagementFull, + } +} + +// 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"` + Shipped struct { + Profiles map[string]weightsResp `json:"profiles"` + Taste tasteTuningResp `json:"taste"` + } `json:"shipped"` +} + +func (h *handlers) tuningSnapshot() tuningSnapshot { + var out tuningSnapshot + out.Profiles = map[string]weightsResp{ + recsettings.ScopeRadio: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeRadio)), + recsettings.ScopeDailyMix: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeDailyMix)), + } + out.Taste = tasteRespFrom(h.recSettings.Taste()) + out.Shipped.Profiles = map[string]weightsResp{ + recsettings.ScopeRadio: weightsRespFrom(recsettings.ShippedRadioWeights()), + recsettings.ScopeDailyMix: weightsRespFrom(recsettings.ShippedDailyMixWeights()), + } + out.Shipped.Taste = tasteRespFrom(recsettings.ShippedTasteTuning()) + return out +} + +// handleGetRecommendationTuning implements GET /api/admin/recommendation-tuning. +func (h *handlers) handleGetRecommendationTuning(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, h.tuningSnapshot()) +} + +// patchTuningReq carries the partial update: field name → new value, +// using the same snake_case names the GET response emits. +type patchTuningReq struct { + Values map[string]float64 `json:"values"` +} + +// handlePatchRecommendationTuning implements +// PATCH /api/admin/recommendation-tuning/{scope}. +func (h *handlers) handlePatchRecommendationTuning(w http.ResponseWriter, r *http.Request) { + scope := chi.URLParam(r, "scope") + var body patchTuningReq + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + writeErr(w, apierror.BadRequest("bad_body", "invalid JSON")) + return + } + if len(body.Values) == 0 { + writeErr(w, apierror.BadRequest("bad_body", "values is empty")) + return + } + + var err error + if scope == recsettings.ScopeTaste { + err = h.recSettings.UpdateTaste(r.Context(), body.Values) + } else { + err = h.recSettings.UpdateProfile(r.Context(), scope, body.Values) + } + if err != nil { + writeTuningErr(w, h, scope, err) + return + } + writeJSON(w, http.StatusOK, h.tuningSnapshot()) +} + +// handleResetRecommendationTuning implements +// POST /api/admin/recommendation-tuning/{scope}/reset. +func (h *handlers) handleResetRecommendationTuning(w http.ResponseWriter, r *http.Request) { + scope := chi.URLParam(r, "scope") + if err := h.recSettings.Reset(r.Context(), scope); err != nil { + writeTuningErr(w, h, scope, err) + return + } + writeJSON(w, http.StatusOK, h.tuningSnapshot()) +} + +// writeTuningErr maps recsettings validation errors to 400s and +// everything else to a logged 500. +func writeTuningErr(w http.ResponseWriter, h *handlers, scope string, err error) { + switch { + case errors.Is(err, recsettings.ErrUnknownScope): + writeErr(w, &apierror.Error{ + Status: http.StatusNotFound, Code: "not_found", Message: "no such tuning scope", + }) + case errors.Is(err, recsettings.ErrUnknownField), errors.Is(err, recsettings.ErrOutOfRange): + writeErr(w, apierror.BadRequest("invalid_tuning", err.Error())) + default: + h.logger.Error("admin: recommendation tuning", "scope", scope, "err", err) + writeErr(w, apierror.InternalMsg("tuning update failed", err)) + } +} diff --git a/internal/api/admin_recommendation_tuning_test.go b/internal/api/admin_recommendation_tuning_test.go new file mode 100644 index 00000000..4c6bba88 --- /dev/null +++ b/internal/api/admin_recommendation_tuning_test.go @@ -0,0 +1,110 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" +) + +func newTuningRouter(h *handlers) chi.Router { + r := chi.NewRouter() + r.Get("/api/admin/recommendation-tuning", h.handleGetRecommendationTuning) + r.Patch("/api/admin/recommendation-tuning/{scope}", h.handlePatchRecommendationTuning) + r.Post("/api/admin/recommendation-tuning/{scope}/reset", h.handleResetRecommendationTuning) + return r +} + +func decodeTuning(t *testing.T, rec *httptest.ResponseRecorder) tuningSnapshot { + t.Helper() + var snap tuningSnapshot + if err := json.Unmarshal(rec.Body.Bytes(), &snap); err != nil { + t.Fatalf("decode: %v", err) + } + return snap +} + +func TestRecommendationTuning_GetReturnsShippedDefaults(t *testing.T) { + h, _ := testHandlers(t) + req := httptest.NewRequest(http.MethodGet, "/api/admin/recommendation-tuning", nil) + rec := httptest.NewRecorder() + newTuningRouter(h).ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + snap := decodeTuning(t, rec) + if snap.Profiles["radio"].TasteWeight != 1.0 || snap.Profiles["daily_mix"].TasteWeight != 1.5 { + t.Errorf("profiles = %+v, want shipped taste weights 1.0 / 1.5", snap.Profiles) + } + if snap.Taste.HalfLifeDays != 75 { + t.Errorf("taste half-life = %v, want shipped 75", snap.Taste.HalfLifeDays) + } + if snap.Shipped.Profiles["radio"] != snap.Profiles["radio"] { + t.Error("untouched values must equal shipped defaults") + } +} + +func TestRecommendationTuning_PatchAndReset(t *testing.T) { + h, _ := testHandlers(t) + r := newTuningRouter(h) + + req := httptest.NewRequest(http.MethodPatch, "/api/admin/recommendation-tuning/radio", + strings.NewReader(`{"values":{"taste_weight": 2.5}}`)) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("patch status = %d, want 200 (%s)", rec.Code, rec.Body.String()) + } + snap := decodeTuning(t, rec) + if snap.Profiles["radio"].TasteWeight != 2.5 { + t.Errorf("patched taste_weight = %v, want 2.5", snap.Profiles["radio"].TasteWeight) + } + // The change is live for the radio scoring path. + if got := h.recSettings.Weights(recsettings.ScopeRadio).TasteWeight; got != 2.5 { + t.Errorf("service taste_weight = %v, want 2.5 (live effect)", got) + } + + req = httptest.NewRequest(http.MethodPost, "/api/admin/recommendation-tuning/radio/reset", nil) + rec = httptest.NewRecorder() + r.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("reset status = %d, want 200", rec.Code) + } + snap = decodeTuning(t, rec) + if snap.Profiles["radio"].TasteWeight != 1.0 { + t.Errorf("reset taste_weight = %v, want shipped 1.0", snap.Profiles["radio"].TasteWeight) + } +} + +func TestRecommendationTuning_PatchErrors(t *testing.T) { + h, _ := testHandlers(t) + r := newTuningRouter(h) + cases := []struct { + name, path, body string + want int + }{ + {"unknown scope", "/api/admin/recommendation-tuning/banana", + `{"values":{"taste_weight":1}}`, http.StatusNotFound}, + {"unknown field", "/api/admin/recommendation-tuning/radio", + `{"values":{"vibes":1}}`, http.StatusBadRequest}, + {"out of range", "/api/admin/recommendation-tuning/taste", + `{"values":{"engagement_neutral":2}}`, http.StatusBadRequest}, + {"empty values", "/api/admin/recommendation-tuning/radio", + `{"values":{}}`, http.StatusBadRequest}, + {"bad json", "/api/admin/recommendation-tuning/radio", + `{`, http.StatusBadRequest}, + } + for _, c := range cases { + req := httptest.NewRequest(http.MethodPatch, c.path, strings.NewReader(c.body)) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + if rec.Code != c.want { + t.Errorf("%s: status = %d, want %d", c.name, rec.Code, c.want) + } + } +} diff --git a/internal/api/api.go b/internal/api/api.go index 5a7e4bfe..a8649fb7 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -22,16 +22,18 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/mailer" "git.fabledsword.com/bvandeusen/minstrel/internal/playevents" "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" + "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" "git.fabledsword.com/bvandeusen/minstrel/internal/tracks" ) // Mount attaches /api/* handlers to r. Public endpoints (login) are outside // RequireUser; everything else is gated by the middleware. The events writer // is shared with the Subsonic mount so /rest/scrobble feeds the same store. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte) { +func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte) { rng := rand.New(rand.NewSource(rand.Int63())) h := &handlers{ pool: pool, logger: logger, events: events, recCfg: recCfg, + recSettings: recSettings, rng: rng.Float64, lidarrCfg: lidarrCfg, lidarrRequests: lidarrReqs, @@ -201,6 +203,14 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Get("/smtp-config", h.handleGetSMTPConfig) admin.Put("/smtp-config", h.handleUpdateSMTPConfig) admin.Post("/smtp-config/test", h.handleTestSMTPConfig) + + // Recommendation tuning lab (#1250): scoring-weight + // profiles + taste-build knobs, DB-backed, live effect. + admin.Get("/recommendation-tuning", h.handleGetRecommendationTuning) + admin.Patch("/recommendation-tuning/{scope}", h.handlePatchRecommendationTuning) + admin.Post("/recommendation-tuning/{scope}/reset", h.handleResetRecommendationTuning) + // Weekly outcome trends + knob-turn markers (#1251). + admin.Get("/recommendation-trends", h.handleGetRecommendationTrends) }) authed.Get("/playlists", h.handleListPlaylists) @@ -223,6 +233,7 @@ type handlers struct { logger *slog.Logger events *playevents.Writer recCfg config.RecommendationConfig + recSettings *recsettings.Service rng func() float64 lidarrCfg *lidarrconfig.Service lidarrRequests *lidarrrequests.Service diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 333256bf..ae564b9c 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -29,6 +29,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/mailer" "git.fabledsword.com/bvandeusen/minstrel/internal/playevents" "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" + "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" "git.fabledsword.com/bvandeusen/minstrel/internal/tracks" ) @@ -56,11 +57,12 @@ func testHandlers(t *testing.T) (*handlers, *pgxpool.Pool) { dbtest.ResetDB(t, pool) w := playevents.NewWriter(pool, logger, 30*time.Minute, 0.5, 30000) recCfg := config.RecommendationConfig{ - BaseWeight: 1.0, LikeBoost: 2.0, RecencyWeight: 1.0, - SkipPenalty: 1.0, JitterMagnitude: 0.1, - ContextWeight: 2.0, SimilarityWeight: 2.0, RecentlyPlayedHours: 1, RadioSize: 50, RadioSizeMax: 200, } + recSettings, err := recsettings.New(context.Background(), pool, logger) + if err != nil { + t.Fatalf("recsettings: %v", err) + } lidarrCfg := lidarrconfig.New(pool) lidarrReqs := lidarrrequests.NewService(pool, lidarrCfg, nil, nil) lidarrQuar := lidarrquarantine.NewService(pool, lidarrCfg, nil) @@ -70,7 +72,7 @@ func testHandlers(t *testing.T) (*handlers, *pgxpool.Pool) { dataDir := t.TempDir() tracksSvc := tracks.NewService(pool, logger, nil, dataDir) playlistsSvc := playlists.NewService(pool, logger, dataDir) - h := &handlers{pool: pool, logger: logger, events: w, recCfg: recCfg, rng: func() float64 { return 0.5 }, lidarrCfg: lidarrCfg, lidarrRequests: lidarrReqs, lidarrQuarantine: lidarrQuar, tracks: tracksSvc, playlists: playlistsSvc, dataDir: dataDir, scanner: nil, scanCfg: library.RunScanConfig{}, mailer: &mailer.FakeSender{}} + h := &handlers{pool: pool, logger: logger, events: w, recCfg: recCfg, recSettings: recSettings, rng: func() float64 { return 0.5 }, lidarrCfg: lidarrCfg, lidarrRequests: lidarrReqs, lidarrQuarantine: lidarrQuar, tracks: tracksSvc, playlists: playlistsSvc, dataDir: dataDir, scanner: nil, scanCfg: library.RunScanConfig{}, mailer: &mailer.FakeSender{}} return h, pool } diff --git a/internal/api/events.go b/internal/api/events.go index c7e49f16..150f534f 100644 --- a/internal/api/events.go +++ b/internal/api/events.go @@ -31,6 +31,26 @@ type playStartedResponse struct { SessionID string `json:"session_id"` } +// eventFutureSkew is the tolerated client-clock drift into the future +// before a timestamp is treated as bogus and replaced with now. +const eventFutureSkew = 5 * time.Minute + +// clampEventTime bounds a client-supplied event timestamp to sanity: +// no earlier than the account's creation (offline replays can +// legitimately be days old, but no play can predate the user) and no +// later than now + a small skew allowance. Unbounded client clocks +// previously let a skewed device write arbitrarily old plays, which +// poisoned Rediscover's "not played in 6 months" ordering (#1246). +func clampEventTime(at, userCreatedAt, now time.Time) time.Time { + if at.Before(userCreatedAt) { + return userCreatedAt + } + if at.After(now.Add(eventFutureSkew)) { + return now + } + return at +} + type okResponse struct { OK bool `json:"ok"` } @@ -52,7 +72,7 @@ func (h *handlers) handleEvents(w http.ResponseWriter, r *http.Request) { writeErr(w, apierror.BadRequest("bad_request", "invalid `at` timestamp")) return } - at = parsed + at = clampEventTime(parsed.UTC(), user.CreatedAt.Time, time.Now().UTC()) } clientID := "" if req.ClientID != nil { diff --git a/internal/api/library_test.go b/internal/api/library_test.go index 09d36cf7..370da98e 100644 --- a/internal/api/library_test.go +++ b/internal/api/library_test.go @@ -465,7 +465,7 @@ func TestRoutesRegisteredInMount(t *testing.T) { r := chi.NewRouter() w := playevents.NewWriter(h.pool, slog.New(slog.NewTextHandler(io.Discard, nil)), 30*time.Minute, 0.5, 30000) - Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.scanner, h.scanCfg, h.dataDir, nil, eventbus.New(), nil, nil) + Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.recSettings, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.scanner, h.scanCfg, h.dataDir, nil, eventbus.New(), nil, nil) paths := []string{ "/api/artists", diff --git a/internal/api/me_recommendation_metrics.go b/internal/api/me_recommendation_metrics.go index 05a15a75..75761b05 100644 --- a/internal/api/me_recommendation_metrics.go +++ b/internal/api/me_recommendation_metrics.go @@ -2,7 +2,9 @@ package api import ( "net/http" + "sort" "strconv" + "strings" "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" @@ -11,28 +13,142 @@ import ( const ( recMetricsDefaultDays = 30 recMetricsMaxDays = 365 + + // recMetricsLowVolume marks a family as low-confidence rather than + // hiding it: with fewer plays than this a skip rate is anecdote, not + // signal, but silently dropping the row would misread as "surface + // unused". The web renders low-confidence rows dimmed. + recMetricsLowVolume = 20 ) -// recommendationMetric is one recommendation surface's outcomes. -type recommendationMetric struct { - Source string `json:"source"` // 'for_you' | 'discover' | mixes - Plays int64 `json:"plays"` // plays launched from this surface +// Surface intents (milestone #127): each family is judged against its +// job, not one global bar — discovery mixes are EXPECTED to run higher +// skip rates than the go-to surfaces. +const ( + intentGoTo = "go_to" + intentDiscovery = "discovery" + intentDirect = "direct" +) + +// surfaceMetric is one bucketed surface family's outcomes. +type surfaceMetric struct { + Key string `json:"key"` // stable family key ("for_you", "radio", …) + Label string `json:"label"` // display label + Plays int64 `json:"plays"` // plays launched from this family Skips int64 `json:"skips"` // of those, marked skipped SkipRate float64 `json:"skip_rate"` // skips / plays, [0,1] AvgCompletion float64 `json:"avg_completion"` // mean completion ratio, [0,1] + LowConfidence bool `json:"low_confidence"` // plays < recMetricsLowVolume + // Breakdown splits the family into the pick-kind populations its + // builder stamped (#1249, generalized #1270): For You's taste/fresh, + // Discover's buckets, tier1-3 for tiered mixes — plus earlier plays + // that predate attribution. Present only when the family has at + // least one attributed play; the parent row remains the sum of its + // breakdown. + Breakdown []surfaceMetric `json:"breakdown,omitempty"` +} + +// surfaceGroup is one intent band of surface families. +type surfaceGroup struct { + Intent string `json:"intent"` // go_to | discovery | direct + Label string `json:"label"` + Surfaces []surfaceMetric `json:"surfaces"` } type recommendationMetricsResp struct { - WindowDays int `json:"window_days"` - Sources []recommendationMetric `json:"sources"` + WindowDays int `json:"window_days"` + // Baseline is the control group: plays the user picked manually + // (source IS NULL). Surfaces are judged as deltas against it; nil + // when the window holds no manual plays. + Baseline *surfaceMetric `json:"baseline"` + Groups []surfaceGroup `json:"groups"` +} + +// recFamily is the bucketing target for a raw play_events.source value. +type recFamily struct { + key string + label string + intent string +} + +// bucketRecSource maps a raw client-stamped source string to its stable +// family. One-off sources (album:, radio:) collapse into +// their family so the table stays readable at any library size. +func bucketRecSource(src string) recFamily { + switch { + case src == "for_you": + return recFamily{"for_you", "For You", intentGoTo} + case src == "songs_like_artist": + return recFamily{"songs_like_artist", "Songs like…", intentGoTo} + case src == "radio" || strings.HasPrefix(src, "radio:"): + return recFamily{"radio", "Radio", intentGoTo} + case src == "discover": + return recFamily{"discover", "Discover", intentDiscovery} + case src == "deep_cuts": + return recFamily{"deep_cuts", "Deep cuts", intentDiscovery} + case src == "rediscover": + return recFamily{"rediscover", "Rediscover", intentDiscovery} + case src == "new_for_you": + return recFamily{"new_for_you", "New for you", intentDiscovery} + case src == "on_this_day": + return recFamily{"on_this_day", "On this day", intentDiscovery} + case src == "first_listens": + return recFamily{"first_listens", "First listens", intentDiscovery} + case strings.HasPrefix(src, "album:"): + return recFamily{"direct_album", "Album plays", intentDirect} + case strings.HasPrefix(src, "artist:"): + return recFamily{"direct_artist", "Artist plays", intentDirect} + case strings.HasPrefix(src, "offline:"): + return recFamily{"offline", "Offline pools", intentDirect} + case strings.HasPrefix(src, "home:"): + return recFamily{"home", "Home sections", intentDirect} + case src == "history": + return recFamily{"history", "History", intentDirect} + default: + return recFamily{"other", "Other", intentDirect} + } +} + +// familyAccum merges raw source rows into one family, carrying the +// completion sample count so the merged average stays play-weighted. +type familyAccum struct { + fam recFamily + plays int64 + skips int64 + completionN int64 + // completionSum is avg*count re-expanded, so merging N raw rows + // reduces to a single weighted division at the end. + completionSum float64 +} + +func (a *familyAccum) add(row dbq.RecommendationSourceMetricsForUserRow) { + a.plays += row.Plays + a.skips += row.Skips + a.completionN += row.CompletionN + a.completionSum += row.AvgCompletion * float64(row.CompletionN) +} + +func (a *familyAccum) metric() surfaceMetric { + m := surfaceMetric{ + Key: a.fam.key, + Label: a.fam.label, + Plays: a.plays, + Skips: a.skips, + LowConfidence: a.plays < recMetricsLowVolume, + } + if a.plays > 0 { + m.SkipRate = float64(a.skips) / float64(a.plays) + } + if a.completionN > 0 { + m.AvgCompletion = a.completionSum / float64(a.completionN) + } + return m } // handleGetRecommendationMetrics implements GET /api/me/recommendation-metrics. -// Per-source play outcomes (plays / skips / skip-rate / avg-completion) for the -// caller over the last `days` (default 30, capped at 365), so the operator can -// see which recommendation surfaces are landing and tune the taste weights. -// Only plays tagged with a system-playlist source count; library/radio plays -// (no source) are excluded. +// Bucketed per-surface-family outcomes for the caller over the last `days` +// (default 30, capped at 365), grouped by surface intent and anchored by the +// manual-plays baseline so the numbers are judgeable, not just observable. func (h *handlers) handleGetRecommendationMetrics(w http.ResponseWriter, r *http.Request) { caller, ok := requireUser(w, r) if !ok { @@ -51,28 +167,141 @@ func (h *handlers) handleGetRecommendationMetrics(w http.ResponseWriter, r *http return } - out := recommendationMetricsResp{ - WindowDays: days, - Sources: make([]recommendationMetric, 0, len(rows)), + writeJSON(w, http.StatusOK, bucketMetricsResponse(days, rows)) +} + +// pickKindLabels is the display vocabulary for play_events.pick_kind +// values (mirrors the CHECK in migration 0039). Every system mix that +// stamps provenance gets its breakdown from this one map — adding a +// stamping mix needs no metrics change. +var pickKindLabels = map[string]string{ + "taste": "Taste picks", + "fresh": "Fresh picks", + "dormant": "Dormant artists", + "cross_user": "Liked by others", + "random": "Random unheard", + "tier1": "Tier 1 (exact)", + "tier2": "Tier 2 (relaxed)", + "tier3": "Tier 3 (stretched)", +} + +// pickKindOrder fixes breakdown row order; unattributed ("", i.e. NULL +// pick_kind — plays recorded before the mix stamped provenance, or +// whose track had rotated out of the snapshot at ingestion) renders +// last, kept visible so the parent row's sums stay transparent instead +// of silently shrinking. The DB CHECK gates pick_kind to exactly this +// vocabulary, so iterating the list is exhaustive. +var pickKindOrder = []string{ + "taste", "fresh", "dormant", "cross_user", "random", + "tier1", "tier2", "tier3", "", +} + +// pickKindFamily derives the sub-family for one (family, pick_kind) +// population, e.g. ("for_you", "taste") → for_you_taste "Taste picks". +func pickKindFamily(parent recFamily, kind string) recFamily { + if kind == "" { + return recFamily{parent.key + "_unattributed", "Earlier plays", parent.intent} } + label, ok := pickKindLabels[kind] + if !ok { + label = kind + } + return recFamily{parent.key + "_" + kind, label, parent.intent} +} + +// pickKindBreakdown folds a family's per-pick-kind accums into its +// Breakdown rows. Attached only when at least one attributed play +// exists — an all-unattributed breakdown would just repeat the parent +// row, and families that never stamp (radio, direct plays) stay flat. +func pickKindBreakdown(picks map[string]*familyAccum) []surfaceMetric { + attributed := int64(0) + for kind, acc := range picks { + if kind != "" { + attributed += acc.plays + } + } + if attributed == 0 { + return nil + } + out := make([]surfaceMetric, 0, len(picks)) + for _, kind := range pickKindOrder { + if acc, ok := picks[kind]; ok && acc.plays > 0 { + out = append(out, acc.metric()) + } + } + return out +} + +// bucketMetricsResponse folds the raw per-source rows into the grouped, +// baseline-anchored response shape. Split from the handler for pure-unit +// testability. +func bucketMetricsResponse( + days int, rows []dbq.RecommendationSourceMetricsForUserRow, +) recommendationMetricsResp { + baseline := &familyAccum{fam: recFamily{"manual", "Manual library plays", ""}} + families := map[string]*familyAccum{} + picks := map[string]map[string]*familyAccum{} for _, row := range rows { - source := "" - if row.Source != nil { - source = *row.Source + if row.Source == nil || *row.Source == "" { + baseline.add(row) + continue } - var skipRate float64 - if row.Plays > 0 { - skipRate = float64(row.Skips) / float64(row.Plays) + fam := bucketRecSource(*row.Source) + acc, exists := families[fam.key] + if !exists { + acc = &familyAccum{fam: fam} + families[fam.key] = acc } - out.Sources = append(out.Sources, recommendationMetric{ - Source: source, - Plays: row.Plays, - Skips: row.Skips, - SkipRate: skipRate, - AvgCompletion: row.AvgCompletion, - }) + acc.add(row) + // Accumulate the pick-kind population unconditionally; families + // that never stamp end up all-unattributed and get no breakdown. + kind := "" + if row.PickKind != nil { + kind = *row.PickKind + } + byKind, ok := picks[fam.key] + if !ok { + byKind = map[string]*familyAccum{} + picks[fam.key] = byKind + } + pick, ok := byKind[kind] + if !ok { + pick = &familyAccum{fam: pickKindFamily(fam, kind)} + byKind[kind] = pick + } + pick.add(row) } - writeJSON(w, http.StatusOK, out) + + resp := recommendationMetricsResp{WindowDays: days, Groups: []surfaceGroup{}} + if baseline.plays > 0 { + m := baseline.metric() + resp.Baseline = &m + } + for _, g := range []struct{ intent, label string }{ + {intentGoTo, "Go-to surfaces"}, + {intentDiscovery, "Discovery mixes"}, + {intentDirect, "Direct plays"}, + } { + group := surfaceGroup{Intent: g.intent, Label: g.label} + for _, acc := range families { + if acc.fam.intent == g.intent { + m := acc.metric() + m.Breakdown = pickKindBreakdown(picks[acc.fam.key]) + group.Surfaces = append(group.Surfaces, m) + } + } + if len(group.Surfaces) == 0 { + continue + } + sort.Slice(group.Surfaces, func(i, j int) bool { + if group.Surfaces[i].Plays != group.Surfaces[j].Plays { + return group.Surfaces[i].Plays > group.Surfaces[j].Plays + } + return group.Surfaces[i].Key < group.Surfaces[j].Key + }) + resp.Groups = append(resp.Groups, group) + } + return resp } // parseMetricsDays reads the `days` query param (default 30, capped at 365). diff --git a/internal/api/me_recommendation_metrics_test.go b/internal/api/me_recommendation_metrics_test.go index 2a1af4c3..de717b79 100644 --- a/internal/api/me_recommendation_metrics_test.go +++ b/internal/api/me_recommendation_metrics_test.go @@ -11,6 +11,8 @@ import ( "github.com/go-chi/chi/v5" "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) func newMetricsRouter(h *handlers) chi.Router { @@ -20,17 +22,19 @@ func newMetricsRouter(h *handlers) chi.Router { } // seedSourcedPlay inserts a play_event with an explicit source + completion + -// skip flag. A nil source inserts NULL (library/radio play). +// skip flag. A nil source inserts NULL (manual library play → baseline). +// pickKind is the For You taste/fresh attribution (#1249); nil everywhere +// except attributed for_you plays. func seedSourcedPlay( t *testing.T, h *handlers, userID, trackID, sessionID pgtype.UUID, - source *string, completion float64, skipped bool, + source, pickKind *string, completion float64, skipped bool, ) { t.Helper() if _, err := h.pool.Exec(context.Background(), `INSERT INTO play_events - (user_id, track_id, session_id, started_at, source, completion_ratio, was_skipped) - VALUES ($1, $2, $3, now(), $4, $5, $6)`, - userID, trackID, sessionID, source, completion, skipped); err != nil { + (user_id, track_id, session_id, started_at, source, pick_kind, completion_ratio, was_skipped) + VALUES ($1, $2, $3, now(), $4, $5, $6, $7)`, + userID, trackID, sessionID, source, pickKind, completion, skipped); err != nil { t.Fatalf("seed sourced play: %v", err) } } @@ -45,7 +49,168 @@ func TestRecommendationMetrics_NoSession401(t *testing.T) { } } -func TestRecommendationMetrics_AggregatesBySourceExcludingNull(t *testing.T) { +// findSurface returns the named family from any group, or nil. +func findSurface(resp recommendationMetricsResp, key string) *surfaceMetric { + for _, g := range resp.Groups { + for i := range g.Surfaces { + if g.Surfaces[i].Key == key { + return &g.Surfaces[i] + } + } + } + return nil +} + +func TestBucketMetricsResponse_FamiliesGroupsBaseline(t *testing.T) { + src := func(s string) *string { return &s } + rows := []dbq.RecommendationSourceMetricsForUserRow{ + {Source: src("for_you"), Plays: 3, Skips: 1, CompletionN: 3, AvgCompletion: 2.0 / 3}, + // Two radio sessions collapse into one "radio" family; weighted + // completion = (0.2*1 + 0.8*1) / 2 = 0.5. + {Source: src("radio:aaaa"), Plays: 1, Skips: 1, CompletionN: 1, AvgCompletion: 0.2}, + {Source: src("radio:bbbb"), Plays: 1, Skips: 0, CompletionN: 1, AvgCompletion: 0.8}, + {Source: src("album:cccc"), Plays: 1, Skips: 0, CompletionN: 0, AvgCompletion: 0}, + {Source: src("discover"), Plays: 1, Skips: 0, CompletionN: 1, AvgCompletion: 0.8}, + // NULL source = manual plays → baseline, not a group row. + {Source: nil, Plays: 25, Skips: 5, CompletionN: 20, AvgCompletion: 0.9}, + } + resp := bucketMetricsResponse(recMetricsDefaultDays, rows) + + if resp.Baseline == nil { + t.Fatal("baseline missing") + } + if resp.Baseline.Plays != 25 || resp.Baseline.SkipRate != 0.2 { + t.Errorf("baseline = %+v, want plays=25 skip_rate=0.2", resp.Baseline) + } + if resp.Baseline.LowConfidence { + t.Error("baseline with 25 plays should not be low-confidence") + } + + radio := findSurface(resp, "radio") + if radio == nil { + t.Fatal("radio family missing") + } + if radio.Plays != 2 || radio.Skips != 1 { + t.Errorf("radio plays/skips = %d/%d, want 2/1", radio.Plays, radio.Skips) + } + if radio.AvgCompletion < 0.49 || radio.AvgCompletion > 0.51 { + t.Errorf("radio avg_completion = %.3f, want 0.5 (play-weighted merge)", radio.AvgCompletion) + } + if !radio.LowConfidence { + t.Error("radio with 2 plays should be low-confidence") + } + + if s := findSurface(resp, "direct_album"); s == nil || s.Plays != 1 { + t.Errorf("direct_album = %+v, want plays=1", s) + } + if s := findSurface(resp, ""); s != nil { + t.Error("NULL source must not appear as a surface family") + } + + // No attributed (taste/fresh) plays in this fixture → no breakdown; + // an all-unattributed breakdown would just repeat the parent row. + if fy := findSurface(resp, "for_you"); fy == nil || fy.Breakdown != nil { + t.Errorf("for_you breakdown = %+v, want nil without attributed plays", fy) + } + + // Group ordering is intent-banded: go_to before discovery before direct. + wantOrder := []string{intentGoTo, intentDiscovery, intentDirect} + if len(resp.Groups) != len(wantOrder) { + t.Fatalf("groups = %d, want %d", len(resp.Groups), len(wantOrder)) + } + for i, g := range resp.Groups { + if g.Intent != wantOrder[i] { + t.Errorf("group[%d].intent = %s, want %s", i, g.Intent, wantOrder[i]) + } + } +} + +func TestBucketMetricsResponse_ForYouBreakdown(t *testing.T) { + src := func(s string) *string { return &s } + kind := func(s string) *string { return &s } + rows := []dbq.RecommendationSourceMetricsForUserRow{ + {Source: src("for_you"), PickKind: kind("taste"), Plays: 30, Skips: 3, + CompletionN: 30, AvgCompletion: 0.9}, + {Source: src("for_you"), PickKind: kind("fresh"), Plays: 10, Skips: 4, + CompletionN: 10, AvgCompletion: 0.5}, + // NULL pick_kind = plays that predate attribution. + {Source: src("for_you"), Plays: 5, Skips: 1, CompletionN: 5, AvgCompletion: 0.7}, + } + resp := bucketMetricsResponse(recMetricsDefaultDays, rows) + + fy := findSurface(resp, "for_you") + if fy == nil { + t.Fatal("for_you family missing") + } + // The parent row stays the sum of its breakdown. + if fy.Plays != 45 || fy.Skips != 8 { + t.Errorf("for_you plays/skips = %d/%d, want 45/8", fy.Plays, fy.Skips) + } + if len(fy.Breakdown) != 3 { + t.Fatalf("breakdown rows = %d, want 3 (taste, fresh, earlier)", len(fy.Breakdown)) + } + wantKeys := []string{"for_you_taste", "for_you_fresh", "for_you_unattributed"} + for i, k := range wantKeys { + if fy.Breakdown[i].Key != k { + t.Errorf("breakdown[%d].key = %s, want %s", i, fy.Breakdown[i].Key, k) + } + } + taste, fresh := fy.Breakdown[0], fy.Breakdown[1] + if taste.Plays != 30 || taste.SkipRate != 0.1 || taste.LowConfidence { + t.Errorf("taste = %+v, want plays=30 skip_rate=0.1 confident", taste) + } + if fresh.Plays != 10 || fresh.SkipRate != 0.4 || !fresh.LowConfidence { + t.Errorf("fresh = %+v, want plays=10 skip_rate=0.4 low-confidence", fresh) + } + // Breakdown rows never appear as their own surface families. + if s := findSurface(resp, "for_you_taste"); s != nil { + t.Error("for_you_taste must not be a top-level surface") + } +} + +func TestBucketMetricsResponse_DiscoverBucketBreakdown(t *testing.T) { + // Provenance is standard (#1270): Discover's bucket stamps surface as + // a breakdown exactly like For You's taste/fresh — this is what makes + // the 40/30/30 allocation judgeable instead of a guess. + src := func(s string) *string { return &s } + kind := func(s string) *string { return &s } + rows := []dbq.RecommendationSourceMetricsForUserRow{ + {Source: src("discover"), PickKind: kind("dormant"), Plays: 8, Skips: 2, + CompletionN: 8, AvgCompletion: 0.8}, + {Source: src("discover"), PickKind: kind("cross_user"), Plays: 6, Skips: 3, + CompletionN: 6, AvgCompletion: 0.6}, + {Source: src("discover"), PickKind: kind("random"), Plays: 4, Skips: 3, + CompletionN: 4, AvgCompletion: 0.4}, + // NULL pick_kind = plays that predate bucket stamping. + {Source: src("discover"), Plays: 2, Skips: 0, CompletionN: 2, AvgCompletion: 0.9}, + } + resp := bucketMetricsResponse(recMetricsDefaultDays, rows) + + d := findSurface(resp, "discover") + if d == nil { + t.Fatal("discover family missing") + } + if d.Plays != 20 || d.Skips != 8 { + t.Errorf("discover plays/skips = %d/%d, want 20/8", d.Plays, d.Skips) + } + wantKeys := []string{ + "discover_dormant", "discover_cross_user", "discover_random", + "discover_unattributed", + } + if len(d.Breakdown) != len(wantKeys) { + t.Fatalf("breakdown rows = %d, want %d", len(d.Breakdown), len(wantKeys)) + } + for i, k := range wantKeys { + if d.Breakdown[i].Key != k { + t.Errorf("breakdown[%d].key = %s, want %s", i, d.Breakdown[i].Key, k) + } + } + if b := d.Breakdown[0]; b.Label != "Dormant artists" || b.Plays != 8 { + t.Errorf("dormant row = %+v, want label=Dormant artists plays=8", b) + } +} + +func TestRecommendationMetrics_BucketsWithBaseline(t *testing.T) { if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { t.Skip("MINSTREL_TEST_DATABASE_URL not set") } @@ -57,15 +222,17 @@ func TestRecommendationMetrics_AggregatesBySourceExcludingNull(t *testing.T) { session := seedPlaySession(t, pool, user.ID, time.Now()) forYou := "for_you" - discover := "discover" + radioA := "radio:11111111-1111-1111-1111-111111111111" + taste, fresh := "taste", "fresh" // for_you: 3 plays, 1 skipped; completions 1.0, 0.95, 0.05 → mean 0.6667. - seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, 1.0, false) - seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, 0.95, false) - seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, 0.05, true) - // discover: 1 play. - seedSourcedPlay(t, h, user.ID, tk.ID, session, &discover, 0.8, false) - // library play (NULL source) — must be excluded. - seedSourcedPlay(t, h, user.ID, tk.ID, session, nil, 1.0, false) + // Two attributed as taste picks, the skipped one as a fresh pick (#1249). + seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, &taste, 1.0, false) + seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, &taste, 0.95, false) + seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, &fresh, 0.05, true) + // A radio session play collapses into the "radio" family. + seedSourcedPlay(t, h, user.ID, tk.ID, session, &radioA, nil, 0.8, false) + // Manual play (NULL source) — the baseline row. + seedSourcedPlay(t, h, user.ID, tk.ID, session, nil, nil, 1.0, false) req := httptest.NewRequest(http.MethodGet, "/api/me/recommendation-metrics", nil) req = withUser(req, user) @@ -82,15 +249,11 @@ func TestRecommendationMetrics_AggregatesBySourceExcludingNull(t *testing.T) { if resp.WindowDays != recMetricsDefaultDays { t.Errorf("window_days = %d, want %d", resp.WindowDays, recMetricsDefaultDays) } - bySource := map[string]recommendationMetric{} - for _, m := range resp.Sources { - bySource[m.Source] = m + if resp.Baseline == nil || resp.Baseline.Plays != 1 { + t.Fatalf("baseline = %+v, want plays=1", resp.Baseline) } - if _, present := bySource[""]; present { - t.Error("NULL-source (library) plays should be excluded") - } - fy, ok := bySource["for_you"] - if !ok { + fy := findSurface(resp, "for_you") + if fy == nil { t.Fatal("for_you metrics missing") } if fy.Plays != 3 || fy.Skips != 1 { @@ -102,7 +265,17 @@ func TestRecommendationMetrics_AggregatesBySourceExcludingNull(t *testing.T) { if fy.AvgCompletion < 0.66 || fy.AvgCompletion > 0.67 { t.Errorf("for_you avg_completion = %.4f, want ~0.6667", fy.AvgCompletion) } - if d, ok := bySource["discover"]; !ok || d.Plays != 1 || d.Skips != 0 { - t.Errorf("discover metrics = %+v, want plays=1 skips=0", d) + // Pick-kind attribution surfaces as the For You breakdown (#1249). + if len(fy.Breakdown) != 2 { + t.Fatalf("for_you breakdown rows = %d, want 2 (taste, fresh)", len(fy.Breakdown)) + } + if b := fy.Breakdown[0]; b.Key != "for_you_taste" || b.Plays != 2 || b.Skips != 0 { + t.Errorf("breakdown[0] = %+v, want for_you_taste plays=2 skips=0", b) + } + if b := fy.Breakdown[1]; b.Key != "for_you_fresh" || b.Plays != 1 || b.Skips != 1 { + t.Errorf("breakdown[1] = %+v, want for_you_fresh plays=1 skips=1", b) + } + if radio := findSurface(resp, "radio"); radio == nil || radio.Plays != 1 { + t.Errorf("radio family = %+v, want plays=1 (collapsed from radio:)", radio) } } diff --git a/internal/api/radio.go b/internal/api/radio.go index c99a26f2..5aa68f6c 100644 --- a/internal/api/radio.go +++ b/internal/api/radio.go @@ -15,6 +15,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/recommendation" + "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" ) // RadioResponse is the body of GET /api/radio. @@ -100,16 +101,9 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) { } } - weights := recommendation.ScoringWeights{ - BaseWeight: h.recCfg.BaseWeight, - LikeBoost: h.recCfg.LikeBoost, - RecencyWeight: h.recCfg.RecencyWeight, - SkipPenalty: h.recCfg.SkipPenalty, - JitterMagnitude: h.recCfg.JitterMagnitude, - ContextWeight: h.recCfg.ContextWeight, - SimilarityWeight: h.recCfg.SimilarityWeight, - TasteWeight: h.recCfg.TasteWeight, - } + // Scoring weights come from the DB-backed tuning lab (#1250) — + // read per request so an admin change takes effect live. + weights := h.recSettings.Weights(recsettings.ScopeRadio) picks := recommendation.Shuffle(candidates, weights, time.Now().UTC(), h.rng, limit-1) out := make([]TrackRef, 0, len(picks)+1) diff --git a/internal/config/config.go b/internal/config/config.go index dfd42189..dd2e4ac8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -88,20 +88,14 @@ type EventsConfig struct { SkipMaxDurationPlayedMs int `yaml:"skip_max_duration_played_ms"` } -// RecommendationConfig governs the M3 weighted-shuffle scoring (spec §6). -// All weights are operator-tunable; defaults match the spec recommendations. +// RecommendationConfig holds the radio path's operational knobs. The +// scoring WEIGHTS moved to DB-backed admin settings (#1250, +// internal/recsettings) — YAML is bootstrap-only; anything an operator +// tunes lives in the UI with live effect. type RecommendationConfig struct { - BaseWeight float64 `yaml:"base_weight"` - LikeBoost float64 `yaml:"like_boost"` - RecencyWeight float64 `yaml:"recency_weight"` - SkipPenalty float64 `yaml:"skip_penalty"` - JitterMagnitude float64 `yaml:"jitter_magnitude"` - ContextWeight float64 `yaml:"context_weight"` - SimilarityWeight float64 `yaml:"similarity_weight"` - TasteWeight float64 `yaml:"taste_weight"` - RecentlyPlayedHours int `yaml:"recently_played_hours"` - RadioSize int `yaml:"radio_size"` - RadioSizeMax int `yaml:"radio_size_max"` + RecentlyPlayedHours int `yaml:"recently_played_hours"` + RadioSize int `yaml:"radio_size"` + RadioSizeMax int `yaml:"radio_size_max"` } func Default() Config { @@ -120,16 +114,6 @@ func Default() Config { SkipMaxDurationPlayedMs: 30000, }, Recommendation: RecommendationConfig{ - BaseWeight: 1.0, - LikeBoost: 2.0, - RecencyWeight: 1.0, - SkipPenalty: 1.0, - JitterMagnitude: 0.1, - ContextWeight: 2.0, - SimilarityWeight: 2.0, - // Radio is seed-directed (the user picked a direction), so taste - // is a lighter nudge here than in the daily mixes (1.5). - TasteWeight: 1.0, RecentlyPlayedHours: 1, RadioSize: 50, RadioSizeMax: 200, diff --git a/internal/db/dbq/events.sql.go b/internal/db/dbq/events.sql.go index 59f08d04..6db1800c 100644 --- a/internal/db/dbq/events.sql.go +++ b/internal/db/dbq/events.sql.go @@ -54,7 +54,7 @@ func (q *Queries) GetMostRecentPlaySessionForUser(ctx context.Context, userID pg } const getOpenPlayEventForUser = `-- name: GetOpenPlayEventForUser :one -SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source FROM play_events +SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source, pick_kind FROM play_events WHERE user_id = $1 AND ended_at IS NULL ORDER BY started_at DESC LIMIT 1 @@ -79,12 +79,13 @@ func (q *Queries) GetOpenPlayEventForUser(ctx context.Context, userID pgtype.UUI &i.SessionVectorAtPlay, &i.ScrobbledAt, &i.Source, + &i.PickKind, ) return i, err } const getPlayEventByID = `-- name: GetPlayEventByID :one -SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source FROM play_events WHERE id = $1 +SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source, pick_kind FROM play_events WHERE id = $1 ` func (q *Queries) GetPlayEventByID(ctx context.Context, id pgtype.UUID) (PlayEvent, error) { @@ -104,15 +105,48 @@ func (q *Queries) GetPlayEventByID(ctx context.Context, id pgtype.UUID) (PlayEve &i.SessionVectorAtPlay, &i.ScrobbledAt, &i.Source, + &i.PickKind, ) return i, err } +const getSystemPickKindForTrack = `-- name: GetSystemPickKindForTrack :one +SELECT pt.pick_kind +FROM playlist_tracks pt +JOIN playlists p ON p.id = pt.playlist_id +WHERE p.user_id = $1 + AND p.system_variant = $2 + AND pt.track_id = $3 +LIMIT 1 +` + +type GetSystemPickKindForTrackParams struct { + UserID pgtype.UUID + SystemVariant *string + TrackID pgtype.UUID +} + +// Looks a track up in the user's CURRENT snapshot of the given system +// variant and returns its pick_kind. Used at play-ingestion time to +// freeze provenance onto the play_event — snapshots rebuild daily, so +// attribution can't be reconstructed at read time (#1249/#1270). No +// row = track not in today's snapshot (caller stores NULL); a row with +// NULL pick_kind = the variant doesn't stamp (yet). songs_like_artist +// is non-singleton (up to 3 mixes/user); a track in two of them takes +// whichever LIMIT 1 hits — acceptable, tier stamps there describe the +// same eligibility ladder. +func (q *Queries) GetSystemPickKindForTrack(ctx context.Context, arg GetSystemPickKindForTrackParams) (*string, error) { + row := q.db.QueryRow(ctx, getSystemPickKindForTrack, arg.UserID, arg.SystemVariant, arg.TrackID) + var pick_kind *string + err := row.Scan(&pick_kind) + return pick_kind, err +} + const insertPlayEvent = `-- name: InsertPlayEvent :one INSERT INTO play_events ( - user_id, track_id, session_id, started_at, client_id, source -) VALUES ($1, $2, $3, $4, $5, $6) -RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source + user_id, track_id, session_id, started_at, client_id, source, pick_kind +) VALUES ($1, $2, $3, $4, $5, $6, $7::text) +RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source, pick_kind ` type InsertPlayEventParams struct { @@ -122,8 +156,12 @@ type InsertPlayEventParams struct { StartedAt pgtype.Timestamptz ClientID *string Source *string + PickKind *string } +// pick_kind is non-NULL only for system-playlist plays whose track was +// found (with a stamped kind) in the user's live snapshot for that +// variant at ingestion time (#1249, generalized in #1270). func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams) (PlayEvent, error) { row := q.db.QueryRow(ctx, insertPlayEvent, arg.UserID, @@ -132,6 +170,7 @@ func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams arg.StartedAt, arg.ClientID, arg.Source, + arg.PickKind, ) var i PlayEvent err := row.Scan( @@ -148,6 +187,7 @@ func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams &i.SessionVectorAtPlay, &i.ScrobbledAt, &i.Source, + &i.PickKind, ) return i, err } @@ -291,7 +331,7 @@ SET ended_at = $2, completion_ratio = $4, was_skipped = $5 WHERE id = $1 -RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source +RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source, pick_kind ` type UpdatePlayEventEndedParams struct { @@ -328,6 +368,7 @@ func (q *Queries) UpdatePlayEventEnded(ctx context.Context, arg UpdatePlayEventE &i.SessionVectorAtPlay, &i.ScrobbledAt, &i.Source, + &i.PickKind, ) return i, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index bdf64e72..29528972 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -383,6 +383,7 @@ type PlayEvent struct { SessionVectorAtPlay []byte ScrobbledAt pgtype.Timestamptz Source *string + PickKind *string } type PlaySession struct { @@ -433,6 +434,28 @@ type PlaylistTrack struct { AlbumTitle string DurationSec int32 AddedAt pgtype.Timestamptz + PickKind *string +} + +type RecommendationTuningAudit struct { + ID int64 + ChangedAt pgtype.Timestamptz + Scope string + Action string + Changes []byte +} + +type RecommendationWeightProfile struct { + Profile string + BaseWeight float64 + LikeBoost float64 + RecencyWeight float64 + SkipPenalty float64 + JitterMagnitude float64 + ContextWeight float64 + SimilarityWeight float64 + TasteWeight float64 + UpdatedAt pgtype.Timestamptz } type RegistrationSetting struct { @@ -523,6 +546,15 @@ type TasteProfileTag struct { UpdatedAt pgtype.Timestamptz } +type TasteTuning struct { + Singleton bool + HalfLifeDays float64 + EngagementHardSkip float64 + EngagementNeutral float64 + EngagementFull float64 + UpdatedAt pgtype.Timestamptz +} + type Track struct { ID pgtype.UUID Title string diff --git a/internal/db/dbq/playlists.sql.go b/internal/db/dbq/playlists.sql.go index 71825f80..e4a9855e 100644 --- a/internal/db/dbq/playlists.sql.go +++ b/internal/db/dbq/playlists.sql.go @@ -12,7 +12,7 @@ import ( ) const appendPlaylistTrack = `-- name: AppendPlaylistTrack :one -INSERT INTO playlist_tracks (playlist_id, position, track_id, title, artist_name, album_title, duration_sec) +INSERT INTO playlist_tracks (playlist_id, position, track_id, title, artist_name, album_title, duration_sec, pick_kind) SELECT $1::uuid, COALESCE((SELECT MAX(position) + 1 FROM playlist_tracks WHERE playlist_id = $1::uuid), 0), @@ -20,24 +20,27 @@ SELECT t.title, artists.name, albums.title, - (t.duration_ms / 1000)::integer + (t.duration_ms / 1000)::integer, + $2::text FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id -WHERE t.id = $2::uuid -RETURNING playlist_id, position, track_id, title, artist_name, album_title, duration_sec, added_at +WHERE t.id = $3::uuid +RETURNING playlist_id, position, track_id, title, artist_name, album_title, duration_sec, added_at, pick_kind ` type AppendPlaylistTrackParams struct { PlaylistID pgtype.UUID + PickKind *string TrackID pgtype.UUID } // Inserts at the next available position. Snapshot fields are copied // from the tracks/albums/artists join at insert time. tracks.duration_ms -// is converted to seconds for the snapshot. +// is converted to seconds for the snapshot. pick_kind is NULL for manual +// playlists; the For You builder stamps 'taste'/'fresh' (#1249). func (q *Queries) AppendPlaylistTrack(ctx context.Context, arg AppendPlaylistTrackParams) (PlaylistTrack, error) { - row := q.db.QueryRow(ctx, appendPlaylistTrack, arg.PlaylistID, arg.TrackID) + row := q.db.QueryRow(ctx, appendPlaylistTrack, arg.PlaylistID, arg.PickKind, arg.TrackID) var i PlaylistTrack err := row.Scan( &i.PlaylistID, @@ -48,6 +51,7 @@ func (q *Queries) AppendPlaylistTrack(ctx context.Context, arg AppendPlaylistTra &i.AlbumTitle, &i.DurationSec, &i.AddedAt, + &i.PickKind, ) return i, err } @@ -257,7 +261,7 @@ func (q *Queries) ListAllPlaylistTracksForCollage(ctx context.Context, arg ListA } const listPlaylistTracks = `-- name: ListPlaylistTracks :many -SELECT pt.playlist_id, pt.position, pt.track_id, pt.title, pt.artist_name, pt.album_title, pt.duration_sec, pt.added_at, +SELECT pt.playlist_id, pt.position, pt.track_id, pt.title, pt.artist_name, pt.album_title, pt.duration_sec, pt.added_at, pt.pick_kind, t.id AS live_track_id, albums.id AS album_id, artists.id AS artist_id @@ -278,6 +282,7 @@ type ListPlaylistTracksRow struct { AlbumTitle string DurationSec int32 AddedAt pgtype.Timestamptz + PickKind *string LiveTrackID pgtype.UUID AlbumID pgtype.UUID ArtistID pgtype.UUID @@ -305,6 +310,7 @@ func (q *Queries) ListPlaylistTracks(ctx context.Context, playlistID pgtype.UUID &i.AlbumTitle, &i.DurationSec, &i.AddedAt, + &i.PickKind, &i.LiveTrackID, &i.AlbumID, &i.ArtistID, diff --git a/internal/db/dbq/recommendation_metrics.sql.go b/internal/db/dbq/recommendation_metrics.sql.go index db1f6e77..daee78a4 100644 --- a/internal/db/dbq/recommendation_metrics.sql.go +++ b/internal/db/dbq/recommendation_metrics.sql.go @@ -12,19 +12,17 @@ import ( ) const recommendationSourceMetricsForUser = `-- name: RecommendationSourceMetricsForUser :many - SELECT pe.source, + pe.pick_kind, count(*)::bigint AS plays, count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips, - COALESCE( - avg(pe.completion_ratio) FILTER (WHERE pe.completion_ratio IS NOT NULL), - 0)::float8 AS avg_completion + count(pe.completion_ratio)::bigint AS completion_n, + COALESCE(avg(pe.completion_ratio), 0)::float8 AS avg_completion FROM play_events pe WHERE pe.user_id = $1 - AND pe.source IS NOT NULL AND pe.started_at > now() - ($2::float8 * INTERVAL '1 day') -GROUP BY pe.source +GROUP BY pe.source, pe.pick_kind ORDER BY plays DESC ` @@ -35,18 +33,17 @@ type RecommendationSourceMetricsForUserParams struct { type RecommendationSourceMetricsForUserRow struct { Source *string + PickKind *string Plays int64 Skips int64 + CompletionN int64 AvgCompletion float64 } -// Recommendation observability (#796 phase 4). Per-source play outcomes so the -// operator can see whether each recommendation surface is landing and tune the -// taste weights. Source is stamped on play_events when a play is launched from -// a system-playlist surface ('for_you' | 'discover' | the discovery mixes); -// NULL for library / radio / user-playlist plays, which are excluded here. // $1 user_id, $2 window_days. plays/skips are counts; avg_completion is the -// mean completion ratio over plays that recorded one (0 when none did). +// mean completion ratio over the completion_n plays that recorded one. +// pick_kind splits For You plays into taste/fresh/unattributed (#1249); +// it is NULL for every other source, so those still group to one row. func (q *Queries) RecommendationSourceMetricsForUser(ctx context.Context, arg RecommendationSourceMetricsForUserParams) ([]RecommendationSourceMetricsForUserRow, error) { rows, err := q.db.Query(ctx, recommendationSourceMetricsForUser, arg.UserID, arg.Column2) if err != nil { @@ -58,8 +55,10 @@ func (q *Queries) RecommendationSourceMetricsForUser(ctx context.Context, arg Re var i RecommendationSourceMetricsForUserRow if err := rows.Scan( &i.Source, + &i.PickKind, &i.Plays, &i.Skips, + &i.CompletionN, &i.AvgCompletion, ); err != nil { return nil, err @@ -71,3 +70,83 @@ func (q *Queries) RecommendationSourceMetricsForUser(ctx context.Context, arg Re } return items, nil } + +const recommendationWeeklyTrends = `-- name: RecommendationWeeklyTrends :many + +SELECT + date_trunc('week', pe.started_at)::date AS week_start, + pe.source, + count(*)::bigint AS plays, + count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips, + count(pe.completion_ratio)::bigint AS completion_n, + COALESCE(avg(pe.completion_ratio), 0)::float8 AS avg_completion, + count(*) FILTER (WHERE tpa.artist_id IS NOT NULL)::bigint AS taste_hits +FROM play_events pe +JOIN tracks t ON t.id = pe.track_id +LEFT JOIN taste_profile_artists tpa + ON tpa.user_id = pe.user_id + AND tpa.artist_id = t.artist_id + AND tpa.weight > 0 +WHERE pe.started_at > now() - ($1::int * INTERVAL '1 week') +GROUP BY 1, 2 +ORDER BY 1, 2 +` + +type RecommendationWeeklyTrendsRow struct { + WeekStart pgtype.Date + Source *string + Plays int64 + Skips int64 + CompletionN int64 + AvgCompletion float64 + TasteHits int64 +} + +// Recommendation observability (#796 phase 4). Per-source play outcomes so the +// operator can see whether each recommendation surface is landing and tune the +// taste weights. Source is stamped on play_events when a play is launched from +// a recommendation surface; NULL means the user picked the track manually — +// those rows are INCLUDED here as the baseline control group the surfaces are +// judged against (milestone #127: delta-vs-baseline is what makes the numbers +// actionable). Raw source strings are bucketed into stable surface families in +// the Go handler; completion_n is carried so family merges can weight +// avg_completion correctly. +// Weekly per-source outcome series for the tuning lab's trend view +// (#1251). Aggregated across ALL users: the tuning knobs are global, +// so judging a knob turn needs global outcomes — rows carry rates +// only, no track or user identity. NULL-source (manual) rows are +// included as the baseline family. +// +// taste_hits counts plays whose track's artist has a positive weight +// in that user's CURRENT taste profile — the "cheap recompute" option: +// retroactive over the whole window, at the cost of drift (the profile +// is today's, the play may be weeks old). Good enough to read whether +// a surface is feeding taste-fitting tracks. +// $1 window in weeks. +func (q *Queries) RecommendationWeeklyTrends(ctx context.Context, weeks int32) ([]RecommendationWeeklyTrendsRow, error) { + rows, err := q.db.Query(ctx, recommendationWeeklyTrends, weeks) + if err != nil { + return nil, err + } + defer rows.Close() + var items []RecommendationWeeklyTrendsRow + for rows.Next() { + var i RecommendationWeeklyTrendsRow + if err := rows.Scan( + &i.WeekStart, + &i.Source, + &i.Plays, + &i.Skips, + &i.CompletionN, + &i.AvgCompletion, + &i.TasteHits, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/db/dbq/recommendation_tuning.sql.go b/internal/db/dbq/recommendation_tuning.sql.go new file mode 100644 index 00000000..d41c0f19 --- /dev/null +++ b/internal/db/dbq/recommendation_tuning.sql.go @@ -0,0 +1,272 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: recommendation_tuning.sql + +package dbq + +import ( + "context" +) + +const getTasteTuning = `-- name: GetTasteTuning :one +SELECT singleton, half_life_days, engagement_hard_skip, engagement_neutral, engagement_full, updated_at FROM taste_tuning WHERE singleton = true +` + +func (q *Queries) GetTasteTuning(ctx context.Context) (TasteTuning, error) { + row := q.db.QueryRow(ctx, getTasteTuning) + var i TasteTuning + err := row.Scan( + &i.Singleton, + &i.HalfLifeDays, + &i.EngagementHardSkip, + &i.EngagementNeutral, + &i.EngagementFull, + &i.UpdatedAt, + ) + return i, err +} + +const insertTuningAudit = `-- name: InsertTuningAudit :exec +INSERT INTO recommendation_tuning_audit (scope, action, changes) +VALUES ($1, $2, $3) +` + +type InsertTuningAuditParams struct { + Scope string + Action string + Changes []byte +} + +// changes is a jsonb array of {field, old, new} objects. +func (q *Queries) InsertTuningAudit(ctx context.Context, arg InsertTuningAuditParams) error { + _, err := q.db.Exec(ctx, insertTuningAudit, arg.Scope, arg.Action, arg.Changes) + return err +} + +const listTuningAudit = `-- name: ListTuningAudit :many +SELECT id, changed_at, scope, action, changes + FROM recommendation_tuning_audit + ORDER BY changed_at DESC, id DESC + LIMIT $1 +` + +// Newest first; consumed by the metrics trend view (#1251) to annotate +// knob turns on the timeline. +func (q *Queries) ListTuningAudit(ctx context.Context, limit int32) ([]RecommendationTuningAudit, error) { + rows, err := q.db.Query(ctx, listTuningAudit, limit) + if err != nil { + return nil, err + } + defer rows.Close() + var items []RecommendationTuningAudit + for rows.Next() { + var i RecommendationTuningAudit + if err := rows.Scan( + &i.ID, + &i.ChangedAt, + &i.Scope, + &i.Action, + &i.Changes, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listWeightProfiles = `-- name: ListWeightProfiles :many +SELECT profile, base_weight, like_boost, recency_weight, skip_penalty, jitter_magnitude, context_weight, similarity_weight, taste_weight, updated_at FROM recommendation_weight_profiles ORDER BY profile +` + +func (q *Queries) ListWeightProfiles(ctx context.Context) ([]RecommendationWeightProfile, error) { + rows, err := q.db.Query(ctx, listWeightProfiles) + if err != nil { + return nil, err + } + defer rows.Close() + var items []RecommendationWeightProfile + for rows.Next() { + var i RecommendationWeightProfile + if err := rows.Scan( + &i.Profile, + &i.BaseWeight, + &i.LikeBoost, + &i.RecencyWeight, + &i.SkipPenalty, + &i.JitterMagnitude, + &i.ContextWeight, + &i.SimilarityWeight, + &i.TasteWeight, + &i.UpdatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const updateTasteTuning = `-- name: UpdateTasteTuning :one +UPDATE taste_tuning + SET half_life_days = $1, + engagement_hard_skip = $2, + engagement_neutral = $3, + engagement_full = $4, + updated_at = now() + WHERE singleton = true +RETURNING singleton, half_life_days, engagement_hard_skip, engagement_neutral, engagement_full, updated_at +` + +type UpdateTasteTuningParams struct { + HalfLifeDays float64 + EngagementHardSkip float64 + EngagementNeutral float64 + EngagementFull float64 +} + +func (q *Queries) UpdateTasteTuning(ctx context.Context, arg UpdateTasteTuningParams) (TasteTuning, error) { + row := q.db.QueryRow(ctx, updateTasteTuning, + arg.HalfLifeDays, + arg.EngagementHardSkip, + arg.EngagementNeutral, + arg.EngagementFull, + ) + var i TasteTuning + err := row.Scan( + &i.Singleton, + &i.HalfLifeDays, + &i.EngagementHardSkip, + &i.EngagementNeutral, + &i.EngagementFull, + &i.UpdatedAt, + ) + return i, err +} + +const updateWeightProfile = `-- name: UpdateWeightProfile :one +UPDATE recommendation_weight_profiles + SET base_weight = $2, + like_boost = $3, + recency_weight = $4, + skip_penalty = $5, + jitter_magnitude = $6, + context_weight = $7, + similarity_weight = $8, + taste_weight = $9, + updated_at = now() + WHERE profile = $1 +RETURNING profile, base_weight, like_boost, recency_weight, skip_penalty, jitter_magnitude, context_weight, similarity_weight, taste_weight, updated_at +` + +type UpdateWeightProfileParams struct { + Profile string + BaseWeight float64 + LikeBoost float64 + RecencyWeight float64 + SkipPenalty float64 + JitterMagnitude float64 + ContextWeight float64 + SimilarityWeight float64 + TasteWeight float64 +} + +func (q *Queries) UpdateWeightProfile(ctx context.Context, arg UpdateWeightProfileParams) (RecommendationWeightProfile, error) { + row := q.db.QueryRow(ctx, updateWeightProfile, + arg.Profile, + arg.BaseWeight, + arg.LikeBoost, + arg.RecencyWeight, + arg.SkipPenalty, + arg.JitterMagnitude, + arg.ContextWeight, + arg.SimilarityWeight, + arg.TasteWeight, + ) + var i RecommendationWeightProfile + err := row.Scan( + &i.Profile, + &i.BaseWeight, + &i.LikeBoost, + &i.RecencyWeight, + &i.SkipPenalty, + &i.JitterMagnitude, + &i.ContextWeight, + &i.SimilarityWeight, + &i.TasteWeight, + &i.UpdatedAt, + ) + return i, err +} + +const upsertTasteTuningDefaults = `-- name: UpsertTasteTuningDefaults :exec +INSERT INTO taste_tuning ( + singleton, half_life_days, engagement_hard_skip, + engagement_neutral, engagement_full +) VALUES (true, $1, $2, $3, $4) +ON CONFLICT (singleton) DO NOTHING +` + +type UpsertTasteTuningDefaultsParams struct { + HalfLifeDays float64 + EngagementHardSkip float64 + EngagementNeutral float64 + EngagementFull float64 +} + +func (q *Queries) UpsertTasteTuningDefaults(ctx context.Context, arg UpsertTasteTuningDefaultsParams) error { + _, err := q.db.Exec(ctx, upsertTasteTuningDefaults, + arg.HalfLifeDays, + arg.EngagementHardSkip, + arg.EngagementNeutral, + arg.EngagementFull, + ) + return err +} + +const upsertWeightProfileDefaults = `-- name: UpsertWeightProfileDefaults :exec + +INSERT INTO recommendation_weight_profiles ( + profile, base_weight, like_boost, recency_weight, skip_penalty, + jitter_magnitude, context_weight, similarity_weight, taste_weight +) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) +ON CONFLICT (profile) DO NOTHING +` + +type UpsertWeightProfileDefaultsParams struct { + Profile string + BaseWeight float64 + LikeBoost float64 + RecencyWeight float64 + SkipPenalty float64 + JitterMagnitude float64 + ContextWeight float64 + SimilarityWeight float64 + TasteWeight float64 +} + +// Recommendation tuning lab queries (#1250). Seeding happens via the +// recsettings boot reconcile; shipped defaults live in Go only. +// Boot reconcile: insert the shipped defaults for a profile if the row +// doesn't exist yet. Never overwrites operator-tuned values. +func (q *Queries) UpsertWeightProfileDefaults(ctx context.Context, arg UpsertWeightProfileDefaultsParams) error { + _, err := q.db.Exec(ctx, upsertWeightProfileDefaults, + arg.Profile, + arg.BaseWeight, + arg.LikeBoost, + arg.RecencyWeight, + arg.SkipPenalty, + arg.JitterMagnitude, + arg.ContextWeight, + arg.SimilarityWeight, + arg.TasteWeight, + ) + return err +} diff --git a/internal/db/dbq/system_mixes.sql.go b/internal/db/dbq/system_mixes.sql.go index 3ea0e5b9..ce98bd19 100644 --- a/internal/db/dbq/system_mixes.sql.go +++ b/internal/db/dbq/system_mixes.sql.go @@ -27,12 +27,20 @@ play_counts AS ( FROM play_events WHERE user_id = $1 AND was_skipped = false GROUP BY track_id +), +skip_counts AS ( + SELECT track_id, COUNT(*) AS c + FROM play_events + WHERE user_id = $1 AND was_skipped = true + GROUP BY track_id ) SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN affinity_artists aa ON aa.artist_id = t.artist_id LEFT JOIN play_counts pc ON pc.track_id = t.id + LEFT JOIN skip_counts sc ON sc.track_id = t.id WHERE COALESCE(pc.c, 0) <= 2 + AND COALESCE(sc.c, 0) < 2 AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id @@ -60,6 +68,11 @@ type ListDeepCutsTracksRow struct { // gives per-play variety on top. // #419 Deep Cuts: low-play tracks (<=2 plays) from artists the user // has liked OR played heavily (>=5 non-skip plays across the artist). +// Tracks the user has skipped twice or more don't qualify (#1257): +// eligibility used to count only unskipped plays, so the most actively +// rejected tracks read as "barely heard" and kept being re-offered. +// Threshold 2 so a single accidental skip doesn't banish a track; +// passive-signal only (no dislike UI, rule #101). // $1 user_id, $2 date string. func (q *Queries) ListDeepCutsTracks(ctx context.Context, arg ListDeepCutsTracksParams) ([]ListDeepCutsTracksRow, error) { rows, err := q.db.Query(ctx, listDeepCutsTracks, arg.UserID, arg.Column2) @@ -82,31 +95,49 @@ func (q *Queries) ListDeepCutsTracks(ctx context.Context, arg ListDeepCutsTracks } const listFirstListensTracks = `-- name: ListFirstListensTracks :many -WITH heard_albums AS ( - SELECT DISTINCT t.album_id - FROM play_events pe JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 +WITH attempted AS ( + SELECT DISTINCT pe.track_id + FROM play_events pe + WHERE pe.user_id = $1 AND pe.duration_played_ms >= 30000 ), -played_artists AS ( +album_attempts AS ( + SELECT t.album_id, + COUNT(a.track_id) AS attempted_count, + COUNT(*) AS track_count + FROM tracks t + LEFT JOIN attempted a ON a.track_id = t.id + GROUP BY t.album_id +), +attempted_artists AS ( SELECT DISTINCT t.artist_id - FROM play_events pe JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 + FROM attempted a JOIN tracks t ON t.id = a.track_id +), +albums_tiered AS ( + SELECT aa.album_id, + CASE + WHEN aa.attempted_count = 0 THEN 1 + WHEN aa.attempted_count::float / GREATEST(aa.track_count, 1) <= 0.25 THEN 2 + END AS tier + FROM album_attempts aa ) -SELECT t.id, t.album_id, t.artist_id +SELECT t.id, t.album_id, t.artist_id, alt.tier::int AS tier FROM tracks t JOIN albums al ON al.id = t.album_id - WHERE NOT EXISTS (SELECT 1 FROM heard_albums h WHERE h.album_id = al.id) + JOIN albums_tiered alt ON alt.album_id = al.id + WHERE alt.tier IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM attempted a WHERE a.track_id = t.id) AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) ORDER BY + alt.tier, (CASE WHEN EXISTS ( SELECT 1 FROM general_likes_artists gla WHERE gla.user_id = $1 AND gla.artist_id = al.artist_id ) THEN 0 - WHEN al.artist_id IN (SELECT artist_id FROM played_artists) THEN 1 + WHEN al.artist_id IN (SELECT artist_id FROM attempted_artists) THEN 1 ELSE 2 END), al.id, t.disc_number NULLS FIRST, t.track_number NULLS FIRST @@ -117,12 +148,25 @@ type ListFirstListensTracksRow struct { ID pgtype.UUID AlbumID pgtype.UUID ArtistID pgtype.UUID + Tier int32 } -// #423 First Listens: albums the user has never played any track of. -// Tiered: liked-artist albums first, then played-artist albums, then -// the rest — album-coherent within each tier. Not diversity-capped -// (whole-album discovery). $1 user_id. +// #423 First Listens: songs the user has never heard or even +// attempted. "Attempted" is track-level with a >=30s listen threshold +// (#1268, operator decision 2026-07-03) — the old version disqualified +// a whole album on ANY play_event, so a 2-second accidental brush +// banished it permanently. +// +// tier 1 the exact desire: albums with ZERO attempted tracks +// tier 2 step back: barely-attempted albums (<=25% of tracks +// reached 30s) — brushed, never explored. The already- +// attempted tracks themselves are excluded; they're not +// first listens. +// +// Within each tier, affinity-ordered as before: liked artist, then +// attempted-played artist (>=30s — skip-only contact isn't trust), +// then the rest; album-coherent. The tier lands on +// playlist_tracks.pick_kind via the producer. $1 user_id. func (q *Queries) ListFirstListensTracks(ctx context.Context, userID pgtype.UUID) ([]ListFirstListensTracksRow, error) { rows, err := q.db.Query(ctx, listFirstListensTracks, userID) if err != nil { @@ -132,7 +176,12 @@ func (q *Queries) ListFirstListensTracks(ctx context.Context, userID pgtype.UUID var items []ListFirstListensTracksRow for rows.Next() { var i ListFirstListensTracksRow - if err := rows.Scan(&i.ID, &i.AlbumID, &i.ArtistID); err != nil { + if err := rows.Scan( + &i.ID, + &i.AlbumID, + &i.ArtistID, + &i.Tier, + ); err != nil { return nil, err } items = append(items, i) @@ -144,39 +193,76 @@ func (q *Queries) ListFirstListensTracks(ctx context.Context, userID pgtype.UUID } const listNewForYouTracks = `-- name: ListNewForYouTracks :many -WITH affinity_artists AS ( - SELECT artist_id FROM general_likes_artists WHERE user_id = $1 +WITH attempted AS ( + SELECT DISTINCT pe.track_id + FROM play_events pe + WHERE pe.user_id = $1 AND pe.duration_played_ms >= 30000 +), +consumed_albums AS ( + SELECT DISTINCT t.album_id + FROM attempted a JOIN tracks t ON t.id = a.track_id +), +affinity_artists AS ( + SELECT gla.artist_id FROM general_likes_artists gla WHERE gla.user_id = $1 UNION SELECT t.artist_id - FROM play_events pe JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 AND pe.was_skipped = false + FROM attempted a JOIN tracks t ON t.id = a.track_id GROUP BY t.artist_id HAVING COUNT(*) >= 3 +), +albums_tiered AS ( + SELECT al.id AS album_id, al.created_at, + CASE + WHEN al.created_at >= now() - interval '30 days' + AND al.artist_id IN (SELECT artist_id FROM affinity_artists) THEN 1 + WHEN al.artist_id IN (SELECT artist_id FROM affinity_artists) THEN 2 + ELSE 3 + END AS tier + FROM albums al + WHERE al.created_at >= now() - interval '90 days' + AND al.id NOT IN (SELECT album_id FROM consumed_albums) ) -SELECT t.id, t.album_id, t.artist_id +SELECT t.id, t.album_id, t.artist_id, alt.tier::int AS tier FROM tracks t - JOIN albums al ON al.id = t.album_id - JOIN affinity_artists aa ON aa.artist_id = al.artist_id - WHERE al.created_at >= now() - interval '30 days' - AND NOT EXISTS ( + JOIN albums_tiered alt ON alt.album_id = t.album_id + WHERE NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) - ORDER BY al.created_at DESC, t.disc_number NULLS FIRST, t.track_number NULLS FIRST - LIMIT 200 + ORDER BY alt.tier, alt.created_at DESC, t.album_id, + t.disc_number NULLS FIRST, t.track_number NULLS FIRST + LIMIT 300 ` type ListNewForYouTracksRow struct { ID pgtype.UUID AlbumID pgtype.UUID ArtistID pgtype.UUID + Tier int32 } -// #421 New for you: tracks from albums added in the last 30 days -// whose artist the user has liked OR played (>=3 non-skip). Album- -// coherent (newest album first, then disc/track) — the producer does -// NOT diversity-cap these; they're meant as whole-album discovery. -// $1 user_id. +// #421 New for you, tiered per project rule #131 (#1267). The old +// single hard rule (added <30d AND direct affinity) had two failure +// modes with one root: no notion of consumption — the album you +// devoured in week one crowded the mix for three more weeks, and +// after a quiet month nothing qualified at all. +// +// "Consumed" = any track attempted for >=30s (the First Listens +// threshold, #1268): once you've meaningfully engaged, the album is +// no longer news. Applies to every tier — played albums leave the +// mix the next build. +// +// tier 1 the exact desire: unconsumed albums added <30d by +// direct-affinity artists (liked, or >=3 attempted plays) +// tier 2 step back a little: unconsumed affinity albums from the +// wider 30-90d window — added while you weren't looking +// tier 3 step back more: any unconsumed album added <90d +// regardless of affinity, newest first +// +// Album-coherent within tiers (newest album first, then disc/track); +// the producer fills top-down and rotates within tier blocks. The +// tier lands on playlist_tracks.pick_kind so metrics can price what +// each step-back trades away. $1 user_id. func (q *Queries) ListNewForYouTracks(ctx context.Context, userID pgtype.UUID) ([]ListNewForYouTracksRow, error) { rows, err := q.db.Query(ctx, listNewForYouTracks, userID) if err != nil { @@ -186,7 +272,12 @@ func (q *Queries) ListNewForYouTracks(ctx context.Context, userID pgtype.UUID) ( var items []ListNewForYouTracksRow for rows.Next() { var i ListNewForYouTracksRow - if err := rows.Scan(&i.ID, &i.AlbumID, &i.ArtistID); err != nil { + if err := rows.Scan( + &i.ID, + &i.AlbumID, + &i.ArtistID, + &i.Tier, + ); err != nil { return nil, err } items = append(items, i) @@ -203,7 +294,10 @@ WITH windowed AS ( FROM play_events WHERE user_id = $1 AND was_skipped = false AND started_at < now() - interval '30 days' - AND ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM now())) <= 10 + AND LEAST( + ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM $2::date)), + 365 - ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM $2::date)) + ) <= 10 GROUP BY track_id ) SELECT t.id, t.album_id, t.artist_id @@ -235,6 +329,13 @@ type ListOnThisDayTracksRow struct { // and window ±7→±10 so it surfaces on a months-old library instead // of needing a full year of history; still skips cleanly (no rows → // no playlist) when there's no qualifying history yet. +// Day-of-year distance is circular (#1256): plain ABS made Dec 28 vs +// Jan 3 read as 359 days apart, silently gutting the window for ~3 +// weeks around every New Year — precisely when holiday nostalgia is +// the point. LEAST(d, 365-d) wraps the boundary; leap-year drift of +// ±1 is absorbed by the ±10 window. The build date ($2, already here +// for the md5 rotation) anchors "today" instead of now() so the +// window is testable and consistent with the mix's daily determinism. // $1 user_id, $2 date string. func (q *Queries) ListOnThisDayTracks(ctx context.Context, arg ListOnThisDayTracksParams) ([]ListOnThisDayTracksRow, error) { rows, err := q.db.Query(ctx, listOnThisDayTracks, arg.UserID, arg.Column2) @@ -262,37 +363,32 @@ WITH stats AS ( FROM play_events pe WHERE pe.user_id = $1 AND pe.was_skipped = false GROUP BY pe.track_id -), -deep AS ( - SELECT t.id, t.album_id, t.artist_id, s.c, 0 AS tier - FROM tracks t - JOIN stats s ON s.track_id = t.id - WHERE s.c >= 5 - AND s.last_at <= now() - interval '6 months' - AND NOT EXISTS ( - SELECT 1 FROM lidarr_quarantine q - WHERE q.user_id = $1 AND q.track_id = t.id - ) -), -shallow AS ( - SELECT t.id, t.album_id, t.artist_id, s.c, 1 AS tier - FROM tracks t - JOIN stats s ON s.track_id = t.id - WHERE s.c >= 5 - AND s.last_at <= now() - interval '30 days' - AND NOT EXISTS ( - SELECT 1 FROM lidarr_quarantine q - WHERE q.user_id = $1 AND q.track_id = t.id - ) ) -SELECT id, album_id, artist_id - FROM ( - SELECT id, album_id, artist_id, c, tier FROM deep - UNION ALL - SELECT id, album_id, artist_id, c, tier FROM shallow - WHERE NOT EXISTS (SELECT 1 FROM deep) - ) u - ORDER BY tier, c DESC, id +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN stats s ON s.track_id = t.id + WHERE s.c >= 3 + AND s.last_at <= now() - interval '30 days' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + -- Declined-resurfacing cooldown (#1258): a skip on a + -- rediscover-sourced play is the user explicitly saying "I've moved + -- on" to the exact invitation this mix extends — such tracks sit + -- out ~90 days instead of re-qualifying the next day. Passive + -- signal only (rule #101). + AND NOT EXISTS ( + SELECT 1 FROM play_events rp + WHERE rp.user_id = $1 + AND rp.track_id = t.id + AND rp.source = 'rediscover' + AND rp.was_skipped = true + AND rp.started_at > now() - interval '90 days' + ) + ORDER BY (s.last_at <= now() - interval '6 months') DESC, + (s.c >= 5) DESC, + s.c DESC, t.id LIMIT 200 ` @@ -302,13 +398,17 @@ type ListRediscoverTracksRow struct { ArtistID pgtype.UUID } -// #420 Rediscover: tracks the user played a lot (>=5 non-skip) but -// has drifted away from. Tiered so a young library still gets a mix: -// -// tier 0 not played in the last 6 months (true rediscovery) -// tier 1 (only if tier 0 empty) not played in the last 30 days -// -// Ordered by historical affection. $1 user_id. +// #420 Rediscover: tracks the user played a lot but has drifted away +// from. One blended pool (issue #1246: the old two-tier UNION was +// all-or-nothing — a single ">=6 months cold" row suppressed the whole +// ">=30 days" tier, which is how a one-song playlist shipped; the 6mo +// tier was also a strict subset of the 30d tier). Eligibility is >=3 +// non-skip plays and >=30 days cold; the bar sits at 3 rather than 5 +// so a weeks-old library still fields a pool — on young histories the +// >=5-play tracks are precisely the ones still in rotation. Ordering +// prefers true rediscoveries (>=6 months cold), then strong affection +// (>=5 plays), then raw play count; the producer's minimum floor +// decides whether the pool is big enough to ship at all. $1 user_id. func (q *Queries) ListRediscoverTracks(ctx context.Context, userID pgtype.UUID) ([]ListRediscoverTracksRow, error) { rows, err := q.db.Query(ctx, listRediscoverTracks, userID) if err != nil { diff --git a/internal/db/dbq/system_playlists.sql.go b/internal/db/dbq/system_playlists.sql.go index aa1c41cb..4f8d1a86 100644 --- a/internal/db/dbq/system_playlists.sql.go +++ b/internal/db/dbq/system_playlists.sql.go @@ -290,9 +290,13 @@ func (q *Queries) ListPlaylistsByUserAndKind(ctx context.Context, arg ListPlayli } const pickSeedArtists = `-- name: PickSeedArtists :many -WITH plays AS ( +WITH liked AS ( + SELECT gla.artist_id FROM general_likes_artists gla WHERE gla.user_id = $1 +), +recent7 AS ( SELECT t.artist_id, - COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count + COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, + 0 AS tier FROM play_events pe JOIN tracks t ON t.id = pe.track_id WHERE pe.user_id = $1 @@ -300,27 +304,79 @@ WITH plays AS ( AND t.artist_id IS NOT NULL GROUP BY t.artist_id ), -liked AS ( - SELECT artist_id FROM general_likes_artists WHERE user_id = $1 +recent30 AS ( + SELECT t.artist_id, + COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, + 1 AS tier + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 + AND pe.started_at > now() - INTERVAL '30 days' + AND t.artist_id IS NOT NULL + GROUP BY t.artist_id +), +alltime AS ( + SELECT t.artist_id, + COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, + 2 AS tier + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 + AND t.artist_id IS NOT NULL + GROUP BY t.artist_id +), +likedonly AS ( + SELECT l.artist_id, 0::bigint AS play_count, 3 AS tier + FROM liked l +), +chosen AS ( + SELECT artist_id, play_count, tier FROM recent7 + UNION ALL + SELECT artist_id, play_count, tier FROM recent30 + WHERE NOT EXISTS (SELECT 1 FROM recent7) + UNION ALL + SELECT artist_id, play_count, tier FROM alltime + WHERE NOT EXISTS (SELECT 1 FROM recent7) + AND NOT EXISTS (SELECT 1 FROM recent30) + UNION ALL + SELECT artist_id, play_count, tier FROM likedonly + WHERE NOT EXISTS (SELECT 1 FROM recent7) + AND NOT EXISTS (SELECT 1 FROM recent30) + AND NOT EXISTS (SELECT 1 FROM alltime) ) -SELECT p.artist_id, - (p.play_count + CASE WHEN l.artist_id IS NOT NULL THEN 5 ELSE 0 END)::bigint AS score - FROM plays p - LEFT JOIN liked l ON l.artist_id = p.artist_id - ORDER BY score DESC, p.artist_id +SELECT c.artist_id, + (c.play_count + CASE WHEN l.artist_id IS NOT NULL THEN 5 ELSE 0 END)::bigint AS score, + c.tier::int AS tier + FROM chosen c + LEFT JOIN liked l ON l.artist_id = c.artist_id + ORDER BY score DESC, c.artist_id LIMIT 5 ` type PickSeedArtistsRow struct { ArtistID pgtype.UUID Score int64 + Tier int32 } -// Top-5 most-engaged distinct artist candidates in the user's last 7 -// days. The Go-side picker (pickSeedArtistsForDay) shuffles these -// daily-deterministically and takes the first 3 so the set of -// "Songs like X" mixes rotates day-to-day. -// Score = unskipped-play count + 5 if user has liked the artist. +// Top-5 most-engaged distinct artist candidates, tiered so the +// "Songs like X" mixes never silently vanish (#1255): the old hard +// 7-day window emptied the seed pool after a quiet week, and the +// daily atomic-replace build then deleted every existing mix until +// the user played something again. Same fallback shape as +// PickTopPlayedTracksForUser (For You's seeds): +// +// tier 0 engagement in the last 7 days +// tier 1 (only if tier 0 empty) last 30 days +// tier 2 (only if tiers 0+1 empty) all-time +// tier 3 (only if 0-2 empty) liked artists with no play history +// +// All returned rows share one tier; produceSeedMixes maps it onto the +// rule-#131 pick-kind ladder and stamps the built tracks, so metrics +// can compare mixes seeded from fresh vs stale engagement. +// Score = unskipped-play count + 5 if user has liked the artist. The +// Go-side picker (pickSeedArtistsForDay) shuffles the 5 +// daily-deterministically and takes 3 so the mix set rotates. func (q *Queries) PickSeedArtists(ctx context.Context, userID pgtype.UUID) ([]PickSeedArtistsRow, error) { rows, err := q.db.Query(ctx, pickSeedArtists, userID) if err != nil { @@ -330,7 +386,7 @@ func (q *Queries) PickSeedArtists(ctx context.Context, userID pgtype.UUID) ([]Pi var items []PickSeedArtistsRow for rows.Next() { var i PickSeedArtistsRow - if err := rows.Scan(&i.ArtistID, &i.Score); err != nil { + if err := rows.Scan(&i.ArtistID, &i.Score, &i.Tier); err != nil { return nil, err } items = append(items, i) @@ -457,10 +513,10 @@ SELECT id // tier 2 (only if tiers 0+1 empty) liked tracks // // Returns up to 5 ids; tie-break by track_id for determinism. The -// Go-side picker (pickForYouSeedForDay) rotates one per day via -// userIDHash. Widened from a hard 7-day window, which made For-You -// disappear after a week of not listening and never recover on a -// self-hosted library with sparse history. +// Go-side picker (pickDailySeeds) draws the day's seeds from these. +// Widened from a hard 7-day window, which made For-You disappear +// after a week of not listening and never recover on a self-hosted +// library with sparse history. func (q *Queries) PickTopPlayedTracksForUser(ctx context.Context, userID pgtype.UUID) ([]pgtype.UUID, error) { rows, err := q.db.Query(ctx, pickTopPlayedTracksForUser, userID) if err != nil { diff --git a/internal/db/migrations/0038_for_you_pick_kind.down.sql b/internal/db/migrations/0038_for_you_pick_kind.down.sql new file mode 100644 index 00000000..7158bd2f --- /dev/null +++ b/internal/db/migrations/0038_for_you_pick_kind.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE play_events DROP COLUMN pick_kind; +ALTER TABLE playlist_tracks DROP COLUMN pick_kind; diff --git a/internal/db/migrations/0038_for_you_pick_kind.up.sql b/internal/db/migrations/0038_for_you_pick_kind.up.sql new file mode 100644 index 00000000..fdf86a39 --- /dev/null +++ b/internal/db/migrations/0038_for_you_pick_kind.up.sql @@ -0,0 +1,34 @@ +-- For You exploration attribution (milestone #127 step 2, Scribe #1249). +-- +-- For You is deliberately composed of two populations: a head of +-- top-scored similarity matches ("taste" picks) and a tail sampled from +-- deeper in the candidate ranking ("fresh" picks — the freshness +-- injection). Until now the metrics could only judge For You as one +-- blob, so a high skip rate couldn't be read as "the taste engine is +-- missing" vs "the freshness tax is too high". +-- +-- Two denormalized columns, both nullable: +-- +-- 1. playlist_tracks.pick_kind — stamped by the system-playlist builder +-- at snapshot build time. NULL for manual playlists and for system +-- variants that don't split (Discover, Songs like X, discovery +-- mixes). System snapshots are atomically replaced daily, so the +-- column never goes stale within a snapshot. +-- +-- 2. play_events.pick_kind — stamped at play-ingestion time by looking +-- the track up in the user's CURRENT For You snapshot. Attribution +-- must be frozen at play time: For You rebuilds daily, so a 30-day +-- metrics window spans ~30 snapshots and a join-at-read against the +-- live snapshot would misattribute nearly everything. Plays that +-- predate this feature (or whose track already rotated out, e.g. a +-- late offline replay) stay NULL → reported as unattributed. + +ALTER TABLE playlist_tracks + ADD COLUMN pick_kind text + CONSTRAINT playlist_tracks_pick_kind_check + CHECK (pick_kind IN ('taste', 'fresh')); + +ALTER TABLE play_events + ADD COLUMN pick_kind text + CONSTRAINT play_events_pick_kind_check + CHECK (pick_kind IN ('taste', 'fresh')); diff --git a/internal/db/migrations/0039_pick_kind_vocabulary.down.sql b/internal/db/migrations/0039_pick_kind_vocabulary.down.sql new file mode 100644 index 00000000..eb447d38 --- /dev/null +++ b/internal/db/migrations/0039_pick_kind_vocabulary.down.sql @@ -0,0 +1,19 @@ +-- Rows stamped with the widened vocabulary must be cleared before the +-- narrow CHECK can be re-added; NULL reads as "unattributed", which is +-- the honest downgrade. +UPDATE play_events SET pick_kind = NULL + WHERE pick_kind NOT IN ('taste', 'fresh'); +UPDATE playlist_tracks SET pick_kind = NULL + WHERE pick_kind NOT IN ('taste', 'fresh'); + +ALTER TABLE play_events + DROP CONSTRAINT play_events_pick_kind_check; +ALTER TABLE play_events + ADD CONSTRAINT play_events_pick_kind_check + CHECK (pick_kind IN ('taste', 'fresh')); + +ALTER TABLE playlist_tracks + DROP CONSTRAINT playlist_tracks_pick_kind_check; +ALTER TABLE playlist_tracks + ADD CONSTRAINT playlist_tracks_pick_kind_check + CHECK (pick_kind IN ('taste', 'fresh')); diff --git a/internal/db/migrations/0039_pick_kind_vocabulary.up.sql b/internal/db/migrations/0039_pick_kind_vocabulary.up.sql new file mode 100644 index 00000000..36f619d2 --- /dev/null +++ b/internal/db/migrations/0039_pick_kind_vocabulary.up.sql @@ -0,0 +1,41 @@ +-- Provenance as standard (milestone #127, Scribe #1270). +-- +-- #1249 introduced pick_kind as a For You one-off ('taste'/'fresh'). +-- The mechanism — stamp WHY a track is in the snapshot at build time, +-- freeze it onto the play at ingestion, break it down in metrics — is +-- now the standard for every system mix, so the CHECK vocabulary +-- widens to cover: +-- +-- taste / fresh For You head vs freshness-injection tail +-- dormant / cross_user / random +-- Discover's three candidate buckets — makes +-- the 40/30/30 allocation measurable instead +-- of a guess +-- tier1 / tier2 / tier3 the tiered-eligibility ladder (project rule +-- #131): tier 1 pins the mix's exact desire, +-- each higher tier steps back a little. Tier +-- provenance is how we measure what the +-- step-back trades away. +-- +-- Postgres CHECK whitelists can't be altered in place: DROP + re-ADD +-- with the expanded list, in the same migration. + +ALTER TABLE playlist_tracks + DROP CONSTRAINT playlist_tracks_pick_kind_check; +ALTER TABLE playlist_tracks + ADD CONSTRAINT playlist_tracks_pick_kind_check + CHECK (pick_kind IN ( + 'taste', 'fresh', + 'dormant', 'cross_user', 'random', + 'tier1', 'tier2', 'tier3' + )); + +ALTER TABLE play_events + DROP CONSTRAINT play_events_pick_kind_check; +ALTER TABLE play_events + ADD CONSTRAINT play_events_pick_kind_check + CHECK (pick_kind IN ( + 'taste', 'fresh', + 'dormant', 'cross_user', 'random', + 'tier1', 'tier2', 'tier3' + )); diff --git a/internal/db/migrations/0040_recommendation_tuning.down.sql b/internal/db/migrations/0040_recommendation_tuning.down.sql new file mode 100644 index 00000000..9575d0eb --- /dev/null +++ b/internal/db/migrations/0040_recommendation_tuning.down.sql @@ -0,0 +1,3 @@ +DROP TABLE recommendation_tuning_audit; +DROP TABLE taste_tuning; +DROP TABLE recommendation_weight_profiles; diff --git a/internal/db/migrations/0040_recommendation_tuning.up.sql b/internal/db/migrations/0040_recommendation_tuning.up.sql new file mode 100644 index 00000000..7aba7c30 --- /dev/null +++ b/internal/db/migrations/0040_recommendation_tuning.up.sql @@ -0,0 +1,57 @@ +-- Recommendation tuning lab (milestone #127, Scribe #1250). +-- +-- The scoring weights move out of YAML (radio profile) and out of the +-- systemMixWeights hard-code (daily_mix profile) into DB-backed +-- settings with live effect — the defaults-discovery lab (decision +-- #1247): the operator turns knobs here to FIND good values, which +-- then get baked into shipped defaults; end users and other operators +-- should never need this card. +-- +-- Rows are seeded by the recsettings service's boot reconcile (the +-- coverart SettingsService pattern), not by this migration, so shipped +-- defaults live in exactly one place (Go). + +CREATE TABLE recommendation_weight_profiles ( + profile text PRIMARY KEY + CONSTRAINT recommendation_weight_profiles_profile_check + CHECK (profile IN ('radio', 'daily_mix')), + base_weight double precision NOT NULL, + like_boost double precision NOT NULL, + recency_weight double precision NOT NULL, + skip_penalty double precision NOT NULL, + jitter_magnitude double precision NOT NULL, + context_weight double precision NOT NULL, + similarity_weight double precision NOT NULL, + taste_weight double precision NOT NULL, + updated_at timestamptz NOT NULL DEFAULT now() +); + +-- Taste-profile build knobs: engagement half-life + the completion→ +-- engagement curve points (taste.Config's tunable subset). +CREATE TABLE taste_tuning ( + singleton boolean PRIMARY KEY DEFAULT true + CONSTRAINT taste_tuning_singleton_check CHECK (singleton), + half_life_days double precision NOT NULL, + engagement_hard_skip double precision NOT NULL, + engagement_neutral double precision NOT NULL, + engagement_full double precision NOT NULL, + updated_at timestamptz NOT NULL DEFAULT now() +); + +-- Every knob turn writes one audit row; the metrics trend view +-- (#1251) annotates these on its timeline so cause→effect is visible +-- after a change. changes is a jsonb array of {field, old, new}. +CREATE TABLE recommendation_tuning_audit ( + id bigserial PRIMARY KEY, + changed_at timestamptz NOT NULL DEFAULT now(), + scope text NOT NULL + CONSTRAINT recommendation_tuning_audit_scope_check + CHECK (scope IN ('radio', 'daily_mix', 'taste')), + action text NOT NULL + CONSTRAINT recommendation_tuning_audit_action_check + CHECK (action IN ('update', 'reset')), + changes jsonb NOT NULL +); + +CREATE INDEX recommendation_tuning_audit_changed_at_idx + ON recommendation_tuning_audit (changed_at DESC); diff --git a/internal/db/queries/events.sql b/internal/db/queries/events.sql index db0d4829..565b510c 100644 --- a/internal/db/queries/events.sql +++ b/internal/db/queries/events.sql @@ -24,11 +24,32 @@ ORDER BY started_at DESC LIMIT 1; -- name: InsertPlayEvent :one +-- pick_kind is non-NULL only for system-playlist plays whose track was +-- found (with a stamped kind) in the user's live snapshot for that +-- variant at ingestion time (#1249, generalized in #1270). INSERT INTO play_events ( - user_id, track_id, session_id, started_at, client_id, source -) VALUES ($1, $2, $3, $4, $5, $6) + user_id, track_id, session_id, started_at, client_id, source, pick_kind +) VALUES ($1, $2, $3, $4, $5, $6, sqlc.narg(pick_kind)::text) RETURNING *; +-- name: GetSystemPickKindForTrack :one +-- Looks a track up in the user's CURRENT snapshot of the given system +-- variant and returns its pick_kind. Used at play-ingestion time to +-- freeze provenance onto the play_event — snapshots rebuild daily, so +-- attribution can't be reconstructed at read time (#1249/#1270). No +-- row = track not in today's snapshot (caller stores NULL); a row with +-- NULL pick_kind = the variant doesn't stamp (yet). songs_like_artist +-- is non-singleton (up to 3 mixes/user); a track in two of them takes +-- whichever LIMIT 1 hits — acceptable, tier stamps there describe the +-- same eligibility ladder. +SELECT pt.pick_kind +FROM playlist_tracks pt +JOIN playlists p ON p.id = pt.playlist_id +WHERE p.user_id = $1 + AND p.system_variant = $2 + AND pt.track_id = $3 +LIMIT 1; + -- name: UpdatePlayEventEnded :one -- Closes a play_event by id with the given ended_at, duration, and skip flag. -- completion_ratio is computed from duration_played_ms and the track duration diff --git a/internal/db/queries/playlists.sql b/internal/db/queries/playlists.sql index 349e1940..058a1ffa 100644 --- a/internal/db/queries/playlists.sql +++ b/internal/db/queries/playlists.sql @@ -68,8 +68,9 @@ ORDER BY pt.position; -- name: AppendPlaylistTrack :one -- Inserts at the next available position. Snapshot fields are copied -- from the tracks/albums/artists join at insert time. tracks.duration_ms --- is converted to seconds for the snapshot. -INSERT INTO playlist_tracks (playlist_id, position, track_id, title, artist_name, album_title, duration_sec) +-- is converted to seconds for the snapshot. pick_kind is NULL for manual +-- playlists; the For You builder stamps 'taste'/'fresh' (#1249). +INSERT INTO playlist_tracks (playlist_id, position, track_id, title, artist_name, album_title, duration_sec, pick_kind) SELECT sqlc.arg(playlist_id)::uuid, COALESCE((SELECT MAX(position) + 1 FROM playlist_tracks WHERE playlist_id = sqlc.arg(playlist_id)::uuid), 0), @@ -77,7 +78,8 @@ SELECT t.title, artists.name, albums.title, - (t.duration_ms / 1000)::integer + (t.duration_ms / 1000)::integer, + sqlc.narg(pick_kind)::text FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id diff --git a/internal/db/queries/recommendation_metrics.sql b/internal/db/queries/recommendation_metrics.sql index c7d04159..48d66565 100644 --- a/internal/db/queries/recommendation_metrics.sql +++ b/internal/db/queries/recommendation_metrics.sql @@ -1,22 +1,58 @@ -- Recommendation observability (#796 phase 4). Per-source play outcomes so the -- operator can see whether each recommendation surface is landing and tune the -- taste weights. Source is stamped on play_events when a play is launched from --- a system-playlist surface ('for_you' | 'discover' | the discovery mixes); --- NULL for library / radio / user-playlist plays, which are excluded here. +-- a recommendation surface; NULL means the user picked the track manually — +-- those rows are INCLUDED here as the baseline control group the surfaces are +-- judged against (milestone #127: delta-vs-baseline is what makes the numbers +-- actionable). Raw source strings are bucketed into stable surface families in +-- the Go handler; completion_n is carried so family merges can weight +-- avg_completion correctly. --- name: RecommendationSourceMetricsForUser :many --- $1 user_id, $2 window_days. plays/skips are counts; avg_completion is the --- mean completion ratio over plays that recorded one (0 when none did). +-- name: RecommendationWeeklyTrends :many +-- Weekly per-source outcome series for the tuning lab's trend view +-- (#1251). Aggregated across ALL users: the tuning knobs are global, +-- so judging a knob turn needs global outcomes — rows carry rates +-- only, no track or user identity. NULL-source (manual) rows are +-- included as the baseline family. +-- +-- taste_hits counts plays whose track's artist has a positive weight +-- in that user's CURRENT taste profile — the "cheap recompute" option: +-- retroactive over the whole window, at the cost of drift (the profile +-- is today's, the play may be weeks old). Good enough to read whether +-- a surface is feeding taste-fitting tracks. +-- $1 window in weeks. SELECT + date_trunc('week', pe.started_at)::date AS week_start, pe.source, count(*)::bigint AS plays, count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips, - COALESCE( - avg(pe.completion_ratio) FILTER (WHERE pe.completion_ratio IS NOT NULL), - 0)::float8 AS avg_completion + count(pe.completion_ratio)::bigint AS completion_n, + COALESCE(avg(pe.completion_ratio), 0)::float8 AS avg_completion, + count(*) FILTER (WHERE tpa.artist_id IS NOT NULL)::bigint AS taste_hits +FROM play_events pe +JOIN tracks t ON t.id = pe.track_id +LEFT JOIN taste_profile_artists tpa + ON tpa.user_id = pe.user_id + AND tpa.artist_id = t.artist_id + AND tpa.weight > 0 +WHERE pe.started_at > now() - (sqlc.arg(weeks)::int * INTERVAL '1 week') +GROUP BY 1, 2 +ORDER BY 1, 2; + +-- name: RecommendationSourceMetricsForUser :many +-- $1 user_id, $2 window_days. plays/skips are counts; avg_completion is the +-- mean completion ratio over the completion_n plays that recorded one. +-- pick_kind splits For You plays into taste/fresh/unattributed (#1249); +-- it is NULL for every other source, so those still group to one row. +SELECT + pe.source, + pe.pick_kind, + count(*)::bigint AS plays, + count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips, + count(pe.completion_ratio)::bigint AS completion_n, + COALESCE(avg(pe.completion_ratio), 0)::float8 AS avg_completion FROM play_events pe WHERE pe.user_id = $1 - AND pe.source IS NOT NULL AND pe.started_at > now() - ($2::float8 * INTERVAL '1 day') -GROUP BY pe.source +GROUP BY pe.source, pe.pick_kind ORDER BY plays DESC; diff --git a/internal/db/queries/recommendation_tuning.sql b/internal/db/queries/recommendation_tuning.sql new file mode 100644 index 00000000..760ea716 --- /dev/null +++ b/internal/db/queries/recommendation_tuning.sql @@ -0,0 +1,61 @@ +-- Recommendation tuning lab queries (#1250). Seeding happens via the +-- recsettings boot reconcile; shipped defaults live in Go only. + +-- name: UpsertWeightProfileDefaults :exec +-- Boot reconcile: insert the shipped defaults for a profile if the row +-- doesn't exist yet. Never overwrites operator-tuned values. +INSERT INTO recommendation_weight_profiles ( + profile, base_weight, like_boost, recency_weight, skip_penalty, + jitter_magnitude, context_weight, similarity_weight, taste_weight +) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) +ON CONFLICT (profile) DO NOTHING; + +-- name: ListWeightProfiles :many +SELECT * FROM recommendation_weight_profiles ORDER BY profile; + +-- name: UpdateWeightProfile :one +UPDATE recommendation_weight_profiles + SET base_weight = $2, + like_boost = $3, + recency_weight = $4, + skip_penalty = $5, + jitter_magnitude = $6, + context_weight = $7, + similarity_weight = $8, + taste_weight = $9, + updated_at = now() + WHERE profile = $1 +RETURNING *; + +-- name: UpsertTasteTuningDefaults :exec +INSERT INTO taste_tuning ( + singleton, half_life_days, engagement_hard_skip, + engagement_neutral, engagement_full +) VALUES (true, $1, $2, $3, $4) +ON CONFLICT (singleton) DO NOTHING; + +-- name: GetTasteTuning :one +SELECT * FROM taste_tuning WHERE singleton = true; + +-- name: UpdateTasteTuning :one +UPDATE taste_tuning + SET half_life_days = $1, + engagement_hard_skip = $2, + engagement_neutral = $3, + engagement_full = $4, + 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) +VALUES ($1, $2, $3); + +-- name: ListTuningAudit :many +-- Newest first; consumed by the metrics trend view (#1251) to annotate +-- knob turns on the timeline. +SELECT id, changed_at, scope, action, changes + FROM recommendation_tuning_audit + ORDER BY changed_at DESC, id DESC + LIMIT $1; diff --git a/internal/db/queries/system_mixes.sql b/internal/db/queries/system_mixes.sql index b483fad0..7503a122 100644 --- a/internal/db/queries/system_mixes.sql +++ b/internal/db/queries/system_mixes.sql @@ -8,6 +8,11 @@ -- name: ListDeepCutsTracks :many -- #419 Deep Cuts: low-play tracks (<=2 plays) from artists the user -- has liked OR played heavily (>=5 non-skip plays across the artist). +-- Tracks the user has skipped twice or more don't qualify (#1257): +-- eligibility used to count only unskipped plays, so the most actively +-- rejected tracks read as "barely heard" and kept being re-offered. +-- Threshold 2 so a single accidental skip doesn't banish a track; +-- passive-signal only (no dislike UI, rule #101). -- $1 user_id, $2 date string. WITH affinity_artists AS ( SELECT artist_id FROM general_likes_artists WHERE user_id = $1 @@ -23,12 +28,20 @@ play_counts AS ( FROM play_events WHERE user_id = $1 AND was_skipped = false GROUP BY track_id +), +skip_counts AS ( + SELECT track_id, COUNT(*) AS c + FROM play_events + WHERE user_id = $1 AND was_skipped = true + GROUP BY track_id ) SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN affinity_artists aa ON aa.artist_id = t.artist_id LEFT JOIN play_counts pc ON pc.track_id = t.id + LEFT JOIN skip_counts sc ON sc.track_id = t.id WHERE COALESCE(pc.c, 0) <= 2 + AND COALESCE(sc.c, 0) < 2 AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id @@ -37,76 +50,113 @@ SELECT t.id, t.album_id, t.artist_id LIMIT 200; -- name: ListRediscoverTracks :many --- #420 Rediscover: tracks the user played a lot (>=5 non-skip) but --- has drifted away from. Tiered so a young library still gets a mix: --- tier 0 not played in the last 6 months (true rediscovery) --- tier 1 (only if tier 0 empty) not played in the last 30 days --- Ordered by historical affection. $1 user_id. +-- #420 Rediscover: tracks the user played a lot but has drifted away +-- from. One blended pool (issue #1246: the old two-tier UNION was +-- all-or-nothing — a single ">=6 months cold" row suppressed the whole +-- ">=30 days" tier, which is how a one-song playlist shipped; the 6mo +-- tier was also a strict subset of the 30d tier). Eligibility is >=3 +-- non-skip plays and >=30 days cold; the bar sits at 3 rather than 5 +-- so a weeks-old library still fields a pool — on young histories the +-- >=5-play tracks are precisely the ones still in rotation. Ordering +-- prefers true rediscoveries (>=6 months cold), then strong affection +-- (>=5 plays), then raw play count; the producer's minimum floor +-- decides whether the pool is big enough to ship at all. $1 user_id. WITH stats AS ( SELECT pe.track_id, COUNT(*) AS c, MAX(pe.started_at) AS last_at FROM play_events pe WHERE pe.user_id = $1 AND pe.was_skipped = false GROUP BY pe.track_id -), -deep AS ( - SELECT t.id, t.album_id, t.artist_id, s.c, 0 AS tier - FROM tracks t - JOIN stats s ON s.track_id = t.id - WHERE s.c >= 5 - AND s.last_at <= now() - interval '6 months' - AND NOT EXISTS ( - SELECT 1 FROM lidarr_quarantine q - WHERE q.user_id = $1 AND q.track_id = t.id - ) -), -shallow AS ( - SELECT t.id, t.album_id, t.artist_id, s.c, 1 AS tier - FROM tracks t - JOIN stats s ON s.track_id = t.id - WHERE s.c >= 5 - AND s.last_at <= now() - interval '30 days' - AND NOT EXISTS ( - SELECT 1 FROM lidarr_quarantine q - WHERE q.user_id = $1 AND q.track_id = t.id - ) -) -SELECT id, album_id, artist_id - FROM ( - SELECT id, album_id, artist_id, c, tier FROM deep - UNION ALL - SELECT id, album_id, artist_id, c, tier FROM shallow - WHERE NOT EXISTS (SELECT 1 FROM deep) - ) u - ORDER BY tier, c DESC, id - LIMIT 200; - --- name: ListNewForYouTracks :many --- #421 New for you: tracks from albums added in the last 30 days --- whose artist the user has liked OR played (>=3 non-skip). Album- --- coherent (newest album first, then disc/track) — the producer does --- NOT diversity-cap these; they're meant as whole-album discovery. --- $1 user_id. -WITH affinity_artists AS ( - SELECT artist_id FROM general_likes_artists WHERE user_id = $1 - UNION - SELECT t.artist_id - FROM play_events pe JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 AND pe.was_skipped = false - GROUP BY t.artist_id - HAVING COUNT(*) >= 3 ) SELECT t.id, t.album_id, t.artist_id FROM tracks t - JOIN albums al ON al.id = t.album_id - JOIN affinity_artists aa ON aa.artist_id = al.artist_id - WHERE al.created_at >= now() - interval '30 days' + JOIN stats s ON s.track_id = t.id + WHERE s.c >= 3 + AND s.last_at <= now() - interval '30 days' AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) - ORDER BY al.created_at DESC, t.disc_number NULLS FIRST, t.track_number NULLS FIRST + -- Declined-resurfacing cooldown (#1258): a skip on a + -- rediscover-sourced play is the user explicitly saying "I've moved + -- on" to the exact invitation this mix extends — such tracks sit + -- out ~90 days instead of re-qualifying the next day. Passive + -- signal only (rule #101). + AND NOT EXISTS ( + SELECT 1 FROM play_events rp + WHERE rp.user_id = $1 + AND rp.track_id = t.id + AND rp.source = 'rediscover' + AND rp.was_skipped = true + AND rp.started_at > now() - interval '90 days' + ) + ORDER BY (s.last_at <= now() - interval '6 months') DESC, + (s.c >= 5) DESC, + s.c DESC, t.id LIMIT 200; +-- name: ListNewForYouTracks :many +-- #421 New for you, tiered per project rule #131 (#1267). The old +-- single hard rule (added <30d AND direct affinity) had two failure +-- modes with one root: no notion of consumption — the album you +-- devoured in week one crowded the mix for three more weeks, and +-- after a quiet month nothing qualified at all. +-- +-- "Consumed" = any track attempted for >=30s (the First Listens +-- threshold, #1268): once you've meaningfully engaged, the album is +-- no longer news. Applies to every tier — played albums leave the +-- mix the next build. +-- +-- tier 1 the exact desire: unconsumed albums added <30d by +-- direct-affinity artists (liked, or >=3 attempted plays) +-- tier 2 step back a little: unconsumed affinity albums from the +-- wider 30-90d window — added while you weren't looking +-- tier 3 step back more: any unconsumed album added <90d +-- regardless of affinity, newest first +-- +-- Album-coherent within tiers (newest album first, then disc/track); +-- the producer fills top-down and rotates within tier blocks. The +-- tier lands on playlist_tracks.pick_kind so metrics can price what +-- each step-back trades away. $1 user_id. +WITH attempted AS ( + SELECT DISTINCT pe.track_id + FROM play_events pe + WHERE pe.user_id = $1 AND pe.duration_played_ms >= 30000 +), +consumed_albums AS ( + SELECT DISTINCT t.album_id + FROM attempted a JOIN tracks t ON t.id = a.track_id +), +affinity_artists AS ( + SELECT gla.artist_id FROM general_likes_artists gla WHERE gla.user_id = $1 + UNION + SELECT t.artist_id + FROM attempted a JOIN tracks t ON t.id = a.track_id + GROUP BY t.artist_id + HAVING COUNT(*) >= 3 +), +albums_tiered AS ( + SELECT al.id AS album_id, al.created_at, + CASE + WHEN al.created_at >= now() - interval '30 days' + AND al.artist_id IN (SELECT artist_id FROM affinity_artists) THEN 1 + WHEN al.artist_id IN (SELECT artist_id FROM affinity_artists) THEN 2 + ELSE 3 + END AS tier + FROM albums al + WHERE al.created_at >= now() - interval '90 days' + AND al.id NOT IN (SELECT album_id FROM consumed_albums) +) +SELECT t.id, t.album_id, t.artist_id, alt.tier::int AS tier + FROM tracks t + JOIN albums_tiered alt ON alt.album_id = t.album_id + WHERE NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY alt.tier, alt.created_at DESC, t.album_id, + t.disc_number NULLS FIRST, t.track_number NULLS FIRST + LIMIT 300; + -- name: ListOnThisDayTracks :many -- #422 On This Day: tracks the user played around this calendar date -- (±10 day-of-year) in the past, excluding the very recent (>30 days @@ -115,13 +165,23 @@ SELECT t.id, t.album_id, t.artist_id -- and window ±7→±10 so it surfaces on a months-old library instead -- of needing a full year of history; still skips cleanly (no rows → -- no playlist) when there's no qualifying history yet. +-- Day-of-year distance is circular (#1256): plain ABS made Dec 28 vs +-- Jan 3 read as 359 days apart, silently gutting the window for ~3 +-- weeks around every New Year — precisely when holiday nostalgia is +-- the point. LEAST(d, 365-d) wraps the boundary; leap-year drift of +-- ±1 is absorbed by the ±10 window. The build date ($2, already here +-- for the md5 rotation) anchors "today" instead of now() so the +-- window is testable and consistent with the mix's daily determinism. -- $1 user_id, $2 date string. WITH windowed AS ( SELECT track_id, COUNT(*) AS c FROM play_events WHERE user_id = $1 AND was_skipped = false AND started_at < now() - interval '30 days' - AND ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM now())) <= 10 + AND LEAST( + ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM $2::date)), + 365 - ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM $2::date)) + ) <= 10 GROUP BY track_id ) SELECT t.id, t.album_id, t.artist_id @@ -135,35 +195,65 @@ SELECT t.id, t.album_id, t.artist_id LIMIT 200; -- name: ListFirstListensTracks :many --- #423 First Listens: albums the user has never played any track of. --- Tiered: liked-artist albums first, then played-artist albums, then --- the rest — album-coherent within each tier. Not diversity-capped --- (whole-album discovery). $1 user_id. -WITH heard_albums AS ( - SELECT DISTINCT t.album_id - FROM play_events pe JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 +-- #423 First Listens: songs the user has never heard or even +-- attempted. "Attempted" is track-level with a >=30s listen threshold +-- (#1268, operator decision 2026-07-03) — the old version disqualified +-- a whole album on ANY play_event, so a 2-second accidental brush +-- banished it permanently. +-- +-- tier 1 the exact desire: albums with ZERO attempted tracks +-- tier 2 step back: barely-attempted albums (<=25% of tracks +-- reached 30s) — brushed, never explored. The already- +-- attempted tracks themselves are excluded; they're not +-- first listens. +-- +-- Within each tier, affinity-ordered as before: liked artist, then +-- attempted-played artist (>=30s — skip-only contact isn't trust), +-- then the rest; album-coherent. The tier lands on +-- playlist_tracks.pick_kind via the producer. $1 user_id. +WITH attempted AS ( + SELECT DISTINCT pe.track_id + FROM play_events pe + WHERE pe.user_id = $1 AND pe.duration_played_ms >= 30000 ), -played_artists AS ( +album_attempts AS ( + SELECT t.album_id, + COUNT(a.track_id) AS attempted_count, + COUNT(*) AS track_count + FROM tracks t + LEFT JOIN attempted a ON a.track_id = t.id + GROUP BY t.album_id +), +attempted_artists AS ( SELECT DISTINCT t.artist_id - FROM play_events pe JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 + FROM attempted a JOIN tracks t ON t.id = a.track_id +), +albums_tiered AS ( + SELECT aa.album_id, + CASE + WHEN aa.attempted_count = 0 THEN 1 + WHEN aa.attempted_count::float / GREATEST(aa.track_count, 1) <= 0.25 THEN 2 + END AS tier + FROM album_attempts aa ) -SELECT t.id, t.album_id, t.artist_id +SELECT t.id, t.album_id, t.artist_id, alt.tier::int AS tier FROM tracks t JOIN albums al ON al.id = t.album_id - WHERE NOT EXISTS (SELECT 1 FROM heard_albums h WHERE h.album_id = al.id) + JOIN albums_tiered alt ON alt.album_id = al.id + WHERE alt.tier IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM attempted a WHERE a.track_id = t.id) AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) ORDER BY + alt.tier, (CASE WHEN EXISTS ( SELECT 1 FROM general_likes_artists gla WHERE gla.user_id = $1 AND gla.artist_id = al.artist_id ) THEN 0 - WHEN al.artist_id IN (SELECT artist_id FROM played_artists) THEN 1 + WHEN al.artist_id IN (SELECT artist_id FROM attempted_artists) THEN 1 ELSE 2 END), al.id, t.disc_number NULLS FIRST, t.track_number NULLS FIRST diff --git a/internal/db/queries/system_playlists.sql b/internal/db/queries/system_playlists.sql index 305c8b75..4f8a7696 100644 --- a/internal/db/queries/system_playlists.sql +++ b/internal/db/queries/system_playlists.sql @@ -47,14 +47,29 @@ UPDATE system_playlist_runs UPDATE system_playlist_runs SET in_flight = false WHERE in_flight = true; -- name: PickSeedArtists :many --- Top-5 most-engaged distinct artist candidates in the user's last 7 --- days. The Go-side picker (pickSeedArtistsForDay) shuffles these --- daily-deterministically and takes the first 3 so the set of --- "Songs like X" mixes rotates day-to-day. --- Score = unskipped-play count + 5 if user has liked the artist. -WITH plays AS ( +-- Top-5 most-engaged distinct artist candidates, tiered so the +-- "Songs like X" mixes never silently vanish (#1255): the old hard +-- 7-day window emptied the seed pool after a quiet week, and the +-- daily atomic-replace build then deleted every existing mix until +-- the user played something again. Same fallback shape as +-- PickTopPlayedTracksForUser (For You's seeds): +-- tier 0 engagement in the last 7 days +-- tier 1 (only if tier 0 empty) last 30 days +-- tier 2 (only if tiers 0+1 empty) all-time +-- tier 3 (only if 0-2 empty) liked artists with no play history +-- All returned rows share one tier; produceSeedMixes maps it onto the +-- rule-#131 pick-kind ladder and stamps the built tracks, so metrics +-- can compare mixes seeded from fresh vs stale engagement. +-- Score = unskipped-play count + 5 if user has liked the artist. The +-- Go-side picker (pickSeedArtistsForDay) shuffles the 5 +-- daily-deterministically and takes 3 so the mix set rotates. +WITH liked AS ( + SELECT gla.artist_id FROM general_likes_artists gla WHERE gla.user_id = $1 +), +recent7 AS ( SELECT t.artist_id, - COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count + COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, + 0 AS tier FROM play_events pe JOIN tracks t ON t.id = pe.track_id WHERE pe.user_id = $1 @@ -62,14 +77,52 @@ WITH plays AS ( AND t.artist_id IS NOT NULL GROUP BY t.artist_id ), -liked AS ( - SELECT artist_id FROM general_likes_artists WHERE user_id = $1 +recent30 AS ( + SELECT t.artist_id, + COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, + 1 AS tier + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 + AND pe.started_at > now() - INTERVAL '30 days' + AND t.artist_id IS NOT NULL + GROUP BY t.artist_id +), +alltime AS ( + SELECT t.artist_id, + COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, + 2 AS tier + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 + AND t.artist_id IS NOT NULL + GROUP BY t.artist_id +), +likedonly AS ( + SELECT l.artist_id, 0::bigint AS play_count, 3 AS tier + FROM liked l +), +chosen AS ( + SELECT artist_id, play_count, tier FROM recent7 + UNION ALL + SELECT artist_id, play_count, tier FROM recent30 + WHERE NOT EXISTS (SELECT 1 FROM recent7) + UNION ALL + SELECT artist_id, play_count, tier FROM alltime + WHERE NOT EXISTS (SELECT 1 FROM recent7) + AND NOT EXISTS (SELECT 1 FROM recent30) + UNION ALL + SELECT artist_id, play_count, tier FROM likedonly + WHERE NOT EXISTS (SELECT 1 FROM recent7) + AND NOT EXISTS (SELECT 1 FROM recent30) + AND NOT EXISTS (SELECT 1 FROM alltime) ) -SELECT p.artist_id, - (p.play_count + CASE WHEN l.artist_id IS NOT NULL THEN 5 ELSE 0 END)::bigint AS score - FROM plays p - LEFT JOIN liked l ON l.artist_id = p.artist_id - ORDER BY score DESC, p.artist_id +SELECT c.artist_id, + (c.play_count + CASE WHEN l.artist_id IS NOT NULL THEN 5 ELSE 0 END)::bigint AS score, + c.tier::int AS tier + FROM chosen c + LEFT JOIN liked l ON l.artist_id = c.artist_id + ORDER BY score DESC, c.artist_id LIMIT 5; -- name: PickTopPlayedTracksForUser :many @@ -78,10 +131,10 @@ SELECT p.artist_id, -- tier 1 (only if tier 0 empty) all-time top non-skip plays -- tier 2 (only if tiers 0+1 empty) liked tracks -- Returns up to 5 ids; tie-break by track_id for determinism. The --- Go-side picker (pickForYouSeedForDay) rotates one per day via --- userIDHash. Widened from a hard 7-day window, which made For-You --- disappear after a week of not listening and never recover on a --- self-hosted library with sparse history. +-- Go-side picker (pickDailySeeds) draws the day's seeds from these. +-- Widened from a hard 7-day window, which made For-You disappear +-- after a week of not listening and never recover on a self-hosted +-- library with sparse history. WITH recent AS ( SELECT t.id, COUNT(*) AS c, 0 AS tier FROM play_events pe diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go index eba016c9..b8c1465a 100644 --- a/internal/dbtest/reset.go +++ b/internal/dbtest/reset.go @@ -62,6 +62,11 @@ var dataTables = []string{ // (never recreates the singleton, seeded once by 0018); ResetDB // resets its counter via UPDATE below instead. "cover_art_provider_settings", + // recsettings.New reconciles shipped defaults on every construction + // (#1250), so truncating gives each test pristine tuning values. + "recommendation_weight_profiles", + "taste_tuning", + "recommendation_tuning_audit", "tracks", "albums", "artists", diff --git a/internal/playevents/writer.go b/internal/playevents/writer.go index a65ba169..22999945 100644 --- a/internal/playevents/writer.go +++ b/internal/playevents/writer.go @@ -97,6 +97,36 @@ var systemPlaylistSources = map[string]bool{ "songs_like_artist": true, } +// lookupSystemPickKind resolves a system-playlist play's pick_kind +// (taste/fresh, Discover's bucket, or a rule-#131 tier) from the user's +// live snapshot of that variant at ingestion time. Attribution must be +// frozen now — snapshots rebuild daily, so it can't be reconstructed at +// metrics-read time (#1249, generalized in #1270). Source strings +// double as system_variant values (see systemPlaylistSources). Returns +// nil (unattributed) when the track isn't in the current snapshot — +// plays predating the feature, or an offline replay arriving after the +// track rotated out — and when the variant doesn't stamp yet. Real DB +// errors propagate and fail the caller's transaction. +func lookupSystemPickKind( + ctx context.Context, + q *dbq.Queries, + userID, trackID pgtype.UUID, + variant string, +) (*string, error) { + kind, err := q.GetSystemPickKindForTrack(ctx, dbq.GetSystemPickKindForTrackParams{ + UserID: userID, + SystemVariant: &variant, + TrackID: trackID, + }) + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, err + } + return kind, nil +} + // RecordPlayStartedWithSource is RecordPlayStarted plus a `source` // tag identifying which surface the play came from. When source is a // known system-playlist kind the track is appended to that user's @@ -127,6 +157,13 @@ func (w *Writer) RecordPlayStartedWithSource( if source != "" { sourcePtr = &source } + var pickKind *string + if systemPlaylistSources[source] { + pickKind, err = lookupSystemPickKind(ctx, q, userID, trackID, source) + if err != nil { + return err + } + } ev, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ UserID: userID, TrackID: trackID, @@ -134,6 +171,7 @@ func (w *Writer) RecordPlayStartedWithSource( StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, ClientID: clientIDPtr, Source: sourcePtr, + PickKind: pickKind, }) if err != nil { return err @@ -395,6 +433,13 @@ func (w *Writer) RecordOfflinePlay( if source != "" { sourcePtr = &source } + var pickKind *string + if systemPlaylistSources[source] { + pickKind, err = lookupSystemPickKind(ctx, q, userID, trackID, source) + if err != nil { + return err + } + } ev, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ UserID: userID, TrackID: trackID, @@ -402,6 +447,7 @@ func (w *Writer) RecordOfflinePlay( StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, ClientID: clientIDPtr, Source: sourcePtr, + PickKind: pickKind, }) if err != nil { return err diff --git a/internal/playevents/writer_test.go b/internal/playevents/writer_test.go index a0158c11..b6dc1bd6 100644 --- a/internal/playevents/writer_test.go +++ b/internal/playevents/writer_test.go @@ -488,3 +488,123 @@ func TestRecordPlayStarted_NoRotationWithoutSource(t *testing.T) { t.Errorf("expected no rotation row for a source-less play") } } + +// seedSystemSnapshot creates a system playlist of the given variant +// containing the fixture track with the given pick_kind, mimicking +// what the builder persists (#1249, generalized #1270). +func seedSystemSnapshot(t *testing.T, f fixture, variant, pickKind string) { + t.Helper() + pl, err := f.q.CreateSystemPlaylist(context.Background(), dbq.CreateSystemPlaylistParams{ + UserID: f.user, Name: variant, SystemVariant: &variant, + }) + if err != nil { + t.Fatalf("CreateSystemPlaylist: %v", err) + } + if _, err := f.q.AppendPlaylistTrack(context.Background(), dbq.AppendPlaylistTrackParams{ + PlaylistID: pl.ID, TrackID: f.track, PickKind: &pickKind, + }); err != nil { + t.Fatalf("AppendPlaylistTrack: %v", err) + } +} + +func TestRecordPlayStartedWithSource_StampsForYouPickKind(t *testing.T) { + f := newFixture(t, 200_000) + seedSystemSnapshot(t, f, "for_you", "fresh") + res, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "for_you", time.Now().UTC(), + ) + if err != nil { + t.Fatalf("RecordPlayStartedWithSource: %v", err) + } + got, err := f.q.GetPlayEventByID(context.Background(), res.PlayEventID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.PickKind == nil || *got.PickKind != "fresh" { + t.Errorf("pick_kind = %v, want fresh (frozen from live snapshot)", got.PickKind) + } +} + +func TestRecordPlayStartedWithSource_NoSnapshotMatch_PickKindNull(t *testing.T) { + // A for_you play whose track isn't in the current snapshot (rotated + // out, or no snapshot exists) stays unattributed rather than guessing. + f := newFixture(t, 200_000) + res, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "for_you", time.Now().UTC(), + ) + if err != nil { + t.Fatalf("RecordPlayStartedWithSource: %v", err) + } + got, err := f.q.GetPlayEventByID(context.Background(), res.PlayEventID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.PickKind != nil { + t.Errorf("pick_kind = %q, want NULL (track not in snapshot)", *got.PickKind) + } +} + +func TestRecordPlayStartedWithSource_CrossVariant_PickKindNull(t *testing.T) { + // Attribution is variant-scoped: even with a For You snapshot + // containing the track, a play from a different surface must not + // pick up For You's stamp — it reads its OWN variant's snapshot, + // which here doesn't exist. + f := newFixture(t, 200_000) + seedSystemSnapshot(t, f, "for_you", "taste") + res, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "discover", time.Now().UTC(), + ) + if err != nil { + t.Fatalf("RecordPlayStartedWithSource: %v", err) + } + got, err := f.q.GetPlayEventByID(context.Background(), res.PlayEventID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.PickKind != nil { + t.Errorf("pick_kind = %q, want NULL for a cross-variant play", *got.PickKind) + } +} + +func TestRecordPlayStartedWithSource_StampsDiscoverBucket(t *testing.T) { + // Discover stamps its candidate bucket (#1270); a discover play whose + // track is in the live snapshot freezes that bucket onto the play. + f := newFixture(t, 200_000) + seedSystemSnapshot(t, f, "discover", "dormant") + res, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "discover", time.Now().UTC(), + ) + if err != nil { + t.Fatalf("RecordPlayStartedWithSource: %v", err) + } + got, err := f.q.GetPlayEventByID(context.Background(), res.PlayEventID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.PickKind == nil || *got.PickKind != "dormant" { + t.Errorf("pick_kind = %v, want dormant (frozen from live snapshot)", got.PickKind) + } +} + +func TestRecordOfflinePlay_StampsForYouPickKind(t *testing.T) { + // The offline replay path threads source the same way; a replayed + // for_you play whose track is still in the snapshot gets attributed. + f := newFixture(t, 200_000) + seedSystemSnapshot(t, f, "for_you", "taste") + at := time.Now().UTC().Add(-time.Hour) + if err := f.w.RecordOfflinePlay( + context.Background(), f.user, f.track, "c", "for_you", at, 180_000, + ); err != nil { + t.Fatalf("RecordOfflinePlay: %v", err) + } + var pickKind *string + if err := f.pool.QueryRow(context.Background(), + `SELECT pick_kind FROM play_events WHERE user_id = $1 AND track_id = $2`, + f.user, f.track, + ).Scan(&pickKind); err != nil { + t.Fatalf("select: %v", err) + } + if pickKind == nil || *pickKind != "taste" { + t.Errorf("pick_kind = %v, want taste", pickKind) + } +} diff --git a/internal/playlists/discover.go b/internal/playlists/discover.go index 65f41d48..809e33fb 100644 --- a/internal/playlists/discover.go +++ b/internal/playlists/discover.go @@ -20,11 +20,15 @@ const ( // discoverTrack is the common shape used by the bucket allocator. The // three sqlc-generated row types collapse into this internal struct so -// downstream functions don't need to be generic over them. +// downstream functions don't need to be generic over them. PickKind is +// the originating bucket (dormant/cross_user/random, #1270), stamped in +// the row adapters so it survives the interleave — bucket identity is +// what makes the 40/30/30 allocation measurable in the metrics. type discoverTrack struct { ID pgtype.UUID AlbumID pgtype.UUID ArtistID pgtype.UUID + PickKind string } // buildDiscoverCandidates assembles the Discover playlist track list. @@ -91,7 +95,7 @@ func buildDiscoverCandidates(ctx context.Context, q *dbq.Queries, logger *slog.L // already gave us the ranking. ranked := make([]rankedCandidate, 0, len(out)) for _, t := range out { - ranked = append(ranked, rankedCandidate{TrackID: t.ID}) + ranked = append(ranked, rankedCandidate{TrackID: t.ID, PickKind: t.PickKind}) } return ranked, nil } @@ -99,7 +103,10 @@ func buildDiscoverCandidates(ctx context.Context, q *dbq.Queries, logger *slog.L func dormantRowsToTracks(rows []dbq.ListDormantArtistTracksForDiscoverRow) []discoverTrack { out := make([]discoverTrack, len(rows)) for i, r := range rows { - out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + out[i] = discoverTrack{ + ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID, + PickKind: pickKindDormant, + } } return out } @@ -107,7 +114,10 @@ func dormantRowsToTracks(rows []dbq.ListDormantArtistTracksForDiscoverRow) []dis func crossUserRowsToTracks(rows []dbq.ListCrossUserLikedTracksForDiscoverRow) []discoverTrack { out := make([]discoverTrack, len(rows)) for i, r := range rows { - out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + out[i] = discoverTrack{ + ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID, + PickKind: pickKindCrossUser, + } } return out } @@ -115,7 +125,10 @@ func crossUserRowsToTracks(rows []dbq.ListCrossUserLikedTracksForDiscoverRow) [] func randomRowsToTracks(rows []dbq.ListRandomUnheardTracksForDiscoverRow) []discoverTrack { out := make([]discoverTrack, len(rows)) for i, r := range rows { - out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + out[i] = discoverTrack{ + ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID, + PickKind: pickKindRandom, + } } return out } diff --git a/internal/playlists/discover_test.go b/internal/playlists/discover_test.go index e299cb11..fc1d54a8 100644 --- a/internal/playlists/discover_test.go +++ b/internal/playlists/discover_test.go @@ -161,3 +161,27 @@ func TestInterleaveBuckets_DedupsAcrossBuckets(t *testing.T) { } } } + +func TestInterleaveBuckets_PreservesBucketPickKind(t *testing.T) { + // Bucket provenance (#1270) is stamped on discoverTrack before the + // interleave and must survive it — including dedup, where a track in + // two buckets keeps the stamp of the bucket it was taken from. + dormant := []discoverTrack{ + {ID: uuidN(1), PickKind: pickKindDormant}, + {ID: uuidN(2), PickKind: pickKindDormant}, + } + random := []discoverTrack{ + {ID: uuidN(1), PickKind: pickKindRandom}, // shared with dormant + {ID: uuidN(3), PickKind: pickKindRandom}, + } + got := interleaveBuckets(dormant, nil, random) + if len(got) != 3 { + t.Fatalf("len = %d, want 3", len(got)) + } + want := map[byte]string{1: pickKindDormant, 2: pickKindDormant, 3: pickKindRandom} + for _, tr := range got { + if w := want[tr.ID.Bytes[15]]; tr.PickKind != w { + t.Errorf("track %d pick_kind = %q, want %q", tr.ID.Bytes[15], tr.PickKind, w) + } + } +} diff --git a/internal/playlists/foryou_test.go b/internal/playlists/foryou_test.go index 0d69a7ae..2a102f9e 100644 --- a/internal/playlists/foryou_test.go +++ b/internal/playlists/foryou_test.go @@ -99,7 +99,7 @@ func TestPickHeadAndTail_SmallPool(t *testing.T) { makeCand(2, 11, 101, 0.9), makeCand(3, 12, 102, 0.8), } - got := pickHeadAndTail(in, testUserID, "2026-05-07", time.Now(), 5, 2) + got := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", time.Now(), 5, 2) if len(got) != 3 { t.Errorf("len = %d, want 3 (pool too small for head/tail split)", len(got)) } @@ -112,7 +112,7 @@ func TestPickHeadAndTail_ExactlyTotal(t *testing.T) { for i := 0; i < 7; i++ { in = append(in, makeCand(i+1, i+1, i+1, float64(7-i))) } - got := pickHeadAndTail(in, testUserID, "2026-05-07", time.Now(), 5, 2) + got := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", time.Now(), 5, 2) if len(got) != 7 { t.Errorf("len = %d, want 7 (pool == total)", len(got)) } @@ -126,7 +126,7 @@ func TestPickHeadAndTail_HeadAndTailSplit(t *testing.T) { for i := 0; i < 100; i++ { in = append(in, makeCand(i+1, i+1, i+1, float64(100-i))) } - got := pickHeadAndTail(in, testUserID, "2026-05-07", time.Now(), 20, 5) + got := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", time.Now(), 20, 5) if len(got) != 25 { t.Errorf("len = %d, want 25 (20 head + 5 tail)", len(got)) } @@ -139,8 +139,8 @@ func TestPickHeadAndTail_Determinism(t *testing.T) { in = append(in, makeCand(i+1, i+1, i+1, float64(100-i))) } now := time.Now() - got1 := pickHeadAndTail(in, testUserID, "2026-05-07", now, 20, 5) - got2 := pickHeadAndTail(in, testUserID, "2026-05-07", now, 20, 5) + got1 := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", now, 20, 5) + got2 := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", now, 20, 5) if len(got1) != len(got2) { t.Fatalf("len mismatch: %d vs %d", len(got1), len(got2)) } @@ -161,8 +161,8 @@ func TestPickHeadAndTail_HeadStable_TailVariesAcrossDays(t *testing.T) { in = append(in, makeCand(i+1, i+1, i+1, float64(100-i))) } now := time.Now() - day1 := pickHeadAndTail(in, testUserID, "2026-05-07", now, 20, 5) - day2 := pickHeadAndTail(in, testUserID, "2026-05-08", now, 20, 5) + day1 := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", now, 20, 5) + day2 := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-08", now, 20, 5) if len(day1) != 25 || len(day2) != 25 { t.Fatalf("len mismatch: day1=%d day2=%d", len(day1), len(day2)) } @@ -205,7 +205,7 @@ func TestPickHeadAndTail_TailFromBeyond2xHeadN(t *testing.T) { sim := float64(50 - i) in = append(in, makeCand(i+1, i+1, i+1, sim)) } - got := pickHeadAndTail(in, testUserID, "2026-05-07", time.Now(), 5, 3) + got := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", time.Now(), 5, 3) if len(got) != 8 { t.Fatalf("len = %d, want 8 (5 head + 3 tail)", len(got)) } @@ -243,3 +243,190 @@ func TestPickTopN_DiversityCap(t *testing.T) { t.Errorf("len = %d, want 3 (artist 100 capped at 3)", len(got)) } } + +func TestPickHeadAndTail_MarksPickKinds(t *testing.T) { + // 100-deep pool with headN=20, tailN=5: the 20 head entries are the + // taste picks, the 5 tail entries the freshness injection (#1249). + in := make([]recommendation.Candidate, 0, 100) + for i := 0; i < 100; i++ { + in = append(in, makeCand(i+1, i+1, i+1, float64(100-i))) + } + got := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", time.Now(), 20, 5) + if len(got) != 25 { + t.Fatalf("len = %d, want 25", len(got)) + } + for i := 0; i < 20; i++ { + if got[i].PickKind != pickKindTaste { + t.Errorf("head[%d].PickKind = %q, want %q", i, got[i].PickKind, pickKindTaste) + } + } + for i := 20; i < 25; i++ { + if got[i].PickKind != pickKindFresh { + t.Errorf("tail[%d].PickKind = %q, want %q", i-20, got[i].PickKind, pickKindFresh) + } + } +} + +func TestPickHeadAndTail_SmallPoolAllTaste(t *testing.T) { + // The small-pool fallback is pure top-N-by-score — that IS the taste + // mechanism, so nothing on this path is an exploration pick. + in := []recommendation.Candidate{ + makeCand(1, 10, 100, 1.0), + makeCand(2, 11, 101, 0.9), + makeCand(3, 12, 102, 0.8), + } + got := pickHeadAndTail(in, nil, 1, testUserID, "2026-05-07", time.Now(), 5, 2) + for i, rc := range got { + if rc.PickKind != pickKindTaste { + t.Errorf("got[%d].PickKind = %q, want %q (fallback is all taste)", + i, rc.PickKind, pickKindTaste) + } + } +} + +func TestPickTopN_NoPickKind(t *testing.T) { + // Songs-like-X and the discovery mixes don't split; their rows must + // persist pick_kind NULL (empty string here). + in := []recommendation.Candidate{makeCand(1, 10, 100, 1.0)} + got := pickTopN(in, testUserID, "2026-05-07", time.Now(), 25) + if len(got) != 1 || got[0].PickKind != "" { + t.Errorf("pickTopN PickKind = %q, want empty (persists as NULL)", got[0].PickKind) + } +} + +func TestHeadQuotas(t *testing.T) { + cases := []struct { + seeds, headN int + want []int + }{ + {1, 50, []int{50}}, + {2, 50, []int{30, 20}}, + {3, 50, []int{25, 15, 10}}, + {3, 7, []int{4, 2, 1}}, // rounding remainder → primary seed + } + for _, c := range cases { + got := headQuotas(c.seeds, c.headN) + if len(got) != len(c.want) { + t.Fatalf("headQuotas(%d, %d) len = %d, want %d", c.seeds, c.headN, len(got), len(c.want)) + } + sum := 0 + for i := range got { + sum += got[i] + if got[i] != c.want[i] { + t.Errorf("headQuotas(%d, %d)[%d] = %d, want %d", c.seeds, c.headN, i, got[i], c.want[i]) + } + } + if sum != c.headN { + t.Errorf("headQuotas(%d, %d) sums to %d, want %d", c.seeds, c.headN, sum, c.headN) + } + } +} + +func TestPickQuotaHead_BlendsSeeds(t *testing.T) { + // Seed 0's neighborhood out-scores everything (#1269): without + // quotas the head would be all seed-0. With headN=10 and quotas + // [5,3,2], the walk takes the best 5 from seed 0, then seeds 1 and + // 2 get their guaranteed slots from deeper ranks. + in := make([]recommendation.Candidate, 0, 30) + seedOf := map[pgtype.UUID]int{} + for i := 0; i < 30; i++ { + c := makeCand(i+1, i+1, i+1, float64(30-i)) + si := 0 + if i >= 20 && i < 25 { + si = 1 + } else if i >= 25 { + si = 2 + } + seedOf[c.Track.ID] = si + in = append(in, c) + } + // Input is already score-descending and cap-free. + head := pickQuotaHead(in, seedOf, 3, 10) + if len(head) != 10 { + t.Fatalf("head len = %d, want 10", len(head)) + } + want := []byte{1, 2, 3, 4, 5, 21, 22, 23, 26, 27} + for i, w := range want { + if head[i].Track.ID.Bytes[15] != w { + t.Errorf("head[%d] = track %d, want %d", i, head[i].Track.ID.Bytes[15], w) + } + } +} + +func TestPickQuotaHead_ThinSeedSpillsBestFirst(t *testing.T) { + // Seed 1 has a single candidate; its unfilled quota goes to the + // best remaining candidates regardless of seed. + in := make([]recommendation.Candidate, 0, 12) + seedOf := map[pgtype.UUID]int{} + for i := 0; i < 12; i++ { + c := makeCand(i+1, i+1, i+1, float64(12-i)) + si := 0 + if i == 11 { + si = 1 // the worst-ranked candidate is seed 1's only one + } + seedOf[c.Track.ID] = si + in = append(in, c) + } + head := pickQuotaHead(in, seedOf, 2, 10) + if len(head) != 10 { + t.Fatalf("head len = %d, want 10", len(head)) + } + // Quotas [6,4]: seed 0 fills 6 (tracks 1-6), seed 1 fills 1 + // (track 12), spill tops up with tracks 7-9. + got := map[byte]bool{} + for _, c := range head { + got[c.Track.ID.Bytes[15]] = true + } + for _, w := range []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 12} { + if !got[w] { + t.Errorf("head missing track %d (spill should fill best-first)", w) + } + } +} + +func TestPickWeightedTail_PrefersEarlyRanks(t *testing.T) { + // The fresh tail's sample is rank-weighted (#1269): across many + // days, the mean selected rank must sit well below the uniform + // draw's ~99.5 for a 200-deep pool. + pool := make([]recommendation.Candidate, 0, 200) + for i := 0; i < 200; i++ { + c := makeCand(1, i+1, i+1, 1.0) + // Distinct track IDs beyond one byte: bytes wrap at 256. + c.Track.ID.Bytes[14] = byte(i / 200) + c.Track.ID.Bytes[15] = byte(i % 200) + pool = append(pool, c) + } + rankOf := map[pgtype.UUID]int{} + for i, c := range pool { + rankOf[c.Track.ID] = i + } + totalRank, picks := 0, 0 + for day := 1; day <= 28; day++ { + got := pickWeightedTail(pool, "2026-06-"+twoDigits(day), 10) + if len(got) != 10 { + t.Fatalf("day %d: len = %d, want 10", day, len(got)) + } + for _, c := range got { + totalRank += rankOf[c.Track.ID] + picks++ + } + } + mean := float64(totalRank) / float64(picks) + if mean > 80 { + t.Errorf("mean selected rank = %.1f; want well below uniform ~99.5", mean) + } +} + +func TestPickWeightedTail_DeterministicWithinDay(t *testing.T) { + pool := make([]recommendation.Candidate, 0, 60) + for i := 0; i < 60; i++ { + pool = append(pool, makeCand(i+1, i+1, i+1, 1.0)) + } + a := pickWeightedTail(pool, "2026-06-05", 8) + b := pickWeightedTail(pool, "2026-06-05", 8) + for i := range a { + if a[i].Track.ID != b[i].Track.ID { + t.Fatal("weighted tail must be deterministic within a day") + } + } +} diff --git a/internal/playlists/scheduler.go b/internal/playlists/scheduler.go index 97ab098f..c68d2da7 100644 --- a/internal/playlists/scheduler.go +++ b/internal/playlists/scheduler.go @@ -236,7 +236,7 @@ func (s *Scheduler) runStartupCatchUp(ctx context.Context, users []dbq.ListActiv // build can read a fresh profile (phase-2 consumption); both steps are // best-effort and a failure in one is logged without blocking the other. func (s *Scheduler) rebuildUserDaily(ctx context.Context, userID pgtype.UUID, now time.Time) { - if err := taste.BuildTasteProfile(ctx, s.pool, s.logger, userID, taste.DefaultConfig()); err != nil { + if err := taste.BuildTasteProfile(ctx, s.pool, s.logger, userID, currentTasteConfig()); err != nil { s.logger.Warn("scheduler: taste profile rebuild failed", "user_id", uuidStringPL(userID), "err", err) } diff --git a/internal/playlists/seed_selection_test.go b/internal/playlists/seed_selection_test.go index c3e5ec43..e3399f3b 100644 --- a/internal/playlists/seed_selection_test.go +++ b/internal/playlists/seed_selection_test.go @@ -46,6 +46,26 @@ func TestPickSeedArtistsFromRows_Empty(t *testing.T) { } } +func TestPickKindForSeedTier(t *testing.T) { + // The seed query's fallback tiers map onto the rule-#131 ladder: + // fresh 7-day engagement is the exact desire, everything past the + // 30-day step-back collapses into the far tier (#1255). + cases := []struct { + tier int32 + want string + }{ + {0, pickKindTier1}, + {1, pickKindTier2}, + {2, pickKindTier3}, + {3, pickKindTier3}, + } + for _, c := range cases { + if got := pickKindForSeedTier(c.tier); got != c.want { + t.Errorf("pickKindForSeedTier(%d) = %q, want %q", c.tier, got, c.want) + } + } +} + // userIDHash is the per-user, per-day hash that drives the daily- // determinism RNGs. Same family as tieBreakHash, just keyed on user // ID instead of track ID. @@ -76,60 +96,65 @@ func TestUserIDHash_DifferentUserChangesHash(t *testing.T) { } } -// pickForYouSeedForDay rotates the chosen For-You seed across the -// user's top-played candidates using userIDHash. Verifies the picker -// is deterministic within a day, varies across days, and degrades -// gracefully when fewer than 5 candidates exist. +// pickDailySeeds shuffles the candidate pool daily-deterministically +// and takes up to n — For-You uses n=forYouSeedCount for its +// multi-seed blend (#1269), Songs-like uses n=3 via the +// pickSeedArtistsForDay wrapper. Verifies determinism within a day, +// variation across days, and graceful degradation on small pools. -func TestPickForYouSeedForDay_DeterministicWithinDay(t *testing.T) { +func TestPickDailySeeds_DeterministicWithinDay(t *testing.T) { u := pgtype.UUID{Bytes: [16]byte{1}, Valid: true} - seeds := []pgtype.UUID{ + pool := []pgtype.UUID{ {Bytes: [16]byte{10}, Valid: true}, {Bytes: [16]byte{20}, Valid: true}, {Bytes: [16]byte{30}, Valid: true}, {Bytes: [16]byte{40}, Valid: true}, {Bytes: [16]byte{50}, Valid: true}, } - a := pickForYouSeedForDay(seeds, u, "2026-05-04") - b := pickForYouSeedForDay(seeds, u, "2026-05-04") - if a != b { - t.Fatalf("same day should pick same seed; got %v then %v", a, b) + a := pickDailySeeds(pool, u, "2026-05-04", 3) + b := pickDailySeeds(pool, u, "2026-05-04", 3) + if len(a) != 3 || len(b) != 3 { + t.Fatalf("lens = %d, %d; want 3, 3", len(a), len(b)) + } + for i := range a { + if a[i] != b[i] { + t.Fatalf("same day should pick same seeds; got %v then %v", a, b) + } } } -func TestPickForYouSeedForDay_VariesAcrossDays(t *testing.T) { +func TestPickDailySeeds_VariesAcrossDays(t *testing.T) { u := pgtype.UUID{Bytes: [16]byte{1}, Valid: true} - seeds := []pgtype.UUID{ + pool := []pgtype.UUID{ {Bytes: [16]byte{10}, Valid: true}, {Bytes: [16]byte{20}, Valid: true}, {Bytes: [16]byte{30}, Valid: true}, {Bytes: [16]byte{40}, Valid: true}, {Bytes: [16]byte{50}, Valid: true}, } - picks := map[[16]byte]bool{} + firstPicks := map[[16]byte]bool{} for i := 1; i <= 30; i++ { date := "2026-05-" + twoDigits(i) - picks[pickForYouSeedForDay(seeds, u, date).Bytes] = true + firstPicks[pickDailySeeds(pool, u, date, 3)[0].Bytes] = true } - if len(picks) < 2 { - t.Errorf("expected >=2 distinct seeds across 30 days; got %d", len(picks)) + if len(firstPicks) < 2 { + t.Errorf("expected >=2 distinct lead seeds across 30 days; got %d", len(firstPicks)) } } -func TestPickForYouSeedForDay_SingleCandidate(t *testing.T) { +func TestPickDailySeeds_SmallPool(t *testing.T) { u := pgtype.UUID{Bytes: [16]byte{1}, Valid: true} only := pgtype.UUID{Bytes: [16]byte{99}, Valid: true} - got := pickForYouSeedForDay([]pgtype.UUID{only}, u, "2026-05-04") - if got != only { - t.Errorf("single-seed pool should return that seed; got %v", got) + got := pickDailySeeds([]pgtype.UUID{only}, u, "2026-05-04", 3) + if len(got) != 1 || got[0] != only { + t.Errorf("single-entry pool should return just that entry; got %v", got) } } -func TestPickForYouSeedForDay_EmptyPool(t *testing.T) { +func TestPickDailySeeds_EmptyPool(t *testing.T) { u := pgtype.UUID{Bytes: [16]byte{1}, Valid: true} - got := pickForYouSeedForDay(nil, u, "2026-05-04") - if got.Valid { - t.Errorf("empty pool should return zero UUID; got %v", got) + if got := pickDailySeeds(nil, u, "2026-05-04", 3); len(got) != 0 { + t.Errorf("empty pool should return nothing; got %v", got) } } diff --git a/internal/playlists/system.go b/internal/playlists/system.go index b618d9d7..5103c14f 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -12,8 +12,10 @@ import ( "errors" "fmt" "log/slog" + "math" "math/rand" "sort" + "sync" "time" "github.com/jackc/pgx/v5" @@ -22,6 +24,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/recommendation" + "git.fabledsword.com/bvandeusen/minstrel/internal/taste" ) // seedArtistRow mirrors the sqlc-generated PickSeedArtistsRow shape. @@ -83,32 +86,15 @@ func userIDHash(userID pgtype.UUID, dateStr string) uint64 { return binary.BigEndian.Uint64(sum[:8]) } -// pickForYouSeedForDay picks one of the user's top-played candidate -// tracks as today's For-You seed. With 5 candidates and SHA-256-based -// rotation, each candidate gets picked roughly 1 day in 5; the chosen -// seed drives the similarity candidate pool, so the resulting mix is -// substantially different across days for a user with diverse top-N -// plays. -// -// Degrades gracefully: 1 candidate → returns it; 0 → returns zero UUID -// (caller treats as "no seed available" and skips For-You). -func pickForYouSeedForDay(seeds []pgtype.UUID, userID pgtype.UUID, dateStr string) pgtype.UUID { - if len(seeds) == 0 { - return pgtype.UUID{} - } - idx := int(userIDHash(userID, dateStr) % uint64(len(seeds))) - return seeds[idx] -} - -// pickSeedArtistsForDay takes a candidate pool of up to N artists and -// returns up to 3 of them, daily-deterministically shuffled. Each day -// gets a different ordering / selection, but within-day stability is -// preserved (same inputs always produce the same output). +// pickDailySeeds takes a candidate pool of UUIDs and returns up to n +// of them, daily-deterministically shuffled. Each day gets a different +// ordering / selection, but within-day stability is preserved (same +// inputs always produce the same output). // // Uses Fisher-Yates over a copy of the input, seeded by userIDHash. -// Degrades gracefully: <3 candidates returns whatever is available in +// Degrades gracefully: len(fractions) { + // Not reachable at forYouSeedCount = 3; even split keeps any + // future seed-count change from silently starving seeds. + fractions = make([]float64, numSeeds) + for i := range fractions { + fractions[i] = 1.0 / float64(numSeeds) + } + } + quotas := make([]int, numSeeds) + assigned := 0 + for i := 0; i < numSeeds; i++ { + quotas[i] = int(float64(headN) * fractions[i]) + assigned += quotas[i] + } + quotas[0] += headN - assigned + return quotas +} + +// pickQuotaHead walks the score-sorted capped pool best-first, taking +// candidates whose originating seed still has head quota; slots a thin +// seed can't fill spill best-first regardless of seed in a second +// pass. A nil seedOf (single-seed path, tests) attributes everything +// to seed 0, which reduces to plain top-headN. +func pickQuotaHead(capped []recommendation.Candidate, seedOf map[pgtype.UUID]int, numSeeds, headN int) []recommendation.Candidate { + quotas := headQuotas(numSeeds, headN) + taken := make([]int, len(quotas)) + inHead := make(map[pgtype.UUID]bool, headN) + head := make([]recommendation.Candidate, 0, headN) + for _, c := range capped { + if len(head) >= headN { + break + } + si := seedOf[c.Track.ID] + if si < 0 || si >= len(quotas) { + si = 0 + } + if taken[si] >= quotas[si] { + continue + } + taken[si]++ + head = append(head, c) + inHead[c.Track.ID] = true + } + for _, c := range capped { + if len(head) >= headN { + break + } + if inHead[c.Track.ID] { + continue + } + head = append(head, c) + inHead[c.Track.ID] = true + } + return head +} + +// forYouTailHalfLifeRanks tunes the fresh tail's rank bias (#1269): a +// candidate's sampling weight halves every 50 ranks, so the 101st-best +// candidate is far likelier than the 401st-best instead of equal — +// freshness keeps its "you'll probably enjoy this" half. +const forYouTailHalfLifeRanks = 50.0 + +// tailSampleUniverse quantizes tieBreakHash into a uniform (0,1] draw +// for the weighted sample; 1e6 buckets is plenty of resolution for +// pools of a few hundred. +const tailSampleUniverse uint64 = 1_000_000 + +// pickWeightedTail samples up to tailN candidates from the rank- +// ordered tail pool with exponentially rank-decaying weights, using +// deterministic Efraimidis-Spirakis keys (u^(1/w), u derived from +// tieBreakHash): same (track, day) → same key, so the sample is stable +// within a day and rotates across days. Replaces the uniform daily +// draw where rank 380 had the same chance as rank 101. +func pickWeightedTail(tailPool []recommendation.Candidate, dateStr string, tailN int) []recommendation.Candidate { + if len(tailPool) <= tailN { + return tailPool + } + type keyed struct { + c recommendation.Candidate + key float64 + } + keys := make([]keyed, len(tailPool)) + for i, c := range tailPool { + u := (float64(tieBreakHash(c.Track.ID, dateStr)%tailSampleUniverse) + 1) / + float64(tailSampleUniverse+1) + w := math.Exp2(-float64(i) / forYouTailHalfLifeRanks) + keys[i] = keyed{c: c, key: math.Pow(u, 1/w)} + } + sort.SliceStable(keys, func(i, j int) bool { return keys[i].key > keys[j].key }) + out := make([]recommendation.Candidate, tailN) + for i := 0; i < tailN; i++ { + out[i] = keys[i].c + } + return out +} + // scoreAndSortCandidates scores every candidate with recommendation.Score // and returns a new slice sorted by score DESC (ties broken by // tieBreakHash). The scoring RNG is seeded by userIDHash so jitter is @@ -199,9 +397,10 @@ func scoreAndSortCandidates(cands []recommendation.Candidate, userID pgtype.UUID sort.SliceStable(ordered, func(i, j int) bool { return uuidLessPL(ordered[i].Track.ID, ordered[j].Track.ID) }) + weights := currentSystemMixWeights() pairs := make([]scored, len(ordered)) for i, c := range ordered { - pairs[i] = scored{c: c, score: recommendation.Score(c.Inputs, systemMixWeights, now, rng.Float64)} + pairs[i] = scored{c: c, score: recommendation.Score(c.Inputs, weights, now, rng.Float64)} } sort.SliceStable(pairs, func(i, j int) bool { if pairs[i].score != pairs[j].score { @@ -336,10 +535,11 @@ func systemForYouSourceLimits() recommendation.CandidateSourceLimits { } } -// produceForYou: today's seed from the user's top-5 played tracks -// (rotates daily via userIDHash), similarity candidate pool, head+ -// tail composition. The base seed query failing is fatal; a -// candidate-load failure is logged and yields no For-You. +// produceForYou: blend candidate pools from up to forYouSeedCount of +// the user's top-5 played tracks (rotating daily via pickDailySeeds), +// then head+tail composition with per-seed head quotas (#1269). The +// base seed query failing is fatal; a per-seed candidate-load failure +// is logged and that seed just contributes nothing. func produceForYou( ctx context.Context, q *dbq.Queries, logger *slog.Logger, userID pgtype.UUID, dateStr string, now time.Time, @@ -348,24 +548,40 @@ func produceForYou( if err != nil { return nil, fmt.Errorf("pick for-you seed candidates: %w", err) } - forYouSeed := pickForYouSeedForDay(forYouSeeds, userID, dateStr) - if !forYouSeed.Valid { + seeds := pickDailySeeds(forYouSeeds, userID, dateStr, forYouSeedCount) + if len(seeds) == 0 { return nil, nil } zeroVec := recommendation.SessionVector{Seed: true} - cands, cerr := recommendation.LoadCandidatesFromSimilarity( - ctx, q, userID, forYouSeed, - 1, // recentlyPlayedHours — small to avoid filtering the seed's recent neighbourhood - zeroVec, - []pgtype.UUID{forYouSeed}, - systemForYouSourceLimits(), - ) - if cerr != nil { - logger.Warn("system playlist: for-you candidates load failed; skipping", - "user_id", uuidStringPL(userID), "err", cerr) + // Merge per-seed pools; first-seen wins on dedup, and seedOf + // remembers which seed sourced each track for the head quotas. + var merged []recommendation.Candidate + seedOf := map[pgtype.UUID]int{} + for i, seed := range seeds { + cands, cerr := recommendation.LoadCandidatesFromSimilarity( + ctx, q, userID, seed, + 1, // recentlyPlayedHours — small to avoid filtering the seed's recent neighbourhood + zeroVec, + seeds, + systemForYouSourceLimits(), + ) + if cerr != nil { + logger.Warn("system playlist: for-you candidates load failed for seed; continuing", + "user_id", uuidStringPL(userID), "seed", uuidStringPL(seed), "err", cerr) + continue + } + for _, c := range cands { + if _, seen := seedOf[c.Track.ID]; seen { + continue + } + seedOf[c.Track.ID] = i + merged = append(merged, c) + } + } + if len(merged) == 0 { return nil, nil } - tracks := pickHeadAndTail(cands, userID, dateStr, now, forYouHeadN, forYouTailN) + tracks := pickHeadAndTail(merged, seedOf, len(seeds), userID, dateStr, now, forYouHeadN, forYouTailN) if len(tracks) == 0 { return nil, nil } @@ -373,8 +589,11 @@ func produceForYou( } // produceSeedMixes: up to 3 "Songs like {artist}" mixes. Seed -// artists rotate daily-deterministically. The base seed-artist -// query failing is fatal; per-artist failures are logged + skipped. +// artists rotate daily-deterministically; the seed query falls back +// through widening engagement windows (#1255) and every returned row +// shares the winning tier, stamped onto the built tracks as their +// pick_kind. The base seed-artist query failing is fatal; per-artist +// failures are logged + skipped. func produceSeedMixes( ctx context.Context, q *dbq.Queries, logger *slog.Logger, userID pgtype.UUID, dateStr string, now time.Time, @@ -383,6 +602,10 @@ func produceSeedMixes( if err != nil { return nil, fmt.Errorf("pick seed artists: %w", err) } + seedTierKind := "" + if len(seedRows) > 0 { + seedTierKind = pickKindForSeedTier(seedRows[0].Tier) + } seedRowsLocal := make([]seedArtistRow, 0, len(seedRows)) for _, r := range seedRows { seedRowsLocal = append(seedRowsLocal, seedArtistRow{ @@ -427,6 +650,9 @@ func produceSeedMixes( if len(tracks) == 0 { continue } + for i := range tracks { + tracks[i].PickKind = seedTierKind + } out = append(out, builtPlaylist{ Name: fmt.Sprintf("Songs like %s", artistRow.Name), Variant: "songs_like_artist", @@ -601,20 +827,22 @@ func pickTopN(cands []recommendation.Candidate, userID pgtype.UUID, dateStr stri capped = capped[:n] } rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr)))) + weights := currentSystemMixWeights() out := make([]rankedCandidate, len(capped)) for i, c := range capped { out[i] = rankedCandidate{ TrackID: c.Track.ID, - Score: recommendation.Score(c.Inputs, systemMixWeights, now, rng.Float64), + Score: recommendation.Score(c.Inputs, weights, now, rng.Float64), } } return out } -// pickHeadAndTail picks headN from the score-sorted head plus tailN from -// positions 2*headN onward (the tail), with the tail sampled -// daily-deterministically via tieBreakHash. Caps applied before the -// head/tail split. Used by For-You only. +// pickHeadAndTail picks headN taste anchors from the score-sorted pool +// (under per-seed quotas when seedOf/numSeeds describe a multi-seed +// blend, #1269) plus tailN freshness picks sampled rank-weighted from +// positions 2*headN onward. Caps applied before the head/tail split. +// Used by For-You only. // // The "tail" — candidates ranked beyond 2*headN — is still similarity- // related (every candidate passed the similarity filter) but isn't among @@ -624,58 +852,76 @@ func pickTopN(cands []recommendation.Candidate, userID pgtype.UUID, dateStr stri // Falls back to standard pickTopN behavior when the candidate pool is too // small to support a meaningful head/tail split (capped pool <= // headN+tailN, or no candidates at or beyond position 2*headN). -func pickHeadAndTail(cands []recommendation.Candidate, userID pgtype.UUID, dateStr string, now time.Time, headN, tailN int) []rankedCandidate { +func pickHeadAndTail( + cands []recommendation.Candidate, seedOf map[pgtype.UUID]int, numSeeds int, + userID pgtype.UUID, dateStr string, now time.Time, headN, tailN int, +) []rankedCandidate { sorted := scoreAndSortCandidates(cands, userID, dateStr, now) capped := capCandidatesByAlbumAndArtist(sorted) rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr)))) + weights := currentSystemMixWeights() total := headN + tailN if len(capped) <= total { // Pool too small for a head/tail split — return up to total entries. + // All marked taste: top-N-by-score IS the taste mechanism; no + // exploration sampling happens on this path. if len(capped) < total { total = len(capped) } out := make([]rankedCandidate, total) for i := 0; i < total; i++ { out[i] = rankedCandidate{ - TrackID: capped[i].Track.ID, - Score: recommendation.Score(capped[i].Inputs, systemMixWeights, now, rng.Float64), + TrackID: capped[i].Track.ID, + Score: recommendation.Score(capped[i].Inputs, weights, now, rng.Float64), + PickKind: pickKindTaste, } } return out } - head := capped[:headN] + head := pickQuotaHead(capped, seedOf, numSeeds, headN) + inHead := make(map[pgtype.UUID]bool, len(head)) + for _, c := range head { + inHead[c.Track.ID] = true + } tailStart := 2 * headN if tailStart >= len(capped) { tailStart = headN } - // Defensive copy so that sorting the tail pool does not mutate capped. - tailPool := append([]recommendation.Candidate{}, capped[tailStart:]...) - - // Sort tail pool by tieBreakHash (daily-deterministic), take tailN. - // Sample is stable across requests within a day but varies across days. - sort.SliceStable(tailPool, func(i, j int) bool { - return tieBreakHash(tailPool[i].Track.ID, dateStr) < tieBreakHash(tailPool[j].Track.ID, dateStr) - }) - tail := tailPool - if len(tail) > tailN { - tail = tail[:tailN] + // The tail pool keeps its rank order (position drives the sampling + // weight); head members are excluded — a quota walk can reach past + // tailStart when a seed's candidates rank deep. + tailPool := make([]recommendation.Candidate, 0, len(capped)-tailStart) + for _, c := range capped[tailStart:] { + if inHead[c.Track.ID] { + continue + } + tailPool = append(tailPool, c) } + tail := pickWeightedTail(tailPool, dateStr, tailN) - // Combine: head order preserved (score-sorted), tail in tieBreakHash - // order. "First similar, then surprise" reads naturally in playback. + // Combine: head first (score-sorted under quotas), then the fresh + // sample. "First similar, then surprise" reads naturally in + // playback. Head entries are the taste picks; tail entries are the + // freshness injection (#1249) — the split the metrics page + // attributes skips to. combined := make([]recommendation.Candidate, 0, len(head)+len(tail)) combined = append(combined, head...) combined = append(combined, tail...) out := make([]rankedCandidate, len(combined)) for i, c := range combined { + kind := pickKindTaste + if i >= len(head) { + kind = pickKindFresh + } out[i] = rankedCandidate{ - TrackID: c.Track.ID, - Score: recommendation.Score(c.Inputs, systemMixWeights, now, rng.Float64), + TrackID: c.Track.ID, + Score: recommendation.Score(c.Inputs, weights, now, rng.Float64), + PickKind: kind, } } return out @@ -709,9 +955,15 @@ func insertSystemPlaylist(ctx context.Context, qtx *dbq.Queries, userID pgtype.U } for _, t := range tracks { + var pickKind *string + if t.PickKind != "" { + k := t.PickKind + pickKind = &k + } if _, err := qtx.AppendPlaylistTrack(ctx, dbq.AppendPlaylistTrackParams{ PlaylistID: p.ID, TrackID: t.TrackID, + PickKind: pickKind, }); err != nil { // Track may have been deleted between candidate-load and insert; // skip silently rather than failing the whole build. diff --git a/internal/playlists/system_mixes.go b/internal/playlists/system_mixes.go index 4bd862e4..2ca6ef78 100644 --- a/internal/playlists/system_mixes.go +++ b/internal/playlists/system_mixes.go @@ -40,6 +40,17 @@ import ( // Discover so shuffle-on-play has a varied pool within a day. const discoveryMixLen = 100 +// Minimum viable mix sizes (issue #1246): below the floor the variant +// is withheld entirely so Home renders its "listen more to unlock" +// placeholder — a playlist with a couple of songs reads as a build +// bug, not a mix. Album-coherent mixes (NewForYou / FirstListens) get +// a lower floor because one legitimate new album (~5+ tracks) is a +// useful mix on its own; the scattered mixes need more to feel real. +const ( + discoveryMixMinLen = 15 + discoveryMixMinLenAlbum = 5 +) + // discoveryMixSpec describes one discovery mix. The unified producer // reads the spec and runs a single code path for all variants. type discoveryMixSpec struct { @@ -55,6 +66,11 @@ type discoveryMixSpec struct { // or when day-over-day stability is the intended UX (NewForYou). dailyRotate bool + // minLen is the minimum viable mix size: a finished pool below it + // is withheld (no playlist row) so the client renders the locked + // placeholder instead of a mix that looks built-wrong (#1246). + minLen int + // fetch returns the raw ranked rows. dateStr is supplied for // queries that accept it (passed as the second positional arg // historically); queries that don't accept it ignore the param. @@ -79,25 +95,49 @@ func produceDiscoveryMix(spec discoveryMixSpec) systemPlaylistProducer { if spec.dailyRotate { pool = rotateForDay(pool, userID, dateStr) } - return emit(spec.name, spec.variant, finishMix(pool, spec.diversify)), nil + tracks := finishMix(pool, spec.diversify) + if len(tracks) < spec.minLen { + logger.Info("system playlist: "+spec.variant+" below minimum viable size; withholding", + "user_id", uuidStringPL(userID), "pool", len(tracks), "min", spec.minLen) + return nil, nil + } + return emit(spec.name, spec.variant, tracks), nil } } -// rotateForDay rotates pool left by a daily-deterministic offset so -// each day's downstream truncate-to-N surfaces a different slice of -// the pool while contiguous-block ordering inside the slice is -// preserved. Empty / single-element pools pass through unchanged. +// rotateForDay rotates the pool left by a daily-deterministic offset +// so each day's downstream truncate-to-N surfaces a different slice +// while contiguous-block ordering inside the slice is preserved. +// +// Rotation happens WITHIN each contiguous same-pick-kind block +// (#1267): tiered mixes arrive tier-ordered, and a whole-pool +// rotation would hoist tier-3 filler above tier-1's exact fits. +// Untiered pools are a single block, which reduces to the original +// whole-pool rotation (same seed, same first draw). Empty / +// single-element pools pass through unchanged. func rotateForDay(pool []discoverTrack, userID pgtype.UUID, dateStr string) []discoverTrack { n := len(pool) if n <= 1 { return pool } rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr)))) - offset := rng.Intn(n) - rotated := make([]discoverTrack, 0, n) - rotated = append(rotated, pool[offset:]...) - rotated = append(rotated, pool[:offset]...) - return rotated + out := make([]discoverTrack, 0, n) + for start := 0; start < n; { + end := start + 1 + for end < n && pool[end].PickKind == pool[start].PickKind { + end++ + } + block := pool[start:end] + if len(block) > 1 { + offset := rng.Intn(len(block)) + out = append(out, block[offset:]...) + out = append(out, block[:offset]...) + } else { + out = append(out, block...) + } + start = end + } + return out } // discoveryMixSpecs is the concrete spec list used by the registry in @@ -109,6 +149,7 @@ var discoveryMixSpecs = []discoveryMixSpec{ { name: "Deep Cuts", variant: "deep_cuts", diversify: true, dailyRotate: false, // SQL day-keys via md5(id||$2) + minLen: discoveryMixMinLen, fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, ds string) ([]discoverTrack, error) { rows, err := q.ListDeepCutsTracks(ctx, dbq.ListDeepCutsTracksParams{ UserID: uid, Column2: ds, @@ -126,6 +167,7 @@ var discoveryMixSpecs = []discoveryMixSpec{ { name: "Rediscover", variant: "rediscover", diversify: true, dailyRotate: true, // SQL has no date arg + minLen: discoveryMixMinLen, fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, _ string) ([]discoverTrack, error) { rows, err := q.ListRediscoverTracks(ctx, uid) if err != nil { @@ -141,6 +183,7 @@ var discoveryMixSpecs = []discoveryMixSpec{ { name: "New for you", variant: "new_for_you", diversify: true, dailyRotate: true, // operator wants daily rotation on all deterministic mixes + minLen: discoveryMixMinLenAlbum, fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, _ string) ([]discoverTrack, error) { rows, err := q.ListNewForYouTracks(ctx, uid) if err != nil { @@ -148,7 +191,10 @@ var discoveryMixSpecs = []discoveryMixSpec{ } out := make([]discoverTrack, len(rows)) for i, r := range rows { - out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + out[i] = discoverTrack{ + ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID, + PickKind: pickKindForMixTier(r.Tier), + } } return out, nil }, @@ -156,6 +202,7 @@ var discoveryMixSpecs = []discoveryMixSpec{ { name: "On this day", variant: "on_this_day", diversify: true, dailyRotate: false, // SQL day-keys via md5(id||$2) + minLen: discoveryMixMinLen, fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, ds string) ([]discoverTrack, error) { rows, err := q.ListOnThisDayTracks(ctx, dbq.ListOnThisDayTracksParams{ UserID: uid, Column2: ds, @@ -173,6 +220,7 @@ var discoveryMixSpecs = []discoveryMixSpec{ { name: "First listens", variant: "first_listens", diversify: true, dailyRotate: true, // SQL has no date arg; daily rotate + diversity top-up + minLen: discoveryMixMinLenAlbum, fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, _ string) ([]discoverTrack, error) { rows, err := q.ListFirstListensTracks(ctx, uid) if err != nil { @@ -180,7 +228,10 @@ var discoveryMixSpecs = []discoveryMixSpec{ } out := make([]discoverTrack, len(rows)) for i, r := range rows { - out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + out[i] = discoverTrack{ + ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID, + PickKind: pickKindForMixTier(r.Tier), + } } return out, nil }, @@ -217,7 +268,7 @@ func finishMix(rows []discoverTrack, diversify bool) []rankedCandidate { } tracks := make([]rankedCandidate, len(pool)) for i, t := range pool { - tracks[i] = rankedCandidate{TrackID: t.ID} + tracks[i] = rankedCandidate{TrackID: t.ID, PickKind: t.PickKind} } return tracks } diff --git a/internal/playlists/system_mixes_db_test.go b/internal/playlists/system_mixes_db_test.go new file mode 100644 index 00000000..a2a65c09 --- /dev/null +++ b/internal/playlists/system_mixes_db_test.go @@ -0,0 +1,342 @@ +package playlists_test + +import ( + "context" + "fmt" + "path/filepath" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// seedTrackForArtist creates another track under an existing artist + +// album pair. seedTrack's mbid-less upsert never dedupes artists, and +// Deep Cuts' affinity rule needs several tracks sharing one artist_id. +func seedTrackForArtist(t *testing.T, pool *pgxpool.Pool, title string, albumID, artistID pgtype.UUID) dbq.Track { + t.Helper() + track, err := dbq.New(pool).UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: title, AlbumID: albumID, ArtistID: artistID, + DurationMs: 1000, + FilePath: filepath.Join(t.TempDir(), fmt.Sprintf("%s.mp3", title)), + FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("seed track for artist: %v", err) + } + return track +} + +// seedSourcedPlayEvent is seedPlayEvent plus a source tag — needed by +// the Rediscover cooldown, which keys off rediscover-sourced skips. +func seedSourcedPlayEvent(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time, wasSkipped bool, source string) { + t.Helper() + _, err := pool.Exec(context.Background(), ` + WITH s AS ( + INSERT INTO play_sessions (user_id, started_at, last_event_at) + VALUES ($1, $3, $3) + RETURNING id + ) + INSERT INTO play_events (user_id, track_id, session_id, started_at, was_skipped, source) + SELECT $1, $2, s.id, $3, $4, $5 FROM s + `, userID, trackID, startedAt, wasSkipped, source) + if err != nil { + t.Fatalf("seed sourced play_event: %v", err) + } +} + +// idSet collapses candidate rows into a lookup by track id. +func idSet[T any](rows []T, id func(T) pgtype.UUID) map[pgtype.UUID]bool { + out := make(map[pgtype.UUID]bool, len(rows)) + for _, r := range rows { + out[id(r)] = true + } + return out +} + +// seedPlayWithDuration inserts a completed play with an explicit +// duration_played_ms — the "attempted >= 30s" queries key off it. +func seedPlayWithDuration(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time, durationMs int32) { + t.Helper() + _, err := pool.Exec(context.Background(), ` + WITH s AS ( + INSERT INTO play_sessions (user_id, started_at, last_event_at) + VALUES ($1, $3, $3) + RETURNING id + ) + INSERT INTO play_events (user_id, track_id, session_id, started_at, duration_played_ms) + SELECT $1, $2, s.id, $3, $4 FROM s + `, userID, trackID, startedAt, durationMs) + if err != nil { + t.Fatalf("seed play with duration: %v", err) + } +} + +// seedAlbumForArtist creates a fresh album under an existing artist — +// the tiered album queries need several albums sharing one artist_id. +func seedAlbumForArtist(t *testing.T, pool *pgxpool.Pool, title string, artistID pgtype.UUID) pgtype.UUID { + t.Helper() + al, err := dbq.New(pool).UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{ + Title: title, SortTitle: title, ArtistID: artistID, + }) + if err != nil { + t.Fatalf("seed album for artist: %v", err) + } + return al.ID +} + +func backdateAlbum(t *testing.T, pool *pgxpool.Pool, albumID pgtype.UUID, to time.Time) { + t.Helper() + if _, err := pool.Exec(context.Background(), + `UPDATE albums SET created_at = $2 WHERE id = $1`, albumID, to); err != nil { + t.Fatalf("backdate album: %v", err) + } +} + +func likeArtist(t *testing.T, pool *pgxpool.Pool, userID, artistID pgtype.UUID) { + t.Helper() + if _, err := pool.Exec(context.Background(), + `INSERT INTO general_likes_artists (user_id, artist_id) VALUES ($1, $2)`, + userID, artistID); err != nil { + t.Fatalf("like artist: %v", err) + } +} + +func TestListDeepCutsTracks_SkippedTracksExcluded(t *testing.T) { + // #1257: eligibility used to count only unskipped plays, so a track + // skipped repeatedly (and never finished) read as "barely heard". + pool := newPool(t) + q := dbq.New(pool) + u := seedUser(t, pool, "dc1") + now := time.Now().UTC() + + // Affinity artist: 5 non-skip plays on the heavy track. + heavy := seedTrack(t, pool, "dc1-heavy", "dc1-artist") + for p := 0; p < 5; p++ { + seedPlayEvent(t, pool, u.ID, heavy.ID, now.Add(-time.Duration(p+1)*time.Hour), false) + } + // Actively rejected: two skips, zero completed plays. + rejected := seedTrackForArtist(t, pool, "dc1-rejected", heavy.AlbumID, heavy.ArtistID) + seedPlayEvent(t, pool, u.ID, rejected.ID, now.Add(-2*time.Hour), true) + seedPlayEvent(t, pool, u.ID, rejected.ID, now.Add(-3*time.Hour), true) + // Genuinely unexplored. + unheard := seedTrackForArtist(t, pool, "dc1-unheard", heavy.AlbumID, heavy.ArtistID) + // One accidental skip stays below the threshold. + oneSkip := seedTrackForArtist(t, pool, "dc1-oneskip", heavy.AlbumID, heavy.ArtistID) + seedPlayEvent(t, pool, u.ID, oneSkip.ID, now.Add(-4*time.Hour), true) + + rows, err := q.ListDeepCutsTracks(context.Background(), dbq.ListDeepCutsTracksParams{ + UserID: u.ID, Column2: "2026-07-03", + }) + if err != nil { + t.Fatalf("ListDeepCutsTracks: %v", err) + } + got := idSet(rows, func(r dbq.ListDeepCutsTracksRow) pgtype.UUID { return r.ID }) + if got[rejected.ID] { + t.Error("twice-skipped track qualified as a deep cut; skips must disqualify") + } + if !got[unheard.ID] { + t.Error("unexplored track missing from deep cuts") + } + if !got[oneSkip.ID] { + t.Error("single accidental skip must not banish a track") + } + if got[heavy.ID] { + t.Error("heavily-played track must not be a deep cut") + } +} + +func TestListOnThisDayTracks_WrapsYearBoundary(t *testing.T) { + // #1256: plain ABS on day-of-year made Dec 28 vs Jan 3 read as 359 + // days apart. The build date parameter anchors "today" so the wrap + // is testable at any run date. + pool := newPool(t) + q := dbq.New(pool) + u := seedUser(t, pool, "otd1") + + // Played Dec 28, 2025 — 6 circular days from the Jan 3 build date. + winter := seedTrack(t, pool, "otd1-winter", "otd1-artist") + seedPlayEvent(t, pool, u.ID, winter.ID, + time.Date(2025, 12, 28, 12, 0, 0, 0, time.UTC), false) + // Played mid-March — far outside the ±10-day window. + spring := seedTrack(t, pool, "otd1-spring", "otd1-artist") + seedPlayEvent(t, pool, u.ID, spring.ID, + time.Date(2025, 3, 15, 12, 0, 0, 0, time.UTC), false) + + rows, err := q.ListOnThisDayTracks(context.Background(), dbq.ListOnThisDayTracksParams{ + UserID: u.ID, Column2: "2026-01-03", + }) + if err != nil { + t.Fatalf("ListOnThisDayTracks: %v", err) + } + got := idSet(rows, func(r dbq.ListOnThisDayTracksRow) pgtype.UUID { return r.ID }) + if !got[winter.ID] { + t.Error("play 6 calendar days across the year boundary must qualify (circular DOY)") + } + if got[spring.ID] { + t.Error("play ~70 days away must not qualify") + } +} + +func TestListNewForYouTracks_TiersAndConsumption(t *testing.T) { + // #1267: tiered eligibility with consume-on-play. Tier 1 = fresh + // affinity albums; tier 2 = affinity albums from the wider 90-day + // window; tier 3 = fresh additions without affinity; any attempted + // (>=30s) track consumes the album out of the mix entirely. + pool := newPool(t) + q := dbq.New(pool) + u := seedUser(t, pool, "nfy1") + now := time.Now().UTC() + + // Affinity artist via a like; anchor track establishes the artist. + anchor := seedTrack(t, pool, "nfy1-anchor", "nfy1-artistX") + likeArtist(t, pool, u.ID, anchor.ArtistID) + backdateAlbum(t, pool, anchor.AlbumID, now.Add(-200*24*time.Hour)) // out of every window + + // Fresh affinity album → tier 1. + freshAlbum := seedAlbumForArtist(t, pool, "nfy1-fresh", anchor.ArtistID) + freshTrack := seedTrackForArtist(t, pool, "nfy1-fresh-1", freshAlbum, anchor.ArtistID) + // Affinity album added 60 days ago → tier 2. + olderAlbum := seedAlbumForArtist(t, pool, "nfy1-older", anchor.ArtistID) + olderTrack := seedTrackForArtist(t, pool, "nfy1-older-1", olderAlbum, anchor.ArtistID) + backdateAlbum(t, pool, olderAlbum, now.Add(-60*24*time.Hour)) + // Fresh album by an unrelated artist → tier 3. + otherTrack := seedTrack(t, pool, "nfy1-other-1", "nfy1-artistY") + // Fresh affinity album with one attempted track → consumed, absent. + consumedAlbum := seedAlbumForArtist(t, pool, "nfy1-consumed", anchor.ArtistID) + consumedTrack := seedTrackForArtist(t, pool, "nfy1-consumed-1", consumedAlbum, anchor.ArtistID) + seedPlayWithDuration(t, pool, u.ID, consumedTrack.ID, now.Add(-time.Hour), 35_000) + // Affinity album added 120 days ago → outside every window, absent. + ancientAlbum := seedAlbumForArtist(t, pool, "nfy1-ancient", anchor.ArtistID) + ancientTrack := seedTrackForArtist(t, pool, "nfy1-ancient-1", ancientAlbum, anchor.ArtistID) + backdateAlbum(t, pool, ancientAlbum, now.Add(-120*24*time.Hour)) + + rows, err := q.ListNewForYouTracks(context.Background(), u.ID) + if err != nil { + t.Fatalf("ListNewForYouTracks: %v", err) + } + tiers := map[pgtype.UUID]int32{} + for _, r := range rows { + tiers[r.ID] = r.Tier + } + if got := tiers[freshTrack.ID]; got != 1 { + t.Errorf("fresh affinity album tier = %d, want 1", got) + } + if got := tiers[olderTrack.ID]; got != 2 { + t.Errorf("60-day affinity album tier = %d, want 2", got) + } + if got := tiers[otherTrack.ID]; got != 3 { + t.Errorf("fresh non-affinity album tier = %d, want 3", got) + } + if _, in := tiers[consumedTrack.ID]; in { + t.Error("album with an attempted track must be consumed out of the mix") + } + if _, in := tiers[ancientTrack.ID]; in { + t.Error("album added 120 days ago must be outside every window") + } +} + +func TestListFirstListensTracks_AttemptThreshold(t *testing.T) { + // #1268: "attempted" is track-level at >=30s. A 2-second brush no + // longer disqualifies an album; barely-attempted albums step back + // to tier 2 with the attempted tracks themselves excluded. + pool := newPool(t) + q := dbq.New(pool) + u := seedUser(t, pool, "fl1") + now := time.Now().UTC() + + // Never touched → tier 1. + untouched := seedTrack(t, pool, "fl1-untouched-1", "fl1-artistA") + // One 2-second accidental brush → still tier 1 (the headline bug). + brushed := seedTrack(t, pool, "fl1-brushed-1", "fl1-artistB") + seedPlayWithDuration(t, pool, u.ID, brushed.ID, now.Add(-time.Hour), 2_000) + // 4-track album with one attempted (25%) → tier 2, attempted track + // excluded, siblings present. + barely1 := seedTrack(t, pool, "fl1-barely-1", "fl1-artistC") + barely2 := seedTrackForArtist(t, pool, "fl1-barely-2", barely1.AlbumID, barely1.ArtistID) + barely3 := seedTrackForArtist(t, pool, "fl1-barely-3", barely1.AlbumID, barely1.ArtistID) + barely4 := seedTrackForArtist(t, pool, "fl1-barely-4", barely1.AlbumID, barely1.ArtistID) + seedPlayWithDuration(t, pool, u.ID, barely1.ID, now.Add(-time.Hour), 40_000) + // 2-track album with one attempted (50%) → past the brush + // threshold, absent entirely. + explored1 := seedTrack(t, pool, "fl1-explored-1", "fl1-artistD") + explored2 := seedTrackForArtist(t, pool, "fl1-explored-2", explored1.AlbumID, explored1.ArtistID) + seedPlayWithDuration(t, pool, u.ID, explored1.ID, now.Add(-time.Hour), 40_000) + + rows, err := q.ListFirstListensTracks(context.Background(), u.ID) + if err != nil { + t.Fatalf("ListFirstListensTracks: %v", err) + } + tiers := map[pgtype.UUID]int32{} + for _, r := range rows { + tiers[r.ID] = r.Tier + } + if got := tiers[untouched.ID]; got != 1 { + t.Errorf("untouched album tier = %d, want 1", got) + } + if got := tiers[brushed.ID]; got != 1 { + t.Errorf("2-second brush must not disqualify: tier = %d, want 1", got) + } + if _, in := tiers[barely1.ID]; in { + t.Error("the attempted track itself is not a first listen") + } + for _, tr := range []dbq.Track{barely2, barely3, barely4} { + if got := tiers[tr.ID]; got != 2 { + t.Errorf("sibling of attempted track tier = %d, want 2", got) + } + } + if _, in := tiers[explored1.ID]; in { + t.Error("half-attempted album must be excluded (attempted track)") + } + if _, in := tiers[explored2.ID]; in { + t.Error("half-attempted album must be excluded (sibling)") + } +} + +func TestListRediscoverTracks_SkippedResurfacingCoolsDown(t *testing.T) { + // #1258: a skip on a rediscover-sourced play is a declined + // invitation — the track sits out ~90 days instead of re-qualifying + // the next day. Skips from other surfaces don't cool down, and the + // cooldown expires. + pool := newPool(t) + q := dbq.New(pool) + u := seedUser(t, pool, "rd1") + now := time.Now().UTC() + + eligible := func(name string, playsAge time.Duration) dbq.Track { + tk := seedTrack(t, pool, name, name+"-artist") + for p := 0; p < 3; p++ { + seedPlayEvent(t, pool, u.ID, tk.ID, + now.Add(-playsAge-time.Duration(p)*time.Hour), false) + } + return tk + } + + // Declined 5 days ago from Rediscover itself → cooling down. + declined := eligible("rd1-declined", 60*24*time.Hour) + seedSourcedPlayEvent(t, pool, u.ID, declined.ID, now.Add(-5*24*time.Hour), true, "rediscover") + // Skipped 5 days ago, but from a different surface → still eligible. + otherSkip := eligible("rd1-otherskip", 60*24*time.Hour) + seedSourcedPlayEvent(t, pool, u.ID, otherSkip.ID, now.Add(-5*24*time.Hour), true, "radio") + // Declined 100 days ago → cooldown expired, eligible again. + lapsed := eligible("rd1-lapsed", 120*24*time.Hour) + seedSourcedPlayEvent(t, pool, u.ID, lapsed.ID, now.Add(-100*24*time.Hour), true, "rediscover") + + rows, err := q.ListRediscoverTracks(context.Background(), u.ID) + if err != nil { + t.Fatalf("ListRediscoverTracks: %v", err) + } + got := idSet(rows, func(r dbq.ListRediscoverTracksRow) pgtype.UUID { return r.ID }) + if got[declined.ID] { + t.Error("track declined from rediscover 5 days ago must be cooling down") + } + if !got[otherSkip.ID] { + t.Error("skip from a non-rediscover surface must not cool the track down") + } + if !got[lapsed.ID] { + t.Error("cooldown must expire after ~90 days") + } +} diff --git a/internal/playlists/system_mixes_test.go b/internal/playlists/system_mixes_test.go new file mode 100644 index 00000000..64e863d0 --- /dev/null +++ b/internal/playlists/system_mixes_test.go @@ -0,0 +1,104 @@ +package playlists + +import ( + "testing" + + "github.com/jackc/pgx/v5/pgtype" +) + +func mixTrack(b byte, kind string) discoverTrack { + return discoverTrack{ID: pgtype.UUID{Bytes: [16]byte{b}, Valid: true}, PickKind: kind} +} + +func idsOf(pool []discoverTrack) []byte { + out := make([]byte, len(pool)) + for i, t := range pool { + out[i] = t.ID.Bytes[0] + } + return out +} + +func TestRotateForDay_RotatesWithinTierBlocks(t *testing.T) { + // Tiered pools arrive tier-ordered; rotation must vary the daily + // slice WITHIN each tier without hoisting tier-3 filler above + // tier-1's exact fits (#1267). + pool := []discoverTrack{ + mixTrack(1, pickKindTier1), mixTrack(2, pickKindTier1), mixTrack(3, pickKindTier1), + mixTrack(4, pickKindTier2), mixTrack(5, pickKindTier2), + mixTrack(6, pickKindTier3), + } + u := pgtype.UUID{Bytes: [16]byte{42}, Valid: true} + got := rotateForDay(pool, u, "2026-07-03") + if len(got) != len(pool) { + t.Fatalf("len = %d, want %d", len(got), len(pool)) + } + wantKinds := []string{ + pickKindTier1, pickKindTier1, pickKindTier1, + pickKindTier2, pickKindTier2, pickKindTier3, + } + for i, k := range wantKinds { + if got[i].PickKind != k { + t.Fatalf("pos %d kind = %q, want %q (tier order broken: %v)", + i, got[i].PickKind, k, idsOf(got)) + } + } + // Each block is a rotation of its input: contiguity check for the + // 3-element tier-1 block (successor relation preserved cyclically). + wantOrder := map[byte]byte{1: 2, 2: 3, 3: 1} + for i := 0; i < 2; i++ { + if got[i+1].ID.Bytes[0] != wantOrder[got[i].ID.Bytes[0]] { + t.Errorf("tier1 block is not a rotation: %v", idsOf(got[:3])) + break + } + } + // Determinism within a day. + again := rotateForDay(pool, u, "2026-07-03") + for i := range got { + if got[i].ID != again[i].ID { + t.Fatal("rotation must be deterministic for the same (user, day)") + } + } +} + +func TestRotateForDay_UntieredPoolIsWholeRotation(t *testing.T) { + // Untiered pools (all PickKind "") are one block — the original + // whole-pool rotation semantics. + pool := []discoverTrack{mixTrack(1, ""), mixTrack(2, ""), mixTrack(3, ""), mixTrack(4, "")} + got := rotateForDay(pool, pgtype.UUID{Bytes: [16]byte{7}, Valid: true}, "2026-07-03") + if len(got) != 4 { + t.Fatalf("len = %d, want 4", len(got)) + } + wantNext := map[byte]byte{1: 2, 2: 3, 3: 4, 4: 1} + for i := 0; i < 3; i++ { + cur, next := got[i].ID.Bytes[0], got[i+1].ID.Bytes[0] + if wantNext[cur] != next { + t.Errorf("not a rotation of the input: %v", idsOf(got)) + break + } + } +} + +func TestFinishMix_PropagatesPickKind(t *testing.T) { + // Tier stamps ride discoverTrack through diversify/truncate into + // the rankedCandidates that insertSystemPlaylist persists (#1267). + pool := []discoverTrack{mixTrack(1, pickKindTier1), mixTrack(2, pickKindTier2)} + got := finishMix(pool, false) + if len(got) != 2 { + t.Fatalf("len = %d, want 2", len(got)) + } + if got[0].PickKind != pickKindTier1 || got[1].PickKind != pickKindTier2 { + t.Errorf("pick kinds = %q, %q — want tier1, tier2", got[0].PickKind, got[1].PickKind) + } +} + +func TestPickKindForMixTier(t *testing.T) { + cases := []struct { + tier int32 + want string + }{{1, pickKindTier1}, {2, pickKindTier2}, {3, pickKindTier3}, {9, pickKindTier3}} + for _, c := range cases { + if got := pickKindForMixTier(c.tier); got != c.want { + t.Errorf("pickKindForMixTier(%d) = %q, want %q", c.tier, got, c.want) + } + } +} diff --git a/internal/playlists/system_test.go b/internal/playlists/system_test.go index 2e5fb5cf..e0757758 100644 --- a/internal/playlists/system_test.go +++ b/internal/playlists/system_test.go @@ -135,6 +135,59 @@ func TestBuildSystemPlaylists_SufficientActivity(t *testing.T) { } } +func TestBuildSystemPlaylists_StaleActivity_SongsLikeSurvives(t *testing.T) { + // #1255: the seed-artist query's old hard 7-day window emptied the + // pool after a quiet week, and the atomic-replace build then deleted + // every "Songs like X" mix. With the tiered fallback, plays that are + // ~20 days old (outside 7d, inside 30d) must still seed the mixes — + // and the built tracks carry the tier2 stamp so metrics can compare + // stale-seeded mixes against fresh ones. + pool := newPool(t) + logger := discardLogger() + u := seedUser(t, pool, "stale1") + old := time.Now().UTC().Add(-20 * 24 * time.Hour) + for a := 0; a < 4; a++ { + for k := 0; k < 3; k++ { + tk := seedTrack(t, pool, + "stale1-track-"+string(rune('A'+a))+string(rune('0'+k)), + "stale1-artist-"+string(rune('A'+a))) + for p := 0; p < 3; p++ { + seedPlayEvent(t, pool, u.ID, tk.ID, + old.Add(-time.Duration(a*10+k+p)*time.Hour), false) + } + } + } + + if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil { + t.Fatalf("build: %v", err) + } + + rows, err := pool.Query(context.Background(), ` + SELECT DISTINCT COALESCE(pt.pick_kind, '') + FROM playlist_tracks pt + JOIN playlists p ON p.id = pt.playlist_id + WHERE p.user_id = $1 AND p.system_variant = 'songs_like_artist' + `, u.ID) + if err != nil { + t.Fatalf("query pick_kinds: %v", err) + } + defer rows.Close() + var kinds []string + for rows.Next() { + var k string + if err := rows.Scan(&k); err != nil { + t.Fatalf("scan: %v", err) + } + kinds = append(kinds, k) + } + if len(kinds) == 0 { + t.Fatal("expected songs_like_artist tracks from the 30-day fallback tier; got none (the vanish bug)") + } + if len(kinds) != 1 || kinds[0] != "tier2" { + t.Errorf("pick_kinds = %v, want exactly [tier2] (30-day fallback seeds)", kinds) + } +} + func TestBuildSystemPlaylists_QuarantineExcluded(t *testing.T) { pool := newPool(t) logger := discardLogger() diff --git a/internal/playlists/you_might_like.go b/internal/playlists/you_might_like.go index 3d40e919..27ccaf58 100644 --- a/internal/playlists/you_might_like.go +++ b/internal/playlists/you_might_like.go @@ -83,10 +83,14 @@ func buildYouMightLike( "user_id", uuidStringPL(userID), "err", err) return youMightLikeResult{built: false} } - seed := pickForYouSeedForDay(seeds, userID, dateStr) - if !seed.Valid { + // One rotating seed is right here (unlike For-You's multi-seed + // blend, #1269): the row is a short shelf, not a mix, and a single + // neighborhood per day keeps it coherent. + daily := pickDailySeeds(seeds, userID, dateStr, 1) + if len(daily) == 0 { return youMightLikeResult{built: true} } + seed := daily[0] zeroVec := recommendation.SessionVector{Seed: true} // You-might-like surfaces in-library artists the user does NOT actively @@ -123,11 +127,12 @@ func rollUpCandidates( cands []recommendation.Candidate, userID pgtype.UUID, dateStr string, now time.Time, ) (albumIDs, artistIDs []pgtype.UUID) { rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr)))) + weights := currentSystemMixWeights() albumScores := map[pgtype.UUID][]float64{} artistScores := map[pgtype.UUID][]float64{} albumArtist := map[pgtype.UUID]pgtype.UUID{} for _, c := range cands { - s := recommendation.Score(c.Inputs, systemMixWeights, now, rng.Float64) + s := recommendation.Score(c.Inputs, weights, now, rng.Float64) if c.Track.AlbumID.Valid { albumScores[c.Track.AlbumID] = append(albumScores[c.Track.AlbumID], s) albumArtist[c.Track.AlbumID] = c.Track.ArtistID diff --git a/internal/recsettings/patch.go b/internal/recsettings/patch.go new file mode 100644 index 00000000..e3c78ab6 --- /dev/null +++ b/internal/recsettings/patch.go @@ -0,0 +1,178 @@ +// patch.go — field-name mapping + validation for the tuning patches. +// Wire field names are the snake_case column names; the admin API and +// web card use them verbatim. +package recsettings + +import ( + "errors" + "fmt" + + "git.fabledsword.com/bvandeusen/minstrel/internal/recommendation" +) + +var ( + ErrUnknownScope = errors.New("unknown tuning scope") + ErrUnknownField = errors.New("unknown tuning field") + ErrOutOfRange = errors.New("tuning value out of range") +) + +// weightBound caps every scoring weight's magnitude. The scoring +// terms are all in [-1, 1] before weighting, so ±10 is far past any +// useful setting — the bound exists to catch typos (e.g. 100 for +// 1.00), not to constrain exploration. +const weightBound = 10.0 + +// weightField describes one patchable ScoringWeights field. +type weightField struct { + get func(recommendation.ScoringWeights) float64 + set func(*recommendation.ScoringWeights, float64) + // nonNegative marks fields where a negative value is meaningless + // (a negative jitter magnitude or skip penalty inverts intent in a + // way the score formula already expresses through its sign). + nonNegative bool +} + +var weightFields = map[string]weightField{ + "base_weight": { + get: func(w recommendation.ScoringWeights) float64 { return w.BaseWeight }, + set: func(w *recommendation.ScoringWeights, v float64) { w.BaseWeight = v }, + }, + "like_boost": { + get: func(w recommendation.ScoringWeights) float64 { return w.LikeBoost }, + set: func(w *recommendation.ScoringWeights, v float64) { w.LikeBoost = v }, + }, + "recency_weight": { + get: func(w recommendation.ScoringWeights) float64 { return w.RecencyWeight }, + set: func(w *recommendation.ScoringWeights, v float64) { w.RecencyWeight = v }, + }, + "skip_penalty": { + get: func(w recommendation.ScoringWeights) float64 { return w.SkipPenalty }, + set: func(w *recommendation.ScoringWeights, v float64) { w.SkipPenalty = v }, + nonNegative: true, + }, + "jitter_magnitude": { + get: func(w recommendation.ScoringWeights) float64 { return w.JitterMagnitude }, + set: func(w *recommendation.ScoringWeights, v float64) { w.JitterMagnitude = v }, + nonNegative: true, + }, + "context_weight": { + get: func(w recommendation.ScoringWeights) float64 { return w.ContextWeight }, + set: func(w *recommendation.ScoringWeights, v float64) { w.ContextWeight = v }, + }, + "similarity_weight": { + get: func(w recommendation.ScoringWeights) float64 { return w.SimilarityWeight }, + set: func(w *recommendation.ScoringWeights, v float64) { w.SimilarityWeight = v }, + }, + "taste_weight": { + get: func(w recommendation.ScoringWeights) float64 { return w.TasteWeight }, + set: func(w *recommendation.ScoringWeights, v float64) { w.TasteWeight = v }, + }, +} + +// applyWeightPatch validates and applies a partial update, returning +// the new weights and the list of actual changes (values equal to the +// current setting are dropped, so a re-submitted form is a no-op). +func applyWeightPatch( + current recommendation.ScoringWeights, patch map[string]float64, +) (recommendation.ScoringWeights, []fieldChange, error) { + next := current + var changes []fieldChange + for field, v := range patch { + f, ok := weightFields[field] + if !ok { + return current, nil, fmt.Errorf("%w: %q", ErrUnknownField, field) + } + if v < -weightBound || v > weightBound { + return current, nil, fmt.Errorf("%w: %s = %v (|v| must be <= %v)", + ErrOutOfRange, field, v, weightBound) + } + if f.nonNegative && v < 0 { + return current, nil, fmt.Errorf("%w: %s = %v (must be >= 0)", + ErrOutOfRange, field, v) + } + old := f.get(next) + if old == v { + continue + } + f.set(&next, v) + changes = append(changes, fieldChange{Field: field, Old: old, New: v}) + } + return next, changes, nil +} + +// Taste tuning bounds. The half-life window is generous — from "taste +// is last week" to "taste is a decade" — and the curve points must +// stay ordered inside [0, 1] or the engagement ramps degenerate. +const ( + tasteHalfLifeMin = 1.0 + tasteHalfLifeMax = 3650.0 +) + +// applyTastePatch validates and applies a partial taste update. The +// curve-ordering invariant (hard_skip < neutral < full) is checked on +// the PATCHED result, so a patch may move several points at once. +func applyTastePatch(current TasteTuning, patch map[string]float64) (TasteTuning, []fieldChange, error) { + next := current + var changes []fieldChange + for field, v := range patch { + var target *float64 + switch field { + case "half_life_days": + if v < tasteHalfLifeMin || v > tasteHalfLifeMax { + return current, nil, fmt.Errorf("%w: %s = %v (must be in [%v, %v])", + ErrOutOfRange, field, v, tasteHalfLifeMin, tasteHalfLifeMax) + } + target = &next.HalfLifeDays + case "engagement_hard_skip": + target = &next.EngagementHardSkip + case "engagement_neutral": + target = &next.EngagementNeutral + case "engagement_full": + target = &next.EngagementFull + default: + return current, nil, fmt.Errorf("%w: %q", ErrUnknownField, field) + } + if field != "half_life_days" && (v < 0 || v > 1) { + return current, nil, fmt.Errorf("%w: %s = %v (must be in [0, 1])", + ErrOutOfRange, field, v) + } + if *target == v { + continue + } + changes = append(changes, fieldChange{Field: field, Old: *target, New: v}) + *target = v + } + if !(next.EngagementHardSkip < next.EngagementNeutral && + next.EngagementNeutral < next.EngagementFull) { + return current, nil, fmt.Errorf( + "%w: engagement curve must satisfy hard_skip < neutral < full (got %v < %v < %v)", + ErrOutOfRange, next.EngagementHardSkip, next.EngagementNeutral, next.EngagementFull) + } + return next, changes, nil +} + +// diffWeights returns per-field changes from a to b (empty when equal). +func diffWeights(a, b recommendation.ScoringWeights) []fieldChange { + var out []fieldChange + for field, f := range weightFields { + if f.get(a) != f.get(b) { + out = append(out, fieldChange{Field: field, Old: f.get(a), New: f.get(b)}) + } + } + return out +} + +// diffTaste returns per-field changes from a to b (empty when equal). +func diffTaste(a, b TasteTuning) []fieldChange { + var out []fieldChange + add := func(field string, oldV, newV float64) { + if oldV != newV { + out = append(out, fieldChange{Field: field, Old: oldV, New: newV}) + } + } + add("half_life_days", a.HalfLifeDays, b.HalfLifeDays) + add("engagement_hard_skip", a.EngagementHardSkip, b.EngagementHardSkip) + add("engagement_neutral", a.EngagementNeutral, b.EngagementNeutral) + add("engagement_full", a.EngagementFull, b.EngagementFull) + return out +} diff --git a/internal/recsettings/service.go b/internal/recsettings/service.go new file mode 100644 index 00000000..4a1a0b0b --- /dev/null +++ b/internal/recsettings/service.go @@ -0,0 +1,374 @@ +// Package recsettings is the DB-backed home of the recommendation +// tuning knobs (#1250): the two scoring-weight profiles (radio / +// daily_mix) and the taste-profile build settings (engagement +// half-life + completion curve). It follows the coverart +// SettingsService pattern — boot reconcile seeds shipped defaults for +// missing rows, values are cached under a RWMutex, and every change +// takes effect live (the daily_mix profile + taste config are pushed +// into package playlists; radio reads the cache per request). +// +// Framing (decision #1247): this is the defaults-discovery lab. The +// operator turns knobs to FIND good values; found-good values get +// baked back into the Shipped* functions below as new shipped +// defaults. End users and other operators should never need the card. +package recsettings + +import ( + "context" + "encoding/json" + "fmt" + "log/slog" + "sort" + "sync" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" + "git.fabledsword.com/bvandeusen/minstrel/internal/recommendation" + "git.fabledsword.com/bvandeusen/minstrel/internal/taste" +) + +// Scope names — the three tunable groups. radio + daily_mix are weight +// profiles; taste is the profile-build settings singleton. +const ( + ScopeRadio = "radio" + ScopeDailyMix = "daily_mix" + ScopeTaste = "taste" +) + +// TasteTuning is the tunable subset of taste.Config: the engagement +// half-life and the completion→engagement curve points. +type TasteTuning struct { + HalfLifeDays float64 + EngagementHardSkip float64 + EngagementNeutral float64 + EngagementFull float64 +} + +// ShippedRadioWeights are the shipped radio-profile defaults (moved +// here from config.RecommendationConfig — YAML is bootstrap-only, +// rule: config in UI). Radio is seed-directed (the user picked a +// direction), so taste is a lighter nudge than in the daily mixes. +func ShippedRadioWeights() recommendation.ScoringWeights { + return recommendation.ScoringWeights{ + BaseWeight: 1.0, + LikeBoost: 2.0, + RecencyWeight: 1.0, + SkipPenalty: 1.0, + JitterMagnitude: 0.1, + ContextWeight: 2.0, + SimilarityWeight: 2.0, + TasteWeight: 1.0, + } +} + +// ShippedDailyMixWeights are the shipped daily_mix-profile defaults. +// Must stay in sync with the pre-push literal in playlists/system.go. +func ShippedDailyMixWeights() recommendation.ScoringWeights { + return recommendation.ScoringWeights{ + BaseWeight: 1.0, + LikeBoost: 2.0, + RecencyWeight: 1.0, + SkipPenalty: 2.0, + JitterMagnitude: 0.1, + ContextWeight: 0.5, + SimilarityWeight: 1.5, + TasteWeight: 1.5, + } +} + +// ShippedTasteTuning mirrors taste.DefaultConfig's tunable subset. +func ShippedTasteTuning() TasteTuning { + d := taste.DefaultConfig() + return TasteTuning{ + HalfLifeDays: d.HalfLifeDays, + EngagementHardSkip: d.Engagement.HardSkip, + EngagementNeutral: d.Engagement.NeutralCompletion, + EngagementFull: d.Engagement.FullCompletion, + } +} + +// Service caches the tuning values and owns their DB persistence + +// audit trail. Construct with New at boot. +type Service struct { + pool *pgxpool.Pool + logger *slog.Logger + + mu sync.RWMutex + profiles map[string]recommendation.ScoringWeights + taste TasteTuning +} + +// New boots the service: seeds shipped defaults for missing rows, +// loads the current values, and pushes the daily_mix weights + taste +// config into package playlists so the daily builds pick them up. +func New(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*Service, error) { + s := &Service{ + pool: pool, + logger: logger, + profiles: map[string]recommendation.ScoringWeights{}, + } + if err := s.reconcile(ctx); err != nil { + return nil, fmt.Errorf("recsettings boot: %w", err) + } + return s, nil +} + +// reconcile seeds missing rows with shipped defaults, reads everything +// back into the cache, and pushes the playlist-side values. +func (s *Service) reconcile(ctx context.Context) error { + q := dbq.New(s.pool) + for profile, w := range map[string]recommendation.ScoringWeights{ + ScopeRadio: ShippedRadioWeights(), + ScopeDailyMix: ShippedDailyMixWeights(), + } { + if err := q.UpsertWeightProfileDefaults(ctx, upsertParams(profile, w)); err != nil { + return fmt.Errorf("seed profile %q: %w", profile, err) + } + } + st := ShippedTasteTuning() + if err := q.UpsertTasteTuningDefaults(ctx, dbq.UpsertTasteTuningDefaultsParams{ + HalfLifeDays: st.HalfLifeDays, + EngagementHardSkip: st.EngagementHardSkip, + EngagementNeutral: st.EngagementNeutral, + EngagementFull: st.EngagementFull, + }); err != nil { + return fmt.Errorf("seed taste tuning: %w", err) + } + + rows, err := q.ListWeightProfiles(ctx) + if err != nil { + return fmt.Errorf("list weight profiles: %w", err) + } + tt, err := q.GetTasteTuning(ctx) + if err != nil { + return fmt.Errorf("get taste tuning: %w", err) + } + + s.mu.Lock() + s.profiles = map[string]recommendation.ScoringWeights{} + for _, r := range rows { + s.profiles[r.Profile] = weightsFromRow(r) + } + s.taste = TasteTuning{ + HalfLifeDays: tt.HalfLifeDays, + EngagementHardSkip: tt.EngagementHardSkip, + EngagementNeutral: tt.EngagementNeutral, + EngagementFull: tt.EngagementFull, + } + s.mu.Unlock() + + s.push() + return nil +} + +// push installs the daily-build values into package playlists (the +// coverart Configure() pattern). Radio needs no push — its handler +// reads Weights(ScopeRadio) per request. +func (s *Service) push() { + playlists.SetSystemMixWeights(s.Weights(ScopeDailyMix)) + playlists.SetTasteConfig(s.TasteConfig()) +} + +// Weights returns the cached weights for a profile scope. Unknown +// scopes return the shipped radio defaults (defensive; callers pass +// the Scope* constants). +func (s *Service) Weights(profile string) recommendation.ScoringWeights { + s.mu.RLock() + defer s.mu.RUnlock() + if w, ok := s.profiles[profile]; ok { + return w + } + return ShippedRadioWeights() +} + +// Taste returns the cached taste-tuning values. +func (s *Service) Taste() TasteTuning { + s.mu.RLock() + defer s.mu.RUnlock() + return s.taste +} + +// 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 +// half-life at the shipped ratio (270/75 = 3.6 half-lives) — the +// window is a query-cost bound, not an independent knob. +func (s *Service) TasteConfig() taste.Config { + t := s.Taste() + cfg := taste.DefaultConfig() + cfg.HalfLifeDays = t.HalfLifeDays + cfg.WindowDays = t.HalfLifeDays * 3.6 + cfg.Engagement = taste.EngagementParams{ + HardSkip: t.EngagementHardSkip, + NeutralCompletion: t.EngagementNeutral, + FullCompletion: t.EngagementFull, + } + return cfg +} + +// fieldChange is one entry of an audit row's changes array. +type fieldChange struct { + Field string `json:"field"` + Old float64 `json:"old"` + New float64 `json:"new"` +} + +// UpdateProfile applies a partial update to one weight profile. +// Unknown fields and out-of-range values reject the whole patch. A +// no-op patch (all values equal to current) writes no audit row. +func (s *Service) UpdateProfile(ctx context.Context, profile string, patch map[string]float64) error { + if profile != ScopeRadio && profile != ScopeDailyMix { + return fmt.Errorf("%w: %q", ErrUnknownScope, profile) + } + current := s.Weights(profile) + next, changes, err := applyWeightPatch(current, patch) + if err != nil { + return err + } + if len(changes) == 0 { + return nil + } + return s.persistProfile(ctx, profile, next, "update", changes) +} + +// UpdateTaste applies a partial update to the taste tuning singleton. +func (s *Service) UpdateTaste(ctx context.Context, patch map[string]float64) error { + current := s.Taste() + next, changes, err := applyTastePatch(current, patch) + if err != nil { + return err + } + if len(changes) == 0 { + return nil + } + return s.persistTaste(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 { + switch scope { + case ScopeRadio, ScopeDailyMix: + shipped := ShippedRadioWeights() + if scope == ScopeDailyMix { + shipped = ShippedDailyMixWeights() + } + changes := diffWeights(s.Weights(scope), shipped) + if len(changes) == 0 { + return nil + } + return s.persistProfile(ctx, scope, shipped, "reset", changes) + case ScopeTaste: + shipped := ShippedTasteTuning() + changes := diffTaste(s.Taste(), shipped) + if len(changes) == 0 { + return nil + } + return s.persistTaste(ctx, shipped, "reset", changes) + default: + return fmt.Errorf("%w: %q", ErrUnknownScope, scope) + } +} + +// persistProfile writes the profile row + audit entry, refreshes the +// cache, and pushes daily-build values. +func (s *Service) persistProfile( + ctx context.Context, profile string, w recommendation.ScoringWeights, + action string, changes []fieldChange, +) error { + q := dbq.New(s.pool) + if _, err := q.UpdateWeightProfile(ctx, updateParams(profile, w)); err != nil { + return fmt.Errorf("update profile %q: %w", profile, err) + } + if err := s.audit(ctx, q, profile, action, changes); err != nil { + return err + } + s.mu.Lock() + s.profiles[profile] = w + s.mu.Unlock() + s.push() + return nil +} + +func (s *Service) persistTaste( + ctx context.Context, t TasteTuning, action string, changes []fieldChange, +) error { + q := dbq.New(s.pool) + if _, err := q.UpdateTasteTuning(ctx, dbq.UpdateTasteTuningParams{ + HalfLifeDays: t.HalfLifeDays, + EngagementHardSkip: t.EngagementHardSkip, + EngagementNeutral: t.EngagementNeutral, + EngagementFull: t.EngagementFull, + }); err != nil { + return fmt.Errorf("update taste tuning: %w", err) + } + if err := s.audit(ctx, q, ScopeTaste, action, changes); err != nil { + return err + } + s.mu.Lock() + s.taste = t + s.mu.Unlock() + s.push() + 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( + ctx context.Context, q *dbq.Queries, scope, action string, changes []fieldChange, +) error { + sort.Slice(changes, func(i, j int) bool { return changes[i].Field < changes[j].Field }) + payload, err := json.Marshal(changes) + if err != nil { + return fmt.Errorf("marshal audit changes: %w", err) + } + if err := q.InsertTuningAudit(ctx, dbq.InsertTuningAuditParams{ + Scope: scope, Action: action, Changes: payload, + }); err != nil { + return fmt.Errorf("insert audit row: %w", err) + } + return nil +} + +func upsertParams(profile string, w recommendation.ScoringWeights) dbq.UpsertWeightProfileDefaultsParams { + return dbq.UpsertWeightProfileDefaultsParams{ + Profile: profile, + BaseWeight: w.BaseWeight, + LikeBoost: w.LikeBoost, + RecencyWeight: w.RecencyWeight, + SkipPenalty: w.SkipPenalty, + JitterMagnitude: w.JitterMagnitude, + ContextWeight: w.ContextWeight, + SimilarityWeight: w.SimilarityWeight, + TasteWeight: w.TasteWeight, + } +} + +func updateParams(profile string, w recommendation.ScoringWeights) dbq.UpdateWeightProfileParams { + return dbq.UpdateWeightProfileParams{ + Profile: profile, + BaseWeight: w.BaseWeight, + LikeBoost: w.LikeBoost, + RecencyWeight: w.RecencyWeight, + SkipPenalty: w.SkipPenalty, + JitterMagnitude: w.JitterMagnitude, + ContextWeight: w.ContextWeight, + SimilarityWeight: w.SimilarityWeight, + TasteWeight: w.TasteWeight, + } +} + +func weightsFromRow(r dbq.RecommendationWeightProfile) recommendation.ScoringWeights { + return recommendation.ScoringWeights{ + BaseWeight: r.BaseWeight, + LikeBoost: r.LikeBoost, + RecencyWeight: r.RecencyWeight, + SkipPenalty: r.SkipPenalty, + JitterMagnitude: r.JitterMagnitude, + ContextWeight: r.ContextWeight, + SimilarityWeight: r.SimilarityWeight, + TasteWeight: r.TasteWeight, + } +} diff --git a/internal/recsettings/service_test.go b/internal/recsettings/service_test.go new file mode 100644 index 00000000..65019ff3 --- /dev/null +++ b/internal/recsettings/service_test.go @@ -0,0 +1,222 @@ +package recsettings + +import ( + "context" + "encoding/json" + "errors" + "io" + "log/slog" + "os" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" +) + +func newPool(t *testing.T) *pgxpool.Pool { + t.Helper() + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + if err := db.Migrate(dsn, logger); err != nil { + t.Fatalf("migrate: %v", err) + } + pool, err := pgxpool.New(context.Background(), dsn) + if err != nil { + t.Fatalf("pool: %v", err) + } + t.Cleanup(pool.Close) + dbtest.ResetDB(t, pool) + return pool +} + +func newService(t *testing.T, pool *pgxpool.Pool) *Service { + t.Helper() + s, err := New(context.Background(), pool, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatalf("recsettings.New: %v", err) + } + return s +} + +func auditRows(t *testing.T, pool *pgxpool.Pool) []struct { + Scope, Action string + Changes []fieldChange +} { + t.Helper() + rows, err := pool.Query(context.Background(), + `SELECT scope, action, changes FROM recommendation_tuning_audit ORDER BY id`) + if err != nil { + t.Fatalf("query audit: %v", err) + } + defer rows.Close() + var out []struct { + Scope, Action string + Changes []fieldChange + } + for rows.Next() { + var scope, action string + var raw []byte + if err := rows.Scan(&scope, &action, &raw); err != nil { + t.Fatalf("scan audit: %v", err) + } + var changes []fieldChange + if err := json.Unmarshal(raw, &changes); err != nil { + t.Fatalf("unmarshal audit changes: %v", err) + } + out = append(out, struct { + Scope, Action string + Changes []fieldChange + }{scope, action, changes}) + } + return out +} + +func TestNew_SeedsShippedDefaults(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + if got := s.Weights(ScopeRadio); got != ShippedRadioWeights() { + t.Errorf("radio weights = %+v, want shipped defaults", got) + } + if got := s.Weights(ScopeDailyMix); got != ShippedDailyMixWeights() { + t.Errorf("daily_mix weights = %+v, want shipped defaults", got) + } + if got := s.Taste(); got != ShippedTasteTuning() { + t.Errorf("taste tuning = %+v, want shipped defaults", got) + } + // Seeding must not write audit rows — nothing changed. + if rows := auditRows(t, pool); len(rows) != 0 { + t.Errorf("boot reconcile wrote %d audit rows, want 0", len(rows)) + } +} + +func TestUpdateProfile_PersistsAndAudits(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + if err := s.UpdateProfile(context.Background(), ScopeRadio, + map[string]float64{"taste_weight": 2.5, "skip_penalty": 3.0}); err != nil { + t.Fatalf("UpdateProfile: %v", err) + } + got := s.Weights(ScopeRadio) + if got.TasteWeight != 2.5 || got.SkipPenalty != 3.0 { + t.Errorf("weights = %+v, want taste 2.5 / skip 3.0", got) + } + // The other profile is untouched. + if s.Weights(ScopeDailyMix) != ShippedDailyMixWeights() { + t.Error("daily_mix must be unaffected by a radio update") + } + // Values survive a fresh boot (persisted, not just cached), and the + // reconcile's ON CONFLICT DO NOTHING doesn't clobber tuned rows. + s2 := newService(t, pool) + if got := s2.Weights(ScopeRadio); got.TasteWeight != 2.5 { + t.Errorf("rebooted taste_weight = %v, want 2.5", got.TasteWeight) + } + rows := auditRows(t, pool) + if len(rows) != 1 { + t.Fatalf("audit rows = %d, want 1", len(rows)) + } + if rows[0].Scope != ScopeRadio || rows[0].Action != "update" || len(rows[0].Changes) != 2 { + t.Errorf("audit row = %+v, want radio/update with 2 changes", rows[0]) + } + // Changes are field-sorted: skip_penalty before taste_weight. + if rows[0].Changes[0].Field != "skip_penalty" || rows[0].Changes[0].New != 3.0 { + t.Errorf("changes[0] = %+v, want skip_penalty → 3.0", rows[0].Changes[0]) + } +} + +func TestUpdateProfile_Validation(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + cases := []struct { + name string + scope string + patch map[string]float64 + want error + }{ + {"unknown scope", "banana", map[string]float64{"taste_weight": 1}, ErrUnknownScope}, + {"unknown field", ScopeRadio, map[string]float64{"vibes": 1}, ErrUnknownField}, + {"over bound", ScopeRadio, map[string]float64{"taste_weight": 11}, ErrOutOfRange}, + {"negative jitter", ScopeRadio, map[string]float64{"jitter_magnitude": -0.1}, ErrOutOfRange}, + } + for _, c := range cases { + if err := s.UpdateProfile(context.Background(), c.scope, c.patch); !errors.Is(err, c.want) { + t.Errorf("%s: err = %v, want %v", c.name, err, c.want) + } + } + if s.Weights(ScopeRadio) != ShippedRadioWeights() { + t.Error("rejected patches must not partially apply") + } + if rows := auditRows(t, pool); len(rows) != 0 { + t.Errorf("rejected patches wrote %d audit rows, want 0", len(rows)) + } +} + +func TestUpdateTaste_CurveOrderingEnforced(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + // Moving neutral above full must reject on the PATCHED result. + err := s.UpdateTaste(context.Background(), map[string]float64{"engagement_neutral": 0.95}) + if !errors.Is(err, ErrOutOfRange) { + t.Fatalf("err = %v, want ErrOutOfRange (curve ordering)", err) + } + // A coherent multi-point move is fine. + if err := s.UpdateTaste(context.Background(), map[string]float64{ + "engagement_neutral": 0.40, "engagement_full": 0.95, "half_life_days": 30, + }); err != nil { + t.Fatalf("UpdateTaste: %v", err) + } + cfg := s.TasteConfig() + if cfg.HalfLifeDays != 30 || cfg.Engagement.NeutralCompletion != 0.40 { + t.Errorf("taste config = %+v, want tuned values", cfg) + } + // WindowDays scales with the half-life at the shipped ratio. + if cfg.WindowDays != 30*3.6 { + t.Errorf("WindowDays = %v, want %v", cfg.WindowDays, 30*3.6) + } +} + +func TestReset_RestoresShippedAndAudits(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + // Reset with nothing changed is a no-op (no audit row). + if err := s.Reset(context.Background(), ScopeDailyMix); err != nil { + t.Fatalf("no-op reset: %v", err) + } + if rows := auditRows(t, pool); len(rows) != 0 { + t.Fatalf("no-op reset wrote audit rows") + } + if err := s.UpdateProfile(context.Background(), ScopeDailyMix, + map[string]float64{"similarity_weight": 4}); err != nil { + t.Fatalf("UpdateProfile: %v", err) + } + if err := s.Reset(context.Background(), ScopeDailyMix); err != nil { + t.Fatalf("Reset: %v", err) + } + if s.Weights(ScopeDailyMix) != ShippedDailyMixWeights() { + t.Error("reset must restore shipped defaults") + } + rows := auditRows(t, pool) + if len(rows) != 2 || rows[1].Action != "reset" { + t.Fatalf("audit rows = %+v, want update then reset", rows) + } +} + +func TestUpdate_NoOpWritesNoAudit(t *testing.T) { + pool := newPool(t) + s := newService(t, pool) + shipped := ShippedRadioWeights() + if err := s.UpdateProfile(context.Background(), ScopeRadio, + map[string]float64{"taste_weight": shipped.TasteWeight}); err != nil { + t.Fatalf("UpdateProfile: %v", err) + } + if rows := auditRows(t, pool); len(rows) != 0 { + t.Errorf("no-op update wrote %d audit rows, want 0", len(rows)) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 248340c1..05b3cb65 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -27,6 +27,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/mailer" "git.fabledsword.com/bvandeusen/minstrel/internal/playevents" "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" + "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" "git.fabledsword.com/bvandeusen/minstrel/internal/subsonic" "git.fabledsword.com/bvandeusen/minstrel/internal/tracks" "git.fabledsword.com/bvandeusen/minstrel/web" @@ -88,6 +89,13 @@ type Server struct { // PUT /api/me/timezone and POST /api/auth/register can call // Refresh synchronously. PlaylistScheduler *playlists.Scheduler + // RecSettings is the DB-backed recommendation tuning lab (#1250): + // scoring-weight profiles + taste-build knobs. Constructed in + // cmd/minstrel/main.go (it pushes daily-mix weights into package + // playlists at boot); the API layer reads radio weights per request + // and serves the admin tuning endpoints from it. Router() constructs + // a fallback when nil (tests). + RecSettings *recsettings.Service // StreamSecret is the HMAC key used by /api/cast/stream-token to // mint signed UPnP / Sonos stream URLs and by /api/tracks/{id}/stream // to verify them. Sourced from config.Config.StreamSecret. Tests that @@ -143,7 +151,18 @@ func (s *Server) Router() http.Handler { if bus == nil { bus = eventbus.New() } - api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.LibraryScanner, s.ScanCfg, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret) + recSettings := s.RecSettings + if recSettings == nil { + // Test contexts construct Server directly without main.go's + // boot wiring; reconcile here so radio + the admin tuning + // endpoints work against the same pool. + var err error + recSettings, err = recsettings.New(context.Background(), s.Pool, s.Logger) + if err != nil { + s.Logger.Error("server: recsettings boot failed", "err", err) + } + } + api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, recSettings, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.LibraryScanner, s.ScanCfg, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret) // /api/admin/scan is the only admin route owned by the server package // (it needs the Scanner). Register it as a single inline-middleware // route — using r.Route("/api/admin", ...) here would create a second diff --git a/web/src/lib/api/metrics.ts b/web/src/lib/api/metrics.ts index be1fb87d..3f467d30 100644 --- a/web/src/lib/api/metrics.ts +++ b/web/src/lib/api/metrics.ts @@ -1,18 +1,36 @@ import { createQuery } from '@tanstack/svelte-query'; import { api } from './client'; -// Mirrors internal/api/me_recommendation_metrics.go. -export type RecommendationMetric = { - source: string; +// Mirrors internal/api/me_recommendation_metrics.go: raw play sources are +// bucketed server-side into stable surface families, grouped by intent, and +// anchored by the manual-plays baseline (milestone #127). +export type SurfaceMetric = { + key: string; + label: string; plays: number; skips: number; skip_rate: number; avg_completion: number; + low_confidence: boolean; + // Present when the surface's builder stamps pick-kind provenance and + // the window holds attributed plays (#1249, generalized #1270): For + // You's taste/fresh split, Discover's candidate buckets, the tiered + // mixes' tier1-3 — plus pre-attribution plays. + breakdown?: SurfaceMetric[]; +}; + +export type SurfaceIntent = 'go_to' | 'discovery' | 'direct'; + +export type SurfaceGroup = { + intent: SurfaceIntent; + label: string; + surfaces: SurfaceMetric[]; }; export type RecommendationMetrics = { window_days: number; - sources: RecommendationMetric[]; + baseline: SurfaceMetric | null; + groups: SurfaceGroup[]; }; export function getRecommendationMetrics(): Promise { @@ -28,19 +46,3 @@ export function createRecommendationMetricsQuery() { staleTime: 60_000 }); } - -// Friendly labels for the system-playlist source keys (play_events.source). -const SOURCE_LABELS: Record = { - for_you: 'For You', - discover: 'Discover', - deep_cuts: 'Deep cuts', - rediscover: 'Rediscover', - new_for_you: 'New for you', - on_this_day: 'On this day', - first_listens: 'First listens', - songs_like_artist: 'Songs like…' -}; - -export function sourceLabel(source: string): string { - return SOURCE_LABELS[source] ?? source; -} diff --git a/web/src/lib/api/tuning.ts b/web/src/lib/api/tuning.ts new file mode 100644 index 00000000..8ffd0218 --- /dev/null +++ b/web/src/lib/api/tuning.ts @@ -0,0 +1,86 @@ +import { api } from './client'; + +// Mirrors internal/api/admin_recommendation_tuning.go (#1250): the +// recommendation tuning lab. Field names are the server's snake_case +// keys, used verbatim in PATCH bodies. + +export type WeightProfile = { + base_weight: number; + like_boost: number; + recency_weight: number; + skip_penalty: number; + jitter_magnitude: number; + context_weight: number; + similarity_weight: number; + taste_weight: number; +}; + +export type TasteTuning = { + half_life_days: number; + engagement_hard_skip: number; + engagement_neutral: number; + engagement_full: number; +}; + +export type TuningScope = 'radio' | 'daily_mix' | 'taste'; + +export type TuningSnapshot = { + profiles: Record<'radio' | 'daily_mix', WeightProfile>; + taste: TasteTuning; + shipped: { + profiles: Record<'radio' | 'daily_mix', WeightProfile>; + taste: TasteTuning; + }; +}; + +export function getTuning(): Promise { + return api.get('/api/admin/recommendation-tuning'); +} + +export function patchTuning( + scope: TuningScope, + values: Record +): Promise { + return api.patch(`/api/admin/recommendation-tuning/${scope}`, { values }); +} + +export function resetTuning(scope: TuningScope): Promise { + return api.post(`/api/admin/recommendation-tuning/${scope}/reset`, {}); +} + +// Trends (#1251): weekly per-surface outcome series with knob-turn +// markers. Mirrors internal/api/admin_recommendation_trends.go. + +export type TrendPoint = { + week_start: string; + plays: number; + skips: number; + skip_rate: number; + avg_completion: number; + taste_hit_rate: number; +}; + +export type TrendSeries = { + key: string; + label: string; + intent: string; + plays: number; + points: TrendPoint[]; +}; + +export type TrendMarker = { + changed_at: string; + scope: string; + action: string; + changes: { field: string; old: number; new: number }[]; +}; + +export type TrendsResponse = { + weeks: number; + series: TrendSeries[]; + markers: TrendMarker[]; +}; + +export function getTrends(weeks = 12): Promise { + return api.get(`/api/admin/recommendation-trends?weeks=${weeks}`); +} diff --git a/web/src/lib/components/AdminTabs.svelte b/web/src/lib/components/AdminTabs.svelte index f7acbabd..08a21264 100644 --- a/web/src/lib/components/AdminTabs.svelte +++ b/web/src/lib/components/AdminTabs.svelte @@ -10,6 +10,7 @@ { href: '/admin/quarantine', label: 'Quarantine' }, { href: '/admin/playback-errors', label: 'Playback errors' }, { href: '/admin/diagnostics', label: 'Diagnostics' }, + { href: '/admin/tuning', label: 'Tuning' }, { href: '/admin/users', label: 'Users' } ]; diff --git a/web/src/lib/components/AdminTabs.test.ts b/web/src/lib/components/AdminTabs.test.ts index 2163c178..171b4d3e 100644 --- a/web/src/lib/components/AdminTabs.test.ts +++ b/web/src/lib/components/AdminTabs.test.ts @@ -52,7 +52,7 @@ describe('AdminTabs', () => { ); }); - test('renders all seven tabs in order', () => { + test('renders all eight tabs in order', () => { state.pageUrl = new URL('http://localhost/admin'); render(AdminTabs); const links = screen.getAllByRole('link'); @@ -63,6 +63,7 @@ describe('AdminTabs', () => { 'Quarantine', 'Playback errors', 'Diagnostics', + 'Tuning', 'Users' ]); }); diff --git a/web/src/routes/admin/tuning/+page.svelte b/web/src/routes/admin/tuning/+page.svelte new file mode 100644 index 00000000..ef7a9d7a --- /dev/null +++ b/web/src/routes/admin/tuning/+page.svelte @@ -0,0 +1,434 @@ + + + + {pageTitle('Tuning')} + + +
+
+

Recommendation tuning

+

+ The defaults-discovery lab. Changes apply live — radio on the next request, daily mixes on + the next rebuild — and every change is recorded so the metrics page can tie outcome shifts + to knob turns. Found-good values get baked into shipped defaults; a dot marks knobs that + currently deviate from them. +

+
+ + {#if loadFailed} +

Couldn't load tuning settings.

+ {:else if !snapshot} +

Loading…

+ {:else} +
+ {#each profileScopes as p (p.scope)} +
+
+

{p.label}

+

{p.blurb}

+
+
+ {#each weightFields as f (f.key)} +
+ + +
+ {/each} +
+
+ + +
+
+ {/each} +
+ +
+
+

Taste profile build

+

+ How plays become the learned taste profile: the influence half-life and the + completion→engagement curve (must stay ordered: hard-skip < neutral < full). +

+
+
+ {#each tasteFields as f (f.key)} +
+ + +
+ {/each} +
+
+ + +
+
+ {/if} + + +
+
+

Weekly trends

+

+ Skip rate per surface over the last {trends?.weeks ?? 12} weeks (lower is better; all + users aggregated, rates only). Dashed ticks mark tuning changes. Taste hit is the share + of plays whose artist fits the current taste profile. +

+
+ {#if trendsFailed} +

Couldn't load trends.

+ {:else if !trends} +

Loading…

+ {:else if trends.series.length === 0} +

+ No plays recorded yet — trends appear once listening accumulates. +

+ {:else} + + + + + + + + + + + + + {#each trends.series as s (s.key)} + + + + + + + + + {/each} + +
SurfaceSkip rate by weekPlaysLatest skipLatest completionTaste hit
{s.label} + + + {#each trends.markers as m, i (i)} + + {markerSummary(m)} + + {/each} + {#if s.points.length > 1} + + {:else if s.points.length === 1} + + {/if} + + {s.plays}{pct(latest(s).skip)}{pct(latest(s).completion)}{pct(windowTasteHitRate(s))}
+ {#if trends.markers.length > 0} +
+

Tuning changes in this window

+
    + {#each trends.markers as m, i (i)} +
  • {markerSummary(m)}
  • + {/each} +
+
+ {/if} + {/if} +
+
diff --git a/web/src/routes/admin/tuning/tuning.test.ts b/web/src/routes/admin/tuning/tuning.test.ts new file mode 100644 index 00000000..d953d164 --- /dev/null +++ b/web/src/routes/admin/tuning/tuning.test.ts @@ -0,0 +1,197 @@ +import { describe, expect, test, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; +import type { TuningSnapshot, TrendsResponse } from '$lib/api/tuning'; + +vi.mock('$lib/api/tuning', async (orig) => { + const actual = (await orig()) as Record; + return { + ...actual, + getTuning: vi.fn(), + patchTuning: vi.fn(), + resetTuning: vi.fn(), + getTrends: vi.fn() + }; +}); + +const pushToast = vi.fn(); +vi.mock('$lib/stores/toast.svelte', () => ({ pushToast: (...a: unknown[]) => pushToast(...a) })); + +import TuningPage from './+page.svelte'; +import { getTuning, patchTuning, resetTuning, getTrends } from '$lib/api/tuning'; + +const weights = (over: Partial> = {}) => ({ + base_weight: 1, + like_boost: 2, + recency_weight: 1, + skip_penalty: 2, + jitter_magnitude: 0.1, + context_weight: 0.5, + similarity_weight: 1.5, + taste_weight: 1.5, + ...over +}); + +const taste = (over: Partial> = {}) => ({ + half_life_days: 75, + engagement_hard_skip: 0.05, + engagement_neutral: 0.3, + engagement_full: 0.9, + ...over +}); + +function snapshot(over: Partial = {}): TuningSnapshot { + return { + profiles: { radio: weights({ taste_weight: 1 }), daily_mix: weights() }, + taste: taste(), + shipped: { + profiles: { radio: weights({ taste_weight: 1 }), daily_mix: weights() }, + taste: taste() + }, + ...over + } as TuningSnapshot; +} + +const emptyTrends: TrendsResponse = { weeks: 12, series: [], markers: [] }; + +beforeEach(() => { + vi.clearAllMocks(); + (getTrends as ReturnType).mockResolvedValue(emptyTrends); +}); + +describe('Admin tuning page', () => { + test('renders both profiles and the taste card with current values', async () => { + (getTuning as ReturnType).mockResolvedValue(snapshot()); + render(TuningPage); + await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument()); + expect(screen.getByText('Daily mixes')).toBeInTheDocument(); + expect(screen.getByText('Taste profile build')).toBeInTheDocument(); + const radioTaste = screen.getByLabelText(/taste weight/i, { + selector: '#radio-taste_weight' + }) as HTMLInputElement; + expect(radioTaste.value).toBe('1'); + const halfLife = document.getElementById('taste-half_life_days') as HTMLInputElement; + expect(halfLife.value).toBe('75'); + }); + + test('save sends only the changed fields for the scope', async () => { + (getTuning as ReturnType).mockResolvedValue(snapshot()); + (patchTuning as ReturnType).mockResolvedValue(snapshot()); + render(TuningPage); + await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument()); + const input = document.getElementById('radio-taste_weight') as HTMLInputElement; + await fireEvent.input(input, { target: { value: '2.5' } }); + await fireEvent.click(screen.getByRole('button', { name: /save radio/i })); + await waitFor(() => + expect(patchTuning).toHaveBeenCalledWith('radio', { taste_weight: 2.5 }) + ); + }); + + test('save with no changes calls nothing and says so', async () => { + (getTuning as ReturnType).mockResolvedValue(snapshot()); + render(TuningPage); + await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument()); + await fireEvent.click(screen.getByRole('button', { name: /save radio/i })); + expect(patchTuning).not.toHaveBeenCalled(); + await waitFor(() => expect(pushToast).toHaveBeenCalledWith('Nothing changed.')); + }); + + test('reset calls resetTuning for the scope', async () => { + (getTuning as ReturnType).mockResolvedValue(snapshot()); + (resetTuning as ReturnType).mockResolvedValue(snapshot()); + render(TuningPage); + await waitFor(() => expect(screen.getByText('Taste profile build')).toBeInTheDocument()); + const resetButtons = screen.getAllByRole('button', { name: /reset to defaults/i }); + await fireEvent.click(resetButtons[resetButtons.length - 1]); + await waitFor(() => expect(resetTuning).toHaveBeenCalledWith('taste')); + }); + + test('marks knobs that deviate from shipped defaults', async () => { + const snap = snapshot(); + snap.profiles.daily_mix = weights({ similarity_weight: 4 }); + (getTuning as ReturnType).mockResolvedValue(snap); + render(TuningPage); + await waitFor(() => expect(screen.getByText('Daily mixes')).toBeInTheDocument()); + expect( + screen.getByTitle(/deviates from the shipped default \(1\.5\)/i) + ).toBeInTheDocument(); + }); + + test('renders weekly trend rows with sparklines and knob-turn markers (#1251)', async () => { + (getTuning as ReturnType).mockResolvedValue(snapshot()); + (getTrends as ReturnType).mockResolvedValue({ + weeks: 12, + series: [ + { + key: 'radio', + label: 'Radio', + intent: 'go_to', + plays: 40, + points: [ + { + week_start: '2026-06-22', + plays: 25, + skips: 5, + skip_rate: 0.2, + avg_completion: 0.8, + taste_hit_rate: 0.6 + }, + { + week_start: '2026-06-29', + plays: 15, + skips: 6, + skip_rate: 0.4, + avg_completion: 0.7, + taste_hit_rate: 0.5 + } + ] + }, + { + key: 'discover', + label: 'Discover', + intent: 'discovery', + plays: 5, + points: [ + { + week_start: '2026-06-29', + plays: 5, + skips: 3, + skip_rate: 0.6, + avg_completion: 0.4, + taste_hit_rate: 0.2 + } + ] + } + ], + markers: [ + { + changed_at: '2026-06-30T10:00:00Z', + scope: 'radio', + action: 'update', + changes: [{ field: 'taste_weight', old: 1, new: 2 }] + } + ] + } satisfies TrendsResponse); + render(TuningPage); + await waitFor(() => expect(screen.getByText('Weekly trends')).toBeInTheDocument()); + expect(screen.getByTestId('sparkline-radio')).toBeInTheDocument(); + expect(screen.getByTestId('sparkline-discover')).toBeInTheDocument(); + // Latest skip rate column for radio = 40% (also discover's latest + // completion, hence getAllBy). + expect(screen.getAllByText('40%').length).toBeGreaterThan(0); + // The knob turn is listed under the chart AND tooltipped on each + // sparkline's marker tick, hence getAllBy. + expect( + screen.getAllByText(/2026-06-30 — radio: taste_weight 1→2/).length + ).toBeGreaterThan(0); + // Low-volume series (5 plays) is dimmed as anecdote. + expect(screen.getByTitle(/fewer than 20 plays in the window/i)).toBeInTheDocument(); + }); + + test('empty trends show the accumulation hint', async () => { + (getTuning as ReturnType).mockResolvedValue(snapshot()); + render(TuningPage); + await waitFor(() => + expect(screen.getByText(/trends appear once listening accumulates/i)).toBeInTheDocument() + ); + }); +}); diff --git a/web/src/routes/settings/+page.svelte b/web/src/routes/settings/+page.svelte index eef29ee9..3c6c0d23 100644 --- a/web/src/routes/settings/+page.svelte +++ b/web/src/routes/settings/+page.svelte @@ -10,8 +10,9 @@ } from '$lib/api/listenbrainz'; import { createRecommendationMetricsQuery, - sourceLabel, - type RecommendationMetrics + type RecommendationMetrics, + type SurfaceIntent, + type SurfaceMetric } from '$lib/api/metrics'; import { theme, setTheme, type ThemePreference } from '$lib/stores/theme.svelte'; import { player, setCrossfade } from '$lib/player/store.svelte'; @@ -30,6 +31,46 @@ const status = createLBStatusQuery() as CreateQueryResult; const metrics = createRecommendationMetricsQuery() as CreateQueryResult; + + // Per-intent expectation copy: each surface band is judged against its + // job — discovery mixes are supposed to run hotter skip rates. + const intentHints: Record = { + go_to: 'Everyday surfaces — these should beat your own picking.', + discovery: 'Exploration surfaces — higher skip rates here are expected and healthy.', + direct: 'Music you chose yourself — naturally close to the baseline.' + }; + + function pct(v: number): string { + return `${(v * 100).toFixed(0)}%`; + } + + // Delta in percentage points vs the baseline, signed ("+12" / "−5"). + function deltaPts(value: number, baseline: number): string { + const pts = Math.round((value - baseline) * 100); + return pts > 0 ? `+${pts}` : `${pts}`; + } + + // A surface's skip delta is "worse" when it skips more than the + // baseline; completion delta is "worse" when it completes less. + function skipDeltaClass(m: SurfaceMetric, baseline: SurfaceMetric): string { + return m.skip_rate > baseline.skip_rate ? 'text-danger' : 'text-text-secondary'; + } + + function completionDeltaClass(m: SurfaceMetric, baseline: SurfaceMetric): string { + return m.avg_completion < baseline.avg_completion ? 'text-danger' : 'text-text-secondary'; + } + + // Pick-kind breakdowns are collapsed by default (#1270): with every + // system mix stamping provenance, always-open sub-rows would triple + // the table height and bury the surface-level comparison. + let expandedBreakdowns = $state(new Set()); + + function toggleBreakdown(key: string) { + const next = new Set(expandedBreakdowns); + if (next.has(key)) next.delete(key); + else next.add(key); + expandedBreakdowns = next; + } const tokenMutation = createTokenMutation(queryClient); const enabledMutation = createEnabledMutation(queryClient); @@ -278,37 +319,131 @@

Recommendation metrics

How plays launched from each recommendation surface land, over the last - {$metrics.data?.window_days ?? 30} days. Lower skip rate and higher average - completion mean the surface is hitting. + {$metrics.data?.window_days ?? 30} days — compared against the baseline of + music you picked yourself. Deltas are percentage points vs that baseline.

{#if $metrics.isPending}

Loading…

{:else if $metrics.isError}

Couldn't load metrics.

- {:else if $metrics.data && $metrics.data.sources.length > 0} - - - - - - - - - - - {#each $metrics.data.sources as m (m.source)} - - - - - - - {/each} - -
SurfacePlaysSkip rateAvg completion
{sourceLabel(m.source)}{m.plays}{(m.skip_rate * 100).toFixed(0)}%{(m.avg_completion * 100).toFixed(0)}%
+ {:else if $metrics.data && ($metrics.data.groups.length > 0 || $metrics.data.baseline)} + {@const baseline = $metrics.data.baseline} + {#if baseline} +

+ Baseline — manual plays: + + {baseline.plays} plays · {pct(baseline.skip_rate)} skip · + {pct(baseline.avg_completion)} completion + +

+ {:else} +

+ No manual plays in this window yet, so deltas are hidden — raw rates only. +

+ {/if} + {#each $metrics.data.groups as group (group.intent)} +
+

{group.label}

+

{intentHints[group.intent]}

+ + + + + + + + + + + {#each group.surfaces as m (m.key)} + + + + + + + + {#if expandedBreakdowns.has(m.key)} + {#each m.breakdown ?? [] as b (b.key)} + + + + + + + {/each} + {/if} + {/each} + +
SurfacePlaysSkip rateAvg completion
+ {#if (m.breakdown ?? []).length > 0} + + {:else} + {m.label} + {/if}{#if m.low_confidence}· low data{/if} + {m.plays} + {pct(m.skip_rate)} + {#if baseline} + + {deltaPts(m.skip_rate, baseline.skip_rate)} + + {/if} + + {pct(m.avg_completion)} + {#if baseline} + + {deltaPts(m.avg_completion, baseline.avg_completion)} + + {/if} +
+ ↳ {b.label}{#if b.low_confidence}· low data{/if} + {b.plays} + {pct(b.skip_rate)} + {#if baseline} + + {deltaPts(b.skip_rate, baseline.skip_rate)} + + {/if} + + {pct(b.avg_completion)} + {#if baseline} + + {deltaPts(b.avg_completion, baseline.avg_completion)} + + {/if} +
+
+ {/each} {:else}

- No recommendation plays yet. Play something from For You, Discover, or a mix. + No plays recorded yet. Play something from For You, Discover, or a mix.

{/if} diff --git a/web/src/routes/settings/Appearance.test.ts b/web/src/routes/settings/Appearance.test.ts index d254e97b..67407e35 100644 --- a/web/src/routes/settings/Appearance.test.ts +++ b/web/src/routes/settings/Appearance.test.ts @@ -28,11 +28,10 @@ vi.mock('$lib/api/listenbrainz', () => { vi.mock('$lib/api/metrics', () => ({ createRecommendationMetricsQuery: () => ({ subscribe: (run: (v: unknown) => void) => { - run({ isPending: false, isError: false, data: { window_days: 30, sources: [] } }); + run({ isPending: false, isError: false, data: { window_days: 30, baseline: null, groups: [] } }); return () => {}; } - }), - sourceLabel: (s: string) => s + }) })); beforeEach(() => { diff --git a/web/src/routes/settings/settings.test.ts b/web/src/routes/settings/settings.test.ts index ae913c07..fcd21fe5 100644 --- a/web/src/routes/settings/settings.test.ts +++ b/web/src/routes/settings/settings.test.ts @@ -20,14 +20,19 @@ vi.mock('$lib/api/me', () => ({ regenerateAPIToken: vi.fn() })); +// Mutable holder so individual tests can inject populated metrics; +// vi.mock is hoisted, hence vi.hoisted for the shared reference. +const metricsMock = vi.hoisted(() => ({ + data: { window_days: 30, baseline: null, groups: [] } as unknown +})); + vi.mock('$lib/api/metrics', () => ({ createRecommendationMetricsQuery: () => ({ subscribe: (run: (v: unknown) => void) => { - run({ isPending: false, isError: false, data: { window_days: 30, sources: [] } }); + run({ isPending: false, isError: false, data: metricsMock.data }); return () => {}; } - }), - sourceLabel: (s: string) => s + }) })); import SettingsPage from './+page.svelte'; @@ -53,7 +58,10 @@ function mockMutationStore(mutateFn: (vars: unknown) => void = vi.fn()) { return readable({ isPending: false, mutate: mutateFn, mutateAsync: vi.fn() }); } -afterEach(() => vi.clearAllMocks()); +afterEach(() => { + vi.clearAllMocks(); + metricsMock.data = { window_days: 30, baseline: null, groups: [] }; +}); describe('Settings page — ListenBrainz', () => { test('token-not-set state renders input + Save button', async () => { @@ -178,6 +186,81 @@ describe('Settings page — Password card', () => { }); }); +describe('Settings page — Recommendation metrics card', () => { + const metric = (key: string, label: string, over: Record = {}) => ({ + key, + label, + plays: 30, + skips: 3, + skip_rate: 0.1, + avg_completion: 0.9, + low_confidence: false, + ...over + }); + + test('breakdown rows are collapsed by default and expand on toggle (#1249/#1270)', async () => { + setupPage(); + metricsMock.data = { + window_days: 30, + baseline: metric('manual', 'Manual library plays', { plays: 100 }), + groups: [ + { + intent: 'go_to', + label: 'Go-to surfaces', + surfaces: [ + metric('for_you', 'For You', { + plays: 45, + breakdown: [ + metric('for_you_taste', 'Taste picks'), + metric('for_you_fresh', 'Fresh picks', { + plays: 10, + skip_rate: 0.4, + low_confidence: true + }), + metric('for_you_unattributed', 'Earlier plays', { plays: 5 }) + ] + }) + ] + } + ] + }; + render(SettingsPage); + const toggle = await screen.findByRole('button', { name: /for you/i }); + // Collapsed by default: every stamping mix now carries a breakdown, + // so always-open sub-rows would swamp the surface-level view. + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + expect(screen.queryByText(/Taste picks/)).not.toBeInTheDocument(); + + await fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'true'); + expect(screen.getByText(/Taste picks/)).toBeInTheDocument(); + expect(screen.getByText(/Fresh picks/)).toBeInTheDocument(); + expect(screen.getByText(/Earlier plays/)).toBeInTheDocument(); + + await fireEvent.click(toggle); + expect(screen.queryByText(/Taste picks/)).not.toBeInTheDocument(); + }); + + test('surfaces without a breakdown render no toggle and no sub-rows', async () => { + setupPage(); + metricsMock.data = { + window_days: 30, + baseline: null, + groups: [ + { + intent: 'go_to', + label: 'Go-to surfaces', + surfaces: [metric('radio', 'Radio')] + } + ] + }; + render(SettingsPage); + await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument()); + expect(screen.queryByRole('button', { name: /radio/i })).not.toBeInTheDocument(); + expect(screen.queryByText(/Taste picks/)).not.toBeInTheDocument(); + }); +}); + describe('Settings page — API Token card', () => { test('first click on Regenerate shows "Click again to confirm"', async () => { setupPage();