fix: log AI responses and user messages to conversations with guild_id#183
fix: log AI responses and user messages to conversations with guild_id#183BillChirico merged 13 commits intomainfrom
Conversation
- Add guild_id parameter to addToHistory() function - Update INSERT to include guild_id column - Call addToHistory() when sending AI responses in triage-respond.js - Call addToHistory() for user messages in accumulateMessage() - Update tests to expect 6 parameters instead of 5
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional trailing Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @BillChirico. The following files were modified: * `src/modules/ai.js` * `src/modules/triage.js` These files were kept as they were: * `src/modules/triage-respond.js` These files were ignored: * `tests/modules/ai.coverage.test.js` * `tests/modules/ai.test.js`
There was a problem hiding this comment.
Pull request overview
This PR fixes conversation logging so that both user messages and AI responses are actually persisted to the conversations table in production, and includes guild_id so the existing conversations API (which filters by guild_id) can retrieve those rows.
Changes:
- Extend
addToHistory()to acceptguildIdand persist it in theconversationsINSERT. - Call
addToHistory()from triage to log user messages duringaccumulateMessage(). - Call
addToHistory()from triage responder to log assistant messages after sending responses. - Update AI module tests to expect the additional SQL parameter.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/modules/ai.js |
Adds guildId support to persisted conversation history rows. |
src/modules/triage.js |
Logs incoming user messages into the conversation history/DB. |
src/modules/triage-respond.js |
Logs outgoing assistant messages into the conversation history/DB after sending. |
tests/modules/ai.test.js |
Updates DB write-through expectation to include guild_id param. |
tests/modules/ai.coverage.test.js |
Updates DB write-through expectation to include guild_id param. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/ai.js (1)
208-216:⚠️ Potential issue | 🟠 MajorUse guild-scoped config when trimming history.
addToHistorynow receivesguildId, but trimming still usesgetHistoryLength()with no guild argument. That can apply the wrong history limit in multi-guild setups and miss per-guild config updates.Suggested fix
-function getHistoryLength() { +function getHistoryLength(guildId = null) { try { - const config = getConfig(); + const config = getConfig(guildId); const len = config?.ai?.historyLength; if (typeof len === 'number' && len > 0) return len; } catch { // Config not loaded yet, use default } return DEFAULT_HISTORY_LENGTH; } @@ export function addToHistory(channelId, role, content, username, discordMessageId, guildId) { @@ - const maxHistory = getHistoryLength(); + const maxHistory = getHistoryLength(guildId);As per coding guidelines
src/modules/*.js: “Per-request modules (AI, spam, moderation) should callgetConfig(guildId)on every invocation for automatic config changes.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/ai.js` around lines 208 - 216, The trimming logic in addToHistory uses a global getHistoryLength() but addToHistory now receives guildId; update addToHistory to fetch per-guild config by calling getConfig(guildId) (or otherwise pass guildId into getHistoryLength if that function supports it) and compute maxHistory from that per-guild config before trimming conversationHistory for the channel; ensure you reference addToHistory, getConfig, getHistoryLength, and conversationHistory when making the change so history limits respect guild-scoped settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/triage-respond.js`:
- Around line 218-230: The moderation-path responses in triage-respond.js are
not being recorded to conversation history; ensure every message sent by
safeSend in both the normal and moderation branches calls addToHistory with the
same parameters (channelId, 'assistant', the message text/chunk used, null for
username, the sent message id, and channel.guild?.id || null). Locate the
moderation branch where a moderation response is sent (the variable analogous to
sentMsg or the safeSend call in that branch) and add an addToHistory call
mirroring the one after safeSend in the normal loop so moderated assistant
outputs are persisted.
---
Outside diff comments:
In `@src/modules/ai.js`:
- Around line 208-216: The trimming logic in addToHistory uses a global
getHistoryLength() but addToHistory now receives guildId; update addToHistory to
fetch per-guild config by calling getConfig(guildId) (or otherwise pass guildId
into getHistoryLength if that function supports it) and compute maxHistory from
that per-guild config before trimming conversationHistory for the channel;
ensure you reference addToHistory, getConfig, getHistoryLength, and
conversationHistory when making the change so history limits respect
guild-scoped settings.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/modules/ai.jssrc/modules/triage-respond.jssrc/modules/triage.jstests/modules/ai.coverage.test.jstests/modules/ai.test.js
📜 Review details
⏰ 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). (3)
- GitHub Check: Greptile Review
- GitHub Check: Agent
- GitHub Check: Docker Build Validation
🧰 Additional context used
📓 Path-based instructions (4)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESM modules — useimport/export, neverrequire()
Always usenode:protocol for Node.js builtins (e.g.import { readFileSync } from 'node:fs')
Always use semicolons in JavaScript code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Files:
src/modules/triage-respond.jstests/modules/ai.coverage.test.jstests/modules/ai.test.jssrc/modules/triage.jssrc/modules/ai.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Always use Winston logger —import { info, warn, error } from '../logger.js'. NEVER useconsole.log,console.warn,console.error, or anyconsole.*method in src/ files — replace any existing console calls with Winston equivalents
Pass structured metadata to Winston logs:info('Message processed', { userId, channelId })
Files:
src/modules/triage-respond.jssrc/modules/triage.jssrc/modules/ai.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Per-request modules (AI, spam, moderation) should call
getConfig(guildId)on every invocation for automatic config changes. Stateful resources should useonConfigChangelisteners for reactive updates
Files:
src/modules/triage-respond.jssrc/modules/triage.jssrc/modules/ai.js
tests/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
All new code must include tests. Test coverage must maintain an 80% threshold on statements, branches, functions, and lines. Run
pnpm testbefore every commit
Files:
tests/modules/ai.coverage.test.jstests/modules/ai.test.js
🧠 Learnings (2)
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Use `splitMessage()` utility for messages exceeding Discord's 2000-character limit
Applied to files:
src/modules/triage-respond.js
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Applies to src/modules/*.js : Per-request modules (AI, spam, moderation) should call `getConfig(guildId)` on every invocation for automatic config changes. Stateful resources should use `onConfigChange` listeners for reactive updates
Applied to files:
src/modules/ai.js
🧬 Code graph analysis (4)
src/modules/triage-respond.js (2)
src/utils/safeSend.js (1)
safeSend(116-123)src/modules/ai.js (2)
addToHistory(208-241)channelId(278-278)
tests/modules/ai.test.js (1)
tests/utils/debugFooter.test.js (1)
mockQuery(5-5)
src/modules/triage.js (1)
src/modules/ai.js (2)
addToHistory(208-241)channelId(278-278)
src/modules/ai.js (3)
src/modules/triage-respond.js (1)
guildId(284-284)src/index.js (1)
guildId(182-182)src/deploy-commands.js (1)
guildId(26-26)
🔇 Additional comments (3)
tests/modules/ai.coverage.test.js (1)
183-186: Expectation update is correct for the new DB argument shape.This assertion now matches the 6-parameter insert path including
guildId.src/modules/triage.js (1)
601-609: Good addition: user messages now persist with guild and Discord message IDs.This is the right call-site wiring for conversation traceability.
tests/modules/ai.test.js (1)
152-155: Test expectation correctly reflects the newguildIdparameter.The DB write assertion now matches the 6-value insert payload.
|
| Filename | Overview |
|---|---|
| src/modules/ai.js | Added guildId parameter to addToHistory() and updated SQL INSERT to include guild_id column with proper parameterized query |
| src/modules/triage-respond.js | Imported addToHistory and created helper to log AI responses after sending; handles both single messages and arrays correctly |
| src/modules/triage.js | Imported addToHistory and calls it for user messages in accumulateMessage(); switched to dynamic config fetch via getConfig() |
| tests/modules/ai.test.js | Updated test expectations for 6-parameter signature; added test coverage for guildId parameter |
| tests/modules/ai.coverage.test.js | Updated test expectations to include the new guildId parameter (6th parameter) |
| tests/modules/triage.test.js | Added mocks for addToHistory and getConfig; added tests verifying user messages are logged with correct parameters including guildId |
Sequence Diagram
sequenceDiagram
participant User
participant Discord
participant accumulateMessage
participant addToHistory
participant Database
participant Triage
participant sendResponses
User->>Discord: Send message
Discord->>accumulateMessage: message event
accumulateMessage->>addToHistory: Log user message<br/>(role: 'user', guildId)
addToHistory->>Database: INSERT conversation<br/>(includes guild_id)
Note over Triage: Periodic evaluation or<br/>trigger word detected
Triage->>Triage: Classify & generate response
Triage->>sendResponses: Send AI response
sendResponses->>Discord: safeSend(message)
Discord-->>sendResponses: Message object(s)
sendResponses->>addToHistory: Log AI response<br/>(role: 'assistant', guildId)
addToHistory->>Database: INSERT conversation<br/>(includes guild_id)
Last reviewed commit: 66153ce
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/ai.js (1)
209-217:⚠️ Potential issue | 🟠 MajorPass guild-scoped config into history trimming.
Line 216 still uses
getHistoryLength()without guild context, even though Line 209 now receivesguildId. In multi-guild usage this can apply the wrong config and miss per-request config refresh behavior.Suggested fix
-function getHistoryLength() { +function getHistoryLength(guildId = null) { try { - const config = getConfig(); + const config = getConfig(guildId); const len = config?.ai?.historyLength; if (typeof len === 'number' && len > 0) return len; } catch { // Config not loaded yet, use default } return DEFAULT_HISTORY_LENGTH; } export function addToHistory(channelId, role, content, username, discordMessageId, guildId) { @@ - const maxHistory = getHistoryLength(); + const maxHistory = getHistoryLength(guildId);As per coding guidelines: "Per-request modules (AI, spam, moderation) should call
getConfig(guildId)on every invocation for automatic config changes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/ai.js` around lines 209 - 217, addToHistory must fetch per-guild config and use it when computing max history: call getConfig(guildId) at the start of addToHistory, then derive maxHistory from that config (either by passing the config/guildId into getHistoryLength or reading config.historyLength) and use that value for trimming. Update references to getHistoryLength() in addToHistory to use the guild-scoped config so per-request config changes are honored (function: addToHistory; helper: getConfig and getHistoryLength or config.historyLength).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/triage.js`:
- Line 33: The import ordering in triage.js is incorrect; move or reorder the
import of addToHistory (import { addToHistory } from './ai.js') so that imports
are sorted per the project's Biome rules (or run Biome --fix) to resolve the
"imports/exports not sorted" CI failure; specifically ensure addToHistory's
import is placed in the correct alphabetical/grouping position among other
imports in the module (or run the auto-fix) so the file's import block complies
with the linter.
---
Outside diff comments:
In `@src/modules/ai.js`:
- Around line 209-217: addToHistory must fetch per-guild config and use it when
computing max history: call getConfig(guildId) at the start of addToHistory,
then derive maxHistory from that config (either by passing the config/guildId
into getHistoryLength or reading config.historyLength) and use that value for
trimming. Update references to getHistoryLength() in addToHistory to use the
guild-scoped config so per-request config changes are honored (function:
addToHistory; helper: getConfig and getHistoryLength or config.historyLength).
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/modules/ai.jssrc/modules/triage.js
📜 Review details
⏰ 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: Greptile Review
🧰 Additional context used
📓 Path-based instructions (3)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESM modules — useimport/export, neverrequire()
Always usenode:protocol for Node.js builtins (e.g.import { readFileSync } from 'node:fs')
Always use semicolons in JavaScript code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Files:
src/modules/ai.jssrc/modules/triage.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Always use Winston logger —import { info, warn, error } from '../logger.js'. NEVER useconsole.log,console.warn,console.error, or anyconsole.*method in src/ files — replace any existing console calls with Winston equivalents
Pass structured metadata to Winston logs:info('Message processed', { userId, channelId })
Files:
src/modules/ai.jssrc/modules/triage.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Per-request modules (AI, spam, moderation) should call
getConfig(guildId)on every invocation for automatic config changes. Stateful resources should useonConfigChangelisteners for reactive updates
Files:
src/modules/ai.jssrc/modules/triage.js
🧠 Learnings (1)
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Applies to src/modules/*.js : Per-request modules (AI, spam, moderation) should call `getConfig(guildId)` on every invocation for automatic config changes. Stateful resources should use `onConfigChange` listeners for reactive updates
Applied to files:
src/modules/ai.js
🧬 Code graph analysis (2)
src/modules/ai.js (4)
src/modules/triage.js (1)
channelId(567-567)src/modules/triage-respond.js (1)
guildId(284-284)src/index.js (1)
guildId(182-182)src/deploy-commands.js (1)
guildId(26-26)
src/modules/triage.js (1)
src/modules/ai.js (2)
addToHistory(209-242)channelId(279-279)
🪛 GitHub Actions: CI
src/modules/triage.js
[error] 18-18: Biome: Imports and exports are not sorted. Auto-fix available.
🔇 Additional comments (2)
src/modules/ai.js (1)
228-237: Guild ID persistence and logging context look correct.Line 228-Line 230 correctly persists
guild_id, and Line 237 addsguildIdto structured error metadata for DB-write failure diagnosis.src/modules/triage.js (1)
606-614: User-message conversation logging is correctly integrated.Line 606-Line 614 logs the sanitized buffered user message with channel/message context and guild fallback, which aligns with the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/triage-respond.js`:
- Around line 181-191: safeSend can return an array of message objects, but the
current check only logs history when sentMsg is non-array, causing dropped
history entries; update the logic around safeSend (the call at triage-respond.js
where sentMsg is awaited) to handle both single Message and Array<Message>
returns by iterating when Array.isArray(sentMsg) and calling addToHistory for
each individual message (preserving channelId, 'assistant', the corresponding
chunks[i] content, null, each message.id, and channel.guild?.id || null); apply
the same change to the equivalent block handling chunks at the other occurrence
(the block around addToHistory at lines ~228-239) so all returned messages are
persisted.
In `@src/modules/triage.js`:
- Around line 555-565: The accumulateMessage function currently uses the passed
msgConfig instead of fetching per-invocation settings; update accumulateMessage
to call getConfig(guildId) at the start (use message.guild?.id or guildId
extracted from message) and use that result for all triage behavior (enabled
flag, maxBufferSize, trigger words, timers) rather than the msgConfig parameter;
remove or ignore the incoming msgConfig where used (e.g., checks for triage
enabled, buffer size, trigger-word logic) so runtime config updates take effect.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/modules/triage-respond.jssrc/modules/triage.js
📜 Review details
⏰ 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: Greptile Review
🧰 Additional context used
📓 Path-based instructions (3)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESM modules — useimport/export, neverrequire()
Always usenode:protocol for Node.js builtins (e.g.import { readFileSync } from 'node:fs')
Always use semicolons in JavaScript code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Files:
src/modules/triage-respond.jssrc/modules/triage.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Always use Winston logger —import { info, warn, error } from '../logger.js'. NEVER useconsole.log,console.warn,console.error, or anyconsole.*method in src/ files — replace any existing console calls with Winston equivalents
Pass structured metadata to Winston logs:info('Message processed', { userId, channelId })
Files:
src/modules/triage-respond.jssrc/modules/triage.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Per-request modules (AI, spam, moderation) should call
getConfig(guildId)on every invocation for automatic config changes. Stateful resources should useonConfigChangelisteners for reactive updates
Files:
src/modules/triage-respond.jssrc/modules/triage.js
🧠 Learnings (2)
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Applies to src/commands/*.js : Moderation commands must follow the shared pattern: `deferReply({ ephemeral: true })`, validate inputs, `sendDmNotification()`, execute Discord action, `createCase()`, `sendModLogEmbed()`, `checkEscalation()`
Applied to files:
src/modules/triage-respond.js
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Use `splitMessage()` utility for messages exceeding Discord's 2000-character limit
Applied to files:
src/modules/triage-respond.js
🧬 Code graph analysis (2)
src/modules/triage-respond.js (2)
src/utils/safeSend.js (1)
safeSend(116-123)src/modules/ai.js (2)
addToHistory(209-242)channelId(279-279)
src/modules/triage.js (1)
src/modules/ai.js (2)
addToHistory(209-242)channelId(279-279)
🔇 Additional comments (3)
src/modules/triage-respond.js (1)
11-11: Good import wiring for conversation persistence.
addToHistoryis imported cleanly and aligns with the updated assistant-history flow.src/modules/triage.js (2)
26-26:addToHistoryimport integration looks good.This import cleanly enables user-message persistence in the triage flow.
612-620: No action needed — bot-message filtering is already in place.The bot guard (
if (message.author.bot) return;) at line 122 insrc/modules/events.jsexecutes before any call toaccumulateMessage. BothaccumulateMessagecalls (lines 226 and 259) are downstream from this guard and therefore only process non-bot messages. Assistant outputs cannot be stored as user messages through this code path.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
User message logging in accumulateMessage broke triage tests due to missing mocks. AI responses are still logged in triage-respond.js. Reverts user message logging - can add back with proper test mocks later.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/triage.js (1)
557-610:⚠️ Potential issue | 🟠 MajorUser messages are still not persisted to conversation history.
accumulateMessagedocuments that it records messages in history, but after buffering (Line 610) there is noaddToHistory(...)call. This leaves conversation records incomplete and misses the user-turn logging objective.Suggested fix
+import { addToHistory } from './ai.js'; import { getConfig } from './config.js'; @@ // Push to ring buffer (with truncation warning) pushToBuffer(channelId, entry, maxBufferSize); + + // Persist user message to conversation history + addToHistory( + channelId, + 'user', + entry.content, + message.author.username || null, + message.id, + message.guild?.id || null, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/triage.js` around lines 557 - 610, accumulateMessage builds a buffer entry but never persists the same data to conversation history; after pushToBuffer(channelId, entry, maxBufferSize) add a call to the history persistence function (e.g., addToHistory or whatever history writer is used in the module) passing the entry and channel/guild context so the user turn is recorded (include entry, channelId and message.guild?.id or null). Ensure you call the same sanitized/truncated entry (including replyTo) so buffer and history stay consistent and handle any errors from addToHistory with debug logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/modules/triage.js`:
- Around line 557-610: accumulateMessage builds a buffer entry but never
persists the same data to conversation history; after pushToBuffer(channelId,
entry, maxBufferSize) add a call to the history persistence function (e.g.,
addToHistory or whatever history writer is used in the module) passing the entry
and channel/guild context so the user turn is recorded (include entry, channelId
and message.guild?.id or null). Ensure you call the same sanitized/truncated
entry (including replyTo) so buffer and history stay consistent and handle any
errors from addToHistory with debug logging.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/modules/triage-respond.jssrc/modules/triage.js
📜 Review details
⏰ 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: Greptile Review
🧰 Additional context used
📓 Path-based instructions (3)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESM modules — useimport/export, neverrequire()
Always usenode:protocol for Node.js builtins (e.g.import { readFileSync } from 'node:fs')
Always use semicolons in JavaScript code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Files:
src/modules/triage-respond.jssrc/modules/triage.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Always use Winston logger —import { info, warn, error } from '../logger.js'. NEVER useconsole.log,console.warn,console.error, or anyconsole.*method in src/ files — replace any existing console calls with Winston equivalents
Pass structured metadata to Winston logs:info('Message processed', { userId, channelId })
Files:
src/modules/triage-respond.jssrc/modules/triage.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Per-request modules (AI, spam, moderation) should call
getConfig(guildId)on every invocation for automatic config changes. Stateful resources should useonConfigChangelisteners for reactive updates
Files:
src/modules/triage-respond.jssrc/modules/triage.js
🧠 Learnings (2)
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Applies to src/commands/*.js : Moderation commands must follow the shared pattern: `deferReply({ ephemeral: true })`, validate inputs, `sendDmNotification()`, execute Discord action, `createCase()`, `sendModLogEmbed()`, `checkEscalation()`
Applied to files:
src/modules/triage-respond.jssrc/modules/triage.js
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Applies to src/modules/*.js : Per-request modules (AI, spam, moderation) should call `getConfig(guildId)` on every invocation for automatic config changes. Stateful resources should use `onConfigChange` listeners for reactive updates
Applied to files:
src/modules/triage.js
🧬 Code graph analysis (2)
src/modules/triage-respond.js (2)
src/modules/ai.js (2)
channelId(279-279)addToHistory(209-242)src/utils/safeSend.js (1)
safeSend(116-123)
src/modules/triage.js (2)
src/modules/config.js (2)
getConfig(282-313)err(94-94)src/modules/triage-filter.js (2)
triageConfig(52-52)checkTriggerWords(51-65)
🔇 Additional comments (3)
src/modules/triage-respond.js (2)
32-38: Robust handling ofsafeSendreturn shapes in history helper.This helper correctly handles both
MessageandMessage[]results and avoids dropping history entries when splitting occurs.
204-205: History persistence is correctly applied in both response paths.Wiring
logAssistantHistory(...)in both moderation and normal send loops closes previous assistant-history gaps.Also applies to: 242-244
src/modules/triage.js (1)
564-567: Per-invocation live config usage is correctly applied.Using
getConfig(message.guild?.id || null)and threadingliveConfigthrough trigger evaluation and scheduling aligns this path with runtime config refresh behavior.As per coding guidelines:
src/modules/*.js: Per-request modules (AI, spam, moderation) should callgetConfig(guildId)on every invocation for automatic config changes.Also applies to: 613-618, 623-623
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
BillChirico
left a comment
BillChirico
left a comment
There was a problem hiding this comment.
Fixed: re-added addToHistory call in accumulateMessage() with proper vi.mock for ai.js and getConfig mock for tests. User messages are now logged to conversations with guild_id alongside AI responses. All 54 tests passing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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 (2)
tests/modules/ai.test.js (1)
145-160:⚠️ Potential issue | 🟠 MajorAdd a positive-path test for non-null
guildIdpersistence.The updated assertion covers only
guild_id = null. Please add one case that callsaddToHistory(..., guildId)and asserts the 6th SQL parameter contains that guild ID.🧪 Suggested test addition
it('should write to DB when pool is available', () => { @@ ]); }); + + it('should persist guild_id when provided', () => { + const mockQuery = vi.fn().mockResolvedValue({}); + setPool({ query: mockQuery }); + + addToHistory('ch1', 'user', 'hello', 'testuser', 'msg-1', 'guild-1'); + + expect(mockQuery).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO conversations'), [ + 'ch1', + 'user', + 'hello', + 'testuser', + 'msg-1', + 'guild-1', + ]); + });As per coding guidelines:
tests/**/*.js: "All new code must include tests."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/modules/ai.test.js` around lines 145 - 160, Add a positive-path test that calls addToHistory with a non-null guildId and asserts the SQL parameter array includes that guildId in the 6th position: create a new it() similar to the existing one, set a fresh mockQuery/mockPool, call addToHistory('chX', 'user', 'msg', 'testuser', 'GUILD_ID'), then expect(mockQuery).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO conversations'), expect.arrayContaining([]) replaced by an explicit array where the 6th element (index 5) equals 'GUILD_ID'); ensure you reference addToHistory and the test's mockQuery/mockPool so the new test verifies the non-null guildId persistence.src/modules/triage.js (1)
590-608:⚠️ Potential issue | 🟠 MajorAdd timeout to referenced message fetch to prevent internal blocking.
The
message.channel.messages.fetch()call at line 593 has no timeout. Even though external callers use fire-and-forget patterns (lines 226, 259 in events.js), the fetch blocks the internal pipeline—delayingpushToBufferandscheduleEvaluationif the Discord API is slow or unresponsive. Your codebase already usesPromise.racewith timeouts elsewhere (line 135-138 for memory context). Apply the same pattern here with a 500ms timeout for message fetches.⏱️ Suggested bounded fetch
if (message.reference?.messageId) { try { - const ref = await message.channel.messages.fetch(message.reference.messageId); + const ref = await Promise.race([ + message.channel.messages.fetch(message.reference.messageId), + new Promise((_, reject) => + setTimeout(() => reject(new Error('Referenced message fetch timeout')), 500), + ), + ]); entry.replyTo = { author: ref.author.username, userId: ref.author.id, content: sanitizeText(ref.content?.slice(0, 500)) || '', messageId: ref.id, }; } catch (err) { debug('Referenced message fetch failed', {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/triage.js` around lines 590 - 608, The referenced-message fetch (message.channel.messages.fetch) can block the pipeline—wrap that fetch in a bounded promise using Promise.race with a 500ms timeout and abort/failover if the timeout elapses; on success populate entry.replyTo as before (author, userId, sanitized content, messageId), and on timeout or error call the existing debug(...) branch to log channelId, messageId, referenceId and the timeout error message; use the same timeout pattern used for memory context (the Promise.race timeout helper used around memory context code) to implement this change.
♻️ Duplicate comments (1)
src/modules/triage.js (1)
18-48:⚠️ Potential issue | 🟠 MajorOrganize imports to resolve the current Biome CI failure.
The file still fails
Organize Imports(pipeline error at Line 18). Please run Biome import organization on this module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/triage.js` around lines 18 - 48, Run the Biome import organizer on this module to fix the Organize Imports CI failure; reorder and group the imports (external libs first, then local modules, then sub-module imports) and remove any duplicate or unused imports such as duplicates of debug/info or repeated block imports; ensure the existing symbols (debug, info, error/logError, warn, loadPrompt, promptPath, safeSend, CLIProcess, CLIProcessError, buildMemoryContext, extractAndStoreMemories, getConfig, addToHistory, channelBuffers, clearEvaluatedMessages, consumePendingReeval, pushToBuffer, getDynamicInterval, isChannelEligible, resolveTriageConfig, checkTriggerWords, sanitizeText, parseClassifyResult, parseRespondResult, buildClassifyPrompt, buildRespondPrompt, buildStatsAndLog, fetchChannelContext, sendModerationLog, sendResponses) remain correctly imported and not duplicated after organization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/modules/triage.test.js`:
- Line 68: The mock implementation for getConfig in tests/modules/triage.test.js
declares an unused parameter named guildId which triggers a lint warning; update
the mock (getConfig: vi.fn(...)) to either remove the parameter or rename it to
_guildId (e.g., getConfig: vi.fn((_guildId) => mockGlobalConfig)) so the linter
recognizes it as intentionally unused and the CI warning is resolved.
- Around line 53-63: The test must assert that accumulateMessage invokes
addToHistory with the expected triage-side log payload: call
accumulateMessage(...) in the test with a sample message object (including
author.id, content, id, channel.id, guild?.id) and then expect the mocked
addToHistory to have been called once with an object containing those fields
(user/message content, user id, message id, channel id, guild id) and any
required context flags; also ensure you clear/reset mocks before the assertion.
Target the addToHistory mock and the accumulateMessage function name when adding
the expectation so the test directly verifies the new history-logging behavior.
---
Outside diff comments:
In `@src/modules/triage.js`:
- Around line 590-608: The referenced-message fetch
(message.channel.messages.fetch) can block the pipeline—wrap that fetch in a
bounded promise using Promise.race with a 500ms timeout and abort/failover if
the timeout elapses; on success populate entry.replyTo as before (author,
userId, sanitized content, messageId), and on timeout or error call the existing
debug(...) branch to log channelId, messageId, referenceId and the timeout error
message; use the same timeout pattern used for memory context (the Promise.race
timeout helper used around memory context code) to implement this change.
In `@tests/modules/ai.test.js`:
- Around line 145-160: Add a positive-path test that calls addToHistory with a
non-null guildId and asserts the SQL parameter array includes that guildId in
the 6th position: create a new it() similar to the existing one, set a fresh
mockQuery/mockPool, call addToHistory('chX', 'user', 'msg', 'testuser',
'GUILD_ID'), then
expect(mockQuery).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO
conversations'), expect.arrayContaining([]) replaced by an explicit array where
the 6th element (index 5) equals 'GUILD_ID'); ensure you reference addToHistory
and the test's mockQuery/mockPool so the new test verifies the non-null guildId
persistence.
---
Duplicate comments:
In `@src/modules/triage.js`:
- Around line 18-48: Run the Biome import organizer on this module to fix the
Organize Imports CI failure; reorder and group the imports (external libs first,
then local modules, then sub-module imports) and remove any duplicate or unused
imports such as duplicates of debug/info or repeated block imports; ensure the
existing symbols (debug, info, error/logError, warn, loadPrompt, promptPath,
safeSend, CLIProcess, CLIProcessError, buildMemoryContext,
extractAndStoreMemories, getConfig, addToHistory, channelBuffers,
clearEvaluatedMessages, consumePendingReeval, pushToBuffer, getDynamicInterval,
isChannelEligible, resolveTriageConfig, checkTriggerWords, sanitizeText,
parseClassifyResult, parseRespondResult, buildClassifyPrompt,
buildRespondPrompt, buildStatsAndLog, fetchChannelContext, sendModerationLog,
sendResponses) remain correctly imported and not duplicated after organization.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/modules/triage.jstests/modules/ai.coverage.test.jstests/modules/ai.test.jstests/modules/triage.test.js
📜 Review details
⏰ 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). (2)
- GitHub Check: Greptile Review
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (4)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESM modules — useimport/export, neverrequire()
Always usenode:protocol for Node.js builtins (e.g.import { readFileSync } from 'node:fs')
Always use semicolons in JavaScript code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Files:
tests/modules/ai.coverage.test.jssrc/modules/triage.jstests/modules/triage.test.jstests/modules/ai.test.js
tests/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
All new code must include tests. Test coverage must maintain an 80% threshold on statements, branches, functions, and lines. Run
pnpm testbefore every commit
Files:
tests/modules/ai.coverage.test.jstests/modules/triage.test.jstests/modules/ai.test.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Always use Winston logger —import { info, warn, error } from '../logger.js'. NEVER useconsole.log,console.warn,console.error, or anyconsole.*method in src/ files — replace any existing console calls with Winston equivalents
Pass structured metadata to Winston logs:info('Message processed', { userId, channelId })
Files:
src/modules/triage.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Per-request modules (AI, spam, moderation) should call
getConfig(guildId)on every invocation for automatic config changes. Stateful resources should useonConfigChangelisteners for reactive updates
Files:
src/modules/triage.js
🧠 Learnings (2)
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Applies to src/modules/*.js : Per-request modules (AI, spam, moderation) should call `getConfig(guildId)` on every invocation for automatic config changes. Stateful resources should use `onConfigChange` listeners for reactive updates
Applied to files:
src/modules/triage.jstests/modules/triage.test.js
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Applies to src/commands/*.js : Moderation commands must follow the shared pattern: `deferReply({ ephemeral: true })`, validate inputs, `sendDmNotification()`, execute Discord action, `createCase()`, `sendModLogEmbed()`, `checkEscalation()`
Applied to files:
src/modules/triage.js
🧬 Code graph analysis (2)
src/modules/triage.js (3)
src/modules/config.js (2)
getConfig(282-313)err(94-94)src/modules/triage-filter.js (2)
triageConfig(52-52)checkTriggerWords(51-65)src/modules/ai.js (2)
addToHistory(209-242)channelId(279-279)
tests/modules/triage.test.js (2)
src/modules/triage.js (1)
config(60-60)src/modules/ai.js (2)
config(34-34)config(49-49)
🪛 GitHub Actions: CI
src/modules/triage.js
[error] 18-18: The imports and exports are not sorted. Biome fix: Organize Imports.
tests/modules/triage.test.js
[warning] 68-68: lint/correctness/noUnusedFunctionParameters: Unused parameter 'guildId'. If this is intentional, prefix with an underscore.
🔇 Additional comments (1)
tests/modules/ai.coverage.test.js (1)
183-190: DB insert expectation is correctly aligned with the new 6-parameter write.This update matches the
guild_idexpansion and keeps the write-through assertion accurate.
Add two new test cases in the accumulateMessage describe block: - verifies addToHistory is called with correct args for a guild message (channelId, 'user', content, author, messageId, guildId) - verifies addToHistory receives null guildId for DMs (no guild property) Addresses CodeRabbit thread PRRT_kwDORICdSM5xcDiy (major: core PR behavior was not asserted in tests).
The existing DB write test only asserted that guildId was null in the
INSERT params. Add a new test that passes a real guildId ('guild-456')
and verifies it flows through to the INSERT statement.
Addresses Copilot review thread PRRT_kwDORICdSM5xb_9O.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Log an assistant message (or multiple messages when safeSend splits into an array) | ||
| * to conversation history. | ||
| * | ||
| * `safeSend` can return either a single Message object or an array of Message objects | ||
| * when the content was split across multiple Discord messages. Both cases are handled | ||
| * here so history is never silently dropped. | ||
| * | ||
| * @param {string} channelId - The channel the message was sent in. | ||
| * @param {string|null} guildId - The guild ID, or null for DMs. | ||
| * @param {string} fallbackContent - Text to use when the sent message has no `.content`. | ||
| * @param {import('discord.js').Message|import('discord.js').Message[]|null} sentMsg - Return value of safeSend. | ||
| */ | ||
| function logAssistantHistory(channelId, guildId, fallbackContent, sentMsg) { | ||
| const sentMessages = Array.isArray(sentMsg) ? sentMsg : [sentMsg]; | ||
| for (const m of sentMessages) { | ||
| if (!m?.id) continue; | ||
| addToHistory(channelId, 'assistant', m.content || fallbackContent, null, m.id, guildId || null); | ||
| } | ||
| } |
There was a problem hiding this comment.
History logging for assistant responses was added via logAssistantHistory/addToHistory, but existing triage-respond tests don’t assert that history is recorded (including the safeSend single-message vs array return cases). Adding a focused unit test that mocks addToHistory and verifies the expected calls (channelId, role, content, discordMessageId, guildId) would prevent regressions in this production-critical logging path.
Summary
Fix conversation logging to actually store messages in production and include
guild_id.Problem
addToHistory()was defined but never called in production codeguild_idcolumn existed but was never populatedtriage-respond.jssent AI responses without logging themChanges
guildIdparameter toaddToHistory(), update INSERTaddToHistory, call after sending AI responsesaddToHistory, call for user messages inaccumulateMessage()Files Changed
src/modules/ai.js— Add guildId param, update INSERT statementsrc/modules/triage-respond.js— Log AI responses after sendingsrc/modules/triage.js— Log user messages in accumulateMessage()tests/modules/ai.test.js— Update test expectationstests/modules/ai.coverage.test.js— Update test expectationsTesting
Related