fix: AI response feedback review comments #190
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||
| /** | ||||||
| * Migration 003: AI Response Feedback Table | ||||||
| * | ||||||
| * Stores 👍/👎 reactions from users on AI-generated messages. | ||||||
| * Per-user per-message deduplication via UNIQUE constraint. | ||||||
| * Gated behind ai.feedback.enabled in config (opt-in per guild). | ||||||
| */ | ||||||
|
|
||||||
| /** @param {import('node-pg-migrate').MigrationBuilder} pgm */ | ||||||
| exports.up = (pgm) => { | ||||||
| pgm.sql(` | ||||||
| CREATE TABLE IF NOT EXISTS ai_feedback ( | ||||||
| id SERIAL PRIMARY KEY, | ||||||
| message_id TEXT NOT NULL, | ||||||
| channel_id TEXT NOT NULL, | ||||||
| guild_id TEXT NOT NULL, | ||||||
| user_id TEXT NOT NULL, | ||||||
| feedback_type TEXT NOT NULL CHECK (feedback_type IN ('positive', 'negative')), | ||||||
| created_at TIMESTAMP NOT NULL DEFAULT NOW(), | ||||||
|
There was a problem hiding this comment. This migration uses
Suggested change
|
||||||
| UNIQUE(message_id, user_id) | ||||||
| ) | ||||||
| `); | ||||||
|
|
||||||
| pgm.sql(` | ||||||
| CREATE INDEX IF NOT EXISTS idx_ai_feedback_guild_id | ||||||
| ON ai_feedback(guild_id) | ||||||
| `); | ||||||
|
|
||||||
| pgm.sql(` | ||||||
| CREATE INDEX IF NOT EXISTS idx_ai_feedback_message_id | ||||||
| ON ai_feedback(message_id) | ||||||
| `); | ||||||
|
Comment on lines
+24
to
+32
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial Add a composite index for trend/stats query paths. Current indexes don’t optimize ♻️ Proposed migration addition pgm.sql(`
CREATE INDEX IF NOT EXISTS idx_ai_feedback_message_id
ON ai_feedback(message_id)
`);
+
+ pgm.sql(`
+ CREATE INDEX IF NOT EXISTS idx_ai_feedback_guild_created_at
+ ON ai_feedback(guild_id, created_at DESC)
+ `);🤖 Prompt for AI Agents |
||||||
| }; | ||||||
|
|
||||||
| /** @param {import('node-pg-migrate').MigrationBuilder} pgm */ | ||||||
| exports.down = (pgm) => { | ||||||
| pgm.sql(`DROP INDEX IF EXISTS idx_ai_feedback_message_id`); | ||||||
| pgm.sql(`DROP INDEX IF EXISTS idx_ai_feedback_guild_id`); | ||||||
| pgm.sql(`DROP TABLE IF EXISTS ai_feedback`); | ||||||
| }; | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| /** | ||
| * AI Feedback Routes | ||
| * Endpoints for reading AI response feedback (👍/👎) stats. | ||
| * | ||
| * Mounted at /api/v1/guilds/:id/ai-feedback | ||
| */ | ||
|
|
||
| import { Router } from 'express'; | ||
| import { error as logError } from '../../logger.js'; | ||
| import { getFeedbackStats, getFeedbackTrend, getRecentFeedback } from '../../modules/aiFeedback.js'; | ||
| import { rateLimit } from '../middleware/rateLimit.js'; | ||
| import { requireGuildAdmin, validateGuild } from './guilds.js'; | ||
|
|
||
| const router = Router({ mergeParams: true }); | ||
|
|
||
| /** Rate limiter: 60 requests / 1 min per IP */ | ||
| const feedbackRateLimit = rateLimit({ windowMs: 60 * 1000, max: 60 }); | ||
|
|
||
| // ── GET /stats ───────────────────────────────────────────────────────────── | ||
|
|
||
| /** | ||
| * @openapi | ||
| * /guilds/{id}/ai-feedback/stats: | ||
| * get: | ||
| * tags: | ||
| * - AI Feedback | ||
| * summary: Get AI feedback statistics | ||
| * description: Returns aggregate 👍/👎 feedback counts and daily trend for a guild. | ||
| * security: | ||
| * - ApiKeyAuth: [] | ||
| * - BearerAuth: [] | ||
| * parameters: | ||
| * - in: path | ||
| * name: id | ||
| * required: true | ||
| * schema: | ||
| * type: string | ||
| * description: Guild ID | ||
| * - in: query | ||
| * name: days | ||
| * schema: | ||
| * type: integer | ||
| * default: 30 | ||
| * minimum: 1 | ||
| * maximum: 90 | ||
| * description: Number of days for the trend window | ||
| * responses: | ||
| * "200": | ||
| * description: Feedback statistics | ||
| * content: | ||
| * application/json: | ||
| * schema: | ||
| * type: object | ||
| * properties: | ||
| * positive: | ||
| * type: integer | ||
| * negative: | ||
| * type: integer | ||
| * total: | ||
| * type: integer | ||
| * ratio: | ||
| * type: integer | ||
| * nullable: true | ||
| * description: Positive feedback percentage (0–100), or null if no feedback | ||
| * trend: | ||
| * type: array | ||
| * items: | ||
| * type: object | ||
| * properties: | ||
| * date: | ||
| * type: string | ||
| * format: date | ||
| * positive: | ||
| * type: integer | ||
| * negative: | ||
| * type: integer | ||
| * "401": | ||
| * $ref: "#/components/responses/Unauthorized" | ||
| * "403": | ||
| * $ref: "#/components/responses/Forbidden" | ||
| * "429": | ||
| * $ref: "#/components/responses/RateLimited" | ||
| * "500": | ||
| * $ref: "#/components/responses/ServerError" | ||
| * "503": | ||
| * $ref: "#/components/responses/ServiceUnavailable" | ||
| */ | ||
| router.get('/stats', feedbackRateLimit, requireGuildAdmin, validateGuild, async (req, res, next) => { | ||
Check failureCode scanning / CodeQL Missing rate limiting High Copilot AutofixAI 13 days ago Copilot could not generate an autofix suggestion Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support. |
||
| try { | ||
| const guildId = req.params.id; | ||
|
|
||
| let days = 30; | ||
| if (req.query.days !== undefined) { | ||
| const parsed = Number.parseInt(req.query.days, 10); | ||
| if (!Number.isNaN(parsed) && parsed >= 1 && parsed <= 90) { | ||
| days = parsed; | ||
| } | ||
| } | ||
|
|
||
| const [stats, trend] = await Promise.all([ | ||
| getFeedbackStats(guildId), | ||
| getFeedbackTrend(guildId, days), | ||
| ]); | ||
|
|
||
|
Comment on lines
+88
to
+104
There was a problem hiding this comment. These route handlers don’t check |
||
| res.json({ | ||
| ...stats, | ||
| trend, | ||
| }); | ||
| } catch (err) { | ||
| next(err); | ||
| } | ||
|
Comment on lines
+109
to
+111
Contributor
There was a problem hiding this comment. Use route-standard error handling in catch blocks. Both handlers forward raw errors with As per coding guidelines: " Also applies to: 192-194 🤖 Prompt for AI Agents
Comment on lines
+109
to
+111
There was a problem hiding this comment. Error handling here uses |
||
| }); | ||
|
|
||
| // ── GET /recent ────────────────────────────────────────────────────────────── | ||
|
|
||
| /** | ||
| * @openapi | ||
| * /guilds/{id}/ai-feedback/recent: | ||
| * get: | ||
| * tags: | ||
| * - AI Feedback | ||
| * summary: Get recent feedback entries | ||
| * description: Returns the most recent feedback entries for a guild (newest first). | ||
| * security: | ||
| * - ApiKeyAuth: [] | ||
| * - BearerAuth: [] | ||
| * parameters: | ||
| * - in: path | ||
| * name: id | ||
| * required: true | ||
| * schema: | ||
| * type: string | ||
| * description: Guild ID | ||
| * - in: query | ||
| * name: limit | ||
| * schema: | ||
| * type: integer | ||
| * default: 25 | ||
| * maximum: 100 | ||
| * responses: | ||
| * "200": | ||
| * description: Recent feedback entries | ||
| * content: | ||
| * application/json: | ||
| * schema: | ||
| * type: object | ||
| * properties: | ||
| * feedback: | ||
| * type: array | ||
| * items: | ||
| * type: object | ||
| * properties: | ||
| * id: | ||
| * type: integer | ||
| * messageId: | ||
| * type: string | ||
| * channelId: | ||
| * type: string | ||
| * userId: | ||
| * type: string | ||
| * feedbackType: | ||
| * type: string | ||
| * enum: [positive, negative] | ||
| * createdAt: | ||
| * type: string | ||
| * format: date-time | ||
| * "401": | ||
| * $ref: "#/components/responses/Unauthorized" | ||
| * "403": | ||
| * $ref: "#/components/responses/Forbidden" | ||
| * "429": | ||
| * $ref: "#/components/responses/RateLimited" | ||
| * "500": | ||
| * $ref: "#/components/responses/ServerError" | ||
| * "503": | ||
| * $ref: "#/components/responses/ServiceUnavailable" | ||
| */ | ||
| router.get('/recent', feedbackRateLimit, requireGuildAdmin, validateGuild, async (req, res, next) => { | ||
Check failureCode scanning / CodeQL Missing rate limiting High Copilot AutofixAI 13 days ago Copilot could not generate an autofix suggestion Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support. |
||
| try { | ||
| const guildId = req.params.id; | ||
|
|
||
| let limit = 25; | ||
| if (req.query.limit !== undefined) { | ||
| const parsed = Number.parseInt(req.query.limit, 10); | ||
| if (!Number.isNaN(parsed) && parsed >= 1 && parsed <= 100) { | ||
| limit = parsed; | ||
| } | ||
| } | ||
|
|
||
| const feedback = await getRecentFeedback(guildId, limit); | ||
| res.json({ feedback }); | ||
|
Comment on lines
+178
to
+191
There was a problem hiding this comment. Same as |
||
| } catch (err) { | ||
| next(err); | ||
| } | ||
| }); | ||
|
|
||
| export default router; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: VolvoxLLC/volvox-bot
Length of output: 1427
🏁 Script executed:
Repository: VolvoxLLC/volvox-bot
Length of output: 4831
Document the new
ai.feedback.enabledconfig key in README.md's configuration reference.This adds a new config key
ai.feedback.enabledto theaisection ofconfig.json, but the corresponding documentation in README.md's Configuration section is missing. Add an entry to the "AI Chat (ai)" table with the key, type, and description.🤖 Prompt for AI Agents