Skip to content

Conversation

@BoryaGames
Copy link

@BoryaGames BoryaGames commented Jan 2, 2026

Описание

Фиксит критичный баг с получением сообщений + добавляет рандомный размер экрана.

Тип изменений

  • Исправление бага
  • Новая функциональность
  • Улучшение документации
  • Рефакторинг

Связанные задачи / Issue

Ссылка на issue, если есть: #

Тестирование

Покажите пример кода, который проверяет изменения:

import asyncio
from pymax import MaxClient
from pymax.types import Message

client = MaxClient(phone="+79221231234")

@client.on_message()
async def echo(message: Message) -> None:
    if message.text:
        await client.send_message(
            chat_id=message.chat_id,
            text=f"Echo: {message.text}"
        )

if __name__ == "__main__":
    asyncio.run(client.start())

Summary by CodeRabbit

  • Refactor
    • Notification delivery now goes through the outgoing queue and requires an active connection, changing timing and delivery path.
    • Default screen resolution is now chosen from available options at startup instead of using a fixed value, making the initial screen selection variable.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Notification responses now require an active socket and are enqueued for outbound delivery (command id changed from 0 to 1). The DEFAULT_SCREEN constant is now chosen randomly from SCREEN_SIZES at import time.

Changes

Cohort / File(s) Summary
Notification Response Queuing
src/pymax/interfaces.py
_send_notification_response now checks for an active socket and enqueues NOTIF_MESSAGE via _queue_message instead of sending synchronously; queued payload uses command 1 (was 0).
Screen Size Default
src/pymax/static/constant.py
DEFAULT_SCREEN changed from the fixed string "1080x1920 1.0x" to a dynamic choice(SCREEN_SIZES) at import time, introducing nondeterministic selection.

Sequence Diagram(s)

sequenceDiagram
    participant Interface as Interfaces._send_notification_response
    participant Queue as OutgoingQueue/_queue_message
    participant Socket as SocketConnection

    Note over Interface,Socket: New path requires active socket
    Interface->>Socket: check active connection?
    alt active
        Interface->>Queue: enqueue NOTIF_MESSAGE (command=1)
        Queue->>Socket: deliver from outgoing queue (async)
        Socket-->>Queue: ack / send result
        Queue-->>Interface: delivery result/log
    else inactive
        Interface-->>Interface: skip enqueue (no socket)
        Interface-->>Interface: log skipped notification
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 I queued a note instead of sprinting fast,
A tiny change that altered how things pass.
Screens pick sizes from a playful hat,
I nibble logs and tip my hat 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Фикс критичного бага' (Fix critical bug) is vague and generic, lacking specific details about what bug was fixed or which system is affected. Provide a more specific title that identifies the actual bug being fixed, such as 'Fix critical message reception bug' or similar descriptive phrasing.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description mostly follows the template with main sections completed: problem description, change type selected, and testing code provided. However, the related issue section lacks a concrete issue reference.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6966d4 and 4d2f019.

📒 Files selected for processing (1)
  • src/pymax/interfaces.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ink-developer
Copy link
Collaborator

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/pymax/static/constant.py (1)

80-80: Consider the implications of import-time randomness.

The screen size is now randomly selected once at module import, making it consistent within a process but different across restarts. While this approach provides fingerprinting variety, it may complicate testing and debugging.

If you need per-instance randomization (e.g., different screen sizes for multiple client instances), consider moving this selection to the client initialization instead of module-level.

Note: The Ruff S311 warning is a false positive here since this isn't a cryptographic context.

</review_comment_end>

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9973b9 and e6966d4.

📒 Files selected for processing (2)
  • src/pymax/interfaces.py
  • src/pymax/static/constant.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/pymax/interfaces.py (1)
src/pymax/protocols.py (1)
  • _queue_message (110-118)
🪛 Ruff (0.14.10)
src/pymax/static/constant.py

80-80: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

🔇 Additional comments (1)
src/pymax/interfaces.py (1)

292-296: Verify cmd=1 is the correct value for NOTIF_MESSAGE with the server.

The switch from synchronous _send_and_wait to asynchronous _queue_message is a solid architectural improvement. However, cmd=1 appears to be unique to NOTIF_MESSAGE (all other message types use the default cmd=0), so this is a protocol-level detail that needs confirmation with server expectations.

@ink-developer
Copy link
Collaborator

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ink-developer ink-developer merged commit 2f55acb into MaxApiTeam:dev/1.2.6 Jan 2, 2026
1 check passed
@BoryaGames BoryaGames deleted the dev/1.2.6 branch January 2, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants