feat(xp): level-up action pipeline engine (Phase 1)#373
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
🚅 Deployed to the volvox-bot-pr-373 environment in volvox-bot
|
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughMoves XP/leveling config from Changes
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. ✨ Finishing Touches📝 Generate docstrings
🧪 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/levelUpActions.js | New pipeline executor: resolves actions per level (including skips), manages rate-limit quota centrally, and dispatches to action registry handlers with per-action error isolation. Design is solid. |
| src/modules/actions/roleUtils.js | Role permission helpers, sliding-window rate limiter, and enforceRoleLevelDown. collectXpManagedRoles now correctly covers both levelActions and defaultActions; collectRolesToRemove still only walks levelActions, leaving defaultActions-granted roles unrevoked on level-down. |
| src/modules/actions/grantRole.js | Grant role handler with replace-mode loop and per-removal canManageRole guard. The final member.roles.add still executes without a rate-limit check (flagged in a prior review thread). |
| src/api/routes/members.js | XP adjustment endpoint now attempts to compute oldLevel via newXp - amount, but this gives a wrong result when GREATEST(0,...) clamps XP — causing enforceRoleLevelDown to fire spuriously. rows[0].level is the correct pre-update source. Also uses an unnecessary dynamic import. |
| src/utils/templateEngine.js | Stateless template renderer and context builder. Two hardcoded 'en-US' locale strings in formatNumber and joinDate violate the project's no-hardcoded-locale rule. |
| src/modules/reputation.js | Cleaned up nicely — ~90 lines of inline side-effect logic removed, replaced with an executeLevelUpPipeline call gated on xpCfg.enabled. XP/level thresholds now read from config.xp. |
| src/modules/xpDefaults.js | Single source of truth for XP config defaults; clean and straightforward. |
| src/commands/rank.js | Switched to XP_DEFAULTS/config.xp for thresholds. Does NOT gate on xp.enabled, preserving backward compatibility for existing guilds. |
| src/api/routes/community.js | Migrated from getRepConfig/REPUTATION_DEFAULTS to getXpConfig; max-level nextLevelXp now correctly returns 0 instead of repeating the last threshold. |
| web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx | New Level-Up Actions settings card added. XP threshold input now sorts values before saving. Badge list key updated to use stable composite key. |
| web/src/types/config.ts | New XpConfig, XpLevelAction, and XpLevelActionEntry types added cleanly; BotConfig and ConfigSection union updated accordingly. |
Sequence Diagram
sequenceDiagram
participant M as Message Event
participant R as reputation.js
participant P as levelUpActions.js
participant RL as roleUtils.js
participant GR as grantRole.js
participant D as Discord API
M->>R: handleXpGain(message)
R->>R: computeLevel(newXp, thresholds)
R->>R: UPDATE reputation SET level
R->>P: executeLevelUpPipeline({member, guild, previousLevel, newLevel, xp, config})
P->>P: resolveActions(previousLevel, newLevel, config)
P->>RL: checkRoleRateLimit(guildId, userId)
RL-->>P: rateLimitOk (quota=2)
loop For each level action
P->>P: buildTemplateContext(level)
P->>P: roleChangesRemaining--
P->>GR: handleGrantRole(action, context)
GR->>RL: canManageRole(guild, roleId)
alt stackRoles == false
GR->>RL: checkRoleRateLimit / recordRoleChange
GR->>D: member.roles.remove(oldRoleId)
end
GR->>D: member.roles.add(roleId)
GR->>RL: recordRoleChange
end
note over M,D: XP API Reduction path
participant API as POST /xp
API->>API: UPDATE xp = GREATEST(0, xp+amount) RETURNING xp level
API->>API: oldLevel = rows[0].level (pre-update stored level)
API->>API: newLevel = computeLevel(newXp, thresholds)
alt newLevel < oldLevel and removeOnLevelDown
API->>RL: enforceRoleLevelDown(member, newLevel, xpConfig)
RL->>RL: collectRolesToRemove (levelActions only)
RL->>D: member.roles.remove(roleIds[])
end
Comments Outside Diff (1)
-
src/api/routes/members.js, line 977-978 (link)oldXp = newXp - amountis wrong when XP is clamped byGREATEST(0, ...)The SQL clause
SET xp = GREATEST(0, reputation.xp + $3)clamps the result to zero. When a large negativeamountwould push XP below zero,newXpis 0 — butnewXp - amountgives back the unclamped value, not the real pre-update XP.Concrete example: user has 50 XP (level 0), admin removes 200 (
amount = -200):newXp = GREATEST(0, 50 − 200) = 0oldXp = 0 − (−200) = 200← wrong; real old XP was 50oldLevel = computeLevel(200, [100, 300, ...]) = 2← wrong; user was level 0
newLevel (0) < oldLevel (2)evaluates totrue, soenforceRoleLevelDownfires and strips roles the user never earned.rows[0].levelfrom theRETURNINGclause is already the pre-update stored level (onlyxpis in theSETclause). Use it directly:
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/api/routes/members.js
Line: 977-978
Comment:
**`oldXp = newXp - amount` is wrong when XP is clamped by `GREATEST(0, ...)`**
The SQL clause `SET xp = GREATEST(0, reputation.xp + $3)` clamps the result to zero. When a large negative `amount` would push XP below zero, `newXp` is 0 — but `newXp - amount` gives back the unclamped value, not the real pre-update XP.
Concrete example: user has 50 XP (level 0), admin removes 200 (`amount = -200`):
- `newXp = GREATEST(0, 50 − 200) = 0`
- `oldXp = 0 − (−200) = 200` ← wrong; real old XP was 50
- `oldLevel = computeLevel(200, [100, 300, ...]) = 2` ← wrong; user was level 0
`newLevel (0) < oldLevel (2)` evaluates to `true`, so `enforceRoleLevelDown` fires and strips roles the user never earned.
`rows[0].level` from the `RETURNING` clause is already the pre-update stored level (only `xp` is in the `SET` clause). Use it directly:
```suggestion
oldLevel = rows[0].level;
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/utils/templateEngine.js
Line: 53
Comment:
**Hardcoded `'en-US'` locale in `formatNumber` and `joinDate`**
Two places in this file hardcode `'en-US'`, violating the project's no-hardcoded-locale rule — use `undefined` to inherit the runtime locale:
- Line 53 (`formatNumber`):
```suggestion
return Number(n).toLocaleString(undefined);
```
- Line 154–158 (`joinDate`):
```
joinDate: member.joinedAt
? member.joinedAt.toLocaleDateString(undefined, {
month: 'short',
day: 'numeric',
year: 'numeric',
})
: '',
```
**Rule Used:** Do not hardcode locale strings like 'en-US' in Int... ([source](https://app.greptile.com/review/custom-context?memory=no-hardcoded-locale))
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/api/routes/members.js
Line: 1004
Comment:
**Unnecessary dynamic import — prefer static import**
`enforceRoleLevelDown` is imported dynamically here, but there is no circular dependency that would require it. Dynamic `import()` defers module loading to runtime and adds async overhead on every XP reduction that triggers level-down enforcement. A static import at the top of the file would be cleaner and consistent with how the rest of this file's dependencies are handled.
```
// At the top of members.js with the other imports:
import { enforceRoleLevelDown } from '../../modules/actions/roleUtils.js';
```
Then the call site becomes:
```suggestion
await enforceRoleLevelDown(member, newLevel, xpConfig);
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (13): Last reviewed commit: "test: add enforceRoleLevelDown coverage ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Introduces a configurable XP level-up “action pipeline” under config.xp, replacing the prior inline role/announcement side effects and wiring the new XP config into bot commands and API routes.
Changes:
- Adds XP defaults + a level-up pipeline executor with an action registry and level-skip handling.
- Implements Phase 1 role actions (
grantRole,removeRole) plus shared permission/rate-limit utilities and level-down enforcement. - Adds a template interpolation/context engine and updates config + call sites to read
levelThresholdsfromconfig.xp(with accompanying tests).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
config.json |
Moves XP thresholds/settings into new xp section. |
src/modules/xpDefaults.js |
Defines default config.xp values. |
src/modules/reputationDefaults.js |
Removes XP-specific fields moved to xpDefaults. |
src/modules/reputation.js |
Switches level-up side effects to executeLevelUpPipeline() (fire-and-forget). |
src/modules/levelUpActions.js |
Adds pipeline executor + action registry + action resolution across level skips. |
src/modules/actions/roleUtils.js |
Adds role permission checks, in-memory rate limiter, and level-down enforcement. |
src/modules/actions/grantRole.js |
Adds grantRole handler with stack/replace behavior and template-context role info. |
src/modules/actions/removeRole.js |
Adds removeRole handler. |
src/utils/templateEngine.js |
Adds template renderer, context builder, and length validator. |
src/commands/rank.js |
Reads thresholds from config.xp defaults instead of reputation defaults. |
src/api/utils/configAllowlist.js |
Allows xp to be written via API/dashboard. |
src/api/routes/community.js |
Reads thresholds from config.xp. |
src/api/routes/members.js |
Reads thresholds from config.xp; enforces role level-down on XP reduction. |
tests/utils/templateEngine.test.js |
New tests for template rendering/context/length validation. |
tests/modules/levelUpActions.test.js |
New tests for action resolution + sequential execution + error isolation. |
tests/modules/actions/roleUtils.test.js |
New tests for permission checks, rate limiting, role collection, and level-down enforcement. |
tests/modules/actions/grantRole.test.js |
New tests for grantRole behavior including stack/replace. |
tests/modules/actions/removeRole.test.js |
New tests for removeRole behavior. |
tests/modules/reputationDefaults.test.js |
Updates expectations for moved XP fields. |
tests/modules/reputation.test.js |
Updates to assert pipeline invocation and new config shape. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/routes/members.js (1)
994-1037:⚠️ Potential issue | 🟠 MajorSuccessful XP adjustments leave cached reads stale.
This route updates
reputation, but it never evicts the per-user enrichment cache populated earlier in this file, andsrc/api/routes/community.jsalso caches XP-based reads. After an admin adjustment,/membersand community endpoints can keep serving the old XP/level until TTL expiry. Please invalidate the affected member and community cache keys before returning success.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/routes/members.js` around lines 994 - 1037, This route updates a member's XP/level but doesn't evict cached reads—before sending the success response (before the res.json call), invalidate the per-user enrichment cache key populated earlier in this file and the XP/community cache key(s) used by src/api/routes/community.js for the same guildId/userId so subsequent reads return the updated newXp/newLevel; use the same cache helper/functions those modules use (the same cache client or cache.del/cache.invalidate functions) and reference the current guildId and userId when building the cache keys to remove the entries prior to returning from this handler.
🤖 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 286-295: Existing guild configs using legacy keys
reputation.levelThresholds and reputation.roleRewards are being ignored because
getXpConfig() only reads cfg.xp and falls back to XP_DEFAULTS; implement a fix
that either migrates stored configs into the new xp shape on load/save or adds a
compatibility branch inside getXpConfig() in src/modules/reputation.js: when
cfg.xp is missing/empty, detect and map legacy keys reputation.levelThresholds
-> xp.levelThresholds and reputation.roleRewards -> xp.roleRewards (preserving
stackRoles/removeOnLevelDown semantics) before returning (or persist the
migrated xp back to the DB), ensuring getXpConfig() returns the
migrated/compatible object instead of defaulting to XP_DEFAULTS.
In `@src/api/routes/community.js`:
- Around line 203-206: The code sets nextLevelXp to the last threshold for
max-level users, making currentLevelXp === nextLevelXp; change the logic in the
block that computes currentLevelXp and nextLevelXp (referencing
xpConfig.levelThresholds and level) so that when level is at or beyond the
highest threshold you assign nextLevelXp = null (matching members.js's
nextThreshold sentinel) instead of mirroring the last threshold; also ensure any
response schema or docs that describe nextLevelXp are updated to allow null for
max-level users.
In `@src/api/routes/members.js`:
- Around line 663-666: The current code trusts rep?.level which may be stale;
change the member-detail logic to always recompute level from XP by calling
computeLevel(xp, xpConfig.levelThresholds) (use getXpConfig and xp from rep?.xp
?? 0 as before) and then derive nextThreshold from xpConfig.levelThresholds
using that computed level; replace any use of rep?.level with the computed value
so nextThreshold = xpConfig.levelThresholds[computedLevel] ?? null.
In `@src/modules/actions/grantRole.js`:
- Around line 22-35: The per-call rate limiter in grantRole.js (calls to
checkRoleRateLimit and recordRoleChange around member.roles.add/remove) causes
skipped-level rewards when executeLevelUpPipeline invokes the handler multiple
times; change the flow so rate limiting is applied once per pipeline run rather
than per crossed level: add a boolean parameter (e.g., bypassRateLimit or
pipelineRun) to the grantRole handler or to checkRoleRateLimit so
executeLevelUpPipeline can call it once before iterating levels, and ensure
recordRoleChange is invoked only once after the final role mutation; update
canManageRole, checkRoleRateLimit, and the call sites in executeLevelUpPipeline
to accept/propagate the new flag and move/remove per-call
checkRoleRateLimit/recordRoleChange invocations so role removals/additions
(member.roles.remove/add) are batched for the pipeline run.
In `@src/modules/actions/removeRole.js`:
- Around line 21-25: This file currently consumes a rate-limit slot and records
a role change on every individual removal (calls to checkRoleRateLimit and
recordRoleChange) which prevents multi-role removals from completing; remove the
per-removal rate-limit and recording from the remove path by deleting the
checkRoleRateLimit(guild.id, member.user?.id) guard and the
recordRoleChange(guild.id, member.user?.id) call surrounding
member.roles.remove(roleId), and instead have the batched caller perform one
checkRoleRateLimit and one recordRoleChange for the entire set of removals so
the batch is treated as a single logical operation.
In `@src/modules/actions/roleUtils.js`:
- Around line 129-145: The enforceRoleLevelDown function has high cognitive
complexity due to nested loops and multiple continue guards; extract the inner
action-processing loop into a new helper (e.g., processLevelEntryActions or
handleEntryActions) that accepts (guild, member, entry, newLevel) and contains
the per-action checks (action.type==='grantRole', action.roleId,
member.roles.cache.has, canManageRole, checkRoleRateLimit) and the await
member.roles.remove + recordRoleChange call; then simplify enforceRoleLevelDown
to iterate xpConfig.levelActions, skip entries where entry.level <= newLevel,
and call the helper for each remaining entry, preserving current logic and async
behavior.
- Around line 139-142: In enforceRoleLevelDown, avoid calling
member.roles.remove(action.roleId) repeatedly; instead gather all action.roleId
values that should be removed for a given member into a single array, call
checkRoleRateLimit(guild.id, member.user.id) once before the batch, then call
member.roles.remove([...roleIds]) once and invoke recordRoleChange(guild.id,
member.user.id) a single time after the batch removal so the operation counts as
one rate-limit change rather than N individual removals.
In `@src/utils/templateEngine.js`:
- Around line 82-84: The template token `nextLevel` is currently set to the XP
threshold (via `nextThreshold`) instead of the numeric next level; update the
token mapping so `nextLevel` returns the next level number (e.g., `level + 1` or
null if no next level) and keep `xpToNext` as the XP delta; if the XP threshold
itself is required, expose it under a new token name (e.g., `nextThreshold`)
rather than reusing `nextLevel`, and ensure any code that constructs tokens uses
these corrected identifiers (`nextLevel`, `nextThreshold`, `xpToNext`).
- Line 138: The buildTemplateContext function currently dereferences
message.channel unguarded (see the line setting channel: message.channel?.name ?
`#${message.channel.name}` : ''), which throws when buildTemplateContext is
called without a message; fix it by guarding access to message first (e.g., use
message?.channel?.name or check message exists before reading channel) so
channel is computed only when message is present and otherwise set to ''. Ensure
the change only affects the channel expression in buildTemplateContext so other
callers remain unaffected.
In `@tests/modules/actions/grantRole.test.js`:
- Around line 57-121: Add a regression test that simulates three sequential
grants for the same user to exercise the 2/60s rate limiter and ensure the
highest reward still lands: using makeContext(), call handleGrantRole() three
times in order with relevant roleIds (e.g., lower, mid, highest), mock or let
checkRoleRateLimit behave normally so the limiter can block the later calls, and
then assert that the final member state contains the highest role
(ctx._mocks.rolesAdd called with highest roleId) and recordRoleChange was
invoked for the user; reference handleGrantRole, makeContext,
checkRoleRateLimit, ctx._mocks.rolesAdd and recordRoleChange to locate code to
test.
In `@tests/modules/actions/roleUtils.test.js`:
- Around line 68-97: Tests share in-memory rate limit state stored in the
roleLimits Map which can leak between tests; add a reset step for isolation by
either exporting a new clearRoleLimits() helper from the module that empties the
roleLimits Map, or call an existing reset function, and invoke that helper in
the test file's beforeEach (or afterEach) to clear roleLimits prior to each
test; reference the roleLimits Map and the checkRoleRateLimit and
recordRoleChange functions so reviewers can locate where to add the
clearRoleLimits() implementation and where to call it in the test setup.
- Around line 38-66: Add a beforeEach that calls vi.clearAllMocks() to ensure
warn mock assertions in canManageRole tests are isolated; update the test suite
for describe('canManageRole') so beforeEach invokes vi.clearAllMocks(), leaving
the existing test cases and references to canManageRole and warn unchanged so
each it-block verifies warn calls reliably.
In `@tests/utils/templateEngine.test.js`:
- Around line 99-204: Add two tests to tests/utils/templateEngine.test.js for
buildTemplateContext: (1) mock the DB query function that buildTemplateContext
uses (e.g., db.query or the module's query helper) to return populated rows for
rank, messages, voiceHours, and daysActive and assert that ctx.rank,
ctx.messages, ctx.voiceHours, and ctx.daysActive match the expected formatted
values; (2) add a test where that same query mock rejects (throws) to exercise
the rejected-query fallback and assert the fallback values (e.g., '0' or empty
string) and that no exception bubbles up. Ensure you restore/reset the mock
between tests.
---
Outside diff comments:
In `@src/api/routes/members.js`:
- Around line 994-1037: This route updates a member's XP/level but doesn't evict
cached reads—before sending the success response (before the res.json call),
invalidate the per-user enrichment cache key populated earlier in this file and
the XP/community cache key(s) used by src/api/routes/community.js for the same
guildId/userId so subsequent reads return the updated newXp/newLevel; use the
same cache helper/functions those modules use (the same cache client or
cache.del/cache.invalidate functions) and reference the current guildId and
userId when building the cache keys to remove the entries prior to returning
from this handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 09b64035-4730-4b17-812a-4437d10e9503
📒 Files selected for processing (20)
config.jsonsrc/api/routes/community.jssrc/api/routes/members.jssrc/api/utils/configAllowlist.jssrc/commands/rank.jssrc/modules/actions/grantRole.jssrc/modules/actions/removeRole.jssrc/modules/actions/roleUtils.jssrc/modules/levelUpActions.jssrc/modules/reputation.jssrc/modules/reputationDefaults.jssrc/modules/xpDefaults.jssrc/utils/templateEngine.jstests/modules/actions/grantRole.test.jstests/modules/actions/removeRole.test.jstests/modules/actions/roleUtils.test.jstests/modules/levelUpActions.test.jstests/modules/reputation.test.jstests/modules/reputationDefaults.test.jstests/utils/templateEngine.test.js
💤 Files with no reviewable changes (1)
- src/modules/reputationDefaults.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). (5)
- GitHub Check: Cursor Bugbot
- GitHub Check: Agent
- GitHub Check: Greptile Review
- GitHub Check: Test
- GitHub Check: Docker Build Validation
🧰 Additional context used
📓 Path-based instructions (13)
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
src/commands/rank.jssrc/api/utils/configAllowlist.jssrc/modules/xpDefaults.jstests/modules/reputationDefaults.test.jssrc/api/routes/community.jstests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jssrc/modules/reputation.jssrc/modules/actions/removeRole.jstests/modules/levelUpActions.test.jstests/utils/templateEngine.test.jssrc/modules/actions/grantRole.jstests/modules/reputation.test.jstests/modules/actions/roleUtils.test.jssrc/utils/templateEngine.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.jssrc/api/routes/members.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
src/commands/rank.jssrc/api/utils/configAllowlist.jssrc/modules/xpDefaults.jstests/modules/reputationDefaults.test.jssrc/api/routes/community.jstests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jssrc/modules/reputation.jssrc/modules/actions/removeRole.jstests/modules/levelUpActions.test.jstests/utils/templateEngine.test.jssrc/modules/actions/grantRole.jstests/modules/reputation.test.jstests/modules/actions/roleUtils.test.jssrc/utils/templateEngine.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.jssrc/api/routes/members.js
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Never useconsole.*methods; use the Winston logger instead viaimport logger from '../logger.js'(adjust path as needed), then calllogger.info(),logger.warn(),logger.error(), orlogger.debug()
Always usesafeReply(),safeSend(), orsafeEditReply()instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Files:
src/commands/rank.jssrc/api/utils/configAllowlist.jssrc/modules/xpDefaults.jssrc/api/routes/community.jssrc/modules/reputation.jssrc/modules/actions/removeRole.jssrc/modules/actions/grantRole.jssrc/utils/templateEngine.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.jssrc/api/routes/members.js
src/commands/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create slash command definitions in
src/commands/, exporting a slash command builder and anexecutefunction
Files:
src/commands/rank.js
**/*.{js,mjs,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only
Files:
src/commands/rank.jssrc/api/utils/configAllowlist.jssrc/modules/xpDefaults.jstests/modules/reputationDefaults.test.jssrc/api/routes/community.jstests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jssrc/modules/reputation.jssrc/modules/actions/removeRole.jstests/modules/levelUpActions.test.jstests/utils/templateEngine.test.jssrc/modules/actions/grantRole.jstests/modules/reputation.test.jstests/modules/actions/roleUtils.test.jssrc/utils/templateEngine.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.jssrc/api/routes/members.js
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts}: Use src/logger.js; do not use console.*
Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Use parameterized SQL only
Community features should be gated behind config..enabled. Moderation commands are the exception
Files:
src/commands/rank.jssrc/api/utils/configAllowlist.jssrc/modules/xpDefaults.jstests/modules/reputationDefaults.test.jssrc/api/routes/community.jstests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jssrc/modules/reputation.jssrc/modules/actions/removeRole.jstests/modules/levelUpActions.test.jstests/utils/templateEngine.test.jssrc/modules/actions/grantRole.jstests/modules/reputation.test.jstests/modules/actions/roleUtils.test.jssrc/utils/templateEngine.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.jssrc/api/routes/members.js
src/api/utils/configAllowlist.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When adding a new config section, add it to
SAFE_CONFIG_KEYSto enable saves via the API/dashboard, and toREADABLE_CONFIG_KEYSfor API read access
Files:
src/api/utils/configAllowlist.js
{config.json,**/configAllowlist.{js,ts}}
📄 CodeRabbit inference engine (AGENTS.md)
Config-backed features must be added to config.json and src/api/utils/configAllowlist.js. If a key is missing from SAFE_CONFIG_KEYS, the dashboard cannot save it
Files:
src/api/utils/configAllowlist.jsconfig.json
src/modules/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create feature modules in
src/modules/and add corresponding config sections toconfig.json
Files:
src/modules/xpDefaults.jssrc/modules/reputation.jssrc/modules/actions/removeRole.jssrc/modules/actions/grantRole.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.js
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/modules/reputationDefaults.test.jstests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jstests/modules/levelUpActions.test.jstests/utils/templateEngine.test.jstests/modules/reputation.test.jstests/modules/actions/roleUtils.test.js
**/*.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use double quotes in JSON files (standard JSON format); this exception applies only to JSON files, not JavaScript/TypeScript
Files:
config.json
config.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All community features in
config.jsonshould be gated behind aconfig.<feature>.enabledflag; moderation commands are always available regardless of config settings
Files:
config.json
src/api/routes/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create API route handlers in
src/api/routes/and mount them insrc/api/server.js; add auth middleware if the endpoint requires authentication
Files:
src/api/routes/community.jssrc/api/routes/members.js
🧠 Learnings (11)
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/utils/cache.js : Use `src/utils/cache.js` for generic caching with Redis primary + in-memory fallback; use `src/utils/discordCache.js` for Discord API response caching (channels, roles, members); use `src/utils/reputationCache.js` for leaderboard and user reputation data; all caches auto-invalidate when config changes
Applied to files:
src/commands/rank.jssrc/api/routes/community.jssrc/modules/reputation.jssrc/api/routes/members.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/api/utils/configAllowlist.js : When adding a new config section, add it to `SAFE_CONFIG_KEYS` to enable saves via the API/dashboard, and to `READABLE_CONFIG_KEYS` for API read access
Applied to files:
src/api/utils/configAllowlist.js
📚 Learning: 2026-03-22T01:55:35.229Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-22T01:55:35.229Z
Learning: Applies to {config.json,**/configAllowlist.{js,ts}} : Config-backed features must be added to config.json and src/api/utils/configAllowlist.js. If a key is missing from SAFE_CONFIG_KEYS, the dashboard cannot save it
Applied to files:
src/api/utils/configAllowlist.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to config.json : All community features in `config.json` should be gated behind a `config.<feature>.enabled` flag; moderation commands are always available regardless of config settings
Applied to files:
src/api/utils/configAllowlist.js
📚 Learning: 2026-03-22T01:55:35.229Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-22T01:55:35.229Z
Learning: Applies to **/*.{js,ts} : Community features should be gated behind config.<feature>.enabled. Moderation commands are the exception
Applied to files:
src/api/utils/configAllowlist.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Write web dashboard tests using Vitest 4 with the `jsdom` environment and React Testing Library, matching the `web/src/` structure
Applied to files:
tests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jstests/modules/levelUpActions.test.jstests/utils/templateEngine.test.jstests/modules/actions/roleUtils.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Write bot tests using Vitest 4 with the `node` environment, matching the `src/` structure in the `tests/` directory
Applied to files:
tests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jstests/modules/levelUpActions.test.jstests/utils/templateEngine.test.jstests/modules/actions/roleUtils.test.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
tests/modules/actions/grantRole.test.jssrc/modules/actions/removeRole.jssrc/modules/actions/grantRole.jstests/modules/actions/roleUtils.test.jssrc/modules/actions/roleUtils.js
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Applied to files:
src/modules/actions/removeRole.jssrc/api/routes/members.js
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Applied to files:
src/modules/actions/removeRole.jssrc/api/routes/members.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/reactionrole.js : Enforce invoker role hierarchy check in /reactionrole add command to prevent non-owner users from configuring roles at or above their highest role
Applied to files:
src/modules/actions/removeRole.jssrc/modules/actions/grantRole.jstests/modules/actions/roleUtils.test.jssrc/modules/actions/roleUtils.js
🪛 GitHub Check: SonarCloud Code Analysis
tests/utils/templateEngine.test.js
[warning] 9-9: 'vitest' imported multiple times.
[warning] 1-1: 'vitest' imported multiple times.
src/utils/templateEngine.js
[warning] 134-134: Unexpected negated condition.
[warning] 134-134: Remove this "!==" check; it will always be true. Did you mean to use "!="?
[warning] 83-83: Remove this "!==" check; it will always be true. Did you mean to use "!="?
[warning] 26-26: Prefer String#replaceAll() over String#replace().
[warning] 83-83: Unexpected negated condition.
src/modules/actions/roleUtils.js
[failure] 129-129: Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.
🔇 Additional comments (11)
src/api/utils/configAllowlist.js (1)
34-37: Good allowlist update for the newxpsection.
READABLE_CONFIG_KEYSinherits fromSAFE_CONFIG_KEYS, so addingxphere keeps the dashboard/API read-write path aligned with the new top-level config. Based on learnings, "When adding a new config section, add it toSAFE_CONFIG_KEYSto enable saves via the API/dashboard, and toREADABLE_CONFIG_KEYSfor API read access".src/commands/rank.js (1)
13-13: Nice switch to the shared XP config.Using
XP_DEFAULTS+cfg.xpkeeps/rankaligned with the new top-level XP settings.Also applies to: 48-49
src/modules/levelUpActions.js (3)
1-34: LGTM! Well-structured action registry pattern.The registry pattern with
registerActionprovides clean extensibility for Phase 2 additions. The built-in handler registration is appropriately placed at module load time.
44-64: LGTM! Correct level-skip handling.The
resolveActionsfunction properly handles level skips by iterating frompreviousLevel + 1throughnewLevel, collecting actions for each crossed threshold. The fallback todefaultActionswhen no level-specific actions exist matches the PR requirements.
79-149: LGTM! Robust pipeline execution with proper error isolation.The implementation correctly:
- Returns early when no actions need execution
- Builds shared context once for efficiency
- Uses per-action try/catch to ensure failures don't abort the pipeline
- Logs structured warnings for unknown action types and handler errors
The optional chaining on
member.user?.idprovides good defensive coding against edge cases.tests/modules/actions/roleUtils.test.js (2)
116-141: LGTM! Comprehensive coverage ofcollectXpManagedRoles.Tests correctly verify that only
grantRoleandremoveRoleaction types contribute role IDs, and other action types likesendDmandaddReactionare ignored.
143-208: LGTM! Good coverage ofenforceRoleLevelDownedge cases.Tests cover the key scenarios: removing roles above new level, keeping roles at/below new level, and skipping roles the member doesn't have. The mock setup correctly simulates Discord.js member/guild structures.
src/modules/actions/roleUtils.js (4)
1-60: LGTM! Solid permission and hierarchy checks.The
canManageRolefunction correctly implements Discord's permission model:
- Verifies bot has
ManageRolespermission- Ensures role exists in cache
- Enforces role hierarchy (role must be strictly below bot's highest)
Warning logs provide good observability for debugging permission issues.
70-103: LGTM! Clean separation of check vs. record for rate limiting.The two-step pattern (
checkRoleRateLimitthenrecordRoleChange) allows callers to verify before attempting changes, and only record on success. The sliding window implementation correctly filters expired timestamps.
147-148: LGTM! Good use of.unref()for the sweep interval.Using
.unref()ensures the timer doesn't prevent Node.js from exiting when the process should terminate. This matches the pattern used elsewhere (reputation.js cooldowns per the comment).
157-167: LGTM! Correctly extracts XP-managed role IDs.The function properly iterates through
levelActionsand collects role IDs from bothgrantRoleandremoveRoleaction types, which is needed for the stack/replace logic in grant handlers.
| const templateContext = await buildTemplateContext({ | ||
| member, | ||
| message, | ||
| guild, | ||
| level, | ||
| previousLevel, |
There was a problem hiding this comment.
previousLevel passed into buildTemplateContext() is always the original pre-jump level. During level skips (e.g. 4→12), intermediate levels (5, 6, …) will render {{previousLevel}} incorrectly. Consider passing level - 1 (or otherwise tracking the per-step previous level) when building the context for each intermediate level.
| const templateContext = await buildTemplateContext({ | |
| member, | |
| message, | |
| guild, | |
| level, | |
| previousLevel, | |
| // For templates, previousLevel should reflect the level immediately before this step. | |
| const stepPreviousLevel = level > 0 ? level - 1 : 0; | |
| const templateContext = await buildTemplateContext({ | |
| member, | |
| message, | |
| guild, | |
| level, | |
| previousLevel: stepPreviousLevel, |
| await member.roles.remove(action.roleId); | ||
| recordRoleChange(guild.id, member.user.id); |
There was a problem hiding this comment.
enforceRoleLevelDown() awaits member.roles.remove() without handling failures. A single Discord API error (missing perms, unknown role, transient failure) will throw and abort the loop, potentially leaving other above-level roles intact. Wrap each removal in a try/catch (log + continue) so level-down enforcement is best-effort like the rest of the pipeline.
| await member.roles.remove(action.roleId); | |
| recordRoleChange(guild.id, member.user.id); | |
| try { | |
| await member.roles.remove(action.roleId); | |
| recordRoleChange(guild.id, member.user.id); | |
| } catch (error) { | |
| warn('Failed to remove XP level-down role', { | |
| guildId: guild.id, | |
| userId: member.user.id, | |
| roleId: action.roleId, | |
| error, | |
| }); | |
| } |
| } | ||
|
|
||
| return { | ||
| username: member.user.displayName ?? '', |
There was a problem hiding this comment.
username is derived from member.user.displayName, which won’t reflect a guild nickname. Since this context builder receives a GuildMember, using member.displayName (with a fallback to the user name) will better match how the rest of the codebase surfaces display names and what users expect in server-scoped templates.
| username: member.user.displayName ?? '', | |
| username: member.displayName ?? member.user.username ?? '', |
✅ Actions performedReview triggered.
|
| newXp = rows[0].xp; | ||
| // Calculate oldLevel from pre-update XP (newXp - amount) | ||
| const oldXp = newXp - amount; | ||
| oldLevel = computeLevel(oldXp, xpConfig.levelThresholds); | ||
|
|
There was a problem hiding this comment.
oldXp is inferred as newXp - amount, but the upsert floors XP at 0 via GREATEST(0, ...). When the floor is hit (e.g., existing XP=10, amount=-20 → newXp=0), newXp - amount produces 20, which is not the pre-update XP. That makes oldLevel incorrect and can trigger incorrect level-down enforcement logic. To compute oldLevel reliably, fetch the current XP/level inside the same transaction (e.g., SELECT xp, level ... FOR UPDATE) before applying the adjustment, then compute oldLevel from that value.
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (6)
src/modules/actions/removeRole.js (1)
24-25:⚠️ Potential issue | 🟠 MajorDon't record limiter usage inside each
removeRoleaction.The pipeline can hit this handler several times in one skipped-level run.
Recording here turns one logical batch into N limiter entries, which defeats the
upstream "check once per pipeline" design. Record the successful batch once in
the caller instead.♻️ Minimal fix
await member.roles.remove(roleId); - recordRoleChange(guild.id, member.user?.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/actions/removeRole.js` around lines 24 - 25, The removeRole action currently calls recordRoleChange(guild.id, member.user?.id) after awaiting member.roles.remove(roleId), which records limiter usage per individual removal; remove that call from this handler (i.e., delete the recordRoleChange invocation in the removeRole flow) so the handler only performs the role removal and lets the caller/batch processor record a single limiter entry for the whole pipeline run.tests/modules/actions/removeRole.test.js (1)
9-13:⚠️ Potential issue | 🟡 MinorThese tests don't actually cover pipeline rate limiting.
handleRemoveRole()never callscheckRoleRateLimit(), so the mock at
Lines 9-13 is dead. The case at Lines 50-58 duplicates the earlier
canManageRole === falsepath, and the "pipeline" case at Lines 60-73 just
rechecks the happy path without invoking the pipeline. Move rate-limit coverage
tolevelUpActions.test.jsand trim the duplicate assertions here.Also applies to: 50-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/modules/actions/removeRole.test.js` around lines 9 - 13, The test file mocks checkRoleRateLimit but handleRemoveRole never calls it, so remove the dead mock of checkRoleRateLimit from tests/modules/actions/removeRole.test.js (keep canManageRole and recordRoleChange as needed), delete the duplicated canManageRole===false case and the duplicated "pipeline" happy-path assertions, and instead add a dedicated rate-limit test in levelUpActions.test.js that exercises the codepath which calls checkRoleRateLimit (invoke the pipeline-related function used in level-up flow and assert rate-limit behavior), ensuring you reference handleRemoveRole for trimming and checkRoleRateLimit for where to add coverage.src/modules/actions/grantRole.js (1)
25-49:⚠️ Potential issue | 🟠 MajorReplace mode still mutates roles one by one.
Despite the comment at Line 23, Lines 31-34 still call
checkRoleRateLimit()andrecordRoleChange()for every old XP role. On
skipped levels, cleanup can stop after the second mutation, and because the add
happens afterward, a laterroles.add()failure can leave the member with no XP
reward. Compute the replace-mode diff and apply/record it as one logical
operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/actions/grantRole.js` around lines 25 - 49, The replace-mode flow mutates roles one-by-one (loop using member.roles.remove, calling checkRoleRateLimit and recordRoleChange per removed role) which can leave users in an inconsistent state if the later member.roles.add fails; instead compute the set of roles to remove (using xpManagedRoles and roleId), batch the intended changes, perform the removes and the add as a single logical operation (e.g., collect removal IDs, ensure rate limit once via checkRoleRateLimit(guild.id, member.user?.id) before applying, then call member.roles.remove with the full array and member.roles.add(roleId) and only then call recordRoleChange once), and update logging/permission checks (canManageRole) to run during diff computation rather than per removal so the operation is atomic from the recordRoleChange perspective.web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx (1)
528-550:⚠️ Potential issue | 🟠 MajorKeep raw threshold text in local state.
Because this field renders from
levelThresholds.join(', '), theonChange
handler reparses and re-sorts every keystroke. Partial edits get rewritten
mid-entry, and thenums.length > 0guard still drops the clear/all-invalid
case instead of persisting it. Parse on blur/save, not while the user is typing.src/modules/actions/roleUtils.js (1)
152-179:⚠️ Potential issue | 🟠 MajorCount a level-down batch once in the limiter.
After the single check at Line 193, both the batch path and the fallback path
still record one change per removed role. A large level drop can burn the entire
2/60s budget in one logical operation, which defeats the batching goal for
level-down cleanup. Record the whole removal batch once instead.Also applies to: 217-235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/actions/roleUtils.js` around lines 152 - 179, remove the per-role counting from inside removeSingleRole by deleting the recordRoleChange(guild.id, member.user.id) call and have removeSingleRole only attempt removal and return success/failure; instead, in the higher-level level-down batch handlers (the batch path and the fallback path referenced around the other block at lines 217-235), call recordRoleChange once per logical level-down operation (once per member/guild batch) after all role removals complete so the limiter is incremented a single time for the whole batch rather than once per removed role.src/modules/reputation.js (1)
164-180:⚠️ Potential issue | 🟠 MajorAdd announcement action handler or document required configuration.
The level-up pipeline now delegates all effects to registered action handlers, but only
grantRoleandremoveRoleare implemented. Without an announcement handler, level-ups produce silent success—users receive no feedback on their progression.Guilds need either:
- A built-in announcement action handler (e.g.,
src/modules/actions/announce.js) registered and wired into the pipeline, or- Clear documentation requiring admins to configure
levelActionswith custom handlers to restore visible feedback.Without this, deployments will have broken user experience.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/reputation.js` around lines 164 - 180, The level-up pipeline (executeLevelUpPipeline) currently invokes action handlers but only has grantRole/removeRole implemented, so no announcement is made; add a built-in announcement action handler (e.g., create src/modules/actions/announce.js exporting the handler and register it where other actions are collected/registered) and ensure executeLevelUpPipeline looks up and invokes the "announce" action when levelActions include an announcement, or alternatively update docs to require guilds include a custom "announce" handler in their levelActions configuration; reference executeLevelUpPipeline, grantRole, removeRole, and levelActions when wiring/ documenting the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/routes/members.js`:
- Around line 1002-1005: Replace the dynamic import with a static import: add a
top-of-file import for enforceRoleLevelDown from roleUtils.js in members.js,
then remove the await import('../../modules/actions/roleUtils.js') line and call
enforceRoleLevelDown(member, newLevel, xpConfig) directly where it’s currently
used to avoid per-call latency.
In `@src/modules/levelUpActions.js`:
- Around line 79-192: The executeLevelUpPipeline function is too complex;
extract the template-context resolution and per-action dispatch into helpers:
create a resolveTemplateContext(level) helper that uses templateContextCache,
calls buildTemplateContext with ({member,message,guild,level,previousLevel:
level-1,xp,levelThresholds:config.levelThresholds,roleName:null,roleId:null}),
catches errors, logs via warn and returns the cached or empty context; and
create a dispatchLevelAction(action, pipelineContext, roleChangesRemainingRef)
helper that contains rate-limit checks for grantRole/removeRole, retrieves the
handler via actionRegistry.get(action.type), logs unknown types, decrements
roleChangesRemainingRef when consuming quota, calls
handler(action,pipelineContext) inside try/catch and logs failures; replace the
in-loop templateContext construction and action handling with calls to these two
helpers, passing necessary values
(member,message,guild,config,xp,templateContextCache,roleChangesRemaining) so
cognitive complexity is reduced.
In `@src/modules/reputation.js`:
- Around line 48-55: getXpConfig currently only reads cfg.xp and will drop
legacy reputation.* keys; add a rollout fallback that reads
cfg.reputation.levelThresholds and cfg.reputation.roleRewards when cfg.xp.* are
missing and merge them into the returned config (preserving XP_DEFAULTS and
XP_DEFAULTS.roleRewards), or run a one-time migration of stored guild configs to
move reputation.* into cfg.xp before shipping; specifically update getXpConfig
to check cfg.xp.levelThresholds and cfg.xp.roleRewards and if absent copy/merge
from cfg.reputation.levelThresholds and cfg.reputation.roleRewards into the
merged object so legacy customizations are preserved.
In `@tests/api/routes/members.test.js`:
- Around line 179-180: The test fixture for getConfig() still places thresholds
under reputation, but the route now reads xp.levelThresholds, so update the
mocked config returned by getConfig() in the tests to include an xp object with
levelThresholds (distinct values from XP_DEFAULTS) so the expectation on
alice.level is deterministic; locate the getConfig() mock used by
tests/api/routes/members.test.js and replace reputation.thresholds (or similar)
with xp: { levelThresholds: [...] } ensuring values differ from
XP_DEFAULTS.levelThresholds so the assertion against alice.level exercises the
new code path.
In `@tests/modules/actions/grantRole.test.js`:
- Around line 119-142: The test uses roles ('role-l2'...'role-l5') that aren't
in the default guild cache; extend makeContext to accept a guildRoles parameter
(or add these roles to its existing guild.roles.cache) so the guild role cache
contains those role objects, then update the test to call makeContext with a
guildRoles Map containing entries for 'role-l2'...'role-l5'; this ensures
handleGrantRole sees real guild roles in the guild.roles.cache when it runs and
keeps the rest of the test (xpManagedRoles, memberRoles, stackRoles) unchanged.
In `@web/src/components/dashboard/config-categories/onboarding-growth.tsx`:
- Line 136: Replace the double-quoted string literals in this .tsx file with
single-quoted strings per the style guide: locate the onboarding messages (the
array/const containing strings such as "Things are moving at a healthy pace in
`#general`, so you'll fit right in.") and change them to use single quotes and
escape any internal apostrophes (e.g., you'll -> you\'ll). Apply the same change
to the other occurrences referenced (the strings at the other spots noted) so
all string literals in onboarding-growth.tsx use single quotes.
In `@web/src/components/landing/bento/BentoChart.tsx`:
- Around line 257-258: The day label rendering in BentoChart uses non-unique
keys (dayLabels) which repeat across weeks; update the map where dayLabels is
rendered (the JSX mapping that produces <span key={label}>{label}</span>) to use
a unique key per item—e.g., combine the weekday label with the source date or
index (such as `${label}-${i}` or the original dailyActivity date string) so
each key is globally unique and avoids React reconciliation collisions.
In `@web/src/components/landing/Footer.tsx`:
- Line 37: The long JSX className string in the Footer component (the className
prop on the rounded button element) exceeds the 100-char width; split the
classes into multiple shorter lines by turning the single string into an array
of class tokens and using .join(' ') (e.g., replace className="rounded-full h-14
px-12 font-bold text-sm tracking-widest uppercase shadow-lg
shadow-[var(--color-discord)]/20" with className={[ 'rounded-full', 'h-14',
'px-12', 'font-bold', 'text-sm', 'tracking-widest', 'uppercase', 'shadow-lg',
'shadow-[var(--color-discord)]/20' ].join(' ')}), and do the same for the other
long className on the same file to keep each source line under 100 characters.
In `@web/src/components/ui/neo-minimal-footer.tsx`:
- Line 27: Extract the repeated external URLs (Docs/GitHub/Discord) into shared
constants (e.g., DOCS_URL, GITHUB_URL, DISCORD_URL) in a central module and
replace hard-coded strings in NeoMinimalFooter (the menu items like the object
with label 'Self-Host') and other components that reference the same links by
importing these constants; update any arrays or href usages in the footer
component and corresponding header/navigation components to use the constants so
all places reference a single source of truth.
- Around line 50-52: The long className strings in the neo-minimal-footer
component exceed the 100-char line limit; split/wrap these className values into
multiple concatenated strings or template literals so each line stays ≤100
chars. Specifically update the footer element's className (the long string
starting with "max-w-7xl..."), the absolute background grid div's className (the
long bg-[linear-gradient(...)] string), and the other oversized className props
referenced around lines 78, 103-105, 116, and 158 in this file (search for
multiple long className="..." occurrences in neo-minimal-footer.tsx) so each
class segment is on its own line but retains the same final combined className
at runtime.
---
Duplicate comments:
In `@src/modules/actions/grantRole.js`:
- Around line 25-49: The replace-mode flow mutates roles one-by-one (loop using
member.roles.remove, calling checkRoleRateLimit and recordRoleChange per removed
role) which can leave users in an inconsistent state if the later
member.roles.add fails; instead compute the set of roles to remove (using
xpManagedRoles and roleId), batch the intended changes, perform the removes and
the add as a single logical operation (e.g., collect removal IDs, ensure rate
limit once via checkRoleRateLimit(guild.id, member.user?.id) before applying,
then call member.roles.remove with the full array and member.roles.add(roleId)
and only then call recordRoleChange once), and update logging/permission checks
(canManageRole) to run during diff computation rather than per removal so the
operation is atomic from the recordRoleChange perspective.
In `@src/modules/actions/removeRole.js`:
- Around line 24-25: The removeRole action currently calls
recordRoleChange(guild.id, member.user?.id) after awaiting
member.roles.remove(roleId), which records limiter usage per individual removal;
remove that call from this handler (i.e., delete the recordRoleChange invocation
in the removeRole flow) so the handler only performs the role removal and lets
the caller/batch processor record a single limiter entry for the whole pipeline
run.
In `@src/modules/actions/roleUtils.js`:
- Around line 152-179: remove the per-role counting from inside removeSingleRole
by deleting the recordRoleChange(guild.id, member.user.id) call and have
removeSingleRole only attempt removal and return success/failure; instead, in
the higher-level level-down batch handlers (the batch path and the fallback path
referenced around the other block at lines 217-235), call recordRoleChange once
per logical level-down operation (once per member/guild batch) after all role
removals complete so the limiter is incremented a single time for the whole
batch rather than once per removed role.
In `@src/modules/reputation.js`:
- Around line 164-180: The level-up pipeline (executeLevelUpPipeline) currently
invokes action handlers but only has grantRole/removeRole implemented, so no
announcement is made; add a built-in announcement action handler (e.g., create
src/modules/actions/announce.js exporting the handler and register it where
other actions are collected/registered) and ensure executeLevelUpPipeline looks
up and invokes the "announce" action when levelActions include an announcement,
or alternatively update docs to require guilds include a custom "announce"
handler in their levelActions configuration; reference executeLevelUpPipeline,
grantRole, removeRole, and levelActions when wiring/ documenting the change.
In `@tests/modules/actions/removeRole.test.js`:
- Around line 9-13: The test file mocks checkRoleRateLimit but handleRemoveRole
never calls it, so remove the dead mock of checkRoleRateLimit from
tests/modules/actions/removeRole.test.js (keep canManageRole and
recordRoleChange as needed), delete the duplicated canManageRole===false case
and the duplicated "pipeline" happy-path assertions, and instead add a dedicated
rate-limit test in levelUpActions.test.js that exercises the codepath which
calls checkRoleRateLimit (invoke the pipeline-related function used in level-up
flow and assert rate-limit behavior), ensuring you reference handleRemoveRole
for trimming and checkRoleRateLimit for where to add coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7902ff0c-512a-4e89-91e1-640fb125688f
📒 Files selected for processing (18)
src/api/routes/community.jssrc/api/routes/members.jssrc/modules/actions/grantRole.jssrc/modules/actions/removeRole.jssrc/modules/actions/roleUtils.jssrc/modules/levelUpActions.jssrc/modules/reputation.jstests/api/routes/members.test.jstests/modules/actions/grantRole.test.jstests/modules/actions/removeRole.test.jsweb/src/components/dashboard/config-categories/onboarding-growth.tsxweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsxweb/src/components/landing/DashboardShowcase.tsxweb/src/components/landing/Footer.tsxweb/src/components/landing/bento/BentoChart.tsxweb/src/components/ui/neo-minimal-footer.tsxweb/src/components/ui/simple-icon.tsxweb/tests/components/landing/footer.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Greptile Review
- GitHub Check: Cursor Bugbot
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/landing/Footer.tsxweb/tests/components/landing/footer.test.tsxsrc/modules/actions/removeRole.jsweb/src/components/landing/bento/BentoChart.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsxsrc/modules/reputation.jssrc/api/routes/community.jsweb/src/components/ui/simple-icon.tsxtests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jsweb/src/components/ui/neo-minimal-footer.tsxsrc/api/routes/members.jssrc/modules/actions/grantRole.jsweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsxsrc/modules/levelUpActions.jstests/api/routes/members.test.jssrc/modules/actions/roleUtils.js
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use
console.*methods in web dashboard code; use appropriate logging mechanisms for React applications
Files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/landing/Footer.tsxweb/src/components/landing/bento/BentoChart.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsxweb/src/components/ui/simple-icon.tsxweb/src/components/ui/neo-minimal-footer.tsxweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: ESM only - do not use CommonJS modules
Use src/logger.js; do not use console.*
Files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/landing/Footer.tsxweb/tests/components/landing/footer.test.tsxsrc/modules/actions/removeRole.jsweb/src/components/landing/bento/BentoChart.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsxsrc/modules/reputation.jssrc/api/routes/community.jsweb/src/components/ui/simple-icon.tsxtests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jsweb/src/components/ui/neo-minimal-footer.tsxsrc/api/routes/members.jssrc/modules/actions/grantRole.jsweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsxsrc/modules/levelUpActions.jstests/api/routes/members.test.jssrc/modules/actions/roleUtils.js
web/tests/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/tests/**/*.test.{ts,tsx}: Write web dashboard tests using Vitest 4 with thejsdomenvironment and React Testing Library, matching theweb/src/structure
Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Files:
web/tests/components/landing/footer.test.tsx
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
src/modules/actions/removeRole.jssrc/modules/reputation.jssrc/api/routes/community.jstests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jssrc/api/routes/members.jssrc/modules/actions/grantRole.jssrc/modules/levelUpActions.jstests/api/routes/members.test.jssrc/modules/actions/roleUtils.js
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Never useconsole.*methods; use the Winston logger instead viaimport logger from '../logger.js'(adjust path as needed), then calllogger.info(),logger.warn(),logger.error(), orlogger.debug()
Always usesafeReply(),safeSend(), orsafeEditReply()instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Files:
src/modules/actions/removeRole.jssrc/modules/reputation.jssrc/api/routes/community.jssrc/api/routes/members.jssrc/modules/actions/grantRole.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.js
src/modules/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create feature modules in
src/modules/and add corresponding config sections toconfig.json
Files:
src/modules/actions/removeRole.jssrc/modules/reputation.jssrc/modules/actions/grantRole.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.js
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts}: Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Community features should be gated behind config..enabled. Moderation commands are the exception.
Files:
src/modules/actions/removeRole.jssrc/modules/reputation.jssrc/api/routes/community.jssrc/api/routes/members.jssrc/modules/actions/grantRole.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.js
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL only; do not use string concatenation for SQL queries
Files:
src/modules/actions/removeRole.jssrc/modules/reputation.jssrc/api/routes/community.jstests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jssrc/api/routes/members.jssrc/modules/actions/grantRole.jssrc/modules/levelUpActions.jstests/api/routes/members.test.jssrc/modules/actions/roleUtils.js
src/api/routes/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create API route handlers in
src/api/routes/and mount them insrc/api/server.js; add auth middleware if the endpoint requires authentication
Files:
src/api/routes/community.jssrc/api/routes/members.js
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jstests/api/routes/members.test.js
🧠 Learnings (18)
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/src/app/dashboard/**/*.tsx : For dashboard routes, add a matcher entry to `dashboardTitleMatchers` in `web/src/lib/page-titles.ts`: use exact equality for leaf routes (`pathname === '/dashboard/my-route'`) and subtree checks (`pathname.startsWith('/dashboard/my-route/')`); export `metadata` using `createPageMetadata(title)` for SSR entry points
Applied to files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsx
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/pages/dashboard/**/*.{ts,tsx} : Use shared title helpers from web/src/lib/page-titles.ts for setting browser titles in dashboard pages
Applied to files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsx
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to web/src/components/layout/dashboard-shell.tsx : Dashboard page titles should sync with route changes using DashboardTitleSync component mounted in dashboard-shell.tsx and canonical title string 'Volvox.Bot - AI Powered Discord Bot'
Applied to files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsx
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.{ts,tsx} : Web dashboard config editor should use category workspace navigation with reusable SettingsFeatureCard pattern (header + master toggle + Basic/Advanced blocks)
Applied to files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsxweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsx
📚 Learning: 2026-03-26T00:04:14.673Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.673Z
Learning: Applies to {web/src/lib/page-titles.ts,web/src/**/+page.ts,web/src/**/+page.svelte} : New dashboard routes need title wiring in web/src/lib/page-titles.ts: use createPageMetadata() for SSR and keep DashboardTitleSync aligned for client navigation.
Applied to files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsx
📚 Learning: 2026-03-11T05:32:46.325Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T05:32:46.325Z
Learning: Applies to web/src/app/**/*.{ts,tsx} : Apply static metadata to server-rendered dashboard entry pages and use title template format for root app metadata
Applied to files:
web/src/components/landing/DashboardShowcase.tsxweb/src/components/dashboard/config-categories/onboarding-growth.tsx
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Applied to files:
src/modules/actions/removeRole.jssrc/api/routes/community.jssrc/api/routes/members.jssrc/modules/actions/grantRole.jssrc/modules/actions/roleUtils.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
src/modules/actions/removeRole.jssrc/modules/reputation.jstests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.jssrc/modules/actions/grantRole.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/reactionrole.js : Enforce invoker role hierarchy check in /reactionrole add command to prevent non-owner users from configuring roles at or above their highest role
Applied to files:
src/modules/actions/removeRole.jssrc/modules/reputation.jstests/modules/actions/grantRole.test.jssrc/api/routes/members.jssrc/modules/actions/grantRole.jssrc/modules/levelUpActions.jssrc/modules/actions/roleUtils.js
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Applied to files:
src/modules/actions/removeRole.jssrc/api/routes/members.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to **/*.{js,ts,tsx} : Use single quotes for strings (except in JSON files); no double quotes
Applied to files:
web/src/components/dashboard/config-categories/onboarding-growth.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Never use `console.*` methods; use the Winston logger instead via `import logger from '../logger.js'` (adjust path as needed), then call `logger.info()`, `logger.warn()`, `logger.error()`, or `logger.debug()`
Applied to files:
src/modules/reputation.jssrc/modules/actions/roleUtils.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/utils/cache.js : Use `src/utils/cache.js` for generic caching with Redis primary + in-memory fallback; use `src/utils/discordCache.js` for Discord API response caching (channels, roles, members); use `src/utils/reputationCache.js` for leaderboard and user reputation data; all caches auto-invalidate when config changes
Applied to files:
src/modules/reputation.jssrc/api/routes/community.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Write web dashboard tests using Vitest 4 with the `jsdom` environment and React Testing Library, matching the `web/src/` structure
Applied to files:
tests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Write bot tests using Vitest 4 with the `node` environment, matching the `src/` structure in the `tests/` directory
Applied to files:
tests/modules/actions/removeRole.test.jstests/modules/actions/grantRole.test.js
📚 Learning: 2026-03-26T00:04:14.673Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.673Z
Learning: Applies to src/**/*.{js,ts} : Community features should be gated behind config.<feature>.enabled. Moderation commands are the exception.
Applied to files:
web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Always use `safeReply()`, `safeSend()`, or `safeEditReply()` instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Applied to files:
src/modules/actions/roleUtils.js
📚 Learning: 2026-03-26T00:04:14.673Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.673Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Use src/logger.js; do not use console.*
Applied to files:
src/modules/actions/roleUtils.js
🪛 GitHub Check: SonarCloud Code Analysis
src/modules/actions/grantRole.js
[failure] 18-18: Refactor this function to reduce its Cognitive Complexity from 20 to the 15 allowed.
src/modules/levelUpActions.js
[failure] 79-79: Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.
src/modules/actions/roleUtils.js
[failure] 191-191: Refactor this function to reduce its Cognitive Complexity from 30 to the 15 allowed.
[warning] 221-221: Remove the declaration of the unused '_roleId' variable.
🔇 Additional comments (26)
web/src/components/landing/DashboardShowcase.tsx (1)
42-44: LGTM! Clean import/re-export pattern.The reorganization correctly imports the type for internal use (line 54) while also re-exporting it for consumers. The blank line separator improves readability between import and export declarations.
web/src/components/dashboard/config-categories/onboarding-growth.tsx (1)
331-345: Good accessibility improvement.The refactoring from
aria-labelledby(which ChannelSelector never forwarded to its internal Button) to implicit label association via wrapping is correct. The nested<Button role="combobox">is a labelable element, so the browser will properly associate the label with it.The biome-ignore comments are appropriate and well-documented.
Also applies to: 347-361
web/src/components/ui/simple-icon.tsx (1)
1-15: Clean, reusable icon helper implementation.This abstraction is concise and integrates well for footer social icons.
web/src/components/landing/Footer.tsx (1)
61-63: Good separation of concerns with footer extraction.Delegating footer UI to
NeoMinimalFooterkeeps this component focused on the CTA section.web/tests/components/landing/footer.test.tsx (1)
73-97: Solid coverage additions for the new footer UX.These tests validate the new newsletter and social/status link behavior with good, user-facing selectors.
web/src/components/landing/bento/BentoChart.tsx (2)
71-72: Date parsing refactor is clean and safe.The template-literal form keeps behavior intact and improves readability in both label paths.
Also applies to: 87-93
215-218: Hit-zone key change is a good improvement.Using coordinate-based keys is stronger than index-only keys for these rendered SVG hit areas.
web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx (2)
58-60: No changes needed - variable names are correct.The variables at lines 61-63 are already named
tldrDefaultMessages,tldrMaxMessages, andtldrCooldownSeconds(without leading underscores), and they are correctly referenced at lines 646, 666, and 686. There are no unresolved identifiers or compilation issues. The suggested diff does not apply to the current code state.> Likely an incorrect or invalid review comment.
252-255: Fix TypeScript error and key instability in badge rendering.
badge.emojidoes not exist on theBadgetype (onlydaysandlabelare defined), causing a TypeScript error. Additionally, usingbadge.daysin the key causes the key to change whenever the value is edited, which remounts the element mid-edit.Change the key to a stable identifier:
key={`badge-${index}`}src/api/routes/members.js (6)
11-11: LGTM!Clean import of
computeLevelandgetXpConfigfrom the reputation module, following ESM conventions.
86-86: LGTM!The CSV export correctly computes level from XP using
computeLevel(xp, xpConfig.levelThresholds)instead of relying on the potentially stalerep.leveldatabase column. This ensures consistency after threshold configuration changes.Also applies to: 138-150
383-406: LGTM!The member list enrichment properly computes level from XP for both cached and freshly-fetched data, ensuring consistent level computation across the codebase.
656-660: LGTM!Good addition of the explanatory comment at line 658 clarifying why level is recomputed from XP rather than trusting the stored
reputation.levelcolumn.
950-957: LGTM!The
xpConfigvariable is now properly hoisted to the outer scope (line 950), andoldLevelis correctly computed fromoldXp = newXp - amount(lines 976-978). This addresses the previous review concerns about config drift and stale level detection.Also applies to: 976-981
999-1015: LGTM on the level-down enforcement logic.The error isolation with try/catch ensures that failures in role removal don't affect the API response. The response is correctly sent after this block completes, so even if
enforceRoleLevelDownthrows, the client still receives the updated XP/level data.tests/modules/actions/grantRole.test.js (4)
1-15: LGTM!Clean test setup with proper mocking of dependencies. The mocks for
logger.jsandroleUtils.jsare minimal and focused on the functions actually used by the handler.
17-50: LGTM!The
makeContexthelper is well-structured and provides sensible defaults while allowing customization ofmemberRoles,xpManagedRoles, andstackRolesfor different test scenarios.
57-89: LGTM!Good coverage of the core grant path: verifying
roles.addis called,recordRoleChangeis invoked, andtemplateContextis populated with role metadata.
91-117: LGTM!The stackRoles tests correctly verify that when
stackRoles: false, other XP-managed roles are removed (but unrelated roles are preserved), and whenstackRoles: true, no roles are removed.src/modules/levelUpActions.js (7)
1-13: LGTM!Clean module setup with clear documentation and proper ESM imports. The issue link in the JSDoc is helpful for traceability.
14-34: LGTM!The action registry pattern provides good extensibility for Phase 2 additions. Registration of built-in handlers (
grantRole,removeRole) at module load time ensures they're available before any pipeline execution.
44-64: LGTM!
resolveActionscorrectly handles:
- Early return when no level-up occurred (
newLevel <= previousLevel)- Level skips by iterating from
previousLevel + 1tonewLevel- Per-level overrides via
config.levelActionswith fallback toconfig.defaultActions
99-102: LGTM on rate limit quota tracking.The approach of checking rate limit once and tracking a per-pipeline quota of 2 role changes addresses the previous review concern. Role actions now decrement this quota and are skipped when exhausted, while non-role actions continue executing.
129-152: LGTM on template context error handling.The try/catch around
buildTemplateContextwith fallback to an empty object ensures per-action error isolation as required by#365. The warning log includes sufficient context for debugging.
167-179: LGTM on per-action rate limit enforcement.Role actions (
grantRole,removeRole) are correctly gated by the remaining quota, while other action types bypass this check entirely. This addresses the concern about the rate limiter being a pipeline-wide kill switch.
181-191: LGTM on per-action error isolation.The try/catch around handler execution ensures failures don't abort the pipeline, satisfying the error isolation requirement from
#365. The warning log includes action type, level, and error details.
| try { | ||
| const member = await req.guild.members.fetch(userId); | ||
| const { enforceRoleLevelDown } = await import('../../modules/actions/roleUtils.js'); | ||
| await enforceRoleLevelDown(member, newLevel, xpConfig); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using a static import for enforceRoleLevelDown.
The dynamic import() here adds latency on each XP reduction that triggers level-down. Since roleUtils.js is already imported elsewhere in the codebase and this code path is not conditionally loaded, a static import at the top of the file would be more efficient.
♻️ Proposed fix — use static import
Add at the top with other imports:
import { computeLevel, getXpConfig } from '../../modules/reputation.js';
+import { enforceRoleLevelDown } from '../../modules/actions/roleUtils.js';Then simplify the usage:
try {
const member = await req.guild.members.fetch(userId);
- const { enforceRoleLevelDown } = await import('../../modules/actions/roleUtils.js');
await enforceRoleLevelDown(member, newLevel, xpConfig);
} catch (err) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/routes/members.js` around lines 1002 - 1005, Replace the dynamic
import with a static import: add a top-of-file import for enforceRoleLevelDown
from roleUtils.js in members.js, then remove the await
import('../../modules/actions/roleUtils.js') line and call
enforceRoleLevelDown(member, newLevel, xpConfig) directly where it’s currently
used to avoid per-call latency.
| function collectRolesToRemove(member, newLevel, xpConfig) { | ||
| const rolesToRemove = []; | ||
| const seenRoleIds = new Set(); | ||
|
|
||
| // Collect roles from levelActions (above newLevel) | ||
| for (const entry of xpConfig.levelActions ?? []) { | ||
| if (entry.level <= newLevel) continue; | ||
|
|
||
| for (const action of entry.actions ?? []) { | ||
| if (action.type !== 'grantRole' || !action.roleId) continue; | ||
| if (!member.roles.cache.has(action.roleId)) continue; | ||
| if (seenRoleIds.has(action.roleId)) continue; | ||
|
|
||
| seenRoleIds.add(action.roleId); | ||
| rolesToRemove.push({ roleId: action.roleId, entry }); | ||
| } | ||
| } | ||
|
|
||
| return rolesToRemove; |
There was a problem hiding this comment.
defaultActions are ignored during level-down reconciliation.
This code only computes removals/restores from levelActions, but
defaultActions are part of the resolved XP config and are also treated as
XP-managed roles later in this module. A role granted only via defaultActions
will survive a drop to level 0, and replace mode will not restore it when a
member falls below a higher per-level reward.
Also applies to: 238-251
| export async function executeLevelUpPipeline({ | ||
| member, | ||
| message, | ||
| guild, | ||
| previousLevel, | ||
| newLevel, | ||
| xp, | ||
| config, | ||
| }) { | ||
| const actions = resolveActions(previousLevel, newLevel, config); | ||
| if (actions.length === 0) return; | ||
|
|
||
| info('Executing level-up pipeline', { | ||
| guildId: guild.id, | ||
| userId: member.user?.id, | ||
| previousLevel, | ||
| newLevel, | ||
| actionCount: actions.length, | ||
| }); | ||
|
|
||
| // Check rate limit and track remaining quota (2 changes per pipeline) | ||
| // Note: We don't return early here - rate limit only skips role actions, not the whole pipeline | ||
| const rateLimitOk = checkRoleRateLimit(guild.id, member.user?.id); | ||
| let roleChangesRemaining = rateLimitOk ? 2 : 0; | ||
|
|
||
| // Compute XP-managed roles once for stack/replace logic | ||
| const xpManagedRoles = collectXpManagedRoles(config); | ||
|
|
||
| // Build base pipeline context | ||
| const basePipelineContext = { | ||
| member, | ||
| message, | ||
| guild, | ||
| previousLevel, | ||
| newLevel, | ||
| xp, | ||
| config, | ||
| xpManagedRoles, | ||
| }; | ||
|
|
||
| // Cache template contexts per level to avoid duplicate DB queries | ||
| const templateContextCache = new Map(); | ||
|
|
||
| for (const { level, action } of actions) { | ||
| // Track previousLevel incrementally for correct intermediate level context | ||
| const levelPreviousLevel = level - 1; | ||
|
|
||
| // Rebuild template context for each intermediate level during level-skip | ||
| // Cache per level to avoid duplicate DB queries | ||
| let templateContext = templateContextCache.get(level); | ||
| if (!templateContext) { | ||
| try { | ||
| templateContext = await buildTemplateContext({ | ||
| member, | ||
| message, | ||
| guild, | ||
| level, | ||
| previousLevel: levelPreviousLevel, | ||
| xp, | ||
| levelThresholds: config.levelThresholds ?? [], | ||
| roleName: null, | ||
| roleId: null, | ||
| }); | ||
| templateContextCache.set(level, templateContext); | ||
| } catch (err) { | ||
| warn('Template context build failed — continuing with empty context', { | ||
| level, | ||
| guildId: guild.id, | ||
| userId: member.user?.id, | ||
| error: err.message, | ||
| }); | ||
| templateContext = {}; | ||
| templateContextCache.set(level, templateContext); | ||
| } | ||
| } | ||
|
|
||
| const pipelineContext = { ...basePipelineContext, templateContext, currentLevel: level }; | ||
|
|
||
| const handler = actionRegistry.get(action.type); | ||
| if (!handler) { | ||
| warn('Unknown action type — skipping', { | ||
| actionType: action.type, | ||
| level, | ||
| guildId: guild.id, | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| // Skip role-related actions if rate limit quota is exhausted | ||
| if (action.type === 'grantRole' || action.type === 'removeRole') { | ||
| if (roleChangesRemaining <= 0) { | ||
| warn('Role action skipped due to rate limit quota exhausted', { | ||
| actionType: action.type, | ||
| level, | ||
| guildId: guild.id, | ||
| userId: member.user?.id, | ||
| }); | ||
| continue; | ||
| } | ||
| roleChangesRemaining--; | ||
| } | ||
|
|
||
| try { | ||
| await handler(action, pipelineContext); | ||
| } catch (err) { | ||
| warn('Action failed in level-up pipeline — continuing', { | ||
| actionType: action.type, | ||
| level, | ||
| guildId: guild.id, | ||
| userId: member.user?.id, | ||
| error: err.message, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reduce cognitive complexity by extracting helper functions.
SonarCloud flags this function with cognitive complexity 17 (limit 15). Consider extracting the template context resolution and the action dispatch logic into helper functions.
♻️ Suggested refactor — extract helpers
+/**
+ * Get or build template context for a level, caching to avoid duplicate DB queries.
+ */
+async function getTemplateContext(cache, params) {
+ const { level, member, message, guild, xp, config } = params;
+ if (cache.has(level)) return cache.get(level);
+
+ try {
+ const ctx = await buildTemplateContext({
+ member,
+ message,
+ guild,
+ level,
+ previousLevel: level - 1,
+ xp,
+ levelThresholds: config.levelThresholds ?? [],
+ roleName: null,
+ roleId: null,
+ });
+ cache.set(level, ctx);
+ return ctx;
+ } catch (err) {
+ warn('Template context build failed — continuing with empty context', {
+ level,
+ guildId: guild.id,
+ userId: member.user?.id,
+ error: err.message,
+ });
+ cache.set(level, {});
+ return {};
+ }
+}
+
+const ROLE_ACTION_TYPES = new Set(['grantRole', 'removeRole']);
+
export async function executeLevelUpPipeline({ ... }) {
// ... setup code ...
for (const { level, action } of actions) {
- const levelPreviousLevel = level - 1;
-
- let templateContext = templateContextCache.get(level);
- if (!templateContext) {
- try {
- templateContext = await buildTemplateContext({ ... });
- templateContextCache.set(level, templateContext);
- } catch (err) {
- warn(...);
- templateContext = {};
- templateContextCache.set(level, templateContext);
- }
- }
+ const templateContext = await getTemplateContext(templateContextCache, {
+ level, member, message, guild, xp, config,
+ });
// ... rest of loop ...
- if (action.type === 'grantRole' || action.type === 'removeRole') {
+ if (ROLE_ACTION_TYPES.has(action.type)) {
if (roleChangesRemaining <= 0) {
// ... warn and skip ...
}
roleChangesRemaining--;
}🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 79-79: Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/levelUpActions.js` around lines 79 - 192, The
executeLevelUpPipeline function is too complex; extract the template-context
resolution and per-action dispatch into helpers: create a
resolveTemplateContext(level) helper that uses templateContextCache, calls
buildTemplateContext with ({member,message,guild,level,previousLevel:
level-1,xp,levelThresholds:config.levelThresholds,roleName:null,roleId:null}),
catches errors, logs via warn and returns the cached or empty context; and
create a dispatchLevelAction(action, pipelineContext, roleChangesRemainingRef)
helper that contains rate-limit checks for grantRole/removeRole, retrieves the
handler via actionRegistry.get(action.type), logs unknown types, decrements
roleChangesRemainingRef when consuming quota, calls
handler(action,pipelineContext) inside try/catch and logs failures; replace the
in-loop templateContext construction and action handling with calls to these two
helpers, passing necessary values
(member,message,guild,config,xp,templateContextCache,roleChangesRemaining) so
cognitive complexity is reduced.
| export function getXpConfig(guildId) { | ||
| const cfg = getConfig(guildId); | ||
| const merged = { ...XP_DEFAULTS, ...cfg.xp }; | ||
| // Deep merge roleRewards to preserve defaults for missing nested properties | ||
| if (cfg.xp?.roleRewards) { | ||
| merged.roleRewards = { ...XP_DEFAULTS.roleRewards, ...cfg.xp.roleRewards }; | ||
| } | ||
| return merged; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the structure of the src/modules directory
find src/modules -type f -name "*.js" | head -20Repository: VolvoxLLC/volvox-bot
Length of output: 634
🏁 Script executed:
# Read the reputation.js file to see the full implementation
wc -l src/modules/reputation.jsRepository: VolvoxLLC/volvox-bot
Length of output: 93
🏁 Script executed:
# Get the complete reputation.js file
cat -n src/modules/reputation.jsRepository: VolvoxLLC/volvox-bot
Length of output: 7047
🏁 Script executed:
# Search for config migration logic and how old config keys are handled
rg "levelThresholds|roleRewards" --type js -B 2 -A 2Repository: VolvoxLLC/volvox-bot
Length of output: 17913
🏁 Script executed:
# Look for executeLevelUpPipeline implementation and action handlers
ast-grep --pattern 'function executeLevelUpPipeline($_) { $$$ }'Repository: VolvoxLLC/volvox-bot
Length of output: 7196
🏁 Script executed:
# Search for action handlers, particularly announcement handlers
rg "action|handler|announce" src/modules/ --type js -iRepository: VolvoxLLC/volvox-bot
Length of output: 50376
🏁 Script executed:
# Check the config.json or config defaults to understand the old vs new structure
fd config.json | head -5Repository: VolvoxLLC/volvox-bot
Length of output: 93
🏁 Script executed:
# Check if there's migration logic in the config module
cat -n src/modules/config.js | head -100Repository: VolvoxLLC/volvox-bot
Length of output: 4725
🏁 Script executed:
# Look for any backwards compatibility or migration comments in the codebase
rg "migration|migrate|backward|compat|legacy" --type js -iRepository: VolvoxLLC/volvox-bot
Length of output: 12825
🏁 Script executed:
# Check the config.json structure
cat config.json | head -100Repository: VolvoxLLC/volvox-bot
Length of output: 3296
🏁 Script executed:
# Search for any mention of old reputation config keys still being used
rg "reputation\.levelThresholds|reputation\.roleRewards" --type jsRepository: VolvoxLLC/volvox-bot
Length of output: 46
Add a rollout fallback for legacy XP config keys, or migrate configs before shipping.
Existing guild configs storing custom reputation.levelThresholds or reputation.roleRewards will silently lose these customizations because getXpConfig() only reads from cfg.xp. Unlike other backward compatibility patterns in the codebase (e.g., adminRoleId fallback), there is no fallback to the old reputation keys. Additionally, if guilds rely on automatic level-up announcements, these will be missing unless an action handler is explicitly registered, since no announcement/message action type is implemented.
Either migrate stored configs before this ships, or add temporary fallback logic in getXpConfig() to read legacy keys during a transition period.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/reputation.js` around lines 48 - 55, getXpConfig currently only
reads cfg.xp and will drop legacy reputation.* keys; add a rollout fallback that
reads cfg.reputation.levelThresholds and cfg.reputation.roleRewards when
cfg.xp.* are missing and merge them into the returned config (preserving
XP_DEFAULTS and XP_DEFAULTS.roleRewards), or run a one-time migration of stored
guild configs to move reputation.* into cfg.xp before shipping; specifically
update getXpConfig to check cfg.xp.levelThresholds and cfg.xp.roleRewards and if
absent copy/merge from cfg.reputation.levelThresholds and
cfg.reputation.roleRewards into the merged object so legacy customizations are
preserved.
| // 250 XP with thresholds [100, 300, 600, 1000] = level 1 (0-indexed) | ||
| expect(alice.level).toBe(1); |
There was a problem hiding this comment.
Mock the new config shape here.
This expectation is right, but the getConfig() fixture above still places
thresholds under reputation. The route now reads xp.levelThresholds, so this
passes only because XP_DEFAULTS.levelThresholds currently match the old
fixture. Move the test data to xp and make it distinct from the defaults.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/api/routes/members.test.js` around lines 179 - 180, The test fixture
for getConfig() still places thresholds under reputation, but the route now
reads xp.levelThresholds, so update the mocked config returned by getConfig() in
the tests to include an xp object with levelThresholds (distinct values from
XP_DEFAULTS) so the expectation on alice.level is deterministic; locate the
getConfig() mock used by tests/api/routes/members.test.js and replace
reputation.thresholds (or similar) with xp: { levelThresholds: [...] } ensuring
values differ from XP_DEFAULTS.levelThresholds so the assertion against
alice.level exercises the new code path.
| .replace( | ||
| /{vibeLine}/g, | ||
| 'Things are moving at a healthy pace in #general, so you\'ll fit right in.', | ||
| "Things are moving at a healthy pace in #general, so you'll fit right in.", |
There was a problem hiding this comment.
Use single quotes per coding guidelines.
These strings use double quotes to avoid escaping the apostrophes, but the coding guidelines require single quotes for all strings in .tsx files.
Proposed fix
@@ -133,7 +133,7 @@
.replace(
/{vibeLine}/g,
- "Things are moving at a healthy pace in `#general`, so you'll fit right in.",
+ 'Things are moving at a healthy pace in `#general`, so you\'ll fit right in.',
)@@ -228,9 +228,9 @@
<div>
<code>{'{vibeLine}'}</code> — Activity description{' '}
<span className="italic text-muted-foreground/70">
- {
- "You're catching us in a quiet window - perfect time to introduce yourself."
- }
+ {'You\'re catching us in a quiet window - perfect time to introduce yourself.'}
</span>
</div>As per coding guidelines: "Use single quotes for strings (except in JSON files); no double quotes".
Also applies to: 231-233
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/dashboard/config-categories/onboarding-growth.tsx` at line
136, Replace the double-quoted string literals in this .tsx file with
single-quoted strings per the style guide: locate the onboarding messages (the
array/const containing strings such as "Things are moving at a healthy pace in
`#general`, so you'll fit right in.") and change them to use single quotes and
escape any internal apostrophes (e.g., you'll -> you\'ll). Apply the same change
to the other occurrences referenced (the strings at the other spots noted) so
all string literals in onboarding-growth.tsx use single quotes.
| <GetStartedButton | ||
| variant="discord" | ||
| href={botInviteUrl} | ||
| className="rounded-full h-14 px-12 font-bold text-sm tracking-widest uppercase shadow-lg shadow-[var(--color-discord)]/20" |
There was a problem hiding this comment.
Split long className values to stay within max width.
Line 37 and Line 44 exceed the 100-character limit.
As per coding guidelines **/*.{js,ts,tsx}: Maintain a maximum line width of 100 characters.
Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/landing/Footer.tsx` at line 37, The long JSX className
string in the Footer component (the className prop on the rounded button
element) exceeds the 100-char width; split the classes into multiple shorter
lines by turning the single string into an array of class tokens and using
.join(' ') (e.g., replace className="rounded-full h-14 px-12 font-bold text-sm
tracking-widest uppercase shadow-lg shadow-[var(--color-discord)]/20" with
className={[ 'rounded-full', 'h-14', 'px-12', 'font-bold', 'text-sm',
'tracking-widest', 'uppercase', 'shadow-lg', 'shadow-[var(--color-discord)]/20'
].join(' ')}), and do the same for the other long className on the same file to
keep each source line under 100 characters.
| { label: 'Features', href: '#features' }, | ||
| { label: 'Pricing', href: '#pricing' }, | ||
| { label: 'Dashboard', href: '/login' }, | ||
| { label: 'Self-Host', href: 'https://docs.volvox.bot' }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract repeated external URLs into shared constants.
Docs/GitHub/Discord links are duplicated in multiple places (including header). Centralizing them reduces drift and update mistakes.
Also applies to: 33-35, 125-125, 134-134, 143-143, 155-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ui/neo-minimal-footer.tsx` at line 27, Extract the
repeated external URLs (Docs/GitHub/Discord) into shared constants (e.g.,
DOCS_URL, GITHUB_URL, DISCORD_URL) in a central module and replace hard-coded
strings in NeoMinimalFooter (the menu items like the object with label
'Self-Host') and other components that reference the same links by importing
these constants; update any arrays or href usages in the footer component and
corresponding header/navigation components to use the constants so all places
reference a single source of truth.
| <footer className="max-w-7xl mx-auto bg-card/10 border-t rounded-t-lg border-card/10 flex flex-wrap pt-16 pb-8 relative overflow-hidden"> | ||
| {/* Background grid pattern */} | ||
| <div className="absolute inset-0 bg-[linear-gradient(hsl(var(--foreground)/0.02)_1px,transparent_1px),linear-gradient(90deg,hsl(var(--foreground)/0.02)_1px,transparent_1px)] bg-[size:40px_40px] [mask-image:radial-gradient(circle_at_center,black,transparent_80%)]" /> |
There was a problem hiding this comment.
Wrap oversized class strings to satisfy line-width rule.
Line 50, Line 52, Line 78, Line 103, Line 105, Line 116, and Line 158 exceed 100 chars.
Example fix pattern (Line 50)
- <footer className="max-w-7xl mx-auto bg-card/10 border-t rounded-t-lg border-card/10 flex flex-wrap pt-16 pb-8 relative overflow-hidden">
+ <footer
+ className={[
+ 'max-w-7xl mx-auto bg-card/10 border-t rounded-t-lg border-card/10',
+ 'flex flex-wrap pt-16 pb-8 relative overflow-hidden',
+ ].join(' ')}
+ >As per coding guidelines **/*.{js,ts,tsx}: Maintain a maximum line width of 100 characters.
Also applies to: 78-78, 103-105, 116-116, 158-158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ui/neo-minimal-footer.tsx` around lines 50 - 52, The long
className strings in the neo-minimal-footer component exceed the 100-char line
limit; split/wrap these className values into multiple concatenated strings or
template literals so each line stays ≤100 chars. Specifically update the footer
element's className (the long string starting with "max-w-7xl..."), the absolute
background grid div's className (the long bg-[linear-gradient(...)] string), and
the other oversized className props referenced around lines 78, 103-105, 116,
and 158 in this file (search for multiple long className="..." occurrences in
neo-minimal-footer.tsx) so each class segment is on its own line but retains the
same final combined className at runtime.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/actions/grantRole.test.js`:
- Around line 16-17: The test named "no internal rate-limit" currently only
verifies roles.add() ran but doesn't confirm checkRoleRateLimit wasn't invoked;
update the test to import the mocked checkRoleRateLimit from the same module
(roleUtils.js) and add an assertion that checkRoleRateLimit was not called after
calling handleGrantRole(), so the test fails if internal rate-limit checks are
executed; reference handleGrantRole, roles.add, and checkRoleRateLimit when
locating where to add the import and the expect(...).not.toHaveBeenCalled()
assertion.
In `@web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx`:
- Around line 255-258: The current key for each badge element uses value-derived
parts (`badge.days`/`badge.label`) which causes React to remount inputs and lose
focus while typing; update the key in the badge mapping inside
CommunitySettingsSection (the JSX that renders each badge with
key={`badge-${badge.days}-${badge.label}`}) to use a stable index-based key
(e.g., `badge-${index}`) or a composite that includes the index (e.g.,
`badge-${index}-${badge.label}`) so elements are not remounted on input changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01281080-9242-4f95-b50c-0f46fc3ef81c
📒 Files selected for processing (3)
tests/modules/actions/grantRole.test.jsweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsxweb/src/components/landing/bento/BentoChart.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Greptile Review
- GitHub Check: Cursor Bugbot
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
web/src/components/landing/bento/BentoChart.tsxweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsxtests/modules/actions/grantRole.test.js
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use
console.*methods in web dashboard code; use appropriate logging mechanisms for React applications
Files:
web/src/components/landing/bento/BentoChart.tsxweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: ESM only - do not use CommonJS modules
Use src/logger.js; do not use console.*
Files:
web/src/components/landing/bento/BentoChart.tsxweb/src/components/dashboard/config-sections/CommunitySettingsSection.tsxtests/modules/actions/grantRole.test.js
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
tests/modules/actions/grantRole.test.js
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/modules/actions/grantRole.test.js
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL only; do not use string concatenation for SQL queries
Files:
tests/modules/actions/grantRole.test.js
🧠 Learnings (5)
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.{ts,tsx} : Web dashboard config editor should use category workspace navigation with reusable SettingsFeatureCard pattern (header + master toggle + Basic/Advanced blocks)
Applied to files:
web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx
📚 Learning: 2026-03-26T00:04:14.673Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.673Z
Learning: Applies to src/**/*.{js,ts} : Community features should be gated behind config.<feature>.enabled. Moderation commands are the exception.
Applied to files:
web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
tests/modules/actions/grantRole.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Write web dashboard tests using Vitest 4 with the `jsdom` environment and React Testing Library, matching the `web/src/` structure
Applied to files:
tests/modules/actions/grantRole.test.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/reactionrole.js : Enforce invoker role hierarchy check in /reactionrole add command to prevent non-owner users from configuring roles at or above their highest role
Applied to files:
tests/modules/actions/grantRole.test.js
🔇 Additional comments (6)
web/src/components/landing/bento/BentoChart.tsx (3)
71-73: Date parsing update looks correct and consistent.Using template literals for ISO date construction is clean and preserves UTC weekday formatting behavior.
Also applies to: 87-93
215-225: SVG hit-zone interaction handling is acceptable here.The lint suppression is narrowly scoped and the hover hit areas remain clear and functional for chart tooltip behavior.
217-217: Key updates are good and address reconciliation stability.Both key changes improve sibling key specificity in mapped elements and avoid the previously flagged collision risk.
Also applies to: 258-258
web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx (3)
536-553: Sorting still happens on every keystroke.The input value is derived from
levelThresholds.join(', ')andonChangeimmediately parses, filters, and sorts. This causes partial edits to be rewritten mid-entry (e.g., typing100, 5instantly becomes5, 100), making thresholds hard to edit and moving the cursor unexpectedly. Consider storing raw text in local state and normalizing on blur.
466-528: LGTM!The new Level-Up Actions feature card follows the
SettingsFeatureCardpattern correctly, gates the feature behinddraftConfig.xp?.enabled, and properly wires toggles todraftConfig.xp.roleRewards.stackRolesandremoveOnLevelDown. Category/feature visibility checks align with the registeredxp-level-actionsfeature inconfig-categories.ts.
383-383: LGTM!Description update accurately reflects the new XP behavior.
| import { canManageRole, recordRoleChange } from '../../../src/modules/actions/roleUtils.js'; | ||
|
|
There was a problem hiding this comment.
The “no internal rate-limit” test doesn’t validate its claim.
At Line 85, the test name says internal rate limits are not checked, but it only checks that roles.add() ran. If handleGrantRole() starts calling checkRoleRateLimit() (mocked true), this test still passes. Import the mock and assert it was not called.
✅ Suggested patch
-import { canManageRole, recordRoleChange } from '../../../src/modules/actions/roleUtils.js';
+import {
+ canManageRole,
+ checkRoleRateLimit,
+ recordRoleChange,
+} from '../../../src/modules/actions/roleUtils.js';
@@
it('should not check rate limit internally (handled at pipeline level)', async () => {
const ctx = makeContext();
await handleGrantRole({ type: 'grantRole', roleId: 'role-a' }, ctx);
+ expect(checkRoleRateLimit).not.toHaveBeenCalled();
expect(ctx._mocks.rolesAdd).toHaveBeenCalledWith('role-a');
});Also applies to: 85-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/modules/actions/grantRole.test.js` around lines 16 - 17, The test named
"no internal rate-limit" currently only verifies roles.add() ran but doesn't
confirm checkRoleRateLimit wasn't invoked; update the test to import the mocked
checkRoleRateLimit from the same module (roleUtils.js) and add an assertion that
checkRoleRateLimit was not called after calling handleGrantRole(), so the test
fails if internal rate-limit checks are executed; reference handleGrantRole,
roles.add, and checkRoleRateLimit when locating where to add the import and the
expect(...).not.toHaveBeenCalled() assertion.
| <div | ||
| key={`badge-${badge.days}-${badge.label}`} | ||
| className="flex items-center gap-2" | ||
| > |
There was a problem hiding this comment.
Value-derived key causes input focus loss during edits.
Using badge-${badge.days}-${badge.label} as the key means that whenever the user types in the "days" or "label" input, the key changes, React remounts the element, and the input loses focus. Since badges are edited in place and not reordered externally, an index-based key is actually stable for this use case.
🔧 Proposed fix: revert to index-based key or use a composite
- <div
- key={`badge-${badge.days}-${badge.label}`}
- className="flex items-center gap-2"
- >
+ <div key={index} className="flex items-center gap-2">Alternatively, if you want to guard against future reordering, use a composite that includes index:
- key={`badge-${badge.days}-${badge.label}`}
+ key={`badge-${index}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| key={`badge-${badge.days}-${badge.label}`} | |
| className="flex items-center gap-2" | |
| > | |
| <div key={index} className="flex items-center gap-2"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/dashboard/config-sections/CommunitySettingsSection.tsx`
around lines 255 - 258, The current key for each badge element uses
value-derived parts (`badge.days`/`badge.label`) which causes React to remount
inputs and lose focus while typing; update the key in the badge mapping inside
CommunitySettingsSection (the JSX that renders each badge with
key={`badge-${badge.days}-${badge.label}`}) to use a stable index-based key
(e.g., `badge-${index}`) or a composite that includes the index (e.g.,
`badge-${index}-${badge.label}`) so elements are not remounted on input changes.
| const xpToNext = nextThreshold !== null ? nextThreshold - xp : 0; | ||
|
|
||
| // DB queries for rank, messages, voiceHours, daysActive — all best-effort | ||
| let rank = '?'; |
There was a problem hiding this comment.
buildTemplateContext initializes rank to '?' and the catch block leaves it unchanged when DB queries fail. This conflicts with the documented fallback behavior and the new tests that expect rank to be '#0' when the DB is unavailable. Set a deterministic fallback value for rank (and keep it on failure) so rendering is consistent.
| let rank = '?'; | |
| let rank = '#0'; |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| return safeEditReply(interaction, { content: 'Reputation system is not enabled.' }); | ||
| } | ||
|
|
||
| const xpCfg = { ...XP_DEFAULTS, ...cfg.xp }; |
There was a problem hiding this comment.
Rank command uses shallow merge unlike getXpConfig
Low Severity
rank.js merges XP config with { ...XP_DEFAULTS, ...cfg.xp } (shallow), while all other consumers use getXpConfig() from reputation.js which performs a deep merge of nested roleRewards. This inconsistency means rank.js could get a different resolved config shape than the rest of the system if a guild partially overrides roleRewards. Using the shared getXpConfig helper would keep behavior consistent.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
tests/modules/actions/roleUtils.test.js (1)
72-78:⚠️ Potential issue | 🟠 Major
sweepRoleLimits()is not a true test-state reset.
beforeEachsays “clear” state, butsweepRoleLimits()only removes stale entries. Recent entries can leak across tests (especially with reused keys likeguild1:user1), making rate-limit assertions order-dependent.💡 Suggested fix
import { canManageRole, checkRoleRateLimit, + clearRoleLimits, collectXpManagedRoles, enforceRoleLevelDown, recordRoleChange, sweepRoleLimits, } from '../../../src/modules/actions/roleUtils.js'; describe('checkRoleRateLimit / recordRoleChange', () => { beforeEach(() => { vi.useFakeTimers(); vi.clearAllMocks(); - // Clear rate limit state between tests - sweepRoleLimits(); + clearRoleLimits(); });describe('enforceRoleLevelDown', () => { beforeEach(() => { vi.clearAllMocks(); - // Clear rate limit state between tests - sweepRoleLimits(); + clearRoleLimits(); });And in
src/modules/actions/roleUtils.js, export a test helper:export function clearRoleLimits() { roleLimits.clear(); }Also applies to: 170-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/modules/actions/roleUtils.test.js` around lines 72 - 78, The tests leak recent rate-limit state because sweepRoleLimits() only prunes stale entries; add and export a true reset helper in the module (e.g., export function clearRoleLimits() { roleLimits.clear(); }) inside src/modules/actions/roleUtils.js and update the tests (describe('checkRoleRateLimit / recordRoleChange', ...) and the other block at lines ~170-175) to call clearRoleLimits() in beforeEach instead of sweepRoleLimits() so each test gets a clean roleLimits Map.
🤖 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/utils/templateEngine.test.js`:
- Around line 99-269: Tests for buildTemplateContext don't exercise
optional-chaining fallbacks (e.g., guild.iconURL?.(), message?.channel?.name,
member.user.displayAvatarURL?.()), so add a test named "should handle missing
optional properties gracefully" that calls buildTemplateContext with a member
lacking displayAvatarURL and joinedAt, a guild lacking iconURL, and message set
to null, then assert ctx.avatar === '', ctx.serverIcon === '', ctx.channel ===
'', and ctx.joinDate === '' (optionally assert other fields like
roleName/roleMention remain sensible); this targets the buildTemplateContext
behavior and ensures the empty-string fallback paths are covered.
- Around line 100-146: Add a new unit test that covers the primary
member.displayName path: call buildTemplateContext with a member object that
includes displayName (e.g., member.displayName = 'GuildNickname') plus
member.user.displayName different from it, a simple message.channel, and a
minimal guild object, then assert that ctx.username equals 'GuildNickname'; this
verifies the templateEngine logic that prefers member.displayName over
member.user.displayName.
- Around line 1-9: Consolidate the two separate Vitest imports into a single
import to remove duplication: replace the separate vi import and the later
"import { describe, expect, it } from 'vitest'" with one line importing vi,
describe, expect, and it together; keep the vi.mock(...) call (and its mock
factory) in place and ensure vi.mock is executed before any imports of modules
that depend on the mocked db (reference symbols: vi, vi.mock, describe, expect,
it).
---
Duplicate comments:
In `@tests/modules/actions/roleUtils.test.js`:
- Around line 72-78: The tests leak recent rate-limit state because
sweepRoleLimits() only prunes stale entries; add and export a true reset helper
in the module (e.g., export function clearRoleLimits() { roleLimits.clear(); })
inside src/modules/actions/roleUtils.js and update the tests
(describe('checkRoleRateLimit / recordRoleChange', ...) and the other block at
lines ~170-175) to call clearRoleLimits() in beforeEach instead of
sweepRoleLimits() so each test gets a clean roleLimits Map.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca915fae-dd71-4062-98d5-482453e4fcd3
📒 Files selected for processing (2)
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.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: Greptile Review
- GitHub Check: Cursor Bugbot
- GitHub Check: Test
🧰 Additional context used
📓 Path-based instructions (5)
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.test.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.test.js
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.test.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: ESM only - do not use CommonJS modules
Use src/logger.js; do not use console.*
Files:
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.test.js
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL only; do not use string concatenation for SQL queries
Files:
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.test.js
🧠 Learnings (6)
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Write bot tests using Vitest 4 with the `node` environment, matching the `src/` structure in the `tests/` directory
Applied to files:
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.test.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/reactionrole.js : Enforce invoker role hierarchy check in /reactionrole add command to prevent non-owner users from configuring roles at or above their highest role
Applied to files:
tests/modules/actions/roleUtils.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Write web dashboard tests using Vitest 4 with the `jsdom` environment and React Testing Library, matching the `web/src/` structure
Applied to files:
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.test.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
tests/modules/actions/roleUtils.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Applied to files:
tests/modules/actions/roleUtils.test.jstests/utils/templateEngine.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Applied to files:
tests/utils/templateEngine.test.js
🪛 GitHub Check: SonarCloud Code Analysis
tests/utils/templateEngine.test.js
[warning] 9-9: 'vitest' imported multiple times.
[warning] 1-1: 'vitest' imported multiple times.
🔇 Additional comments (3)
tests/modules/actions/roleUtils.test.js (3)
38-70:canManageRolecoverage is solid and focused.Good matrix across permission, hierarchy, equality edge, and missing-role cache with logger assertions.
126-168:collectXpManagedRolestests correctly cover both action sources.Nice inclusion of
defaultActionsalongsidelevelActions, plus empty-config behavior.
282-334: Great regression coverage for level-down edge paths.The fallback-to-individual removal, replace-mode restore, and rate-limit skip scenarios are high-value tests.
Also applies to: 336-362
| import { vi } from 'vitest'; | ||
|
|
||
| vi.mock('../../src/db.js', () => ({ | ||
| getPool: vi.fn(() => ({ | ||
| query: vi.fn().mockResolvedValue({ rows: [] }), | ||
| })), | ||
| })); | ||
|
|
||
| import { describe, expect, it } from 'vitest'; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consolidating the Vitest imports.
The file imports from 'vitest' twice (lines 1 and 9), which triggers a SonarCloud warning. While this pattern is valid for separating mock utilities from test utilities, you can consolidate them into a single import for cleaner code.
♻️ Proposed consolidation
-import { vi } from 'vitest';
+import { describe, expect, it, vi } from 'vitest';
vi.mock('../../src/db.js', () => ({
getPool: vi.fn(() => ({
query: vi.fn().mockResolvedValue({ rows: [] }),
})),
}));
-import { describe, expect, it } from 'vitest';
-
import {
buildTemplateContext,
renderTemplate,
validateLength,
} from '../../src/utils/templateEngine.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { vi } from 'vitest'; | |
| vi.mock('../../src/db.js', () => ({ | |
| getPool: vi.fn(() => ({ | |
| query: vi.fn().mockResolvedValue({ rows: [] }), | |
| })), | |
| })); | |
| import { describe, expect, it } from 'vitest'; | |
| import { describe, expect, it, vi } from 'vitest'; | |
| vi.mock('../../src/db.js', () => ({ | |
| getPool: vi.fn(() => ({ | |
| query: vi.fn().mockResolvedValue({ rows: [] }), | |
| })), | |
| })); | |
| import { | |
| buildTemplateContext, | |
| renderTemplate, | |
| validateLength, | |
| } from '../../src/utils/templateEngine.js'; |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 9-9: 'vitest' imported multiple times.
[warning] 1-1: 'vitest' imported multiple times.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/utils/templateEngine.test.js` around lines 1 - 9, Consolidate the two
separate Vitest imports into a single import to remove duplication: replace the
separate vi import and the later "import { describe, expect, it } from 'vitest'"
with one line importing vi, describe, expect, and it together; keep the
vi.mock(...) call (and its mock factory) in place and ensure vi.mock is executed
before any imports of modules that depend on the mocked db (reference symbols:
vi, vi.mock, describe, expect, it).
| describe('buildTemplateContext', () => { | ||
| it('should populate all Discord-derived variables from member/guild/message', async () => { | ||
| const member = { | ||
| user: { | ||
| id: '123456789', | ||
| displayName: 'TestUser', | ||
| displayAvatarURL: () => 'https://cdn.example.com/avatar.png', | ||
| }, | ||
| joinedAt: new Date('2025-01-15T12:00:00Z'), | ||
| }; | ||
| const message = { | ||
| channel: { name: 'general' }, | ||
| }; | ||
| const guild = { | ||
| id: 'guild1', | ||
| name: 'Test Server', | ||
| iconURL: () => 'https://cdn.example.com/icon.png', | ||
| memberCount: 1234, | ||
| }; | ||
|
|
||
| const ctx = await buildTemplateContext({ | ||
| member, | ||
| message, | ||
| guild, | ||
| level: 5, | ||
| previousLevel: 4, | ||
| xp: 1500, | ||
| levelThresholds: [100, 300, 600, 1000, 1500, 2500], | ||
| roleName: null, | ||
| roleId: null, | ||
| }); | ||
|
|
||
| expect(ctx.username).toBe('TestUser'); | ||
| expect(ctx.mention).toBe('<@123456789>'); | ||
| expect(ctx.avatar).toBe('https://cdn.example.com/avatar.png'); | ||
| expect(ctx.level).toBe('5'); | ||
| expect(ctx.previousLevel).toBe('4'); | ||
| expect(ctx.xp).toBe('1,500'); | ||
| expect(ctx.xpToNext).toBe('1,000'); | ||
| expect(ctx.nextLevel).toBe('6'); | ||
| expect(ctx.server).toBe('Test Server'); | ||
| expect(ctx.serverIcon).toBe('https://cdn.example.com/icon.png'); | ||
| expect(ctx.memberCount).toBe('1,234'); | ||
| expect(ctx.channel).toBe('#general'); | ||
| expect(ctx.joinDate).toMatch(/Jan/); | ||
| expect(ctx.roleName).toBe(''); | ||
| expect(ctx.roleMention).toBe(''); | ||
| }); | ||
|
|
||
| it('should populate roleName and roleMention when roleId is provided', async () => { | ||
| const member = { | ||
| user: { | ||
| id: '123', | ||
| displayName: 'User', | ||
| displayAvatarURL: () => '', | ||
| }, | ||
| joinedAt: new Date(), | ||
| }; | ||
| const message = { channel: { name: 'ch' } }; | ||
| const guild = { | ||
| id: 'guild1', | ||
| name: 'S', | ||
| iconURL: () => '', | ||
| memberCount: 1, | ||
| }; | ||
|
|
||
| const ctx = await buildTemplateContext({ | ||
| member, | ||
| message, | ||
| guild, | ||
| level: 1, | ||
| previousLevel: 0, | ||
| xp: 100, | ||
| levelThresholds: [100], | ||
| roleName: 'Regular', | ||
| roleId: '999888777', | ||
| }); | ||
|
|
||
| expect(ctx.roleName).toBe('Regular'); | ||
| expect(ctx.roleMention).toBe('<@&999888777>'); | ||
| }); | ||
|
|
||
| it('should return "0" for xpToNext when at max level', async () => { | ||
| const member = { | ||
| user: { id: '1', displayName: 'U', displayAvatarURL: () => '' }, | ||
| joinedAt: new Date(), | ||
| }; | ||
| const message = { channel: { name: 'ch' } }; | ||
| const guild = { id: 'guild1', name: 'S', iconURL: () => '', memberCount: 1 }; | ||
|
|
||
| const ctx = await buildTemplateContext({ | ||
| member, | ||
| message, | ||
| guild, | ||
| level: 3, | ||
| previousLevel: 2, | ||
| xp: 700, | ||
| levelThresholds: [100, 300, 600], | ||
| roleName: null, | ||
| roleId: null, | ||
| }); | ||
|
|
||
| expect(ctx.xpToNext).toBe('0'); | ||
| expect(ctx.nextLevel).toBe('0'); | ||
| }); | ||
|
|
||
| it('should populate DB-derived variables when query returns rows', async () => { | ||
| const { getPool } = await import('../../src/db.js'); | ||
| getPool.mockReturnValueOnce({ | ||
| query: vi | ||
| .fn() | ||
| .mockResolvedValueOnce({ rows: [{ rank: 3 }] }) | ||
| .mockResolvedValueOnce({ | ||
| rows: [{ messages_count: 42, days_active: 7, voice_seconds: 3600 }], | ||
| }), | ||
| }); | ||
|
|
||
| const member = { | ||
| user: { id: 'u1', displayName: 'U', displayAvatarURL: () => '' }, | ||
| joinedAt: new Date(), | ||
| }; | ||
| const guild = { id: 'g1', name: 'S', iconURL: () => '', memberCount: 1 }; | ||
|
|
||
| const ctx = await buildTemplateContext({ | ||
| member, | ||
| message: { channel: { name: 'ch' } }, | ||
| guild, | ||
| level: 1, | ||
| previousLevel: 0, | ||
| xp: 100, | ||
| levelThresholds: [100, 300], | ||
| roleName: null, | ||
| roleId: null, | ||
| }); | ||
|
|
||
| expect(ctx.rank).toBe('#3'); | ||
| expect(ctx.messages).toBe('42'); | ||
| expect(ctx.daysActive).toBe('7'); | ||
| expect(ctx.voiceHours).toBe('1'); | ||
| }); | ||
|
|
||
| it('should use fallback values when DB query rejects', async () => { | ||
| const { getPool } = await import('../../src/db.js'); | ||
| getPool.mockReturnValueOnce({ | ||
| query: vi.fn().mockRejectedValue(new Error('DB down')), | ||
| }); | ||
|
|
||
| const member = { | ||
| user: { id: 'u1', displayName: 'U', displayAvatarURL: () => '' }, | ||
| joinedAt: new Date(), | ||
| }; | ||
| const guild = { id: 'g1', name: 'S', iconURL: () => '', memberCount: 1 }; | ||
|
|
||
| const ctx = await buildTemplateContext({ | ||
| member, | ||
| message: { channel: { name: 'ch' } }, | ||
| guild, | ||
| level: 1, | ||
| previousLevel: 0, | ||
| xp: 100, | ||
| levelThresholds: [100, 300], | ||
| roleName: null, | ||
| roleId: null, | ||
| }); | ||
|
|
||
| expect(ctx.rank).toBe('?'); | ||
| expect(ctx.messages).toBe('0'); | ||
| expect(ctx.daysActive).toBe('0'); | ||
| expect(ctx.voiceHours).toBe('0'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding tests for optional property fallbacks.
The implementation uses optional chaining for properties like guild.iconURL?.(), message?.channel?.name, and member.user.displayAvatarURL?.(). All current test fixtures provide these properties, so the fallback paths ('' empty string) are not exercised.
🧪 Proposed edge-case test
it('should handle missing optional properties gracefully', async () => {
const member = {
user: { id: '123', displayName: 'User' },
// No displayAvatarURL method, no joinedAt
};
const guild = {
id: 'g1',
name: 'Server',
// No iconURL method
memberCount: 10,
};
// message is null
const ctx = await buildTemplateContext({
member,
message: null,
guild,
level: 1,
previousLevel: 0,
xp: 100,
levelThresholds: [100],
roleName: null,
roleId: null,
});
expect(ctx.avatar).toBe('');
expect(ctx.serverIcon).toBe('');
expect(ctx.channel).toBe('');
expect(ctx.joinDate).toBe('');
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/utils/templateEngine.test.js` around lines 99 - 269, Tests for
buildTemplateContext don't exercise optional-chaining fallbacks (e.g.,
guild.iconURL?.(), message?.channel?.name, member.user.displayAvatarURL?.()), so
add a test named "should handle missing optional properties gracefully" that
calls buildTemplateContext with a member lacking displayAvatarURL and joinedAt,
a guild lacking iconURL, and message set to null, then assert ctx.avatar === '',
ctx.serverIcon === '', ctx.channel === '', and ctx.joinDate === '' (optionally
assert other fields like roleName/roleMention remain sensible); this targets the
buildTemplateContext behavior and ensures the empty-string fallback paths are
covered.
| it('should populate all Discord-derived variables from member/guild/message', async () => { | ||
| const member = { | ||
| user: { | ||
| id: '123456789', | ||
| displayName: 'TestUser', | ||
| displayAvatarURL: () => 'https://cdn.example.com/avatar.png', | ||
| }, | ||
| joinedAt: new Date('2025-01-15T12:00:00Z'), | ||
| }; | ||
| const message = { | ||
| channel: { name: 'general' }, | ||
| }; | ||
| const guild = { | ||
| id: 'guild1', | ||
| name: 'Test Server', | ||
| iconURL: () => 'https://cdn.example.com/icon.png', | ||
| memberCount: 1234, | ||
| }; | ||
|
|
||
| const ctx = await buildTemplateContext({ | ||
| member, | ||
| message, | ||
| guild, | ||
| level: 5, | ||
| previousLevel: 4, | ||
| xp: 1500, | ||
| levelThresholds: [100, 300, 600, 1000, 1500, 2500], | ||
| roleName: null, | ||
| roleId: null, | ||
| }); | ||
|
|
||
| expect(ctx.username).toBe('TestUser'); | ||
| expect(ctx.mention).toBe('<@123456789>'); | ||
| expect(ctx.avatar).toBe('https://cdn.example.com/avatar.png'); | ||
| expect(ctx.level).toBe('5'); | ||
| expect(ctx.previousLevel).toBe('4'); | ||
| expect(ctx.xp).toBe('1,500'); | ||
| expect(ctx.xpToNext).toBe('1,000'); | ||
| expect(ctx.nextLevel).toBe('6'); | ||
| expect(ctx.server).toBe('Test Server'); | ||
| expect(ctx.serverIcon).toBe('https://cdn.example.com/icon.png'); | ||
| expect(ctx.memberCount).toBe('1,234'); | ||
| expect(ctx.channel).toBe('#general'); | ||
| expect(ctx.joinDate).toMatch(/Jan/); | ||
| expect(ctx.roleName).toBe(''); | ||
| expect(ctx.roleMention).toBe(''); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add test coverage for the primary member.displayName path.
The test fixture provides user.displayName but not member.displayName. The implementation in src/utils/templateEngine.js line 139 prioritizes member.displayName (the guild-specific nickname) and only falls back to member.user.displayName. This test currently exercises only the fallback path.
🧪 Proposed test addition
Add a test case that verifies guild nicknames take precedence:
it('should use member.displayName (guild nickname) over user.displayName', async () => {
const member = {
displayName: 'GuildNickname',
user: {
id: '123',
displayName: 'UserDisplayName',
displayAvatarURL: () => '',
},
joinedAt: new Date(),
};
const guild = { id: 'g1', name: 'S', iconURL: () => '', memberCount: 1 };
const ctx = await buildTemplateContext({
member,
message: { channel: { name: 'ch' } },
guild,
level: 1,
previousLevel: 0,
xp: 100,
levelThresholds: [100],
roleName: null,
roleId: null,
});
expect(ctx.username).toBe('GuildNickname');
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/utils/templateEngine.test.js` around lines 100 - 146, Add a new unit
test that covers the primary member.displayName path: call buildTemplateContext
with a member object that includes displayName (e.g., member.displayName =
'GuildNickname') plus member.user.displayName different from it, a simple
message.channel, and a minimal guild object, then assert that ctx.username
equals 'GuildNickname'; this verifies the templateEngine logic that prefers
member.displayName over member.user.displayName.
Summary
reputation.jswith a configurable action pipeline underconfig.xpgrantRoleandremoveRoleaction handlers with permission checks, rate limiting, and stack/replace modereputation.js,rank.js,community.js, andmembers.jsCloses #365, closes #366, closes #367
What Changed
New files:
src/modules/xpDefaults.jsconfig.xpvaluessrc/utils/templateEngine.jsrenderTemplate(),buildTemplateContext(),validateLength()src/modules/levelUpActions.jssrc/modules/actions/roleUtils.jsenforceRoleLevelDownsrc/modules/actions/grantRole.jssrc/modules/actions/removeRole.jsModified files:
config.json— newxpsection; trimmedreputationsectionsrc/modules/reputation.js— removed ~90 lines of inline role/announcement logic, replaced withexecuteLevelUpPipeline()callsrc/modules/reputationDefaults.js— removedroleRewards,announceChannelId,levelThresholdssrc/api/utils/configAllowlist.js— added'xp'toSAFE_CONFIG_KEYSsrc/commands/rank.js— readslevelThresholdsfromconfig.xpsrc/api/routes/community.js— readslevelThresholdsfromconfig.xpsrc/api/routes/members.js— readslevelThresholdsfromconfig.xp; callsenforceRoleLevelDownon XP reductionTest coverage: 69 new tests across 5 test files. All 4123 bot tests passing.
Design Decisions
Map<string, handler>makes Phase 2 action types (sendDm, announce, xpBonus, etc.) a one-line registration eachdefaultActionsvslevelActions— per-level entries fully override defaults (not additive)buildTemplateContext(), rendering is syncTest plan
enforceRoleLevelDownremoves roles above new levelreputation.jscalls pipeline on level-up, skips when disabled