Repository navigation
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py" line_range="698-711" />
<code_context>
- ret = await send_func(fallback_payload)
- logger.info("[QQOfficial] 使用主动发送接口发送成功。")
- return ret
- except _QQOFFICIAL_SEND_API_ERRORS as fallback_err:
- err = fallback_err
- payload = fallback_payload
-
- if not isinstance(err, botpy.errors.ServerError):
- raise
</code_context>
<issue_to_address>
**Markdown fallback is skipped**
When the passive send fails for a non-Markdown reason and the active retry is rejected for Markdown, `_send_with_markdown_fallback` propagates the active retry's Markdown error before attempting the plain-content downgrade, so the message is lost.
Route Markdown rejection from the active retry through the plain-content fallback.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the fallback classification or reply-reference handling is wrong, the code can send an unintended proactive or duplicate message, or retry with an invalid reference; those messages are externally delivered and cannot be undone by reverting the change. The impact is bounded to affected sends, but correcting the code will not retract messages already sent.
Blocking findings: astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py:711
|
The sourcery finding was real. Traced it: a passive send failing for a non-markdown reason (expired msg_id) drops into the active retry with the markdown payload still intact, and when that retry is rejected for markdown the error propagated before any plain-content downgrade, so the message was lost. Fixed in 0d456f6: the active retry's markdown rejection now routes through the same plain-content downgrade (markdown popped, msg_type 2 to 0, stream newline fix kept). The reference retry order is unchanged, so a valid message_reference still survives. Added a regression test that pins the three-call sequence (passive fail, active markdown rejection, downgraded send) and verified it goes red without the fix. |
When the QQ official API refuses a native markdown payload, the fallback used to drop msg_id and resend through the active-message interface, which runs into the 304049 active-message rate limit and loses the reply threading. Reorder the fallback so a rejected markdown degrades to a plain content reply that keeps msg_id (passive reply, msg_type reset from 2 to 0, stream trailing newline restored). The stream newline repair retry also keeps msg_id. Only errors that are not markdown rejections take the active-send fallback, and if the active retry itself hits the markdown rejection (the passive failure was unrelated, e.g. an expired msg_id) it downgrades to plain content too instead of losing the message. Fallback raises are chained so the original rejection stays visible in logs.
0d456f6 to
c0321a2
Compare
|
重建说明：这个 PR 原本叠在 #10401 的两个提交上。#10489 已把出站 message_reference 的填充合进 master（595f990b6），走的是 msg_idx 语义，比 #10401 的裸 id 透传更正确，所以叠放部分整个摘掉，改为在 master 上重放本修复，diff 现在只剩 fallback 重排和 4 个测试。此前那个 follow-up（主动重试也撞 markdown 拒绝时同样降级纯 content）也一并带上了。 验证：tests/test_qqofficial_group_message_create.py 41/41 过；拿 master 版 helper 跑 markdown-rejected 和 stream-newline 两个用例定败（负对照），本分支定过。 |
Fixes #10420.
When the QQ official API refuses a native markdown payload, the current fallback in
_send_with_markdown_fallbackdropsmsg_idand resends through the active-message interface. Active messages are quota limited (304049), so a rejected markdown turns an ordinary reply into a rate-limit error and the message is lost.This branch is stacked on #10401 because both PRs touch
_send_with_markdown_fallback; only the tip commit is new.Modifications / 改动点
qqofficial_message_event.py: reorder the fallback so a rejected markdown degrades to a plaincontentreply that keepsmsg_id, staying a passive reply (msg_type reset from 2 to 0, stream trailing newline restored before the retry). The stream newline repair retry also keepsmsg_id. Only errors that are not markdown rejections go to the active-send fallback, which keeps the message_reference two-stage retry from #10401. Fallback raises are chained so the original rejection stays visible in the logs.test_qqofficial_group_message_create.py: three new tests covering the markdown-rejected passive path, the non-markdown active fallback, and the stream newline retry.Screenshots or Test Results / 运行截图或测试结果
pytest tests/test_qqofficial_group_message_create.pygives 50 passed. Red/green verified: with the source change stashed, the two behavior tests fail while the control test passes; with the change restored, all 50 pass.ruff checkandruff format --checkare clean on ruff 0.15.22, the same pin CI uses.Checklist / 检查清单
Summary by Sourcery
Keep QQ Official Markdown fallback replies passive and reliably deliver messages when Markdown payloads are rejected.
Bug Fixes:
Tests: