feat: AI response feedback via 👍👎 reactions (#182)#184
Closed
BillChirico wants to merge 7 commits intomainfrom
Closed
feat: AI response feedback via 👍👎 reactions (#182)#184BillChirico wants to merge 7 commits intomainfrom
BillChirico wants to merge 7 commits intomainfrom
Conversation
- Add ai_feedback table migration (003_ai_feedback.cjs) - Add aiFeedback.js module with recordFeedback, getFeedbackStats, getFeedbackTrend - Register AI message IDs in-memory (LRU-capped at 2000) for reaction filtering - Add 👍👎 reactions to first chunk of AI responses (opt-in via ai.feedback.enabled) - Handle feedback reactions in registerReactionHandlers (events.js) - Add /api/v1/guilds/:id/ai-feedback/stats and /recent endpoints - Add AiFeedbackStats dashboard component with pie chart + bar trend - Set ai.feedback.enabled: false (default opt-in) in config.json - 26 new tests (unit + API) all passing - Lint clean (1 pre-existing Biome noArrayIndexKey warning only) Closes #182
Contributor
|
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 (8)
📝 WalkthroughWalkthroughImplements AI response feedback tracking system enabling users to upvote/downvote bot responses using 👍/👎 reactions. Introduces database schema, API endpoints for feedback statistics, event handlers for capturing reactions, and dashboard visualization component. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an AI response feedback feature that lets users rate AI-generated Discord messages with 👍/👎, persists that feedback, exposes aggregated stats via API endpoints, and renders a dashboard card showing split + approval ratio + trend (opt-in per guild via config).
Changes:
- Add automatic 👍/👎 reactions to the first chunk of AI triage responses and record reactions as feedback.
- Introduce AI feedback persistence + stats/recent API endpoints (with tests).
- Add a dashboard component to visualize feedback stats (pie + daily trend).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/components/dashboard/ai-feedback-stats.tsx | New dashboard card fetching and charting AI feedback stats. |
| src/modules/triage-respond.js | Adds helper to react to AI responses with 👍/👎 and registers message IDs for tracking. |
| src/modules/events.js | Records 👍/👎 reaction adds as AI feedback (behind per-guild config). |
| src/modules/aiFeedback.js | New module for feedback recording + stats/trend queries + in-memory AI message registry. |
| src/api/routes/ai-feedback.js | New REST endpoints for stats and recent feedback, with rate limiting + authz. |
| src/api/index.js | Mounts the ai-feedback router under guild routes. |
| migrations/003_ai_feedback.cjs | Adds ai_feedback table and indexes. |
| tests/modules/aiFeedback.test.js | Unit tests for aiFeedback module behavior. |
| tests/api/routes/ai-feedback.test.js | API route tests for stats and recent endpoints. |
| config.json | Adds ai.feedback.enabled config flag (default false) and normalizes some Unicode characters. |
💡 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config.json`:
- Around line 13-15: Add documentation for the new config key
ai.feedback.enabled to the README.md config reference: describe the ai.feedback
section, list the ai.feedback.enabled boolean with its default false, explain
its effect/behavior and any related runtime implications, and include an example
snippet showing how to enable it; update the config reference section so readers
can find ai.feedback and ai.feedback.enabled alongside other top-level keys from
config.json.
In `@migrations/003_ai_feedback.cjs`:
- Around line 18-19: The migration leaves feedback_type and created_at nullable;
update the migration (migrations/003_ai_feedback.cjs) to make feedback_type TEXT
NOT NULL with the CHECK (feedback_type IN ('positive','negative')) and
created_at TIMESTAMP NOT NULL DEFAULT NOW(); before adding NOT NULL constraints,
add backfill statements to set a safe default for existing rows (e.g., UPDATE
... SET feedback_type = 'positive' WHERE feedback_type IS NULL and set
created_at = NOW() WHERE created_at IS NULL) so the ALTER TABLE to add NOT NULL
will succeed.
In `@src/api/routes/ai-feedback.js`:
- Around line 104-125: The route duplicates SQL already implemented in the
aiFeedback module; replace the inline queries in this route with calls to the
existing functions getFeedbackStats and getFeedbackTrend from
src/modules/aiFeedback.js (use the same guildId and days params), remove the
duplicated dbPool.query blocks, and return the results from those module
functions instead so all SQL/logic lives in aiFeedback.js.
- Around line 87-149: The /stats route handler currently returns inline
res.status(...).json(...) responses on DB-unavailable and on catch; instead,
import and throw the appropriate custom errors from src/utils/errors.js (e.g.
ServiceUnavailableError for missing dbPool and a InternalServerError/ApiError
for failures) after calling logError with context (use logError('Failed to fetch
AI feedback stats', { error: err.message, guild: guildId }) style), so the
centralized error middleware handles the actual response; update the
router.get('/stats'...) handler to remove direct res.status... calls and throw
the custom errors (also apply the same change pattern to the other handler block
noted in lines 215-255).
- Around line 232-249: The /recent handler in src/api/routes/ai-feedback.js
queries only ai_feedback so the response lacks the AI message text; update the
SQL in the dbPool.query call to join the messages table (or the table that
stores the AI response text) on ai_feedback.message_id and select the message
content (e.g., messages.content AS message_text), keep the existing WHERE
guild_id and LIMIT params, then add a messageContent (or messageText) field in
the result.rows.map callback (e.g., messageContent: r.message_text) so the JSON
response includes the AI response text along with id, messageId, channelId,
userId, feedbackType, and createdAt.
In `@src/modules/aiFeedback.js`:
- Around line 93-186: The three per-request functions recordFeedback,
getFeedbackStats, and getFeedbackTrend must call getConfig(guildId) at the start
of each invocation and short-circuit if the guild config disables AI feedback;
add a call like const cfg = await getConfig(guildId) (or equivalent) in each
function, check cfg?.ai?.feedback?.enabled (or the appropriate flag) and return
early (no DB queries) when disabled, and ensure getConfig is imported/available;
do not convert these to long-lived state — use onConfigChange only for stateful
resources elsewhere if you need reactive updates.
- Around line 27-33: The current LRU logic evicts the oldest ID even when
re-registering an existing messageId; change aiMessageIds handling so that if
aiMessageIds.has(messageId) you refresh its recency by deleting then re-adding
it (aiMessageIds.delete(messageId); aiMessageIds.add(messageId);) and return;
otherwise, when aiMessageIds.size >= AI_MESSAGE_ID_LIMIT delete the oldest (as
current) and then add the new messageId; this preserves tracked IDs and
implements proper LRU behavior for aiMessageIds.
In `@src/modules/events.js`:
- Around line 300-319: The reaction-remove path currently lacks reconciliation
for AI feedback so removing a 👍/👎 leaves stale DB entries; add logic in the
MessageReactionRemove handler (mirror of the block using isAiMessage,
FEEDBACK_EMOJI, and channelId resolution) to detect positive/negative emoji
removals and then either call a new deleteFeedback/removeFeedback function or
call recordFeedback with a neutral/removed flag so the DB record is
deleted/updated accordingly; ensure you reuse isAiMessage(reaction.message.id),
FEEDBACK_EMOJI checks, the same channelId/userId/guildId extraction, and handle
promise errors the same way.
In `@web/src/components/dashboard/ai-feedback-stats.tsx`:
- Around line 41-71: This component currently uses local useState/useCallback
(stats, loading, error, fetchStats, setStats, setLoading, setError) and must
instead read/update a shared Zustand store; replace the local state and fetch
logic with calls to a zustand-backed store (e.g., an aiFeedbackStore or extend
an existing store) that exposes stats, loading, error, and a fetchStats method,
wire the component to consume those selectors instead of useState/useCallback,
and update the fetch side-effect to call the store.fetchStats in useEffect so
the data is cached and shared across components.
- Around line 21-31: The inline FeedbackStats interface in ai-feedback-stats.tsx
should be moved into the shared analytics types module as an exported type so UI
and API response models share a single contract; create an exported
FeedbackStats type in the central analytics types file, update
ai-feedback-stats.tsx to import and use that shared FeedbackStats (instead of
the inline definition), and ensure any API response typings also reference the
new shared FeedbackStats to avoid drift; run the TypeScript checks and fix any
import/usage mismatches.
- Around line 40-53: The code destructures useGuildSelection() as {
selectedGuild, apiBase } but the hook actually returns a string | null (the
guild ID), so fix either by changing the hook or the component: replace the
destructuring with a single guildId variable (e.g., const guildId =
useGuildSelection()) and update fetchStats to check guildId and call
fetch(.../guilds/${guildId}/ai-feedback/stats?days=30) instead of
selectedGuild.id; also remove apiBase usage or supply a real base URL (e.g.,
from an existing useApiBase hook or config) so fetch has a valid base — update
references in fetchStats and anywhere else (e.g., the later usage at the second
mention) to use guildId and the chosen api base.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
config.jsonmigrations/003_ai_feedback.cjssrc/api/index.jssrc/api/routes/ai-feedback.jssrc/modules/aiFeedback.jssrc/modules/events.jssrc/modules/triage-respond.jstests/api/routes/ai-feedback.test.jstests/modules/aiFeedback.test.jsweb/src/components/dashboard/ai-feedback-stats.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). (3)
- GitHub Check: Greptile Review
- GitHub Check: Agent
- GitHub Check: Docker Build Validation
🧰 Additional context used
📓 Path-based instructions (8)
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/003_ai_feedback.cjs
**/*.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/api/index.jstests/api/routes/ai-feedback.test.jssrc/api/routes/ai-feedback.jssrc/modules/events.jstests/modules/aiFeedback.test.jssrc/modules/triage-respond.jssrc/modules/aiFeedback.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/index.jssrc/api/routes/ai-feedback.jssrc/modules/events.jssrc/modules/triage-respond.jssrc/modules/aiFeedback.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/api/routes/ai-feedback.test.jstests/modules/aiFeedback.test.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/ai-feedback.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/events.jssrc/modules/triage-respond.jssrc/modules/aiFeedback.js
config.json
📄 CodeRabbit inference engine (AGENTS.md)
When adding a new config section or key, document it in README.md's config reference section
Files:
config.json
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/ai-feedback-stats.tsx
🧠 Learnings (3)
📚 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/003_ai_feedback.cjs
📚 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 web/src/**/*.ts : Use TypeScript with type safety. Share contracts between dashboard UI and API responses via `web/src/types/analytics.ts` and similar type definition files
Applied to files:
web/src/components/dashboard/ai-feedback-stats.tsx
📚 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/aiFeedback.js
🧬 Code graph analysis (8)
src/api/index.js (2)
src/api/routes/ai-feedback.js (1)
router(13-13)src/api/middleware/auth.js (1)
requireAuth(36-70)
tests/api/routes/ai-feedback.test.js (2)
src/api/routes/ai-feedback.js (2)
req(88-88)req(216-216)src/api/server.js (1)
createApp(28-91)
src/api/routes/ai-feedback.js (4)
src/api/middleware/rateLimit.js (1)
rateLimit(15-61)src/api/routes/guilds.js (1)
validateGuild(302-312)src/index.js (1)
dbPool(365-365)src/modules/aiFeedback.js (7)
row(140-140)positive(141-141)negative(142-142)total(143-143)ratio(144-144)result(130-138)result(164-175)
src/modules/events.js (1)
src/modules/aiFeedback.js (4)
isAiMessage(40-42)FEEDBACK_EMOJI(10-13)FEEDBACK_EMOJI(10-13)recordFeedback(93-118)
tests/modules/aiFeedback.test.js (1)
src/modules/aiFeedback.js (8)
clearAiMessages(47-49)setPool(71-73)_setPoolGetter(60-62)registerAiMessage(26-33)isAiMessage(40-42)recordFeedback(93-118)getFeedbackStats(125-151)getFeedbackTrend(159-186)
src/modules/triage-respond.js (1)
src/modules/aiFeedback.js (4)
first(29-29)registerAiMessage(26-33)FEEDBACK_EMOJI(10-13)FEEDBACK_EMOJI(10-13)
web/src/components/dashboard/ai-feedback-stats.tsx (1)
web/src/hooks/use-guild-selection.ts (1)
useGuildSelection(12-54)
src/modules/aiFeedback.js (3)
src/db.js (1)
getPool(314-319)src/logger.js (2)
warn(238-240)info(231-233)src/api/routes/ai-feedback.js (4)
guildId(93-93)guildId(221-221)result(232-239)row(128-128)
🪛 GitHub Check: CodeQL
src/api/index.js
[failure] 41-41: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
src/api/routes/ai-feedback.js
[failure] 87-87: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 215-215: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
🔇 Additional comments (4)
src/api/index.js (1)
40-42: Route mount looks correct and consistently protected.Good placement and middleware chain (
requireAuth+auditLogMiddleware) for the new guild-scoped AI feedback routes.src/modules/triage-respond.js (1)
27-46: Nice non-blocking integration for feedback reactions.The first-chunk-only strategy plus fire-and-forget reaction adds keeps response latency safe while enabling feedback capture.
Also applies to: 276-277
tests/api/routes/ai-feedback.test.js (1)
73-156: Great route-level coverage for/statsand/recent.The suite exercises auth, bounds/defaults, DB unavailability, and error paths thoroughly.
Also applies to: 160-222
tests/modules/aiFeedback.test.js (1)
59-119: Module test coverage looks solid.Good job covering both happy paths and failure/no-pool fallbacks for
recordFeedback, stats, and trend logic.Also applies to: 123-195
|
| Filename | Overview |
|---|---|
| migrations/003_ai_feedback.cjs | Creates ai_feedback table with proper indexes, parameterized queries, and UNIQUE constraint for deduplication |
| src/modules/aiFeedback.js | Implements feedback tracking with LRU cache (2000 limit), parameterized queries, proper error handling; minor design limitation with dual reactions |
| src/api/routes/ai-feedback.js | Two endpoints (/stats, /recent) with proper auth, rate limiting, parameterized queries, and OpenAPI documentation |
| src/modules/events.js | Integrates feedback handlers into reaction events with config checks and fire-and-forget pattern; dual-reaction edge case exists |
| web/src/components/dashboard/ai-feedback-stats.tsx | React dashboard component with pie chart and trend graph, proper TypeScript types, loading/error states, and credentials handling |
Sequence Diagram
sequenceDiagram
participant User
participant Discord
participant Bot
participant aiFeedback
participant Database
participant Dashboard
Note over User,Dashboard: AI Response Flow
User->>Discord: Mentions bot or replies
Discord->>Bot: MessageCreate event
Bot->>Bot: AI generates response
Bot->>aiFeedback: registerAiMessage(messageId)
Bot->>Discord: Send AI response
Bot->>Discord: Add 👍 and 👎 reactions
Note over User,Dashboard: Feedback Recording Flow
User->>Discord: Reacts with 👍 or 👎
Discord->>Bot: MessageReactionAdd event
Bot->>aiFeedback: isAiMessage(messageId)?
aiFeedback-->>Bot: true
Bot->>aiFeedback: recordFeedback(messageId, userId, type)
aiFeedback->>Database: INSERT ... ON CONFLICT UPDATE
Database-->>aiFeedback: OK
Note over User,Dashboard: Feedback Removal Flow
User->>Discord: Removes reaction
Discord->>Bot: MessageReactionRemove event
Bot->>aiFeedback: deleteFeedback(messageId, userId)
aiFeedback->>Database: DELETE FROM ai_feedback
Database-->>aiFeedback: OK
Note over User,Dashboard: Dashboard Stats Flow
Dashboard->>Bot: GET /guilds/:id/ai-feedback/stats
Bot->>Database: Query aggregate stats + trend
Database-->>Bot: Results
Bot-->>Dashboard: JSON response (positive, negative, ratio, trend)
Last reviewed commit: 49af040
When a user removes their 👍/👎 reaction, delete the corresponding ai_feedback row so analytics stay accurate. - Import deleteFeedback from aiFeedback.js - Add AI feedback deletion block in MessageReactionRemove handler, mirroring the existing MessageReactionAdd pattern - Gate on guildConfig.ai.feedback.enabled and isAiMessage check - Runs independently of starboard (no early return before feedback check) Resolves: PRRT_kwDORICdSM5xcZ7Q, PRRT_kwDORICdSM5xcaNG
… index
- feedback_type: add NOT NULL (NULL was previously passable through CHECK)
- created_at: change TIMESTAMP -> TIMESTAMPTZ NOT NULL (rest of schema uses TIMESTAMPTZ)
- updated_at: add TIMESTAMPTZ column (used by ON CONFLICT to avoid clobbering created_at)
- Down migration: remove escaped backticks (`) that caused syntax errors in pgm.sql()
- Add composite index idx_ai_feedback_guild_created on (guild_id, created_at)
to support the trend query that filters by guild_id + time window
Resolves: PRRT_kwDORICdSM5xcZ7g, PRRT_kwDORICdSM5xcZ7Y, PRRT_kwDORICdSM5xcaM2,
PRRT_kwDORICdSM5xcbZb, PRRT_kwDORICdSM5xcZ7k
…om db.js
The internal setPool/_setPoolGetter pattern was never wired on startup,
so no pool was ever injected. Replace with import { getPool } from '../db.js'
which is the shared pool used by all other modules.
- Remove setPool, _setPoolGetter, _setPool exports
- Import getPool from ../db.js directly
- Wrap getPool() calls in try/catch (db.js throws if not yet initialized)
matching the pattern used by debugFooter.js and logQuery.js
- Update tests: mock db.js instead of using setPool/DI helpers
- Update tests: mock config.js (needed for per-request config check in fix 9)
Also includes related fixes in this commit:
- Fix 5: ON CONFLICT no longer sets created_at = NOW(); sets updated_at instead
- Fix 6: registerAiMessage skips eviction if messageId is already tracked
- Fix 9: recordFeedback/deleteFeedback re-check guild config on each call
- Add deleteFeedback export (used by events.js MessageReactionRemove handler)
- Add tests for deleteFeedback, config-disabled paths, and LRU eviction
Resolves: PRRT_kwDORICdSM5xcZ7u, PRRT_kwDORICdSM5xcZ7y, PRRT_kwDORICdSM5xcaM_,
PRRT_kwDORICdSM5xcaNE
…content /recent previously returned only ai_feedback rows with no message text. JOIN with conversations on discord_message_id = ai_feedback.message_id (role = 'assistant') to include the original AI response content. - LEFT JOIN so rows without a matching conversation still appear - Add aiResponseContent field to response (null if no match) Resolves: PRRT_kwDORICdSM5xcaM-
Add table entry for ai.feedback.enabled in the AI Chat config reference. Resolves: PRRT_kwDORICdSM5xcaM1
FeedbackStats was defined inline in ai-feedback-stats.tsx. Move it to a shared types file so it can be reused by other components/routes. - Create web/src/types/api.ts with FeedbackStats, FeedbackTrendPoint, and RecentFeedbackEntry (covers the /recent endpoint response shape) - Import FeedbackStats from @/types/api in the dashboard component Resolves: PRRT_kwDORICdSM5xcaNI
Collaborator
Author
|
Superseded by #190 which resolves all review comments and merge conflicts. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+348
to
+360
| // AI feedback removal — delete DB record when user removes 👍/👎 | ||
| if (guildConfig.ai?.feedback?.enabled && isAiMessage(reaction.message.id)) { | ||
| const emoji = reaction.emoji.name; | ||
| const isFeedbackEmoji = | ||
| emoji === FEEDBACK_EMOJI.positive || emoji === FEEDBACK_EMOJI.negative; | ||
|
|
||
| if (isFeedbackEmoji) { | ||
| deleteFeedback({ | ||
| messageId: reaction.message.id, | ||
| guildId, | ||
| userId: user.id, | ||
| }).catch(() => {}); | ||
| } |
There was a problem hiding this comment.
dual-reaction edge case: if user has both 👍 and 👎 active, removing either deletes the entire feedback record while the other reaction remains visible
Discord allows multiple reactions per user, but the database schema (UNIQUE(message_id, user_id)) enforces one feedback type per user. Consider checking if the user still has the opposite reaction before deleting, or document this limitation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/modules/events.js
Line: 348-360
Comment:
dual-reaction edge case: if user has both 👍 and 👎 active, removing either deletes the entire feedback record while the other reaction remains visible
Discord allows multiple reactions per user, but the database schema (`UNIQUE(message_id, user_id)`) enforces one feedback type per user. Consider checking if the user still has the opposite reaction before deleting, or document this limitation.
How can I resolve this? If you propose a fix, please make it concise.
BillChirico
added a commit
that referenced
this pull request
Mar 2, 2026
…eviction The aiMessageIds Set is memory-only (cleared on restart) and capped at 2 000 entries. Add explicit JSDoc explaining the trade-off and a warn() log when the cap is hit so operators can see it in logs if it becomes a concern. Addresses PR #184 review comment on src/modules/aiFeedback.js:37.
BillChirico
added a commit
that referenced
this pull request
Mar 2, 2026
- Create src/api/errors.js with send400/401/403/404/500/503 helpers - /stats route now delegates to getFeedbackStats() + getFeedbackTrend() from aiFeedback.js instead of duplicating SQL queries - Both /stats and /recent use send503/send500 from errors.js instead of inline res.status().json() calls Addresses PR #184 review comments on: src/api/routes/ai-feedback.js:126 (DRY) src/api/routes/ai-feedback.js:149 (centralized errors)
BillChirico
added a commit
that referenced
this pull request
Mar 2, 2026
…Stats hook
CRITICAL FIX: useGuildSelection() returns string|null but the component
was destructuring { selectedGuild, apiBase } from it — both always
undefined, so the component silently rendered nothing and never fetched
data.
Fixes:
- Extract useAiFeedbackStats hook (web/src/hooks/use-ai-feedback-stats.ts)
following the existing use-moderation-stats.ts pattern
- Use AbortController via useRef to cancel in-flight requests on guild
change or component unmount — prevents stale-state races
- Component now calls useGuildSelection() correctly (string|null)
- URL follows project convention: /api/guilds/:id/ai-feedback/stats
- State management moved to the dedicated hook (addresses refactor
suggestion for better separation of concerns)
Addresses PR #184 review comments:
web/src/components/dashboard/ai-feedback-stats.tsx:42 (CRITICAL)
web/src/components/dashboard/ai-feedback-stats.tsx:60 (state mgmt)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements AI response feedback system with thumbs up/down reactions.
Changes
Testing
Closes #182