Repository navigation
refactor(bot): break Cluster C autoplay cycles (#871 PR 3) - #888
Merged
LucasSantana-Dev merged 17 commits intoMay 16, 2026
Merged
Conversation
Extract utility functions to neutral scoringUtils module to break queueManipulation hub cycles.
Update candidateCollector to import normalizeTrackKey from scoringUtils instead of queueManipulation.
Update candidateScorer to import normalizeTrackKey and normalizeText from scoringUtils.
Update spotifyRecommender to import utilities from scoringUtils and candidateCollector.
Update diversitySelector to import extractYouTubeVideoId from scoringUtils instead of local duplicate.
Update lastFmSeeder to import normalizeTrackKey from scoringUtils instead of queueManipulation.
Move normalizeText and stripFeaturing definitions before normalizeTrackKey to ensure proper function ordering.
…y from scoringUtils The refactored code now imports normalizeTrackKey from ./scoringUtils instead of ../queueManipulation. Update the jest mock to target the correct module.
… scoringUtils The refactored code now imports normalizeTrackKey from ./scoringUtils instead of ../queueManipulation. Update the jest mock to target the correct module.
candidateScorer.ts now imports normalizeText and normalizeTrackKey from ./scoringUtils. Add jest mocks for these functions to allow tests to run.
…malizeText mock - Change calculateRecommendationScore mock from ../queueManipulation to ./candidateScorer - Add normalizeText mock to scoringUtils mock The code now imports calculateRecommendationScore from candidateScorer, which itself uses normalizeText from scoringUtils.
The mock normalizeText function now properly implements NFKC normalization, lowercase conversion, and alphanumeric filtering to match the actual scoringUtils.normalizeText behavior.
The original tests didn't mock normalizeText since it was imported from queueManipulation as a real function. After refactoring, normalizeText is imported from scoringUtils and should also use the real implementation. Remove the overly-specific mocks that were causing test failures.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Failed to generate code suggestions for PR |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThe PR refactors the music autoplay scoring pipeline by extracting shared utilities into a new ChangesAutoplay Pipeline Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
✨ 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 |
|
LucasSantana-Dev
added a commit
that referenced
this pull request
May 21, 2026
## Summary Cut v2.13.0 of Lucky. Bumps root + 4 workspaces from `2.11.0` → `2.13.0` (skipping the archived `2.12.0`) and promotes the CHANGELOG `[Unreleased]` block to `[2.13.0] - 2026-05-21`. ## Headline changes since v2.11.0 **Added** - Guild Automation Module Executor seam + AutoMessages pilot (#901) - Sentry React SDK + Router v7 tracing/replay on frontend (#876) - Prometheus `/metrics` on backend (#875) + bot (#873) - Guild join/leave history tracking (#872) - Trivy image-scan on docker-publish, Phase A audit-only (#883) - Self-hosted developer-tooling register on landing page (#868) **Changed** - Backend migrated to Zod 4 API (#919) — unblocked the CVE patch + ended the lockfile fragility loop - 3 bot circular-deps clusters broken (#885, #886, #888) **Fixed** - brace-expansion DoS + ws uninit-memory CVEs patched (#921) - nginx-alpine CVEs (#881) - CI postinstall rate limit + madge actionlint (#878, #905) Full list in CHANGELOG.md. ## Next steps (after this PR merges) 1. Open `release/v2.13.0 → main` PR with merge-commit method 2. Tag `v2.13.0` on the merge commit 3. Cut next `release` (homelab-style bare branch) — Lucky's bare-release migration is still pending the user removing protection on `release/v2.11.0`
LucasSantana-Dev
added a commit
that referenced
this pull request
May 21, 2026
## Release v2.13.0 Promotes \`release/v2.13.0\` to \`main\` for the v2.13.0 cut. **$AHEAD commits across all merged PRs since v2.11.0 ship.** (Skipping v2.12.0 — the branch existed but its work was rolled forward into v2.13.0 alongside this session's Zod migration + CVE patches + standards adoption.) ## Headline changes **Added** — Guild Automation Module Executor pilot (#901), Sentry frontend (#876), Prometheus metrics on bot+backend (#873, #875), guild membership history (#872), Trivy image-scan Phase A (#883), landing redesign (#868). **Changed** — Backend migrated to Zod 4 API (#919), 3 bot circular-deps clusters broken (#885/#886/#888). **Fixed** — brace-expansion + ws moderate CVEs (#921), nginx-alpine CVEs (#881), CI postinstall rate limit (#878), madge actionlint (#905). **Internal** — shared coverageThreshold gate (#909/#914), Feature-removal sweep checklist + dangerfile guard (#908/#913), monitoring network, AI-doc policy, 4 new ADRs. Full list in [CHANGELOG.md](./CHANGELOG.md). ## Merge method This PR should land via **merge commit** (NOT squash) to preserve the individual PR SHAs in main's history. After merge: 1. Tag \`v2.13.0\` on the merge commit 2. Create GitHub release with notes from CHANGELOG.md 3. Fast-forward \`release/v2.13.0\` to match the new main HEAD ## Test plan - [ ] All 30 checks green except infra (snyk plan cap) - [ ] Verify \`gh pr view 922 --json mergeCommit\` shows the chore-bump commit on release tip - [ ] After merge: confirm \`origin/main\` contains the full $AHEAD commits
This was referenced May 21, 2026
This branch was successfully deployed
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
PR 3 of #871 (bot circular-deps refactor). 10 → 4 cycles in
packages/bot/src(60% reduction, from the post-PR-2 baseline).Implemented via
/three-man-team(Architect: Opus / Builder: Sonnet / Reviewer: Sonnet):normalizeText,stripFeaturing,normalizeTrackKey,extractSpotifyTrackId,extractYouTubeVideoId) into the new dependency-freepackages/bot/src/utils/music/autoplay/scoringUtils.ts. Five autoplay siblings (candidateCollector,candidateScorer,spotifyRecommender,diversitySelector,lastFmSeeder) now import from there instead of round-tripping throughqueueManipulation. Backward-compatible re-export retained onqueueManipulationfor unrelated consumers.replenisher ↔ trackHandlerscycle via dependency injection. NewSkipStateProvidertype inautoplay/skipStateProvider.ts.replenishQueuetakes an optionalskipStateProvidercallback (backwards-compatible — defaults to no-op).trackHandlersinjectsgetRecentSkipCountat the call sites.lastFmSeedsto import directly fromlastFmApi.tsinstead of throughlastfm/index.ts, breaking the cycle at its source. Addedautoplay/lastfmExports.tsbarrel.autoplayAudit ↔ candidateCollectorcycle broken by reordering theScoredTracktype import.Validation
npx madge --circular --extensions ts packages/bot/src: 4 cycles (down from 10)npx tsc --noEmit: clean (modulo pre-existingprom-clientmodule-resolution error, unrelated)scoringUtils|replenisher|trackHandlers|candidateCollector|candidateScorer|diversitySelector|lastFm|autoplayAudit): 2846/2846 passingRemaining cycles (follow-up work)
types/CustomClient ↔ models/Command ↔ types/CommandData: type-only, erased by tsc, intentionally deferred (documented in ADR + plan).queueManipulation → candidateCollector → autoplayAudit → diversitySelector → queueManipulation: newly-visible after Phase 1; back-edge isdiversitySelector → queueManipulationformarkAsAutoplayTrack. Needs another extraction.candidateCollector ↔ spotifyRecommender: residual function-level mutual dep.queueManipulation ↔ replenisher: residual;replenisherstill importsenrichWithAudioFeatures/getTrackAudioFeatures/interleaveByArtist/buildVcContributionWeightsfromqueueManipulation.A follow-up issue will be filed for cycles 2–4.
Refs
docs/decisions/2026-05-16-next-refactor-target-bot-circular-deps.md.claude/plans/2026-05-16-bot-circular-deps-pr3.mdTest plan
Summary by CodeRabbit