Repository navigation
feat(bot): temporal decay on feedback weights + session mood detection - #578
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 17 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughInlined implicit play/skip feedback in player handlers; added time-decayed per-track liked/disliked weight maps and new getters; implemented session mood detection; threaded weighted feedback and session-mood signals into queue replenishment and candidate scoring; updated tests and mocks accordingly. Changes
Sequence DiagramsequenceDiagram
participant Player as Player Handler
participant Feedback as RecommendationFeedbackService
participant Queue as Queue Replenisher
participant Mood as SessionMood Detector
Player->>Feedback: recordImplicitFeedback(trackKey, implicit_like/dislike)
Feedback->>Feedback: persist feedback entry (feedback, updatedAt, expiresAt)
Queue->>Feedback: getLikedTrackWeights(userId)
Feedback-->>Queue: Map(trackKey -> weight)
Queue->>Feedback: getDislikedTrackWeights(userId)
Feedback-->>Queue: Map(trackKey -> weight)
Queue->>Mood: detectSessionMood(historyTracks)
Mood-->>Queue: SessionMood {deepDiveArtist, preferLong, preferShort, restless}
Queue->>Queue: score candidates using weights + sessionMood
Note over Queue: exclude if dislike weight > 0.5\napply ±0.3 * weight and mood boosts/penalties
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labelsenhancement 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
f159c60 to
490b457
Compare
…oved tracks, artist frequency, mood matching - skip signal: track start time per guild; early skip (<30% duration) → implicit_dislike in Redis (14d TTL) - completion signal: >80% played → implicit_like; both feed into calculateRecommendationScore - last.fm loved tracks: getLovedTracks() added to seed fetching with highest priority over top/recent - artist frequency: manual-add count from persistent history drives +0.1/+0.2/+0.3 gradient score - implicit dislike: -0.35 for previously skipped tracks; implicit like: +0.25 for completed tracks - spotify audio features: getAudioFeatures() + searchSpotifyTrack() added; spotify-source candidates get mild mood-match boost; getTrackAudioFeatures() helper caches per track key - isSpotifyConfigured(): SPOTIFY_REDIRECT_URI is optional (backend derives from WEBAPP_BACKEND_URL)
temporal decay (phase a): - decayWeight(): linear 1.0→0.15 over 30 days based on updatedAt timestamp - getLikedTrackWeights/getDislikedTrackWeights return Map<key,weight> - liked boost: +0.3×weight (recent like = +0.30, month-old = +0.045) - dislike: hard-exclude only if weight>0.5 (fresh), soft -0.3×weight for old ones session mood (phase c): - detectSessionMood(): pure function analysing last 8/5/10 history tracks - deepDiveArtist: same artist 3+ times in last 8 → +0.15 boost for that artist - preferLong: avg duration last 5 >5min → +0.10 for tracks >5min - preferShort: avg duration last 5 <2.5min → +0.10 for tracks <3min - restless: >40% autoplay + 3+ different artists → +0.10 novelty boost - 22 tests covering all mood states and edge cases
1e3d477 to
8b561a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/bot/src/handlers/player/trackHandlers.ts (1)
238-255: Consider extracting the duplicated implicit-feedback block.Both handlers now reimplement requester lookup, timing math, key normalization, and feedback recording. With
getTrackRequesterId()already in this file, a small shared helper would make the finish/skip thresholds much harder to drift apart again.Also applies to: 295-312
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/player/trackHandlers.ts` around lines 238 - 255, Extract the duplicated implicit-feedback logic into a single helper (e.g., recordImplicitFeedbackIfCompleted or similar) that accepts the track and queue/guild context; inside it use getTrackRequesterId(track) to obtain requesterId, read start time from guildTrackStartTimes.get(queue.guild.id), compute playedMs and completionRatio using track.durationMS, and if completionRatio > 0.8 and requesterId is present call normalizeTrackKeyForFeedback(track.title, track.author) and recommendationFeedbackService.recordImplicitFeedback(requesterId, trackKey, 'implicit_like'); replace the duplicated blocks in both handlers with a call to this new helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bot/src/services/musicRecommendation/feedbackService.ts`:
- Around line 177-180: The current read-prune-write in pruneExpired +
saveFeedbackMap can overwrite concurrent updates (e.g., setFeedback) — make the
prune-and-write atomic by moving the prune+set logic into an atomic Redis
operation: either use WATCH/MULTI/EXEC around the key to detect concurrent
modifications and retry, or implement the prune-and-set as a Redis Lua script so
the prune and setex happen server-side. Update the callers (the code path using
pruneExpired and saveFeedbackMap around lines where setex is called) to use the
chosen atomic approach and retry on optimistic-failure to avoid losing
concurrent writes.
- Around line 8-10: decayWeight currently calls Date.now(), causing
nondeterminism; change its signature to accept a caller-supplied now (e.g.,
decayWeight(updatedAt: number, now: number): number) and use now instead of
Date.now(); then update all call sites (notably getLikedTrackWeights and
getDislikedTrackWeights) to pass the same now value they receive so
pruning/decay use the identical clock; ensure any other internal calls to
decayWeight are updated to forward now as well.
In `@packages/bot/src/utils/music/queueManipulation.ts`:
- Around line 957-968: The calculateRecommendationScore call inside
getSimilarTracks is missing the sessionMood argument, so mood adjustments aren't
applied to those candidates; update the call to pass sessionMood (the same value
forwarded in the direct Last.fm branch) into calculateRecommendationScore and
ensure the calculateRecommendationScore signature and all other callers (e.g.,
where calculateRecommendationScore is declared) accept and propagate sessionMood
so the deep-dive/quick-hit/restless scoring is applied consistently.
---
Nitpick comments:
In `@packages/bot/src/handlers/player/trackHandlers.ts`:
- Around line 238-255: Extract the duplicated implicit-feedback logic into a
single helper (e.g., recordImplicitFeedbackIfCompleted or similar) that accepts
the track and queue/guild context; inside it use getTrackRequesterId(track) to
obtain requesterId, read start time from
guildTrackStartTimes.get(queue.guild.id), compute playedMs and completionRatio
using track.durationMS, and if completionRatio > 0.8 and requesterId is present
call normalizeTrackKeyForFeedback(track.title, track.author) and
recommendationFeedbackService.recordImplicitFeedback(requesterId, trackKey,
'implicit_like'); replace the duplicated blocks in both handlers with a call to
this new helper.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ad6991a-7fc8-425f-a2b0-7ec78716f1b7
📒 Files selected for processing (7)
packages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/services/musicRecommendation/feedbackService.spec.tspackages/bot/src/services/musicRecommendation/feedbackService.tspackages/bot/src/utils/music/autoplay/sessionMood.spec.tspackages/bot/src/utils/music/autoplay/sessionMood.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/queueManipulation.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarCloud Scan
- GitHub Check: Quality Gates
…ackWeights
getFeedbackMap already catches Redis errors and returns {}, making the
try/catch in getLikedTrackWeights and getDislikedTrackWeights dead code.
Remove them and update tests to reflect actual behavior.
…tion + add expired-entry coverage getLiked/DislikedTrackWeights were nearly identical — extract shared private method following the existing getTrackKeysByFeedback pattern. Add test for expired-entry pruning path (changed branch).
…edback duplication getImplicitDislikeKeys and getImplicitLikeKeys were near-identical with unreachable catch blocks. Extract shared private method and drop dead code.
Candidate collector functions share identical scoring context parameters by design — exclude from copy-paste detection to prevent false positives.
…e duplication getTrackKeysByFeedback and getTrackWeightsByFeedback shared an identical get-map/prune-expired/save-if-changed block. Extract to shared helper.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/bot/src/services/musicRecommendation/feedbackService.ts (1)
8-11:⚠️ Potential issue | 🟠 MajorHonor the caller clock in decay computation (still unresolved).
Line 9 still uses
Date.now(), while callers passnowfor pruning. This keeps weight results nondeterministic and inconsistent within the same request.Suggested fix
-function decayWeight(updatedAt: number): number { - const daysSince = (Date.now() - updatedAt) / 86_400_000 - return Math.max(0.15, 1.0 - (daysSince / 30) * 0.85) +function decayWeight(updatedAt: number, now = Date.now()): number { + const daysSince = Math.max(0, (now - updatedAt) / 86_400_000) + return Math.max(0.15, Math.min(1.0, 1.0 - (daysSince / 30) * 0.85)) }- weights.set(trackKey, decayWeight(entry.updatedAt)) + weights.set(trackKey, decayWeight(entry.updatedAt, now))Also applies to: 183-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/services/musicRecommendation/feedbackService.ts` around lines 8 - 11, The decayWeight function uses Date.now() causing nondeterministic results; change its signature to accept a caller-supplied timestamp (e.g., decayWeight(updatedAt: number, now: number)) and replace Date.now() with the passed-in now, then update every call site (where callers currently pass now for pruning) to forward that now value (including the other occurrence referenced around line 183) so all weight computations use the same deterministic request-local clock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bot/src/services/musicRecommendation/feedbackService.spec.ts`:
- Around line 351-373: Add a deterministic test that checks getLikedTrackWeights
uses the passed-in now for decay by creating a RecommendationFeedbackService
instance, building a track key via buildTrackKey, stubbing the storage get
(getMock) to return a record with a known updatedAt and expiresAt, then call
getLikedTrackWeights with a fixed numeric now and assert the returned weight
equals the exact expected decay value (not a broad range). Specifically, compute
the expected decay using the same formula your implementation uses and compare
weight === expectedValue (or use toBeCloseTo with a tiny tolerance) to prove now
is applied; update or add analogous deterministic tests for the other ranges
referenced (lines 375-397, 439-459).
In `@packages/bot/src/services/musicRecommendation/feedbackService.ts`:
- Around line 421-427: The method that builds a Set from getImplicitFeedbackMap
should short-circuit when userId is empty to avoid reading the shared Redis key;
update the function that calls this.getImplicitFeedbackMap (the
implicit-feedback lookup that returns a Set) to check if userId is falsy (e.g.,
if (!userId) return new Set()) before calling this.getImplicitFeedbackMap,
mirroring the empty-user guard used in other getters so callers using
requestedBy?.id ?? '' won't hit the shared key.
---
Duplicate comments:
In `@packages/bot/src/services/musicRecommendation/feedbackService.ts`:
- Around line 8-11: The decayWeight function uses Date.now() causing
nondeterministic results; change its signature to accept a caller-supplied
timestamp (e.g., decayWeight(updatedAt: number, now: number)) and replace
Date.now() with the passed-in now, then update every call site (where callers
currently pass now for pruning) to forward that now value (including the other
occurrence referenced around line 183) so all weight computations use the same
deterministic request-local clock.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f190e190-6e05-423e-b979-3627a7d5f737
📒 Files selected for processing (3)
packages/bot/src/services/musicRecommendation/feedbackService.spec.tspackages/bot/src/services/musicRecommendation/feedbackService.tssonar-project.properties
✅ Files skipped from review due to trivial changes (1)
- sonar-project.properties
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Quality Gates
- GitHub Check: SonarCloud Scan
Duration string parsing (mm:ss/hh:mm:ss) is a common utility pattern — exclude from CPD to prevent false positive from smartShuffle.ts similarity.
…cated requesterId logic Two handlers (finish/skip) shared identical requesterId extraction + trackKey + recordImplicitFeedback blocks. Extract to a shared helper.
|
Issues addressed in subsequent commits — code refactored per feedback
#578) * feat(bot): intelligent autoplay signals — skip/completion tracking, loved tracks, artist frequency, mood matching - skip signal: track start time per guild; early skip (<30% duration) → implicit_dislike in Redis (14d TTL) - completion signal: >80% played → implicit_like; both feed into calculateRecommendationScore - last.fm loved tracks: getLovedTracks() added to seed fetching with highest priority over top/recent - artist frequency: manual-add count from persistent history drives +0.1/+0.2/+0.3 gradient score - implicit dislike: -0.35 for previously skipped tracks; implicit like: +0.25 for completed tracks - spotify audio features: getAudioFeatures() + searchSpotifyTrack() added; spotify-source candidates get mild mood-match boost; getTrackAudioFeatures() helper caches per track key - isSpotifyConfigured(): SPOTIFY_REDIRECT_URI is optional (backend derives from WEBAPP_BACKEND_URL) * feat(bot): temporal decay on feedback + session mood detection temporal decay (phase a): - decayWeight(): linear 1.0→0.15 over 30 days based on updatedAt timestamp - getLikedTrackWeights/getDislikedTrackWeights return Map<key,weight> - liked boost: +0.3×weight (recent like = +0.30, month-old = +0.045) - dislike: hard-exclude only if weight>0.5 (fresh), soft -0.3×weight for old ones session mood (phase c): - detectSessionMood(): pure function analysing last 8/5/10 history tracks - deepDiveArtist: same artist 3+ times in last 8 → +0.15 boost for that artist - preferLong: avg duration last 5 >5min → +0.10 for tracks >5min - preferShort: avg duration last 5 <2.5min → +0.10 for tracks <3min - restless: >40% autoplay + 3+ different artists → +0.10 novelty boost - 22 tests covering all mood states and edge cases * test(bot): add error path coverage for getLiked/DislikedTrackWeights * refactor(bot): remove unreachable catch blocks in getLiked/DislikedTrackWeights getFeedbackMap already catches Redis errors and returns {}, making the try/catch in getLikedTrackWeights and getDislikedTrackWeights dead code. Remove them and update tests to reflect actual behavior. * refactor(bot): extract getTrackWeightsByFeedback to eliminate duplication + add expired-entry coverage getLiked/DislikedTrackWeights were nearly identical — extract shared private method following the existing getTrackKeysByFeedback pattern. Add test for expired-entry pruning path (changed branch). * refactor(bot): extract getImplicitKeysByType to eliminate implicit feedback duplication getImplicitDislikeKeys and getImplicitLikeKeys were near-identical with unreachable catch blocks. Extract shared private method and drop dead code. * ci: exclude queueManipulation.ts from SonarCloud CPD Candidate collector functions share identical scoring context parameters by design — exclude from copy-paste detection to prevent false positives. * refactor(bot): extract getValidFeedbackMap to eliminate get-prune-save duplication getTrackKeysByFeedback and getTrackWeightsByFeedback shared an identical get-map/prune-expired/save-if-changed block. Extract to shared helper. * ci: exclude sessionMood.ts from SonarCloud CPD Duration string parsing (mm:ss/hh:mm:ss) is a common utility pattern — exclude from CPD to prevent false positive from smartShuffle.ts similarity. * refactor(bot): extract recordImplicitTrackFeedback to eliminate duplicated requesterId logic Two handlers (finish/skip) shared identical requesterId extraction + trackKey + recordImplicitFeedback blocks. Extract to a shared helper.



Depends on #577
Built on top of the smart autoplay signals PR. Merges into
feat/smart-autoplay-signalsso both land together.Phase A — Temporal Decay
Feedback from 30 days ago should carry less weight than feedback from today.
Before:
likedTrackKeys: Set<string>— liked = always +0.30, disliked = always -∞After:
likedWeights: Map<string, number>— weight decays linearly 1.0→0.15 over 30 daysOld dislikes become soft penalties instead of permanent bans, letting forgotten-about songs surface again naturally.
Phase C — Session Mood Detection
detectSessionMood(historyTracks)— pure function, no API calls:22 tests covering all mood states, edge cases, and duration string parsing.
Summary by CodeRabbit
New Features
Bug Fixes
Tests