fix: address PR #121 review comments post-merge#178
Conversation
|
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 (1)
📝 WalkthroughWalkthroughIntroduces Discord message ID tracking throughout the conversation system. Adds database migration for discord_message_id column, updates API routes to retrieve and return message IDs and channel names, extracts pagination logic as shared utility, extends AI history API with message ID persistence, and enhances frontend to display Discord links and channel information. 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 |
There was a problem hiding this comment.
Pull request overview
Follow-up fixes to the AI conversations viewer work (PR #121), addressing unresolved review feedback across backend, migrations, frontend UI, and tests—primarily improving query safety/ordering, reducing duplication, and enabling Discord “jump” links.
Changes:
- Backend: harden
fromdate fallback, reuseparsePagination, fetch newest conversations first (then reverse for grouping), and enrich detail responses withchannelName+messageUrl. - Migrations + persistence: add
discord_message_idcolumn and persist it viaaddToHistory(). - Frontend + tests: display channel name, show “View in Discord” links, and add comprehensive
escapeIliketests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/components/dashboard/conversation-replay.tsx | Adds optional channelName display and a “View in Discord” link per message via messageUrl. |
| web/src/app/dashboard/conversations/[conversationId]/page.tsx | Uses channelName when available and passes it into the replay component. |
| tests/utils/escapeIlike.test.js | New unit tests validating escaping for %, _, and \\ combinations. |
| tests/modules/ai.test.js | Updates addToHistory() test expectations for the new optional DB column value. |
| tests/api/routes/conversations.test.js | Adjusts mocks to reflect ORDER BY created_at DESC + reverse-before-grouping behavior. |
| src/utils/logQuery.js | Deduplicates ILIKE escaping by importing shared escapeIlike. |
| src/modules/ai.js | Extends addToHistory() to persist discord_message_id. |
| src/api/routes/guilds.js | Exports parsePagination for reuse in other route modules. |
| src/api/routes/conversations.js | Applies safer date fallback, changes ordering to DESC + reverse, and returns channelName + messageUrl in detail endpoint. |
| migrations/014_conversations_discord_message_id.cjs | Adds discord_message_id column with up/down migration. |
| migrations/012_flagged_messages.cjs | Adds a down() migration (but see review comment re: migration runner compatibility). |
💡 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
🤖 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/api/routes/conversations.js`:
- Around line 482-484: The detail endpoint sets channelName to null when the
channel name can't be resolved, causing inconsistent behavior with
buildConversationSummary which falls back to the channel ID; update the
assignment in the detail handler so channelName uses the resolved name or falls
back to anchor.channel_id (e.g., replace the null fallback with
anchor.channel_id) to match buildConversationSummary's behavior and ensure
consistent API responses.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
migrations/012_flagged_messages.cjsmigrations/014_conversations_discord_message_id.cjssrc/api/routes/conversations.jssrc/api/routes/guilds.jssrc/modules/ai.jssrc/utils/logQuery.jstests/api/routes/conversations.test.jstests/modules/ai.test.jstests/utils/escapeIlike.test.jsweb/src/app/dashboard/conversations/[conversationId]/page.tsxweb/src/components/dashboard/conversation-replay.tsx
📜 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 (7)
**/*.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.test.jssrc/api/routes/guilds.jssrc/modules/ai.jstests/utils/escapeIlike.test.jssrc/api/routes/conversations.jstests/api/routes/conversations.test.jssrc/utils/logQuery.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.test.jstests/utils/escapeIlike.test.jstests/api/routes/conversations.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/api/routes/guilds.jssrc/modules/ai.jssrc/api/routes/conversations.jssrc/utils/logQuery.js
src/api/routes/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Always use custom error classes from
src/utils/errors.jsand log errors with context before re-throwing
Files:
src/api/routes/guilds.jssrc/api/routes/conversations.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.js
migrations/*.cjs
📄 CodeRabbit inference engine (AGENTS.md)
Database migrations must use CommonJS (.cjs) format and follow naming convention
NNN_description.cjswhere NNN is a zero-padded sequence number
Files:
migrations/012_flagged_messages.cjsmigrations/014_conversations_discord_message_id.cjs
web/src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Component files should integrate with Zustand stores for state management (e.g., discord-entities store for caching Discord channels and roles per guild)
Files:
web/src/components/dashboard/conversation-replay.tsx
🧠 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 migrations/*.cjs : Database migrations must use CommonJS (.cjs) format and follow naming convention `NNN_description.cjs` where NNN is a zero-padded sequence number
Applied to files:
migrations/012_flagged_messages.cjsmigrations/014_conversations_discord_message_id.cjs
🧬 Code graph analysis (4)
src/api/routes/guilds.js (1)
src/api/routes/conversations.js (2)
parsePagination(140-140)parsePagination(330-330)
web/src/components/dashboard/conversation-replay.tsx (3)
src/api/routes/conversations.js (4)
channelName(114-114)channelName(482-483)channelId(48-48)msg(583-583)src/api/routes/guilds.js (1)
channelId(592-592)src/modules/ai.js (1)
channelId(276-276)
tests/utils/escapeIlike.test.js (1)
src/utils/escapeIlike.js (1)
escapeIlike(10-12)
src/api/routes/conversations.js (2)
src/api/routes/guilds.js (2)
from(84-84)from(106-106)src/utils/logQuery.js (1)
paramIndex(33-33)
🔇 Additional comments (18)
src/utils/logQuery.js (1)
10-10: Shared escaping utility import is a solid consolidation.Centralizing
escapeIlikeusage here keeps query escaping behavior consistent and easier to maintain.tests/utils/escapeIlike.test.js (1)
1-82: Escape-pattern test suite is comprehensive and well-targeted.Coverage across
%,_,\\, and mixed patterns gives strong confidence inescapeIlikebehavior.src/api/routes/guilds.js (1)
35-48: LGTM! Clean export of shared pagination utility.The
parsePaginationfunction is now properly exported for reuse inconversations.js, eliminating code duplication. The updated JSDoc accurately documents the return shape includingoffset.migrations/012_flagged_messages.cjs (1)
55-61: LGTM! Proper rollback migration with correct drop order.The
down()function correctly drops indexes before the table, maintaining proper dependency order. UsingIF EXISTSensures idempotency for safe re-runs. Based on learnings: "Database migrations must use CommonJS (.cjs) format" — this file correctly follows that convention.src/modules/ai.js (1)
207-228: LGTM! Clean backward-compatible addition of Discord message ID tracking.The new
discordMessageIdparameter is correctly placed as optional at the end of the signature, maintaining backward compatibility with existing callers. The|| nullcoalescing ensures NULL is stored for missing values, which aligns with the migration's nullable column definition.tests/modules/ai.test.js (1)
145-159: LGTM! Test correctly updated for new parameter.The test expectation now includes
nullas the fifth parameter, accurately reflecting thatdiscordMessageIddefaults tonullwhen not provided. This maintains proper test coverage for the DB write path.tests/api/routes/conversations.test.js (1)
299-337: LGTM! Test data correctly reflects new DESC-then-reverse ordering.The mock data now accurately simulates the database returning rows in
DESCorder (id:2 before id:1), matching the route's updated query behavior. The comment on lines 301-302 clearly explains the ordering logic, and assertions correctly expect the conversation anchor to beid: 1after reversal.migrations/014_conversations_discord_message_id.cjs (1)
1-23: LGTM! Clean migration for Discord message ID support.The migration correctly adds a nullable
TEXTcolumn for storing Discord snowflake IDs (which exceed JavaScript'sNumber.MAX_SAFE_INTEGER). BothupanddownuseIF EXISTS/IF NOT EXISTSfor idempotency. Based on learnings: "Database migrations must use CommonJS (.cjs) format and follow naming conventionNNN_description.cjs" — this file correctly follows both requirements.web/src/app/dashboard/conversations/[conversationId]/page.tsx (2)
12-18: LGTM! Interface correctly updated for optional channel name.The
channelNamefield is properly typed as optional and nullable (string | null), matching the API response shape where the field may be absent or explicitly null.
83-88: LGTM! Proper fallback display logic.Using nullish coalescing (
??) correctly falls back to the rawchannelIdwhenchannelNameisnullorundefined, ensuring the UI always displays something meaningful.web/src/components/dashboard/conversation-replay.tsx (2)
229-242: LGTM! Discord jump link with proper security and accessibility.The external link correctly implements:
target="_blank"withrel="noopener noreferrer"preventing reverse tabnabbing- Descriptive
aria-labelfor screen readers- Visible text label ("View in Discord") alongside the icon
136-152: LGTM! Header badge correctly displays channel name with fallback.The nullish coalescing on line 142 ensures the badge shows
channelNamewhen available, gracefully falling back tochannelIdotherwise.src/api/routes/conversations.js (6)
12-12: LGTM!Good consolidation - importing
parsePaginationfrom./guilds.jsremoves code duplication as noted in the PR objectives.
167-183: LGTM!The
fromFilterAppliedflag correctly tracks whether a validfromparameter was provided, and the 30-day default prevents unbounded queries whenfromis missing or invalid.
202-209: LGTM!Fetching DESC then reversing is a sound approach: it ensures the most recent 10k rows are captured when hitting the limit, while preserving the chronological order that
groupMessagesIntoConversationsexpects for proper time-gap grouping.
435-435: LGTM!Adding
discord_message_idto the SELECT aligns with the new migration and enables jump URLs in the response.
488-491: LGTM!The
messageUrlconstruction correctly builds Discord jump URLs using the standard format. The null fallback handles cases wherediscord_message_idwasn't stored.
497-497: LGTM!Adding
channelNameto the response fulfills the PR objective of displaying channel name instead of a sliced snowflake.
Docstrings generation was requested by @BillChirico. The following files were modified: * `migrations/012_flagged_messages.cjs` * `src/api/routes/guilds.js` * `src/modules/ai.js` * `web/src/components/dashboard/conversation-replay.tsx` These files were kept as they were: * `web/src/app/dashboard/conversations/[conversationId]/page.tsx` These files were ignored: * `tests/api/routes/conversations.test.js` * `tests/modules/ai.test.js` * `tests/utils/escapeIlike.test.js`
BillChirico
force-pushed
the
fix/pr121-followup
branch
from
8cd88de to
29c703a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| Filename | Overview |
|---|---|
| migrations/002_conversations_discord_message_id.cjs | Adds discord_message_id column with partial index for jump URLs; includes proper up/down migrations |
| src/api/routes/conversations.js | Improves date validation, extracts parsePagination (DRY), optimizes query ordering, adds channel name and message URL support |
| src/modules/ai.js | Adds optional discordMessageId parameter to addToHistory for jump URL support; backward compatible |
| web/src/components/dashboard/conversation-replay.tsx | Adds clickable Discord jump links with proper accessibility attributes and channel name display |
Last reviewed commit: 0d00f5d
- Fix migration comment (014 → 002) - Update limit comment to match actual LIMIT 10000 - Consistent channelName fallback (null instead of channelId) - Update test expectations for 5th discordMessageId parameter
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/api/routes/conversations.js:120
buildConversationSummary()now setschannelNametonullwhen the channel isn’t present in the guild channel cache. The dashboard conversations list (web/src/app/dashboard/conversations/page.tsx) typeschannelNameas a required string and renders it directly, so a cache miss will produce an empty channel column (and violates the current API contract). Consider restoring the previous fallback toconvo.channelId(or updating the list UI + types to handlenullconsistently).
const channelName = guild?.channels?.cache?.get(convo.channelId)?.name || null;
return {
id: convo.id,
channelId: convo.channelId,
channelName,
participants: Array.from(participantMap.values()),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/routes/conversations.js (1)
114-120:⚠️ Potential issue | 🟠 Major
channelNamecan now benull, but response contracts still appear string-only.Line 114 changed the fallback to
null. If OpenAPI/consumer types still definechannelNameasstring, this is a breaking contract drift for typed clients. Please either (a) make the schema/type nullable, or (b) keep a string fallback consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/routes/conversations.js` around lines 114 - 120, The response now sets channelName = guild?.channels?.cache?.get(convo.channelId)?.name || null which can return null but the API contract likely expects a string; either make the OpenAPI/type definition nullable for channelName or preserve a string fallback. Fix by updating the channelName assignment in the conversations response (variable channelName) to use a consistent string fallback (e.g., empty string) if you choose to keep string-only, or update the OpenAPI/schema and any TypeScript types that reference the conversations response (response DTO/interface used by this route) to mark channelName as string | null so consumers and generated clients remain in sync.web/src/components/dashboard/conversation-replay.tsx (1)
149-150:⚠️ Potential issue | 🟡 MinorRemove unintended
~character before token count.Line 149 currently renders a literal tilde before
{tokenEstimate.toLocaleString()}, causing a visible UI artifact.Suggested fix
- <Zap className="h-3 w-3" />~{tokenEstimate.toLocaleString()} tokens + <Zap className="h-3 w-3" />{tokenEstimate.toLocaleString()} tokens🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/dashboard/conversation-replay.tsx` around lines 149 - 150, Remove the unintended literal tilde character rendered before the token count in the conversation-replay component: in the JSX where <Zap className="h-3 w-3" />~{tokenEstimate.toLocaleString()} is used (inside the Badge), delete the "~" so it renders <Zap ... /> followed immediately by {tokenEstimate.toLocaleString()} and the "tokens" text; ensure spacing is correct around the expression so the token count displays cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/002_conversations_discord_message_id.cjs`:
- Around line 1-33: The migration file is mis-numbered as 002; rename the file
to the next sequence number (e.g., 014_conversations_discord_message_id.cjs)
following the NNN_description.cjs convention (zero-padded three-digit prefix and
.cjs CommonJS extension) so it runs in order, leaving the existing exports.up
and exports.down SQL contents unchanged and update any migration
manifest/registration if your system references the filename elsewhere.
In `@src/api/routes/conversations.js`:
- Around line 819-821: The formatting around the channelName assignment is off;
run the project formatter (e.g., prettier/biome) and commit the formatted
changes so the block using
req.guild?.channels?.cache?.get(anchor.channel_id)?.name || null (variable
channelName) conforms to the repo's style rules; ensure you format the
surrounding file src/api/routes/conversations.js and re-run CI to verify the
formatting warnings are resolved.
In `@web/src/components/dashboard/conversation-replay.tsx`:
- Around line 36-40: Remove the channelName prop from ConversationReplayProps
and stop prop-drilling it; instead import and use the discord-entities Zustand
store (e.g., useDiscordEntitiesStore or the selector that returns channels by
guildId+channelId) inside ConversationReplay to resolve the channel object/name
from channelId (and guildId — add a guildId prop or derive it from messages if
available). Update all usages in this file (references around lines 76-84 and
142-143) to read channel.displayName or channel.name from the store result,
handle missing channel gracefully, and update the prop/interface definition
(ConversationReplayProps) and callers to remove channelName.
---
Outside diff comments:
In `@src/api/routes/conversations.js`:
- Around line 114-120: The response now sets channelName =
guild?.channels?.cache?.get(convo.channelId)?.name || null which can return null
but the API contract likely expects a string; either make the OpenAPI/type
definition nullable for channelName or preserve a string fallback. Fix by
updating the channelName assignment in the conversations response (variable
channelName) to use a consistent string fallback (e.g., empty string) if you
choose to keep string-only, or update the OpenAPI/schema and any TypeScript
types that reference the conversations response (response DTO/interface used by
this route) to mark channelName as string | null so consumers and generated
clients remain in sync.
In `@web/src/components/dashboard/conversation-replay.tsx`:
- Around line 149-150: Remove the unintended literal tilde character rendered
before the token count in the conversation-replay component: in the JSX where
<Zap className="h-3 w-3" />~{tokenEstimate.toLocaleString()} is used (inside the
Badge), delete the "~" so it renders <Zap ... /> followed immediately by
{tokenEstimate.toLocaleString()} and the "tokens" text; ensure spacing is
correct around the expression so the token count displays cleanly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
migrations/002_conversations_discord_message_id.cjssrc/api/routes/conversations.jssrc/api/routes/guilds.jssrc/modules/ai.jssrc/utils/logQuery.jstests/api/routes/conversations.test.jstests/modules/ai.coverage.test.jstests/modules/ai.test.jstests/utils/escapeIlike.test.jsweb/src/app/dashboard/conversations/[conversationId]/page.tsxweb/src/components/dashboard/conversation-replay.tsx
📜 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 (7)
**/*.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.test.jstests/api/routes/conversations.test.jssrc/utils/logQuery.jssrc/api/routes/guilds.jssrc/api/routes/conversations.jssrc/modules/ai.jstests/modules/ai.coverage.test.jstests/utils/escapeIlike.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.test.jstests/api/routes/conversations.test.jstests/modules/ai.coverage.test.jstests/utils/escapeIlike.test.js
migrations/*.cjs
📄 CodeRabbit inference engine (AGENTS.md)
Database migrations must use CommonJS (.cjs) format and follow naming convention
NNN_description.cjswhere NNN is a zero-padded sequence number
Files:
migrations/002_conversations_discord_message_id.cjs
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/utils/logQuery.jssrc/api/routes/guilds.jssrc/api/routes/conversations.jssrc/modules/ai.js
src/api/routes/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Always use custom error classes from
src/utils/errors.jsand log errors with context before re-throwing
Files:
src/api/routes/guilds.jssrc/api/routes/conversations.js
web/src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Component files should integrate with Zustand stores for state management (e.g., discord-entities store for caching Discord channels and roles per guild)
Files:
web/src/components/dashboard/conversation-replay.tsx
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.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 migrations/*.cjs : Database migrations must use CommonJS (.cjs) format and follow naming convention `NNN_description.cjs` where NNN is a zero-padded sequence number
Applied to files:
migrations/002_conversations_discord_message_id.cjs
🧬 Code graph analysis (4)
src/api/routes/guilds.js (1)
src/api/routes/conversations.js (2)
parsePagination(248-248)parsePagination(593-593)
web/src/components/dashboard/conversation-replay.tsx (3)
src/api/routes/conversations.js (3)
channelName(114-114)channelName(819-820)channelId(48-48)src/api/routes/guilds.js (1)
channelId(942-942)src/modules/ai.js (1)
channelId(276-276)
src/modules/ai.js (3)
src/api/routes/conversations.js (1)
channelId(48-48)src/api/routes/guilds.js (1)
channelId(942-942)src/modules/triage.js (1)
channelId(561-561)
tests/utils/escapeIlike.test.js (1)
src/utils/escapeIlike.js (1)
escapeIlike(10-12)
🪛 GitHub Actions: CI
src/api/routes/conversations.js
[error] 817-821: Biome formatter would have applied formatting changes. Formatting issue detected at lines 817-821 (expected whitespace/indentation changes).
🔇 Additional comments (12)
src/api/routes/guilds.js (1)
35-48:parsePaginationexport is clean and safe.The normalization and clamping behavior is consistent and suitable for shared use across routes.
tests/utils/escapeIlike.test.js (1)
4-82: Great coverage forescapeIlikeedge cases.The suite exercises plain input plus all wildcard/escape combinations thoroughly.
src/utils/logQuery.js (1)
10-10: Nice deduplication: shared escaping utility is the right direction.Importing
escapeIlikefrom a single source avoids drift across query builders.src/modules/ai.js (1)
198-229: History persistence update looks correct.Adding
discord_message_idto the write-through path is consistent with the new schema and keeps failure handling non-blocking.tests/modules/ai.coverage.test.js (1)
188-189: Test expectation update is correct.Including the extra trailing
nullkeeps this assertion aligned with the newdiscord_message_idinsert parameter.tests/modules/ai.test.js (1)
157-158: Assertion now matches the new DB write contract.Good update to keep the test synchronized with
addToHistoryinserts.src/api/routes/conversations.js (3)
275-291: Good safeguard against unbounded conversation scans.Applying a 30-day default when
fromis missing/invalid is a strong defensive change.
304-317: DESC fetch + reverse is a solid approach for bounded “most-recent” grouping.This preserves chronological grouping input while prioritizing newest rows under the 10k cap.
772-829: Discord jump URL enrichment is implemented cleanly.Including
discordMessageIdand derivingmessageUrlonly when data is present is correct and safe.tests/api/routes/conversations.test.js (1)
301-320: DESC-order fixture update matches route behavior.This test change correctly models
ORDER BY ... DESCretrieval while preserving oldest-message anchoring after reversal.web/src/components/dashboard/conversation-replay.tsx (1)
230-242: Discord jump-link rendering looks solid.Conditional rendering plus
target="_blank"andrel="noopener noreferrer"are correctly implemented.web/src/app/dashboard/conversations/[conversationId]/page.tsx (1)
15-16: Channel-name propagation and fallback rendering are coherent.The interface, header display (
channelName ?? channelId), and prop wiring stay aligned across this page.Also applies to: 85-86, 119-123
This PR addresses the review comments from #121 that were unresolved at merge time.
Fixes
Backend
fromparam is invalid (prevents unbounded queries)channelNameto response using guild channel cachediscord_message_idcolumn via migration, updatedaddToHistory(), returnmessageUrlin APIMigration
down()to012_flagged_messages.cjsdiscord_message_idcolumn for jump URLsFrontend
messageUrlto interface + clickable link UITests & Utilities
Test Results
All 113 test files pass (2408 tests).