Repository navigation
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true✨ Finishing Touches
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 |
biradrags
left a comment
biradrags
left a comment
There was a problem hiding this comment.
Inline-ревью по строкам. Дизайн RichFormat+RichParams в data норм, send-путь и фиксы html/md/async/link_preview ок. Блокеры — в edit-пути и совместимости, плюс фича пока не включается end-to-end (нет конкретного RichFormat). Детали в комментах ниже.
Не привязалось к строкам, но важно:
- Публичный API:
RichParams/RichParseMode/RichFormatнет в пакетном__init__.__all__, юзеры импортят из internal. - Тестов на rich-путь ноль;
MockMessageManagerбезsend_rich→ тест сWindow(rich=)упадётAttributeError. - HTML-инъекция: rich-html строится из текста виджетов без экранирования, юзер-данные с
<>&ломают разметку — нужен контракт/хелпер.
Готов взять edit_rich+_combine (главные рантайм-баги) отдельным PR.
| from bot_dialogs.main import main_dialog | ||
| from bot_dialogs.mutltiwidget import multiwidget_dialog | ||
| from bot_dialogs.reply_buttons import reply_kbd_dialog | ||
| from bot_dialogs.rich import rich_dialog |
There was a problem hiding this comment.
Блокер. example/mega/bot_dialogs/rich.py в ветке нет → пример падает ModuleNotFoundError. Плюс конкретного класса под RichFormat нигде нет (только Protocol), rich=Rich(...) из примера в чате не импортируется → фичу нельзя включить end-to-end. Нужен Rich-класс + сам пример.
| ] | ||
| dependencies = [ | ||
| "aiogram>=3.14.0", | ||
| "aiogram>=3.29.0", |
There was a problem hiding this comment.
Блокер (совместимость). Floor aiogram>=3.14→>=3.29 двигает всем ради опциональной фичи. Вместе с безусловным импортом InputRichMessage и убранным ButtonStyle-шимом → юзеры на 3.14-3.28 ловят ImportError на старте, даже без rich. Вынести rich в extra [rich] + guarded import?
There was a problem hiding this comment.
Нужна версия новее, не вижу смысла делать extra
| InputMediaDocument, | ||
| InputMediaPhoto, | ||
| InputMediaVideo, | ||
| InputRichMessage, |
There was a problem hiding this comment.
Безусловный импорт InputRichMessage (есть только в aiogram 3.29) на уровне модуля → import aiogram_dialog падает на старых aiogram. См. коммент к pyproject.toml про extra [rich] + try/except.
| # we cannot actually compare reply keyboards | ||
| (new_message.reply_markup or old_message.has_reply_keyboard) or | ||
| # formatting changed | ||
| (new_message.rich_params != old_message.rich_params) or |
There was a problem hiding this comment.
Блокер. Сравнение всегда True для rich-окна: _combine (строка 63) строит OldMessage без rich_params, поле остаётся None, а тут set != None → лишний edit/resend на каждое взаимодействие, no-op оптимизация мертва. Фикс на строку: rich_params=sent_message.rich_params в _combine.
| chat_id=old_message.chat.id, | ||
| business_connection_id=new_message.business_connection_id, | ||
| text=new_message.text, | ||
| rich_message=self._get_rich_input(new_message), |
There was a problem hiding this comment.
Блокер. edit_text всегда шлёт text + rich_message + parse_mode разом, а по Bot API text/rich_message взаимоисключающие. send-путь роутится в send_rich (поля убраны), edit — нет. Первый показ ок, любая перерисовка окна → TelegramBadRequest. Нужен edit_rich симметрично send_rich, либо delete-and-resend при rich в _can_edit.
There was a problem hiding this comment.
не вижу где сказано что они взаимоисключающие, всё и так работает.
| manager, | ||
| ) | ||
|
|
||
| current_data["rich_params"] = rich_params |
There was a problem hiding this comment.
current_data["rich_params"] = rich_params инъектит computed-значение в getter-data юзера — прецедента в классе нет (parse_mode/protect_content так не прокидываются), течёт в jinja/format контекст, может затереть одноимённый ключ. Плюс избыточно — rich_params уже идёт прямым полем в NewMessage. Убрать, если виджетам не нужно читать из data.
| @@ -1,5 +1,6 @@ | |||
| from dataclasses import dataclass | |||
| from enum import Enum | |||
| from typing import Literal, TypeAlias | |||
There was a problem hiding this comment.
Literal/TypeAlias больше не используются после перехода на RichParseMode → ruff F401 завалит CI.
| UNKNOWN = object() | ||
|
|
||
|
|
||
| class RichParseMode(str, Enum): |
There was a problem hiding this comment.
Nit. Члены RichParseMode lowercase (html/markdown) против titlecase у aiogram ParseMode — путает при смешивании.
| @dataclass | ||
| class RichParams: | ||
| parse_mode: RichParseMode | ||
| is_rtl: bool |
There was a problem hiding this comment.
is_rtl/skip_entity_detection без дефолтов, хотя в InputRichMessage опциональны → caller всегда пишет все три поля. is_rtl: bool = False / skip_entity_detection: bool = False?
|
|
||
| @runtime_checkable | ||
| class RichFormat(Protocol): | ||
| async def render_rich_params( |
There was a problem hiding this comment.
Nit (×3): render_rich_params без @abstractmethod (соседние протоколы в файле ставят), нет финального newline (W292, строка 151), докстринг "Transform new_message" (150) ничего не описывает.
|
Todo: check if we can edit from rich to not rich and vice versa |
TODO: