diff --git a/api/v1_users_weekly_rotation.go b/api/v1_users_weekly_rotation.go index 7e9da475..d534b81e 100644 --- a/api/v1_users_weekly_rotation.go +++ b/api/v1_users_weekly_rotation.go @@ -28,6 +28,13 @@ const ( // track outrank a genuinely better one. weeklyRotationJitterFloor = 0.85 weeklyRotationJitterRange = 0.30 + + // The mix rolls over on Wednesday 00:00 UTC, not at the ISO week boundary + // (Monday). Product call: Monday already belongs to the other weekly + // surfaces, and Friday is Spotify's day. Expressed as an offset from the + // ISO week's Monday so the period is still identified by (iso_year, + // iso_week) everywhere -- cache keys, seed, share links. + weeklyRotationRolloverOffsetDays = 2 ) /* @@ -55,6 +62,19 @@ rolls. Nothing here uses random() — the week-to-week variation comes from `week_seed` below, which is a hash of (track_id, user_id, year, week). Same inputs, same mix, all week. +The listener's own history (plays, saves, reposts, follows) is read as of +the period's rollover instant, not as of the request. Without that anchor +the mix quietly ate itself: the moment someone played a track *from* the +mix, that track met the played-exclusion and vanished on the next cache +miss, so a link shared on Wednesday showed a different, shorter list by +Thursday. The mix is a shareable artifact; it has to survive being +listened to. What still drifts is the candidate pool (trending refreshes +continuously) and engagement counts, which can reorder comparable tracks +mid-week -- accepted, since freezing those needs a stored snapshot. + +The period rolls over on Wednesday 00:00 UTC, see +weeklyRotationRolloverOffsetDays. + SCORING. quality_score = ln(1 + 3*saves + 2*reposts + 1*plays) / 12 @@ -105,7 +125,7 @@ func (app *ApiServer) v1UsersWeeklyRotation(c *fiber.Ctx) error { userId := app.getUserId(c) myId := app.getMyId(c) - year, week := weeklyRotationPeriod(time.Now().UTC()) + year, week := weeklyRotationPeriod(time.Now()) trackIds, err := app.getWeeklyRotationTrackIds( c.Context(), @@ -133,11 +153,29 @@ func (app *ApiServer) v1UsersWeeklyRotation(c *fiber.Ctx) error { return v1TracksResponse(c, tracks) } -// weeklyRotationPeriod returns the ISO year and ISO week that `t` falls in. -// The mix is keyed on this pair, so it changes exactly once a week at the -// ISO week boundary (Monday 00:00 UTC). +// weeklyRotationPeriod returns the (ISO year, ISO week) pair that identifies +// the rotation period `t` falls in. Periods run Wednesday 00:00 UTC to the +// following Wednesday: shifting `t` back by the rollover offset maps each +// period onto the ISO week whose Monday it started counting from, so the +// pair still reads as a normal ISO week everywhere it's used as a key. func weeklyRotationPeriod(t time.Time) (int, int) { - return t.ISOWeek() + return t.UTC().AddDate(0, 0, -weeklyRotationRolloverOffsetDays).ISOWeek() +} + +// weeklyRotationPeriodStart is the inverse: the instant the (year, week) +// period began, i.e. Wednesday 00:00 UTC of that ISO week. Everything the +// listener did before this instant counts as history for the mix; +// everything after it does not. +func weeklyRotationPeriodStart(year, week int) time.Time { + // ISO week 1 is the week containing January 4th. + jan4 := time.Date(year, time.January, 4, 0, 0, 0, 0, time.UTC) + weekday := int(jan4.Weekday()) + if weekday == 0 { + weekday = 7 // Sunday: Go says 0, ISO says 7 + } + mondayOfWeek1 := jan4.AddDate(0, 0, -(weekday - 1)) + monday := mondayOfWeek1.AddDate(0, 0, (week-1)*7) + return monday.AddDate(0, 0, weeklyRotationRolloverOffsetDays) } func (app *ApiServer) getWeeklyRotationTrackIds( @@ -160,12 +198,17 @@ func (app *ApiServer) getWeeklyRotationTrackIds( -- listener has hundreds of thousands of play rows and an unbounded scan -- is what put the older recommendation endpoints over the upstream -- timeout (see PRs #805, #806). + -- + -- Every history CTE is cut off at @periodStart, the rollover instant, + -- so playing (or saving) a track from this week's mix doesn't remove it + -- from this week's mix. See STABILITY in the handler doc. my_played AS ( SELECT DISTINCT play_item_id AS track_id FROM ( SELECT play_item_id FROM plays WHERE user_id = @userId + AND created_at < @periodStart ORDER BY created_at DESC LIMIT 10000 ) p @@ -177,6 +220,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( AND save_type = 'track' AND is_current = true AND is_delete = false + AND created_at < @periodStart ), -- Capped the same way as the For You feed's follow_set, and for the -- same reason: a power user with thousands of follows otherwise pulls a @@ -187,6 +231,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( WHERE follower_user_id = @userId AND is_current = true AND is_delete = false + AND created_at < @periodStart ORDER BY created_at DESC LIMIT 500 ), @@ -200,6 +245,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( SELECT play_item_id AS track_id FROM plays WHERE user_id = @userId + AND created_at < @periodStart ORDER BY created_at DESC LIMIT 1000 ) p @@ -218,6 +264,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( SELECT save_item_id AS track_id FROM saves WHERE user_id = @userId AND save_type = 'track' AND is_current = true AND is_delete = false + AND created_at < @periodStart ORDER BY created_at DESC LIMIT 200 ) s @@ -230,6 +277,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( SELECT repost_item_id AS track_id FROM reposts WHERE user_id = @userId AND repost_type = 'track' AND is_current = true AND is_delete = false + AND created_at < @periodStart ORDER BY created_at DESC LIMIT 200 ) r @@ -382,6 +430,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( rows, err := app.pool.Query(ctx, sql, pgx.NamedArgs{ "userId": userId, "seedKey": fmt.Sprintf("%d:%d:%d", userId, year, week), + "periodStart": weeklyRotationPeriodStart(year, week), "limit": limit, "maxAgeDays": weeklyRotationMaxAgeDays, "jitterFloor": weeklyRotationJitterFloor, diff --git a/api/v1_users_weekly_rotation_test.go b/api/v1_users_weekly_rotation_test.go index 58f76e70..a0a12d7a 100644 --- a/api/v1_users_weekly_rotation_test.go +++ b/api/v1_users_weekly_rotation_test.go @@ -75,16 +75,21 @@ func weeklyRotationFixtures() database.FixtureMap { // My listening history: a rock track by user 6, which both establishes // Rock as my affinity genre and makes track 600 an already-played // exclusion. + // + // History only counts if it predates the period's rollover, and the + // rollover is at most seven days back, so everything here is dated + // eight days ago. See TestV1UsersWeeklyRotationKeepsTracksPlayedThisPeriod + // for the other side of that line. plays := []map[string]any{ - {"id": 1, "user_id": 1, "play_item_id": 600, "created_at": daysAgo(1)}, + {"id": 1, "user_id": 1, "play_item_id": 600, "created_at": daysAgo(8)}, } saves := []map[string]any{ - {"user_id": 1, "save_item_id": 700, "save_type": "track"}, + {"user_id": 1, "save_item_id": 700, "save_type": "track", "created_at": daysAgo(8)}, } follows := []map[string]any{ - {"follower_user_id": 1, "followee_user_id": 4}, + {"follower_user_id": 1, "followee_user_id": 4, "created_at": daysAgo(8)}, } // Every candidate needs a trending row to be retrieved at all. @@ -201,7 +206,7 @@ func TestV1UsersWeeklyRotationDemotesFollowedArtists(t *testing.T) { {"track_id": 300, "save_count": 100, "repost_count": 50}, }, "follows": []map[string]any{ - {"follower_user_id": 1, "followee_user_id": 2}, + {"follower_user_id": 1, "followee_user_id": 2, "created_at": time.Now().AddDate(0, 0, -8)}, }, "track_trending_scores": []map[string]any{ {"track_id": 200, "score": 1_000_000_000, "time_range": "week"}, @@ -401,3 +406,102 @@ func TestV1UsersWeeklyRotationReadsGenreCarryingTrendingRows(t *testing.T) { assert.Len(t, resp.Data, 1, "a score row carrying a genre must still be a candidate") } + +// Playing a track from the mix must not remove it from the mix. The +// played-exclusion is anchored at the period's rollover, so a play dated +// now -- inside the current period -- is invisible to it. Without the +// anchor the mix shrank as it was listened to, which made a shared link +// show a different list by the next day. +func TestV1UsersWeeklyRotationKeepsTracksPlayedThisPeriod(t *testing.T) { + app := emptyTestApp(t) + + fixtures := database.FixtureMap{ + "users": []map[string]any{ + {"user_id": 1, "handle": "me", "handle_lc": "me", "wallet": "0x0000000000000000000000000000000000000001"}, + {"user_id": 2, "handle": "artist", "handle_lc": "artist", "wallet": "0x0000000000000000000000000000000000000002"}, + {"user_id": 3, "handle": "other", "handle_lc": "other", "wallet": "0x0000000000000000000000000000000000000003"}, + }, + "aggregate_user": []map[string]any{ + {"user_id": 1, "follower_count": 0, "following_count": 0}, + {"user_id": 2, "follower_count": 5000, "following_count": 10}, + {"user_id": 3, "follower_count": 5000, "following_count": 10}, + }, + "tracks": []map[string]any{ + {"track_id": 200, "owner_id": 2, "title": "played this week", "genre": "Rock"}, + {"track_id": 300, "owner_id": 3, "title": "played last week", "genre": "Rock"}, + }, + "aggregate_track": []map[string]any{ + {"track_id": 200, "save_count": 100, "repost_count": 50}, + {"track_id": 300, "save_count": 100, "repost_count": 50}, + }, + "plays": []map[string]any{ + // Inside the current period: does not count as history. + {"id": 1, "user_id": 1, "play_item_id": 200, "created_at": time.Now()}, + // Before any possible rollover: counts, and excludes track 300. + {"id": 2, "user_id": 1, "play_item_id": 300, "created_at": time.Now().AddDate(0, 0, -8)}, + }, + "track_trending_scores": []map[string]any{ + {"track_id": 200, "score": 1_000_000_000, "time_range": "week"}, + {"track_id": 300, "score": 1_000_000_000, "time_range": "week"}, + }, + } + database.Seed(app.pool.Replicas[0], fixtures) + + var resp struct { + Data []dbv1.Track + } + status, _ := testGet(t, app, "/v1/users/7eP5n/weekly-rotation", &resp) + assert.Equal(t, 200, status) + + titles := weeklyRotationTitles(resp.Data) + assert.Contains(t, titles, "played this week", "a play inside the period leaves the mix alone") + assert.NotContains(t, titles, "played last week", "a play before the rollover still excludes") +} + +// Pure period math, no database. The period is identified by an ISO +// (year, week) pair but starts on that week's Wednesday, so the two +// functions have to agree with each other across the rollover and across +// a year boundary. +func TestWeeklyRotationPeriod(t *testing.T) { + utc := func(y int, m time.Month, d, h int) time.Time { + return time.Date(y, m, d, h, 0, 0, 0, time.UTC) + } + + // 2026-09-09 is a Wednesday. + rollover := utc(2026, time.September, 9, 0) + + y, w := weeklyRotationPeriod(rollover) + assert.Equal(t, [2]int{2026, 37}, [2]int{y, w}, "the rollover instant opens ISO week 37's period") + assert.Equal(t, rollover, weeklyRotationPeriodStart(y, w)) + + y, w = weeklyRotationPeriod(rollover.Add(-time.Second)) + assert.Equal(t, [2]int{2026, 36}, [2]int{y, w}, "one second earlier is still the previous period") + assert.Equal(t, utc(2026, time.September, 2, 0), weeklyRotationPeriodStart(y, w)) + + y, w = weeklyRotationPeriod(utc(2026, time.September, 7, 12)) // the Monday + assert.Equal(t, [2]int{2026, 36}, [2]int{y, w}, "Monday and Tuesday belong to the period that started the previous Wednesday") + + // Non-UTC input is normalised: 2026-09-08 20:00 PDT is 2026-09-09 03:00 UTC. + pdt := time.FixedZone("PDT", -7*3600) + y, w = weeklyRotationPeriod(time.Date(2026, time.September, 8, 20, 0, 0, 0, pdt)) + assert.Equal(t, [2]int{2026, 37}, [2]int{y, w}) + + // Year boundary: ISO week 1 of 2027 starts Monday 2027-01-04, so its + // period starts Wednesday 2027-01-06, and the days before that belong + // to 2026's last ISO week (53). + y, w = weeklyRotationPeriod(utc(2027, time.January, 6, 0)) + assert.Equal(t, [2]int{2027, 1}, [2]int{y, w}) + assert.Equal(t, utc(2027, time.January, 6, 0), weeklyRotationPeriodStart(2027, 1)) + + y, w = weeklyRotationPeriod(utc(2027, time.January, 5, 23)) + assert.Equal(t, [2]int{2026, 53}, [2]int{y, w}) + assert.Equal(t, utc(2026, time.December, 30, 0), weeklyRotationPeriodStart(2026, 53)) + + // Round trip across a whole year of hours. + for tm := utc(2026, time.January, 1, 0); tm.Before(utc(2027, time.January, 1, 0)); tm = tm.Add(time.Hour) { + py, pw := weeklyRotationPeriod(tm) + start := weeklyRotationPeriodStart(py, pw) + require.False(t, tm.Before(start), "%v is before its own period start %v", tm, start) + require.True(t, tm.Before(start.AddDate(0, 0, 7)), "%v is past the end of its period starting %v", tm, start) + } +}