feat: source last month's top 5 from Spotify official top tracks API#88
Merged
Conversation
Replace the Supabase listening_history aggregation (previous UTC calendar month, only recorded while the site is polled) with Spotify's GET /me/top/tracks?time_range=short_term, which reflects the user's real top 5 over the last ~4 weeks across all devices. Requires the user-top-read scope on the refresh token. Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReplaces the Supabase-driven "last month top five tracks" feature with a direct Spotify Web API integration: adds a new mapping module ( ChangesSpotify Top Tracks Migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as API Route (/api/spotify/last-month-top)
participant TokenCache as Spotify Access Token
participant SpotifyAPI as Spotify Web API
participant Mapper as mapSpotifyTopTracks
Client->>Route: GET /api/spotify/last-month-top
Route->>TokenCache: get access token
TokenCache-->>Route: token (or none)
alt no token
Route-->>Client: { tracks: [] }
else token available
Route->>SpotifyAPI: fetch top tracks (Bearer token)
SpotifyAPI-->>Route: response
alt 401 Unauthorized
Route->>TokenCache: invalidate token
Route->>TokenCache: get refreshed token
Route->>SpotifyAPI: retry fetch top tracks
SpotifyAPI-->>Route: response
end
alt non-OK or JSON parse failure
Route-->>Client: { tracks: [] }
else success
Route->>Mapper: mapSpotifyTopTracks(payload)
Mapper-->>Route: ListeningHighlightTrack[]
Route-->>Client: { tracks } with cache headers
end
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/api/spotify/last-month-top/route.ts (2)
27-53: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider logging failures for observability.
All failure branches (missing token, non-OK Spotify response, JSON parse error, exhausted 401 retry) silently return
{ tracks: [] }with no logging. Given the PR notes this endpoint currently returns empty due to a missing scope, some server-side logging would make it easier to distinguish "no plays" from "auth/scope failure" in production.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/spotify/last-month-top/route.ts` around lines 27 - 53, Add server-side logging in GET for each empty-response path so failures are observable. In app/api/spotify/last-month-top/route.ts, use the existing GET flow, getSpotifyAccessToken, fetchTopTracks, and invalidateSpotifyAccessTokenCache to log when the access token is missing, when the first or retried Spotify response is 401, when res.ok is false, and when res.json() throws. Keep returning the same emptyResponse payload, but make each branch emit a distinct message so auth/scope failures can be distinguished from a true no-data case.
14-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant
revalidateexport
fetch(..., { cache: "no-store" })already makes this route dynamic, soexport const revalidate = 600doesn’t change the handler’s behavior. Keep theCache-Controlheader if CDN caching is the goal; otherwise droprevalidateto avoid implying Next is handling revalidation here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/spotify/last-month-top/route.ts` around lines 14 - 23, Remove the redundant revalidate export from the last-month-top route, since fetchTopTracks already uses fetch with cache: "no-store" and the route is therefore dynamic. Keep responseHeaders and its Cache-Control behavior as-is if CDN caching is still intended, but eliminate the export const revalidate declaration so the route doesn’t imply Next.js revalidation is being used here.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/spotify/last-month-top/route.ts`:
- Around line 20-25: The fetchTopTracks helper currently has no timeout, so
Spotify calls can hang on both the initial request and the 401 retry path.
Update fetchTopTracks to use an AbortController with a timeout, and make sure
the caller in the last-month-top route passes/handles the same timeout behavior
for retries so stalled requests are aborted promptly.
---
Nitpick comments:
In `@app/api/spotify/last-month-top/route.ts`:
- Around line 27-53: Add server-side logging in GET for each empty-response path
so failures are observable. In app/api/spotify/last-month-top/route.ts, use the
existing GET flow, getSpotifyAccessToken, fetchTopTracks, and
invalidateSpotifyAccessTokenCache to log when the access token is missing, when
the first or retried Spotify response is 401, when res.ok is false, and when
res.json() throws. Keep returning the same emptyResponse payload, but make each
branch emit a distinct message so auth/scope failures can be distinguished from
a true no-data case.
- Around line 14-23: Remove the redundant revalidate export from the
last-month-top route, since fetchTopTracks already uses fetch with cache:
"no-store" and the route is therefore dynamic. Keep responseHeaders and its
Cache-Control behavior as-is if CDN caching is still intended, but eliminate the
export const revalidate declaration so the route doesn’t imply Next.js
revalidation is being used here.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b6feeb17-2a20-4e57-9e8b-7e66f4fd587b
📒 Files selected for processing (10)
.env.exampleCLAUDE.mdREADME.mdapp/api/spotify/last-month-top/route.tsapp/components/listening-last-month-top.tsxlib/listening-monthly-top.test.tslib/listening-monthly-top.tslib/listening-supabase.tslib/spotify-top-tracks.test.tslib/spotify-top-tracks.ts
💤 Files with no reviewable changes (3)
- lib/listening-monthly-top.test.ts
- lib/listening-monthly-top.ts
- lib/listening-supabase.ts
Comment on lines
+20
to
+25
| function fetchTopTracks(accessToken: string) { | ||
| return fetch(TOP_TRACKS_URL, { | ||
| headers: { Authorization: `Bearer ${accessToken}` }, | ||
| cache: "no-store", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout to the Spotify fetch.
fetchTopTracks has no AbortController/timeout. If Spotify's API stalls, the request will hang until the platform's function timeout kicks in, tying up the request thread on every retry path (initial call and the 401 retry).
🕐 Proposed fix: add a request timeout
function fetchTopTracks(accessToken: string) {
- return fetch(TOP_TRACKS_URL, {
- headers: { Authorization: `Bearer ${accessToken}` },
- cache: "no-store",
- });
+ return fetch(TOP_TRACKS_URL, {
+ headers: { Authorization: `Bearer ${accessToken}` },
+ cache: "no-store",
+ signal: AbortSignal.timeout(5000),
+ });
}Also applies to: 34-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/spotify/last-month-top/route.ts` around lines 20 - 25, The
fetchTopTracks helper currently has no timeout, so Spotify calls can hang on
both the initial request and the 401 retry path. Update fetchTopTracks to use an
AbortController with a timeout, and make sure the caller in the last-month-top
route passes/handles the same timeout behavior for retries so stalled requests
are aborted promptly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/api/spotify/last-month-topnow calls Spotify's officialGET /me/top/tracks?time_range=short_term&limit=5(rolling ~4 weeks, computed by Spotify across all devices) instead of aggregating Supabaselistening_historyover the previous UTC calendar month — which both lagged behind (early-July views showed June data) and only captured plays observed while the site was being polled.getSpotifyAccessToken) with a 401 invalidate-and-retry; returns{ tracks: [] }on any failure. Newlib/spotify-top-tracks.tsmaps the response (affinity order preserved) with unit tests; removed the now-unusedlib/listening-monthly-top.tsand Supabase aggregation helpers.now-playing's Supabase writes are untouched..env.example): the refresh token now needs theuser-top-readscope.Deployment note
The current
SPOTIFY_REFRESH_TOKENlacksuser-top-read, so the endpoint returns an empty list (tooltip shows the empty state) until a re-authorized token is set in Vercel env.Test plan
npm run lint && npm run typecheck && npm run test && npm run buildall greenSPOTIFY_REFRESH_TOKEN(withuser-top-read), verify/api/spotify/last-month-topreturns 5 tracks matching the Spotify app's last-month topMade with Cursor
Summary by CodeRabbit
New Features
Documentation