-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
修复多轮/引用识图串图:历史不再持久化原图,失败图片 URL 丢弃,请求图本地优先 #9308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -302,6 +302,72 @@ def get_checkpoint_id(message: Message | dict) -> str | None: | |
| return None | ||
|
|
||
|
|
||
|
|
||
| _IMAGE_HISTORY_PLACEHOLDER = "[Image]" | ||
|
|
||
|
|
||
| def _is_image_content_part(part: Any) -> bool: | ||
| if isinstance(part, ImageURLPart): | ||
| return True | ||
| if isinstance(part, dict) and part.get("type") == "image_url": | ||
| return True | ||
| return getattr(part, "type", None) == "image_url" | ||
|
|
||
|
|
||
| def _image_placeholder_part_dict() -> dict[str, Any]: | ||
| return {"type": "text", "text": _IMAGE_HISTORY_PLACEHOLDER} | ||
|
|
||
|
|
||
| def strip_images_from_content_parts(content: Any) -> Any: | ||
| """Replace image parts with a short text placeholder. | ||
|
|
||
| Used when persisting or reloading conversation history so previous-turn | ||
| image pixels / remote URLs do not keep leaking into later requests. | ||
| """ | ||
| if not isinstance(content, list): | ||
| return content | ||
|
|
||
| stripped: list[Any] = [] | ||
| previous_was_image_placeholder = False | ||
| for part in content: | ||
| if _is_image_content_part(part): | ||
| if previous_was_image_placeholder: | ||
| continue | ||
| if isinstance(part, ContentPart): | ||
| stripped.append(TextPart(text=_IMAGE_HISTORY_PLACEHOLDER)) | ||
| else: | ||
| stripped.append(_image_placeholder_part_dict()) | ||
| previous_was_image_placeholder = True | ||
| continue | ||
|
|
||
| previous_was_image_placeholder = False | ||
| stripped.append(part) | ||
| return stripped | ||
|
|
||
|
|
||
| def strip_images_from_history_message_dict(message: dict[str, Any]) -> dict[str, Any]: | ||
| """Return a shallow-copied history message dict without image payloads.""" | ||
| if not isinstance(message, dict): | ||
| return message | ||
| if is_checkpoint_message(message): | ||
| return message | ||
|
|
||
| content = message.get("content") | ||
| if not isinstance(content, list): | ||
| return message | ||
|
|
||
| new_message = dict(message) | ||
| new_message["content"] = strip_images_from_content_parts(content) | ||
| return new_message | ||
|
|
||
|
|
||
| def strip_images_from_history_messages(history: list[dict] | None) -> list[dict]: | ||
| """Strip image payloads from a full persisted history list.""" | ||
| if not history: | ||
| return [] | ||
| return [strip_images_from_history_message_dict(item) for item in history] | ||
|
|
||
|
|
||
| def strip_checkpoint_messages(history: list[dict]) -> list[dict]: | ||
| """Remove internal checkpoint messages from provider-facing history.""" | ||
| return [message for message in history if not is_checkpoint_message(message)] | ||
|
|
@@ -327,7 +393,8 @@ def _get_checkpoint_data(message: Message | dict) -> CheckpointData | None: | |
| def bind_checkpoint_messages(history: list[dict]) -> list[Message]: | ||
| """Load persisted history and bind checkpoint segments to prior messages.""" | ||
| messages: list[Message] = [] | ||
| for item in history: | ||
| for raw_item in history: | ||
| item = strip_images_from_history_message_dict(raw_item) | ||
| if is_checkpoint_message(item): | ||
| checkpoint = _get_checkpoint_data(item) | ||
| if checkpoint is not None and messages: | ||
|
|
@@ -348,10 +415,22 @@ def dump_messages_with_checkpoints(messages: list[Message]) -> list[dict]: | |
| for message in messages: | ||
| message_data = message.model_dump() | ||
| if isinstance(message.content, list): | ||
| persisted_parts: list[Any] = [] | ||
| previous_was_image_placeholder = False | ||
| for part in message.content: | ||
| if getattr(part, "_no_save", False): | ||
| continue | ||
| if _is_image_content_part(part): | ||
| if previous_was_image_placeholder: | ||
| continue | ||
| persisted_parts.append(TextPart(text=_IMAGE_HISTORY_PLACEHOLDER)) | ||
| previous_was_image_placeholder = True | ||
| continue | ||
| previous_was_image_placeholder = False | ||
| persisted_parts.append(part) | ||
| message_data["content"] = [ | ||
| part.model_dump() | ||
| for part in message.content | ||
| if not getattr(part, "_no_save", False) | ||
| part.model_dump() if isinstance(part, ContentPart) else part | ||
| for part in persisted_parts | ||
| ] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 在 filtered_parts = [part for part in message.content if not getattr(part, "_no_save", False)]
persisted_parts = strip_images_from_content_parts(filtered_parts)
message_data["content"] = [
part.model_dump() if isinstance(part, ContentPart) else part
for part in persisted_parts
]References
|
||
| dumped.append(message_data) | ||
| if message._checkpoint_after is not None: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -115,6 +115,7 @@ | |||||||||
| extract_quoted_message_text, | ||||||||||
| ) | ||||||||||
| from astrbot.core.utils.string_utils import normalize_and_dedupe_strings | ||||||||||
| from astrbot.core.agent.message import strip_images_from_history_messages | ||||||||||
| from astrbot.core.workspace import ( | ||||||||||
| normalize_umo_for_workspace, | ||||||||||
| resolve_workspace_root_for_umo, | ||||||||||
|
|
@@ -1351,6 +1352,43 @@ def _select_image_chat_provider( | |||||||||
| return provider | ||||||||||
|
|
||||||||||
|
|
||||||||||
|
|
||||||||||
| def _is_remote_image_ref(image_ref: str) -> bool: | ||||||||||
| return image_ref.startswith("http://") or image_ref.startswith("https://") | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 为了更健壮地处理可能包含大写字母的 URL 协议头(例如
Suggested change
|
||||||||||
|
|
||||||||||
|
|
||||||||||
| def _finalize_request_image_urls( | ||||||||||
| image_urls: list[str] | None, | ||||||||||
| *, | ||||||||||
| max_total: int = 4, | ||||||||||
| ) -> list[str]: | ||||||||||
| """Dedupe and prioritize local image refs over remote URLs. | ||||||||||
|
|
||||||||||
| Remote CDN links (especially QQ multimedia URLs) expire easily. Keeping them | ||||||||||
| beside already-downloaded local paths causes models to "see" missing/wrong | ||||||||||
| images in later turns. | ||||||||||
| """ | ||||||||||
| urls = normalize_and_dedupe_strings(image_urls or []) | ||||||||||
| if not urls: | ||||||||||
| return [] | ||||||||||
|
|
||||||||||
| local_refs: list[str] = [] | ||||||||||
| embedded_refs: list[str] = [] | ||||||||||
| remote_refs: list[str] = [] | ||||||||||
| for ref in urls: | ||||||||||
| if _is_remote_image_ref(ref): | ||||||||||
| remote_refs.append(ref) | ||||||||||
| elif ref.startswith("data:") or ref.startswith("base64://"): | ||||||||||
| embedded_refs.append(ref) | ||||||||||
| else: | ||||||||||
| local_refs.append(ref) | ||||||||||
|
|
||||||||||
| ordered = local_refs + embedded_refs + remote_refs | ||||||||||
| if max_total > 0: | ||||||||||
| ordered = ordered[:max_total] | ||||||||||
| return ordered | ||||||||||
|
|
||||||||||
|
|
||||||||||
| async def build_main_agent( | ||||||||||
| *, | ||||||||||
| event: AstrMessageEvent, | ||||||||||
|
|
@@ -1381,7 +1419,7 @@ async def build_main_agent( | |||||||||
| "provider_request 必须是 ProviderRequest 类型。" | ||||||||||
| ) | ||||||||||
| if req.conversation: | ||||||||||
| req.contexts = json.loads(req.conversation.history) | ||||||||||
| req.contexts = strip_images_from_history_messages(json.loads(req.conversation.history)) | ||||||||||
| else: | ||||||||||
| req = ProviderRequest() | ||||||||||
| req.prompt = "" | ||||||||||
|
|
@@ -1514,7 +1552,7 @@ async def build_main_agent( | |||||||||
|
|
||||||||||
| conversation = await _get_session_conv(event, plugin_context) | ||||||||||
| req.conversation = conversation | ||||||||||
| req.contexts = json.loads(conversation.history) | ||||||||||
| req.contexts = strip_images_from_history_messages(json.loads(conversation.history)) | ||||||||||
| event.set_extra("provider_request", req) | ||||||||||
|
|
||||||||||
| if isinstance(req.contexts, str): | ||||||||||
|
sourcery-ai[bot] marked this conversation as resolved.
|
||||||||||
|
|
@@ -1530,7 +1568,10 @@ async def build_main_agent( | |||||||||
| ) | ||||||||||
| ) | ||||||||||
| ) | ||||||||||
| req.image_urls = normalize_and_dedupe_strings(req.image_urls) | ||||||||||
| req.image_urls = _finalize_request_image_urls( | ||||||||||
| normalize_and_dedupe_strings(req.image_urls), | ||||||||||
| max_total=max(int(getattr(config, "max_quoted_fallback_images", 4) or 4), 4), | ||||||||||
| ) | ||||||||||
| req.audio_urls = normalize_and_dedupe_strings(req.audio_urls) | ||||||||||
|
|
||||||||||
| if config.file_extract_enabled: | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,21 +273,37 @@ async def _transform_content_part(self, part: dict) -> dict: | |
| if part.get("type") == "image_url": | ||
| url, image_detail = self._extract_image_part_info(part) | ||
| if not url: | ||
| return part | ||
| return { | ||
| "type": "text", | ||
| "text": "[Image unavailable]", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Replacing failed image parts with a hardcoded text placeholder may drop useful metadata and is not easily configurable. On missing URL, this now replaces the whole Suggested implementation: if part.get("type") == "image_url":
url, image_detail = self._extract_image_part_info(part)
if not url:
# Build a text placeholder while preserving minimal context from the original part.
# Prefer explicit alt/description fields if available.
alt_text = (
part.get("alt")
or part.get("description")
or (part.get("metadata") or {}).get("alt")
or (part.get("metadata") or {}).get("description")
)
if alt_text:
placeholder_text = f"{IMAGE_UNAVAILABLE_PLACEHOLDER} (original description: {alt_text})"
else:
placeholder_text = IMAGE_UNAVAILABLE_PLACEHOLDER
return {
"type": "text",
"text": placeholder_text,
}
try:
resolved_part = await self._resolve_image_part(
)
except Exception as exc:
logger.warning(
"Image preprocess failed; dropping image content. url=%s err=%s",
url,
exc,
)To fully implement the centralization of the placeholder text:
|
||
| } | ||
|
|
||
| try: | ||
| resolved_part = await self._resolve_image_part( | ||
| url, image_detail=image_detail | ||
| ) | ||
| except Exception as exc: | ||
| logger.warning( | ||
| "图片 %s 预处理失败,将保留原始内容。错误: %s", | ||
| "Image preprocess failed; dropping image content. url=%s err=%s", | ||
| url, | ||
| exc, | ||
| ) | ||
| return part | ||
| return { | ||
| "type": "text", | ||
| "text": "[Image unavailable]", | ||
| } | ||
|
|
||
| return resolved_part or part | ||
| if resolved_part: | ||
| return resolved_part | ||
|
|
||
| logger.warning( | ||
| "Image preprocess returned empty; dropping image content. url=%s", | ||
| url, | ||
| ) | ||
| return { | ||
| "type": "text", | ||
| "text": "[Image unavailable]", | ||
| } | ||
|
|
||
| if part.get("type") == "audio_url": | ||
| audio_ref = self._extract_audio_part_info(part) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| from astrbot.core.agent.message import ( | ||
| ImageURLPart, | ||
| Message, | ||
| TextPart, | ||
| bind_checkpoint_messages, | ||
| dump_messages_with_checkpoints, | ||
| strip_images_from_history_messages, | ||
| ) | ||
| from astrbot.core.astr_main_agent import _finalize_request_image_urls | ||
|
|
||
|
|
||
| def test_dump_messages_strips_image_parts(): | ||
| msg = Message( | ||
| role="user", | ||
| content=[ | ||
| TextPart(text="see this"), | ||
| ImageURLPart(image_url=ImageURLPart.ImageURL(url="data:image/png;base64,AAAA")), | ||
| ImageURLPart(image_url=ImageURLPart.ImageURL(url="https://example.com/x.jpg")), | ||
| ], | ||
| ) | ||
| dumped = dump_messages_with_checkpoints([msg]) | ||
| content = dumped[0]["content"] | ||
| assert all(part.get("type") != "image_url" for part in content) | ||
| assert sum(1 for part in content if part.get("text") == "[Image]") == 1 | ||
|
|
||
|
|
||
| def test_bind_checkpoint_messages_strips_polluted_history(): | ||
| hist = [ | ||
| { | ||
| "role": "user", | ||
| "content": [ | ||
| {"type": "text", "text": "old"}, | ||
| {"type": "image_url", "image_url": {"url": "data:image/jpeg;base64,BBB"}}, | ||
| {"type": "image_url", "image_url": {"url": "https://example.com/a.jpg"}}, | ||
| ], | ||
| } | ||
| ] | ||
| bound = bind_checkpoint_messages(hist) | ||
| types = [getattr(part, "type", None) for part in bound[0].content] | ||
| assert "image_url" not in types | ||
| stripped = strip_images_from_history_messages(hist) | ||
| assert all(part.get("type") != "image_url" for part in stripped[0]["content"]) | ||
|
|
||
|
|
||
| def test_finalize_request_image_urls_prefers_local_and_limits(): | ||
| out = _finalize_request_image_urls( | ||
| [ | ||
| "https://example.com/a", | ||
| r"C:\\tmp\\a.jpg", | ||
| "https://example.com/a", | ||
| r"C:\\tmp\\b.jpg", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| "https://example.com/c.jpg", | ||
| ], | ||
| max_total=2, | ||
| ) | ||
| assert len(out) == 2 | ||
| assert all(not u.startswith("http") for u in out) | ||
Uh oh!
There was an error while loading. Please reload this page.