refactor: Phase 1 — shared helpers rollout (respond_error + typed config accessors)#122
Conversation
…essors Add five pure helpers to easycord/plugins/_shared.py: - respond_error(ctx, message): collapses the 107-site ephemeral error pattern - get_id / set_id: str↔int coercion over ServerConfig.get_other/set_other; None deletes - get_ids / set_ids: list variant with per-item coercion; missing key returns [] All are pure over ServerConfig (no I/O, no lock) so they slot inside existing mutate/guild_lock spans without changing concurrency semantics. Tests: 29 new cases in test_shared_helpers.py covering coercion, defaults, None-delete, non-coercible skipping, and respond ephemeral flag.
…tion Replace bare ctx.respond(msg, ephemeral=True) error/guard calls with respond_error() from _shared. Success embeds and confirmations stay explicit.
economy, starboard, polls, birthday, verification, word_filter, suggestions, tickets, server_stats, levels, giveaway, scheduled_announcements — error/guard paths now use respond_error() from _shared. Success confirms and embeds stay explicit.
Replace error/guard ephemeral responds. Success and info responses (status display, history list, cancellation confirm) stay explicit.
…nd log_channel_id
Replace bare cfg.get_other("support_role_id"/"log_channel_id") and
cfg.set_other(...) calls with get_id/set_id from _shared. Adds str→int
coercion and None-removal semantics that raw get_other/set_other lack.
tags: not-found, unauthorized, and empty-list paths. welcome: five guild-guard paths. Success responses stay explicit.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
🧙 Sourcery has finished reviewing your pull request! Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds shared ephemeral error and ServerConfig ID helpers, migrates plugin error paths to use them, updates ticket configuration access, and adds comprehensive helper tests. ChangesShared helper adoption
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider introducing a lightweight
Protocolor base interface for thectxobject used byrespond_errorso you can drop thetype: ignore[attr-defined]and make the helper’s contract clearer to type checkers and callers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider introducing a lightweight `Protocol` or base interface for the `ctx` object used by `respond_error` so you can drop the `type: ignore[attr-defined]` and make the helper’s contract clearer to type checkers and callers.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@easycord/plugins/_shared.py`:
- Around line 38-46: Update get_id() and get_ids() to validate raw configuration
values before coercion: accept only decimal string representations of positive
Discord snowflakes, rejecting bools, floats, zero, negative values, and
malformed strings. Preserve returning None or excluding invalid entries
according to each function’s existing contract, and avoid relying on int() alone
for validation.
In `@easycord/plugins/auto_role.py`:
- Line 12: Re-export respond_error and every other consumed shared helper from
easycord’s public API, then update imports in easycord/plugins/auto_role.py
(12-12), easycord/plugins/juicewrld.py (105-105), easycord/plugins/levels.py
(14-14), easycord/plugins/server_stats.py (12-12),
easycord/plugins/suggestions.py (12-12), easycord/plugins/tags.py (9-9),
easycord/plugins/verification.py (13-13), easycord/plugins/welcome.py (16-16),
and easycord/plugins/word_filter.py (12-12) to import through easycord rather
than ._shared; tags.py must switch all consumed shared helpers.
- Line 85: Localize every listed error response by resolving each message
through the relevant command context’s ctx.t(...) before passing it to
respond_error. Apply this to easycord/plugins/auto_role.py lines 85-85, 101-101,
116-123, and 135-138; easycord/plugins/juicewrld.py lines 436-444, 483, 539,
567, 622, 632, 655, 693, 713, 760, and 792; easycord/plugins/levels.py lines
362-365; easycord/plugins/server_stats.py lines 136, 166, and 188;
easycord/plugins/suggestions.py lines 82-87, 122, 160, 173, 184, and 197;
easycord/plugins/verification.py lines 246-249, 270-273, and 336-339;
easycord/plugins/welcome.py lines 146, 155, 164, 181, and 198; and
easycord/plugins/word_filter.py lines 100, 116, 129-132, 134-145, 148-153, and
166, preserving each existing message and response flow.
In `@easycord/plugins/birthday.py`:
- Line 13: Re-export respond_error from easycord/__init__.py, then replace the
private ._shared import with the public easycord import in
easycord/plugins/birthday.py:13, economy.py:13, giveaway.py:16, openclaw.py:15,
polls.py:14, reminder.py:14, reputation.py:13, scheduled_announcements.py:14,
and starboard.py:12; all consumers must use the stable public API.
- Line 324: Replace every hardcoded user-facing error response with the
appropriate localization lookup through LocalizationManager, preserving each
existing validation and error path. Update birthday.py lines 324 and 396;
economy.py lines 323-327; giveaway.py lines 286-289, 304, and 391; openclaw.py
lines 119-120, 159-160, 172-173, 195-196, 267-268, and 279-280; reminder.py
lines 206-207, 213-214, 268-269, and 310-311; reputation.py lines 104-105,
108-109, 120-121, 171-172, and 205-206; scheduled_announcements.py lines 211-212
and 268-269; and starboard.py lines 279-280. Add or reuse localization keys for
the birthday, transfer, giveaway, OpenClaw, reminder, reputation,
scheduled-announcement, and threshold messages without introducing hardcoded
response strings.
- Around line 324-325: Move the respond_error calls outside the per-guild
configuration lock: in easycord/plugins/birthday.py lines 324-325,
easycord/plugins/reminder.py lines 310-311, and easycord/plugins/reputation.py
lines 120-121, record the missing-birthday, missing-reminder, or cooldown result
during the _guild_lock load/check flow, then perform the response after the lock
exits and before continuing or returning.
In `@easycord/plugins/giveaway.py`:
- Around line 303-305: Move the SENDABLE_CHANNEL_TYPES validation in the
giveaway command before the initial ctx.respond(embed=embed) call. Return
through respond_error immediately for non-sendable channels, and only create or
persist the giveaway and send the initial response after the channel check
passes.
In `@easycord/plugins/server_stats.py`:
- Line 166: Update the error handling around respond_error in the stat-channel
creation flow to log exc server-side instead of interpolating it into the
user-facing response. Replace the raw exception text with the existing generic
localized failure message, preserving the exception details only in the server
log.
In `@easycord/plugins/tickets.py`:
- Line 13: Update the tickets plugin import to obtain get_id, respond_error, and
set_id from the stable easycord public API instead of the underscore-prefixed
_shared module. Export any of these helpers not already exposed through
easycord/__init__.py, preserving their existing behavior.
- Line 345: Localize every migrated ticket error response through ctx.t(...)
before passing it to respond_error: update easycord/plugins/tickets.py lines
345-345, 408-409, 451-452, and 496-496, covering ticket creation, closed-ticket,
and add-user failure messages while preserving existing error details and
ephemerality.
- Around line 408-409: In both invalid-ticket branches at
easycord/plugins/tickets.py lines 408-409 and 451-452, record the not-open state
while inside the _guild_lock context, exit the async with block, then call
respond_error so Discord I/O occurs after releasing the lock; apply the same
deferred-response flow in both sites.
🪄 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 Plus
Run ID: 7320e4d1-734c-4b8d-99b3-4b477cc14f83
📒 Files selected for processing (21)
easycord/plugins/_shared.pyeasycord/plugins/auto_role.pyeasycord/plugins/birthday.pyeasycord/plugins/economy.pyeasycord/plugins/giveaway.pyeasycord/plugins/juicewrld.pyeasycord/plugins/levels.pyeasycord/plugins/openclaw.pyeasycord/plugins/polls.pyeasycord/plugins/reminder.pyeasycord/plugins/reputation.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/server_stats.pyeasycord/plugins/starboard.pyeasycord/plugins/suggestions.pyeasycord/plugins/tags.pyeasycord/plugins/tickets.pyeasycord/plugins/verification.pyeasycord/plugins/welcome.pyeasycord/plugins/word_filter.pytests/test_shared_helpers.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Analyze (Python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
For any question about the codebase, query
graphifyagainstgraphify-out/graph.jsonbefore opening source, documentation, or context files; do not read the deprecated Wiki or rebuild the graph unless explicitly asked.
Files:
easycord/plugins/economy.pyeasycord/plugins/reminder.pyeasycord/plugins/polls.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/birthday.pyeasycord/plugins/openclaw.pyeasycord/plugins/verification.pyeasycord/plugins/starboard.pyeasycord/plugins/server_stats.pytests/test_shared_helpers.pyeasycord/plugins/reputation.pyeasycord/plugins/auto_role.pyeasycord/plugins/tags.pyeasycord/plugins/suggestions.pyeasycord/plugins/levels.pyeasycord/plugins/giveaway.pyeasycord/plugins/_shared.pyeasycord/plugins/welcome.pyeasycord/plugins/word_filter.pyeasycord/plugins/tickets.pyeasycord/plugins/juicewrld.py
easycord/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
easycord/**/*.py: Treateasycord/__init__.pyas the stable public API. Import consumers fromeasycord, never from_-prefixed internal modules or helpers.
Usectx.userorctx.member, notctx.author; accessctx.is_adminas a property and never call it asctx.is_admin().
Store per-guild plugin state in the database layer using per-guild namespaced records, never on thePlugininstance itself.
Never hardcode plugin response strings; retrieve localized text throughctx.t(...).
Always await asynchronousToolLimitermethods, includingcheck_limit,reset_user, andreset_tool.
Require an explicitToolSafetyannotation when registering an@ai_tool; keepRESTRICTEDtools unexposed.
Preserve Discord command-registration constraints before synchronization: names must be at most 32 characters and match[-_a-z0-9], descriptions at most 100 characters, and commands/options at most 25 choices or options as applicable.
UseServerConfigStore.mutate(guild_id, fn)for every per-guild load-modify-save operation. The callback must be synchronous and local; perform Discord or network I/O outside the lock.
Never perform an unguarded per-guild configuration read-modify-write. Equivalent plugin-specific per-guild locks are acceptable when held across the entire operation, but Discord or network I/O must not occur while the configuration lock is held.
For event-path plugins such as@on("message"), route every destructive action through one governed method that owns rate limiting, channel narrowing, and Discord error handling; Discord exceptions must not escape into the dispatcher.
Before calling.send()on a channel obtained from context or Discord, narrow its type withSENDABLE_CHANNEL_TYPESusingisinstance; do not call.send()on an unnarrowed channel.
InitializeLevelsPlugin._cooldownssentinels tofloat("-inf"), not0.0, so the first message always passes.
sync_commands()must raise on removals unlessconfirm_removals=Trueis exp...
Files:
easycord/plugins/economy.pyeasycord/plugins/reminder.pyeasycord/plugins/polls.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/birthday.pyeasycord/plugins/openclaw.pyeasycord/plugins/verification.pyeasycord/plugins/starboard.pyeasycord/plugins/server_stats.pyeasycord/plugins/reputation.pyeasycord/plugins/auto_role.pyeasycord/plugins/tags.pyeasycord/plugins/suggestions.pyeasycord/plugins/levels.pyeasycord/plugins/giveaway.pyeasycord/plugins/_shared.pyeasycord/plugins/welcome.pyeasycord/plugins/word_filter.pyeasycord/plugins/tickets.pyeasycord/plugins/juicewrld.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run critical-error Ruff checks with
ruff check easycord tests --select E9,F63,F7,F82and verify plugin test-count thresholds withpython scripts/verify_plugin_tests.py.
Files:
easycord/plugins/economy.pyeasycord/plugins/reminder.pyeasycord/plugins/polls.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/birthday.pyeasycord/plugins/openclaw.pyeasycord/plugins/verification.pyeasycord/plugins/starboard.pyeasycord/plugins/server_stats.pytests/test_shared_helpers.pyeasycord/plugins/reputation.pyeasycord/plugins/auto_role.pyeasycord/plugins/tags.pyeasycord/plugins/suggestions.pyeasycord/plugins/levels.pyeasycord/plugins/giveaway.pyeasycord/plugins/_shared.pyeasycord/plugins/welcome.pyeasycord/plugins/word_filter.pyeasycord/plugins/tickets.pyeasycord/plugins/juicewrld.py
easycord/plugins/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Plugin responses must obtain strings through
LocalizationManager; do not hardcode user-facing response strings.
Files:
easycord/plugins/economy.pyeasycord/plugins/reminder.pyeasycord/plugins/polls.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/birthday.pyeasycord/plugins/openclaw.pyeasycord/plugins/verification.pyeasycord/plugins/starboard.pyeasycord/plugins/server_stats.pyeasycord/plugins/reputation.pyeasycord/plugins/auto_role.pyeasycord/plugins/tags.pyeasycord/plugins/suggestions.pyeasycord/plugins/levels.pyeasycord/plugins/giveaway.pyeasycord/plugins/_shared.pyeasycord/plugins/welcome.pyeasycord/plugins/word_filter.pyeasycord/plugins/tickets.pyeasycord/plugins/juicewrld.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Useeasycord.testinghelpers such asinvoke,FakeContextBuilder, andPluginTestSuitefor command and plugin tests instead of requiring a live Discord connection.
When constructing plugins manually in tests, useMyPlugin.__new__, assignplugin._bot = bot, and then callPlugin.__init__(plugin); do not assignplugin.bot.Run tests with pytest; async tests use
pytest-asynciowithasyncio_mode = "auto"and therefore require no manual event-loop setup.
Files:
tests/test_shared_helpers.py
**/test*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain at least 20 tests per plugin, as verified by
scripts/verify_plugin_tests.py.
Files:
tests/test_shared_helpers.py
🔇 Additional comments (7)
easycord/plugins/_shared.py (2)
7-27: LGTM!
49-54: LGTM!Also applies to: 71-73
easycord/plugins/tickets.py (1)
116-116: LGTM!Also applies to: 147-148, 302-303, 330-330, 484-484
tests/test_shared_helpers.py (1)
1-214: 📐 Maintainability & Code QualityNo changes needed.
easycord/plugins/levels.py (1)
328-328: LGTM!Also applies to: 337-337, 399-399, 412-412, 422-422, 435-435
easycord/plugins/tags.py (1)
76-81: LGTM!Also applies to: 89-104
easycord/plugins/polls.py (1)
297-306: LGTM!
3 participants
Summary
easycord/plugins/_shared.py:respond_error(ctx, msg),get_id,set_id,get_ids,set_ids— pure overServerConfig, no I/O or lock changesrespond_erroradopted across 18 plugins (~100 sites): collapses the ubiquitousawait ctx.respond(msg, ephemeral=True)error/guard pattern. Success responses and result embeds stay explicittickets.py:get_id/set_idreplace bareget_other/set_otherforsupport_role_idandlog_channel_id, adding str→int coercion and None-removal semanticstest_shared_helpers.py: coercion, defaults, None-delete, round-trips, ephemeral flagAll 6 commits are behavior-preserving — no concurrency semantics changed, no public API breaks.
Test plan
ruff check --select E9,F63,F7,F82cleanpython scripts/verify_plugin_tests.py— all plugin thresholds metWhat was deferred
Per plan:
GuildLockManagerdedup (Phase 2),send_safeadoption (Phase 3),require_admin+EmbedBuilderconvergence (Phase 4) each ship as their own PR.Summary by Sourcery
Introduce shared helpers for ephemeral error responses and typed config ID accessors, and adopt them across plugins while adding coverage tests.
New Features:
Enhancements:
Tests: