Fix view/modal leak and disable startup chunking (scale plan items 2 & 3)#73
Merged
Conversation
…ems 2 & 3) - View/modal leak: audited every View/Modal subclass in src/ and gave per-interaction ones a finite timeout instead of `timeout=None` (which nextcord never evicts from its internal stores without one). CustomView and CustomModal now default to a shared VIEW_TIMEOUT (5 min, src/config/global_settings.py) instead of each subclass hardcoding its own value. The 10 views registered once via bot.add_view() in init_views() correctly keep timeout=None; 7 decorative link-only views in ui_components.py were left alone (no dispatchable items, so they never populate nextcord's view store regardless of timeout). - Startup chunking: set chunk_guilds_at_startup=False and deleted ensure_guilds_ready() outright, since retrying full-guild REST chunking after boot would have silently reintroduced the same boot-time cost. Moved on_member_remove to on_raw_member_remove (always dispatched, unlike the cache-gated original), refactoring autobans.member_has_autoban, leveling.handle_member_removal, and join_leave_messages.trigger_leave_message to take guild/IDs directly since the raw event only provides a nextcord.User, not a Member. Removed four runtime guild.chunk() calls that would have re-chunked guilds on the message-edit/delete and member-removal/nickname-check hot paths, defeating the point of disabling startup chunking. - Correctness follow-ups needed to make the above safe: resolve an edited message's author via a single-member lookup (cache hit or REST fetch-on-miss) before the profanity check, since it can now be a plain User; implemented ExpiringSet.remove() since the method was incorrectly used in the codebase, and it is a reasonable inclusion. - Fixed a bug found while testing this against a live bot, unrelated to the plan above but blocking verification: nextcord 3.2.0 broke every nextcord.ui.Select's custom_id auto-generation (fixed by passing custom_id explicitly at all 14 call sites). - Added Active Views/Modals, chunked-guild count, and cached member/user/message counts to the `-memory` admin command (src/components/memory_profiler.py) as the verification metric the assessment doc calls for; the active-vs-raw-store distinction matters because nextcord only sweeps timed-out entries as a side effect of other view activity, not on a timer. - documentation/scale_performance_assessment_2026-07.md updated throughout to mark items 2 & 3 done and record all of the above. Full test suite passes (33/33). Reviewed across three passes (in-repo, GitHub Copilot, live-crash triage) with fixes applied for each. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> for monotonous changes. All code reviewed by myself and Claude Fable 5.
There was a problem hiding this comment.
Pull request overview
This PR addresses two scale/performance risks in InfiniBot’s nextcord integration: (1) accumulated View/Modal objects that aren’t evicted without timeouts, and (2) expensive startup/full-guild member chunking. It also adds observability to validate the fixes and makes several correctness refactors required by the raw-event migration.
Changes:
- Default most interaction-scoped Views/Modals to a finite timeout via
CustomView/CustomModal(with a sharedVIEW_TIMEOUT), plus explicitcustom_idfor selects to work around nextcord 3.2.0 serialization behavior. - Disable startup guild chunking (
chunk_guilds_at_startup=False), remove retry chunking, migrate member-removal handling to raw events, and remove several runtimeguild.chunk()calls on hot paths. - Extend
-memorystats with nextcord cache counts (views/modals/chunked guilds/members/users/messages) to verify plateauing under load.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/custom_types.py | Adds ExpiringSet.remove() to match existing usage and avoid runtime errors. |
| src/features/role_messages.py | Switches per-interaction views/modals to default timeouts; adds explicit select custom_ids. |
| src/features/reaction_roles.py | Adds explicit select custom_id to avoid nextcord select serialization issues. |
| src/features/purging.py | Moves confirmation view to default finite timeout. |
| src/features/profile.py | Updates views/modals to default finite timeouts; adds select custom_ids. |
| src/features/options_menu/entrypoint_ui.py | Moves options menu view to default finite timeout. |
| src/features/options_menu/editing/role_messages.py | Updates editing views/modals to default finite timeouts; adds select custom_ids. |
| src/features/options_menu/editing/reaction_roles.py | Moves editing views to default finite timeouts. |
| src/features/options_menu/editing/embeds.py | Adds explicit select custom_id for embed color select; finite view timeout. |
| src/features/options_menu/ban_button.py | Moves ban view to default finite timeout. |
| src/features/onboarding.py | Moves onboarding views to default finite timeout; adds explicit select custom_id. |
| src/features/moderation.py | Removes nickname-check chunking and documents accepted limitations with unchunked guilds. |
| src/features/leveling.py | Refactors member-removal handler to accept guild/member IDs (raw-event friendly). |
| src/features/jokes.py | Adjusts timeouts for joke-related UI; modals now default to finite timeouts. |
| src/features/join_leave_messages.py | Refactors leave-message trigger to accept guild + user (raw-event friendly). |
| src/features/embeds.py | Adds explicit select custom_id for embed color selection. |
| src/features/dashboard.py | Moves dashboard and nested views to default finite timeouts; adds select custom_ids. |
| src/features/autobans.py | Refactors autoban lookup to accept guild/member IDs (raw-event friendly). |
| src/features/admin_commands.py | Extends -memory output with nextcord view/modal/store and cache metrics; finite timeout for embed creation view. |
| src/features/action_logging.py | Updates member-removal logger to accept abc.User and removes chunking. |
| src/core/bot.py | Disables startup chunking, removes retry chunking, shifts member removal to raw event, removes hot-path chunking, resolves message author for profanity checks. |
| src/config/global_settings.py | Introduces shared VIEW_TIMEOUT constant. |
| src/components/utils.py | Allows placeholder replacement to work with raw-event users by guarding joined_at. |
| src/components/ui_components.py | Centralizes default timeouts in CustomView/CustomModal; adds missing custom_id to a shared select. |
| src/components/memory_profiler.py | Adds nextcord internal cache metrics for verification and monitoring. |
| documentation/scale_performance_assessment_2026-07.md | Updates assessment plan/status and documents implementation/verification details. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
299
to
302
| class JokeView(CustomView): | ||
| def __init__(self): | ||
| super().__init__(timeout=None) | ||
| super().__init__(timeout=3600) # 1 hour timeout | ||
|
|
Comment on lines
341
to
+345
| replacements["@member"] = member.mention | ||
| replacements["@username"] = member.name | ||
| replacements["@id"] = str(member.id) | ||
| replacements["@joindate"] = member.joined_at.strftime("%Y-%m-%d") if member.joined_at else "Unknown" | ||
| joined_at = getattr(member, "joined_at", None) | ||
| replacements["@joindate"] = joined_at.strftime("%Y-%m-%d") if joined_at else "Unknown" |
Comment on lines
+28
to
+37
| # all_views() de-duplicates by view id (a view can have multiple | ||
| # dispatchable items, each with its own entry in the raw store dict). | ||
| all_views = connection._view_store.all_views() | ||
| all_modals = list(connection._modal_store._modals.values()) | ||
|
|
||
| return { | ||
| 'view_store_active': sum(1 for v in all_views if not v.is_finished()), | ||
| 'view_store_raw_total': len(all_views), | ||
| 'modal_store_active': sum(1 for m in all_modals if not m.is_finished()), | ||
| 'modal_store_raw_total': len(all_modals), |
Comment on lines
+175
to
+180
| """ | ||
| Initializes the CustomView. | ||
|
|
||
| :param timeout: The timeout for the view. Defaults to None. | ||
| :type timeout: float | None | ||
| """ |
Comment on lines
+225
to
+229
| title: str, | ||
| *, | ||
| timeout: Optional[float] = VIEW_TIMEOUT, | ||
| custom_id: str = MISSING, | ||
| auto_defer: bool = True, |
GitHub Copilot found a few issues in the Pull Request for this branch. Most of its issues I deemed authentic and corrected in this commit.
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
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.
timeout=None(which nextcord never evicts from its internal stores without one). CustomView and CustomModal now default to a shared VIEW_TIMEOUT (5 min, src/config/global_settings.py) instead of each subclass hardcoding its own value. The 10 views registered once via bot.add_view() in init_views() correctly keep timeout=None; 7 decorative link-only views in ui_components.py were left alone (no dispatchable items, so they never populate nextcord's view store regardless of timeout).-memoryadmin command (src/components/memory_profiler.py) as the verification metric the assessment doc calls for; the active-vs-raw-store distinction matters because nextcord only sweeps timed-out entries as a side effect of other view activity, not on a timer.Full test suite passes (33/33). Reviewed across three passes (in-repo, GitHub Copilot, live-crash triage) with fixes applied for each.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com for monotonous changes.
All code reviewed by myself and Claude Fable 5.