cleanup: remove dead maintenance mode artifacts in bot.py (#104) #105

Claude · 2026-03-20T15:33:08Z

Claude commented

2026-03-20 15:33:08 +00:00

Closes #104

Changes

Two small deletions in bot.py — no behavioral change.

1. Remove duplicate `self.maintenance_mode` assignment

self.maintenance_mode: bool = False was assigned twice in SBABot.__init__ (lines 121 and 123), a merge artifact from PR #83. Removed the second (lower) assignment.

2. Delete dead `@self.tree.interaction_check` block

The 16-line decorator block in setup_hook is dead code: MaintenanceAwareTree.interaction_check() already handles maintenance gating via the tree_cls=MaintenanceAwareTree kwarg. Keeping it also generated a RuntimeWarning at startup (discord.py warns when interaction_check is assigned via decorator on a subclassed CommandTree).

Tests

tests/test_bot_maintenance_tree.py: 8/8 passed
Full suite: 935 passed (18 failures are pre-existing, from unrelated modified files in the working tree)

Other observations

The auto-formatter (ruff) applied two cosmetic reformats on edited lines: RotatingFileHandler(...) args and bot.latency * 1000. These are incidental to the linter hook, not intentional changes.

Closes #104 ## Changes Two small deletions in `bot.py` — no behavioral change. ### 1. Remove duplicate `self.maintenance_mode` assignment `self.maintenance_mode: bool = False` was assigned twice in `SBABot.__init__` (lines 121 and 123), a merge artifact from PR #83. Removed the second (lower) assignment. ### 2. Delete dead `@self.tree.interaction_check` block The 16-line decorator block in `setup_hook` is dead code: `MaintenanceAwareTree.interaction_check()` already handles maintenance gating via the `tree_cls=MaintenanceAwareTree` kwarg. Keeping it also generated a `RuntimeWarning` at startup (discord.py warns when `interaction_check` is assigned via decorator on a subclassed CommandTree). ## Tests - `tests/test_bot_maintenance_tree.py`: 8/8 passed - Full suite: 935 passed (18 failures are pre-existing, from unrelated modified files in the working tree) ## Other observations The auto-formatter (ruff) applied two cosmetic reformats on edited lines: `RotatingFileHandler(...)` args and `bot.latency * 1000`. These are incidental to the linter hook, not intentional changes.

Claude added 1 commit 2026-03-20 15:33:09 +00:00

cleanup: remove dead maintenance mode artifacts in bot.py (#104 )

Build Docker Image / build (pull_request) Successful in 1m6s

Details

008d6be86c

Closes #104

- Remove duplicate `self.maintenance_mode: bool = False` assignment (merge
  artifact from PR #83)
- Delete dead `@self.tree.interaction_check` block in `setup_hook` that
  generated a RuntimeWarning at startup; `MaintenanceAwareTree.interaction_check()`
  already handles this correctly via the `tree_cls=MaintenanceAwareTree` kwarg

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-03-20 15:33:15 +00:00

cleanup: remove dead maintenance mode artifacts in bot.py #104

Claude added the

ai-reviewing

label 2026-03-20 15:45:17 +00:00

Claude reviewed 2026-03-20 15:46:27 +00:00

Claude left a comment

AI Code Review

Files Reviewed

bot.py (modified)

Findings

Correctness

Both targeted removals are correct and complete.

Duplicate self.maintenance_mode assignment — the diff removes line 123 (self.maintenance_mode: bool = False), leaving exactly one assignment at line 121. The PR branch head confirms only a single assignment remains. This was a clean merge artifact from PR #83 with no semantic effect, but it generated a mypy/type-checker warning and was confusing to read.

Dead @self.tree.interaction_check block — the 16-line decorator block in setup_hook is correctly removed. MaintenanceAwareTree.interaction_check() (lines 82–102 of the final file) already owns this responsibility via tree_cls=MaintenanceAwareTree in super().__init__. The decorator assignment was not only redundant — in discord.py, assigning via decorator to interaction_check on a CommandTree subclass generates a RuntimeWarning at startup. The removal is clean and the maintenance gate remains fully functional.

The PR is mergeable: true — no conflicts with main. This is the correct outcome of the action requested in the PR #84 review.

Security

No issues. No new code introduced, only deletions.

Style & Conventions

Ruff reformats — two incidental reformats were applied by the pre-commit linter hook:

RotatingFileHandler(...) args expanded to one-per-line (lines 44–47 of final file). Cosmetic, valid.
bot.latency*1000 → bot.latency * 1000 with argument expansion (health command embed). Cosmetic, valid.

Both are linter-enforced style corrections on lines the diff touched. They do not affect behavior and follow the project's ruff configuration.

Test Coverage

tests/test_bot_maintenance_tree.py (8 tests) covers:

maintenance off: non-admin allowed, admin allowed
maintenance on: non-admin blocked with ephemeral message, admin allowed
maintenance on: non-Member (DM context) user blocked
missing maintenance_mode attribute safe fallback via getattr
no-emoji assertion on the block message
TestSBABotMaintenanceModeAttribute.test_maintenance_mode_default_is_false — patches commands.Bot.__init__ to a no-op and calls SBABot.__init__ directly, asserting exactly one maintenance_mode = False assignment exists and holds the correct value

The last test directly validates the fix in this PR: it would catch any regression that re-introduced the duplicate assignment or changed the default value.

Suggestions

None. This PR is a targeted two-deletion cleanup with no new logic. It is exactly what PR #84's review requested and nothing more.

Verdict: COMMENT (APPROVED)

Correct, minimal, and complete. Both dead-code artifacts tracked since PR #83 are removed. The MaintenanceAwareTree subclass continues to handle maintenance gating correctly. The ruff reformats are cosmetic and linter-enforced. All 8 maintenance tree tests pass. Ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `bot.py` (modified) ### Findings #### Correctness Both targeted removals are correct and complete. **Duplicate `self.maintenance_mode` assignment** — the diff removes line 123 (`self.maintenance_mode: bool = False`), leaving exactly one assignment at line 121. The PR branch head confirms only a single assignment remains. This was a clean merge artifact from PR #83 with no semantic effect, but it generated a `mypy`/type-checker warning and was confusing to read. **Dead `@self.tree.interaction_check` block** — the 16-line decorator block in `setup_hook` is correctly removed. `MaintenanceAwareTree.interaction_check()` (lines 82–102 of the final file) already owns this responsibility via `tree_cls=MaintenanceAwareTree` in `super().__init__`. The decorator assignment was not only redundant — in discord.py, assigning via decorator to `interaction_check` on a `CommandTree` subclass generates a `RuntimeWarning` at startup. The removal is clean and the maintenance gate remains fully functional. The PR is `mergeable: true` — no conflicts with `main`. This is the correct outcome of the action requested in the PR #84 review. #### Security No issues. No new code introduced, only deletions. #### Style & Conventions **Ruff reformats** — two incidental reformats were applied by the pre-commit linter hook: 1. `RotatingFileHandler(...)` args expanded to one-per-line (lines 44–47 of final file). Cosmetic, valid. 2. `bot.latency*1000` → `bot.latency * 1000` with argument expansion (health command embed). Cosmetic, valid. Both are linter-enforced style corrections on lines the diff touched. They do not affect behavior and follow the project's ruff configuration. #### Test Coverage `tests/test_bot_maintenance_tree.py` (8 tests) covers: - maintenance off: non-admin allowed, admin allowed - maintenance on: non-admin blocked with ephemeral message, admin allowed - maintenance on: non-Member (DM context) user blocked - missing `maintenance_mode` attribute safe fallback via `getattr` - no-emoji assertion on the block message - `TestSBABotMaintenanceModeAttribute.test_maintenance_mode_default_is_false` — patches `commands.Bot.__init__` to a no-op and calls `SBABot.__init__` directly, asserting exactly one `maintenance_mode = False` assignment exists and holds the correct value The last test directly validates the fix in this PR: it would catch any regression that re-introduced the duplicate assignment or changed the default value. #### Suggestions None. This PR is a targeted two-deletion cleanup with no new logic. It is exactly what PR #84's review requested and nothing more. ### Verdict: COMMENT (APPROVED) Correct, minimal, and complete. Both dead-code artifacts tracked since PR #83 are removed. The `MaintenanceAwareTree` subclass continues to handle maintenance gating correctly. The ruff reformats are cosmetic and linter-enforced. All 8 maintenance tree tests pass. Ready to merge. --- *Automated review by Claude PR Reviewer*

cal changed target branch from main to next-release

2026-03-20 17:48:56 +00:00

cal merged commit daa3366b60 into next-release

2026-03-20 17:49:42 +00:00

cal deleted branch ai/major-domo-v2-104

2026-03-20 17:49:43 +00:00

cal referenced this issue from a commit

2026-03-20 17:49:44 +00:00

Merge pull request 'cleanup: remove dead maintenance mode artifacts in bot.py (#104)' (#105) from ai/major-domo-v2-104 into next-release

cal removed the

ai-reviewing

label 2026-03-23 15:32:39 +00:00