Repository navigation
refactor(bot): break trivial circular deps (Cluster A partial + D) - #885
Merged
LucasSantana-Dev merged 1 commit intoMay 16, 2026
Merged
Conversation
PR 1 of 4 from the /refactor-pipeline plan for #871 (see docs/decisions/2026-05-16-next-refactor-target-bot-circular-deps.md). Madge baseline: 15 cycles. After this PR: 12 cycles. Fixes: Cluster D — dead self-cycle barrels (cycles 14, 15): - packages/bot/src/functions/download/utils/downloadVideo.ts - packages/bot/src/functions/download/utils/ytDlpUtils/downloader.ts Both were one-liners: `export type * from './<self>'`. With both `./<name>.ts` AND `./<name>/` present, TypeScript resolved to the file, producing an infinite self-import. Zero external callers; pure dead code. Deleted. Cluster A (partial) — extract CustomClient from the types barrel: - New file packages/bot/src/types/CustomClient.ts holds the `CustomClient` + `CommandType` declarations (previously inline in types/index.ts). - types/index.ts becomes a pure re-export barrel (no declarations). - types/CommandData.ts and types/QueueMetadata.ts import `CustomClient` from `./CustomClient` directly, breaking the old back-edge through `./index`. Fixes cycle 2 (`types/index.ts → QueueMetadata → types/index.ts`) and the runtime barrel cycle. Cycle 1 (the type-only `CustomClient → models/Command → types/CommandData` path) persists but is `import type` only — erased by tsc, no runtime effect. Deferred to a follow-up after Cluster C lands because the structural-type break cascades into `help.ts` and similar `Collection<string, Command>` consumers that expect the real class; cleaner to revisit alongside the command-loader contracts. No caller changes required — all existing `import { CustomClient } from '.../types'` paths still resolve through the barrel re-export. Validation: - npx madge --circular packages/bot/src → 12 (was 15) - npx tsc --noEmit clean (no new errors) Next: PR 2 will tackle Cluster B (monitoring telemetry cycle).
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ 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 |
![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 |
|
3 of 4 tasks
LucasSantana-Dev
added a commit
that referenced
this pull request
May 16, 2026
## Summary PR 2 of 4 for #871. Breaks the two `SimplifiedTelemetry ↔ healthChecks` and `SimplifiedTelemetry ↔ telemetryMetrics` runtime cycles (Cluster B in the plan). - Extracts singletons + classes into `packages/bot/src/utils/monitoring/clients.ts` - Extracts shared interfaces into `packages/bot/src/utils/monitoring/telemetryTypes.ts` - `SimplifiedTelemetry.ts` keeps its public re-export surface for backwards compatibility with `SimplifiedTelemetryWrapper`, `telemetry`, `health`, `metrics` ## Validation - madge cycles: **12 → 10** (Cluster B done) - `npx tsc --noEmit`: clean (one pre-existing `prom-client` error, unrelated) - Jest monitoring/telemetry/health: 39 passed ## Refs - Issue: #871 - ADR: `docs/decisions/2026-05-16-next-refactor-target-bot-circular-deps.md` - Plan: `.claude/plans/2026-05-16-bot-circular-deps.md` (Phase 2) - Stacked after: #885 (Cluster A + D) - Next: PR 3 (Cluster C, autoplay ↔ queue, three-man-team) ## Test plan - [x] madge cycle count drops by 2 - [x] tsc clean (modulo pre-existing prom-client) - [x] monitoring/telemetry/health tests pass - [ ] CI green
3 tasks
LucasSantana-Dev
added a commit
that referenced
this pull request
May 16, 2026
## Summary PR 4 of 4 for #871. Adds an audit-only `madge --circular` workflow for `packages/bot/src` that runs on every PR + push targeting `release/**` and `main`. - Reports cycle count + full report to the GitHub Actions job summary - Uploads the raw report as a 14-day artifact - `continue-on-error: true` keeps it from blocking merges while Cluster C is still open (current baseline: 10 cycles) ## Promotion path When #871 PR 3 (Cluster C autoplay/queue) lands and the count reaches the agreed baseline (0 for runtime cycles, 1 for the persistent type-only `types/CustomClient` cycle), follow up to: 1. Remove `continue-on-error: true` 2. Add `if [ \"\$COUNT\" -gt 1 ]; then exit 1; fi` after the count export 3. Make it a required check in branch protection ## Refs - Issue: #871 - ADR: `docs/decisions/2026-05-16-next-refactor-target-bot-circular-deps.md` - Stacked after: #885, #886 - Predecessor of: PR 3 (Cluster C, three-man-team) ## Test plan - [ ] CI green - [ ] Job summary on this PR shows cycle count (expect 10) - [ ] Artifact uploaded
3 of 4 tasks
LucasSantana-Dev
added a commit
that referenced
this pull request
May 16, 2026
## 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): - **Phase 1**: Extracted 5 pure utility functions (`normalizeText`, `stripFeaturing`, `normalizeTrackKey`, `extractSpotifyTrackId`, `extractYouTubeVideoId`) into the new dependency-free `packages/bot/src/utils/music/autoplay/scoringUtils.ts`. Five autoplay siblings (`candidateCollector`, `candidateScorer`, `spotifyRecommender`, `diversitySelector`, `lastFmSeeder`) now import from there instead of round-tripping through `queueManipulation`. Backward-compatible re-export retained on `queueManipulation` for unrelated consumers. - **Phase 2**: Inverted the `replenisher ↔ trackHandlers` cycle via dependency injection. New `SkipStateProvider` type in `autoplay/skipStateProvider.ts`. `replenishQueue` takes an optional `skipStateProvider` callback (backwards-compatible — defaults to no-op). `trackHandlers` injects `getRecentSkipCount` at the call sites. - **Phase 3**: Lastfm export hygiene — moved `lastFmSeeds` to import directly from `lastFmApi.ts` instead of through `lastfm/index.ts`, breaking the cycle at its source. Added `autoplay/lastfmExports.ts` barrel. - **Bonus**: `autoplayAudit ↔ candidateCollector` cycle broken by reordering the `ScoredTrack` type import. ## Validation - `npx madge --circular --extensions ts packages/bot/src`: **4 cycles** (down from 10) - `npx tsc --noEmit`: clean (modulo pre-existing `prom-client` module-resolution error, unrelated) - Jest in affected dirs (`scoringUtils|replenisher|trackHandlers|candidateCollector|candidateScorer|diversitySelector|lastFm|autoplayAudit`): **2846/2846 passing** ## Remaining cycles (follow-up work) 1. **Cycle 1** — `types/CustomClient ↔ models/Command ↔ types/CommandData`: type-only, erased by tsc, **intentionally deferred** (documented in ADR + plan). 2. **Cycle 2** — `queueManipulation → candidateCollector → autoplayAudit → diversitySelector → queueManipulation`: newly-visible after Phase 1; back-edge is `diversitySelector → queueManipulation` for `markAsAutoplayTrack`. Needs another extraction. 3. **Cycle 3** — `candidateCollector ↔ spotifyRecommender`: residual function-level mutual dep. 4. **Cycle 4** — `queueManipulation ↔ replenisher`: residual; `replenisher` still imports `enrichWithAudioFeatures`/`getTrackAudioFeatures`/`interleaveByArtist`/`buildVcContributionWeights` from `queueManipulation`. A follow-up issue will be filed for cycles 2–4. ## Refs - Issue: #871 - ADR: `docs/decisions/2026-05-16-next-refactor-target-bot-circular-deps.md` - Plan: `.claude/plans/2026-05-16-bot-circular-deps-pr3.md` - Stacked after: #885, #886, #887 ## Test plan - [x] madge cycle count drops 10 → 4 - [x] tsc clean - [x] Jest green in affected suites - [ ] CI green
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 1 of 4 from the `/refactor-pipeline` plan for #871 (ADR).
Madge baseline: 15 cycles. After this PR: 12 cycles.
Fixes
Cluster D — dead self-cycle barrels (cycles 14, 15)
Two files were one-liners doing `export type * from './'`. With both `./.ts` AND a `.//` directory present, TypeScript resolved to the file → infinite self-import. Zero external callers (verified by grep). Pure dead code.
Cluster A (partial) — extract CustomClient from types barrel
Fixes cycle 2 (`types/index.ts → QueueMetadata → types/index.ts`).
What's deferred
Original cycle 1 (`types/index.ts → models/Command → types/CommandData → types/index.ts`) is now `types/CustomClient.ts → models/Command → types/CommandData → types/CustomClient.ts` — same shape, same files. `import type` only, erased by tsc, no runtime effect.
A structural-type break (replacing `Collection<string, Command>` with a `CommandLike` shape) was attempted but cascades into `help.ts` and similar consumers that expect the real `Command` class. Cleaner to revisit alongside the command-loader contracts in a later PR (likely bundled with Cluster C's autoplay/queue refactor).
Documented in a code comment on `types/CustomClient.ts` so future readers know it's intentional.
Caller impact
None. All existing `import { CustomClient } from '.../types'` paths still resolve through the barrel re-export.
Validation
Test plan
Next
PR 2 — Cluster B (monitoring telemetry: 2 cycles around `SimplifiedTelemetry`).