fix: preserve group image context order - #9635
Open
PeiPei233 wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a concurrency ordering bug in the built-in GroupChatContext so that image captioning cannot be overtaken by later messages in the same group session, ensuring LLM requests see the correct preceding context.
Changes:
- Hold the per-session group context lock while formatting and appending records to prevent message reordering during image captioning.
- Preserve an explicit
[Image]placeholder when caption generation fails, instead of storing an empty sender/timestamp-only record. - Add async regression tests for image/text ordering and caption-failure placeholder behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| astrbot/builtin_stars/astrbot/group_chat_context.py | Makes message formatting+append atomic under the session lock; ensures caption failures still record an [Image] placeholder. |
| tests/unit/test_group_chat_context_wiring.py | Adds deterministic async tests covering ordering under concurrent events and placeholder behavior when captioning fails. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+147
to
+149
| # Caption and append atomically so later messages cannot overtake images. | ||
| async with self._get_lock(umo): | ||
| final_message = await self._format_message(event, cfg) |
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.
Fixes #9634.
Built-in group chat context formatting waits for image captioning before appending a message. Because the event bus processes incoming messages concurrently, a following text message could append first and trigger an LLM request without the preceding image context.
Modifications / 改动点
Hold the existing per-session group context lock while formatting and appending each message, preventing later messages in the same group session from overtaking an image caption request.
Keep an
[Image]placeholder when caption generation fails instead of storing an empty sender/timestamp record.Add deterministic async regression coverage for image/text ordering and caption failure fallback.
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Verification steps:
The concurrency regression test starts an image event, pauses its caption request, and then starts a text event for the same group session. It verifies that the text event cannot complete before the image caption and that the text-triggered LLM request receives the image record.
Checklist / 检查清单
Summary by Sourcery
Ensure group chat image messages are formatted and recorded atomically so subsequent text messages cannot overtake them in concurrent processing.
Bug Fixes:
Enhancements:
Tests: