refactor: extract duplicate command hash logic into _compute_command_hash (#31) #65

cal · 2026-03-05T06:32:17Z

cal commented

2026-03-05 06:32:17 +00:00

Summary

Fixes #31 — DRY violation in bot.py where _should_sync_commands and _save_command_hash both contained an identical ~35-line block that built a command data list and computed a SHA-256 hash.

Changes

Added _compute_command_hash(self) -> str — a new synchronous helper that builds the command data list and returns the hex digest
_should_sync_commands now calls self._compute_command_hash() instead of inlining the logic
_save_command_hash now calls self._compute_command_hash() instead of inlining the logic

Files Changed

bot.py — 37 insertions, 67 deletions (net −30 lines)

Tests

924 tests pass. 6 pre-existing failures in test_commands_profile_images.py are unrelated to this change (those files were already modified on main before this branch).

## Summary Fixes #31 — DRY violation in `bot.py` where `_should_sync_commands` and `_save_command_hash` both contained an identical ~35-line block that built a command data list and computed a SHA-256 hash. ## Changes - Added `_compute_command_hash(self) -> str` — a new synchronous helper that builds the command data list and returns the hex digest - `_should_sync_commands` now calls `self._compute_command_hash()` instead of inlining the logic - `_save_command_hash` now calls `self._compute_command_hash()` instead of inlining the logic ## Files Changed - `bot.py` — 37 insertions, 67 deletions (net −30 lines) ## Tests 924 tests pass. 6 pre-existing failures in `test_commands_profile_images.py` are unrelated to this change (those files were already modified on `main` before this branch).

cal added 1 commit 2026-03-05 06:32:18 +00:00

refactor: extract duplicate command hash logic into _compute_command_hash (#31 )

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

Details

2242140dcd

Both _should_sync_commands and _save_command_hash contained an identical
~35-line block building the command data list and computing the SHA-256 hash.
Extracted into a new synchronous helper method _compute_command_hash() that
both callers now delegate to.

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

cal referenced this pull request

2026-03-05 06:32:26 +00:00

Extract duplicated command hash logic into a shared helper in `bot.py` #31

cal added the

ai-reviewing

label 2026-03-05 06:45:31 +00:00

cal reviewed 2026-03-05 06:46:25 +00:00

cal left a comment

AI Code Review

Files Reviewed

bot.py (modified)

Findings

Correctness

The extracted _compute_command_hash(self) -> str is a faithful, byte-for-byte equivalent of the inlined logic it replaces in both _should_sync_commands and _save_command_hash. No behavioral change.
The method is correctly synchronous — it performs only CPU-bound work (dict building + SHA-256), so no async is needed.
Return type annotation -> str is accurate (hexdigest is always a str).
Both callers retain their try/except blocks, so any exception raised by _compute_command_hash (e.g., from self.tree.get_commands()) is still caught and handled properly.

Security

No security issues. This is a pure refactoring with no behavior or data flow changes.

Style & Conventions

Private helper naming (_compute_command_hash) is consistent with the existing _should_sync_commands / _save_command_hash convention.
Placement just before _should_sync_commands is logical and readable.
Docstring is accurate and concise.

Suggestions

None that would block merging. Optionally, a unit test for _compute_command_hash directly could be added in the future, but the method's behavior is already exercised indirectly through the callers.

Verdict: APPROVED

Clean DRY refactoring. The extracted helper is correctly typed, synchronous, and the calling methods are unchanged in behavior. No issues found. (Note: posted as COMMENT — Gitea does not permit self-approval.)

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `bot.py` (modified) ### Findings #### Correctness - The extracted `_compute_command_hash(self) -> str` is a faithful, byte-for-byte equivalent of the inlined logic it replaces in both `_should_sync_commands` and `_save_command_hash`. No behavioral change. - The method is correctly synchronous — it performs only CPU-bound work (dict building + SHA-256), so no `async` is needed. - Return type annotation `-> str` is accurate (hexdigest is always a `str`). - Both callers retain their `try/except` blocks, so any exception raised by `_compute_command_hash` (e.g., from `self.tree.get_commands()`) is still caught and handled properly. #### Security - No security issues. This is a pure refactoring with no behavior or data flow changes. #### Style & Conventions - Private helper naming (`_compute_command_hash`) is consistent with the existing `_should_sync_commands` / `_save_command_hash` convention. - Placement just before `_should_sync_commands` is logical and readable. - Docstring is accurate and concise. #### Suggestions - None that would block merging. Optionally, a unit test for `_compute_command_hash` directly could be added in the future, but the method's behavior is already exercised indirectly through the callers. ### Verdict: APPROVED Clean DRY refactoring. The extracted helper is correctly typed, synchronous, and the calling methods are unchanged in behavior. No issues found. (Note: posted as COMMENT — Gitea does not permit self-approval.) --- *Automated review by Claude PR Reviewer*

cal added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-05 06:46:34 +00:00

cal changed target branch from main to next-release

2026-03-07 03:28:16 +00:00

cal merged commit 26a22aae4f into next-release

2026-03-07 03:28:29 +00:00

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

2026-03-07 03:28:29 +00:00

cal referenced this issue from a commit