Master#39
Merged
Merged
Conversation
…ands
- `@slash(require_admin=True)` shorthand gates commands to guild admins
- `@command_error("cmd")` registers per-command error handlers on Plugin classes
- `describe(**kw)` attaches per-parameter descriptions shown in Discord's command picker
- All three wired through _scan_methods, _build_slash_callback, and _register_slash
- 9 new tests; 420 pass
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Added a centralized InteractionRegistry - Unified error pipeline for components, modals, autocomplete, and tasks - Added command sync planning and dynamic component routing - Added reusable option validators
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @rolling-codes, your pull request is larger than the review limit of 150000 diff characters
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis pull request bumps EasyCord to v5.2.0 with major feature additions: a centralized interaction registry for command/component/modal/autocomplete tracking, enhanced slash-command guards (require_admin, cooldown with bucket modes, premium_required), a unified error pipeline with plugin-scoped on_error handlers, dynamic component routing with TTL support, config-driven bot startup, testing utilities (FakeContext, invoke helpers), and comprehensive documentation. All changes include extensive test coverage validating new guard behavior, error routing precedence, registry collision detection, and config/sync workflows. ChangesCore Framework Architecture & v5.2.0 Release
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
✨ 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 |
Review Summary by Qodov5.2.0 Release: Command Guards, Interaction Registry, and Testing Infrastructure
WalkthroughsDescription• **Major version release (v5.2.0)** with comprehensive command guards, interaction registry, and testing infrastructure • **New decorator suite**: @describe, @command_error, @cooldown, @require_permissions, @premium_required, @install_type, @autocomplete for advanced command metadata and guards • **Structured interaction registry** (InteractionRegistry) with pattern matching for dynamic component routing, TTL expiration, and collision detection • **Command sync planning**: plan_command_sync() and sync_commands() methods for previewing and executing syncs with dry-run and confirmation support • **Configuration management**: New BotConfig class for environment and file-based bot configuration with validation • **Testing framework**: FakeContext, FakeInteraction, and invoke() helpers for unit testing commands without Discord connection • **Validators module**: Reusable option validators (Duration, URL, Snowflake, Range, Regex, ChoiceSet) with user-safe error messaging • **Enhanced error pipeline**: Unified error routing through per-command, plugin-scoped, and global handlers via @command_error and plugin.on_error() • **Plugin lifecycle improvements**: Task status tracking, restart/backoff support, and plugin-scoped error handling • **Context enhancements**: New app_context, entitlements properties, send() alias, forward() method, and silent/suppress_embeds parameters • **Comprehensive test coverage**: 6 new test modules covering decorators, registry, sync planning, config, testing utilities, and release readiness • **Documentation expansion**: New guides for interactions, dynamic routing, command sync, and updated getting-started with v5.2 features Diagramflowchart LR
A["Decorators<br/>@cooldown @require_permissions<br/>@describe @command_error"] -->|metadata| B["InteractionRegistry<br/>Structured inventory<br/>Pattern matching"]
C["BotConfig<br/>from_env/from_file"] -->|configure| D["Bot<br/>sync_commands<br/>plan_command_sync"]
D -->|inspect| B
E["@slash commands"] -->|register| B
F["Testing<br/>FakeContext<br/>invoke"] -->|test| E
G["Validators<br/>Duration URL Snowflake"] -->|validate| E
H["Error Pipeline<br/>@command_error<br/>plugin.on_error"] -->|route| D
I["Component Routing<br/>Dynamic patterns<br/>TTL expiration"] -->|register| B
File Changes1. tests/test_new_decorators.py
|
Code Review by Qodo
1. New bot behavior in bot.py
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+125
to
+214
| def inspect_interactions(self) -> dict[str, list[dict[str, Any]]]: | ||
| """Return registered interactions grouped by EasyCord interaction type.""" | ||
| return self.registry.grouped() | ||
|
|
||
| def plan_command_sync( | ||
| self, | ||
| *, | ||
| guild_id: int | None = None, | ||
| remote_commands: list[str] | None = None, | ||
| ) -> dict[str, list[str]]: | ||
| """Build a command sync diff without contacting Discord by default. | ||
|
|
||
| Pass ``remote_commands`` in tests or after your own fetch to compare the | ||
| current EasyCord inventory with Discord's current command names. | ||
| """ | ||
| local_entries = self.registry.iter_syncable(guild_id=guild_id) | ||
| local_names = [entry.name for entry in local_entries if entry.enabled] | ||
| remote_names = list(remote_commands or []) | ||
|
|
||
| warnings: list[str] = [] | ||
| duplicates = sorted({name for name in local_names if local_names.count(name) > 1}) | ||
| for name in duplicates: | ||
| warnings.append(f"Duplicate local command name: {name}") | ||
|
|
||
| local_set = set(local_names) | ||
| remote_set = set(remote_names) | ||
| return { | ||
| "added": sorted(local_set - remote_set), | ||
| "changed": [], | ||
| "removed": sorted(remote_set - local_set), | ||
| "unchanged": sorted(local_set & remote_set), | ||
| "warnings": warnings, | ||
| } | ||
|
|
||
| async def sync_commands( | ||
| self, | ||
| *, | ||
| guild_id: int | None = None, | ||
| dry_run: bool = False, | ||
| remote_commands: list[str] | None = None, | ||
| confirm_removals: bool = False, | ||
| ) -> dict[str, list[str]]: | ||
| """Plan or execute command sync through ``discord.py``'s CommandTree. | ||
|
|
||
| If the plan detects removals, pass ``confirm_removals=True`` before a | ||
| non-dry-run sync. This keeps EasyCord from applying a destructive sync | ||
| without an explicit caller decision. | ||
| """ | ||
| plan = self.plan_command_sync( | ||
| guild_id=guild_id, | ||
| remote_commands=remote_commands, | ||
| ) | ||
| if dry_run: | ||
| return plan | ||
| if plan["removed"] and not confirm_removals: | ||
| raise RuntimeError( | ||
| "Command sync would remove remote commands. Re-run with " | ||
| "confirm_removals=True after reviewing plan_command_sync()." | ||
| ) | ||
| guild = discord.Object(id=guild_id) if guild_id is not None else None | ||
| if guild is not None: | ||
| self.tree.copy_global_to(guild=guild) | ||
| await self.tree.sync(guild=guild) | ||
| else: | ||
| await self.tree.sync() | ||
| for entry in self.registry.iter_syncable(guild_id=guild_id): | ||
| entry.sync_state = "synced" | ||
| return plan | ||
|
|
||
| def enable_interaction_inspector(self, *, owner_ids: set[int] | None = None) -> None: | ||
| """Register the optional ``/easycord interactions`` developer command.""" | ||
| group = app_commands.Group( | ||
| name="easycord", | ||
| description="EasyCord developer diagnostics", | ||
| ) | ||
|
|
||
| @group.command(name="interactions", description="Show registered EasyCord interactions") | ||
| async def interactions(interaction: discord.Interaction) -> None: | ||
| if owner_ids is not None and interaction.user.id not in owner_ids: | ||
| await interaction.response.send_message("Not authorized.", ephemeral=True) | ||
| return | ||
| grouped = self.inspect_interactions() | ||
| lines = [ | ||
| f"{kind}: {len(entries)}" | ||
| for kind, entries in grouped.items() | ||
| ] | ||
| await interaction.response.send_message("\n".join(lines), ephemeral=True) | ||
|
|
||
| self.tree.add_command(group) | ||
|
|
There was a problem hiding this comment.
1. New bot behavior in bot.py 📘 Rule violation ⌂ Architecture
easycord/bot.py adds new bot behavior methods (inspect_interactions, plan_command_sync, sync_commands, enable_interaction_inspector) directly in bot.py instead of a _bot_*.py mixin. This violates the project requirement to keep bot.py limited to wiring/composition and can make bot behavior harder to modularize and maintain.
Agent Prompt
## Issue description
New bot behavior was added directly to `easycord/bot.py`, but project policy requires bot behavior to live in `_bot_*.py` mixin modules, keeping `bot.py` as thin wiring/composition.
## Issue Context
The PR adds interaction inspection and command sync planning/execution methods directly on the `Bot` class in `bot.py`.
## Fix Focus Areas
- easycord/bot.py[125-214]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| bot._register_component_handler = MagicMock() | ||
| bot._register_modal_handler = MagicMock() | ||
|
|
||
| from easycord._bot_plugins import _PluginsMixin |
There was a problem hiding this comment.
2. Tests import easycord.bot* 📘 Rule violation ⌂ Architecture
tests/test_new_decorators.py imports internal easycord._bot_* modules from outside the easycord/ package. This violates the rule that external code (including tests outside the package) must only use EasyCord’s public API surface.
Agent Prompt
## Issue description
Tests located outside the `easycord/` package import internal underscored modules (e.g., `easycord._bot_commands`, `easycord._bot_plugins`). External code must only use the public API from `easycord`.
## Issue Context
These tests currently reach into internal mixins to test scanning/building behavior.
## Fix Focus Areas
- tests/test_new_decorators.py[97-97]
- tests/test_new_decorators.py[127-127]
- tests/test_new_decorators.py[183-183]
- tests/test_new_decorators.py[534-534]
- tests/test_new_decorators.py[553-553]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+87
to
+88
| if getattr(method, "_is_command_error", False): | ||
| self._command_error_handlers[method._command_error_for] = method |
There was a problem hiding this comment.
3. Leaking command error handlers 🐞 Bug ☼ Reliability
_scan_methods() registers per-command error handlers into _command_error_handlers, but remove_plugin() never unregisters them, leaving stale bound methods after plugin unload. If another plugin later registers a command with the same name, failures can be routed to the stale handler (wrong behavior or crash).
Agent Prompt
### Issue description
`_command_error_handlers` is populated during plugin scanning, but plugin removal does not clean up entries. This can leave stale handlers that will be invoked for future commands with the same name.
### Issue Context
- Registration happens in `_scan_methods()`.
- Lookup happens in `_build_slash_callback()` when a command raises.
- `remove_plugin()` should remove any handlers owned by that plugin.
### Fix Focus Areas
- easycord/_bot_plugins.py[23-274]
- easycord/_bot_commands.py[274-296]
### Suggested fix
- In `remove_plugin()`, delete any `_command_error_handlers` entries whose value is a bound method on `plugin` (e.g., `getattr(handler, "__self__", None) is plugin`).
- Optionally, store handlers in a structure keyed by plugin name to make cleanup deterministic.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+189
to
+195
| key = f"{self._scope_key(guild_id)}:{command_name}:{option_name}" | ||
| return self._register_named( | ||
| self.autocomplete_callbacks, | ||
| "autocomplete", | ||
| key, | ||
| func, | ||
| source_plugin=source_plugin, |
There was a problem hiding this comment.
4. Autocomplete registry key bug 🐞 Bug ≡ Correctness
Autocomplete registry entries are keyed using an already-scoped name in register_autocomplete(), and _register_named() prepends the scope again; additionally, slash registration passes only the unqualified command name (not the group-qualified name). This combination can raise ValueError collisions for common setups like two SlashGroups both having a subcommand named "kick" with autocomplete.
Agent Prompt
### Issue description
Autocomplete registrations can collide because:
1) `InteractionRegistry.register_autocomplete()` passes a *scoped* string into `_register_named()`, which scopes again.
2) `_register_slash()` registers autocomplete using the unqualified command name (`name`) rather than the group-qualified `registry_name`.
### Issue Context
This causes collisions for SlashGroup subcommands that share a name across groups (a normal pattern), and results in confusing stored `InteractionEntry.name` values.
### Fix Focus Areas
- easycord/registry.py[179-231]
- easycord/_bot_commands.py[339-419]
### Suggested fix
- Change `InteractionRegistry.register_autocomplete()` to pass an *unscoped* name into `_register_named()`, e.g. `name = f"{command_name}:{option_name}"` and let `_register_named()` add the scope.
- In `_register_slash()`, use the computed `registry_name` (includes parent group when present) when calling `registry.register_autocomplete(...)` and in the stored metadata, so grouped commands are uniquely identified.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+281
to
+292
| if per_cmd is not None: | ||
| await per_cmd(ctx, exc) | ||
| return | ||
| plugin = getattr(func, "__self__", None) | ||
| if plugin is not None: | ||
| from .plugin import Plugin as _Plugin | ||
| if isinstance(plugin, _Plugin): | ||
| plugin_on_error = type(plugin).on_error | ||
| base_on_error = _Plugin.on_error | ||
| if plugin_on_error is not base_on_error: | ||
| await plugin.on_error(ctx, exc) | ||
| return |
There was a problem hiding this comment.
5. Handler exceptions escape 🐞 Bug ☼ Reliability
If a per-command error handler or Plugin.on_error raises, that exception is not caught inside _build_slash_callback() and can bubble out of the interaction callback. Because build_chain() does not catch exceptions, this can fail the interaction without invoking global on_error or logging via the framework dispatcher.
Agent Prompt
### Issue description
Exceptions thrown *inside* per-command error handlers or `Plugin.on_error()` are not handled, so they can escape the interaction callback.
### Issue Context
- `build_chain()` does not catch exceptions; it only wraps middleware.
- The framework already has `_dispatch_framework_error()` which safely wraps plugin/global handlers.
### Fix Focus Areas
- easycord/_bot_commands.py[274-296]
- easycord/middleware.py[28-37]
- easycord/_bot_events.py[100-134]
### Suggested fix
- Wrap `await per_cmd(ctx, exc)` and `await plugin.on_error(ctx, exc)` in `try/except Exception as handler_exc`.
- On handler failure, log with `logger.exception(...)` and then fall through to the next handler (plugin → global) or call `_dispatch_framework_error(handler_exc, ctx=ctx, plugin_instance=plugin)` (careful to avoid recursion).
- Similarly consider guarding `self._error_handler(ctx, exc)` for parity with `_dispatch_framework_error()`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
easycord/_bot_events.py (1)
247-251:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
entrywhen using legacy component-prefix fallback.If fallback matching hits (Line 247-Line 251),
entryis never assigned, so Line 267 sendsplugin_name=Noneand skips plugin-scopedon_error.💡 Suggested fix
if handler is None: for registered_id, candidate_entry in self.registry.components.items(): # type: ignore[attr-defined] if registered_id.endswith("_") and custom_id.startswith(registered_id): + entry = candidate_entry handler = candidate_entry["func"] suffix = custom_id[len(registered_id):] breakAlso applies to: 267-268
🤖 Prompt for 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. In `@easycord/_bot_events.py` around lines 247 - 251, The legacy component-prefix fallback loop over self.registry.components sets handler and suffix but never assigns entry, so later code sees plugin_name=None and skips plugin-scoped on_error; update the loop (the block handling registered_id, candidate_entry) to also set entry = candidate_entry (and derive plugin_name from entry, e.g. entry.get("plugin_name") or the equivalent key used elsewhere) whenever you pick the handler/suffix so subsequent error-handling uses the correct plugin-scoped entry and plugin_name.easycord/_bot_plugins.py (1)
138-165:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRequire explicit
ToolSafetyon AI tools.Falling back to
ToolSafety.SAFEsilently accepts tools whose decorator omitted a safety annotation, which weakens the permission model. Reject registration when_ai_tool_safetyis missing instead of defaulting it.As per coding guidelines, "
@ai_tooldecorator must register a function intoToolRegistryand requires explicitToolSafetypermission annotation`."🤖 Prompt for 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. In `@easycord/_bot_plugins.py` around lines 138 - 165, The code currently defaults missing _ai_tool_safety to ToolSafety.SAFE; instead, ensure every decorated AI tool has an explicit safety annotation by checking for the presence of the attribute (use hasattr(method, "_ai_tool_safety") or similar) and reject registration when it's absent (raise a clear exception or skip registration and log an error) before populating self.ai_tools and calling self.tool_registry.register; update the logic around the ai_tools dict, the safety local variable, and the call to tool_registry.register to enforce this check so no tool is accepted without an explicit ToolSafety.
🤖 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 `@AGENTS.md`:
- Line 29: Update the AGENTS.md note to reflect the actual mixin split: state
that the context implementation is split across _context_base.py,
_context_channels.py, _context_moderation.py, and _context_ui.py and that edits
should target the appropriate mixin rather than editing context.py directly;
explicitly add _context_base.py to the list and reword the sentence that
currently implies context.py is the primary edit target so it instead directs
contributors to the named mixin files (use the exact filenames _context_base.py,
_context_channels.py, _context_moderation.py, _context_ui.py in the sentence).
In `@CHANGELOG.md`:
- Around line 5-20: Add the required blank line after each heading in the new
v5.2.0 changelog section so markdownlint MD022 is satisfied: insert a single
empty line immediately below the "### Highlights" and "### Bug Fixes" headings
and also below the subsection headings inside that section (e.g., any secondary
headings around lines referencing v5.2.0) so the pattern matches the existing
sections; update the CHANGELOG.md accordingly ensuring there is exactly one
blank line after each heading.
In `@context/architecture.md`:
- Around line 13-14: There is a stale, duplicated description of registry.py
that conflicts with the new "Interaction registry" entry; remove or merge the
old Line 19 entry so the file is described only once as the authoritative
InteractionRegistry for slash commands, context menus, components, modals, and
autocomplete callbacks. Edit the architecture.md section containing registry.py
so it either deletes the old "**Registry** — registry.py tracks live slash
commands, component handlers, and modal handlers. Commands accessible at
bot.registry.commands." line, or consolidate it into the new "Interaction
registry" paragraph by keeping the broader InteractionRegistry wording and
optionally preserving any additional accurate details (e.g.,
bot.registry.commands) beneath the single authoritative registry entry
referencing registry.py / InteractionRegistry.
In `@easycord/_bot_commands.py`:
- Around line 365-396: The autocomplete wrapper (_make_autocomplete inside the
loop over autocomplete_handlers) must stop using a blanket TypeError to choose
which handler arity to call; instead inspect the callable signature of handler
(_h) once (using inspect.signature or inspect.getfullargspec) and dispatch
explicitly to the correct call form (e.g., bound method signatures like (self,
ctx, current, options) or (ctx, current, options) vs (ctx, current) vs (current)
etc.), preserving the existing Context creation (Context(interaction)), options
retrieval (self._autocomplete_options(interaction)), and error handling via
self._dispatch_framework_error; remove the inner try/except that treats any
TypeError as arity mismatch and use the inspected parameter list to decide
whether to call _h(ctx, current, options), _h(ctx, current), or _h(current)
before converting results to app_commands.Choice and registering via
cmd.autocomplete and self.registry.register_autocomplete.
In `@easycord/_bot_plugins.py`:
- Around line 87-88: remove_plugin() currently doesn't remove entries that
_scan_methods() registered into _command_error_handlers, ai_tools, and
tool_registry, leaving bound methods that retain references to the plugin
instance; update remove_plugin()/unload logic to iterate those collections
(_command_error_handlers, ai_tools, tool_registry) and remove any entries whose
bound method object's __self__ (or attribute identifying the bound instance) is
the plugin instance being removed, so all handlers/tools registered by that
plugin are cleared and the plugin can be garbage-collected.
- Around line 29-30: The code keys plugin ownership by type(plugin).__name__
(see add_plugin and usages of plugin_name and standalone_autocomplete), which
collapses multiple instances of the same class; change to an instance-stable
identifier instead: generate or use a per-instance id (e.g., plugin_id =
getattr(plugin, "instance_id", None) or assign plugin._instance_id = uuid4()
when the plugin is first added) and replace all uses of plugin_name (and any
places building registries or task-status namespaces at the sites you noted: the
add_plugin logic and the other occurrences around the standalone_autocomplete
and registry code) with this plugin_id so each plugin instance has its own
registry entries and task-status namespace.
In `@easycord/bot.py`:
- Around line 129-157: plan_command_sync currently only diffs guild-scoped
entries when guild_id is provided, causing false diffs; update plan_command_sync
to include global commands alongside guild-specific ones by calling
registry.iter_syncable with guild_id=None (or otherwise fetching global entries)
and merging those entries with the guild entries when guild_id is set, ensuring
duplicates are handled and warnings still produced; ensure the returned plan
treats both global and guild-specific names as present (so
added/removed/unchanged reflect the merged set) and mirror this same merge
behavior in the other affected locations (the similar blocks around lines
159-192 and 248-254) so that sync_commands and setup_hook behavior matches the
dry-run plan and global commands are considered "synced" for guild-scoped plans.
In `@easycord/config.py`:
- Around line 143-149: The current override handling incorrectly treats string
values like "false" as True because it simply does auto_sync = bool(raw_sync);
update the logic around raw_sync/auto_sync so that when
overrides.pop("auto_sync", None) returns a str you parse it (e.g., lower() and
compare against ("0","false","no")) to produce a boolean, and only fall back to
bool(raw_sync) for non-string types; change the branch that sets auto_sync
(referencing raw_sync and auto_sync in this block in easycord/config.py)
accordingly so string overrides are handled explicitly.
In `@easycord/decorators.py`:
- Around line 83-90: The decorator currently instantiates
discord.AppInstallationType and discord.AppCommandContext (symbols:
AppInstallationType, AppCommandContext, assigns to func._slash_allowed_installs
and func._slash_allowed_contexts) which require discord.py>=2.4.0; to fix,
either raise the package minimum to discord.py>=2.4.0 in pyproject.toml, or add
a runtime guard in easycord/decorators.py that checks discord.__version__ (or
uses hasattr(discord, "AppInstallationType")) and only constructs
AppInstallationType/AppCommandContext when available, otherwise populate
func._slash_allowed_installs and func._slash_allowed_contexts with safe
fallbacks (e.g., None or a compatibility object) so older discord.py (2.0–2.3.x)
does not raise ImportError/AttributeError.
In `@easycord/registry.py`:
- Around line 258-283: _detect_component_collision currently only compares
existing.regex.pattern strings, so dynamic routes with identical segment
structure but different variable names (e.g., ticket:{id:int} vs
ticket:{other:int}) slip through; update the check to compare parsed segment
sequences instead of raw regexes: add or use the same parser that produces a
list of literal/type tokens for an InteractionEntry (e.g., a segments or tokens
property created when the entry is built), then in _detect_component_collision
compare those token sequences for equality (and for conflicting fullmatches) to
detect collisions regardless of parameter names; keep existing error paths but
change the pattern-equality branch to use the segment/token comparison and raise
the same ValueError including entry.name and existing.name when collisions are
detected so resolve_component cannot silently prefer the earlier registration.
In `@easycord/validators.py`:
- Around line 100-103: The validator's __call__ currently sorts self.choices
which can raise TypeError for mixed-type items and crash validation; change the
formatting logic to avoid direct comparison by sorting using a stable key like
str (e.g., use sorted(self.choices, key=str)) or fall back to an unsorted
iteration if sorting fails, then join those stringified choices into allowed and
raise ValidationError with that message in the same __call__ method.
In `@tests/test_new_decorators.py`:
- Around line 117-123: The test test_slash_default_require_admin_false uses an
undefined symbol reset_cmd in its assertion causing F821; update the assertion
to reference the decorated function ping instead (check
ping._slash_require_admin) so the test asserts ping._slash_require_admin is
False; ensure the unique symbols mentioned are
test_slash_default_require_admin_false, reset_cmd (remove/replace), ping, and
_slash_require_admin.
---
Outside diff comments:
In `@easycord/_bot_events.py`:
- Around line 247-251: The legacy component-prefix fallback loop over
self.registry.components sets handler and suffix but never assigns entry, so
later code sees plugin_name=None and skips plugin-scoped on_error; update the
loop (the block handling registered_id, candidate_entry) to also set entry =
candidate_entry (and derive plugin_name from entry, e.g.
entry.get("plugin_name") or the equivalent key used elsewhere) whenever you pick
the handler/suffix so subsequent error-handling uses the correct plugin-scoped
entry and plugin_name.
In `@easycord/_bot_plugins.py`:
- Around line 138-165: The code currently defaults missing _ai_tool_safety to
ToolSafety.SAFE; instead, ensure every decorated AI tool has an explicit safety
annotation by checking for the presence of the attribute (use hasattr(method,
"_ai_tool_safety") or similar) and reject registration when it's absent (raise a
clear exception or skip registration and log an error) before populating
self.ai_tools and calling self.tool_registry.register; update the logic around
the ai_tools dict, the safety local variable, and the call to
tool_registry.register to enforce this check so no tool is accepted without an
explicit ToolSafety.
🪄 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: 1711b516-462b-4322-8db0-06524a67ff81
📒 Files selected for processing (31)
.gitignoreAGENTS.mdCHANGELOG.mdMANIFEST.inREADME.mdcontext/architecture.mddocs/command-sync.mddocs/components-dynamic-routing.mddocs/getting-started.mddocs/interactions.mdeasycord/__init__.pyeasycord/_bot_commands.pyeasycord/_bot_events.pyeasycord/_bot_plugins.pyeasycord/_context_base.pyeasycord/bot.pyeasycord/config.pyeasycord/decorators.pyeasycord/plugin.pyeasycord/registry.pyeasycord/testing.pyeasycord/validators.pypyproject.tomltests/test_config.pytests/test_context.pytests/test_new_decorators.pytests/test_public_api.pytests/test_release_readiness.pytests/test_testing.pytests/test_v52_error_pipeline.pytests/test_v52_interactions.py
| **Bot core** — `bot.py` defines the `Bot` class via multiple inheritance: `discord.Client` + four mixins (`_bot_commands.py`, `_bot_events.py`, `_bot_guild.py`, `_bot_plugins.py`). Adding bot-level behavior means adding to one of these mixin files. | ||
|
|
||
| **Context** — `context.py` + `_context_channels.py`, `_context_moderation.py`, `_context_ui.py`. This is the user-facing API inside command handlers (`ctx.respond()`, `ctx.send_embed()`, etc.). | ||
| **Context** — `context.py` + `_context_channels.py`, `_context_moderation.py`, `_context_ui.py`. This is the user-facing API inside command handlers (`ctx.respond()`, `ctx.send()`, `ctx.send_embed()`, etc.). |
There was a problem hiding this comment.
Align the context architecture note with the actual mixin split.
Line 29 should include _context_base.py in the context stack and avoid implying context.py is the primary edit target.
Suggested doc correction
-**Context** — `context.py` + `_context_channels.py`, `_context_moderation.py`, `_context_ui.py`. This is the user-facing API inside command handlers (`ctx.respond()`, `ctx.send()`, `ctx.send_embed()`, etc.).
+**Context** — `_context_base.py` + `_context_channels.py`, `_context_moderation.py`, `_context_ui.py` (with `context.py` as the composition entrypoint). This is the user-facing API inside command handlers (`ctx.respond()`, `ctx.send()`, `ctx.send_embed()`, etc.).As per coding guidelines, "**/*_context*.py: Context implementation should be split across _context_base.py, _context_channels.py, _context_moderation.py, and _context_ui.py mixin files — edit the appropriate mixin, not context.py directly."
📝 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.
Suggested change
| **Context** — `context.py` + `_context_channels.py`, `_context_moderation.py`, `_context_ui.py`. This is the user-facing API inside command handlers (`ctx.respond()`, `ctx.send()`, `ctx.send_embed()`, etc.). | |
| **Context** — `_context_base.py` + `_context_channels.py`, `_context_moderation.py`, `_context_ui.py` (with `context.py` as the composition entrypoint). This is the user-facing API inside command handlers (`ctx.respond()`, `ctx.send()`, `ctx.send_embed()`, etc.). |
🤖 Prompt for 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.
In `@AGENTS.md` at line 29, Update the AGENTS.md note to reflect the actual mixin
split: state that the context implementation is split across _context_base.py,
_context_channels.py, _context_moderation.py, and _context_ui.py and that edits
should target the appropriate mixin rather than editing context.py directly;
explicitly add _context_base.py to the list and reword the sentence that
currently implies context.py is the primary edit target so it instead directs
contributors to the named mixin files (use the exact filenames _context_base.py,
_context_channels.py, _context_moderation.py, _context_ui.py in the sentence).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.