From 5749f48b4ac72693e305b1c907d7680a79f04b92 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 14 Jul 2026 12:47:59 -0400 Subject: [PATCH] =?UTF-8?q?feat(taste):=20device-class=20context=20conditi?= =?UTF-8?q?oning=20=E2=80=94=20#1551?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Milestone #160 Opt 3b. Adds device class as a third context axis on top of the #1531 time-of-day/weekday affinity: on the radio path, a candidate is boosted when its artist concentrates in the current (daypart × weekday × device) cell. Client-sent (client_id is opaque; no UA stored), so it's captured going forward and applies to radio only (daily mixes are cron-built with no device → stay device-agnostic). Server: - Migration 0048: play_events.device_class text NULL (no CHECK; normalized in Go — one whitelist entry per new client class, not a migration). - events.go: eventRequest.device_class + normalizeDeviceClass (whitelist → mobile/web/…, else "other", empty → NULL); threaded through both RecordPlayStartedWithSource and RecordOfflinePlay into InsertPlayEvent. - ListArtistContextPlayCountsForUser gains a current-device param; the cell FILTER adds AND ($2='' OR device_class=$2) — '' reproduces the #1531 time-only behaviour exactly (used by mixes). SessionVector.DeviceClass carries it; the radio handler derives the current device from the user's latest play (GetLatestPlayDeviceClassForUser) — request-free proxy. - No new tuning knob: device narrows the existing ContextAffinityScore (reuses context_time_weight). Clients: - web: play_started sends device_class 'web'. - android: play_started + offline replay send 'mobile' (EventsWire + PlayOfflinePayload + MutationReplayer + PlayEventsReporter). Test: LoadContextAffinity device-narrowing integration test (mobile vs web artist separation; device-agnostic parity). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../minstrel/cache/mutations/MutationQueue.kt | 3 + .../cache/mutations/MutationReplayer.kt | 1 + .../minstrel/models/wire/EventsWire.kt | 4 + .../minstrel/player/PlayEventsReporter.kt | 8 ++ internal/api/events.go | 35 +++++++- internal/api/radio.go | 22 +++++ internal/db/dbq/events.sql.go | 34 +++++--- internal/db/dbq/models.go | 1 + internal/db/dbq/recommendation.sql.go | 43 +++++++-- .../0048_play_events_device_class.down.sql | 1 + .../0048_play_events_device_class.up.sql | 9 ++ internal/db/queries/events.sql | 6 +- internal/db/queries/recommendation.sql | 26 ++++-- internal/playevents/writer.go | 45 ++++++---- internal/playevents/writer_test.go | 18 ++-- internal/recommendation/candidates.go | 4 +- internal/recommendation/context.go | 13 ++- .../context_device_integration_test.go | 87 +++++++++++++++++++ internal/recommendation/sessionvector.go | 5 ++ web/src/lib/player/events.svelte.ts | 5 +- 20 files changed, 306 insertions(+), 64 deletions(-) create mode 100644 internal/db/migrations/0048_play_events_device_class.down.sql create mode 100644 internal/db/migrations/0048_play_events_device_class.up.sql create mode 100644 internal/recommendation/context_device_integration_test.go diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt index 607f2839..79dda6ce 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt @@ -239,6 +239,9 @@ data class PlayOfflinePayload( val atIso: String, val durationPlayedMs: Long, val source: String? = null, + // #1551: device class for context conditioning; null on payloads queued + // before this field existed (decodes to null → server stores NULL). + val deviceClass: String? = null, ) /** diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt index a09c1301..c3ddfb2d 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt @@ -254,6 +254,7 @@ class MutationReplayer @Inject constructor( at = decoded.atIso, durationPlayedMs = decoded.durationPlayedMs, source = decoded.source, + deviceClass = decoded.deviceClass, ), ) return Outcome.SENT diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/EventsWire.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/EventsWire.kt index 74d34d96..47072c12 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/EventsWire.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/EventsWire.kt @@ -27,6 +27,8 @@ data class PlayStartedRequest( @SerialName("track_id") val trackId: String, @SerialName("client_id") val clientId: String, val source: String? = null, + // #1551: device class for context conditioning (server normalizes). + @SerialName("device_class") val deviceClass: String? = null, ) @Serializable @@ -56,4 +58,6 @@ data class PlayOfflineRequest( val at: String, @SerialName("duration_played_ms") val durationPlayedMs: Long, val source: String? = null, + // #1551: device class for context conditioning (server normalizes). + @SerialName("device_class") val deviceClass: String? = null, ) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayEventsReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayEventsReporter.kt index 0325fba9..c62cdb32 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayEventsReporter.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayEventsReporter.kt @@ -155,6 +155,7 @@ class PlayEventsReporter @Inject constructor( trackId = trackId, clientId = cid, source = source.takeIf { !it.isNullOrEmpty() }, + deviceClass = DEVICE_CLASS, ), ) if (curTrackId == trackId) { @@ -248,6 +249,7 @@ class PlayEventsReporter @Inject constructor( atIso = startedAt.toString(), durationPlayedMs = durationPlayedMs, source = source.takeIf { !it.isNullOrEmpty() }, + deviceClass = DEVICE_CLASS, ), ) } @@ -268,6 +270,12 @@ class PlayEventsReporter @Inject constructor( // ── Lifecycle: durable-close on app background / detach ──────── + companion object { + // #1551: Android is a mobile client. Refine to tablet/tv via device + // configuration later if the metrics trend view shows it matters. + private const val DEVICE_CLASS = "mobile" + } + override fun onStop(owner: LifecycleOwner) { // App backgrounded. If the tracked play is still PLAYING, leave it // alone: the foreground media service keeps the process — and this diff --git a/internal/api/events.go b/internal/api/events.go index 150f534f..191d1688 100644 --- a/internal/api/events.go +++ b/internal/api/events.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "net/http" + "strings" "time" "github.com/jackc/pgx/v5" @@ -24,6 +25,33 @@ type eventRequest struct { // / "discover" feed the system-playlist rotation dedup (#415); // absent / "" for library, user-playlist, radio, Subsonic. Source *string `json:"source"` + // DeviceClass is the client's self-reported device class (#1551), + // normalized + stored per play to condition radio on the current device. + DeviceClass *string `json:"device_class"` +} + +// knownDeviceClasses whitelists the device classes the context-affinity facet +// buckets by (#1551). A client sends one; anything unrecognized normalizes to +// "other", and empty/absent to "" (stored NULL → excluded from the device +// dimension). Kept permissive (no DB CHECK) so a new client class is one +// whitelist entry, not a migration. +var knownDeviceClasses = map[string]bool{ + "mobile": true, "tablet": true, "desktop": true, + "web": true, "tv": true, "watch": true, +} + +func normalizeDeviceClass(dc *string) string { + if dc == nil { + return "" + } + s := strings.ToLower(strings.TrimSpace(*dc)) + if s == "" { + return "" + } + if knownDeviceClasses[s] { + return s + } + return "other" } type playStartedResponse struct { @@ -114,7 +142,9 @@ func (h *handlers) handleEventPlayStarted( if req.Source != nil { source = *req.Source } - res, err := h.events.RecordPlayStartedWithSource(r.Context(), user.ID, trackID, clientID, source, at) + deviceClass := normalizeDeviceClass(req.DeviceClass) + res, err := h.events.RecordPlayStartedWithSource( + r.Context(), user.ID, trackID, clientID, source, deviceClass, at) if err != nil { h.logger.Error("api: events: play_started", "err", err) writeErr(w, apierror.InternalMsg("record failed", err)) @@ -159,7 +189,8 @@ func (h *handlers) handleEventPlayOffline( source = *req.Source } if err := h.events.RecordOfflinePlay( - r.Context(), user.ID, trackID, clientID, source, at, *req.DurationPlayedMs, + r.Context(), user.ID, trackID, clientID, source, + normalizeDeviceClass(req.DeviceClass), at, *req.DurationPlayedMs, ); err != nil { h.logger.Error("api: events: play_offline", "err", err) writeErr(w, apierror.InternalMsg("record failed", err)) diff --git a/internal/api/radio.go b/internal/api/radio.go index 5aa68f6c..b39ae73f 100644 --- a/internal/api/radio.go +++ b/internal/api/radio.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "errors" "log/slog" @@ -81,6 +82,9 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) { } currentVec := loadCurrentSessionVector(r, q, user.ID, h.logger) + // Condition on the current device (#1551): the latest play's device is a + // cheap, request-free proxy for what the user is on right now. + currentVec.DeviceClass = latestDeviceClass(r.Context(), q, user.ID, h.logger) exclude := parseExcludeParam(r.URL.Query().Get("exclude")) limits := recommendation.DefaultCandidateSourceLimits() @@ -150,6 +154,24 @@ func loadCurrentSessionVector(r *http.Request, q *dbq.Queries, userID pgtype.UUI return v } +// latestDeviceClass returns the device_class of the user's most recent play as +// the "current device" for context conditioning (#1551), or "" when unknown +// (no plays yet, or the latest play predates device capture). Best-effort: a +// lookup failure yields a device-agnostic ("") affinity cell. +func latestDeviceClass(ctx context.Context, q *dbq.Queries, userID pgtype.UUID, logger *slog.Logger) string { + dc, err := q.GetLatestPlayDeviceClassForUser(ctx, userID) + if err != nil { + if !errors.Is(err, pgx.ErrNoRows) { + logger.Warn("api: radio: latest device class", "err", err) + } + return "" + } + if dc == nil { + return "" + } + return *dc +} + // parseExcludeParam parses a comma-separated list of UUIDs from the // `exclude` query string, silently dropping malformed entries. Returns // nil for empty or all-malformed input. diff --git a/internal/db/dbq/events.sql.go b/internal/db/dbq/events.sql.go index dfe68fcd..0fa111d6 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, pick_kind 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, device_class FROM play_events WHERE user_id = $1 AND ended_at IS NULL ORDER BY started_at DESC LIMIT 1 @@ -80,12 +80,13 @@ func (q *Queries) GetOpenPlayEventForUser(ctx context.Context, userID pgtype.UUI &i.ScrobbledAt, &i.Source, &i.PickKind, + &i.DeviceClass, ) 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, pick_kind 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, device_class FROM play_events WHERE id = $1 ` func (q *Queries) GetPlayEventByID(ctx context.Context, id pgtype.UUID) (PlayEvent, error) { @@ -106,6 +107,7 @@ func (q *Queries) GetPlayEventByID(ctx context.Context, id pgtype.UUID) (PlayEve &i.ScrobbledAt, &i.Source, &i.PickKind, + &i.DeviceClass, ) return i, err } @@ -144,19 +146,22 @@ func (q *Queries) GetSystemPickKindForTrack(ctx context.Context, arg GetSystemPi const insertPlayEvent = `-- name: InsertPlayEvent :one INSERT INTO play_events ( - 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 + user_id, track_id, session_id, started_at, client_id, source, pick_kind, + device_class +) VALUES ($1, $2, $3, $4, $5, $6, $7::text, + $8::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, device_class ` type InsertPlayEventParams struct { - UserID pgtype.UUID - TrackID pgtype.UUID - SessionID pgtype.UUID - StartedAt pgtype.Timestamptz - ClientID *string - Source *string - PickKind *string + UserID pgtype.UUID + TrackID pgtype.UUID + SessionID pgtype.UUID + StartedAt pgtype.Timestamptz + ClientID *string + Source *string + PickKind *string + DeviceClass *string } // pick_kind is non-NULL only for system-playlist plays whose track was @@ -171,6 +176,7 @@ func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams arg.ClientID, arg.Source, arg.PickKind, + arg.DeviceClass, ) var i PlayEvent err := row.Scan( @@ -188,6 +194,7 @@ func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams &i.ScrobbledAt, &i.Source, &i.PickKind, + &i.DeviceClass, ) return i, err } @@ -333,7 +340,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, pick_kind +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, device_class ` type UpdatePlayEventEndedParams struct { @@ -371,6 +378,7 @@ func (q *Queries) UpdatePlayEventEnded(ctx context.Context, arg UpdatePlayEventE &i.ScrobbledAt, &i.Source, &i.PickKind, + &i.DeviceClass, ) return i, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 6cc0e6b8..10e990ef 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -384,6 +384,7 @@ type PlayEvent struct { ScrobbledAt pgtype.Timestamptz Source *string PickKind *string + DeviceClass *string } type PlaySession struct { diff --git a/internal/db/dbq/recommendation.sql.go b/internal/db/dbq/recommendation.sql.go index f95f3125..13167583 100644 --- a/internal/db/dbq/recommendation.sql.go +++ b/internal/db/dbq/recommendation.sql.go @@ -11,6 +11,24 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) +const getLatestPlayDeviceClassForUser = `-- name: GetLatestPlayDeviceClassForUser :one +SELECT device_class +FROM play_events +WHERE user_id = $1 +ORDER BY started_at DESC +LIMIT 1 +` + +// The device_class of the user's most recent play (#1551), used as the +// "current device" for radio context conditioning. NULL when the latest play +// predates device capture or came from a client that didn't send one. +func (q *Queries) GetLatestPlayDeviceClassForUser(ctx context.Context, userID pgtype.UUID) (*string, error) { + row := q.db.QueryRow(ctx, getLatestPlayDeviceClassForUser, userID) + var device_class *string + err := row.Scan(&device_class) + return device_class, err +} + const listArtistContextPlayCountsForUser = `-- name: ListArtistContextPlayCountsForUser :many WITH tz AS ( SELECT COALESCE(NULLIF(u.timezone, ''), 'UTC') AS zone @@ -30,6 +48,7 @@ now_cell AS ( ), plays AS ( SELECT t.artist_id, + pe.device_class, CASE WHEN EXTRACT(hour FROM pe.started_at AT TIME ZONE tz.zone) < 5 THEN 3 WHEN EXTRACT(hour FROM pe.started_at AT TIME ZONE tz.zone) < 12 THEN 0 @@ -50,11 +69,17 @@ SELECT p.artist_id, count(*) FILTER ( WHERE p.daypart = (SELECT daypart FROM now_cell) AND p.is_weekend = (SELECT is_weekend FROM now_cell) + AND ($2::text = '' OR p.device_class = $2::text) ) AS cell_plays FROM plays p GROUP BY p.artist_id ` +type ListArtistContextPlayCountsForUserParams struct { + ID pgtype.UUID + Column2 string +} + type ListArtistContextPlayCountsForUserRow struct { ArtistID pgtype.UUID TotalPlays int64 @@ -62,14 +87,16 @@ type ListArtistContextPlayCountsForUserRow struct { } // Per-artist completed-play counts split by whether each play falls in the -// CURRENT daypart × weekday-type cell, in the user's local timezone (#1531). -// Feeds the context-affinity scoring term: an artist whose plays concentrate -// in the current cell (vs the user's overall baseline, computed Go-side from -// these rows) gets boosted right now. Skips excluded; a 365-day window bounds -// cost. Daypart buckets: night [22,5) morning [5,12) afternoon [12,17) -// evening [17,22). Weekend = ISO days 6–7 (Sat/Sun). -func (q *Queries) ListArtistContextPlayCountsForUser(ctx context.Context, id pgtype.UUID) ([]ListArtistContextPlayCountsForUserRow, error) { - rows, err := q.db.Query(ctx, listArtistContextPlayCountsForUser, id) +// CURRENT context cell, in the user's local timezone (#1531). The cell is +// daypart × weekday-type, optionally narrowed by device class (#1551): when +// $2 (the current device) is ”, the device dimension is ignored (identical to +// the time-only #1531 behaviour, used by the daily mixes which have no device); +// when $2 is set (radio), a play only counts in the cell if its device_class +// matches. Feeds the context-affinity scoring term. Skips excluded; a 365-day +// window bounds cost. Daypart buckets: night [22,5) morning [5,12) +// afternoon [12,17) evening [17,22). Weekend = ISO days 6–7 (Sat/Sun). +func (q *Queries) ListArtistContextPlayCountsForUser(ctx context.Context, arg ListArtistContextPlayCountsForUserParams) ([]ListArtistContextPlayCountsForUserRow, error) { + rows, err := q.db.Query(ctx, listArtistContextPlayCountsForUser, arg.ID, arg.Column2) if err != nil { return nil, err } diff --git a/internal/db/migrations/0048_play_events_device_class.down.sql b/internal/db/migrations/0048_play_events_device_class.down.sql new file mode 100644 index 00000000..4742e15a --- /dev/null +++ b/internal/db/migrations/0048_play_events_device_class.down.sql @@ -0,0 +1 @@ +ALTER TABLE play_events DROP COLUMN IF EXISTS device_class; diff --git a/internal/db/migrations/0048_play_events_device_class.up.sql b/internal/db/migrations/0048_play_events_device_class.up.sql new file mode 100644 index 00000000..d8072d25 --- /dev/null +++ b/internal/db/migrations/0048_play_events_device_class.up.sql @@ -0,0 +1,9 @@ +-- 0048_play_events_device_class.up.sql — device-class context conditioning +-- (#1551, milestone #160 Opt 3b). Records which class of device a play came +-- from (mobile / web / …), sent by the client on play-start and normalized +-- server-side. Extends the #1531 time-of-day/weekday affinity cell to +-- (daypart × weekday × device) for the radio path. Nullable + no CHECK: values +-- are normalized in Go (avoids a CHECK-migration each time a client class is +-- added, rule #36), and historical rows stay NULL → excluded from the device +-- dimension (a graceful cold-start; the facet ramps as new plays land). +ALTER TABLE play_events ADD COLUMN device_class text; diff --git a/internal/db/queries/events.sql b/internal/db/queries/events.sql index 565b510c..2be5df2f 100644 --- a/internal/db/queries/events.sql +++ b/internal/db/queries/events.sql @@ -28,8 +28,10 @@ LIMIT 1; -- 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, pick_kind -) VALUES ($1, $2, $3, $4, $5, $6, sqlc.narg(pick_kind)::text) + user_id, track_id, session_id, started_at, client_id, source, pick_kind, + device_class +) VALUES ($1, $2, $3, $4, $5, $6, sqlc.narg(pick_kind)::text, + sqlc.narg(device_class)::text) RETURNING *; -- name: GetSystemPickKindForTrack :one diff --git a/internal/db/queries/recommendation.sql b/internal/db/queries/recommendation.sql index 3cce5883..bf9e10f7 100644 --- a/internal/db/queries/recommendation.sql +++ b/internal/db/queries/recommendation.sql @@ -195,12 +195,14 @@ GROUP BY t.id, t.title, t.album_id, t.artist_id, t.duration_ms, t.file_path, -- name: ListArtistContextPlayCountsForUser :many -- Per-artist completed-play counts split by whether each play falls in the --- CURRENT daypart × weekday-type cell, in the user's local timezone (#1531). --- Feeds the context-affinity scoring term: an artist whose plays concentrate --- in the current cell (vs the user's overall baseline, computed Go-side from --- these rows) gets boosted right now. Skips excluded; a 365-day window bounds --- cost. Daypart buckets: night [22,5) morning [5,12) afternoon [12,17) --- evening [17,22). Weekend = ISO days 6–7 (Sat/Sun). +-- CURRENT context cell, in the user's local timezone (#1531). The cell is +-- daypart × weekday-type, optionally narrowed by device class (#1551): when +-- $2 (the current device) is '', the device dimension is ignored (identical to +-- the time-only #1531 behaviour, used by the daily mixes which have no device); +-- when $2 is set (radio), a play only counts in the cell if its device_class +-- matches. Feeds the context-affinity scoring term. Skips excluded; a 365-day +-- window bounds cost. Daypart buckets: night [22,5) morning [5,12) +-- afternoon [12,17) evening [17,22). Weekend = ISO days 6–7 (Sat/Sun). WITH tz AS ( SELECT COALESCE(NULLIF(u.timezone, ''), 'UTC') AS zone FROM users u WHERE u.id = $1 @@ -219,6 +221,7 @@ now_cell AS ( ), plays AS ( SELECT t.artist_id, + pe.device_class, CASE WHEN EXTRACT(hour FROM pe.started_at AT TIME ZONE tz.zone) < 5 THEN 3 WHEN EXTRACT(hour FROM pe.started_at AT TIME ZONE tz.zone) < 12 THEN 0 @@ -239,10 +242,21 @@ SELECT p.artist_id, count(*) FILTER ( WHERE p.daypart = (SELECT daypart FROM now_cell) AND p.is_weekend = (SELECT is_weekend FROM now_cell) + AND ($2::text = '' OR p.device_class = $2::text) ) AS cell_plays FROM plays p GROUP BY p.artist_id; +-- name: GetLatestPlayDeviceClassForUser :one +-- The device_class of the user's most recent play (#1551), used as the +-- "current device" for radio context conditioning. NULL when the latest play +-- predates device capture or came from a client that didn't send one. +SELECT device_class +FROM play_events +WHERE user_id = $1 +ORDER BY started_at DESC +LIMIT 1; + -- name: SuggestArtistsForUser :many -- M5c: per-user artist suggestions ranked by signal x similarity. The -- seeds CTE collects the user's likes (x5) plus recency-decayed plays diff --git a/internal/playevents/writer.go b/internal/playevents/writer.go index 22999945..f83fb099 100644 --- a/internal/playevents/writer.go +++ b/internal/playevents/writer.go @@ -65,7 +65,16 @@ func (w *Writer) RecordPlayStarted( clientID string, at time.Time, ) (StartedResult, error) { - return w.RecordPlayStartedWithSource(ctx, userID, trackID, clientID, "", at) + return w.RecordPlayStartedWithSource(ctx, userID, trackID, clientID, "", "", at) +} + +// strPtrOrNil maps an empty string to a NULL text column (nullable pointer), +// mirroring the clientID/source nil-on-empty pattern used above. +func strPtrOrNil(s string) *string { + if s == "" { + return nil + } + return &s } // systemPlaylistSources are the play_events.source values that count @@ -136,7 +145,7 @@ func lookupSystemPickKind( func (w *Writer) RecordPlayStartedWithSource( ctx context.Context, userID, trackID pgtype.UUID, - clientID, source string, + clientID, source, deviceClass string, at time.Time, ) (StartedResult, error) { var out StartedResult @@ -165,13 +174,14 @@ func (w *Writer) RecordPlayStartedWithSource( } } ev, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ - UserID: userID, - TrackID: trackID, - SessionID: sessionID, - StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, - ClientID: clientIDPtr, - Source: sourcePtr, - PickKind: pickKind, + UserID: userID, + TrackID: trackID, + SessionID: sessionID, + StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, + ClientID: clientIDPtr, + Source: sourcePtr, + PickKind: pickKind, + DeviceClass: strPtrOrNil(deviceClass), }) if err != nil { return err @@ -389,7 +399,7 @@ func (w *Writer) RecordSyntheticCompletedPlay( func (w *Writer) RecordOfflinePlay( ctx context.Context, userID, trackID pgtype.UUID, - clientID, source string, + clientID, source, deviceClass string, at time.Time, durationPlayedMs int32, ) error { @@ -441,13 +451,14 @@ func (w *Writer) RecordOfflinePlay( } } ev, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ - UserID: userID, - TrackID: trackID, - SessionID: sessionID, - StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, - ClientID: clientIDPtr, - Source: sourcePtr, - PickKind: pickKind, + UserID: userID, + TrackID: trackID, + SessionID: sessionID, + StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, + ClientID: clientIDPtr, + Source: sourcePtr, + PickKind: pickKind, + DeviceClass: strPtrOrNil(deviceClass), }) if err != nil { return err diff --git a/internal/playevents/writer_test.go b/internal/playevents/writer_test.go index b6dc1bd6..bab96826 100644 --- a/internal/playevents/writer_test.go +++ b/internal/playevents/writer_test.go @@ -226,10 +226,10 @@ func TestRecordOfflinePlay_DedupsByUserTrackStartedAt(t *testing.T) { f := newFixture(t, 200_000) ctx := context.Background() at := time.Now().UTC() - if err := f.w.RecordOfflinePlay(ctx, f.user, f.track, "c", "", at, 120_000); err != nil { + if err := f.w.RecordOfflinePlay(ctx, f.user, f.track, "c", "", "", at, 120_000); err != nil { t.Fatalf("first offline: %v", err) } - if err := f.w.RecordOfflinePlay(ctx, f.user, f.track, "c", "", at, 120_000); err != nil { + if err := f.w.RecordOfflinePlay(ctx, f.user, f.track, "c", "", "", at, 120_000); err != nil { t.Fatalf("replay offline: %v", err) } var count int @@ -444,7 +444,7 @@ func TestRecordPlayStartedWithSource_AppendsRotation(t *testing.T) { f := newFixture(t, 200_000) now := time.Now().UTC() if _, err := f.w.RecordPlayStartedWithSource( - context.Background(), f.user, f.track, "c", "for_you", now, + context.Background(), f.user, f.track, "c", "for_you", "", now, ); err != nil { t.Fatalf("RecordPlayStartedWithSource: %v", err) } @@ -460,7 +460,7 @@ func TestRecordPlayStartedWithSource_AppendsRotation(t *testing.T) { // Re-playing the same track stays a set (no duplicate append). if _, err := f.w.RecordPlayStartedWithSource( - context.Background(), f.user, f.track, "c", "for_you", now.Add(time.Minute), + context.Background(), f.user, f.track, "c", "for_you", "", now.Add(time.Minute), ); err != nil { t.Fatalf("RecordPlayStartedWithSource 2: %v", err) } @@ -511,7 +511,7 @@ 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(), + context.Background(), f.user, f.track, "c", "for_you", "", time.Now().UTC(), ) if err != nil { t.Fatalf("RecordPlayStartedWithSource: %v", err) @@ -530,7 +530,7 @@ func TestRecordPlayStartedWithSource_NoSnapshotMatch_PickKindNull(t *testing.T) // 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(), + context.Background(), f.user, f.track, "c", "for_you", "", time.Now().UTC(), ) if err != nil { t.Fatalf("RecordPlayStartedWithSource: %v", err) @@ -552,7 +552,7 @@ func TestRecordPlayStartedWithSource_CrossVariant_PickKindNull(t *testing.T) { 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(), + context.Background(), f.user, f.track, "c", "discover", "", time.Now().UTC(), ) if err != nil { t.Fatalf("RecordPlayStartedWithSource: %v", err) @@ -572,7 +572,7 @@ func TestRecordPlayStartedWithSource_StampsDiscoverBucket(t *testing.T) { 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(), + context.Background(), f.user, f.track, "c", "discover", "", time.Now().UTC(), ) if err != nil { t.Fatalf("RecordPlayStartedWithSource: %v", err) @@ -593,7 +593,7 @@ func TestRecordOfflinePlay_StampsForYouPickKind(t *testing.T) { 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, + context.Background(), f.user, f.track, "c", "for_you", "", at, 180_000, ); err != nil { t.Fatalf("RecordOfflinePlay: %v", err) } diff --git a/internal/recommendation/candidates.go b/internal/recommendation/candidates.go index 74151139..ad6531e4 100644 --- a/internal/recommendation/candidates.go +++ b/internal/recommendation/candidates.go @@ -42,7 +42,7 @@ func LoadCandidates( return nil, err } - affinity, err := LoadContextAffinity(ctx, q, userID) + affinity, err := LoadContextAffinity(ctx, q, userID, currentVector.DeviceClass) if err != nil { return nil, err } @@ -150,7 +150,7 @@ func LoadCandidatesFromSimilarity( return nil, err } - affinity, err := LoadContextAffinity(ctx, q, userID) + affinity, err := LoadContextAffinity(ctx, q, userID, currentVector.DeviceClass) if err != nil { return nil, err } diff --git a/internal/recommendation/context.go b/internal/recommendation/context.go index ad36193f..6f94345d 100644 --- a/internal/recommendation/context.go +++ b/internal/recommendation/context.go @@ -28,15 +28,20 @@ func (c ContextAffinity) Affinity(artistID pgtype.UUID) float64 { } // LoadContextAffinity computes each artist's affinity for the user's CURRENT -// daypart × weekday cell. For every artist with completed plays in the window -// it compares the share of that artist's plays that fall in the current cell +// context cell — daypart × weekday, narrowed by deviceClass when non-empty +// (#1551; radio passes the current device, the daily mixes pass "" for a +// device-agnostic cell). For every artist with completed plays in the window it +// compares the share of that artist's plays that fall in the current cell // against the user's overall baseline share, shrinking sparse artists toward // the baseline. Returns an empty (all-neutral) affinity when the user has no // plays. func LoadContextAffinity( - ctx context.Context, q *dbq.Queries, userID pgtype.UUID, + ctx context.Context, q *dbq.Queries, userID pgtype.UUID, deviceClass string, ) (ContextAffinity, error) { - rows, err := q.ListArtistContextPlayCountsForUser(ctx, userID) + rows, err := q.ListArtistContextPlayCountsForUser(ctx, dbq.ListArtistContextPlayCountsForUserParams{ + ID: userID, + Column2: deviceClass, + }) if err != nil { return ContextAffinity{}, err } diff --git a/internal/recommendation/context_device_integration_test.go b/internal/recommendation/context_device_integration_test.go new file mode 100644 index 00000000..484c5d62 --- /dev/null +++ b/internal/recommendation/context_device_integration_test.go @@ -0,0 +1,87 @@ +package recommendation + +import ( + "context" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func insertPlayEventDevice( + t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time, device string, +) { + t.Helper() + ctx := context.Background() + var sessionID pgtype.UUID + if err := pool.QueryRow(ctx, + `INSERT INTO play_sessions (user_id, started_at, last_event_at, client_id) + VALUES ($1, $2, $2, 'ctx-dev-test') RETURNING id`, + userID, startedAt).Scan(&sessionID); err != nil { + t.Fatalf("insert play_session: %v", err) + } + if _, err := pool.Exec(ctx, + `INSERT INTO play_events (user_id, track_id, session_id, started_at, was_skipped, device_class) + VALUES ($1, $2, $3, $4, false, $5)`, + userID, trackID, sessionID, startedAt, device); err != nil { + t.Fatalf("insert play_event: %v", err) + } +} + +// TestLoadContextAffinity_DeviceNarrowing seeds one artist played only on +// mobile and another only on web (all in the current time cell), and verifies +// the current-device parameter narrows the affinity cell (#1551): on 'mobile' +// the mobile artist out-scores the web artist and vice-versa, while the +// device-agnostic (”) pass treats them equally. +func TestLoadContextAffinity_DeviceNarrowing(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + u := seedUser(t, pool, "ctx-dev") + aMobile := seedArtist(t, pool, "MobileArtist", "") + aWeb := seedArtist(t, pool, "WebArtist", "") + alM := seedAlbumForArtist(t, pool, aMobile.ID, "AlbM") + tM := seedTrackOnAlbum(t, pool, alM.ID, aMobile.ID, "TrkM") + alW := seedAlbumForArtist(t, pool, aWeb.ID, "AlbW") + tW := seedTrackOnAlbum(t, pool, alW.ID, aWeb.ID, "TrkW") + + now := time.Now() + for i := 0; i < 5; i++ { + insertPlayEventDevice(t, pool, u.ID, tM.ID, now, "mobile") + insertPlayEventDevice(t, pool, u.ID, tW.ID, now, "web") + } + + mob, err := LoadContextAffinity(ctx, q, u.ID, "mobile") + if err != nil { + t.Fatalf("mobile: %v", err) + } + if mob.Affinity(aMobile.ID) <= mob.Affinity(aWeb.ID) { + t.Errorf("on mobile, mobile artist (%.3f) should out-score web artist (%.3f)", + mob.Affinity(aMobile.ID), mob.Affinity(aWeb.ID)) + } + + web, err := LoadContextAffinity(ctx, q, u.ID, "web") + if err != nil { + t.Fatalf("web: %v", err) + } + if web.Affinity(aWeb.ID) <= web.Affinity(aMobile.ID) { + t.Errorf("on web, web artist (%.3f) should out-score mobile artist (%.3f)", + web.Affinity(aWeb.ID), web.Affinity(aMobile.ID)) + } + + // Device-agnostic ('' ): all plays fall in the same time cell, so the two + // artists are treated identically — the device dimension is what separates + // them above. + agn, err := LoadContextAffinity(ctx, q, u.ID, "") + if err != nil { + t.Fatalf("agnostic: %v", err) + } + if agn.Affinity(aMobile.ID) != agn.Affinity(aWeb.ID) { + t.Errorf("device-agnostic affinities should match, got mobile=%.3f web=%.3f", + agn.Affinity(aMobile.ID), agn.Affinity(aWeb.ID)) + } +} diff --git a/internal/recommendation/sessionvector.go b/internal/recommendation/sessionvector.go index d244d51e..c98a7d13 100644 --- a/internal/recommendation/sessionvector.go +++ b/internal/recommendation/sessionvector.go @@ -13,6 +13,11 @@ type SessionVector struct { Artists []string `json:"artists"` Tags map[string]int `json:"tags"` RecentTrackIDs []string `json:"recent_track_ids"` + // DeviceClass is the current request's device (#1551), set by the radio + // handler from the user's latest play; drives the device dimension of the + // context-affinity term. Empty (omitted) for the daily mixes and for the + // snapshot stored on play_events, so it never narrows those. + DeviceClass string `json:"device_class,omitempty"` } func BuildSessionVector(priorTracks []dbq.Track) SessionVector { diff --git a/web/src/lib/player/events.svelte.ts b/web/src/lib/player/events.svelte.ts index 6606d648..4240c26b 100644 --- a/web/src/lib/player/events.svelte.ts +++ b/web/src/lib/player/events.svelte.ts @@ -124,7 +124,10 @@ export function useEventsDispatcher(): void { // #415: tag the play with the system playlist it came from // (null for library / user-playlist / radio) so the server // can advance that playlist's rotation. - source: player.queueSource ?? undefined + source: player.queueSource ?? undefined, + // #1551: device class for context conditioning. The web app is a + // browser session, so it reports 'web'. + device_class: 'web' }); // The user may have moved on by the time the response arrives. Only // adopt the id if we're still on the same track and still playing.