feat(reputation): add XP/leveling system with rank and leaderboard#109
feat(reputation): add XP/leveling system with rank and leaderboard#109BillChirico merged 10 commits intomainfrom
Conversation
|
Claude finished @BillChirico's task in 7m 17s —— View job Review Complete3 warnings, 2 nitpicks found across 13 files. The core implementation is solid — well-structured XP engine, atomic DB upserts, parameterized queries, proper 🟡 Warnings (3)
🔵 Nitpicks (2)
AI prompt to fix all issues |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (13)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a reputation/XP system: new Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 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 |
|
| Filename | Overview |
|---|---|
| src/modules/reputation.js | Core XP engine with message-based leveling, cooldown enforcement, role rewards, and level-up announcements — well-structured with comprehensive error handling |
| src/commands/rank.js | Rank command displaying user level, XP, progress bar, and server rank — proper error handling and fallback logic for missing data |
| src/commands/leaderboard.js | Leaderboard command showing top 10 users with batch member fetching and graceful fallback for missing members |
| migrations/007_reputation.cjs | Database migration creating reputation table with proper constraints (guild_id, user_id unique) and performance index for leaderboard queries |
| tests/modules/reputation.test.js | Comprehensive test suite with 22 tests covering XP award, cooldown, level-up, role rewards, announcements, and error handling edge cases |
| src/modules/events.js | Event handler integration wiring handleXpGain as fire-and-forget on messageCreate with proper error logging (lines 155-162) |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User sends message] --> B{Message in guild?}
B -->|No| Z[Skip]
B -->|Yes| C{Reputation enabled?}
C -->|No| Z
C -->|Yes| D{Message >= 10 chars?}
D -->|No| Z
D -->|Yes| E{User on cooldown?}
E -->|Yes| Z
E -->|No| F[Award random XP<br/>5-15 range]
F --> G[Upsert DB:<br/>reputation table]
G --> H[Set cooldown timestamp]
H --> I{Level up detected?}
I -->|No| Z
I -->|Yes| J[Update level in DB]
J --> K{Role reward<br/>configured?}
K -->|Yes| L[Assign role]
K -->|No| M
L --> M{Announce channel<br/>configured?}
M -->|Yes| N[Send level-up<br/>embed]
M -->|No| Z
N --> Z[Done]
Last reviewed commit: 79c82a9
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: config.json
Line: 128-149
Comment:
`rank` and `leaderboard` commands missing from `allowedCommands` - they'll default to admin-only. Add:
```suggestion
"modlog": "moderator",
"announce": "moderator",
"tldr": "everyone",
"rank": "everyone",
"leaderboard": "everyone"
```
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Review Summary
4 warnings and 3 nitpicks found. Solid implementation overall — the core XP engine, SQL, and test coverage are well done. The issues below should be addressed before merge.
🟡 Warnings (4)
- Missing
enabledgate in/rank(src/commands/rank.js:30) — Command runs even whenreputation.enabledisfalse(the default). Other community commands check their feature flag. - Missing
enabledgate in/leaderboard(src/commands/leaderboard.js:22) — Same issue. - Unbounded cooldown map (
src/modules/reputation.js:15-16) — The in-memorycooldownsMap grows per unique guild:user pair and is never pruned. Memory leak in production. - Missing
allowedCommandsentries (config.json) —rankandleaderboardnot inpermissions.allowedCommands. Per AGENTS.md, new commands must be registered here.
🔵 Nitpicks (3)
- Duplicated
DEFAULT_THRESHOLDS(src/commands/rank.js:15) — Already defined inreputation.js; import instead of duplicating. displayAvatarURL({ dynamic: true })(src/modules/reputation.js:163,src/commands/rank.js:81,94) —dynamicwas a discord.js v13 option, silently ignored in v14.- Sequential member fetches in leaderboard (
src/commands/leaderboard.js:47-58) — 10 individual API calls could be batched into oneguild.members.fetch({ user: [...] }).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 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 169-175: The README config reference is missing documentation for
the new "reputation" block added to config.json; update README.md to document
each key under "reputation" (enabled, xpPerMessage, xpCooldownSeconds,
announceChannelId, levelThresholds, roleRewards) describing expected types,
defaults, valid values and brief behavior for each (e.g., enabled: boolean
default false; xpPerMessage: array [min,max] ints; xpCooldownSeconds: number;
announceChannelId: string|null; levelThresholds: array of numeric XP thresholds;
roleRewards: mapping of level->role IDs), and include an example snippet showing
a populated reputation object to match the config.json defaults.
In `@migrations/007_reputation.cjs`:
- Line 23: The CREATE INDEX statement for idx_reputation_guild_xp is not
idempotent and can fail on re-run; update the pgm.sql call that creates
idx_reputation_guild_xp (the statement targeting reputation(guild_id, xp DESC))
to use an idempotent form (e.g., include IF NOT EXISTS or equivalent) so
re-applying the migration or performing drift recovery won't error.
- Around line 10-20: The reputation schema is missing the required counters
voice_minutes and helps_given; update the CREATE TABLE for reputation to add
columns voice_minutes INTEGER NOT NULL DEFAULT 0 and helps_given INTEGER NOT
NULL DEFAULT 0 (or equivalent non-null integer with default 0) so the reputation
table (in migration that defines reputation) includes these counters alongside
xp, level, and messages_count; ensure the UNIQUE(guild_id, user_id) and
created_at/defaults remain unchanged.
In `@src/commands/leaderboard.js`:
- Around line 24-68: Wrap the leaderboard command's main logic in a top-level
try/catch and add a guard that ensures interaction.guild exists before DB or
member lookups; specifically, check interaction.guild and return a safeEditReply
error if missing, then wrap the calls to getPool(), pool.query(...), and the
rows.map async member fetch (interaction.guild.members.fetch) inside a try block
and catch any DB/runtime errors to call safeEditReply with a friendly failure
message. Ensure you reference the existing functions/variables (getPool,
safeEditReply, pool.query, interaction.guild.members.fetch, EmbedBuilder) and
avoid throwing so the command always replies gracefully on any failure.
In `@src/commands/rank.js`:
- Around line 58-62: Fix the progress bar string styling and formatting: ensure
the xpNeeded calculation and ternary expression (xpNeeded, nextThreshold,
currentThreshold) follow project formatter style, and change the MAX branch in
progressBar to use a template literal instead of string concatenation (modify
the progressBar assignment that uses buildProgressBar(xpInLevel, xpNeeded) and
the `'▓'.repeat(10) + ' MAX'` branch to a template literal like
```${'▓'.repeat(10)} MAX```), preserving the existing logic and variable names
(xpInLevel, xpNeeded, nextThreshold, currentThreshold, buildProgressBar).
- Around line 32-70: The code assumes getPool() and subsequent pool.query calls
never throw, but getPool and DB queries (used in getPool(), pool.query(...)
lines and the rankRow query) can throw before the null-check and leave /rank
uncaught; wrap the DB interaction logic in a try/catch: call getPool() and run
all queries (the SELECT for xp/level/messages_count and the rank COUNT query)
inside a single try block, catch any error and call safeEditReply(interaction, {
content: '❌ Database is not available.' }) (or a similar safe error message)
then return; ensure you still handle the case where pool is falsy after
getPool() and use the same safe reply in that path so computeLevel,
buildProgressBar and any use of rows[0] only run when queries succeed.
In `@src/modules/reputation.js`:
- Line 8: Run Biome's organize-imports and formatter on this module to fix the
import ordering and formatting issues; specifically reorder/remove/merge the
import that includes EmbedBuilder so it matches the project's import groups and
style, reformat the file to resolve the formatter diff around the end of the
module, and update the template-literal at the reported location (the string
interpolation used in the reputation-related output around line ~70) to conform
to the project's template-literal style rules (e.g., avoid unnecessary backticks
or concatenation). Ensure the final commit passes Biome checks.
- Around line 160-162: The embed description currently interpolates
`${message.author} reached **Level ${newLevel}**!${roleId ? ` 🏅 Role reward
assigned!` : ''}` and is sent via safeSend without mention-sanitization; wrap
the final description string with sanitizeMentions() before setting it (or call
sanitizeMentions() on the value passed into .setDescription or immediately
before safeSend) so the embed uses the sanitized text; reference
sanitizeMentions, .setDescription, and safeSend when making the change.
- Around line 84-90: handleXpGain currently dereferences message.guild.id and
proceeds before applying the bot-commands-channel exclusion; add a null-check
guard at the start of handleXpGain to return if message.guild is falsy, then
check repCfg (from getRepConfig) and immediately enforce the
bot-commands-channel exclusion (e.g., compare message.channel.id against the
configured bot-commands channel id in repCfg or config) before any XP/anti-spam
checks so no XP is processed for DMs or in the bot-commands channel.
- Around line 102-116: Move the cooldown write so it only happens after the DB
upsert succeeds: remove or postpone the cooldowns.set(key, now) call that
currently runs before the query and instead call cooldowns.set(key, now) after
the await pool.query(...) returns successfully in the same function (referencing
cooldowns, key, now, getPool, pool.query, xpGained, message.guild.id and
message.author.id to locate the code). Also wrap the pool.query call in a
try/catch, and in the catch log the error with contextual details (guild_id,
user_id, xpGained) before re-throwing to comply with the logging guideline.
In `@tests/commands/leaderboard.test.js`:
- Around line 98-108: The test incorrectly mocks getPool() to return null even
though src/db.js throws when uninitialized; update the test to simulate the real
failure path by making getPool() throw (e.g., mockImplementation(() => { throw
new Error(...) })) instead of returning null, then call execute(interaction) and
assert safeEditReply was called with an error message as before; reference
getPool, execute, and safeEditReply when locating the test to change.
- Line 1: The import ordering in tests/commands/leaderboard.test.js is flagged
by Biome; reorganize your imports so they follow the project's import-organize
rules (external modules first, then local files) and ensure the named specifiers
from 'vitest' (beforeEach, describe, expect, it, vi) are sorted alphabetically
and grouped correctly; update the import statement(s) accordingly (or run your
editor's/IDE's "organize imports" action) so the CI lint rule
assist/source/organizeImports passes.
In `@tests/commands/rank.test.js`:
- Around line 118-128: The test incorrectly simulates an unavailable DB by
making getPool.mockReturnValue(null); instead of throwing like production;
update the test to have getPool.mockImplementation(() => { throw new Error(...)
}) (or mockRejectedValue if async) so that execute(interaction) exercises the
real failure path and assertions on safeEditReply (and makeInteraction/execute)
remain valid; ensure you reference getPool() in the mock and keep the rest of
the test unchanged.
- Line 1: Remove the unused import symbol afterEach from the test file import
list and reorder the imports to satisfy organizeImports (keep beforeEach,
describe, expect, it, vi). Specifically, edit the import statement that
currently imports afterEach and remove afterEach from the named imports, then
run the project's import organizer/linter to ensure imports are sorted and the
noUnusedImports rule passes.
In `@tests/modules/reputation.test.js`:
- Line 1: The import block in tests/modules/reputation.test.js is misordered
(imports: afterEach, beforeEach, describe, expect, it, vi) causing
Biome/assist/source/organizeImports to fail; run Biome's import organizer (or
manually reorder/group and dedupe the imports) for that top import statement and
the other import blocks referenced (lines 33-40) so imports follow the project's
canonical order/formatting and the CI lint step passes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
config.jsonmigrations/007_reputation.cjssrc/api/utils/configAllowlist.jssrc/commands/leaderboard.jssrc/commands/rank.jssrc/modules/events.jssrc/modules/reputation.jstests/commands/leaderboard.test.jstests/commands/rank.test.jstests/modules/reputation.test.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Greptile Review
- GitHub Check: claude-review
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use ESM modules withimport/exportsyntax; never userequire()
Always usenode:protocol for Node.js builtins (e.g.,import { readFileSync } from 'node:fs')
Always use semicolons in code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Files:
src/modules/events.jssrc/api/utils/configAllowlist.jssrc/commands/rank.jssrc/commands/leaderboard.jssrc/modules/reputation.jstests/commands/leaderboard.test.jstests/commands/rank.test.jstests/modules/reputation.test.js
src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts,jsx,tsx}: Always use Winston for logging viaimport { info, warn, error } from '../logger.js'; never useconsole.log,console.warn,console.error, or anyconsole.*method in src/ files
Pass structured metadata to Winston logging calls (e.g.,info('Message processed', { userId, channelId }))
Use custom error classes fromsrc/utils/errors.jsfor error handling
Always log errors with context before re-throwing
UsegetConfig(guildId?)fromsrc/modules/config.jsto read configuration values
UsesetConfigValue(path, value, guildId?)fromsrc/modules/config.jsto update configuration at runtime
UsesafeSend()utility for all outgoing Discord messages to enforce allowedMentions and prevent mention spam
UsesanitizeMentions()to strip@everyone/@here from outgoing text via zero-width space insertion before sending
UsesplitMessage()utility to handle Discord's 2000-character message limit
onConfigChangecallbacks receive(newValue, oldValue, fullPath, guildId)as parameters
Files:
src/modules/events.jssrc/api/utils/configAllowlist.jssrc/commands/rank.jssrc/commands/leaderboard.jssrc/modules/reputation.js
src/modules/events.js
📄 CodeRabbit inference engine (AGENTS.md)
Module handler functions must be registered in
src/modules/events.jsviaclient.on()event listeners
Files:
src/modules/events.js
src/modules/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Check
config.yourModule.enabledbefore processing in module handlers
Files:
src/modules/events.jssrc/modules/reputation.js
src/commands/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/commands/**/*.{js,ts,jsx,tsx}: Slash commands must export adataproperty with a SlashCommandBuilder and an asyncexecute(interaction)function
ExportadminOnly = truefor moderator-only slash commands
Duration-based commands (timeout, tempban, slowmode) must useparseDuration()fromsrc/utils/duration.jsfor parsing duration arguments
Always callcheckHierarchy(moderator, target)before executing moderation actions to prevent moderating users with equal or higher roles
Files:
src/commands/rank.jssrc/commands/leaderboard.js
config.json
📄 CodeRabbit inference engine (AGENTS.md)
Document new config sections and keys in
README.md's config reference when updatingconfig.json
Files:
config.json
tests/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.{js,ts,jsx,tsx}: All new code must include tests; test coverage must maintain 80% threshold on statements, branches, functions, and lines
Use Vitest for testing; runpnpm testbefore every commit andpnpm test:coverageto verify 80% coverage threshold
Files:
tests/commands/leaderboard.test.jstests/commands/rank.test.jstests/modules/reputation.test.js
🧠 Learnings (5)
📚 Learning: 2026-02-26T22:59:10.394Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-26T22:59:10.394Z
Learning: Applies to src/**/*.{js,ts,jsx,tsx} : Use `safeSend()` utility for all outgoing Discord messages to enforce allowedMentions and prevent mention spam
Applied to files:
src/modules/events.js
📚 Learning: 2026-02-26T22:59:10.394Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-26T22:59:10.394Z
Learning: Applies to src/commands/**/*{ban,kick,warn,timeout,mute}*.{js,ts,jsx,tsx} : Moderation commands must follow the pattern: deferReply() → validate → sendDmNotification() → execute Discord action → createCase() → sendModLogEmbed() → checkEscalation()
Applied to files:
src/modules/events.jssrc/commands/rank.jssrc/commands/leaderboard.jstests/commands/leaderboard.test.js
📚 Learning: 2026-02-26T22:59:10.394Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-26T22:59:10.394Z
Learning: Applies to config.json : Document new config sections and keys in `README.md`'s config reference when updating `config.json`
Applied to files:
src/api/utils/configAllowlist.jsconfig.json
📚 Learning: 2026-02-26T22:59:10.394Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-26T22:59:10.394Z
Learning: Applies to src/commands/**/*.{js,ts,jsx,tsx} : Slash commands must export a `data` property with a SlashCommandBuilder and an async `execute(interaction)` function
Applied to files:
src/commands/rank.jssrc/commands/leaderboard.jstests/commands/leaderboard.test.jstests/commands/rank.test.js
📚 Learning: 2026-02-26T22:59:10.394Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-26T22:59:10.394Z
Learning: Applies to tests/**/*.{js,ts,jsx,tsx} : All new code must include tests; test coverage must maintain 80% threshold on statements, branches, functions, and lines
Applied to files:
tests/modules/reputation.test.js
🧬 Code graph analysis (7)
src/modules/events.js (1)
src/modules/reputation.js (1)
handleXpGain(84-179)
src/commands/rank.js (4)
src/modules/reputation.js (7)
DEFAULT_THRESHOLDS(21-21)pool(104-104)pool(107-116)thresholds(119-119)level(50-50)computeLevel(49-59)buildProgressBar(69-76)src/db.js (1)
getPool(142-147)src/utils/safeSend.js (1)
safeEditReply(178-185)src/modules/config.js (1)
getConfig(282-313)
src/commands/leaderboard.js (3)
src/commands/rank.js (4)
execute(29-98)pool(32-32)pool(44-47)embed(77-95)src/db.js (1)
getPool(142-147)src/utils/safeSend.js (1)
safeEditReply(178-185)
src/modules/reputation.js (5)
tests/modules/reputation.test.js (1)
DEFAULT_THRESHOLDS(42-42)src/modules/config.js (2)
getConfig(282-313)err(94-94)src/db.js (1)
getPool(142-147)src/logger.js (1)
info(230-232)src/utils/safeSend.js (1)
safeSend(116-123)
tests/commands/leaderboard.test.js (3)
src/commands/leaderboard.js (3)
pool(24-24)pool(30-37)execute(21-69)src/db.js (1)
getPool(142-147)src/utils/safeSend.js (1)
safeEditReply(178-185)
tests/commands/rank.test.js (4)
src/commands/rank.js (4)
target(38-38)pool(32-32)pool(44-47)execute(29-98)src/modules/reputation.js (2)
pool(104-104)pool(107-116)src/db.js (1)
getPool(142-147)src/utils/safeSend.js (1)
safeEditReply(178-185)
tests/modules/reputation.test.js (3)
src/modules/reputation.js (9)
DEFAULT_THRESHOLDS(21-21)computeLevel(49-59)bar(74-74)buildProgressBar(69-76)pool(104-104)pool(107-116)handleXpGain(84-179)announceChannelId(153-153)announceChannel(155-155)src/db.js (1)
getPool(142-147)src/utils/safeSend.js (1)
safeSend(116-123)
🪛 GitHub Actions: CI
src/commands/rank.js
[error] 62-62: lint/style/useTemplate: Unsafe fixable rule - Use a template literal instead of string concatenation. Suggested fix: ${'▓'.repeat(10)} MAX
[error] 58-58: Formatter would have printed different content. Apply formatting changes suggested by Biome formatter.
src/modules/reputation.js
[error] 70-70: lint/style/useTemplate: Unsafe fixable rule - Use a template literal instead of string concatenation. Suggested fix: ${'▓'.repeat(width)} 100%
[error] 122-122: Formatter would have printed different content. Apply formatting changes suggested by Biome formatter (SQL string formatting).
[error] 8-8: assist/source/organizeImports: Imports are not sorted. Organize imports to fix ordering.
tests/commands/leaderboard.test.js
[error] 1-1: assist/source/organizeImports: Imports are not sorted. Organize imports to fix ordering.
tests/commands/rank.test.js
[error] 1-1: lint/correctness/noUnusedImports: Unused imports detected in test file. Remove unused imports to fix.
[error] 1-1: assist/source/organizeImports: Imports are not sorted. Organize imports to fix ordering.
tests/modules/reputation.test.js
[error] 1-1: assist/source/organizeImports: Imports are not sorted. Organize imports to fix ordering.
🔇 Additional comments (2)
src/api/utils/configAllowlist.js (1)
21-21: Looks good: reputation config is correctly allowlisted.Adding
'reputation'toSAFE_CONFIG_KEYSis the correct integration point for API-readable/writable config sections.src/modules/events.js (1)
143-150:⚠️ Potential issue | 🟠 MajorXP awarding path is missing bot-command-channel exclusion.
Line 143 runs XP gain for all eligible messages, but anti-abuse requirements call out excluding bot-command channels. The current flow (with
src/modules/reputation.js:83-178) does not show that guard.⛔ Skipped due to learnings
Learnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-26T22:59:10.394Z Learning: Applies to src/commands/**/*{ban,kick,warn,timeout,mute}*.{js,ts,jsx,tsx} : Moderation commands must follow the pattern: deferReply() → validate → sendDmNotification() → execute Discord action → createCase() → sendModLogEmbed() → checkEscalation()Learnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-26T22:59:10.394Z Learning: Applies to src/commands/**/*{ban,kick}*.{js,ts,jsx,tsx} : Moderation commands must DM the target user before executing kicks/bans (as users cannot receive DMs after being kicked/banned)Learnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-26T22:59:10.394Z Learning: Applies to src/modules/events.js : Module handler functions must be registered in `src/modules/events.js` via `client.on()` event listeners
There was a problem hiding this comment.
Review Summary — 7 issues found
🔴 Critical (1)
DEFAULT_THRESHOLDSundefined inreputation.js:106— Will throwReferenceErrorat runtime whenever a user earns XP. The variable is never imported or defined.
🟡 Warnings (4)
- Missing
enabledgate in/rank(rank.js:28) — Command runs even whenreputation.enabledisfalse. - Missing
enabledgate in/leaderboard(leaderboard.js:22) — Same issue. - Missing
allowedCommandsentries (config.json) —rankandleaderboardnot registered inpermissions.allowedCommands. - Unbounded cooldown map (
reputation.js:16) — Memory leak: Map grows per unique guild:user pair, never pruned.
🔵 Nitpicks (2)
displayAvatarURL({ dynamic: true })deprecated (reputation.js:151,rank.js:76,89) — v13 option, silently ignored in v14.- Sequential member fetches in leaderboard (
leaderboard.js:47-59) — 10 individual API calls could be batched.
See inline comments for fix suggestions.
There was a problem hiding this comment.
Pull request overview
This PR introduces a configurable XP/reputation system to gamify community participation, including persistence (migration), message-driven XP gain, and new /rank and /leaderboard commands for visibility.
Changes:
- Adds a
reputationdatabase table + index to support per-guild XP tracking and leaderboard queries. - Implements XP awarding, cooldown/anti-spam checks, level-up announcements, and role rewards.
- Adds
/rankand/leaderboardslash commands with supporting config + tests.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
migrations/007_reputation.cjs |
Creates the reputation table and guild/xp index for leaderboard performance. |
src/modules/reputation.js |
Core XP awarding + leveling logic, announcements, and role reward assignment. |
src/modules/events.js |
Hooks XP gain handler into the messageCreate pipeline (fire-and-forget). |
src/commands/rank.js |
Implements /rank command with progress bar and server rank calculation. |
src/commands/leaderboard.js |
Implements /leaderboard command showing top 10 by XP. |
src/api/utils/configAllowlist.js |
Allows reputation config section via the API allowlist. |
config.json |
Adds default reputation config (disabled by default). |
tests/modules/reputation.test.js |
Adds unit coverage for XP gain, cooldowns, level-up, announcements, role rewards. |
tests/commands/rank.test.js |
Adds /rank command tests for typical and error paths. |
tests/commands/leaderboard.test.js |
Adds /leaderboard command tests for ordering and resilience. |
pnpm-lock.yaml |
Updates lockfile due to dependency graph changes. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found (1 critical, 3 warnings, 2 nitpicks). The core implementation is solid — well-structured XP engine, atomic DB operations, parameterized queries, proper sanitization, static Winston imports, enabled gates, getPool inside try/catch, and good per-file test coverage. Most previous review findings have been addressed across 8 commits.
🔴 Critical (1) — CI Blocking
- Untested sweep callback (
reputation.js:20-29) — Lines 22-24 are the only uncovered branches in this file (87.09% branch). Global branch coverage is 79.73% < 80%. Extract the sweep into a testable function and add a unit test.
🟡 Warnings (3)
- Level UPDATE query lacks error handling (
reputation.js:113-118) — Role assignment and announcement both have try/catch, but the level UPDATE does not. Per AGENTS.md: "Always log errors with context before re-throwing." - Leaderboard test mock doesn't match batch fetch API (
leaderboard.test.js:62) — Mock returns a plain object, but productionguild.members.fetch({ user: [...] })returns a Collection (iterable Map). The happy path (display name resolution) silently falls back to mention format and is never actually tested. - README.md / AGENTS.md not updated — Per AGENTS.md: new config sections must be documented in README.md config reference; new files must be added to AGENTS.md Key Files table.
🔵 Nitpicks (2)
- xpPerMessage handlers allow inverted ranges (
config-editor.tsx:1249-1264) — Min can exceed max, breaking XP award math. - Level thresholds not normalized (
config-editor.tsx:1288-1291) — Unsorted/duplicate values can be saved, breakingcomputeLevel().
AI prompt to fix all issues
Fix the following issues in VolvoxLLC/volvox-bot on branch feat/reputation-system:
-
src/modules/reputation.js lines 20-29: Extract the anonymous setInterval sweep callback into an exported function
_sweepExpiredCooldowns(map, maxAge = 120_000)that iterates the map and deletes entries whereDate.now() - ts > maxAge. Then call it from the setInterval:setInterval(() => _sweepExpiredCooldowns(cooldowns), 5 * 60 * 1000).unref(); -
tests/modules/reputation.test.js: Import
_sweepExpiredCooldownsfrom reputation.js and add a test that creates a Map with one expired entry (Date.now() - 200_000) and one fresh entry (Date.now()), calls_sweepExpiredCooldowns(map), and asserts only the fresh entry remains. -
src/modules/reputation.js lines 113-118: Wrap the
pool.query('UPDATE reputation SET level...')call in a try/catch. In the catch, calllogError('Failed to update user level', { guildId, userId, newLevel, error: err.message })and return early (skip announcement). -
tests/commands/leaderboard.test.js line 62: Fix the guild.members.fetch mock to return a Map (matching discord.js Collection API):
fetch: vi.fn().mockImplementation(({ user: ids }) => { const map = new Map(); for (const id of ids) map.set(id, { displayName: 'User_' + id }); return Promise.resolve(map); }) -
README.md: Add a Reputation config reference table documenting: enabled (boolean, default false), xpPerMessage (array [min, max], default [5, 15]), xpCooldownSeconds (number, default 60), announceChannelId (string|null, default null), levelThresholds (number[], default [100..12000]), roleRewards (object level→roleId, default {}). Add /rank and /leaderboard to the command list.
-
AGENTS.md: Add reputation.js, reputationDefaults.js, rank.js, leaderboard.js to the Key Files table.
-
web/src/components/dashboard/config-editor.tsx lines 1249-1264: In the xpPerMessage min input onChange, clamp:
const nextMin = Math.min(num, range[1]). In the max input onChange:const nextMax = Math.max(num, range[0]). -
web/src/components/dashboard/config-editor.tsx lines 1288-1291: After parsing nums, sort and dedup:
const thresholds = [...new Set(nums)].sort((a, b) => a - b).
Run pnpm test:coverage after changes to verify branch coverage >= 80%.
Additional Comments (2)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/commands/leaderboard.js (1)
31-40: 🧹 Nitpick | 🔵 TrivialConsider explicit pool null check for a clearer user-facing error.
If
getPool()returnsnull(database unavailable), the error is caught generically. An explicit check could provide a more specific message like "❌ Database is not available."This is a minor UX improvement; the current try/catch ensures the command won't crash.
Optional: Add explicit pool check
try { const pool = getPool(); + if (!pool) { + await safeEditReply(interaction, { content: '❌ Database is not available.' }); + return; + } const { rows } = await pool.query(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/leaderboard.js` around lines 31 - 40, Add an explicit null check after calling getPool() in src/commands/leaderboard.js: if pool is null/undefined, send a clear user-facing reply such as "❌ Database is not available." and return early instead of letting the try/catch handle it generically; place the check immediately after the const pool = getPool() line and before using pool.query to avoid running the query when no connection exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/modules/reputation.test.js`:
- Around line 176-197: Add a test that verifies handleXpGain propagates a failed
upsert by mocking getConfig with reputation enabled, mocking getPool to return a
pool whose query method rejects (e.g., new Error('DB connection lost')),
creating a message via makeMessage with guildId/userId, and asserting await
expect(handleXpGain(message)).rejects.toThrow('DB connection lost'); this
targets the upsert/query failure path in handleXpGain (the INSERT/UPSERT DB
call).
---
Duplicate comments:
In `@src/commands/leaderboard.js`:
- Around line 31-40: Add an explicit null check after calling getPool() in
src/commands/leaderboard.js: if pool is null/undefined, send a clear user-facing
reply such as "❌ Database is not available." and return early instead of letting
the try/catch handle it generically; place the check immediately after the const
pool = getPool() line and before using pool.query to avoid running the query
when no connection exists.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
README.mdsrc/api/utils/configAllowlist.jssrc/commands/leaderboard.jssrc/commands/rank.jssrc/modules/reputation.jstests/commands/leaderboard.test.jstests/commands/rank.test.jstests/modules/reputation.test.jsweb/src/components/dashboard/config-editor.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 only — import/export, never require()
Always use node: protocol for Node.js builtin imports (e.g., import { readFileSync } from 'node:fs')
Files:
src/api/utils/configAllowlist.jstests/modules/reputation.test.jssrc/modules/reputation.jssrc/commands/rank.jstests/commands/leaderboard.test.jssrc/commands/leaderboard.jstests/commands/rank.test.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,tsx}: Always use semicolons
Use single quotes — enforced by Biome
Use 2-space indentation — enforced by Biome
Files:
src/api/utils/configAllowlist.jsweb/src/components/dashboard/config-editor.tsxtests/modules/reputation.test.jssrc/modules/reputation.jssrc/commands/rank.jstests/commands/leaderboard.test.jssrc/commands/leaderboard.jstests/commands/rank.test.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: NEVER use console.log, console.warn, console.error, or any console.* method in src/ files — always use Winston logger instead: import { info, warn, error } from '../logger.js'
Pass structured metadata to Winston logger: info('Message processed', { userId, channelId })
Use custom error classes from src/utils/errors.js and always log errors with context before re-throwing
Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime
Use safeSend() utility for outgoing Discord messages to sanitize mentions and enforce allowedMentions
Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit
Files:
src/api/utils/configAllowlist.jssrc/modules/reputation.jssrc/commands/rank.jssrc/commands/leaderboard.js
web/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use next/image Image component with appropriate layout and sizing props in Next.js components
Files:
web/src/components/dashboard/config-editor.tsx
web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use Zustand store (zustand) for state management in React components; implement fetch-on-demand pattern in stores
Files:
web/src/components/dashboard/config-editor.tsx
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/modules/*.js: Register event handlers in src/modules/events.js by importing handler functions and calling client.on() with config parameter
Check config.yourModule.enabled before processing in module event handlers
Prefer per-request getConfig() pattern in new modules over reactive onConfigChange() wiring; only add onConfigChange() listeners for stateful resources that cannot re-read config on each use
Files:
src/modules/reputation.js
src/commands/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/commands/*.js: Use parseDuration() from src/utils/duration.js for duration-based commands (timeout, tempban, slowmode); enforce Discord duration caps (timeouts max 28 days, slowmode max 6 hours)
Create slash commands by exporting data (SlashCommandBuilder) and execute() function from src/commands/*.js; export adminOnly = true for mod-only commands; commands are auto-discovered on startup
Files:
src/commands/rank.jssrc/commands/leaderboard.js
🧠 Learnings (16)
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime
Applied to files:
web/src/components/dashboard/config-editor.tsx
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit
Applied to files:
tests/modules/reputation.test.jssrc/modules/reputation.jstests/commands/rank.test.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/commands/*mod*.js : Moderation commands must follow the shared pattern: deferReply({ ephemeral: true }), validate inputs, sendDmNotification(), execute Discord action, createCase(), sendModLogEmbed(), checkEscalation()
Applied to files:
src/modules/reputation.jssrc/commands/rank.jssrc/commands/leaderboard.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/modules/moderation.js : Case numbering must be per-guild sequential, assigned atomically using COALESCE(MAX(case_number), 0) + 1 in a single INSERT statement
Applied to files:
src/modules/reputation.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Use safeSend() utility for outgoing Discord messages to sanitize mentions and enforce allowedMentions
Applied to files:
src/modules/reputation.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/modules/triage.js : Triage tracks at most 100 channels; channels inactive for 30 minutes are evicted. If a channel is evicted mid-conversation, restart evaluation from scratch
Applied to files:
src/modules/reputation.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to **/*.{js,ts,tsx} : Use single quotes — enforced by Biome
Applied to files:
src/modules/reputation.jssrc/commands/rank.jstests/commands/leaderboard.test.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to **/*.{js,ts,tsx} : Use 2-space indentation — enforced by Biome
Applied to files:
src/modules/reputation.jssrc/commands/rank.jstests/commands/leaderboard.test.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/index.js : The tempban scheduler runs on a 60s interval and catches up on missed unbans after restart; started in index.js startup and stopped in graceful shutdown
Applied to files:
src/modules/reputation.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/commands/*.js : Use parseDuration() from src/utils/duration.js for duration-based commands (timeout, tempban, slowmode); enforce Discord duration caps (timeouts max 28 days, slowmode max 6 hours)
Applied to files:
src/modules/reputation.jssrc/commands/rank.jssrc/commands/leaderboard.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/commands/*.js : Create slash commands by exporting data (SlashCommandBuilder) and execute() function from src/commands/*.js; export adminOnly = true for mod-only commands; commands are auto-discovered on startup
Applied to files:
src/commands/rank.jstests/commands/leaderboard.test.jssrc/commands/leaderboard.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/commands/*mod*.js : Always call checkHierarchy(moderator, target) before executing moderation actions to prevent moderating users with equal or higher roles
Applied to files:
src/commands/rank.jssrc/commands/leaderboard.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/db.js : Database connection requires PostgreSQL (via pg driver with raw SQL, no ORM); the bot works without a database but config persistence requires PostgreSQL
Applied to files:
src/commands/rank.jssrc/commands/leaderboard.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : NEVER use console.log, console.warn, console.error, or any console.* method in src/ files — always use Winston logger instead: import { info, warn, error } from '../logger.js'
Applied to files:
src/commands/rank.jssrc/commands/leaderboard.js
📚 Learning: 2026-02-27T16:24:15.055Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.055Z
Learning: Applies to src/logger.js : Logging transport must use Winston with daily file rotation; the PostgreSQL logging transport is a stateful long-lived transport requiring reactive onConfigChange() wiring
Applied to files:
src/commands/rank.jssrc/commands/leaderboard.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Use custom error classes from src/utils/errors.js and always log errors with context before re-throwing
Applied to files:
src/commands/rank.jssrc/commands/leaderboard.js
🧬 Code graph analysis (3)
tests/modules/reputation.test.js (2)
src/modules/reputation.js (10)
computeLevel(48-58)bar(73-73)buildProgressBar(68-75)pool(102-102)pool(105-114)handleXpGain(83-192)announceChannelId(164-164)announceChannel(166-166)roleId(148-148)sweepCooldowns(20-25)src/utils/safeSend.js (1)
safeSend(116-123)
src/modules/reputation.js (6)
src/modules/config.js (2)
getConfig(282-313)err(94-94)src/modules/reputationDefaults.js (2)
REPUTATION_DEFAULTS(6-13)REPUTATION_DEFAULTS(6-13)src/db.js (1)
getPool(142-147)src/logger.js (1)
info(230-232)src/utils/sanitizeMentions.js (1)
sanitizeMentions(37-43)src/utils/safeSend.js (1)
safeSend(116-123)
tests/commands/rank.test.js (4)
src/commands/rank.js (3)
target(42-42)pool(41-41)pool(47-50)src/db.js (1)
getPool(142-147)src/utils/safeSend.js (1)
safeEditReply(178-185)src/modules/config.js (1)
getConfig(282-313)
🔇 Additional comments (21)
src/api/utils/configAllowlist.js (1)
22-23: LGTM!The addition of
'reputation'and'github'toSAFE_CONFIG_KEYScorrectly exposes these new config sections via the API, aligning with the new features introduced in this PR.README.md (1)
216-228: LGTM!Clear and comprehensive documentation for the new Reputation/XP system. The config keys and their descriptions accurately reflect the implementation in
src/modules/reputationDefaults.js.src/modules/reputation.js (7)
36-39: Shallow merge is acceptable for the current flat config shape.The spread operator performs a shallow merge, which works for the current config structure with mostly primitives. If
roleRewardsneeds partial merging in the future, consider a deeper merge strategy.
48-58: LGTM!
computeLevelcorrectly iterates through thresholds and returns the appropriate level based on cumulative XP.
68-75: LGTM!
buildProgressBarhandles edge cases well, including the division-by-zero guard whenneeded <= 0.
102-117: LGTM!Cooldown is correctly set after the successful DB write, addressing the previous review concern about suppressing legitimate XP on query failure.
125-138: LGTM!Level update query is properly wrapped in try/catch with structured error logging and early return on failure.
168-189: LGTM!The announcement embed correctly uses
sanitizeMentions()on the description and wrapssafeSend()in try/catch with proper error logging.
83-90: AddexcludedChannelIdssupport to reputation config.The reputation XP system currently lacks channel-based exclusion. Add support for excluding specific channels (e.g., bot-commands) by including the
excludedChannelIdsfield in the reputation configuration.Proposed change
export async function handleXpGain(message) { if (!message.guild) return; const repCfg = getRepConfig(message.guild.id); if (!repCfg.enabled) return; + if (repCfg.excludedChannelIds?.includes(message.channelId)) return; // Minimum length check (anti-spam) if (message.content.length < 10) return;Also add
excludedChannelIds: []toREPUTATION_DEFAULTSinsrc/modules/reputationDefaults.js.src/commands/rank.js (1)
28-101: LGTM!The
/rankcommand implementation is well-structured:
- Guild-context guard at the start
- Reputation enabled check
- DB operations wrapped in try/catch with proper error logging
- Fallbacks for missing data (xp ?? 0, rank ?? 1)
- Clean embed construction with progress bar
tests/commands/rank.test.js (2)
142-154: LGTM!The DB unavailability test correctly mocks
getPool()to throw an error, which matches the actual behavior ofsrc/db.jswhen the database is not initialized.
1-272: Comprehensive test coverage for/rankcommand.Tests cover key scenarios including:
- No data / has data states
- DB unavailability (correctly throws)
- Reputation disabled
- Target user lookup
- Max level display
- Fallback behaviors (interaction.user, rank ?? 1, displayName ?? username)
web/src/components/dashboard/config-editor.tsx (5)
51-51: LGTM!The
knownSectionsarray correctly includes the new config sections (reputation,afk,github) to enable the type guard to recognize these sections in API responses.
1242-1267: LGTM!The XP per message handlers now correctly enforce
min <= maxinvariants by adjusting the counterpart value when needed.
1290-1296: LGTM!Level thresholds are now sorted before being stored, ensuring a strictly ascending sequence for correct level computation.
1227-1301: LGTM!The Reputation / XP settings card provides a clean UI for configuring:
- Enable toggle
- XP per message range with bounds enforcement
- Cooldown seconds
- Announce channel ID
- Level thresholds with auto-sorting
1303-1335: LGTM!The GitHub Activity Feed settings card is well-structured with appropriate inputs for channel ID and poll interval.
tests/modules/reputation.test.js (1)
1-545: Comprehensive test coverage for the reputation module.Excellent coverage including:
computeLevel: 0 XP, first threshold, max threshold, intermediate levelsbuildProgressBar: segment rendering, 100%, 0%, division guardhandleXpGain: disabled config, short messages, XP award, cooldown enforcement/expiry, level-up detection, role rewards, error handling for role assignment, announcement send failures, level update failuressweepCooldowns: stale entry removal, fresh entry retentiontests/commands/leaderboard.test.js (2)
122-134: LGTM!The DB unavailability test correctly mocks
getPool()to throw an error, matching the actual behavior ofsrc/db.js.
1-166: Good test coverage for/leaderboardcommand.Tests cover:
- Empty state (no XP rows)
- Top 10 ordering with proper SQL assertions
- DB unavailability (correctly throws)
- Reputation disabled
- Member fetch failure (graceful fallback)
src/commands/leaderboard.js (1)
1-79: Well-structured command implementation with proper error handling.The code follows all applicable coding guidelines: ESM imports, Winston logger with structured metadata,
getConfig()for configuration, andsafeEditReply()for Discord replies. The try/catch block (lines 31–78) now wraps the main DB and member-fetch logic, addressing the core concern from the previous review about unhandled runtime failures.The batch-fetch approach (line 52) with a graceful fallback to mention format is a good optimization over individual fetches.
Additional Comments (1)
|
BillChirico
force-pushed
the
feat/reputation-system
branch
from
c1f9d28 to
c609a05
Compare
…mmands - Add migrations/007_reputation.cjs with reputation table and guild_xp index - Add src/modules/reputation.js with handleXpGain, computeLevel, buildProgressBar - 5-15 XP per message, 60s cooldown, 10-char minimum - Level-up announcements via safeSend, role reward auto-assignment - Add src/commands/rank.js: /rank [user] with XP progress bar and server rank - Add src/commands/leaderboard.js: /leaderboard showing top 10 by XP - Wire handleXpGain into src/modules/events.js messageCreate handler - Add reputation config to config.json and SAFE_CONFIG_KEYS - Add 22 tests covering XP gain, cooldown, level-up, role rewards, commands Closes #45
…or reputation/github/afk - Single source of truth: src/modules/reputationDefaults.js - Removed duplicate DEFAULT_THRESHOLDS from reputation.js and rank.js - Dashboard: full Reputation card (XP range, cooldown, announce channel, level thresholds) - Dashboard: GitHub Feed card (enabled, channel, poll interval) - Dashboard: AFK toggle in Community Features - Added afk + github to SAFE_CONFIG_KEYS and knownSections
- Fix getPool() null-check dead code in rank/leaderboard (getPool throws, never returns null) - Move getPool() inside try/catch blocks so DB errors produce user-friendly replies - Add reputation.enabled gate to rank and leaderboard commands - Move cooldown.set() after successful DB write in reputation.js - Add TTL eviction for cooldown Map entries to prevent memory leak - Apply sanitizeMentions() to level-up embed description - Add voice_minutes and helps_given columns to migration (issue #45) - Add IF NOT EXISTS to reputation index creation for idempotency - Add rank and leaderboard to config.json allowedCommands - Fix tests: mock getPool() throwing (not returning null), update config mocks - Remove unused afterEach import in rank.test.js
… null guard, add disabled tests
- Add test for xp > all thresholds (nextThreshold = null / MAX LEVEL path) - Add test for user with null displayName to cover username fallback branch - Brings rank.js branch coverage from ~67% to 91.66% - Fixes CI branch coverage threshold failure (was 79.25% < 80%)
BillChirico
force-pushed
the
feat/reputation-system
branch
from
c609a05 to
79c82a9
Compare
| const pool = getPool(); | ||
|
|
||
| // Upsert reputation row | ||
| const { rows } = await pool.query( | ||
| `INSERT INTO reputation (guild_id, user_id, xp, messages_count, last_xp_gain) | ||
| VALUES ($1, $2, $3, 1, NOW()) | ||
| ON CONFLICT (guild_id, user_id) DO UPDATE | ||
| SET xp = reputation.xp + $3, | ||
| messages_count = reputation.messages_count + 1, | ||
| last_xp_gain = NOW() | ||
| RETURNING xp, level`, | ||
| [message.guild.id, message.author.id, xpGained], | ||
| ); |
There was a problem hiding this comment.
🟡 Warning: Upsert query lacks error handling — unhandled rejection in fire-and-forget context
The upsert pool.query (lines 105-114) is not wrapped in a try/catch. If the DB connection drops or the query fails, the error propagates up to the events.js fire-and-forget .catch() handler — which only logs it. However, the user silently loses XP with no structured context logged at the point of failure.
The level UPDATE (line 126), role assignment (line 151), and announcement send (line 180) all have try/catch with structured logging, but this primary query does not. Per AGENTS.md: "Always log errors with context before re-throwing."
| const pool = getPool(); | |
| // Upsert reputation row | |
| const { rows } = await pool.query( | |
| `INSERT INTO reputation (guild_id, user_id, xp, messages_count, last_xp_gain) | |
| VALUES ($1, $2, $3, 1, NOW()) | |
| ON CONFLICT (guild_id, user_id) DO UPDATE | |
| SET xp = reputation.xp + $3, | |
| messages_count = reputation.messages_count + 1, | |
| last_xp_gain = NOW() | |
| RETURNING xp, level`, | |
| [message.guild.id, message.author.id, xpGained], | |
| ); | |
| let rows; | |
| try { | |
| ({ rows } = await pool.query( | |
| `INSERT INTO reputation (guild_id, user_id, xp, messages_count, last_xp_gain) | |
| VALUES ($1, $2, $3, 1, NOW()) | |
| ON CONFLICT (guild_id, user_id) DO UPDATE | |
| SET xp = reputation.xp + $3, | |
| messages_count = reputation.messages_count + 1, | |
| last_xp_gain = NOW() | |
| RETURNING xp, level`, | |
| [message.guild.id, message.author.id, xpGained], | |
| )); | |
| } catch (err) { | |
| logError('Failed to persist XP gain', { | |
| guildId: message.guild.id, | |
| userId: message.author.id, | |
| xpGained, | |
| error: err.message, | |
| }); | |
| return; | |
| } |
| | `enabled` | boolean | Enable the XP / leveling system | | ||
| | `xpPerMessage` | [number, number] | Random XP range awarded per message `[min, max]` (default: `[5, 15]`) | | ||
| | `xpCooldownSeconds` | number | Minimum seconds between XP awards per user (default: `60`) | | ||
| | `announceChannelId` | string\|null | Channel ID for level-up announcements (null = DM user) | |
There was a problem hiding this comment.
🟡 Warning: Documentation says "null = DM user" but implementation skips announcements
The description for announceChannelId says (null = DM user), but the actual implementation in reputation.js:164-190 simply skips the announcement when announceChannelId is null — it never DMs the user. Either implement DM functionality or fix the docs:
| | `announceChannelId` | string\|null | Channel ID for level-up announcements (null = DM user) | | |
| | `announceChannelId` | string\|null | Channel ID for level-up announcements (null = no announcement) | |
| export async function execute(interaction) { | ||
| await interaction.deferReply(); | ||
|
|
||
| const cfg = getConfig(interaction.guildId); |
There was a problem hiding this comment.
🟡 Warning: Missing guild-context guard
/rank has a !interaction.guildId guard (rank.js:31-33) but /leaderboard does not. If somehow invoked outside a guild context, getConfig(interaction.guildId) receives undefined and the query on line 39 uses undefined as a parameter.
| const cfg = getConfig(interaction.guildId); | |
| const cfg = getConfig(interaction.guildId); | |
| if (!interaction.guildId || !cfg?.reputation?.enabled) { |
|
|
||
| /** | ||
| * Build a progress bar string. | ||
| * |
There was a problem hiding this comment.
🔵 Nitpick: computeLevel uses early-break optimization that assumes sorted thresholds
If levelThresholds is ever unsorted (e.g. via the web UI config editor), the break on line 54 will produce incorrect results. The function should either validate/sort the input or iterate all entries. Currently the config editor sorts on save (line 1293), but computeLevel itself is a public export with no such guarantee.
Consider documenting the sorted-ascending precondition in the JSDoc, or removing the break to iterate all entries (negligible cost for 10 elements).
| {/* ═══ GitHub Feed Settings ═══ */} | ||
| <Card> | ||
| <CardContent className="space-y-4 pt-6"> | ||
| <div className="flex items-center justify-between"> | ||
| <CardTitle className="text-base">GitHub Activity Feed</CardTitle> | ||
| <ToggleSwitch | ||
| checked={draftConfig.github?.feed?.enabled ?? false} | ||
| onChange={(v) => setDraftConfig((prev) => ({ ...prev, github: { ...prev.github, feed: { ...prev.github?.feed, enabled: v } } }))} | ||
| disabled={saving} | ||
| label="GitHub Feed" | ||
| /> | ||
| </div> | ||
| <div className="grid grid-cols-2 gap-4"> | ||
| <label className="space-y-2"> | ||
| <span className="text-sm font-medium">Feed Channel ID</span> | ||
| <input type="text" | ||
| value={draftConfig.github?.feed?.channelId ?? ""} | ||
| onChange={(e) => setDraftConfig((prev) => ({ ...prev, github: { ...prev.github, feed: { ...prev.github?.feed, channelId: e.target.value.trim() || null } } }))} | ||
| disabled={saving} className={inputClasses} placeholder="Channel ID for GitHub updates" /> | ||
| </label> | ||
| <label className="space-y-2"> | ||
| <span className="text-sm font-medium">Poll Interval (minutes)</span> | ||
| <input type="number" min={1} | ||
| value={draftConfig.github?.feed?.pollIntervalMinutes ?? 5} | ||
| onChange={(e) => { | ||
| const num = parseNumberInput(e.target.value, 1); | ||
| if (num !== undefined) setDraftConfig((prev) => ({ ...prev, github: { ...prev.github, feed: { ...prev.github?.feed, pollIntervalMinutes: num } } })); | ||
| }} | ||
| disabled={saving} className={inputClasses} /> | ||
| </label> | ||
| </div> | ||
| </CardContent> | ||
| </Card> |
There was a problem hiding this comment.
🔵 Nitpick: Out-of-scope UI for unimplemented features
The GitHub Activity Feed settings card and the 'github' addition to configAllowlist.js are not part of the reputation feature described in the PR title/summary. Consider documenting this preparatory work in the PR description or splitting it into its own PR for cleaner change tracking.
There was a problem hiding this comment.
3 warnings and 2 nitpicks found. See inline comments for details.
Warnings (3)
- Upsert query lacks error handling (src/modules/reputation.js:102-114) — The primary XP upsert pool.query is not wrapped in try/catch, unlike the level UPDATE, role assignment, and announcement send.
- README docs say null=DM user but implementation skips announcements (README.md:223) — announceChannelId description claims null DMs the user, but reputation.js:164-190 simply skips the announcement.
- Missing guild-context guard in /leaderboard (src/commands/leaderboard.js:26) — /rank has a !interaction.guildId guard but /leaderboard does not.
Nitpicks (2)
- computeLevel early-break assumes sorted thresholds (src/modules/reputation.js:48-58)
- Out-of-scope GitHub Feed UI (config-editor.tsx:1303-1335)
Summary
Implements a gamified XP/reputation system that rewards community participation with levels and role rewards.
Changes
migrations/007_reputation.cjs—reputationtable with guild/user unique constraint andidx_reputation_guild_xpindex for leaderboard queriessrc/modules/reputation.js— Core XP engine:handleXpGain(message)called on every non-bot messagesafeSendreputation.roleRewardsconfigsrc/commands/rank.js—/rank [user]showing level, XP,▓▓▓░░░ 60%progress bar, server ranksrc/commands/leaderboard.js—/leaderboardtop 10 by XP with medal emojissrc/modules/events.js— WireshandleXpGainintomessageCreate(fire-and-forget, errors logged not thrown)config.json— Addedreputationsection (enabled: falseby default)src/api/utils/configAllowlist.js— AddedreputationtoSAFE_CONFIG_KEYSTests
22 new tests across 3 files:
tests/modules/reputation.test.js— XP award, cooldown enforcement, short message rejection, level-up detection, announcement, role assignmenttests/commands/rank.test.js— Rank display, no-data state, DB unavailable, user lookuptests/commands/leaderboard.test.js— Top 10 ordering, empty state, DB unavailable, graceful member fetch failureAll 1735 existing tests continue to pass.
Closes #45