Repository navigation
fix(bot): deduplicate same-song versions in autoplay - #569
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 3 minutes and 37 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)
📝 WalkthroughWalkthroughChanged queue candidate upsert/selection to use normalized title/author heuristics and enforce title-level deduplication; expanded hyphenated version-suffix patterns in search title cleaning; added tests validating deduplication and new suffix-cleaning behaviors. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/bot/src/utils/music/queueManipulation.spec.ts (1)
2207-2210: Strengthen dedup assertions to exact expected outcomes.
<= 1is too lenient and can hide regressions where expected candidates are dropped entirely. Use exact counts per scenario (e.g.,1for the first case,0for the current-track duplicate case).🔍 Suggested assertion tightening
- expect(same_song_added).toBeLessThanOrEqual(1) + expect(same_song_added).toBe(1) ... - expect(bohemian_added).toBeLessThanOrEqual(1) + expect(bohemian_added).toBe(0)Also applies to: 2253-2258
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/queueManipulation.spec.ts` around lines 2207 - 2210, The test uses a lenient assertion on duplicate filtering: replace the non-specific expect(same_song_added).toBeLessThanOrEqual(1) with exact expected counts for each scenario so failures reveal dropped candidates; for the scenario computing same_song_added from addedTracks (variable name same_song_added, array addedTracks) assert exactly 1 when a non-current-track duplicate should be kept, and assert exactly 0 in the scenario where the duplicate is the current track. Apply the same tightening to the other assertions referenced around the block for lines 2253–2258 to use precise counts instead of <=1.
🤖 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/utils/music/queueManipulation.ts`:
- Around line 614-618: The current logic sets candidateKey to getTrackKey when
normalizeTrackKey(candidate.title, candidate.author) is very short, which can
reintroduce duplicate variants; instead always base the dedupe key on
normalizeTrackKey but, when its length <= 4, append a stable fingerprint derived
from getTrackKey so short normalized keys still remain tied to the same track
(e.g., candidateKey = normalizedKey + '|' + getTrackKey(candidate) if
normalizedKey.length <= 4); update the code around normalizeTrackKey,
getTrackKey, and the candidateKey assignment to implement this deterministic
combination so deduplication still works for short titles/authors.
In `@packages/bot/src/utils/music/searchQueryCleaner.ts`:
- Line 87: The regex in searchQueryCleaner.ts currently strips any 4-digit
sequence (|\d{4}), which over-cleans numeric titles; update the pattern to
constrain year matches to a plausible range (e.g., use (?:19\d{2}|20\d{2}) or a
narrower span like (?:19[5-9]\d|20[0-4]\d) ) wherever \d{4} is used in the regex
(the leading '^(?:\d{4} +)?' group and the final alternation) so only realistic
years are matched and non-year numeric suffixes are preserved.
---
Nitpick comments:
In `@packages/bot/src/utils/music/queueManipulation.spec.ts`:
- Around line 2207-2210: The test uses a lenient assertion on duplicate
filtering: replace the non-specific
expect(same_song_added).toBeLessThanOrEqual(1) with exact expected counts for
each scenario so failures reveal dropped candidates; for the scenario computing
same_song_added from addedTracks (variable name same_song_added, array
addedTracks) assert exactly 1 when a non-current-track duplicate should be kept,
and assert exactly 0 in the scenario where the duplicate is the current track.
Apply the same tightening to the other assertions referenced around the block
for lines 2253–2258 to use precise counts instead of <=1.
🪄 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: 55a10b1d-092b-4086-b208-8987b90abe07
📒 Files selected for processing (4)
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/searchQueryCleaner.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: Quality Gates
- GitHub Check: SonarCloud Scan
🔇 Additional comments (2)
packages/bot/src/utils/music/searchQueryCleaner.spec.ts (1)
239-263: Good coverage for the new hyphenated suffix handling.These cases validate the newly added remaster/year/original variant normalization paths and help lock dedup behavior.
packages/bot/src/utils/music/queueManipulation.ts (1)
752-767: Selection-level title dedup is a solid guardrail.This is a good second-layer filter to prevent version variants from being selected together.
- use replaceAll() instead of replace() with global regex - split HYPHENATED_VERSION_SUFFIX into array of simple regexes to reduce complexity from 36 to ≤20 per pattern
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/bot/src/utils/music/searchQueryCleaner.ts (1)
87-92:⚠️ Potential issue | 🟡 MinorConstrain year matching to plausible ranges to avoid over-cleaning.
Line 87 and Line 92 currently accept any 4 digits, so numeric non-year suffixes can be stripped as if they were versions.
Proposed fix
const HYPHENATED_VERSION_SUFFIXES: RegExp[] = [ - /^(?:\d{4} +)?remaster(?:ed)?(?:\s+\d{4})?$/i, + /^(?:(?:19|20)\d{2} +)?remaster(?:ed)?(?:\s+(?:19|20)\d{2})?$/i, /^official (?:audio|video|music video)$/i, /^(?:live|acoustic|demo|extended)$/i, /^(?:radio edit|album version|single version)$/i, /^(?:original mix|original version)$/i, - /^\d{4}$/, + /^(?:19|20)\d{2}$/, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/searchQueryCleaner.ts` around lines 87 - 92, The year-matching regexes are too permissive (they use any four digits) and are stripping non-year numeric suffixes; update both the standalone /^\d{4}$/ matcher and the optional year groups inside the remaster pattern to only accept plausible year ranges (e.g., restrict to 1900–2099 by matching 19xx or 20xx) instead of \d{4}; make the change in the array of cleanup regexes in searchQueryCleaner.ts (the entries matching a standalone 4-digit token and the optional year capture in the remaster pattern), and run/update unit tests that cover title cleaning to ensure non-year numeric tokens are no longer removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/bot/src/utils/music/searchQueryCleaner.ts`:
- Around line 87-92: The year-matching regexes are too permissive (they use any
four digits) and are stripping non-year numeric suffixes; update both the
standalone /^\d{4}$/ matcher and the optional year groups inside the remaster
pattern to only accept plausible year ranges (e.g., restrict to 1900–2099 by
matching 19xx or 20xx) instead of \d{4}; make the change in the array of cleanup
regexes in searchQueryCleaner.ts (the entries matching a standalone 4-digit
token and the optional year capture in the remaster pattern), and run/update
unit tests that cover title cleaning to ensure non-year numeric tokens are no
longer removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef514144-ce2c-46ff-ab2c-3c278e1c5553
📒 Files selected for processing (2)
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/searchQueryCleaner.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/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
🔇 Additional comments (2)
packages/bot/src/utils/music/searchQueryCleaner.ts (2)
95-97: Nice extraction of suffix classification logic.
isVersionSuffixkeepscleanTitlesimpler and makes suffix behavior easier to evolve.
128-128: Good defensive integration incleanTitle.Line 128 correctly gates truncation on recognized version suffixes only.
- use normalizedKey !== '::' instead of length check to avoid bypassing dedup for short titles - constrain year-only suffix to (19|20)xx range to avoid over-cleaning non-year 4-digit sequences
|
* fix(bot): handle year-suffix remaster and original mix in title cleaner * fix(bot): key autoplay candidates by normalized title, not url * test(bot): add deduplication tests for same-song candidate selection * fix(bot): fix sonarcloud maintainability issues in dedup code - use replaceAll() instead of replace() with global regex - split HYPHENATED_VERSION_SUFFIX into array of simple regexes to reduce complexity from 36 to ≤20 per pattern * fix(bot): address coderabbit review issues in dedup logic - use normalizedKey !== '::' instead of length check to avoid bypassing dedup for short titles - constrain year-only suffix to (19|20)xx range to avoid over-cleaning non-year 4-digit sequences
This branch was successfully deployed
1 participant
Summary
Root cause
upsertScoredCandidateusedtrack.urlas map key. Two YouTube videos of the same song had different URLs → both entered candidates → both could be selected in the same replenish.Test plan
cleanTitle("Song - Remastered 2011")→"Song"cleanTitle("Song - 2024")→"Song"cleanTitle("Song - Original Mix")→"Song"🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests