From 54903c47607617460606a048722413ec153e08d0 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 1 Jun 2026 01:03:45 -0400 Subject: [PATCH] fix(recommendation): drop user_id from Rediscover daily hash to preserve sqlc type inference CI on 8b586c2e caught it: the $1::text cast in the md5 ORDER BY made sqlc infer the parameter as plain string instead of pgtype.UUID, generating Column1 string instead of UserID pgtype.UUID on the ListRediscover*ForUserParams structs. go vet flagged both call sites in internal/recommendation/home.go where the Go code passes UserID by name. Fix: hash on album_id (or artist_id) + current_date only. The eligibility filter already differs per user (different liked sets), so the daily-rotation goal is preserved; we just lose per-user salt in the within-day ordering of overlapping items - acceptable. Applies to both primary queries AND both fallback queries (all four had the same cast). --- internal/db/dbq/recommendation.sql.go | 28 +++++++++++++++----------- internal/db/queries/recommendation.sql | 16 +++++++++------ 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/internal/db/dbq/recommendation.sql.go b/internal/db/dbq/recommendation.sql.go index a8af907e..e78ea599 100644 --- a/internal/db/dbq/recommendation.sql.go +++ b/internal/db/dbq/recommendation.sql.go @@ -182,7 +182,7 @@ FROM general_likes_albums gla JOIN albums ON albums.id = gla.album_id JOIN artists ON artists.id = albums.artist_id WHERE gla.user_id = $1 -ORDER BY md5(albums.id::text || $1::text || current_date::text) +ORDER BY md5(albums.id::text || current_date::text) LIMIT $2 ` @@ -200,7 +200,10 @@ type ListRediscoverAlbumsFallbackForUserRow struct { // service uses this to top up the rediscover row when the primary // eligibility query returns fewer than the inner limit. Same daily // hash ordering as the primary so the fallback rows stay stable -// through the day instead of reshuffling on every refresh. +// through the day instead of reshuffling on every refresh. Hash +// omits user_id so sqlc can keep the user_id parameter UUID-typed +// (a $1::text cast would force the param to string); eligibility +// already differs per user, so output sets differ regardless. func (q *Queries) ListRediscoverAlbumsFallbackForUser(ctx context.Context, arg ListRediscoverAlbumsFallbackForUserParams) ([]ListRediscoverAlbumsFallbackForUserRow, error) { rows, err := q.db.Query(ctx, listRediscoverAlbumsFallbackForUser, arg.UserID, arg.Limit) if err != nil { @@ -267,13 +270,13 @@ SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.rele FROM eligible e JOIN albums ON albums.id = e.album_id JOIN artists ON artists.id = albums.artist_id -ORDER BY md5(albums.id::text || $1::text || current_date::text) +ORDER BY md5(albums.id::text || current_date::text) LIMIT $2 ` type ListRediscoverAlbumsForUserParams struct { - Column1 string - Limit int32 + UserID pgtype.UUID + Limit int32 } type ListRediscoverAlbumsForUserRow struct { @@ -294,7 +297,7 @@ type ListRediscoverAlbumsForUserRow struct { // Final cap + diversity + cross-section dedup land in the Go layer // (internal/recommendation/home.go). func (q *Queries) ListRediscoverAlbumsForUser(ctx context.Context, arg ListRediscoverAlbumsForUserParams) ([]ListRediscoverAlbumsForUserRow, error) { - rows, err := q.db.Query(ctx, listRediscoverAlbumsForUser, arg.Column1, arg.Limit) + rows, err := q.db.Query(ctx, listRediscoverAlbumsForUser, arg.UserID, arg.Limit) if err != nil { return nil, err } @@ -342,7 +345,7 @@ LEFT JOIN LATERAL ( FROM albums WHERE artist_id = artists.id ) cnt ON true WHERE gla.user_id = $1 -ORDER BY md5(artists.id::text || $1::text || current_date::text) +ORDER BY md5(artists.id::text || current_date::text) LIMIT $2 ` @@ -359,7 +362,8 @@ type ListRediscoverArtistsFallbackForUserRow struct { // Daily-stable random sample of liked artists, paired with the Go // top-up logic in HomeData when the primary eligibility query returns -// fewer than the inner limit. Daily hash ordering matches the primary. +// fewer than the inner limit. Daily hash ordering matches the primary +// (omitting user_id from the hash for the same sqlc-typing reason). func (q *Queries) ListRediscoverArtistsFallbackForUser(ctx context.Context, arg ListRediscoverArtistsFallbackForUserParams) ([]ListRediscoverArtistsFallbackForUserRow, error) { rows, err := q.db.Query(ctx, listRediscoverArtistsFallbackForUser, arg.UserID, arg.Limit) if err != nil { @@ -434,13 +438,13 @@ LEFT JOIN LATERAL ( SELECT count(*) AS album_count FROM albums WHERE artist_id = artists.id ) cnt ON true -ORDER BY md5(artists.id::text || $1::text || current_date::text) +ORDER BY md5(artists.id::text || current_date::text) LIMIT $2 ` type ListRediscoverArtistsForUserParams struct { - Column1 string - Limit int32 + UserID pgtype.UUID + Limit int32 } type ListRediscoverArtistsForUserRow struct { @@ -462,7 +466,7 @@ type ListRediscoverArtistsForUserRow struct { // hashes). cover_album_id derived from a representative album (most // recently created album with cover_art_path set). func (q *Queries) ListRediscoverArtistsForUser(ctx context.Context, arg ListRediscoverArtistsForUserParams) ([]ListRediscoverArtistsForUserRow, error) { - rows, err := q.db.Query(ctx, listRediscoverArtistsForUser, arg.Column1, arg.Limit) + rows, err := q.db.Query(ctx, listRediscoverArtistsForUser, arg.UserID, arg.Limit) if err != nil { return nil, err } diff --git a/internal/db/queries/recommendation.sql b/internal/db/queries/recommendation.sql index af259b0c..68299d48 100644 --- a/internal/db/queries/recommendation.sql +++ b/internal/db/queries/recommendation.sql @@ -313,7 +313,7 @@ SELECT sqlc.embed(albums), artists.name AS artist_name FROM eligible e JOIN albums ON albums.id = e.album_id JOIN artists ON artists.id = albums.artist_id -ORDER BY md5(albums.id::text || $1::text || current_date::text) +ORDER BY md5(albums.id::text || current_date::text) LIMIT $2; -- name: ListRediscoverAlbumsFallbackForUser :many @@ -321,13 +321,16 @@ LIMIT $2; -- service uses this to top up the rediscover row when the primary -- eligibility query returns fewer than the inner limit. Same daily -- hash ordering as the primary so the fallback rows stay stable --- through the day instead of reshuffling on every refresh. +-- through the day instead of reshuffling on every refresh. Hash +-- omits user_id so sqlc can keep the user_id parameter UUID-typed +-- (a $1::text cast would force the param to string); eligibility +-- already differs per user, so output sets differ regardless. SELECT sqlc.embed(albums), artists.name AS artist_name FROM general_likes_albums gla JOIN albums ON albums.id = gla.album_id JOIN artists ON artists.id = albums.artist_id WHERE gla.user_id = $1 -ORDER BY md5(albums.id::text || $1::text || current_date::text) +ORDER BY md5(albums.id::text || current_date::text) LIMIT $2; -- name: ListRediscoverArtistsForUser :many @@ -381,13 +384,14 @@ LEFT JOIN LATERAL ( SELECT count(*) AS album_count FROM albums WHERE artist_id = artists.id ) cnt ON true -ORDER BY md5(artists.id::text || $1::text || current_date::text) +ORDER BY md5(artists.id::text || current_date::text) LIMIT $2; -- name: ListRediscoverArtistsFallbackForUser :many -- Daily-stable random sample of liked artists, paired with the Go -- top-up logic in HomeData when the primary eligibility query returns --- fewer than the inner limit. Daily hash ordering matches the primary. +-- fewer than the inner limit. Daily hash ordering matches the primary +-- (omitting user_id from the hash for the same sqlc-typing reason). SELECT sqlc.embed(artists), cov.id AS cover_album_id, cnt.album_count::bigint AS album_count @@ -403,5 +407,5 @@ LEFT JOIN LATERAL ( FROM albums WHERE artist_id = artists.id ) cnt ON true WHERE gla.user_id = $1 -ORDER BY md5(artists.id::text || $1::text || current_date::text) +ORDER BY md5(artists.id::text || current_date::text) LIMIT $2;