Repository navigation
release: v2.14.0 - #936
Merged
Merged
release: v2.14.0#936
Conversation
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.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
github-actions
Bot
added
dependencies
|
Failed to generate code suggestions for PR |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Size Change: +1.05 kB (+0.25%) Total Size: 424 kB 📦 View Changed
ℹ️ View Unchanged
|
|
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 ignored due to path filters (1)
📒 Files selected for processing (53)
📝 WalkthroughWalkthroughThis PR implements Phase A and Phase B of the autoplay recommendation telemetry roadmap alongside Cycle C module refactoring to eliminate circular imports. It extends the Prisma ChangesAutoplay Recommendation Telemetry Implementation
Supporting Refactors and Feature Toggles
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
packages/bot/src/functions/download/commands/download/service.spec.ts (1)
4-6: ⚡ Quick winAlign toggle assertions with
isEnabledForGuild()These tests currently mock/assert
featureToggleService.isEnabled(...), which codifies the old toggle-check path. Please update the mock contract and expectations toFeatureToggleService.isEnabledForGuild(...)so the suite enforces the required runtime behavior.As per coding guidelines, "For feature toggles, check both global and guild-specific toggles using
FeatureToggleService.isEnabledForGuild()rather than checking them separately".Also applies to: 58-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/functions/download/commands/download/service.spec.ts` around lines 4 - 6, The tests currently mock and assert featureToggleService.isEnabled; update the mock contract to use isEnabledForGuild instead: replace featureToggleServiceMock = { isEnabled: jest.fn() } with featureToggleServiceMock = { isEnabledForGuild: jest.fn() } and change all test stubs/implementations that set return values (e.g., mockReturnValue / mockResolvedValue) to target isEnabledForGuild; also update every expectation/verification to expect(featureToggleServiceMock.isEnabledForGuild).toHaveBeenCalledWith(...) (and any specific args) so the suite verifies FeatureToggleService.isEnabledForGuild is used (apply same change to the other tests currently asserting isEnabled).packages/bot/src/services/musicRecommendation/recommendationTelemetry.ts (1)
9-25: 🏗️ Heavy liftUse branded Discord ID types in new telemetry interfaces.
guildIdanddiscordUserIdare introduced as plainstring. This is a new API surface and loses the ID-type safety contract used elsewhere.As per coding guidelines, "Use branded types (e.g.,
GuildId,UserId,ChannelId) for Discord IDs throughout the codebase to prevent type-level ID confusion".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/services/musicRecommendation/recommendationTelemetry.ts` around lines 9 - 25, Change the plain string ID fields in the new telemetry interfaces to the project's branded ID types: update RecordPickInput.guildId and RecordOutcomeArgs.guildId from string to GuildId, and change RecordPickInput.discordUserId from string | undefined to UserId | undefined; also add imports for GuildId and UserId at the top of the file so the interfaces use the central branded ID types instead of raw strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/decisions/2026-05-21-autoplay-recommendation-roadmap.md`:
- Line 156: The ADR currently embeds a contributor-local absolute filesystem
path in the "Memory:" line (the string starting with Memory:
`~/.claude/.../memory/`), which must be removed; update that line in
docs/decisions/2026-05-21-autoplay-recommendation-roadmap.md to either use a
repo-relative reference (e.g., "memory/") or omit the path entirely and list
only the memory identifiers (observation `#3407`, `#3410`, `#3422`, `#3448`, `#3449`),
ensuring the sentence still reads naturally and preserves the reactive-patch
context reference.
In
`@packages/bot/src/functions/download/utils/ytDlpUtils/downloader/service.spec.ts`:
- Around line 1-2: There are two identical imports of beforeEach, describe,
expect, it, and jest from '`@jest/globals`' in the test file; remove the redundant
import line so only a single import statement remains that imports beforeEach,
describe, expect, it, and jest (i.e., keep one import { beforeEach, describe,
expect, it, jest } from '`@jest/globals`' and delete the duplicate).
In `@packages/bot/src/handlers/player/trackHandlers.ts`:
- Around line 310-315: The recommendation outcome calls use only track.id so
tracks without an id won't match pick records; update both
recordRecommendationOutcome calls (the ones inside the
isRecommendationAutoplay/accept path and the reject path) to pass the same
fallback used by pick recording by supplying track.id || track.url as trackId;
locate calls to recordRecommendationOutcome in trackHandlers.ts (near the
isRecommendationAutoplay checks) and replace the single-field track.id with the
fallback expression so accepted and rejected outcomes persist for ID-less
tracks.
In `@packages/bot/src/services/musicRecommendation/recommendationTelemetry.ts`:
- Around line 54-60: Replace the direct errorLog calls in the catch blocks with
the centralized utility logAndSwallow so telemetry DB errors are logged with
consistent context and then suppressed; specifically, in
recordRecommendationPick (the shown catch) and the other telemetry DB catch (the
block referenced around 104-110), call logAndSwallow(err, { message:
'[recordRecommendationPick] failed to insert Recommendation row',
...anyExistingContext }) (and the corresponding message for the other handler)
instead of errorLog, preserving the same contextual fields previously passed to
errorLog.
In `@packages/bot/src/utils/download/downloadHelpers.ts`:
- Around line 43-49: The function createErrorEmbed currently returns unknown
which loses type safety; define a specific interface (e.g., ErrorEmbed or
EmbedPayload) matching the object shape { title: string; description: string;
color: number } and update createErrorEmbed's signature to return that interface
instead of unknown; ensure callers that consume createErrorEmbed rely on the new
type so they no longer need to cast or guard the result.
In `@packages/bot/src/utils/music/autoplay/audioFeatures.ts`:
- Around line 43-46: Do not negative-cache null on missing token or transient
Spotify errors: remove audioFeatureCache.set(cacheKey, { value: null }) from the
early-return when token is falsy and from error paths (including the spot noted
around lines 70-71); only set audioFeatureCache for confirmed feature results
(or, if you must cache misses, set a short TTL and include an explicit reason
metadata). When catching errors in the function that computes/requests features,
use the existing logAndRethrow() or logAndSwallow() utilities to record context
before rethrowing or suppressing, and ensure cache writes happen only after a
successful response is validated (refer to audioFeatureCache, cacheKey, and the
token-check block).
- Around line 50-54: The code is manually parsing Spotify URLs with a regex
which can produce malformed IDs; replace the manual extraction in
audioFeatures.ts (the block that sets spotifyId from track.url) with a call to
the shared extractor extractSpotifyTrackId(track.url) and assign its return to
spotifyId, ensuring you handle null/undefined returns the same way the regex
branch did before; update any downstream uses of spotifyId accordingly so all
Spotify API calls use the normalized ID from extractSpotifyTrackId().
In `@packages/bot/src/utils/music/autoplay/recommendationSourceMapping.ts`:
- Around line 33-35: The switch's default clause in
recommendationSourceMapping.ts currently declares "const _exhaustive: never =
src" directly in the case body; wrap the default body in braces so the
declaration is block-scoped (e.g., default: { const _exhaustive: never = src;
return _exhaustive; }) to satisfy the noSwitchDeclarations lint rule and avoid
TDZ/switch-scope issues.
In `@packages/shared/src/services/recommendationTelemetryReadService.ts`:
- Around line 41-44: Change the exported shared service functions to return
Result-wrapped responses instead of throwing: update the signature of
getPerSourceAcceptance(guildId: string, days?: number) from
Promise<PerSourceRow[]> to Promise<Result<PerSourceRow[]>> and similarly change
the other export referenced around lines 131-134 (the Summary-returning
function) to Promise<Result<Summary>>; inside each function (e.g.,
getPerSourceAcceptance and the Summary function) return { ok: true, value:
<data> } on success and catch errors to return { ok: false, error: <error or
message> } so callers receive explicit Result<T> outcomes rather than thrown
exceptions.
- Around line 42-43: The exported service signatures in
recommendationTelemetryReadService.ts use plain string for the guildId
parameter; change those guildId parameters to the branded GuildId type in every
exported function/method and exported interface in this file (both occurrences
flagged). Import the GuildId branded type from the shared types module and
replace the guildId: string declarations in the public API (the service function
and any exported method signatures) with guildId: GuildId; update callers or add
minimal casts where necessary to satisfy the new type.
---
Nitpick comments:
In `@packages/bot/src/functions/download/commands/download/service.spec.ts`:
- Around line 4-6: The tests currently mock and assert
featureToggleService.isEnabled; update the mock contract to use
isEnabledForGuild instead: replace featureToggleServiceMock = { isEnabled:
jest.fn() } with featureToggleServiceMock = { isEnabledForGuild: jest.fn() } and
change all test stubs/implementations that set return values (e.g.,
mockReturnValue / mockResolvedValue) to target isEnabledForGuild; also update
every expectation/verification to
expect(featureToggleServiceMock.isEnabledForGuild).toHaveBeenCalledWith(...)
(and any specific args) so the suite verifies
FeatureToggleService.isEnabledForGuild is used (apply same change to the other
tests currently asserting isEnabled).
In `@packages/bot/src/services/musicRecommendation/recommendationTelemetry.ts`:
- Around line 9-25: Change the plain string ID fields in the new telemetry
interfaces to the project's branded ID types: update RecordPickInput.guildId and
RecordOutcomeArgs.guildId from string to GuildId, and change
RecordPickInput.discordUserId from string | undefined to UserId | undefined;
also add imports for GuildId and UserId at the top of the file so the interfaces
use the central branded ID types instead of raw strings.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b26d009b-f151-4c8e-9ed0-5325c6f1505f
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!**/CHANGELOG.md
📒 Files selected for processing (53)
docs/decisions/2026-05-21-autoplay-recommendation-roadmap.mddocs/decisions/2026-05-21-cycle-c-direct-imports-and-audio-features-extraction.mdpackage.jsonpackages/backend/package.jsonpackages/backend/src/routes/index.tspackages/backend/src/routes/recommendations.tspackages/backend/tests/integration/recommendations.test.tspackages/bot/package.jsonpackages/bot/src/functions/download/commands/download/processor.spec.tspackages/bot/src/functions/download/commands/download/service.spec.tspackages/bot/src/functions/download/commands/download/validator.spec.tspackages/bot/src/functions/download/utils/deleteContent.spec.tspackages/bot/src/functions/download/utils/ytDlpUtils/downloader/service.spec.tspackages/bot/src/functions/download/utils/ytDlpUtils/pathManager.spec.tspackages/bot/src/functions/management/commands/customcommand.spec.tspackages/bot/src/functions/management/commands/embed.spec.tspackages/bot/src/functions/music/commands/playlist.spec.tspackages/bot/src/functions/music/commands/recommendation/handlers/presetHandler.spec.tspackages/bot/src/functions/music/commands/recommendation/handlers/resetHandler.spec.tspackages/bot/src/functions/music/commands/recommendation/handlers/settingsHandler.spec.tspackages/bot/src/functions/music/commands/recommendation/handlers/updateHandler.spec.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/services/musicRecommendation/recommendationTelemetry.spec.tspackages/bot/src/services/musicRecommendation/recommendationTelemetry.tspackages/bot/src/utils/download/downloadHelpers.spec.tspackages/bot/src/utils/download/downloadHelpers.tspackages/bot/src/utils/music/autoplay/audioFeatures.tspackages/bot/src/utils/music/autoplay/candidateCollector.tspackages/bot/src/utils/music/autoplay/candidateContracts.tspackages/bot/src/utils/music/autoplay/diversitySelector.spec.tspackages/bot/src/utils/music/autoplay/diversitySelector.tspackages/bot/src/utils/music/autoplay/queueMarkers.spec.tspackages/bot/src/utils/music/autoplay/queueMarkers.tspackages/bot/src/utils/music/autoplay/recommendationSourceMapping.spec.tspackages/bot/src/utils/music/autoplay/recommendationSourceMapping.tspackages/bot/src/utils/music/autoplay/replenisher.tspackages/bot/src/utils/music/autoplay/spotifyRecommender.tspackages/bot/src/utils/music/autoplay/vcWeights.tspackages/bot/src/utils/music/collaborativePlaylist.spec.tspackages/bot/src/utils/music/queueEditOps.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/queueRescue.tspackages/frontend/package.jsonpackages/shared/package.jsonpackages/shared/src/config/featureToggles.tspackages/shared/src/services/index.tspackages/shared/src/services/recommendationTelemetryReadService.spec.tspackages/shared/src/services/recommendationTelemetryReadService.tspackages/shared/src/types/featureToggle.tspackages/shared/src/types/index.tsprisma/migrations/20260521000000_recommendation_telemetry_phase_a/migration.sqlprisma/schema.prisma
…925) ## Summary Partial fix for issue #889 — closes Cycles A and B of the 3 remaining bot autoplay/queue cycles. Cycle C (`queueManipulation ↔ replenisher`) deferred as follow-up. **Madge cycle count:** - Before: 3 runtime cycles + 1 type-only (4 total) - After: 1 runtime cycle (`queueManipulation ↔ replenisher`) + 1 type-only (2 total) **Cycle A — autoplayAudit hub** (CLOSED) - Extracted `markAsAutoplayTrack` (and helpers) into `packages/bot/src/utils/music/autoplay/queueMarkers.ts` (~49 LOC). - `queueEditOps.ts` re-exports the symbols for non-breaking caller migration. - `diversitySelector.ts` imports directly from `queueMarkers.ts` (back-edge removed). **Cycle B — candidateCollector ↔ spotifyRecommender** (CLOSED) - Extracted shared types + helpers into `packages/bot/src/utils/music/autoplay/candidateContracts.ts` (~97 LOC). - Both modules now import from `candidateContracts.ts` instead of each other. **Cycle C — replenisher utility back-imports** (DEFERRED) - Agent attempt to extract `enrichWithAudioFeatures` + `getTrackAudioFeatures` + `buildVcContributionWeights` + `interleaveByArtist` (proposed `audioFeatures.ts` + `vcWeights.ts`) broke 107 bot tests due to closure-captured state in `queueManipulation`. Rolled back the broken extraction. - Recommended follow-up: more careful extraction with proper handling of shared internal helpers, or accept the cycle and document it in an ADR per the issue's "two options" framing. ## Verified - ✓ `npx madge --circular packages/bot/src --extensions ts,tsx`: 2 cycles (1 deferred type-only + 1 Cycle C residual) - ✓ `npm test --workspace=packages/bot`: 2878 / 2878 tests pass ## Closes Partial #889 (Cycles A + B). Cycle C remains open as the issue's residual.
## Summary Added comprehensive unit test coverage for 4 untested recommendation subcommand handlers with 18 tests total, addressing #826. ## Tests Added - **presetHandler.spec.ts** (5 tests): Valid preset application (balanced, conservative), unknown preset rejection, no-guild error, service error - **resetHandler.spec.ts** (4 tests): Successful reset with confirmation, cancellation without confirmation, no-guild error, service error - **settingsHandler.spec.ts** (4 tests): Settings display, default fallback, no-guild error, autoplay stats error handling - **updateHandler.spec.ts** (5 tests): Single-field and multi-field updates, empty-update rejection, no-guild error, service error ## Test Coverage - Test count: 2878 → 2896 tests (+18) - All tests pass (180 suites) - Coverage remains above 65% threshold per repo policy - Each handler tests thin-wrapper logic: option parsing, service calls, embed generation, error handling ## Notes - Handlers carry mock recommendation service implementations; specs focus on handler-level behavior - Service layer tests (recommendationEngine.spec.ts, feedbackService.spec.ts) remain separate and unmodified - No changes to production code Closes #826 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added comprehensive test suites for music recommendation command handlers. * Tests cover preset application, settings reset, settings display, and settings updates with success and error scenarios. * Test coverage includes guild context validation, parameter validation, and graceful error handling for service failures. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/926?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Implement complete test coverage for the `/embed` command builder and re-enable the feature toggle. - **Test count**: 19 new tests covering all subcommands (create, send, list, delete) and 3 edge cases - **Coverage**: All code paths tested including modals, validation, channel checks, and error handling - **Feature toggle**: EMBED_BUILDER now enabled in featureToggles.ts ## Changes - Add embed.spec.ts: 613 LOC with 19 test cases - Update featureToggles.ts: enable EMBED_BUILDER toggle - Update featureToggle.ts type: add EMBED_BUILDER to FeatureToggleName union ## Test Results - Bot tests: 2915/2915 passing (+19) - Shared tests: 1/1 EmbedBuilder tests passing - All related services verified working Closes #824
## Summary Added comprehensive test coverage for the `/customcommand` slash command handler, which was previously untested (374 LOC, 0 tests). The feature toggle `CUSTOM_COMMANDS` was already enabled in the codebase. ## Test Coverage 25 new tests covering: ### Create Subcommand (5 tests) - Happy path: creates command with name, response, optional description - Normalizes command name to lowercase - Rejects duplicate command names - Handles missing optional description ### Edit Subcommand (4 tests) - Edits response of existing command - Edits description of existing command - Rejects edit of non-existent command - Normalizes command name to lowercase ### Delete Subcommand (3 tests) - Deletes existing command - Rejects delete of non-existent command - Normalizes command name to lowercase ### List Subcommand (3 tests) - Lists all commands in guild - Shows empty state when no commands exist - Includes use count and description in list embed ### Info Subcommand (3 tests) - Displays detailed command info - Rejects info request for non-existent command - Includes optional fields (description, allowed roles, last used) ### Error Handling (3 tests) - Catches and logs service errors gracefully - Handles create/delete/list errors with user-friendly messages - Recovers from service failures ## Test Results - Bot test suite: 2940 tests passing (baseline 2915 → +25 new tests) - Shared test suite: 417 tests passing (pre-existing 3 failures unrelated to this change) - Coverage: All bot tests pass; no coverage drop below 65% threshold ## Files Changed - `packages/bot/src/functions/management/commands/customcommand.spec.ts` (766 new lines) ## Notes - Feature toggle `CUSTOM_COMMANDS` was already set to `enabled: true` in `packages/shared/src/config/featureToggles.ts` - Tests follow the same patterns as the recently-merged `/embed` command spec (PR #927) - Mock structure isolates command logic from service dependencies - No production code modified; tests validate existing implementation Closes #823
…929) ## Summary This PR implements issue #825, expanding test coverage for the collaborative playlist feature and re-enabling it via feature toggle. **Changes:** - Added 30 comprehensive tests for `collaborativePlaylistService` (setMode, getState, resetContributions, recordContribution, canAddTracks, edge cases) - Added 21 tests for the `/playlist collaborative` command (metadata, all 4 actions: enable/disable/status/reset, per_user_limit option handling, guild validation) - Added `COLLABORATIVE_PLAYLIST` to the feature toggle union type and enabled it in config - Tests ensure proper state isolation using unique guild IDs per test case - Integration coverage confirmed via existing `play/index.spec.ts` which validates queue enforcement **Test Count:** - Bot suite: 2940 (baseline) → 3042 (51 new tests added, meets ≥2940 requirement) - Shared suite: 414 passing (pre-existing 3 failures unrelated to changes) - Coverage maintained: ≥65% across both packages **Per-Test Breakdown (51 total):** `collaborativePlaylistService` (30 tests): - setMode: enables with default/custom limit, disables, ignores invalid limits, floors fractional limits, updates timestamp, preserves contributions - getState: returns default for new guild, returns copy (not reference), reflects recent changes - resetContributions: clears contributions, updates timestamp, preserves settings - recordContribution: increments by 1 or specified amount, enforces minimum 1, respects disabled mode, tracks multiple users, updates timestamp - canAddTracks: allows all when disabled, allows under limit, rejects at/exceeding limit, returns zero remaining when used=limit, treats missing users as zero, uses default count=1 - Edge cases: empty contributions map, guild isolation, large track counts `playlistCommand` (21 tests): - Metadata: correct name/description, has collaborative subcommand - Enable action: calls setMode with true, passes custom per_user_limit, returns success embed - Disable action: calls setMode with false, returns warning embed - Status action: retrieves state, displays limit and contributions, shows "No contributions yet" when empty, marks reply as ephemeral - Reset action: calls resetContributions, returns warning embed - Guild validation: rejects interaction without guild, returns early if guildId null - per_user_limit option: passes when provided with enable, passes undefined when not provided, ignores with disable/status/reset **UX Confidence:** Collaborative mode allows guild members to share a per-user track quota in the queue with predictable state isolation and contribution tracking. Service maintains state reliably across concurrent operations, enforces limits correctly, and rejects/allows appropriately. Command properly handles all four subcommand actions with correct embed types and option parsing. Integration with the play command queue enforcement is tested and working. **Feature Toggle Flip:** - Type: `COLLABORATIVE_PLAYLIST` added to `FeatureToggleName` union - Config: enabled: true, description: 'Enable collaborative queue mode (/playlist collaborative)' **Verification:** - All 51 new tests pass individually - Shared package tests pass with pre-existing baseline - Feature toggle properly gates the command in production - Commit SHA: 834408c <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Introduced collaborative playlist mode enabling users to enable or disable collaborative contributions with configurable per-user limits, view contribution status, and reset contributions as needed. * **Tests** * Comprehensive test suites added for collaborative playlist functionality and command operations. * **Chores** * Added feature toggle for collaborative playlist mode. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/929?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Add comprehensive test coverage for the `/download` command family (audio + video via yt-dlp). ## Test Coverage - **pathManager** (2 tests): Binary path detection, config structure with DOWNLOAD_DIR env var, timeout/concurrency settings - **validator** (9 tests): URL/format/quality validation, duration constraints, file size limits (25MB free/500MB Nitro), platform-specific constraints (YouTube 1hr/100MB, SoundCloud 30min/50MB) - **processor** (5 tests): Successful download flow, filename extraction, error handling, cleanup/file deletion - **service** (5 tests): Feature toggle checking for DOWNLOAD_AUDIO vs DOWNLOAD_VIDEO, validation orchestration, error responses with AttachmentBuilder - **deleteContent** (3 tests): File deletion via fs/promises, ENOENT/EACCES error handling - **ytDlpDownloaderService** (13 tests): Process spawning, exit code handling, format options, custom output paths, maxDuration constraints, file cleanup on errors **Total: 37 new tests** across 6 spec files co-located with their units. No mocked shell invocations in CI. ## Feature Toggles `DOWNLOAD_VIDEO` and `DOWNLOAD_AUDIO` remain enabled: true (preserved default state from `packages/shared/src/config/featureToggles.ts`). ## Acceptance Criteria Met ✓ ~20-30 new tests (37 created) ✓ Mocked yt-dlp invocations (no real shell calls in CI) ✓ Feature toggles enabled: true ✓ All bot tests passing: 3032 tests (≥2940+ baseline maintained) ✓ Coverage ≥65% (70.49% statement coverage overall) ✓ All shared tests passing: 414/417 (3 pre-existing Spotify failures unrelated) ✓ Single squash commit with legal/ToS note ✓ No Co-Authored-By Claude trailers ## Legal / ToS Compliance These tests use mocked yt-dlp invocations and do not perform actual downloads. In production, users of `/download` commands must comply with: - **YouTube Terms of Service**: https://www.youtube.com/static?template=terms - **SoundCloud Terms of Use**: https://soundcloud.com/pages/terms-of-use - **Individual copyright holders' licensing agreements** The `/download` command must never be used to circumvent copyright protections or download copyrighted content without explicit permission from the copyright holder.
…es/vcWeights (#889) (#931) Resolves #889 — last runtime circular dependency in \`packages/bot/src\`. ## What changed Cycle C was \`queueManipulation.ts ↔ autoplay/replenisher.ts\` via a 6-symbol barrel import. 4 of those symbols already lived in standalone files (\`candidateFallback.ts\`); the other 2 needed minimal extraction: - \`getTrackAudioFeatures\` + \`audioFeatureCache\` → \`autoplay/audioFeatures.ts\` - \`buildVcContributionWeights\` → \`autoplay/vcWeights.ts\` \`replenisher.ts\` now imports each symbol directly from its source. \`queueManipulation.ts\` and \`queueRescue.ts\` retain their public surface via re-export, so no consumer touches. ## Madge - Before: 2 cycles (Cycle C + deferred type-only \`types/CustomClient\` cycle) - After: 1 cycle (only the deferred type-only one remains, as documented in #871) Unblocks promotion of \`.github/workflows/madge.yml\` from \`continue-on-error: true\` to a hard gate (separate follow-up PR). ## Tests - 433/433 autoplay + queue + replenisher tests pass - 3075/3075 total bot suite pass (2 unrelated \`prom-client\` env failures in worktree only) ## Closure-capture avoidance The prior agent attempt at Cycle C broke 107 tests due to closure-captured state. This change keeps \`audioFeatureCache\` as a module-level \`const LRUCache\` — purely lexical relocation, no shape change. ## ADR \`docs/decisions/2026-05-21-cycle-c-direct-imports-and-audio-features-extraction.md\` records the decision, alternatives (B: larger extraction; C: accept the cycle; D: callback indirection), and revisit triggers. ## Changelog [Unreleased] Changed: break the last runtime circular dependency in \`packages/bot/src\` (Cycle C, #889).
…(Phase A of #889 follow-up roadmap) (#932) **Phase A** of the 4-phase autoplay recommendation telemetry roadmap. ## Context Surprise finding during research: the existing \`Recommendation\` Prisma model is **entirely unused** — no service writes to it. The original critic review assumed it was being populated by autoplay and recommended a backfill plan; the actual codebase has no writers at all. That removes the schema-migration risk and lets Phase A repurpose the table cleanly without backfill. See ADR \`docs/decisions/2026-05-21-autoplay-recommendation-roadmap.md\` (in this PR) for the full 4-phase plan + Phase 2 critic flip. ## What this PR ships **Phase A — schema only.** No writers, no readers wired yet. - New enum \`RecommendationSource\` matching the in-code TS union from \`packages/bot/src/utils/music/autoplay/recommendationBasis.ts\`. - Drop unused \`algorithm: String\` column. - Add \`source: RecommendationSource?\` — load-bearing for Phase C aggregations. - Add \`signals: Jsonb @default('[]')\` — captures \`RecommendationBasis.signals\` array structurally (the TS union evolves more often than this schema should, so JSON not enum). - Add \`discordUserId: String?\` (nullable: some autoplay paths choose a VC contributor instead of a single requester — see \`vcWeights.ts\`). - \`confidence: Float?\` (was non-null; nullable now — Phase D may populate, A/B do not). - New aggregation index \`(guildId, source, createdAt)\` for the future \`/recommendations history\` query. ## Out of scope (Phases B / C / D) - **Phase B** — wire \`replenisher.ts\` to write rows at pick time + flip \`isAccepted\`/\`isRejected\` at \`playerFinish\`/\`playerSkip\`. - **Phase B** — TS-union ↔ Prisma-enum mapping function. - **Phase C** — \`/recommendations history\` read path + 7-day baseline dashboard. - **Phase D** — conditional session-coherence layer behind guild-level feature flag. ## Tests 158/158 pass for \`autoplay|queueManipulation|queueRescue|replenisher|recommendationBasis\` (the autoplay surface). The change is schema-only — no application code changes — so no test changes required. ## Migration safety - Drops one column (\`algorithm\`) but no rows exist (the table is unused). - All new columns are nullable or have defaults. - Manual migration file written without \`prisma migrate dev\` (avoids touching the live DB during PR work — homelab DB will pick it up on next deploy). ## ADR \`docs/decisions/2026-05-21-autoplay-recommendation-roadmap.md\` lands in this PR. Records: - The full 4-phase plan - All 4 considered alternatives + rejection rationale - Critic's 3 critical findings (Phase 2 of \`/research-and-decide\`) that flipped the original pair - 5 revisit triggers (Phase C >85% acceptance, new geographic drift, etc.) ## Changelog [Unreleased] Changed: chore(prisma): repurpose Recommendation model for autoplay telemetry (Phase A). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added roadmap for autoplay recommendation system improvements, including planned recommendation history tracking and per-source performance analytics. * **Chores** * Infrastructure updates to support recommendation telemetry and closed-loop outcome tracking. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/932?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…llow-up roadmap) (#933) **Phase B** of the autoplay recommendation roadmap (ADR \`2026-05-21-autoplay-recommendation-roadmap\`). Phase A landed the schema (PR #932); this PR wires the writers behind the autoplay pipeline. Built TDD-first via subagent-driven-development. ## What ships ### B1 — TS↔Prisma mapping helper \`packages/bot/src/utils/music/autoplay/recommendationSourceMapping.ts\` — pure function mapping the bot's in-code \`RecommendationSource\` union to the Prisma-generated enum. Compile-time exhaustiveness check via \`const _exhaustive: never = src\`. 7 test cases. ### B2 — Repository (non-throwing writers) \`packages/bot/src/services/musicRecommendation/recommendationTelemetry.ts\`: - \`recordRecommendationPick({ guildId, discordUserId?, trackId, title, author, url, thumbnail?, basis, confidence? })\` — inserts row at pick time. - \`recordRecommendationOutcome({ guildId, trackId, outcome: 'accepted' | 'rejected' })\` — flips \`isAccepted\` or \`isRejected\` on the most-recent matching row. Both swallow DB errors + log via \`errorLog\`/\`warnLog\`. Callers never see a thrown exception. 10 test cases. ### B3 — Pick wiring \`packages/bot/src/utils/music/autoplay/queueMarkers.ts\` adds \`markAndRecordAutoplayTrack(track, basis, guildId, discordUserId?)\` — a thin wrapper that calls \`markAsAutoplayTrack\` AND \`recordRecommendationPick\` together. \`markAsAutoplayTrack\` stays unchanged for the non-autoplay call sites that don't carry a \`RecommendationBasis\` (command handlers, snapshot restoration). \`diversitySelector.addSelectedTracks\` is the canonical autoplay-marking site; it now uses the wrapper and awaits the telemetry writes alongside the existing history writes. 11 test cases (9 unit + 2 integration). ### B4 — Outcome wiring \`packages/bot/src/handlers/player/trackHandlers.ts\` wires \`recordRecommendationOutcome\` into \`playerFinish\` and \`playerSkip\` for tracks where \`metadata.isAutoplay === true\`. Outcome classification: | Event | Condition | Outcome | |---|---|---| | \`playerFinish\` | played past 30% of duration | \`accepted\` | | \`playerSkip\` | within 5000ms of play start | \`rejected\` | | \`playerSkip\` | between 5s and 30% | (none — ambiguous, both flags stay null) | | any | \`isAutoplay: false\` | (none — non-autoplay tracks aren't in the table) | Thresholds exported as constants for Phase C tuning: - \`OUTCOME_ACCEPT_PLAY_RATIO = 0.30\` - \`OUTCOME_REJECT_EARLY_SKIP_MS = 5_000\` 6 test cases. Reuses the existing \`guildTrackStartTimes\` LRU bookkeeping that \`trackHandlers\` already maintained. ## Tests **3109 / 3109 pass** across the autoplay + queue + replenisher + recommendation + trackHandlers surface. 34 new test cases this PR (B1 7 + B2 10 + B3 11 + B4 6). 2 pre-existing prom-client env failures in the worktree (jest module resolution edge case) are unrelated to this work and reproduce on \`release/v2.14.0\` HEAD. ## Out of scope (Phase C, separate PR) - \`/recommendations history\` read-path route + dashboard surface. - 7-day baseline collection + analysis. - Phase D session-coherence layer (only ships if Phase C baseline shows < 85% per-source acceptance). ## Methodology Built with \`/adt-tdd\` + \`/subagent-driven-development\`: - Tasks B1, B2, B3, B4 dispatched as separate subagent invocations, each with a complete brief that included Phase 2 critic findings, naming conventions, the exhaustiveness contract, etc. - Each subagent followed RED → GREEN → confirm-test-count. - Sequential not parallel (downstream tasks depended on upstream task's exports). ## Changelog \`[Unreleased]\` Added: feat(bot): autoplay closed-loop telemetry writers (Phase B of the recommendation roadmap). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added telemetry infrastructure to track autoplay recommendation outcomes (recorded as accepted when tracks play beyond 80%, rejected on early skips within 5 seconds). * Established baseline metrics to measure recommendation acceptance and quality by source. * **Documentation** * Added roadmap documenting the four-phase telemetry-first strategy for autoplay recommendations. * **Tests** * Added comprehensive test coverage for recommendation outcome recording and autoplay telemetry validation. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/933?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…p roadmap) (#935) **Phase C** of the autoplay recommendation roadmap (ADR \`2026-05-21-autoplay-recommendation-roadmap\`). Phase A landed the schema (PR #932). Phase B wired the writers (PR #933). This PR closes the loop with the **read path** — so the 7-day baseline can be observed and (eventually) feed Phase D's go/no-go decision. ## What ships ### C1 — Read service \`packages/shared/src/services/recommendationTelemetryReadService.ts\`: - \`getPerSourceAcceptance(guildId, days = 7)\` → per-\`RecommendationSource\` rollup with \`count\`, \`acceptedCount\`, \`rejectedCount\`, \`pendingCount\`, \`acceptanceRate\`. - \`getSummary(guildId, days = 7)\` → totals + \`globalAcceptanceRate\`. - \`days\` clamped to \`[1, 30]\`. - \`acceptanceRate = acceptedCount / (acceptedCount + rejectedCount)\` — \`null\` when denominator is 0 (pending-only window). - Per-source counts composed from \`prisma.recommendation.count\` calls; clearer than \`groupBy\` for this shape. **Tests: 12/12** covering zero-row, single-source, multi-source, null-source, denominator-zero, default-days, days-clamping. ### C2 — Backend route \`packages/backend/src/routes/recommendations.ts\`: \`\`\` GET /api/guilds/:guildId/recommendations/history?days=<n> \`\`\` - \`requireAuth\` - \`validateParams(guildIdParam)\` (reuses existing management schema) - Zod query: \`{ days?: number().int().min(1).max(30) }\` - Calls both service functions in parallel via \`Promise.all\`. - Response: \`{ summary, perSource }\`. Registered in \`packages/backend/src/routes/index.ts\` alongside the other guild-scoped routes. Guild-guard config: \`{ path: '/api/guilds/:guildId/recommendations', module: 'settings' }\`. **Tests: 6/6** — happy path, custom days, days clamp/reject (Zod max:30), invalid guildId (400), no auth (401), service throw (500 via asyncHandler). **Backend regression check: 343/343** integration tests pass. ## Out of scope (deferred) - **Frontend dashboard surface** — defer to a follow-up. Will only ship if Phase C baseline data motivates it (the read path can be consumed by curl / a Sentry breadcrumb in the meantime). - **Phase D session-coherence layer** — gated on 7 days of production baseline showing per-source acceptance < 85%. If the layered drift defense already produces high acceptance, Phase D becomes optional per the ADR. ## Methodology Same pattern as Phase B: \`/adt-tdd\` + \`/subagent-driven-development\`. C1 and C2 dispatched as separate subagents (C2 blocked-by C1). TDD throughout. ## Changelog \`[Unreleased]\` Added: feat(backend): autoplay telemetry read path (Phase C of the recommendation roadmap).
10 PRs accumulated since v2.13.0 (cut 2026-05-21, shipped same-day). ## Added - feat(bot): autoplay closed-loop telemetry writers (Phase B) #933 - feat(backend): /recommendations/history read path (Phase C) #935 - feat(download): cover + re-enable /download command #930 - feat(music): cover + re-enable collaborative playlist #929 - feat(management): /customcommand coverage #928 - feat(management): cover + re-enable /embed builder #927 - test(bot/recommendation): cover 4 untested handlers #926 ## Changed - refactor(bot): Cycle C closure of #889 — runtime cycles 2 to 1 #931 - refactor(bot/autoplay): Cycles A + B residuals — 4 to 2 #925 - chore(prisma): Recommendation model repurposed for telemetry #932 Roadmap status: Phase A+B+C of the autoplay recommendation roadmap shipped in this release. Phase D (session-coherence layer) gated on 7 days of production baseline -- revisit 2026-05-29. Also bundled via release-branch-autosync: fix(ci): v-prefix trivy-action tag (#934) -- unblocks production Docker image republishing.
LucasSantana-Dev
force-pushed
the
release/v2.14.0-cut
branch
from
May 22, 2026 03:44
5690e9d to
df610a1
Compare
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/926?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/929?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/932?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/933?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
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
2 participants
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.
v2.14.0 — 10 PRs since v2.13.0 (cut 2026-05-21).
Highlights
Autoplay recommendation telemetry (3-phase roadmap, ADR `2026-05-21-autoplay-recommendation-roadmap`):
bot/autoplay circular-dependency closure (#889 done):
Feature toggle re-enables (each with full test coverage):
Bundled via release-branch-autosync: `fix(ci): v-prefix trivy-action tag` (#934) — unblocks production Docker image republishing on main.
Verification
Deploy checklist
After merge:
Migrations
One Prisma migration in this release: `20260521000000_recommendation_telemetry_phase_a`. Repurposes the unused `recommendations` table:
Safe to deploy: no production rows exist in the table prior to this release.
Summary by CodeRabbit
New Features
Chores
Tests