Dev/2.4.0 - #78
Conversation
Запрос списка участников чата/группы.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughPyMax 2.4.0 добавляет опросы, голосовые сообщения, видеозаметки, управление чатом, параметры конфиденциальности, контроль присутствия, двухэтапный вход на мобильные устройства, создание отпечатков APK, обновленную обработку транспорта и пересмотренные контракты публичного API. ChangesРелиз API и документация
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant App
participant SessionService
participant AuthService
participant FingerprintGenerator
App->>SessionService: выполнить handshake
SessionService-->>App: вернуть HandshakeResponse
App->>AuthService: начать поток входа
AuthService->>FingerprintGenerator: сгенерировать отпечаток
FingerprintGenerator-->>AuthService: вернуть байты отпечатка
AuthService-->>App: вернуть LoginResponse и Login2Response
sequenceDiagram
participant MessageService
participant UploadService
participant UploadAPI
participant Dispatcher
MessageService->>UploadService: upload_voice
UploadService->>UploadAPI: потоковая передача чанков голоса
UploadAPI-->>Dispatcher: отправить VOICE_READY
Dispatcher-->>UploadService: вернуть AudioUploadSignal
UploadService-->>MessageService: вернуть аудиовложение
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pymax/types/domain/attachments/sticker.py (1)
25-26: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the
set_iddocstring type to match the new optional type.Line 45 changes
set_idtoint | None = None. Line 26 still documents:vartype set_id: int. Update the docstring type toint | Noneso the generated documentation matches the actual field type.📝 Proposed docstring fix
:ivar set_id: ID набора стикеров. - :vartype set_id: int + :vartype set_id: int | NoneAlso applies to: 45-45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pymax/types/domain/attachments/sticker.py` around lines 25 - 26, Update the set_id field documentation in the sticker model to declare its type as int | None, matching the field’s optional default value and keeping the docstring aligned with the implementation.
🧹 Nitpick comments (6)
src/pymax/api/uploads/service.py (2)
430-436: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftResolve the video-note duration before the upload.
get_duration()runs after the file was already uploaded. It raisesRuntimeErrorwhen thevideoextra is missing andValueErrorwhen tinytag cannot read the duration. In both cases the upload bandwidth and the server-side slot are already consumed. For aurlsource,get_duration()also callsread(), which downloads the whole video a second time afteriter_chunksstreamed it.Call
get_duration()once, before the POST, and reuse the value in the response payload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pymax/api/uploads/service.py` around lines 430 - 436, In the video-note upload flow, resolve and store the duration by calling uploadable_video.get_duration() once before the POST/upload begins, then reuse that value in the VideoAttachPayload response instead of calling get_duration() after upload. Preserve the existing error propagation for missing video support or unreadable metadata.
425-428: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDo not rebind
thumbhashfromstrtobytes.Line 427 concatenates padding onto a string, and line 428 replaces the same name with
bytes. Use separate names so the types stay stable for static analysis.♻️ Proposed change
- thumbhash = data.get("thumbhash") - if thumbhash: - thumbhash += "=" * (-len(thumbhash) % 4) - thumbhash = base64.b64decode(thumbhash) + raw_thumbhash = data.get("thumbhash") + thumbhash: bytes | None = None + if raw_thumbhash: + padded = raw_thumbhash + "=" * (-len(raw_thumbhash) % 4) + thumbhash = base64.b64decode(padded)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pymax/api/uploads/service.py` around lines 425 - 428, Update the thumbhash handling near data.get("thumbhash") to keep the original string variable unchanged and store the decoded base64 result in a separate bytes variable, using that new variable for subsequent processing.src/pymax/files/video.py (1)
114-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the duplicated
TinyTag.getcall.
BaseFile.read()already returnsself.rawwhen raw bytes are set. Both branches therefore pass the same bytes.♻️ Proposed simplification
- if self.raw: - tag = TinyTag.get( - filename=self.name, - file_obj=BytesIO(self.raw), - tags=False, - duration=True, - ) - else: - tag = TinyTag.get( - filename=self.name, - file_obj=BytesIO(await self.read()), - tags=False, - duration=True, - ) + tag = TinyTag.get( + filename=self.name, + file_obj=BytesIO(await self.read()), + tags=False, + duration=True, + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pymax/files/video.py` around lines 114 - 127, In the duration-loading logic, remove the conditional duplication around TinyTag.get and use a single call that passes BytesIO(await self.read()) as file_obj; BaseFile.read() already handles returning self.raw when available, so preserve the existing filename, tags, and duration arguments.tests/api/test_upload_service.py (1)
147-191: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
VideoNoteupload branch.The voice flow is covered well. The
VideoNotebranch ofupload_videois not. That branch skips the waiter, sendstype=1anduploaderType=1, parses the response JSON, pads and decodesthumbhash, and callsget_duration(). A test withVideoNote(raw=..., name=..., duration=...)and a JSON response body would lock invideo_type=1, the decodedthumbhash, and the duration.Do you want me to generate that test?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/api/test_upload_service.py` around lines 147 - 191, Add a dedicated async test for the VideoNote branch of upload_video, using VideoNote with raw data, name, and duration plus a JSON response body. Assert the request uses video_type=1 and uploaderType=1, the returned attachment contains the decoded padded thumbhash and duration from get_duration(), and no voice-upload waiter is involved.src/pymax/app.py (1)
201-209: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInclude
Opcode.LOGIN2in the revoked-token check.
_is_invalid_login_token_errormatches onlyOpcode.LOGIN. The startup path now issuesOpcode.LOGIN2immediately afterLOGIN. If the token is revoked between the two calls, theLOGIN2ApiErrordoes not match, soBaseClient.startroutes it as anON_STARTerror instead of triggeringrelogin. Accept both opcodes.Note also that the membership test compares whole values. A server message such as
"Login failed: FAIL_LOGIN_TOKEN"does not match. Confirm the server sends the marker as the fullerrorormessagevalue.♻️ Proposed change
`@staticmethod` def _is_invalid_login_token_error(exc: Exception) -> bool: return ( isinstance(exc, ApiError) - and exc.opcode == Opcode.LOGIN + and exc.opcode in (Opcode.LOGIN, Opcode.LOGIN2) and any( err in (exc.error, exc.message) for err in ("FAIL_LOGIN_TOKEN", "FAIL_LOGOUT_ALL") ) )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pymax/app.py` around lines 201 - 209, The _is_invalid_login_token_error method checks only for Opcode.LOGIN, but the startup path now issues Opcode.LOGIN2 immediately after LOGIN, and a revoked token at that point should also be recognized as an invalid login token error. Update the opcode condition in _is_invalid_login_token_error to accept both Opcode.LOGIN and Opcode.LOGIN2, preserving the existing error message matching logic for the error and message fields.src/pymax/api/auth/service.py (1)
63-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared fingerprint derivation.
Lines 68-84 duplicate lines 156-171 in
mobile_login. Both blocks resolvedevice_id, validatecalls_seed, and callgenerate_fingerprintwith the same four arguments. Extract one private helper that takes the caller name for the error message. This keeps the fingerprint inputs in one place if the protocol changes.The
calls_seed is Nonecheck is correct and keeps seed0valid.♻️ Proposed helper
def _device_fingerprint(self, caller: str) -> bytes | None: if not self.app.handshake_response: raise RuntimeError(f"No handshake response available for {caller}") if self.app.handshake_response.calls_seed is None: raise ValueError( "Unexpected internal state: handshake_response.calls_seed is missing " + f"in AuthService.{caller}. Please report this issue to the developer." ) device_id = ( self.app.session.device_id if self.app.session else self.app.config.device.device_id ) return self.app.fingerprint_generator.generate_fingerprint( version=self.app.config.device.user_agent.app_version, device_id=device_id, calls_seed=self.app.handshake_response.calls_seed, arch=self.app.config.device.user_agent.arch or "arm64-v8a", )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pymax/api/auth/service.py` around lines 63 - 88, Extract the duplicated fingerprint generation logic from request_code (lines 63-84) and mobile_login (lines 156-171) into a private helper method named _device_fingerprint that accepts a caller string parameter for error messages. The helper should resolve device_id, validate that calls_seed is not None, and return the result of generate_fingerprint with the four standard arguments (version, device_id, calls_seed, arch). Replace both occurrences of the duplicated block with calls to this new helper, passing the appropriate caller name for each method.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pymax/api/chats/service.py`:
- Around line 409-415: Update the admin payload construction in
ChatMixin.add_admin to validate that permissions is non-empty before calling
reduce(or_, permissions), and raise ValueError for an empty list while
preserving the existing reduction behavior for populated lists.
In `@src/pymax/api/messages/service.py`:
- Around line 232-243: Update the Message.edit method to make its text argument
optional, matching MessageService.edit_message’s text=None contract, so
attachment-only calls such as message.edit(attachments=[poll]) reach the service
without raising TypeError. Preserve existing text-edit behavior and forward
attachments unchanged.
In `@src/pymax/api/uploads/service.py`:
- Around line 438-443: Correct the misspelled service name in the ValueError
message within the future-missing check: change “UplpadService.upload_video” to
“UploadService.upload_video” without altering the surrounding error handling.
- Around line 230-236: Update the Content-Range value in the voice upload
response headers to include the required bytes unit before the existing range,
matching the format used by upload_video while preserving the current file-size
bounds.
In `@src/pymax/connection/connection.py`:
- Around line 156-160: Update the wait_closed handling around the shared
_recv_task await to use asyncio.shield(), preventing cancellation of
wait_closed() from propagating to the receive task while the connection remains
open; preserve the existing CancelledError behavior and closed-event waiting
flow.
- Line 72: Update the no-task branch of wait_closed() to await _closed_event
instead of returning immediately when _recv_task is cleared, ensuring callers
wait for transport cleanup and the closed signal. Add a test that begins
close(), blocks transport cleanup, then invokes wait_closed() and verifies it
remains pending until cleanup completes.
In `@src/pymax/fingerprint/fingerprint.py`:
- Around line 43-45: Update the fingerprint generation flow around the SHA-256
construction to handle an `arch` missing from `model.so_meta_sha256` without
raising `KeyError`; return `None`, matching the existing unknown-version
behavior in `generate_fingerprint`. Preserve normal fingerprint generation for
known architectures.
In `@src/pymax/transport/websocket.py`:
- Around line 20-23: The WebSocket transport applies the max_size limit only to
the proxied client.connect() branch, leaving direct connections with the default
websockets limit that can reject 10 MB messages. Add the same max_size=1024 *
1024 * 10 argument to the direct (non-proxied) client.connect() call so both
connection paths accept identical message sizes. Update
test_websocket_transport_connect_send_recv_and_close to verify that both the
proxied and direct connection branches use consistent max_size arguments.
In `@src/pymax/types/domain/attachments/video.py`:
- Around line 107-109: Update the legacy URL handling in the value conversion
logic so it uses dynamicUrl only when it is a string, then falls back to
dynamic_url when dynamicUrl is absent or non-string. Preserve the existing
behavior of returning the value with the resolved URL under the url key.
In `@src/pymax/types/domain/chat.py`:
- Around line 159-176: Update the public chat.answer() method documentation to
declare ValueError alongside the existing RuntimeError, covering calls where
both text and attachments are absent. Preserve the current parameter and return
documentation without changing behavior.
In `@src/pymax/types/events/reaction.py`:
- Line 20: Update the public documentation for the counters field in the
reaction type to declare it as list[ReactionCounter] | None, matching its
annotation and preserving the documented nullable behavior.
---
Outside diff comments:
In `@src/pymax/types/domain/attachments/sticker.py`:
- Around line 25-26: Update the set_id field documentation in the sticker model
to declare its type as int | None, matching the field’s optional default value
and keeping the docstring aligned with the implementation.
---
Nitpick comments:
In `@src/pymax/api/auth/service.py`:
- Around line 63-88: Extract the duplicated fingerprint generation logic from
request_code (lines 63-84) and mobile_login (lines 156-171) into a private
helper method named _device_fingerprint that accepts a caller string parameter
for error messages. The helper should resolve device_id, validate that
calls_seed is not None, and return the result of generate_fingerprint with the
four standard arguments (version, device_id, calls_seed, arch). Replace both
occurrences of the duplicated block with calls to this new helper, passing the
appropriate caller name for each method.
In `@src/pymax/api/uploads/service.py`:
- Around line 430-436: In the video-note upload flow, resolve and store the
duration by calling uploadable_video.get_duration() once before the POST/upload
begins, then reuse that value in the VideoAttachPayload response instead of
calling get_duration() after upload. Preserve the existing error propagation for
missing video support or unreadable metadata.
- Around line 425-428: Update the thumbhash handling near data.get("thumbhash")
to keep the original string variable unchanged and store the decoded base64
result in a separate bytes variable, using that new variable for subsequent
processing.
In `@src/pymax/app.py`:
- Around line 201-209: The _is_invalid_login_token_error method checks only for
Opcode.LOGIN, but the startup path now issues Opcode.LOGIN2 immediately after
LOGIN, and a revoked token at that point should also be recognized as an invalid
login token error. Update the opcode condition in _is_invalid_login_token_error
to accept both Opcode.LOGIN and Opcode.LOGIN2, preserving the existing error
message matching logic for the error and message fields.
In `@src/pymax/files/video.py`:
- Around line 114-127: In the duration-loading logic, remove the conditional
duplication around TinyTag.get and use a single call that passes BytesIO(await
self.read()) as file_obj; BaseFile.read() already handles returning self.raw
when available, so preserve the existing filename, tags, and duration arguments.
In `@tests/api/test_upload_service.py`:
- Around line 147-191: Add a dedicated async test for the VideoNote branch of
upload_video, using VideoNote with raw data, name, and duration plus a JSON
response body. Assert the request uses video_type=1 and uploaderType=1, the
returned attachment contains the decoded padded thumbhash and duration from
get_duration(), and no voice-upload waiter is involved.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5815758b-e787-4838-a9f4-c03a1e7978af
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (81)
docs/account.rstdocs/api/files.rstdocs/chats.rstdocs/client.rstdocs/files.rstdocs/index.rstdocs/messages.rstdocs/release-2-4-0.rstdocs/types/enums.rstdocs/types/index.rstdocs/types/poll.rstdocs/types/poll_attachment.rstpyproject.tomlsrc/pymax/__init__.pysrc/pymax/_data/apk_fingerprints.jsonsrc/pymax/api/auth/enums.pysrc/pymax/api/auth/payloads.pysrc/pymax/api/auth/service.pysrc/pymax/api/chats/__init__.pysrc/pymax/api/chats/enums.pysrc/pymax/api/chats/payloads.pysrc/pymax/api/chats/service.pysrc/pymax/api/messages/payloads.pysrc/pymax/api/messages/service.pysrc/pymax/api/self/__init__.pysrc/pymax/api/self/enums.pysrc/pymax/api/self/payloads.pysrc/pymax/api/self/service.pysrc/pymax/api/session/service.pysrc/pymax/api/uploads/payloads.pysrc/pymax/api/uploads/service.pysrc/pymax/app.pysrc/pymax/base.pysrc/pymax/client_web.pysrc/pymax/config.pysrc/pymax/connection/connection.pysrc/pymax/dispatch/enums.pysrc/pymax/dispatch/mapping.pysrc/pymax/dispatch/resolvers.pysrc/pymax/files/__init__.pysrc/pymax/files/video.pysrc/pymax/files/voice.pysrc/pymax/fingerprint/__init__.pysrc/pymax/fingerprint/fingerprint.pysrc/pymax/fingerprint/models.pysrc/pymax/infra/auth.pysrc/pymax/infra/bots.pysrc/pymax/infra/chat.pysrc/pymax/infra/message.pysrc/pymax/infra/self.pysrc/pymax/protocol/enums.pysrc/pymax/protocol/tcp/payload.pysrc/pymax/transport/tcp.pysrc/pymax/transport/websocket.pysrc/pymax/types/domain/__init__.pysrc/pymax/types/domain/attachments/__init__.pysrc/pymax/types/domain/attachments/enums.pysrc/pymax/types/domain/attachments/poll.pysrc/pymax/types/domain/attachments/sticker.pysrc/pymax/types/domain/attachments/video.pysrc/pymax/types/domain/chat.pysrc/pymax/types/domain/handshake.pysrc/pymax/types/domain/login.pysrc/pymax/types/domain/message.pysrc/pymax/types/domain/user.pysrc/pymax/types/events/__init__.pysrc/pymax/types/events/reaction.pysrc/pymax/types/events/voice.pytests/api/test_auth_service.pytests/api/test_chat_user_self_session_services.pytests/api/test_message_service.pytests/api/test_upload_service.pytests/app/test_app_runtime.pytests/conftest.pytests/connection/test_connection.pytests/connection/test_readers_and_transports.pytests/dispatch/test_dispatcher.pytests/domain/test_login_models.pytests/domain/test_message_models.pytests/files/test_files_and_formatting.pytests/protocol/test_protocols.py
| permissions: list[ChannelPermissions], | ||
| ) -> None: | ||
| frame = AddAdminPayload( | ||
| chat_id=chat_id, | ||
| user_ids=[user_id], | ||
| permissions=reduce(or_, permissions), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject an empty permissions list.
reduce(or_, permissions) raises TypeError when permissions is empty. The public ChatMixin.add_admin method accepts this input. Validate the list before reduction and raise ValueError.
Proposed fix
async def add_admin(
self,
chat_id: int,
user_id: int,
permissions: list[ChannelPermissions],
) -> None:
+ if not permissions:
+ raise ValueError("permissions must not be empty")
+
frame = AddAdminPayload(
chat_id=chat_id,
user_ids=[user_id],
permissions=reduce(or_, permissions),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: list[ChannelPermissions], | |
| ) -> None: | |
| frame = AddAdminPayload( | |
| chat_id=chat_id, | |
| user_ids=[user_id], | |
| permissions=reduce(or_, permissions), | |
| ) | |
| permissions: list[ChannelPermissions], | |
| ) -> None: | |
| if not permissions: | |
| raise ValueError("permissions must not be empty") | |
| frame = AddAdminPayload( | |
| chat_id=chat_id, | |
| user_ids=[user_id], | |
| permissions=reduce(or_, permissions), | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/api/chats/service.py` around lines 409 - 415, Update the admin
payload construction in ChatMixin.add_admin to validate that permissions is
non-empty before calling reduce(or_, permissions), and raise ValueError for an
empty list while preserving the existing reduction behavior for populated lists.
| text: str | None = None, | ||
| attachments: SendAttachments = None, | ||
| ) -> Message: | ||
| clean_text, elements = Formatter.format_markdown(text) | ||
| if not text and not attachments: | ||
| logger.error("edit_message failed: no text or attachments provided") | ||
| raise ValueError("Either text or attachments must be provided") | ||
|
|
||
| if text: | ||
| clean_text, elements = Formatter.format_markdown(text) | ||
| else: | ||
| clean_text, elements = None, [] | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make Message.edit support attachment-only edits.
MessageService.edit_message now accepts text=None. Message.edit at Line 295 still requires text. Therefore, await message.edit(attachments=[poll]) raises TypeError before this service runs.
Proposed fix
async def edit(
self,
- text: str,
+ text: str | None = None,
attachments: SendAttachments = None,
) -> Message:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/api/messages/service.py` around lines 232 - 243, Update the
Message.edit method to make its text argument optional, matching
MessageService.edit_message’s text=None contract, so attachment-only calls such
as message.edit(attachments=[poll]) reach the service without raising TypeError.
Preserve existing text-edit behavior and forward attachments unchanged.
| headers = { | ||
| "Content-Disposition": f"attachment; filename={quote(voice.name)}", | ||
| "Content-Range": f"0-{file_size - 1}/{file_size}", | ||
| "Content-Length": str(file_size), | ||
| "Connection": "keep-alive", | ||
| "Content-Type": "application/octet-stream", | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare every Content-Range header produced in the upload flows.
rg -n -B3 -A3 'Content-Range' --type=pyRepository: MaxApiTeam/PyMax
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect upload-related files and references without executing repository code.
printf 'Tracked files matching service.py:\n'
git ls-files | rg '((^|/)service\.py$|uploads|upload)' || true
printf '\nSearch Content-Range and upload_voice/upload_video references:\n'
git ls-files -z | xargs -0 rg -n -C 4 'Content-Range|upload_voice|upload_video|uploadable_voice|uploadable_video' || true
# If a repository checkout exists under /repo, inspect it directly; otherwise inspect whatever git ls-files was under sandbox root.
if [ -z "${REPO_ROOT:-}" ]; then
export REPO_ROOT="$(pwd)"
fi
printf '\nGit status/stat:\n'
git status --short || trueRepository: MaxApiTeam/PyMax
Length of output: 15813
🏁 Script executed:
#!/bin/bash
# Read file header/stat and focused snippets for likely upload files identified in previous output.
set -euo pipefail
files=$(git ls-files | rg 'src/pymax/api/uploads/service\.py|service\.py$|upload' | head -50 || true)
printf 'Candidate files:\n%s\n' "$files"
for f in $files; do
if [ -f "$f" ]; then
wc -l "$f"
ast-grep outline "$f" --view compact || true
printf '\n--- file: %s ---\n' "$f"
rg -n -C 3 'Content-Range|upload_voice|upload_video|uploadable_voice|uploadable_video' "$f" || true
fi
doneRepository: MaxApiTeam/PyMax
Length of output: 11201
🌐 Web query:
RFC 9110 Content-Range range unit bytes start-pos "/" end-pos "/" complete-length
💡 Result:
In RFC 9110, the Content-Range header field is used to specify the location of a partial range within a full representation [1][2]. Its ABNF grammar is defined as follows: Content-Range = range-unit SP ( range-resp / unsatisfied-range) [2] Where the components are defined as: range-resp = incl-range "/" ( complete-length / "") [2] incl-range = first-pos "-" last-pos [2] unsatisfied-range = "/" complete-length [2] complete-length = 1DIGIT [2] Key details include: 1. range-unit: Typically bytes, though the ABNF defines this generally as a token [2]. 2. incl-range: Represents the inclusive range of bytes, using zero-based absolute positions (first-pos and last-pos) [2][3]. 3. complete-length: The total length of the full representation in bytes, or an asterisk ("") if the length is unknown [2][3]. 4. unsatisfied-range: Used when a range request cannot be satisfied (often associated with status code 416), where the header indicates the total length of the resource using "*/complete-length" [2][3]. RFC 9110 establishes these semantics as part of the HTTP Semantics specification, superseding previous definitions found in older RFCs like RFC 7233 [1][4].
Citations:
- 1: https://datatracker.ietf.org/doc/html/rfc9110
- 2: https://grammars.wiki/catalog/rfc9110-http.html
- 3: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
- 4: https://www.rfc-editor.org/info/rfc9110/
Add the required range unit to voice upload Content-Range.
HTTP Content-Range must include a range unit before the byte range. Use bytes 0-{file_size - 1}/{file_size} here to align with upload_video and satisfy the HTTP header syntax.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/api/uploads/service.py` around lines 230 - 236, Update the
Content-Range value in the voice upload response headers to include the required
bytes unit before the existing range, matching the format used by upload_video
while preserving the current file-size bounds.
| with suppress(asyncio.CancelledError, Exception): | ||
| await self._recv_task | ||
| logger.debug("receive loop stopped") | ||
| self._recv_task = None |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wait for full cleanup after the receive task is cleared.
Line 72 clears _recv_task before event-task cancellation and transport.close() complete. A concurrent wait_closed() call then returns from its no-task branch before _closed_event is set. A caller can start a replacement connection while the old transport is still closing.
Make the no-task branch await _closed_event. Add a test that starts close() first, then calls wait_closed() while transport cleanup is blocked.
Proposed fix
async def wait_closed(self) -> None:
if not self._recv_task:
- return
+ await self._closed_event.wait()
+ return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/connection/connection.py` at line 72, Update the no-task branch of
wait_closed() to await _closed_event instead of returning immediately when
_recv_task is cleared, ensuring callers wait for transport cleanup and the
closed signal. Add a test that begins close(), blocks transport cleanup, then
invokes wait_closed() and verifies it remains pending until cleanup completes.
| h3 = hashlib.sha256( | ||
| bytes.fromhex(model.so_meta_sha256[arch]) + seed_bytes + device_bytes | ||
| ).digest() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare the arch keys in the packaged fingerprints against the arch values the config can produce.
fd 'apk_fingerprints.json' --exec sh -c 'jq -r "to_entries[] | .key + \": \" + (.value.so_meta_sha256 | keys | join(\",\"))" "$1"' sh {}
fd 'models.py' --base-directory src/pymax/fingerprint --exec cat -n {}
rg -nP -C2 'ANDROID_DEVICES\s*[:=]' --type=pyRepository: MaxApiTeam/PyMax
Length of output: 2183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fingerprint.py outline =="
ast-grep outline src/pymax/fingerprint/fingerprint.py || true
echo "== fingerprint.py relevant section =="
cat -n src/pymax/fingerprint/fingerprint.py | sed -n '1,80p'
echo "== files mentioning user_agent/arch/ExtraConfig =="
rg -n "user_agent|ExtraConfig|MobileUserAgentPayload|ANDROID_DEVICES" --type=py src README.md pyproject.toml 2>/dev/null || true
echo "== locate config/model files =="
fd -i 'config|model|device|user_agent' src/pymax --type=f | sed -n '1,120p'
echo "== deterministic keyset lookup probe =="
python3 - <<'PY'
import json
from pathlib import Path
paths = list(Path(".").rglob("apk_fingerprints.json"))
print("found", len(paths), "apk_fingerprints.json files")
all_keys = {}
for path in paths:
data = json.loads(path.read_text())
keys = set()
missing_versions = []
if isinstance(data, dict):
for v, model in data.items():
meta = model.get("so_meta_sha256")
if not isinstance(meta, dict):
missing_versions.append((v, "missing so_meta_sha256"))
continue
keys.update(meta.keys())
else:
print("unexpected data shape", type(data).__name__, path)
all_keys[path] = sorted(keys)
print(path, ":", ", ".join(sorted(keys)))
samples = ["arm64-v8a", "armeabi-v7a", "x86", "x86_64", "UNKNOWN", "", "undefined"]
for sample in samples:
hits = [p for p, keys in all_keys.items() if sample in keys]
misses = [p for p, keys in all_keys.items() if sample not in keys]
print(sample + " hits:", len(hits), "misses:", len(misses))
PYRepository: MaxApiTeam/PyMax
Length of output: 8566
Handle an unknown arch consistently.
model.so_meta_sha256[arch] can raise KeyError when the config supplies an architecture not present in the packaged fingerprint data; generate_fingerprint(None, ...) already returns None for an unknown version. Use the same fallback policy for unknown arch, such as returning None.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/fingerprint/fingerprint.py` around lines 43 - 45, Update the
fingerprint generation flow around the SHA-256 construction to handle an `arch`
missing from `model.so_meta_sha256` without raising `KeyError`; return `None`,
matching the existing unknown-version behavior in `generate_fingerprint`.
Preserve normal fingerprint generation for known architectures.
| self.url, | ||
| origin=Origin("https://web.max.ru"), | ||
| proxy=self.proxy, | ||
| max_size=1024 * 1024 * 10, # 10 MB |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '17,29p' src/pymax/transport/websocket.py
sed -n '199,220p' tests/connection/test_readers_and_transports.pyRepository: MaxApiTeam/PyMax
Length of output: 1227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,80p' src/pymax/transport/websocket.py
rg -n "max_size|TcpTransmission|TcpProtocol|read.*size|extra" src/pymax/transport -S
sed -n '180,230p' tests/connection/test_readers_and_transports.pyRepository: MaxApiTeam/PyMax
Length of output: 2885
🌐 Web query:
python websockets client.connect extra options max_size WebSocket receive size limit
💡 Result:
In the Python websockets library, the max_size parameter is used to enforce a maximum size (in bytes) for incoming WebSocket messages [1][2]. This setting is a critical security feature that helps prevent denial-of-service (DoS) attacks by bounding memory consumption [1]. Key points regarding max_size and related memory limits include: Default Value and Configuration: - The default value for max_size is 1 MiB (220 bytes) [1][3]. - You can disable the limit by setting max_size to None [2][3]. - For advanced control, you can pass a tuple (max_message_size, max_fragment_size) to set different limits for full messages and individual fragments [2][4]. This is particularly useful when expecting long messages delivered in short, fragmented chunks [2]. Connection Integration: - The max_size argument is available in the connect() method of the websockets client, as well as in serve() for servers [1][2][5]. - When a received message exceeds the defined max_size, the library raises an exception (typically ConnectionClosedError with close code 1009) [6][7][8]. Relationship with max_queue: - Memory usage is further controlled by the max_queue parameter, which limits the number of pending incoming messages stored in the internal queue [1]. - The memory footprint of incoming messages can be estimated as up to 4 * max_size * max_queue bytes [1][6]. Adjusting these parameters allows you to balance memory usage against your application's requirements [1][8]. Documentation References: - For comprehensive details on configuring these limits and understanding their impact on backpressure and memory, refer to the official Memory and buffers guide in the websockets documentation [1].
Citations:
- 1: https://websockets.readthedocs.io/en/stable/topics/memory.html
- 2: https://websockets.readthedocs.io/en/stable/reference/sansio/client.html
- 3: https://websockets.readthedocs.io/en/10.4/reference/client.html
- 4: https://websockets.readthedocs.io/en/latest/reference/asyncio/server.html
- 5: https://websockets.readthedocs.io/en/stable/reference/sync/client.html
- 6: https://websockets.readthedocs.io/en/9.1/api/server.html
- 7: https://websockets.readthedocs.io/en/8.1/api.html
- 8: https://websockets.readthedocs.io/en/9.1/%5Fmodules/websockets/legacy/client.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "websockets|websocket" pyproject.toml requirements*.txt setup.cfg setup.py tox.ini .github -S || true
(fd -a 'requirements.*|pyproject.toml|setup.*|tox.ini' . | sed 's#^\./##' | sort -u)
sed -n '1,80p' src/pymax/transport/tcp.py 2>/dev/null || true
rg -n "class .*Protocol|base_message_size|max_size|message size|send.*message|recv" src/pymax/core src/pymax -SRepository: MaxApiTeam/PyMax
Length of output: 7099
Keep the direct WebSocket transport in sync with the proxied message-size limit.
max_size=1024 * 1024 * 10 is only set for proxied client.connect(). Direct connections keep the default websockets limit and can reject messages that proxied connections accept, including 10 MB messages. Add the same direct client.connect() arguments for both branches and assert them in test_websocket_transport_connect_send_recv_and_close.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/transport/websocket.py` around lines 20 - 23, The WebSocket
transport applies the max_size limit only to the proxied client.connect()
branch, leaving direct connections with the default websockets limit that can
reject 10 MB messages. Add the same max_size=1024 * 1024 * 10 argument to the
direct (non-proxied) client.connect() call so both connection paths accept
identical message sizes. Update
test_websocket_transport_connect_send_recv_and_close to verify that both the
proxied and direct connection branches use consistent max_size arguments.
| legacy_url = value.get("dynamicUrl", value.get("dynamic_url")) | ||
| if isinstance(legacy_url, str): | ||
| return {**value, "url": legacy_url} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fall back to dynamic_url when dynamicUrl is not a string.
Line 107 selects dynamicUrl whenever that key exists. A payload with dynamicUrl=None and a valid string dynamic_url leaves url unset. Check dynamic_url if dynamicUrl is not a string.
Proposed fix
- legacy_url = value.get("dynamicUrl", value.get("dynamic_url"))
+ legacy_url = value.get("dynamicUrl")
+ if not isinstance(legacy_url, str):
+ legacy_url = value.get("dynamic_url")
if isinstance(legacy_url, str):
return {**value, "url": legacy_url}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| legacy_url = value.get("dynamicUrl", value.get("dynamic_url")) | |
| if isinstance(legacy_url, str): | |
| return {**value, "url": legacy_url} | |
| legacy_url = value.get("dynamicUrl") | |
| if not isinstance(legacy_url, str): | |
| legacy_url = value.get("dynamic_url") | |
| if isinstance(legacy_url, str): | |
| return {**value, "url": legacy_url} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/types/domain/attachments/video.py` around lines 107 - 109, Update
the legacy URL handling in the value conversion logic so it uses dynamicUrl only
when it is a string, then falls back to dynamic_url when dynamicUrl is absent or
non-string. Preserve the existing behavior of returning the value with the
resolved URL under the url key.
| text: str | None = None, | ||
| reply_to: int | None = None, | ||
| attachments: SendAttachments = None, | ||
| *, | ||
| notify: bool = True, | ||
| ) -> Message | None: | ||
| ) -> Message: | ||
| """Отправляет сообщение в этот чат. | ||
|
|
||
| :param text: Текст сообщения. | ||
| :type text: str | ||
| :type text: str | None | ||
| :param reply_to: ID сообщения для ответа. | ||
| :type reply_to: int | None | ||
| :param attachments: Файлы, фотографии или видео для отправки. | ||
| :type attachments: SendAttachments | ||
| :param notify: Отправить ли получателям push-уведомление. | ||
| :type notify: bool | ||
| :returns: Отправленное сообщение или ``None``, если сервер не вернул | ||
| его. | ||
| :rtype: Message | None | ||
| :returns: Отправленное сообщение. | ||
| :rtype: Message |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the empty-message ValueError.
Line 159 permits chat.answer() without text or attachments. src/pymax/api/messages/service.py rejects that call with ValueError, but this public method documents only RuntimeError. Add the ValueError contract.
Proposed fix
:rtype: Message
+ :raises ValueError: Если не переданы ни текст, ни вложения.
:raises RuntimeError: Если чат не привязан к клиенту.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| text: str | None = None, | |
| reply_to: int | None = None, | |
| attachments: SendAttachments = None, | |
| *, | |
| notify: bool = True, | |
| ) -> Message | None: | |
| ) -> Message: | |
| """Отправляет сообщение в этот чат. | |
| :param text: Текст сообщения. | |
| :type text: str | |
| :type text: str | None | |
| :param reply_to: ID сообщения для ответа. | |
| :type reply_to: int | None | |
| :param attachments: Файлы, фотографии или видео для отправки. | |
| :type attachments: SendAttachments | |
| :param notify: Отправить ли получателям push-уведомление. | |
| :type notify: bool | |
| :returns: Отправленное сообщение или ``None``, если сервер не вернул | |
| его. | |
| :rtype: Message | None | |
| :returns: Отправленное сообщение. | |
| :rtype: Message | |
| :rtype: Message | |
| :raises ValueError: Если не переданы ни текст, ни вложения. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/types/domain/chat.py` around lines 159 - 176, Update the public
chat.answer() method documentation to declare ValueError alongside the existing
RuntimeError, covering calls where both text and attachments are absent.
Preserve the current parameter and return documentation without changing
behavior.
| chat_id: int | ||
| counters: list[ReactionCounter] | ||
| total_count: int | ||
| counters: list[ReactionCounter] | None = None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the documented type of counters.
Line 20 permits None, but the docstring still declares counters as list[ReactionCounter]. Update the public type documentation.
Proposed fix
- :vartype counters: list[ReactionCounter]
+ :vartype counters: list[ReactionCounter] | None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pymax/types/events/reaction.py` at line 20, Update the public
documentation for the counters field in the reaction type to declare it as
list[ReactionCounter] | None, matching its annotation and preserving the
documented nullable behavior.
2 participants
Описание
Подготовка релиза 2.4.0:
Breaking changes:
fetch_history()теперь возвращает пустой список вместоNone; пустые сообщения без текста и вложений вызываютValueError.Тип изменений
Связанные задачи / Issue
#76
#73
#70
#69
Summary by CodeRabbit