From 7d61aa4e9fe11c3e3db6b2d3ce2546151d5f6294 Mon Sep 17 00:00:00 2001 From: Dylan Jeffers Date: Wed, 23 Sep 2026 16:47:44 -0700 Subject: [PATCH 1/2] fix(api): keep the underground upweight in Weekly Rotation dedup A track in both the trending and underground candidate sets kept the 'trending' row because DISTINCT ON sorted source ascending, so the 1.15 underground weight almost never applied. Also join only the current users row in coin metadata so the default description cites the current handle, and correct the swagger text for suggested-follows (includes playlists) and the Weekly Rotation rollover (Wednesday 00:00 UTC). Co-Authored-By: Claude Opus 5.5 --- api/swagger/swagger-v1.yaml | 10 ++++---- api/v1_coin_metadata.go | 2 +- api/v1_coin_metadata_test.go | 2 ++ api/v1_users_weekly_rotation.go | 6 ++--- api/v1_users_weekly_rotation_test.go | 37 ++++++++++++++++++++++++++++ 5 files changed, 48 insertions(+), 9 deletions(-) diff --git a/api/swagger/swagger-v1.yaml b/api/swagger/swagger-v1.yaml index c2bc90a8..17923ed0 100644 --- a/api/swagger/swagger-v1.yaml +++ b/api/swagger/swagger-v1.yaml @@ -7114,8 +7114,8 @@ paths: description: Gets the user's Weekly Rotation mix - a personalized set of tracks they have not heard, weighted toward artists they do not already - follow. The mix is fixed for the calendar week (ISO week, UTC) and - rotates when the week rolls over. Unlike suggested-follows, this + follow. The mix is fixed for the week and rolls over Wednesday + 00:00 UTC. Unlike suggested-follows, this returns results for users with no listening history. operationId: Get Weekly Rotation security: @@ -7157,9 +7157,9 @@ paths: tags: - users description: - Gets artists to suggest the user follow, based on the tracks and - albums they have favorited or reposted but whose artist they do not - already follow. Returns an empty list for users with no favorites or + Gets artists to suggest the user follow, based on the tracks, albums + and playlists they have favorited or reposted but whose owner they do + not already follow. Returns an empty list for users with no favorites or reposts. operationId: Get Suggested Follows security: diff --git a/api/v1_coin_metadata.go b/api/v1_coin_metadata.go index 58c45400..302c456b 100644 --- a/api/v1_coin_metadata.go +++ b/api/v1_coin_metadata.go @@ -38,7 +38,7 @@ func (app *ApiServer) v1CoinMetadata(c *fiber.Ctx) error { artist_coins.logo_uri, users.handle FROM artist_coins - LEFT JOIN users ON users.user_id = artist_coins.user_id + LEFT JOIN users ON users.user_id = artist_coins.user_id AND users.is_current = true WHERE artist_coins.mint = $1 `, mint).Scan(&name, &ticker, &description, &logoUri, &handle) if err != nil { diff --git a/api/v1_coin_metadata_test.go b/api/v1_coin_metadata_test.go index 7b87436a..5c98efa0 100644 --- a/api/v1_coin_metadata_test.go +++ b/api/v1_coin_metadata_test.go @@ -17,6 +17,8 @@ func TestV1CoinMetadata(t *testing.T) { "users": { {"user_id": 1, "handle": "bearartist", "is_current": true}, {"user_id": 2, "handle": "bareartist", "is_current": true}, + // Superseded row from before a handle change. + {"user_id": 2, "handle": "oldbare", "is_current": false, "txhash": "0xold"}, }, "artist_coins": { { diff --git a/api/v1_users_weekly_rotation.go b/api/v1_users_weekly_rotation.go index f9d9e3a9..2a17fa24 100644 --- a/api/v1_users_weekly_rotation.go +++ b/api/v1_users_weekly_rotation.go @@ -293,12 +293,12 @@ func (app *ApiServer) getWeeklyRotationTrackIds( UNION ALL SELECT track_id, source FROM cand_trending ), - -- One row per track. Underground sorts before trending so a track that - -- qualifies for both keeps the upweighted source. + -- One row per track. A track in both sources keeps 'underground' so it + -- gets the upweight. deduped AS ( SELECT DISTINCT ON (track_id) track_id, source FROM candidates - ORDER BY track_id, source ASC + ORDER BY track_id, (source = 'underground') DESC ), filtered AS ( SELECT diff --git a/api/v1_users_weekly_rotation_test.go b/api/v1_users_weekly_rotation_test.go index e01ee083..5d23ff42 100644 --- a/api/v1_users_weekly_rotation_test.go +++ b/api/v1_users_weekly_rotation_test.go @@ -457,3 +457,40 @@ func TestV1UsersWeeklyRotationKeepsTracksPlayedThisPeriod(t *testing.T) { 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") } + +// A track in both the trending and underground sources keeps the underground +// upweight. Week 9 is pinned because its seeds put the trending-only track +// about 1% ahead, so without the upweight it would sort first. +func TestV1UsersWeeklyRotationPrefersUndergroundSource(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": "small", "handle_lc": "small", "wallet": "0x0000000000000000000000000000000000000002"}, + {"user_id": 3, "handle": "big", "handle_lc": "big", "wallet": "0x0000000000000000000000000000000000000003"}, + }, + "aggregate_user": []map[string]any{ + {"user_id": 1, "follower_count": 0, "following_count": 0}, + {"user_id": 2, "follower_count": 100, "following_count": 50}, + {"user_id": 3, "follower_count": 5000, "following_count": 10}, + }, + "tracks": []map[string]any{ + {"track_id": 200, "owner_id": 2, "title": "underground track", "genre": "Rock"}, + {"track_id": 300, "owner_id": 3, "title": "trending track", "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}, + }, + "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) + + ids, err := app.getWeeklyRotationTrackIds(context.Background(), 1, 2026, 9, 10) + require.NoError(t, err) + assert.Equal(t, []int32{200, 300}, ids) +} From 8daa394285cf734a35566c8cae41cf3d5175aafa Mon Sep 17 00:00:00 2001 From: Dylan Jeffers Date: Wed, 23 Sep 2026 16:50:09 -0700 Subject: [PATCH 2/2] chore(api): shorten comments in recent recommendation and media endpoints Trim comments added with suggested-follows, Weekly Rotation, coin metadata, is_streamable enforcement and owner downloads to plain statements of what the code does and why. Co-Authored-By: Claude Opus 5.5 --- api/dbv1/tracks.go | 17 +-- api/server.go | 8 +- api/v1_coin_metadata.go | 5 +- api/v1_playlist_stream.go | 4 +- api/v1_track_download.go | 31 ++--- api/v1_track_download_test.go | 17 +-- api/v1_track_stream.go | 9 +- api/v1_track_test.go | 11 +- api/v1_users_suggested_follows.go | 37 ++---- api/v1_users_suggested_follows_test.go | 13 +- api/v1_users_weekly_rotation.go | 158 +++++++++---------------- api/v1_users_weekly_rotation_test.go | 55 +++------ 12 files changed, 118 insertions(+), 247 deletions(-) diff --git a/api/dbv1/tracks.go b/api/dbv1/tracks.go index 59461144..d9cf1fb1 100644 --- a/api/dbv1/tracks.go +++ b/api/dbv1/tracks.go @@ -201,19 +201,10 @@ func (q *Queries) TracksKeyed(ctx context.Context, arg TracksParams) (map[int32] // account was delisted by the trusted notifier. isStreamable := !rawTrack.IsDelete && !user.IsDeactivated - // Two reasons to leave a media link nil, both with the same effect: the - // URL should never be handed out, and the endpoints report the track as - // unavailable instead. - // - // A track row can have empty cid columns (e.g. an upload-v2 row whose - // track_cid/orig_file_cid backfill never ran), and signing an empty cid - // produces a content-node URL that is guaranteed to 404. - // - // A non-streamable track is worse: the cid is real, so the signed URL - // works. The stream and download endpoints reject these, but that only - // closes those two routes - anyone reading the track response could - // still fetch the audio straight from the content node. Preview is - // included because a preview clip is still the artist's audio. + // Media links stay nil when there is no cid to sign (the URL would 404) + // or the track is not streamable (the cid is real, so a signed URL would + // bypass the stream and download endpoint checks). Previews count as + // the artist's audio too. var stream *MediaLink if isStreamable && access.Stream && rawTrack.TrackCid.String != "" { stream, err = mediaLink(rawTrack.TrackCid.String, rawTrack.TrackID, arg.MyID.(int32), id3Tags) diff --git a/api/server.go b/api/server.go index a19636e4..e1adaf92 100644 --- a/api/server.go +++ b/api/server.go @@ -171,11 +171,9 @@ func NewApiServer(config config.Config) *ApiServer { } // Caches the track-id list returned by the /v1/users/:userId/weekly-rotation - // query. The mix is deterministic for the whole ISO week and the cache key - // carries the year/week, so entries are immutable for their lifetime and a - // long TTL is safe — a stale entry is the correct answer, not a stale one. - // Sized larger than the other recommendation caches because this is the - // most expensive query of the three and the least likely to be re-derived. + // query. The key includes the period, so entries stay correct for their + // whole TTL. Sized larger than the other recommendation caches because this + // is the most expensive query of the three. weeklyRotationCache, err := otter.MustBuilder[string, []int32](50_000). WithTTL(6 * time.Hour). CollectStats(). diff --git a/api/v1_coin_metadata.go b/api/v1_coin_metadata.go index 302c456b..9f388319 100644 --- a/api/v1_coin_metadata.go +++ b/api/v1_coin_metadata.go @@ -62,9 +62,8 @@ func (app *ApiServer) v1CoinMetadata(c *fiber.Ctx) error { if description != nil && *description != "" { metadata.Description = *description } else if handle != nil { - // The launchpad never persists a description - the Fan Club page builds - // this same sentence client-side, and storing it would make the page cite - // itself. Regenerate it here so wallets and explorers still get one. + // Launchpad coins store no description. Rebuild the default the web + // client shows so wallets and explorers still get one. metadata.Description = defaultCoinDescription(*handle, ticker, appUrl) } if logoUri != nil { diff --git a/api/v1_playlist_stream.go b/api/v1_playlist_stream.go index 70f2932c..38e2b43b 100644 --- a/api/v1_playlist_stream.go +++ b/api/v1_playlist_stream.go @@ -51,9 +51,7 @@ func (app *ApiServer) v1PlaylistStream(c *fiber.Ctx) error { continue } - // Leave deleted tracks, and tracks whose owner is no longer active, out - // of the m3u8 entirely rather than emitting a URL the stream endpoint - // will now reject. + // Skip tracks the stream endpoint rejects (deleted, or owner deactivated). if !track.IsStreamable { continue } diff --git a/api/v1_track_download.go b/api/v1_track_download.go index 23e0ce17..9797c5fd 100644 --- a/api/v1_track_download.go +++ b/api/v1_track_download.go @@ -9,16 +9,14 @@ import ( ) func createFilename(track *dbv1.Track) string { - // The original upload is what a download serves whenever the row kept one, - // so its own name is the right one. + // Downloads serve the original upload when the row has one, so use its name. if track.OrigFileCid.String != "" && track.OrigFilename.String != "" { return track.OrigFilename.String } - // Otherwise the bytes are the mp3 transcode, and the name must not promise - // the format the artist uploaded: a .wav name on mp3 bytes is a file most - // editors refuse to open. Only the recorded filename is stripped of its - // extension - a title is free text and "Vol. 2" has no extension to trim. + // Otherwise the bytes are the mp3 transcode, so swap the recorded + // filename's extension for .mp3. Titles are used as-is since they have no + // real extension. if name := track.OrigFilename.String; name != "" { return strings.TrimSuffix(name, path.Ext(name)) + ".mp3" } @@ -61,11 +59,9 @@ func (app *ApiServer) v1TrackDownload(c *fiber.Ctx) error { return fiber.NewError(fiber.StatusNotFound, "track not found") } - // track.Download is only populated for tracks the public may download, so - // an artist who left downloads off - about four in five tracks - could not - // get their own file back. The edit page's "Download File" button and the - // replace-file flow both come through here, and both were 404ing for the - // owner of the track, surfacing as a generic "something went wrong". + // track.Download is only set when the public may download. Fall back to an + // owner-signed link so the artist or their manager can always fetch their + // own file (used by the edit page's "Download File" and replace-file flows). downloadLink := track.Download if downloadLink == nil { downloadLink, err = app.ownerDownloadLink(c, &track) @@ -95,14 +91,11 @@ func (app *ApiServer) v1TrackDownload(c *fiber.Ctx) error { return c.Redirect(downloadUrl.String(), fiber.StatusFound) } -// ownerDownloadLink signs a download link for a requester who has proven they -// own the track, or manage the account that does. Ownership comes from the -// wallet recovered from the request signature, not from the user_id query -// param behind myId: user_id is the caller's own claim, and it is only -// trustworthy here because this route sits off authMiddleware's advisory- -// user_id allowlist. Handing out an artist's original master should not rest -// on that list continuing to exclude this route. Returns nil - not an error - -// for everyone else, which leaves the caller's 404 in place. +// ownerDownloadLink signs a download link when the wallet recovered from the +// request signature owns the track or holds an approved grant from the owner. +// It checks the signature rather than the user_id query param so it does not +// depend on authMiddleware's user_id handling for this route. Returns nil for +// anyone else. func (app *ApiServer) ownerDownloadLink(c *fiber.Ctx, track *dbv1.Track) (*dbv1.MediaLink, error) { wallet := app.tryGetAuthedWallet(c) if wallet == "" { diff --git a/api/v1_track_download_test.go b/api/v1_track_download_test.go index b7b3daec..0f171db0 100644 --- a/api/v1_track_download_test.go +++ b/api/v1_track_download_test.go @@ -82,9 +82,8 @@ const ( managerWallet = "0x4954d18926ba0ed9378938444731be4e622537b2" ) -// seedNonDownloadableTrack sets up the shape most tracks on the network have: -// an artist who never turned downloads on, whose original upload is still -// sitting on the content node. +// seedNonDownloadableTrack seeds a track with downloads off and an original +// upload kept. func seedNonDownloadableTrack(t *testing.T) *ApiServer { app := emptyTestApp(t) database.Seed(app.pool.Replicas[0], database.FixtureMap{ @@ -120,8 +119,7 @@ func downloadWithWallet(t *testing.T, app *ApiServer, path string, wallet string return res.StatusCode, res.Header.Get("Location") } -// An artist must be able to get their own upload back even with downloads off -// for everyone else - that is what the edit page's "Download File" button does. +// The owner can download their own track with downloads off. func TestGetTrackDownload_OwnerOfNonDownloadableTrack(t *testing.T) { app := seedNonDownloadableTrack(t) path := "/v1/tracks/" + trashid.MustEncodeHashID(1) + "/download" @@ -150,10 +148,8 @@ func TestGetTrackDownload_ManagerOfNonDownloadableTrack(t *testing.T) { assert.Contains(t, location, "tracks/cidstream/QmOriginal") } -// Everyone else still gets the 404 the artist asked for by leaving downloads -// off. Claiming to be the owner through the user_id query param must not be -// enough: the bypass keys on the recovered signature, and the auth middleware -// separately refuses a user_id no signature backs (403 rather than 404). +// Non-owners get 404. A user_id param naming the owner without a matching +// signature gets 403 from the auth middleware. func TestGetTrackDownload_NonOwnerOfNonDownloadableTrack(t *testing.T) { app := seedNonDownloadableTrack(t) trackPath := "/v1/tracks/" + trashid.MustEncodeHashID(1) + "/download" @@ -172,8 +168,7 @@ func TestGetTrackDownload_NonOwnerOfNonDownloadableTrack(t *testing.T) { assert.Equal(t, 403, status, "user_id claiming to be the owner, signed by another wallet") } -// With no original kept, the download serves the mp3 transcode, so the name it -// is served under has to say mp3 rather than the format that was uploaded. +// With no original kept, the download serves the mp3 transcode under a .mp3 name. func TestGetTrackDownload_FilenameFallsBackToMp3(t *testing.T) { app := emptyTestApp(t) database.Seed(app.pool.Replicas[0], database.FixtureMap{ diff --git a/api/v1_track_stream.go b/api/v1_track_stream.go index 98393d22..d0fe37a2 100644 --- a/api/v1_track_stream.go +++ b/api/v1_track_stream.go @@ -29,12 +29,9 @@ func (app *ApiServer) v1TrackStream(c *fiber.Ctx) error { track := tracks[0] - // `is_streamable` is false when the track is deleted or its owner is no - // longer active - either the artist deactivated their own account or the - // account was delisted by the trusted notifier. The track response has - // always reported this, but nothing enforced it, so the audio stayed - // reachable to anyone holding the URL. Treat it as not found rather than - // forbidden so we don't distinguish these from a missing track. + // is_streamable is false when the track is deleted or its owner is + // deactivated or delisted. Return 404 rather than 403 so these look like a + // missing track. if !track.IsStreamable { return fiber.NewError(fiber.StatusNotFound, "track not found") } diff --git a/api/v1_track_test.go b/api/v1_track_test.go index ba503193..2d28443c 100644 --- a/api/v1_track_test.go +++ b/api/v1_track_test.go @@ -175,12 +175,8 @@ func TestGetTrackUsdcPurchaseSelfAccess(t *testing.T) { }) } -// A track whose owner is no longer active - the artist deactivated their own -// account, or the account was delisted by the trusted notifier - must not carry -// signed content-node URLs in its response. The stream and download endpoints -// already reject these, but the media links in the track response bypass those -// endpoints entirely: the cid is real, so the signed URL serves the full audio -// straight from the content node to anyone who reads the response. +// A non-streamable track (deleted, or owner deactivated) must not carry signed +// media links in its response. func TestGetTrack_NonStreamableOmitsMediaLinks(t *testing.T) { for _, tc := range []struct { name string @@ -226,8 +222,7 @@ func TestGetTrack_NonStreamableOmitsMediaLinks(t *testing.T) { } } -// The guard above is scoped to non-streamable tracks: an ordinary track with an -// active owner must still get its signed media links. +// A streamable track keeps its signed media links. func TestGetTrack_StreamableKeepsMediaLinks(t *testing.T) { app := emptyTestApp(t) database.Seed(app.pool.Replicas[0], database.FixtureMap{ diff --git a/api/v1_users_suggested_follows.go b/api/v1_users_suggested_follows.go index f839bbc6..6ffa127a 100644 --- a/api/v1_users_suggested_follows.go +++ b/api/v1_users_suggested_follows.go @@ -15,21 +15,11 @@ type GetUsersSuggestedFollowsParams struct { } const ( - // Only the most recent N favorites and N reposts are considered. A user's - // engagement history is unbounded, and everything downstream of it joins - // per-row, so this caps the worst case for heavy users. Recent engagement - // is also the better signal, so the cap costs little. - // - // Note this cap, not the decay below, is the effective window for anyone - // with a large library: a user who favorites 50 tracks a day is scored on - // their last ~40 days, well inside the decay's range. For everyone else the - // cap never binds and the decay does the shaping. - // - // Lowering it further is tempting for latency but trades against fill rate: - // the cap bounds the candidate pool *before* already-followed artists are - // filtered out, so a user who follows most of the artists they recently - // engaged with gets a short list. saves_user_created_at_active_idx - // (migration 0239) removes the reason to make that trade. + // Only the most recent N favorites and N reposts are scored, which bounds + // the per-row joins for heavy users. For large libraries this cap, not the + // decay, sets the effective window. It applies before followed artists are + // filtered out, so lowering it shortens the list for users who follow most + // of what they engage with. suggestedFollowsEngagementCap = 2000 // Engagement weight decays with e^(-age/tau). At tau = 180 days a favorite @@ -44,19 +34,12 @@ const ( ) /* -Suggests artists to follow based on the user's own favorites and reposts. +Suggests users to follow: owners of tracks, albums and playlists the user has +favorited or reposted but does not follow, ranked by recency-weighted +engagement. -This is the "direct owner" pass: artists whose tracks or albums the user has -already favorited or reposted but has not followed. It is deliberately not a -collaborative filter — those candidates are already engaged with, so they need -no graph traversal to justify, and "you saved three of their tracks and never -followed them" is both the cheapest and the most legible suggestion available. - -Distinct from /users/:userId/related, which is artist-anchored ("followers of X -also follow Y"). This one is viewer-anchored. - -Suggestions exclude anyone the seed user already follows, so the result depends -only on the path userId — not on the caller — and is cached on that alone. +Unlike /users/:userId/related (artist-anchored), this is viewer-anchored. The +result depends only on the path userId, so it is cached on that. */ func (app *ApiServer) v1UsersSuggestedFollows(c *fiber.Ctx) error { params := GetUsersSuggestedFollowsParams{} diff --git a/api/v1_users_suggested_follows_test.go b/api/v1_users_suggested_follows_test.go index bbc717a0..c2402fd7 100644 --- a/api/v1_users_suggested_follows_test.go +++ b/api/v1_users_suggested_follows_test.go @@ -93,7 +93,7 @@ func TestV1UsersSuggestedFollows(t *testing.T) { assert.Equal(t, "onerepost", resp.Data[1].Handle.String) } - // Offset walks the same ordering rather than reshuffling it. + // Offset continues the same ordering. { status, _ := testGet(t, app, "/v1/users/7eP5n/suggested-follows?limit=2&offset=2", &resp) assert.Equal(t, 200, status) @@ -102,8 +102,7 @@ func TestV1UsersSuggestedFollows(t *testing.T) { assert.Equal(t, "albumowner", resp.Data[1].Handle.String) } - // A user with no favorites or reposts gets nothing rather than an error -- - // the caller is expected to fall back to a non-personalized surface. + // A user with no favorites or reposts gets an empty list. { status, _ := testGet(t, app, "/v1/users/ML51L/suggested-follows", &resp) assert.Equal(t, 200, status) @@ -111,9 +110,7 @@ func TestV1UsersSuggestedFollows(t *testing.T) { } } -// The decay term is the only reason a single favorite can outrank another -// single favorite, so it needs a case of its own -- the fixtures above all -// share a created_at and would pass with the decay removed entirely. +// The fixtures above share a created_at, so the decay term needs its own case. func TestV1UsersSuggestedFollowsRecencyDecay(t *testing.T) { app := emptyTestApp(t) @@ -145,8 +142,8 @@ func TestV1UsersSuggestedFollowsRecencyDecay(t *testing.T) { Data []dbv1.User } - // Equal raw engagement (one favorite each), so recency alone decides. User 2 - // sorts first on user_id, which makes this fail loudly if decay stops working. + // Equal engagement, so recency decides. Without decay, user 2 would sort + // first on user_id. status, _ := testGet(t, app, "/v1/users/7eP5n/suggested-follows", &resp) assert.Equal(t, 200, status) assert.Len(t, resp.Data, 2) diff --git a/api/v1_users_weekly_rotation.go b/api/v1_users_weekly_rotation.go index 2a17fa24..8a36cace 100644 --- a/api/v1_users_weekly_rotation.go +++ b/api/v1_users_weekly_rotation.go @@ -16,90 +16,61 @@ type GetUsersWeeklyRotationParams struct { } const ( - // Tracks older than this are excluded outright. A discovery mix is - // allowed to reach much further back than the For You feed (48h - // half-life), but a track from 2019 that never found an audience is - // usually not a hidden gem — it's an abandoned upload. + // Tracks older than this are excluded. Old tracks that never found an + // audience are rarely good discoveries. weeklyRotationMaxAgeDays = 365 - // Week-seeded jitter band. Scores across the candidate pool are tightly - // clustered, so without a deterministic per-week perturbation the same - // user would get a near-identical mix every week. +/-15% is enough to - // rotate the ordering among comparable candidates without letting a weak - // track outrank a genuinely better one. + // Week-seeded jitter band. Candidate scores are tightly clustered, so + // without a per-week perturbation the same user would get a near-identical + // mix every week. +/-15% reorders comparable candidates without letting a + // weak track outrank a clearly better one. weeklyRotationJitterFloor = 0.85 weeklyRotationJitterRange = 0.30 ) /* -Returns a fixed-size, taste-matched track mix that is stable for the -calendar week — the "Weekly Rotation" surface. - -Distinct from GET /v1/users/{id}/feed/for-you in three ways that matter: - - - For You is a *feed*: freshness-weighted (48h half-life), re-ranked on - every load, infinite. This is an *artifact*: a fixed 30 tracks that do - not change until the week rolls over, so it can be linked, revisited, - and talked about. - - For You boosts in-network (followed) creators. This demotes them. The - point of the mix is artists the listener hasn't found yet, so a - followed artist has to clear a higher bar to appear. - - For You soft-penalizes tracks you've already heard. This excludes them - outright, along with anything you've saved. A mix with a track you - already know in it reads as broken. +Returns a taste-matched track mix that is stable for the rotation period (the +"Weekly Rotation" surface). The period rolls over on Wednesday 00:00 UTC, see +weeklyrotation.RolloverOffsetDays. -STABILITY. There is no precompute job and no stored playlist. Audius -playlists are on-chain entities, so minting one per user per week is not -on the table; instead the query is fully deterministic given -(user_id, iso_year, iso_week) and the result is cached until the week -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. +Differences from GET /v1/users/{id}/feed/for-you: + - The mix is fixed for the period instead of re-ranked on every load. + - Followed artists are demoted instead of boosted. + - Played and saved tracks are excluded instead of soft-penalized. -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. +STABILITY. Nothing is precomputed or stored. The query is deterministic given +(user_id, iso_year, iso_week) and the result is cached until the period rolls. +The week-to-week variation comes from week_seed, a hash of (track_id, user_id, +year, week); nothing uses random(). -The period rolls over on Wednesday 00:00 UTC, see -weeklyrotation.RolloverOffsetDays. +The listener's history (plays, saves, reposts, follows) is read as of the +period start, so listening to the mix doesn't change it mid-week. The +candidate pool and engagement counts are still live, so comparable tracks can +reorder during the week. Unsaves and unfollows during the period can still add +tracks. SCORING. quality_score = ln(1 + 3*saves + 2*reposts + 1*plays) / 12 - // same log-compressed engagement blend as For You: - // saves > reposts > plays. + // same engagement blend as For You. genre_affinity = 0.85 + 0.45 * min(genre_share / 0.30, 1) // genre_share is the fraction of my recent plays in - // the track's genre. Carried over from For You - // unchanged — this is the taste signal. - discovery_wt = {not followed, low affinity: 1.25, - not followed, some affinity: 1.00, - followed: 0.70} - // inverted relative to For You's in-network boost. + // the track's genre (same as For You). + discovery_wt = {not followed, no artist affinity: 1.25, + not followed, artist affinity: 1.00, + followed: 0.70} source_weight = {underground: 1.15, trending: 1.00} - // underground is upweighted here; the whole surface - // exists to promote things the listener wouldn't have - // stumbled into on the trending page. week_seed = 0.85 + 0.30 * hash01(track_id, user_id, year, week) final_score = quality_score * genre_affinity * discovery_wt * source_weight * week_seed -FILTERS. Track liveness (is_delete / is_unlisted / is_available / -stem_of), owner liveness (is_deactivated / is_available), gated tracks -excluded entirely (a mix the listener can't play through is worse than a -shorter mix), own uploads, anything played, anything saved, and anything -older than weeklyRotationMaxAgeDays. +FILTERS. Track liveness (is_delete / is_unlisted / is_available / stem_of), +owner liveness (is_deactivated / is_available), gated tracks, own uploads, +anything played, anything saved, and anything older than +weeklyRotationMaxAgeDays. -DIVERSITY. One track per artist, hard. For You allows 3 because a feed is -expected to show you more from someone you follow; a 30-track mix with two -tracks from the same artist has wasted a slot. +DIVERSITY. One track per artist (For You allows 3). Path: - id (required): the user being personalized for. Resolved by @@ -132,7 +103,7 @@ func (app *ApiServer) v1UsersWeeklyRotation(c *fiber.Ctx) error { return err } - // Tracks returns in the order of the id list, which is the product here. + // Tracks preserves the order of Ids. tracks, err := app.queries.Tracks(c.Context(), dbv1.TracksParams{ GetTracksParams: dbv1.GetTracksParams{ Ids: trackIds, @@ -161,16 +132,12 @@ func (app *ApiServer) getWeeklyRotationTrackIds( sql := ` WITH - -- Everything the listener has already heard. Unlike the For You feed, - -- which soft-penalizes repeats, these are excluded outright, so the - -- window is wider than that endpoint's 14 days. Still bounded: a heavy - -- 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). + -- Tracks the listener has already heard, excluded outright. Capped at the + -- latest 10k plays so heavy listeners stay under the upstream timeout + -- (see #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. + -- Every history CTE is cut off at @periodStart so listening to this + -- period's mix doesn't change it. See STABILITY in the handler doc. my_played AS ( SELECT DISTINCT play_item_id AS track_id FROM ( @@ -191,9 +158,8 @@ func (app *ApiServer) getWeeklyRotationTrackIds( 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 - -- hash table wide enough to stall the planner on the join below. + -- Capped like the For You feed's follow_set: thousands of follows + -- otherwise stall the planner on the join below. follow_set AS ( SELECT followee_user_id AS user_id FROM follows @@ -204,9 +170,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( ORDER BY created_at DESC LIMIT 500 ), - -- Genre mix of recent listening. Identical to the For You feed's - -- my_genre_affinity — this is the part of the taste model the two - -- surfaces genuinely share. + -- Genre mix of recent listening (same as the For You feed). my_genre_affinity AS ( SELECT t.genre, COUNT(*)::double precision / SUM(COUNT(*)) OVER () AS share @@ -222,9 +186,8 @@ func (app *ApiServer) getWeeklyRotationTrackIds( WHERE t.genre IS NOT NULL AND t.genre <> '' GROUP BY t.genre ), - -- Owners the listener already engages with. Used to demote, not boost: - -- an artist whose tracks they already save is by definition not a - -- discovery. Bounded by recency like the For You affinity CTE. + -- Artists the listener already saves or reposts. Demoted below, since + -- they aren't discoveries. Bounded by recency like the For You CTE. my_artist_affinity AS ( SELECT owner_id AS artist_id FROM ( @@ -256,14 +219,9 @@ func (app *ApiServer) getWeeklyRotationTrackIds( ), -- Source 1: weekly trending tracks. -- - -- No genre predicate, deliberately. track_trending_scores holds two - -- populations: rows carrying a genre, which are the live list the trending - -- job refreshes (median track age ~4 days), and rows with a null/empty - -- genre, which are stale -- in production those resolve to tracks five to - -- six years old. GET /tracks/trending and /tracks/trending/underground read - -- the live rows by omitting the genre filter, so this does the same. - -- Matching on a null-or-empty genre instead reads the stale population and, - -- combined with the age cutoff below, returns nothing at all. + -- No genre predicate: rows with a genre are the live trending list, and + -- rows with a null/empty genre are years-old leftovers. Matches + -- GET /tracks/trending. cand_trending AS ( SELECT tts.track_id, 'trending'::text AS source FROM track_trending_scores tts @@ -273,8 +231,8 @@ func (app *ApiServer) getWeeklyRotationTrackIds( ORDER BY tts.score DESC, tts.track_id DESC LIMIT 400 ), - -- Source 2: the same trending slice restricted to small creators. The - -- mirror of GET /tracks/trending/underground, and upweighted below. + -- Source 2: the same trending slice restricted to small creators, like + -- GET /tracks/trending/underground. Upweighted below. cand_underground AS ( SELECT tts.track_id, 'underground'::text AS source FROM track_trending_scores tts @@ -325,8 +283,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( AND t.is_unlisted = false AND t.is_available = true AND t.stem_of IS NULL - -- Gated tracks are dropped rather than surfaced-and-locked: a mix - -- the listener can't play straight through is worse than a short one. + -- Gated tracks are excluded so the mix plays straight through. AND t.is_stream_gated = false AND t.created_at >= NOW() - MAKE_INTERVAL(days => @maxAgeDays::int) AND t.owner_id <> @userId @@ -355,16 +312,11 @@ func (app *ApiServer) getWeeklyRotationTrackIds( END AS discovery_weight, CASE WHEN source = 'underground' THEN 1.15 ELSE 1.00 END AS source_weight, - -- hashtextextended is stable across sessions and servers, unlike - -- hashtext's platform-dependent variants, so every API node - -- computes the same mix for the same week. abs() then a mod into - -- [0,1): a plain (x % n) can be negative for negative x. - -- - -- The listener/period half of the seed arrives pre-formatted as - -- @seedKey rather than as separate int params: pgx infers one type - -- per named arg, and @userId is already pinned to int by the - -- equality predicates above, so casting it to text here would - -- conflict. + -- Deterministic jitter in [floor, floor + range). hashtextextended + -- is stable across servers, so every API node computes the same mix. + -- abs() before the mod keeps the result non-negative. @seedKey is pre-formatted + -- because pgx infers one type per named arg and @userId is already + -- used as an int above. @jitterFloor::float8 + @jitterRange::float8 * ( (ABS(HASHTEXTEXTENDED( track_id::text || ':' || @seedKey::text, 0 @@ -380,7 +332,7 @@ func (app *ApiServer) getWeeklyRotationTrackIds( * source_weight * week_seed AS score FROM scored ), - -- One track per artist, hard. + -- One track per artist. capped AS ( SELECT track_id, owner_id, score, ROW_NUMBER() OVER ( diff --git a/api/v1_users_weekly_rotation_test.go b/api/v1_users_weekly_rotation_test.go index 5d23ff42..d5582038 100644 --- a/api/v1_users_weekly_rotation_test.go +++ b/api/v1_users_weekly_rotation_test.go @@ -72,14 +72,10 @@ func weeklyRotationFixtures() database.FixtureMap { {"track_id": 1200, "owner_id": 2, "title": "deleted", "genre": "Rock", "created_at": daysAgo(5), "is_delete": true}, } - // 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. + // My listening history: a rock track by user 6, which makes Rock my + // affinity genre and 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. + // History must predate the period start, so fixtures are dated 8 days ago. plays := []map[string]any{ {"id": 1, "user_id": 1, "play_item_id": 600, "created_at": daysAgo(8)}, } @@ -177,10 +173,9 @@ func TestV1UsersWeeklyRotation(t *testing.T) { assert.Equal(t, 1, twoTrackCount, "an artist never occupies two slots") } -// A followed artist is demoted, not removed. The discovery weight gap -// (0.70 vs 1.25, a 1.79x ratio) is wider than the jitter band can close -// (1.35x at the extremes), so this ordering is guaranteed rather than -// merely likely. +// Followed artists are demoted, not removed. The discovery weight gap +// (0.70 vs 1.25, 1.79x) is wider than the jitter band (1.35x at most), so the +// ordering is deterministic. func TestV1UsersWeeklyRotationDemotesFollowedArtists(t *testing.T) { app := emptyTestApp(t) @@ -229,9 +224,6 @@ func TestV1UsersWeeklyRotationDemotesFollowedArtists(t *testing.T) { } // A listener with no plays, saves, follows, or reposts still gets a mix. -// This is the case that separates the surface from suggested-follows, which -// correctly returns nothing for a cold account: a mix that is empty on -// first open has no reason to exist. func TestV1UsersWeeklyRotationColdStart(t *testing.T) { app := emptyTestApp(t) @@ -265,13 +257,9 @@ func TestV1UsersWeeklyRotationColdStart(t *testing.T) { assert.Equal(t, "a track", resp.Data[0].Title.String) } -// The mix must not move within a week and must move between weeks. Both -// halves matter: the first is the product promise, the second is the only -// thing keeping the mix from being the same 30 tracks forever. -// -// Goes through getWeeklyRotationTrackIds rather than the HTTP handler -// because the handler derives the period from the wall clock, and the point -// here is to vary it. +// The mix is identical within a period and changes across periods and +// listeners. Calls getWeeklyRotationTrackIds directly because the handler +// takes the period from the wall clock. func TestV1UsersWeeklyRotationStableWithinWeek(t *testing.T) { app := emptyTestApp(t) @@ -282,9 +270,8 @@ func TestV1UsersWeeklyRotationStableWithinWeek(t *testing.T) { "aggregate_track": []map[string]any{}, "track_trending_scores": []map[string]any{}, } - // A pool of equally-strong candidates by distinct artists. Equal scores - // mean the week seed is the only thing deciding the order, which is - // exactly what this test is about. + // Equal-score candidates by distinct artists, so only the week seed + // decides the order. for i := 0; i < 40; i++ { userId := 100 + i trackId := 1000 + i @@ -366,18 +353,8 @@ func TestV1UsersWeeklyRotationRequiresValidUserId(t *testing.T) { assert.Equal(t, 400, status) } -// Regression for the bug that shipped in #1025 and returned an empty mix for -// every user in production. -// -// track_trending_scores holds two populations: rows carrying a genre, which -// the trending job keeps current, and rows with a null/empty genre, which are -// stale and resolve to tracks five to six years old. The original query -// matched only the null-genre rows, so every candidate then failed the -// 365-day age cutoff and the mix came back empty. -// -// Every other test here seeds score rows without a genre, so none of them -// could catch it. This one seeds a genre-carrying row specifically -- the -// shape that actually reaches the query in production. +// Candidates must come from genre-carrying trending rows, which are the live +// population in production. The other tests seed genre-less rows. func TestV1UsersWeeklyRotationReadsGenreCarryingTrendingRows(t *testing.T) { app := emptyTestApp(t) @@ -407,11 +384,7 @@ func TestV1UsersWeeklyRotationReadsGenreCarryingTrendingRows(t *testing.T) { "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. +// Plays inside the current period don't exclude tracks from the mix. func TestV1UsersWeeklyRotationKeepsTracksPlayedThisPeriod(t *testing.T) { app := emptyTestApp(t)