Repository navigation
docs: add decision records for may 23 refactor batch - #1040
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds six Architecture Decision Records documenting planned backend refactorings: service extraction (ArtistSuggestionService), pipeline and context value object introductions (AutoplayContext, MessagePipeline), API consolidation (Recommendation Engine single entrypoint), domain layer separation (Guild Automation split), and a test reduction strategy with replacement test protocols. ChangesArchitecture Decision Records – May 2026
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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.
|
Failed to generate code suggestions for PR |
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.
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.
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.
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.
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.
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.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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-23-artist-suggestion-service.md`:
- Around line 24-28: The fenced code block that lists Tier 1–3 suggestions is
missing a language tag and triggers markdownlint MD040; update that block (the
triple-backtick block containing "Tier 1: User's saved preferences..." through
"Tier 3: Static popular fallback list") to include a language identifier (e.g.,
add "text" after the opening ```), so the block becomes ```text and resolves the
lint error.
In `@docs/decisions/2026-05-23-autoplay-context-value-object.md`:
- Around line 23-44: Update the documented AutoplayContext interface in the ADR
to match the actual implemented contract in
packages/bot/src/utils/music/autoplay/autoplayContext.ts: replace/remove fields
that don’t exist (e.g., remove guildId and seedTracks), add missing fields such
as queue, and correct nullability/types for sessionMood, genreContext,
currentTrack, and recentArtists; also remove any extra maps/sets that aren’t
present and ensure field names (e.g., artistFrequency, likedWeights,
preferredArtistKeys, blockSertanejo, replenishCount) and their types exactly
match the implementation so the ADR reflects the real AutoplayContext definition
used by the codebase.
In `@docs/decisions/2026-05-23-bot-test-reduction-phase4-replacement-strategy.md`:
- Line 81: The ADR contains contradictory statements about Jest's it.each
counting behavior; update the document to consistently state that Jest treats
each row in it.each as a separate test (i.e., it DOES NOT reduce the test
count), and apply this correction where referenced (the earlier paragraph around
the current Line 20–23, the entry mentioning `candidateScorer.spec.ts` at Line
81, and the related sections around Lines 103–107) so all mentions reflect that
each it.each row is counted as an individual test for Phase 4 planning.
In `@docs/decisions/2026-05-23-guild-automation-orchestrator-repository-split.md`:
- Line 29: The ADR currently has a mismatch in the documented signature for
orchestrator.run: Line 29 lists (manifest, liveState, executors[], ctx) while
Line 45 omits ctx; make these consistent by choosing one canonical signature and
applying it to both locations—either add ctx to the signature on Line 45 or
remove ctx from Line 29 (and update any other occurrences in the document), and
ensure you reference the same symbol name "orchestrator.run" and parameter order
(manifest, liveState, executors[]) consistently throughout the ADR.
- Around line 43-47: Update the ADR to mandate that any call sequence starting
with repository.acquireLock(guildId) must guarantee
repository.releaseLock(guildId) in a finally path; explicitly state that
repository.getManifest/getLiveState, orchestrator.run, and
repository.persistRunResult may throw and therefore lock release must be
described as happening inside a try/finally (or equivalent) to avoid leaks,
referencing the sequence entries repository.acquireLock(guildId),
repository.getLiveState(guildId) (formerly getManifest), orchestrator.run(...),
repository.persistRunResult(...), and repository.releaseLock(guildId) so readers
know exactly which operations require the finally semantics.
In `@docs/decisions/2026-05-23-message-pipeline-handler-chain.md`:
- Line 58: Update the phrasing on line 58 to reflect actual file ownership:
don't state that messageHandler.ts "becomes messagePipeline.ts"; instead
indicate that messageHandler.ts (wiring/context) remains separate from
message/pipeline.ts (execution chain) and that adding a new concern introduces a
new file (e.g., message/pipeline.ts) rather than editing messageHandler.ts;
reference the filenames messageHandler.ts, messagePipeline.ts, and
message/pipeline.ts in the reworded sentence to make the split and
responsibilities explicit.
In `@docs/decisions/2026-05-23-recommendation-engine-single-entrypoint.md`:
- Around line 35-40: The doc currently defines strategy: 'history' |
'preference' | 'contextual' | 'auto' as required but also states 'auto' is the
default; make the contract unambiguous by either turning the field into an
optional property (e.g., strategy?: ...) and documenting that missing strategy
defaults to 'auto', or keep it required and explicitly state callers must pass
'auto' when they want default behavior; update the prose around the type
definition and the example usage to reflect the chosen approach so the ADR and
the type signature for the strategy field are consistent.
🪄 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: 09bf1d07-f1bc-4aa7-8565-1895411671a2
📒 Files selected for processing (6)
docs/decisions/2026-05-23-artist-suggestion-service.mddocs/decisions/2026-05-23-autoplay-context-value-object.mddocs/decisions/2026-05-23-bot-test-reduction-phase4-replacement-strategy.mddocs/decisions/2026-05-23-guild-automation-orchestrator-repository-split.mddocs/decisions/2026-05-23-message-pipeline-handler-chain.mddocs/decisions/2026-05-23-recommendation-engine-single-entrypoint.md
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
|
This branch was successfully deployed
1 participant
ADRs for the 5 refactors merged in PRs #979–#983 and the Phase 4 test reduction strategy.
Decision records added
2026-05-23-artist-suggestion-service.md— ArtistSuggestionService extraction (refactor(artists): extract ArtistSuggestionService from route handler #980)2026-05-23-autoplay-context-value-object.md— AutoplayContext value object (refactor(bot): introduce AutoplayContext value object #983)2026-05-23-bot-test-reduction-phase4-replacement-strategy.md— Phase 4 test replacement strategy2026-05-23-guild-automation-orchestrator-repository-split.md— GuildAutomationService split (refactor(shared): split GuildAutomationService into Orchestrator + Repository #982)2026-05-23-message-pipeline-handler-chain.md— MessagePipeline handler chain (refactor(bot): extract MessagePipeline handler chain #981)2026-05-23-recommendation-engine-single-entrypoint.md— recommendTracks() single entrypoint (refactor(music-recommendation): collapse to single recommendTracks() entrypoint #979)Summary by CodeRabbit