Repository navigation
fix(bot): relax SoundCloud match threshold — fixes NoResultError on Brazilian funk tracks - #532
Conversation
Fixes LUCKY-26 / LUCKY-2P: NoResultError on Brazilian funk tracks with
DJ credits in parentheses (e.g. 'DogDog' vs 'Dog Dog' title mismatch).
- findMatchingSoundCloudResult: 100% token match → 75% threshold
Tolerates accent stripping differences ('Jacaré'→'jacar' vs 'jacare')
and compound name variations ('DogDog' vs 'Dog Dog').
- Duration tolerance: 15s → 30s to handle version length differences.
- createResilientStream: adds 3rd fallback stage that strips all
parenthetical content from the title (e.g. '(DJ Japa NK e DJ Davi DogDog)')
before retrying SoundCloud when title-only search also fails.
- Tests: updated '15s' → '30s', added 75% threshold + core-title cases.
|
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 8 minutes and 32 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 (1)
📝 WalkthroughWalkthroughThe pull request relaxes SoundCloud stream matching criteria and extends the fallback chain in Changes
Sequence DiagramsequenceDiagram
actor Client
participant createResilientStream
participant yt-dlp
participant SoundCloud as SoundCloud<br/>(Search)
participant Stream as Stream<br/>(Playback)
Client->>createResilientStream: Request stream for track
createResilientStream->>yt-dlp: Attempt direct extraction
yt-dlp-->>createResilientStream: Fails
createResilientStream->>SoundCloud: Search by full title
SoundCloud-->>createResilientStream: No 75% token match<br/>or duration mismatch
createResilientStream->>createResilientStream: Strip parentheticals<br/>from title
createResilientStream->>SoundCloud: Search by core title
SoundCloud-->>createResilientStream: Returns matching result<br/>(±30s tolerance)
createResilientStream->>Stream: Initialize with<br/>matched URL
Stream-->>Client: Playback starts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/bot/src/handlers/player/playerFactory.bridge.spec.ts (2)
158-171: Narrow this case to the 75% threshold.This input changes accents, punctuation, and compound splitting at the same time, so it can pass without proving the specific “missing 1 compound token” behavior. Keep this case to a single missing/split token and add a separate accent-normalization case.
As per coding guidelines "Test behavior, not implementation details. Prefer unit tests for core logic; add integration tests at meaningful boundaries."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/player/playerFactory.bridge.spec.ts` around lines 158 - 171, The test for findMatchingSoundCloudResult is too broad—change it so the query only differs by a single missing/split compound token (e.g., split or remove only "Jacara" vs "Jacaré" or split a compound like "MC Ryan SP MC Jacara" into two tokens) so it specifically exercises the "missing 1 compound token (75% threshold)" scenario, and create a separate test that isolates accent/diacritic normalization (e.g., change only accents and punctuation like "Até"→"Ate") to verify accent-handling independently; ensure both tests still assert the expected URL ('sc://funk') and keep references to findMatchingSoundCloudResult and the existing result object structure.
373-393: Assert the stripped query on the third retry.
toHaveBeenCalledTimes(3)proves the extra stage ran, but not that it searched with the parenthetical-stripped title. Please assert the thirdplaydl.searchcall receivedBohemian Rhapsodyso this fallback cannot regress silently.As per coding guidelines "Test behavior, not implementation details. Prefer unit tests for core logic; add integration tests at meaningful boundaries."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/player/playerFactory.bridge.spec.ts` around lines 373 - 393, Add an assertion that the third playdl.search attempt used the parenthetical-stripped title: after calling createResilientStream with makeTrack({ title: 'Bohemian Rhapsody (Official Music Live Session)' }), assert that playdlSearchMock's third invocation received 'Bohemian Rhapsody' as the query (e.g., inspect playdlSearchMock.mock.calls[2][0] or use toHaveBeenNthCalledWith for the 3rd call) so the fallback behavior in createResilientStream is explicitly tested.
🤖 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/handlers/player/playerFactory.ts`:
- Around line 355-360: In normalizeForMatch, stop stripping accented letters and
instead apply Unicode NFD normalization and remove only combining marks: call
value.normalize('NFD'), then remove diacritics with .replace(/\p{M}/gu, ''),
then lowercase and strip any non-letter/number/space characters with a
Unicode-aware regex like .replace(/[^\p{L}\p{N} ]+/gu, ''), collapse multiple
spaces and trim; this preserves base letters (e.g., "Jacaré" → "jacare") while
still removing punctuation and extra spaces.
---
Nitpick comments:
In `@packages/bot/src/handlers/player/playerFactory.bridge.spec.ts`:
- Around line 158-171: The test for findMatchingSoundCloudResult is too
broad—change it so the query only differs by a single missing/split compound
token (e.g., split or remove only "Jacara" vs "Jacaré" or split a compound like
"MC Ryan SP MC Jacara" into two tokens) so it specifically exercises the
"missing 1 compound token (75% threshold)" scenario, and create a separate test
that isolates accent/diacritic normalization (e.g., change only accents and
punctuation like "Até"→"Ate") to verify accent-handling independently; ensure
both tests still assert the expected URL ('sc://funk') and keep references to
findMatchingSoundCloudResult and the existing result object structure.
- Around line 373-393: Add an assertion that the third playdl.search attempt
used the parenthetical-stripped title: after calling createResilientStream with
makeTrack({ title: 'Bohemian Rhapsody (Official Music Live Session)' }), assert
that playdlSearchMock's third invocation received 'Bohemian Rhapsody' as the
query (e.g., inspect playdlSearchMock.mock.calls[2][0] or use
toHaveBeenNthCalledWith for the 3rd call) so the fallback behavior in
createResilientStream is explicitly tested.
🪄 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: 61b6c0c1-b8d0-4caa-88cc-70083db2c341
📒 Files selected for processing (2)
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.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 context used
📓 Path-based instructions (15)
**/*.{js,jsx,ts,tsx,vue,html}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Provide accessible UI components using semantic HTML and ARIA attributes where necessary
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)
**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier
**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.
**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports
**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validatorsImplement TypeScript typecheck and linter in CI quality checks
**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then internal modules
Use named exports for clear usage and easier refactors
Always validate and sanitize external data (HTTP, DB) at the boundary using type guards ...
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen available for better debugging
Define clear, stable error codes (e.g.,ERR_AUTH_EXPIRED,ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g.,details,retryable,status,correlationId) in error objects
Use domain error classes per area (e.g.,AuthenticationError,ValidationError,NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
MarkretryablevsnonRetryableerrors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)
**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques:React.memo,useMemo,useCallback,Suspense(web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Test behavior, not implementation. Prefer Testing Library utilities for testing React/React Native components.
For React Native tests: mock native modules and test component interactions and accessibility labels.
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever useany- type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid usinganytype; if unavoidable, useunknownwith type guards and justify with code comment
Preferinterfacefor public API shapes; usetypefor unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Test behavior, not implementation details
Prefer unit tests for core logic; add integration tests at meaningful boundaries
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Use Jest + a React testing library for unit and component tests as applicable
Test behavior, not implementation details
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)
**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns
**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env,.env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy comments
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
packages/bot/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
packages/bot/**/*.{ts,tsx}: UseuseMainPlayer()fromdiscord-playerto access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from@lucky/sharedfor persistent data like track history and session information
UseerrorLoganddebugLogfrom@lucky/shared/utilsfor logging throughout the bot package
Use embed and reply utilities from@lucky/sharedfor consistent message formatting and error sanitization across the bot
Use services from@lucky/shared(DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
packages/bot/src/handlers/player/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
Track handling, errors, and lifecycle must be managed through dedicated handlers in
packages/bot/src/handlers/player/(trackHandlers, errorHandlers, lifecycleHandlers)
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
packages/bot/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
botpackage depends onsharedand contains Discord bot commands and player handlers using Discord.js and Discord Player
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
**/*.{js,mjs,ts,mts}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
**/*.{spec,test}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
**/*.{spec,test}.{ts,tsx,js,jsx}: Use Jest for unit and integration tests
Test behavior, not implementation details
Run unit, integration tests, and coverage report in CI quality checks
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
Unit tests must use naming convention
*.spec.ts
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.ts
packages/bot/src/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Use
@lucky/sharedfor database, Redis, logging, and embed utilities instead of implementing them locally
Files:
packages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.ts
| function normalizeForMatch(value: string): string { | ||
| return value | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9\s]/g, '') | ||
| .replace(/[^a-z0-9 ]/g, '') | ||
| .replace(/ {2,}/g, ' ') | ||
| .trim() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and read the specific section
cat -n packages/bot/src/handlers/player/playerFactory.ts | sed -n '350,365p'Repository: LucasSantana-Dev/Lucky
Length of output: 650
🏁 Script executed:
# Also check the broader context of the normalizeForMatch function
cat -n packages/bot/src/handlers/player/playerFactory.ts | sed -n '340,370p'Repository: LucasSantana-Dev/Lucky
Length of output: 1348
🏁 Script executed:
# Let's see how normalizeForMatch is used in the codebase
rg -n "normalizeForMatch" packages/bot/src/handlers/player/playerFactory.ts -B 2 -A 2Repository: LucasSantana-Dev/Lucky
Length of output: 535
🏁 Script executed:
# Check the entire playerFactory.ts file size to understand scope
wc -l packages/bot/src/handlers/player/playerFactory.tsRepository: LucasSantana-Dev/Lucky
Length of output: 119
🏁 Script executed:
# Check tsconfig.json for target
cat tsconfig.json 2>/dev/null || echo "tsconfig.json not found at root"Repository: LucasSantana-Dev/Lucky
Length of output: 1132
🏁 Script executed:
# Check for TypeScript config in bot package
find packages/bot -name "tsconfig.json" -o -name "tsconfig.*.json" | head -5Repository: LucasSantana-Dev/Lucky
Length of output: 92
🏁 Script executed:
# Check package.json for Node.js version requirement and TypeScript target
cat package.json | grep -A 5 -B 5 "engines\|target" || echo "Checking for engines field..."Repository: LucasSantana-Dev/Lucky
Length of output: 295
🏁 Script executed:
# Check if there's a root tsconfig
cat -n tsconfig.json 2>/dev/null | head -20Repository: LucasSantana-Dev/Lucky
Length of output: 796
Diacritics are being deleted, not normalized.
The current implementation removes accented characters entirely (Jacaré → jacar, Jacare → jacare), leaving accent-only variants unmatched despite the PR's stated root cause being accent normalization.
Proposed fix
function normalizeForMatch(value: string): string {
return value
+ .normalize('NFD')
+ .replace(/\p{M}+/gu, '')
.toLowerCase()
.replace(/[^a-z0-9 ]/g, '')
.replace(/ {2,}/g, ' ')
.trim()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function normalizeForMatch(value: string): string { | |
| return value | |
| .toLowerCase() | |
| .replace(/[^a-z0-9\s]/g, '') | |
| .replace(/[^a-z0-9 ]/g, '') | |
| .replace(/ {2,}/g, ' ') | |
| .trim() | |
| function normalizeForMatch(value: string): string { | |
| return value | |
| .normalize('NFD') | |
| .replace(/\p{M}+/gu, '') | |
| .toLowerCase() | |
| .replace(/[^a-z0-9 ]/g, '') | |
| .replace(/ {2,}/g, ' ') | |
| .trim() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/player/playerFactory.ts` around lines 355 - 360, In
normalizeForMatch, stop stripping accented letters and instead apply Unicode NFD
normalization and remove only combining marks: call value.normalize('NFD'), then
remove diacritics with .replace(/\p{M}/gu, ''), then lowercase and strip any
non-letter/number/space characters with a Unicode-aware regex like
.replace(/[^\p{L}\p{N} ]+/gu, ''), collapse multiple spaces and trim; this
preserves base letters (e.g., "Jacaré" → "jacare") while still removing
punctuation and extra spaces.
|
…allback (#532) * fix(bot): relax SoundCloud match threshold and add core-title fallback Fixes LUCKY-26 / LUCKY-2P: NoResultError on Brazilian funk tracks with DJ credits in parentheses (e.g. 'DogDog' vs 'Dog Dog' title mismatch). - findMatchingSoundCloudResult: 100% token match → 75% threshold Tolerates accent stripping differences ('Jacaré'→'jacar' vs 'jacare') and compound name variations ('DogDog' vs 'Dog Dog'). - Duration tolerance: 15s → 30s to handle version length differences. - createResilientStream: adds 3rd fallback stage that strips all parenthetical content from the title (e.g. '(DJ Japa NK e DJ Davi DogDog)') before retrying SoundCloud when title-only search also fails. - Tests: updated '15s' → '30s', added 75% threshold + core-title cases. * fix: replace greedy .*? with [^)]* to avoid ReDoS hotspot * fix: replace \s with literal space in normalizeForMatch to resolve SonarCloud hotspot * fix: replace remaining \s usages with literal spaces to clear SonarCloud hotspots * fix: remove quantifiers from coreTitle paren regex to eliminate ReDoS hotspot (S5852) * fix: replace paren-strip regex with indexOf to eliminate S5852 ReDoS hotspot



Root Cause
Fixes LUCKY-26 / LUCKY-2P (both unresolved in Sentry, last seen ~3h ago).
Track:
MC Ryan SP, MC Jacaré e MC Meno K - Posso Até Não Te Dar Flores (DJ Japa NK e DJ Davi DogDog)The bridge
findMatchingSoundCloudResultrequired 100% of query tokens to appear in the SoundCloud result name. Two failure modes:DogDog→ token"dogdog"not found when SoundCloud stores it as"Dog Dog"→ space makes it"dog dog"which doesn't contain the substring"dogdog"Jacaré→"jacar"(é stripped),Até→"at"(é stripped) — minor differences between how our code and SoundCloud normalize accentsChanges
findMatchingSoundCloudResult: changed fromtokens.every(100%) to 75% match threshold — tolerates 1–2 token mismatches in long titlescreateResilientStream: added 3rd fallback stage that strips all parenthetical content ((DJ Japa NK e DJ Davi DogDog)) before retrying SoundCloud searchTest plan
Summary by CodeRabbit