fix(autoplay): capture skip rejections (symmetric completion threshold)

[LucasSantana-Dev]

Closes #1275.

Evidence

Prod, last 14 days (via scripts/autoplay-telemetry.sql): 68 accepted, 0 rejected, 119 pending → acceptance rate a meaningless 1.000 everywhere. The rejection signal — the input for all autoplay tuning + the ADR 2026-06-22 revisit — was never being captured.

Two root causes, one symmetric fix

Classify outcomes around the existing OUTCOME_ACCEPT_PLAY_RATIO (30%) in both event handlers:

1. playerSkip dead zone: only sub-5s skips recorded rejected; a skip between 5s and 30% was dropped as "ambiguous", so the common "heard ~15s, wrong track, skip" rejection vanished. Now skip before 30% = rejected (mirrors >30% = accepted; aligns with the existing implicit-dislike band). Removed OUTCOME_REJECT_EARLY_SKIP_MS.
2. Wrong event: discord-player v7 routes many manual skips through playerFinish (low completion) rather than playerSkip — which recorded nothing → the 119 pending. playerFinish now records accepted (>30%) or rejected (≤30%).

guildTrackStartTimes is deleted by whichever handler fires first, so a skip that emits both events still records exactly once (no double-count).

Tests

Repurposed the now-incorrect "ambiguous before 30%" test → asserts rejected; added an "ambiguous after 30% → not recorded" test and a "playerFinish ≤30% → rejected" test. Full trackHandlers suite green (16); bot typecheck clean.

Verify post-deploy

Re-run scripts/autoplay-telemetry.sql in ~a week: rejected count should be non-zero and per-source acceptance should finally show a real spread.

---

Summary by cubic

Fixes autoplay rejection telemetry by using a consistent 30% play-ratio threshold across finish and skip, including short tracks. Skips before 30% are rejected; finishes are rejected below 30% and accepted at ≥30%, closing the dead zone and removing the old 5s-only rule.

• Bug Fixes
  • playerSkip: reject skips before 30%; skips at or after 30% remain unrecorded. Short tracks (≤20s) now record outcomes too.
  • playerFinish: record accepted (≥30%) or rejected (<30%) to capture manual skips routed through finish in discord-player v7.
  • Prevent double recording by clearing start-time when the first handler fires.

^{Written for commit a2755ff. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes

  • Updated autoplay recommendation outcome tracking to consistently record results when tracks are skipped or finished.
  • Changed threshold logic from time-based to play-ratio-based for more predictable autoplay behavior.

• Tests

  • Expanded test coverage for autoplay skip and completion scenarios.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
lucky	Ready	Preview, Comment	Jun 9, 2026 11:07pm

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR changes autoplay recommendation outcome telemetry from time-window-based (early skip within 5 seconds) to play-ratio-based classification (skip or finish below 30% completion). The acceptance threshold constant is clarified and the old time-constant is removed; both handlePlayerFinish and handlePlayerSkip now record explicit outcomes (accepted/rejected) for recommendation autoplay tracks based on play progress.

Changes

Autoplay recommendation outcome telemetry

Layer / File(s)	Summary
Acceptance threshold documentation and constant cleanup packages/bot/src/handlers/player/trackHandlers.ts	The exported OUTCOME_ACCEPT_PLAY_RATIO documentation is updated to clarify both playerFinish and playerSkip use it for outcome classification; the old OUTCOME_REJECT_EARLY_SKIP_MS early-skip constant is removed.
playerFinish and playerSkip outcome recording packages/bot/src/handlers/player/trackHandlers.ts	handlePlayerFinish records both accepted (when completion ratio exceeds threshold) and rejected outcomes for recommendation tracks; handlePlayerSkip records rejected when skip ratio is below threshold and updates implicit dislike tracking via the same ratio rule, replacing prior early-skip-time logic.
Test validation of ratio-based classification packages/bot/src/handlers/player/trackHandlers.spec.ts	Tests assert that skips before 30% completion record rejected outcomes, skips after 30% record no outcome, and finishes at or below 30% record rejected outcomes, validating the new accept/reject/ambiguous bands.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• LucasSantana-Dev/Lucky#1097: Updates autoplay recommendation outcome recording in this PR that feeds the guild skip-rate circuit breaker logic in that PR.
• LucasSantana-Dev/Lucky#577: Also modifies the same playerSkip/playerFinish handlers to change ratio-based behavior in autoplay skip/finish tracking.
• LucasSantana-Dev/Lucky#936: Earlier implementation of the autoplay recommendation telemetry infrastructure that this PR refines with updated threshold logic.

Suggested labels

bot, size/m

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing autoplay to capture skip rejections using a symmetric completion threshold, which is the core objective of this PR.
Linked Issues check	✅ Passed	The PR addresses all four acceptance criteria from issue #1275: (1) skips before 30% now record rejected outcome (not just sub-5s), (2) guildTrackStartTimes handling ensures skip events fire, (3) telemetry changes enable post-deploy validation, and (4) unit tests verify accept/reject classification by completion ratio.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the rejection telemetry fix: trackHandlers.ts updates the skip/finish logic and removes the obsolete OUTCOME_REJECT_EARLY_SKIP_MS constant, and the test file updates validate the new classification behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/autoplay-rejection-telemetry

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Failed to generate code suggestions for PR

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/bot/src/handlers/player/trackHandlers.ts (1)

302-328: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Consume guildTrackStartTimes before the first awaited telemetry write.

At Line 302 and Line 360 the handlers read the cached start time, but the key is only deleted at Line 328 / Line 385 after several awaited calls. If the same autoplay track emits both playerSkip and playerFinish, the second handler can still see the entry and persist a second outcome before the first one reaches the delete. Downstream, accepted/rejected are stored on separate fields, so this can leave one recommendation row marked both ways.

Suggested fix

+function consumeGuildTrackStartTime(guildId: string): number | undefined {
+    const startTime = guildTrackStartTimes.get(guildId)
+    if (startTime !== undefined) {
+        guildTrackStartTimes.delete(guildId)
+    }
+    return startTime
+}
+
 const handlePlayerFinish = async (
     queue: GuildQueue,
     track?: Track,
 ): Promise<void> => {
@@
         if (track) {
-            const startTime = guildTrackStartTimes.get(queue.guild.id)
+            const startTime = consumeGuildTrackStartTime(queue.guild.id)
             if (startTime && track.durationMS) {
                 const completionRatio =
                     (Date.now() - startTime) / track.durationMS
@@
-            guildTrackStartTimes.delete(queue.guild.id)
         }
@@
         if (track) {
-            const startTime = guildTrackStartTimes.get(queue.guild.id)
+            const startTime = consumeGuildTrackStartTime(queue.guild.id)
             if (startTime && track.durationMS && track.durationMS > 20_000) {
                 const skipRatio = (Date.now() - startTime) / track.durationMS
@@
-            guildTrackStartTimes.delete(queue.guild.id)
         }

Also applies to: 360-385

🤖 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 `@packages/bot/src/handlers/player/trackHandlers.ts` around lines 302 - 328,
Read and immediately remove the cached start time from guildTrackStartTimes
before performing any awaited telemetry writes: capture const startTime =
guildTrackStartTimes.get(queue.guild.id) into a local variable and call
guildTrackStartTimes.delete(queue.guild.id) right away, then use that local
startTime for completionRatio logic and subsequent awaits
(recordImplicitTrackFeedback, recordRecommendationOutcome) to prevent race
conditions; apply the same pattern in the other handler block that reads
guildTrackStartTimes (the playerSkip/playerFinish pair) so the key cannot be
observed by concurrent handlers.

🤖 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 `@packages/bot/src/handlers/player/trackHandlers.ts`:
- Around line 317-325: The finish-path is recording recommendation outcomes
regardless of short duration while the skip-path suppresses writes for
durationMS <= 20_000, causing path-dependent telemetry; update the logic so both
paths use the same decision and suppression. Create or use a single helper
(e.g., computeAndRecordRecommendationOutcome or similar) called from
playerFinish and playerSkip that first checks durationMS against the short-play
threshold (20_000), returns early if too short, otherwise computes
completionRatio vs OUTCOME_ACCEPT_PLAY_RATIO and calls
recordRecommendationOutcome with guildId, track.id and the 'accepted'|'rejected'
outcome; replace the direct recordRecommendationOutcome call in the finish
handler (where isRecommendationAutoplay(track) is checked) with this shared
helper so both handlers behave identically.

---

Outside diff comments:
In `@packages/bot/src/handlers/player/trackHandlers.ts`:
- Around line 302-328: Read and immediately remove the cached start time from
guildTrackStartTimes before performing any awaited telemetry writes: capture
const startTime = guildTrackStartTimes.get(queue.guild.id) into a local variable
and call guildTrackStartTimes.delete(queue.guild.id) right away, then use that
local startTime for completionRatio logic and subsequent awaits
(recordImplicitTrackFeedback, recordRecommendationOutcome) to prevent race
conditions; apply the same pattern in the other handler block that reads
guildTrackStartTimes (the playerSkip/playerFinish pair) so the key cannot be
observed by concurrent handlers.

🪄 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: d627a923-45d2-434b-81a4-4fb375e0496f

📥 Commits

Reviewing files that changed from the base of the PR and between 7cefbf2 and 49a878c.

📒 Files selected for processing (2)

• packages/bot/src/handlers/player/trackHandlers.spec.ts
• packages/bot/src/handlers/player/trackHandlers.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). (12)

• GitHub Check: Checks
• GitHub Check: Test — backend
• GitHub Check: Test — bot
• GitHub Check: Test — frontend
• GitHub Check: cubic · AI code reviewer
• GitHub Check: quality / Dead code (knip)
• GitHub Check: quality / Lint (lint)
• GitHub Check: quality / SAST (CodeQL) (javascript-typescript)
• GitHub Check: Build — frontend
• GitHub Check: Security
• GitHub Check: Build — bot
• GitHub Check: Build — backend

[cubic-dev-ai]

2 issues found across 2 files

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud