Repository navigation
refactor(bot): break monitoring telemetry cycles (#871 PR 2) - #886
Conversation
Extracts singletons + classes from `SimplifiedTelemetry` into `clients.ts` and shared interfaces into `telemetryTypes.ts`, so `healthChecks.ts` and `telemetryMetrics.ts` no longer import back through `SimplifiedTelemetry` to grab their dependencies. Madge cycle count: 12 → 10 (Cluster B done; remaining are Cluster C autoplay/queue + the type-only `types/CustomClient` cycle deferred to follow-up PRs). Public re-exports preserved on `SimplifiedTelemetry` for backwards compatibility with `SimplifiedTelemetryWrapper`, `telemetry`, `health`, `metrics`. No behavior changes. Plan: .claude/plans/2026-05-16-bot-circular-deps.md (Phase 2) ADR: docs/decisions/2026-05-16-next-refactor-target-bot-circular-deps.md
|
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 (5)
✨ 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 |
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 |
## 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
|
## 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
## 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`
## 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


Summary
PR 2 of 4 for #871. Breaks the two
SimplifiedTelemetry ↔ healthChecksandSimplifiedTelemetry ↔ telemetryMetricsruntime cycles (Cluster B in the plan).packages/bot/src/utils/monitoring/clients.tspackages/bot/src/utils/monitoring/telemetryTypes.tsSimplifiedTelemetry.tskeeps its public re-export surface for backwards compatibility withSimplifiedTelemetryWrapper,telemetry,health,metricsValidation
npx tsc --noEmit: clean (one pre-existingprom-clienterror, unrelated)Refs
docs/decisions/2026-05-16-next-refactor-target-bot-circular-deps.md.claude/plans/2026-05-16-bot-circular-deps.md(Phase 2)Test plan