refactor: extract settings menu from telegram bot - #161
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This was referenced Aug 18, 2026
- Move the /model and /thinking inline-keyboard settings UI (menu building, callback parsing, capability resolution, reasoning options) out of TelegramBot into a standalone SettingsMenu class in telegram/settings_menu.py - TelegramBot now owns a SettingsMenu instance and delegates command dispatch and non-health callback routing to it; _load_chat_profile stays on TelegramBot since turn handling needs it too - bot.py drops from 1891 to 1500 lines; the extracted ~450 lines gain their own module instead of sharing an unrelated God class - Rewrite the coupled test suite: settings-menu behavior now tests SettingsMenu directly in tests/test_telegram_settings_menu.py; the old test_telegram_bot_model_override.py is trimmed to the thin TelegramBot-level delegation tests - No behavior change; full test suite (1560 tests) passes
QueryPlanner
force-pushed
the
refactor/extract-settings-menu
branch
from
August 19, 2026 05:32
02ebfdb to
18dcb67
Compare
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.
What
Extracts the
/modeland/thinkinginline-keyboard settings UI out ofTelegramBotinto a standaloneSettingsMenuclass in a newtelegram/settings_menu.py. Second of the planned cleanup series fortelegram/bot.py(stacked on #160, the pure-dedup PR — please review/merge that one first).Why
The original audit flagged
bot.py's settings-menu code (~300 lines: model/thinking menu building, callback parsing, capability resolution, reasoning-option logic) as a textbook "unrelated UI logic bolted onto a transport class" violation — none of it touches polling, session routing, or ADK runtime state, it only needs a Telegram API client and a way to load a chat's inference profile.Discovered while scoping this PR: the existing test suite for this subsystem (
tests/test_telegram_bot_model_override.py, 908 lines) pokesTelegramBot's private methods and attributes directly (bot._build_model_menu,bot._resolve_capabilities,bot._capabilities_resolver,patch("blacki.telegram.bot.MODEL_CHOICES", ...), etc.) rather than through a public interface — so a clean extraction meant rewriting most of that test file, not just moving code. Flagged this tradeoff and got explicit sign-off before doing the full rewrite rather than either leaving compatibility-shim methods onTelegramBotor silently taking on a much bigger diff than initially scoped.This is a relocation for separation of concerns, not a line-count reduction —
bot.pyshrinks by 391 lines, but the extracted code and its tests now carry their own module/fixture overhead, so total repo lines grow slightly. That's expected; PR #160 was the line-reduction pass, this one is the bloat/separation-of-concerns pass.How
telegram/settings_menu.py:SettingsMenuclass taking anapi_providercallable (sinceTelegramBot.apiis a lazy property) and aload_profilecallable (since_load_chat_profileis shared with turn-handling and stays onTelegramBot). OwnsMODEL_CHOICES, thes:*callback-data constants, reasoning labels, and all the menu-building/callback-handling/capability-resolution methods verbatim from the oldTelegramBot.bot.py:TelegramBot.__init__constructsself._settings_menu = SettingsMenu(...);stop()callsself._settings_menu.aclose()instead of managing_capabilities_resolverdirectly;_handle_command's/model//thinkingbranches and_handle_callback_query's non-health branch delegate toself._settings_menu._load_chat_profilestays onTelegramBotunchanged.bot.py(1891 → 1500 lines); no behavior change to any of it, verified by porting every existing test case.tests/test_telegram_settings_menu.py(new, 856 lines) testsSettingsMenudirectly via a mock API provider and profile loader — this ports essentially all prior settings-behavior test coverage (menu building, callback dispatch for model/reasoning/reset/thinking/back, capability resolution and caching, legacy-model migration race conditions, Telegram callback-data byte-limit check).tests/test_telegram_bot_model_override.pyis trimmed from 908 to 154 lines, keeping only trueTelegramBot-level tests: command dispatch delegates to_settings_menu, callback routing splits health vs. settings correctly, and_load_chat_profile's environment-fallback behavior.Tests
pytest tests/test_telegram_settings_menu.py tests/test_telegram_bot_model_override.py tests/test_telegram_bot.py tests/test_telegram_health.py -q— 300 passedpytest -q(full suite) — 1560 passed, 1 skippedruff check/ruff format --checkon all changed/new files — cleanmypyonsrc/blacki/telegram/— clean