Conversation
When the bot is @mentioned in a regular channel, create a Discord thread and continue the conversation there instead of replying inline. If a thread already exists for that user's recent conversation (within 30 min), reuse it. - Add threading module with thread creation, reuse, and permission checks - Integrate threading into message handler with graceful fallback - Conversation history is scoped to thread ID when in thread mode - Handle edge cases: existing threads (inline), DMs (no threads), missing permissions (fallback to inline) - Add config options: ai.threadMode.enabled, autoArchiveMinutes, reuseWindowMinutes - 37 new threading tests + 3 new events integration tests - 100% coverage on threading module Closes #23
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds configurable thread-based messaging for AI conversations: new threadMode config, a threading module to create/reuse threads, event handler routing to threads when appropriate, and tests for threading behavior and permission flows. Changes
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/modules/threading.js`:
- Around line 14-24: The activeThreads Map lacks eviction, so entries can grow
unbounded; update the module to periodically sweep stale entries and/or enforce
a max-size cap: add a cleanup routine (e.g., scheduled with setInterval during
module init) that iterates activeThreads and deletes entries whose lastActive is
older than DEFAULT_REUSE_WINDOW_MS, and optionally implement a simple cap
eviction in the same module (when inserting into activeThreads in functions that
create/update entries, evict the oldest entries if size exceeds a chosen
MAX_ACTIVE_THREADS). Target symbols to change: activeThreads, findExistingThread
(where entries are read/created), and DEFAULT_REUSE_WINDOW_MS (use as the stale
threshold); ensure the cleanup runs off the main request path to avoid blocking.
- Around line 105-124: In generateThreadName, the computed prefix `${username}:
` can push the final name past MAX_THREAD_NAME_LENGTH when username is very
long; update the logic to first ensure the username portion is clamped so prefix
length never exceeds MAX_THREAD_NAME_LENGTH (e.g., truncate username and append
an ellipsis if needed), then compute maxContentLength = MAX_THREAD_NAME_LENGTH -
prefix.length and only include a truncated firstLine up to that non-negative
length; also apply the same length guard to the fallback `Chat with ${username}`
by truncating the username (with ellipsis) so the full fallback never exceeds
MAX_THREAD_NAME_LENGTH. Use the existing symbols generateThreadName,
MAX_THREAD_NAME_LENGTH, prefix, truncatedContent and the fallback string to
locate and adjust the truncation order and clamping.
- Around line 204-207: Validate threadConfig.autoArchiveMinutes before calling
message.startThread: ensure the value is one of Discord's allowed durations (60,
1440, 4320, 10080) and if not either throw/replace it with the nearest allowed
value, then pass that validated value as autoArchiveDuration in the call to
message.startThread (referencing the threadName/thread variable). Update the
logic around message.startThread to use the validatedDuration variable and add a
clear error or snap-to-nearest fallback when threadConfig.autoArchiveMinutes is
invalid.
In `@tests/modules/threading.test.js`:
- Around line 268-296: Add a unit test exercising generateThreadName with a very
long username to ensure name length handling when username alone approaches the
100-char limit; create a new it block that calls
generateThreadName('A'.repeat(98), 'short message') (and optionally another case
with >=100 chars) and assert the result respects the max length (<=100),
includes the username prefix or falls back correctly, and appends truncation
marker if applicable so the behavior is locked down after the threading.js
overflow fix.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
config.jsonsrc/modules/events.jssrc/modules/threading.jstests/modules/events.test.jstests/modules/threading.test.js
🧰 Additional context used
📓 Path-based instructions (3)
tests/**/*.test.js
📄 CodeRabbit inference engine (AGENTS.md)
Test coverage must maintain at least 80% threshold on statements, branches, functions, and lines
Files:
tests/modules/threading.test.jstests/modules/events.test.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Use ESM modules — import/export syntax; never use require()
Always use node: protocol for Node.js builtins (e.g., import { readFileSync } from 'node:fs')
Always use semicolons in code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Always use Winston for logging (import { info, warn, error } from '../logger.js'); never use console.log, console.warn, console.error, or any console.* method
Use custom error classes from src/utils/errors.js and log errors with context before re-throwing
Use getConfig() from src/modules/config.js to read config and setConfigValue(key, value) to update at runtime
Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Files:
src/modules/threading.jssrc/modules/events.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/modules/*.js: Register event handlers in src/modules/events.js for all modules before processing in handler functions
Check config.moduleName.enabled before processing in module handler functions
Files:
src/modules/threading.jssrc/modules/events.js
🧠 Learnings (2)
📚 Learning: 2026-02-15T03:14:33.915Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-15T03:14:33.915Z
Learning: Applies to src/modules/*.js : Register event handlers in src/modules/events.js for all modules before processing in handler functions
Applied to files:
tests/modules/events.test.jssrc/modules/events.js
📚 Learning: 2026-02-15T03:14:33.915Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-15T03:14:33.915Z
Learning: Applies to src/**/*.js : Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Applied to files:
tests/modules/events.test.jssrc/modules/events.js
🧬 Code graph analysis (4)
tests/modules/threading.test.js (2)
src/modules/threading.js (18)
clearActiveThreads(279-281)config(35-35)getThreadConfig(33-49)shouldUseThread(56-71)canCreateThread(78-96)name(109-109)generateThreadName(105-124)buildThreadKey(132-134)thread(157-157)thread(204-207)thread(257-257)findExistingThread(141-189)key(143-143)key(210-210)getActiveThreads(272-274)createThread(197-225)getOrCreateThread(234-266)existingThread(245-245)src/modules/config.js (1)
getConfig(130-132)
src/modules/threading.js (2)
src/modules/config.js (1)
getConfig(130-132)src/logger.js (2)
warn(223-225)info(216-218)
tests/modules/events.test.js (1)
src/modules/threading.js (2)
shouldUseThread(56-71)getOrCreateThread(234-266)
src/modules/events.js (1)
src/modules/threading.js (5)
shouldUseThread(56-71)getOrCreateThread(234-266)thread(157-157)thread(204-207)thread(257-257)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (9)
src/modules/threading.js (4)
1-12: LGTM — clean module structure, proper imports, and safe config access.The module follows ESM conventions, uses Winston logging, reads config via
getConfig(), and provides sensible defaults with a try/catch fallback. The overall design — separating concerns into small, testable functions — is well done.Also applies to: 33-49
56-71: LGTM —shouldUseThread,canCreateThread, andbuildThreadKeyare well-structured.Good defensive checks: DM guard, thread-inside-thread guard, channel type allowlist, permission checks with proper error handling and fallback to
false.Also applies to: 78-96, 132-134
141-189: LGTM —findExistingThreadhandles edge cases well.Expired window, deleted threads, fetch failures, archived thread unarchival with fallback — all properly handled with cleanup of stale map entries.
234-266: LGTM —getOrCreateThreadorchestrates the workflow cleanly with proper fallbacks.Permission check → reuse attempt → create with try/catch → graceful null return on failure. The contract is clear and the caller in
events.jscan simply check fornull.src/modules/events.js (2)
104-141: LGTM — Threading integration is clean and the fallback logic is correct.The
targetChannelpattern elegantly handles both threaded and inline paths without duplicating the response logic. Reference equality (targetChannel === message.channel) is a reliable way to distinguish the two paths since the variable is directly assigned. History scoping totargetChannel.idcorrectly uses the thread ID when threaded.
64-67: DM guard at line 67 prevents AI responses in DMs entirely, even without threading.
if (!message.guild) return;rejects all DMs at the top of the handler. This is unrelated to the threading PR but worth noting: if the bot is ever intended to respond to DMs via AI, this guard would need to be scoped more narrowly. CurrentlyshouldUseThreadalready has its own DM guard, so the early return here is redundant for threading but serves as a general DM block.config.json (1)
10-14: LGTM — Sensible defaults for the newthreadModeconfiguration.Thread mode is disabled by default (opt-in), and the default values (60-minute auto-archive, 30-minute reuse window) align with the constants in
threading.js.tests/modules/events.test.js (1)
45-49: LGTM — Good coverage of the three key threading paths in events.The new tests cover: (1) successful threading with history scoped to thread ID, (2) graceful fallback to inline reply when thread creation fails, and (3) long-message splitting within threads. The mock setup is clean and consistent with the existing test patterns.
Also applies to: 307-390
tests/modules/threading.test.js (1)
1-581: LGTM — Comprehensive test suite with excellent coverage of the threading module.37 tests covering all exported functions, edge cases (expired/deleted/archived threads, missing permissions, config errors), and state management. The
beforeEach/afterEachcleanup pattern prevents test pollution. Well done.
|
Note Docstrings generation - SUCCESS |
|
Bugbot Autofix prepared fixes for 3 of the 3 bugs found in the latest run.
Or push these changes by commenting: |
Docstrings generation was requested by @BillChirico. * #57 (comment) The following files were modified: * `src/modules/events.js` * `src/modules/threading.js`
Docstrings generation was requested by @BillChirico. * #57 (comment) The following files were modified: * `src/modules/events.js` * `src/modules/threading.js`
…cord values - getThreadConfig now validates reuseWindowMinutes (must be positive number) and autoArchiveMinutes (must be valid number), falling back to safe defaults - Add snapAutoArchiveDuration() that snaps to nearest Discord-allowed value (60, 1440, 4320, or 10080 minutes) - Invalid config values (NaN, negative, non-number) gracefully degrade Addresses PR #57 review threads #3 and #6.
- Add sweepExpiredThreads() that removes entries older than reuse window - Add MAX_CACHE_SIZE (1000) cap with LRU-style oldest-first eviction - Start periodic sweep (every 5 min) on module load with unref'd timer - Export startEvictionTimer/stopEvictionTimer for lifecycle control - Add tests for sweep logic and size cap enforcement Addresses PR #57 review threads #1 and #4.
- Add pendingThreadCreations map to serialize concurrent getOrCreateThread calls for the same user+channel combination - Second concurrent call waits for the first to complete, then reuses the thread from cache instead of creating a duplicate - Lock is automatically released after the promise settles - Add test verifying only one startThread call for concurrent requests Addresses PR #57 review thread #2.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/events.js (1)
104-141: 🧹 Nitpick | 🔵 TrivialThreading integration looks solid overall.
The fallback logic (thread → inline), history scoping via
targetChannel.id, and response routing are well-structured. The reference-equality check on Line 135 (targetChannel === message.channel) works correctly here sincetargetChannelis assigned directly frommessage.channelon Line 106.One minor observation: when
needsSplittingis true and threading is not in use, the response is sent viatargetChannel.send(chunk)(Line 133) rather thanmessage.reply(), so the first chunk loses the reply-reference to the original message. This is pre-existing behavior, but threading makes it more visible — you may want to reply to the original message for the first chunk andsendthe rest when inline.Optional: preserve reply reference for the first chunk when inline
if (needsSplitting(response)) { const chunks = splitMessage(response); - for (const chunk of chunks) { - await targetChannel.send(chunk); - } + for (let i = 0; i < chunks.length; i++) { + if (i === 0 && targetChannel === message.channel) { + await message.reply(chunks[i]); + } else { + await targetChannel.send(chunks[i]); + } + }
🤖 Fix all issues with AI agents
In `@src/modules/events.js`:
- Line 194: There’s a brace-alignment formatting error caused by an unmatched or
misaligned closing brace ('}') at the end of the events module; run the Biome
formatter (biome format) on src/modules/events.js to automatically fix brace
alignment, review the file to ensure the closing brace correctly matches the
surrounding function/module block, and commit the formatted file so CI passes.
In `@src/modules/threading.js`:
- Line 282: CI reports a Biome brace-alignment formatting error at the end of
src/modules/threading.js; run the Biome formatter or adjust the final closing
brace alignment so the file follows project formatting. Specifically, open
src/modules/threading.js and ensure the trailing brace that closes the module
(the final "}" that currently triggers the error) is aligned and followed by a
single newline per Biome rules, then run `biome format` (or your repository's
formatter command) to reformat the file and commit the updated file.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/modules/events.jssrc/modules/threading.js
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Use ESM modules — import/export syntax; never use require()
Always use node: protocol for Node.js builtins (e.g., import { readFileSync } from 'node:fs')
Always use semicolons in code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Always use Winston for logging (import { info, warn, error } from '../logger.js'); never use console.log, console.warn, console.error, or any console.* method
Use custom error classes from src/utils/errors.js and log errors with context before re-throwing
Use getConfig() from src/modules/config.js to read config and setConfigValue(key, value) to update at runtime
Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Files:
src/modules/events.jssrc/modules/threading.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/modules/*.js: Register event handlers in src/modules/events.js for all modules before processing in handler functions
Check config.moduleName.enabled before processing in module handler functions
Files:
src/modules/events.jssrc/modules/threading.js
🧠 Learnings (4)
📚 Learning: 2026-02-15T03:14:33.915Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-15T03:14:33.915Z
Learning: Applies to src/modules/*.js : Register event handlers in src/modules/events.js for all modules before processing in handler functions
Applied to files:
src/modules/events.js
📚 Learning: 2026-02-15T03:14:33.915Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-15T03:14:33.915Z
Learning: Applies to src/**/*.js : Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Applied to files:
src/modules/events.jssrc/modules/threading.js
📚 Learning: 2026-02-15T03:14:33.915Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-15T03:14:33.915Z
Learning: Applies to src/commands/{timeout,slowmode}.js : Discord timeouts max at 28 days and slowmode caps at 6 hours (21600s); enforce these limits in command logic
Applied to files:
src/modules/threading.js
📚 Learning: 2026-02-15T03:14:33.915Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-15T03:14:33.915Z
Learning: Applies to src/commands/{timeout,tempban,slowmode}.js : Use parseDuration() from src/utils/duration.js for duration-based commands (timeout, tempban, slowmode)
Applied to files:
src/modules/threading.js
🧬 Code graph analysis (2)
src/modules/events.js (1)
src/modules/threading.js (5)
shouldUseThread(56-71)getOrCreateThread(235-267)thread(159-159)thread(206-209)thread(258-258)
src/modules/threading.js (2)
src/modules/config.js (1)
getConfig(130-132)src/logger.js (2)
warn(223-225)info(216-218)
🪛 GitHub Actions: CI
src/modules/events.js
[error] 194-195: Biome formatting error: expected formatting changes (brace alignment) detected by formatting tool.
src/modules/threading.js
[error] 282-283: Biome formatting error: expected formatting changes (brace alignment) detected by formatting tool.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (7)
src/modules/threading.js (7)
1-12: Clean module setup — ESM imports, proper logging, good module doc.
33-49: DefensivegetThreadConfigwith safe defaults — LGTM.
56-71:shouldUseThreadlogic is correct and covers all edge cases.DMs, existing threads, and non-threadable channel types are all properly excluded.
78-96: Permission checks are well-handled with a safefalsefallback on error.
199-227:message.author.displayNamemay beundefinedin some contexts.
displayNameis a property onGuildMember, notUser. On theUserobject (message.author),displayNamewas added in discord.js v14.11 as an alias forglobalName ?? username. This should work with v14.25.1, but the fallback chainmessage.author.displayName || message.author.usernameis correct either way.
235-267:getOrCreateThread— clean orchestration with proper fallback.Permission check → reuse → create → fallback is well-structured with appropriate logging at each step.
156-190: Remove the null-check claim —threads.fetch()in discord.js v14 can legitimately returnnull.In discord.js v14,
ThreadManager.fetch(id)returnsPromise<ThreadChannel | null>, wherenullis returned when a thread exists but doesn't belong to the current channel's thread manager. It only throws (DiscordAPIError) when the thread doesn't exist or is inaccessible. Theif (!thread)check on line 160 is not dead code—it's a valid guard for the wrong-parent-channel case.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
When threading is enabled and ai.channels is configured, follow-up messages inside threads were being dropped because isAllowedChannel checked message.channel.id (the thread ID) instead of the parent channel ID. Now uses isThread() to detect threads and checks parentId against the allowlist. Adds tests for thread allowlist behavior (allow when parent matches, block when parent doesn't match).
Docstrings generation was requested by @BillChirico. * #57 (comment) The following files were modified: * `src/modules/events.js` * `src/modules/threading.js`
…cord values - getThreadConfig now validates reuseWindowMinutes (must be positive number) and autoArchiveMinutes (must be valid number), falling back to safe defaults - Add snapAutoArchiveDuration() that snaps to nearest Discord-allowed value (60, 1440, 4320, or 10080 minutes) - Invalid config values (NaN, negative, non-number) gracefully degrade Addresses PR #57 review threads #3 and #6.
- Add sweepExpiredThreads() that removes entries older than reuse window - Add MAX_CACHE_SIZE (1000) cap with LRU-style oldest-first eviction - Start periodic sweep (every 5 min) on module load with unref'd timer - Export startEvictionTimer/stopEvictionTimer for lifecycle control - Add tests for sweep logic and size cap enforcement Addresses PR #57 review threads #1 and #4.
- Add pendingThreadCreations map to serialize concurrent getOrCreateThread calls for the same user+channel combination - Second concurrent call waits for the first to complete, then reuses the thread from cache instead of creating a duplicate - Lock is automatically released after the promise settles - Add test verifying only one startThread call for concurrent requests Addresses PR #57 review thread #2.
1 participant
Summary
When the bot is @mentioned in a regular channel, instead of replying inline (which clutters the channel), it now creates a Discord thread and continues the conversation there. If a thread already exists for that user's recent conversation, it reuses it.
Closes #23
Changes
New:
src/modules/threading.jsThread management module with:
CREATE_PUBLIC_THREADSandSendMessagesInThreadsbefore attemptingModified:
src/modules/events.jsmessage.reply()when threading isn't availableModified:
config.jsonNew config section under
ai.threadMode:{ "ai": { "threadMode": { "enabled": false, "autoArchiveMinutes": 60, "reuseWindowMinutes": 30 } } }Edge Cases Handled
CREATE_PUBLIC_THREADSpermission → graceful fallback to inlineTesting