Skip to content

perf: SQL-based conversation pagination + missing DB indexes#221

Merged
BillChirico merged 1 commit intomainfrom
perf/conversation-query-optimizations
Mar 2, 2026
Merged

perf: SQL-based conversation pagination + missing DB indexes#221
BillChirico merged 1 commit intomainfrom
perf/conversation-query-optimizations

Conversation

@BillChirico
Copy link
Collaborator

Summary

Performance audit of recently merged features — specifically the AI conversations viewer (PR #121) and AI feedback (PR #190). Found three bottlenecks, all fixed here.


Issues Found & Fixed

🔴 Critical: 10,000-row in-memory pagination (conversations list)

File: src/api/routes/conversations.jsGET /guilds/:id/conversations

Every page request fetched up to 10,000 message rows into Node heap, grouped them in JavaScript using groupMessagesIntoConversations(), then sliced the result for the requested page. For active servers this means:

  • 10k rows × ~200 bytes/row ≈ 2MB per request allocated and GC'd
  • O(n) grouping pass over the full dataset for every page 1, 2, 3...
  • Response time scales with conversation volume, not page size

Fix: Replaced with a single SQL CTE that uses window functions (LAG + SUM OVER) to identify conversation boundaries and aggregate summaries directly in Postgres. COUNT(*) OVER() provides the total count without a second query. Pagination via LIMIT/OFFSET on summary rows — Node only processes 25 rows (default page size).

🟡 Missing indexes (4 new indexes, migration 004)

Index Table Columns Fixes
idx_ai_feedback_guild_created ai_feedback (guild_id, created_at DESC) getFeedbackTrend() / getRecentFeedback() full guild scan
idx_conversations_content_trgm conversations content gin_trgm_ops ILIKE '%...%' search sequential scan
idx_conversations_guild_created conversations (guild_id, created_at DESC) Default 30-day listing with no channel filter
idx_flagged_messages_guild_message flagged_messages (guild_id, message_id) Conversation detail + flag endpoints

The pg_trgm extension is enabled idempotently (CREATE EXTENSION IF NOT EXISTS).

🟢 Minor: Sequential verification queries in flag POST

File: src/api/routes/conversations.jsPOST /guilds/:id/conversations/:conversationId/flag

msgCheck and anchorCheck were independent lookups running sequentially (2× DB round-trips). Changed to Promise.all — 1× round-trip.


Test Results

  • 41/41 conversation tests pass (updated 1 test to match new SQL response shape)
  • ✅ Pre-existing failures in redis.test.js, triage.coverage.test.js, and ai-feedback.test.js are unrelated to this PR (confirmed present on main before this branch)

Notes

  • buildConversationSummary() helper removed — its logic is now handled SQL-side
  • groupMessagesIntoConversations() is still used by the conversation detail endpoint and its export is tested; no changes there
  • The pg_trgm ILIKE index requires the pg_trgm extension to be available (standard on Postgres 12+)

Fixes three performance bottlenecks identified in code review of
recently merged features (PR #121 conversations viewer, PR #190 AI feedback).

## Changes

### migrations/004_performance_indexes.cjs (new)
Four new indexes targeting hot query paths:

- idx_ai_feedback_guild_created (guild_id, created_at DESC)
  getFeedbackTrend() and getRecentFeedback() filtered by guild_id
  AND created_at but only had a single-column guild_id index, forcing
  a full guild scan + sort on every trend/recent call.

- idx_conversations_content_trgm (GIN, pg_trgm)
  content ILIKE '%...%' search was a sequential scan. GIN/trgm index
  reduces this from O(n) to O(log n * trigram matches).
  Requires pg_trgm extension (added idempotently).

- idx_conversations_guild_created (guild_id, created_at DESC)
  Default 30-day listing query filters guild_id + created_at. The
  existing 3-column (guild_id, channel_id, created_at) composite is
  suboptimal when channel_id is not in the predicate.

- idx_flagged_messages_guild_message (guild_id, message_id)
  Conversation detail + flag endpoints query flagged_messages by
  guild_id AND message_id = ANY(...). Existing index only covers
  (guild_id, status).

### src/api/routes/conversations.js
**GET / — Replace in-memory pagination with SQL CTE grouping**

Before: fetched up to 10,000 message rows into Node memory, grouped
them in JavaScript (O(n) time + memory), then sliced for pagination.
Every page request loaded the full 10k row dataset.

After: single SQL query using window functions (LAG + SUM OVER) to
identify conversation boundaries and aggregate summaries directly.
COUNT(*) OVER() provides total count without a second query.
Pagination happens at the DB with LIMIT/OFFSET on summary rows.
Memory overhead is now proportional to page size (default 25), not
total conversation volume.

Removed now-unused buildConversationSummary() helper (logic inlined
into the SQL-side aggregation).

**POST /:conversationId/flag — Parallel verification queries**

Before: msgCheck and anchorCheck ran sequentially (~2× RTT).
After: both run in parallel via Promise.all (1× RTT for verification).

### tests/api/routes/conversations.test.js
Updated 'should return paginated conversations' test to mock the new
SQL CTE response shape (pre-aggregated summary rows) instead of raw
message rows. All 41 conversation tests pass.
Copilot AI review requested due to automatic review settings March 2, 2026 04:56
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026

Warning

Rate limit exceeded

@BillChirico has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 40 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between bcf04e2 and 4bffbfb.

📒 Files selected for processing (3)
  • migrations/004_performance_indexes.cjs
  • src/api/routes/conversations.js
  • tests/api/routes/conversations.test.js
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch perf/conversation-query-optimizations

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves performance of the conversations viewer and related endpoints by moving conversation pagination/grouping into SQL and adding missing database indexes to support common query patterns.

Changes:

  • Replaced Node in-memory grouping/pagination with a Postgres CTE + window-function based conversation summarization and SQL-level pagination.
  • Added migration 004 with composite + trigram indexes (and pg_trgm enablement) to eliminate sequential scans and expensive sorts.
  • Parallelized independent verification queries in the flagging endpoint.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/api/routes/conversations.js Moves conversation grouping/pagination into SQL and refactors response shaping; parallelizes verification queries for flagging.
tests/api/routes/conversations.test.js Updates conversation pagination test to match the new SQL summary row shape (total + preview expectations).
migrations/004_performance_indexes.cjs Adds pg_trgm extension enablement and several targeted indexes for conversations/feedback/flags queries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@greptile-apps
Copy link

greptile-apps bot commented Mar 2, 2026

Greptile Summary

Excellent performance optimization that addresses three identified bottlenecks from the AI conversations feature. The critical fix replaces 10k-row in-memory pagination with a sophisticated SQL CTE using window functions (LAG, SUM OVER) to group and paginate conversations directly in Postgres, reducing heap allocation from ~2MB to ~5KB per request and changing complexity from O(n) to O(1) for pagination.

Major Changes:

  • SQL-based conversation grouping using 3-stage CTE (lag_stepnumberedsummaries) with proper 15-minute gap detection
  • 4 new composite indexes in migration 004: ai_feedback(guild_id, created_at), conversations GIN/pg_trgm for ILIKE search, conversations(guild_id, created_at), flagged_messages(guild_id, message_id)
  • Parallel query optimization in flag endpoint (sequential → Promise.all)
  • Removed buildConversationSummary() helper (logic moved to SQL)

Technical Highlights:

  • Window function logic is correct: LAG detects gaps, SUM OVER assigns conversation numbers, COUNT(*) OVER() provides total without second query
  • Participant parsing uses lastIndexOf(':::') to correctly handle usernames containing the separator
  • All user input remains properly parameterized (constant CONVERSATION_GAP_MINUTES * 60 is safe)
  • Maintains identical behavior to previous implementation while eliminating memory bottleneck
  • Test coverage updated and all 41 tests passing

Confidence Score: 5/5

  • This PR is safe to merge with high confidence - well-tested performance optimization with no breaking changes
  • Score of 5 reflects: (1) sophisticated but correct SQL window function logic that properly handles conversation grouping, (2) comprehensive test coverage with all 41 tests passing, (3) proper database migration with idempotent operations and down migration, (4) all user input properly parameterized following security best practices, (5) significant performance improvement without changing API behavior, (6) clear documentation and thorough PR description demonstrating deep understanding of the problem
  • No files require special attention - all changes are well-implemented and thoroughly tested

Important Files Changed

Filename Overview
migrations/004_performance_indexes.cjs Adds 4 well-justified indexes: ai_feedback(guild_id,created_at), conversations GIN/trgm for ILIKE, conversations(guild_id,created_at), flagged_messages(guild_id,message_id). Includes proper up/down migrations and idempotent extension setup.
src/api/routes/conversations.js Replaces 10k-row in-memory pagination with SQL-based CTE using window functions (LAG/SUM OVER) for conversation grouping. Removes buildConversationSummary helper. Optimizes flag endpoint with Promise.all for parallel verification queries.
tests/api/routes/conversations.test.js Updates one test to match new SQL response shape (pre-aggregated conversation summary rows instead of individual message rows). All other tests remain unchanged and passing.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Start[GET /conversations request] --> OldOrNew{Implementation}
    
    OldOrNew -->|Old: PR #121| OldFetch[Fetch up to 10k rows<br/>ORDER BY created_at DESC]
    OldFetch --> OldReverse[Reverse rows to ASC]
    OldReverse --> OldGroup[groupMessagesIntoConversations<br/>in Node.js memory]
    OldGroup --> OldPaginate[Slice array for page]
    OldPaginate --> OldBuild[buildConversationSummary<br/>for each conversation]
    OldBuild --> OldReturn[Return JSON]
    
    OldOrNew -->|New: This PR| NewCTE[Execute SQL CTE with<br/>window functions]
    NewCTE --> NewLag[lag_step: LAG to detect<br/>15-min gaps]
    NewLag --> NewNum[numbered: SUM OVER<br/>to assign conv_num]
    NewNum --> NewSum[summaries: GROUP BY<br/>channel + conv_num]
    NewSum --> NewPage[LIMIT/OFFSET on<br/>summary rows]
    NewPage --> NewMap[Map results in Node<br/>parse participant_pairs]
    NewMap --> NewReturn[Return JSON]
    
    OldReturn --> Impact1[2MB heap per request<br/>O n grouping]
    NewReturn --> Impact2[~5KB heap per request<br/>O 1 pagination]
    
    style OldFetch fill:#ffcccc
    style OldGroup fill:#ffcccc
    style OldPaginate fill:#ffcccc
    style NewCTE fill:#ccffcc
    style NewPage fill:#ccffcc
    style Impact1 fill:#ff9999
    style Impact2 fill:#99ff99
Loading

Last reviewed commit: 4bffbfb

@BillChirico BillChirico merged commit 44a852e into main Mar 2, 2026
17 of 21 checks passed
@BillChirico BillChirico deleted the perf/conversation-query-optimizations branch March 2, 2026 09:21
BillChirico added a commit that referenced this pull request Mar 2, 2026
Fixes three performance bottlenecks identified in code review of
recently merged features (PR #121 conversations viewer, PR #190 AI feedback).

## Changes

### migrations/004_performance_indexes.cjs (new)
Four new indexes targeting hot query paths:

- idx_ai_feedback_guild_created (guild_id, created_at DESC)
  getFeedbackTrend() and getRecentFeedback() filtered by guild_id
  AND created_at but only had a single-column guild_id index, forcing
  a full guild scan + sort on every trend/recent call.

- idx_conversations_content_trgm (GIN, pg_trgm)
  content ILIKE '%...%' search was a sequential scan. GIN/trgm index
  reduces this from O(n) to O(log n * trigram matches).
  Requires pg_trgm extension (added idempotently).

- idx_conversations_guild_created (guild_id, created_at DESC)
  Default 30-day listing query filters guild_id + created_at. The
  existing 3-column (guild_id, channel_id, created_at) composite is
  suboptimal when channel_id is not in the predicate.

- idx_flagged_messages_guild_message (guild_id, message_id)
  Conversation detail + flag endpoints query flagged_messages by
  guild_id AND message_id = ANY(...). Existing index only covers
  (guild_id, status).

### src/api/routes/conversations.js
**GET / — Replace in-memory pagination with SQL CTE grouping**

Before: fetched up to 10,000 message rows into Node memory, grouped
them in JavaScript (O(n) time + memory), then sliced for pagination.
Every page request loaded the full 10k row dataset.

After: single SQL query using window functions (LAG + SUM OVER) to
identify conversation boundaries and aggregate summaries directly.
COUNT(*) OVER() provides total count without a second query.
Pagination happens at the DB with LIMIT/OFFSET on summary rows.
Memory overhead is now proportional to page size (default 25), not
total conversation volume.

Removed now-unused buildConversationSummary() helper (logic inlined
into the SQL-side aggregation).

**POST /:conversationId/flag — Parallel verification queries**

Before: msgCheck and anchorCheck ran sequentially (~2× RTT).
After: both run in parallel via Promise.all (1× RTT for verification).

### tests/api/routes/conversations.test.js
Updated 'should return paginated conversations' test to mock the new
SQL CTE response shape (pre-aggregated summary rows) instead of raw
message rows. All 41 conversation tests pass.
BillChirico added a commit that referenced this pull request Mar 2, 2026
Fixes three performance bottlenecks identified in code review of
recently merged features (PR #121 conversations viewer, PR #190 AI feedback).

## Changes

### migrations/004_performance_indexes.cjs (new)
Four new indexes targeting hot query paths:

- idx_ai_feedback_guild_created (guild_id, created_at DESC)
  getFeedbackTrend() and getRecentFeedback() filtered by guild_id
  AND created_at but only had a single-column guild_id index, forcing
  a full guild scan + sort on every trend/recent call.

- idx_conversations_content_trgm (GIN, pg_trgm)
  content ILIKE '%...%' search was a sequential scan. GIN/trgm index
  reduces this from O(n) to O(log n * trigram matches).
  Requires pg_trgm extension (added idempotently).

- idx_conversations_guild_created (guild_id, created_at DESC)
  Default 30-day listing query filters guild_id + created_at. The
  existing 3-column (guild_id, channel_id, created_at) composite is
  suboptimal when channel_id is not in the predicate.

- idx_flagged_messages_guild_message (guild_id, message_id)
  Conversation detail + flag endpoints query flagged_messages by
  guild_id AND message_id = ANY(...). Existing index only covers
  (guild_id, status).

### src/api/routes/conversations.js
**GET / — Replace in-memory pagination with SQL CTE grouping**

Before: fetched up to 10,000 message rows into Node memory, grouped
them in JavaScript (O(n) time + memory), then sliced for pagination.
Every page request loaded the full 10k row dataset.

After: single SQL query using window functions (LAG + SUM OVER) to
identify conversation boundaries and aggregate summaries directly.
COUNT(*) OVER() provides total count without a second query.
Pagination happens at the DB with LIMIT/OFFSET on summary rows.
Memory overhead is now proportional to page size (default 25), not
total conversation volume.

Removed now-unused buildConversationSummary() helper (logic inlined
into the SQL-side aggregation).

**POST /:conversationId/flag — Parallel verification queries**

Before: msgCheck and anchorCheck ran sequentially (~2× RTT).
After: both run in parallel via Promise.all (1× RTT for verification).

### tests/api/routes/conversations.test.js
Updated 'should return paginated conversations' test to mock the new
SQL CTE response shape (pre-aggregated summary rows) instead of raw
message rows. All 41 conversation tests pass.
BillChirico added a commit that referenced this pull request Mar 2, 2026
* security: escape user content in triage prompt delimiters (#164)

Add escapePromptDelimiters() to HTML-encode < and > in user-supplied
message content before it is inserted between XML-style section tags
in the LLM prompt.

Without escaping, a crafted message containing the literal text
`</messages-to-evaluate>` could break out of the user-content section
and inject attacker-controlled instructions into the prompt structure.

Changes:
- Add escapePromptDelimiters(text) utility exported from triage-prompt.js
- Apply escape to m.content and m.replyTo.content in buildConversationText()
- Add 13 new tests covering the escape function and injection scenarios

Closes #164

* security: escape & chars and author fields in prompt delimiters

* fix(security): escape & in prompt delimiters and escape author fields

- Add & → &amp; escape first in escapePromptDelimiters() to prevent
  HTML entity bypass attacks (e.g. &lt;/messages-to-evaluate&gt;)
- Also escape m.author and m.replyTo.author since Discord display
  names are user-controlled and can contain < / > characters

Addresses review feedback on PR #204.

* fix: guard replyTo.content before .slice() to handle null/undefined

* perf: SQL-based conversation pagination + missing DB indexes (#221)

Fixes three performance bottlenecks identified in code review of
recently merged features (PR #121 conversations viewer, PR #190 AI feedback).

## Changes

### migrations/004_performance_indexes.cjs (new)
Four new indexes targeting hot query paths:

- idx_ai_feedback_guild_created (guild_id, created_at DESC)
  getFeedbackTrend() and getRecentFeedback() filtered by guild_id
  AND created_at but only had a single-column guild_id index, forcing
  a full guild scan + sort on every trend/recent call.

- idx_conversations_content_trgm (GIN, pg_trgm)
  content ILIKE '%...%' search was a sequential scan. GIN/trgm index
  reduces this from O(n) to O(log n * trigram matches).
  Requires pg_trgm extension (added idempotently).

- idx_conversations_guild_created (guild_id, created_at DESC)
  Default 30-day listing query filters guild_id + created_at. The
  existing 3-column (guild_id, channel_id, created_at) composite is
  suboptimal when channel_id is not in the predicate.

- idx_flagged_messages_guild_message (guild_id, message_id)
  Conversation detail + flag endpoints query flagged_messages by
  guild_id AND message_id = ANY(...). Existing index only covers
  (guild_id, status).

### src/api/routes/conversations.js
**GET / — Replace in-memory pagination with SQL CTE grouping**

Before: fetched up to 10,000 message rows into Node memory, grouped
them in JavaScript (O(n) time + memory), then sliced for pagination.
Every page request loaded the full 10k row dataset.

After: single SQL query using window functions (LAG + SUM OVER) to
identify conversation boundaries and aggregate summaries directly.
COUNT(*) OVER() provides total count without a second query.
Pagination happens at the DB with LIMIT/OFFSET on summary rows.
Memory overhead is now proportional to page size (default 25), not
total conversation volume.

Removed now-unused buildConversationSummary() helper (logic inlined
into the SQL-side aggregation).

**POST /:conversationId/flag — Parallel verification queries**

Before: msgCheck and anchorCheck ran sequentially (~2× RTT).
After: both run in parallel via Promise.all (1× RTT for verification).

### tests/api/routes/conversations.test.js
Updated 'should return paginated conversations' test to mock the new
SQL CTE response shape (pre-aggregated summary rows) instead of raw
message rows. All 41 conversation tests pass.

* feat: channel-level quiet mode via bot mention (#173) (#213)

* feat: quiet mode per-channel via bot mention (#173)

- Add quietMode.js module with Redis+memory storage
- Parse duration from natural language (30m, 1 hour, etc.)
- Permission gated via config.quietMode.allowedRoles
- Commands: quiet, unquiet, status
- Suppress AI responses during quiet mode in events.js
- Add quietMode section to config.json (disabled by default)
- Add quietMode to configAllowlist.js for dashboard editing

* test: add quiet mode tests (41 tests, all passing)

* style: fix biome formatting in quietMode.js, events.js, and test

* fix(web): fix ai-feedback-stats TypeScript and formatting errors

* fix: gate quiet mode checks on enabled flag, validate TTL, honor maxDurationMinutes config

- events.js: Wrap isQuietMode() calls in guildConfig.quietMode?.enabled check
  to avoid unnecessary Redis lookups and prevent stale records from suppressing
  AI responses when the feature is disabled (PRRT_kwDORICdSM5xdbmp, PRRT_kwDORICdSM5xdbmx)

- quietMode.js: Add TTL validation in setQuiet() to guard against 0, negative,
  or NaN values that would error in Redis (PRRT_kwDORICdSM5xdbm3)

- quietMode.js: Update parseDurationFromContent() to accept config parameter
  and honor guildConfig.quietMode.maxDurationMinutes. Also clamp defaultSeconds
  to the effective max (PRRT_kwDORICdSM5xdbm_)

- configValidation.js: Add quietMode schema entry with enabled, maxDurationMinutes,
  and allowedRoles properties (PRRT_kwDORICdSM5xdbnH)

* style: fix biome formatting in quietMode.js and ai-feedback-stats.tsx

* feat: audit log improvements — CSV/JSON export and real-time WebSocket stream (#215)

* feat: audit log improvements — CSV/JSON export, real-time WebSocket stream

- Add GET /:id/audit-log/export endpoint (CSV and JSON, up to 10k rows)
- Add /ws/audit-log WebSocket server for real-time audit entry broadcast
- Refactor buildFilters() shared helper to eliminate duplication
- Hook broadcastAuditEntry() into insertAuditEntry (RETURNING id+created_at)
- Wire setupAuditStream/stopAuditStream into startServer/stopServer lifecycle
- Add escapeCsvValue/rowsToCsv helpers with full test coverage
- 30 route tests + 17 WebSocket stream tests, all green

Closes #136

* fix: PR #215 review feedback - audit stream fixes

- ws.ping() crash: guard with readyState check + try/catch to avoid
  crashing heartbeat interval when socket not OPEN
- stopAuditStream race: make setupAuditStream async and await
  stopAuditStream() to prevent concurrent WebSocketServer creation
- Query param array coercion: add typeof === 'string' checks for
  startDate/endDate to handle Express string|string[]|undefined
- CSV CRLF quoting: add \r to RFC 4180 special-char check for proper
  Windows line ending handling
- Test timeouts: make AUTH_TIMEOUT_MS configurable via
  AUDIT_STREAM_AUTH_TIMEOUT_MS env var, use 100ms in tests

* feat: voice channel activity tracking — join/leave/move, leaderboard, export (#212)

* feat: add voice_sessions migration (#135)

* feat: add voice tracking module — join/leave/move/flush/leaderboard (#135)

* feat: wire voiceStateUpdate handler into event registration (#135)

* feat: add /voice command — leaderboard, stats, export subcommands (#135)

* feat: add voice config defaults to config.json (#135)

* feat: wire voice flush start/stop into bot lifecycle (#135)

* feat: add voice to config API allowlist (#135)

* fix: SQL UPDATE subquery for closeSession, fix import order (#135)

* fix(voice): resolve race conditions and missing config schema

- Fix openSession: update in-memory state only AFTER DB INSERT succeeds
- Fix closeSession: delete from in-memory state only AFTER DB UPDATE succeeds
- Fix: allow closeSession on leave/move even when feature is disabled
- Fix migration: add UNIQUE constraint to partial index to prevent duplicates
- Fix: move 'Voice join' log to after openSession succeeds
- Add voice config to CONFIG_SCHEMA for validation

---------

Co-authored-by: Bill <[email protected]>

* feat(dashboard): auto-save config with 500ms debounce (#199)

* feat(dashboard): replace manual save with auto-save (500ms debounce)

- Remove 'Save Changes' button; saving now fires automatically 500ms
  after the last config change (no changes → no network call)
- Add saveStatus state ('idle' | 'saving' | 'saved' | 'error') with
  AutoSaveStatus component showing spinner, check, or error+retry
- Add isLoadingConfigRef guard so the initial fetchConfig load never
  triggers a spurious PATCH
- Ctrl+S still works: clears debounce timer and saves immediately
- Keep 'beforeunload' warning for validation errors that block save
- Replace yellow unsaved-changes banner with a destructive validation
  error banner (only shown when save is actually blocked)
- Error state shows 'Save failed' + 'Retry' button for user recovery

Closes #189

* test(dashboard): add auto-save tests for ConfigEditor

- No PATCH on initial config load
- Validation error banner suppresses auto-save
- 'Saving...' spinner visible while PATCH in-flight
- 'Save failed' + Retry button on PATCH error

* fix(dashboard): prevent fetchConfig from overwriting saveStatus after successful save

Add skipSaveStatusReset parameter to fetchConfig so that post-save reloads
preserve the 'saved' status indicator instead of immediately resetting to 'idle'.

* test(dashboard): use fake timers, restore vi.stubGlobal, fix assertions, add idle/saved coverage

- Replace real setTimeout delays with vi.useFakeTimers() + vi.advanceTimersByTimeAsync()
  for deterministic, fast debounce tests
- Add afterEach cleanup: vi.unstubAllGlobals() + vi.useRealTimers()
- Replace toBeTruthy() with toBeInTheDocument() for Testing Library queries
- Add idle state test (no status indicator shown after load)
- Add saved state test (shows 'Saved' after successful save)
- Update file-level comment to list all four states

---------

Co-authored-by: Bill Chirico <[email protected]>

* feat: Reaction role menus (#162) (#205)

* feat: reaction role menus - core module, command, event hooks, migration

Implements issue #162: reaction role menus.

- Add migration 004 creating reaction_role_menus and reaction_role_entries tables
- Add src/modules/reactionRoles.js with DB helpers, embed builder, event handlers
- Add src/commands/reactionrole.js with /reactionrole create|add|remove|delete|list
- Wire handleReactionRoleAdd/Remove into registerReactionHandlers in events.js

Roles are granted on reaction add and revoked on reaction remove.
All mappings persist in PostgreSQL across bot restarts.

* test: reaction role menus - 40 tests covering module and command

- tests/modules/reactionRoles.test.js: resolveEmojiString, buildReactionRoleEmbed,
  all DB helpers, handleReactionRoleAdd, handleReactionRoleRemove
- tests/commands/reactionrole.test.js: all 5 subcommands (create, add, remove,
  delete, list) including error paths and guild ownership checks
- Fix biome lint: import sort order + unused import removal

* fix: remove unused import in reactionrole command

---------

Co-authored-by: Bill Chirico <[email protected]>

* fix(security): validate GitHub owner/repo format before gh CLI call (#198)

* fix(security): validate GitHub owner/repo format before gh CLI call

Prevents API path traversal by validating owner/repo segments against
a strict allowlist regex before interpolating them into the gh CLI
invocation.

Adds:
- VALID_GH_NAME regex (/^[a-zA-Z0-9._-]+$/)
- isValidGhRepo() helper (exported for testing)
- Guard in fetchRepoEvents() — returns [] and warns on invalid input
- Strengthened guard in pollGuildFeed() split logic

Fixes #160

* test(security): add validation tests for GitHub owner/repo format

Covers isValidGhRepo(), VALID_GH_NAME regex, and fetchRepoEvents()
validation guard introduced in fix for #160.

19 new tests verify:
- Valid alphanumeric/dot/hyphen/underscore names pass
- Path traversal (../../etc/passwd) is rejected at both entry points
- Slashes, empty strings, non-strings, spaces all rejected
- Shell metacharacters (; && $()) blocked
- gh CLI is NOT invoked when validation fails
- warn() fires with the invalid values (observable audit trail)
- Valid owner/repo still reach gh CLI unchanged

* fix(security): reject pure-dot owner/repo names to prevent path traversal

* test(githubFeed): add tests for pure-dot path traversal bypass

---------

Co-authored-by: Bill Chirico <[email protected]>

---------

Co-authored-by: Bill <[email protected]>
Co-authored-by: Bill Chirico <[email protected]>
BillChirico added a commit that referenced this pull request Mar 2, 2026
* feat: add role_menu_templates migration (#135)

* feat: add roleMenuTemplates module with built-ins, CRUD, and share (#135)

* feat: add /rolemenu command with template CRUD, apply, share (#135)

* feat: seed built-in role menu templates on startup (#135)

* test: add roleMenuTemplates tests — 36 passing (#135)

* test: add /rolemenu command tests — 19 passing (#135)

* fix: typo hasModeatorPerms → hasModeratorPerms

* perf: SQL-based conversation pagination + missing DB indexes (#221)

Fixes three performance bottlenecks identified in code review of
recently merged features (PR #121 conversations viewer, PR #190 AI feedback).

## Changes

### migrations/004_performance_indexes.cjs (new)
Four new indexes targeting hot query paths:

- idx_ai_feedback_guild_created (guild_id, created_at DESC)
  getFeedbackTrend() and getRecentFeedback() filtered by guild_id
  AND created_at but only had a single-column guild_id index, forcing
  a full guild scan + sort on every trend/recent call.

- idx_conversations_content_trgm (GIN, pg_trgm)
  content ILIKE '%...%' search was a sequential scan. GIN/trgm index
  reduces this from O(n) to O(log n * trigram matches).
  Requires pg_trgm extension (added idempotently).

- idx_conversations_guild_created (guild_id, created_at DESC)
  Default 30-day listing query filters guild_id + created_at. The
  existing 3-column (guild_id, channel_id, created_at) composite is
  suboptimal when channel_id is not in the predicate.

- idx_flagged_messages_guild_message (guild_id, message_id)
  Conversation detail + flag endpoints query flagged_messages by
  guild_id AND message_id = ANY(...). Existing index only covers
  (guild_id, status).

### src/api/routes/conversations.js
**GET / — Replace in-memory pagination with SQL CTE grouping**

Before: fetched up to 10,000 message rows into Node memory, grouped
them in JavaScript (O(n) time + memory), then sliced for pagination.
Every page request loaded the full 10k row dataset.

After: single SQL query using window functions (LAG + SUM OVER) to
identify conversation boundaries and aggregate summaries directly.
COUNT(*) OVER() provides total count without a second query.
Pagination happens at the DB with LIMIT/OFFSET on summary rows.
Memory overhead is now proportional to page size (default 25), not
total conversation volume.

Removed now-unused buildConversationSummary() helper (logic inlined
into the SQL-side aggregation).

**POST /:conversationId/flag — Parallel verification queries**

Before: msgCheck and anchorCheck ran sequentially (~2× RTT).
After: both run in parallel via Promise.all (1× RTT for verification).

### tests/api/routes/conversations.test.js
Updated 'should return paginated conversations' test to mock the new
SQL CTE response shape (pre-aggregated summary rows) instead of raw
message rows. All 41 conversation tests pass.

* feat: channel-level quiet mode via bot mention (#173) (#213)

* feat: quiet mode per-channel via bot mention (#173)

- Add quietMode.js module with Redis+memory storage
- Parse duration from natural language (30m, 1 hour, etc.)
- Permission gated via config.quietMode.allowedRoles
- Commands: quiet, unquiet, status
- Suppress AI responses during quiet mode in events.js
- Add quietMode section to config.json (disabled by default)
- Add quietMode to configAllowlist.js for dashboard editing

* test: add quiet mode tests (41 tests, all passing)

* style: fix biome formatting in quietMode.js, events.js, and test

* fix(web): fix ai-feedback-stats TypeScript and formatting errors

* fix: gate quiet mode checks on enabled flag, validate TTL, honor maxDurationMinutes config

- events.js: Wrap isQuietMode() calls in guildConfig.quietMode?.enabled check
  to avoid unnecessary Redis lookups and prevent stale records from suppressing
  AI responses when the feature is disabled (PRRT_kwDORICdSM5xdbmp, PRRT_kwDORICdSM5xdbmx)

- quietMode.js: Add TTL validation in setQuiet() to guard against 0, negative,
  or NaN values that would error in Redis (PRRT_kwDORICdSM5xdbm3)

- quietMode.js: Update parseDurationFromContent() to accept config parameter
  and honor guildConfig.quietMode.maxDurationMinutes. Also clamp defaultSeconds
  to the effective max (PRRT_kwDORICdSM5xdbm_)

- configValidation.js: Add quietMode schema entry with enabled, maxDurationMinutes,
  and allowedRoles properties (PRRT_kwDORICdSM5xdbnH)

* style: fix biome formatting in quietMode.js and ai-feedback-stats.tsx

* Fix: unterminated string in rolemenu.js

* Fix: lint issues and formatting

* fix: deterministic template lookup and correct roleId precedence

- Add ORDER BY to getTemplateByName for deterministic results
- Fix roleId precedence to preserve existing roleIds during merge
- Truncate Discord embed field values to 1024 chars

* fix: test assertion matches comment intent

The test expected template roleId to win, but the comment said existing
should take precedence. Fixed assertion to match documented behavior.

* fix: filter empty roleIds and only enable when valid options exist

- Filter out options with empty roleIds before saving
- Only enable role menu for non-built-in templates with valid options
- Add user-facing note when options are filtered

* chore: remove unused _MAX_DESCRIPTION_LEN constant

* fix: case-insensitive unique index for template names

Use LOWER(name) in unique index to match case-insensitive queries
and prevent duplicate templates differing only by case.

* fix(roleMenuTemplates): add type validation for roleId and description

- validateTemplateOptions now validates that optional roleId and
  description fields are strings when present
- Update JSDoc @see reference from issue #135 (voice tracking) to
  issue #216 (role menu templates)
- Update ON CONFLICT clause to use constraint name for consistency
  with the new LOWER(name) index

---------

Co-authored-by: Bill <[email protected]>
Co-authored-by: Bill Chirico <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants