fix(core): 主模型缺少图像模态时,由配置的转述模型进行内容转述 | caption tool-returned images when the main model lacks image modality - #9686
Open
SweetenedSuzuka wants to merge 5 commits into
Conversation
…ge modality (AstrBotDevs#9675) When the main chat model does not support image input, images returned by tools (e.g. via ImageContent in a CallToolResult) were silently dropped: the cached_images block only injected base64 image parts when the model supported the image modality, with no fallback. This adds an else branch that reuses the configured default_image_caption_provider_id to generate a text caption per cached image, mirroring astr_main_agent._ensure_img_caption(), and injects it as a user message so the model can still see the content.
… call (AstrBotDevs#9675) Caption all cached tool images with one _request_img_caption call per step instead of one call per image, matching astr_main_agent._ensure_img_caption(). Bounding the caption calls to one per image-returning step keeps long tool loops from being slowed by N serial caption round-trips when a tool returns many images. Add a test asserting N images trigger exactly one caption call.
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
_tool_image_runner_setup, avoid using a mutable default argument (modalities: list[str] | None = ["tool_use"]) and instead default toNoneand setmodalities = ["tool_use"]inside the function. - In
_caption_cached_tool_images, you always return at least oneTextPart, so theif caption_parts:guard before appending the message is redundant and could be removed to simplify the control flow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_tool_image_runner_setup`, avoid using a mutable default argument (`modalities: list[str] | None = ["tool_use"]`) and instead default to `None` and set `modalities = ["tool_use"]` inside the function.
- In `_caption_cached_tool_images`, you always return at least one `TextPart`, so the `if caption_parts:` guard before appending the message is redundant and could be removed to simplify the control flow.
## Individual Comments
### Comment 1
<location path="astrbot/core/agent/runners/tool_loop_agent_runner.py" line_range="1074" />
<code_context>
self.req.append_tool_calls_result(tool_calls_result)
+ async def _caption_cached_tool_images(
+ self, cached_images: list[CachedImage]
+ ) -> list[TextPart]:
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring `_caption_cached_tool_images` into smaller helpers that encapsulate config lookup and caption requests while reusing the existing captioning abstraction.
You can keep the functionality intact while reducing complexity and duplication by splitting `_caption_cached_tool_images` into smaller helpers and reusing the existing captioning abstraction.
### 1. Extract provider config resolution
Move the inline config handling into a small helper so `_caption_cached_tool_images` is mostly orchestration:
```python
# near the runner class, but outside methods
def _get_image_caption_config(run_context) -> tuple[str, dict]:
plugin_context = getattr(run_context.context, "context", None)
event = getattr(run_context.context, "event", None)
if plugin_context is None:
return "", {}
try:
cfg = plugin_context.get_config(
umo=getattr(event, "unified_msg_origin", None)
).get("provider_settings", {})
except Exception as exc: # noqa: BLE001
logger.debug("Failed to read provider settings: %s", exc)
return "", {}
return cfg.get("default_image_caption_provider_id") or "", cfg
```
Then `_caption_cached_tool_images` delegates to this:
```python
async def _caption_cached_tool_images(
self, cached_images: list[CachedImage]
) -> list[TextPart]:
img_cap_prov_id, cfg = _get_image_caption_config(self.run_context)
parts: list[TextPart] = [
TextPart(
text=(
f"[Image from tool '{cached_img.tool_name}', "
f"path='{cached_img.file_path}']"
)
)
for cached_img in cached_images
]
caption = await self._request_tool_image_captions(
img_cap_prov_id, cfg, cached_images
)
if caption:
parts.append(TextPart(text=f"<image_caption>{caption}</image_caption>"))
else:
parts.append(
TextPart(
text=(
"[Image not visible to the current model]"
if not img_cap_prov_id
else "[Image Captioning Failed]"
)
)
)
return parts
```
### 2. Isolate caption request and dynamic import
The dynamic import and path filtering can be moved into a dedicated helper, which also mirrors `astr_main_agent._ensure_img_caption` more explicitly:
```python
async def _request_tool_image_captions(
self,
img_cap_prov_id: str,
cfg: dict,
cached_images: list[CachedImage],
) -> str | None:
if not img_cap_prov_id:
return None
caption_paths = [
cached_img.file_path
for cached_img in cached_images
if Path(cached_img.file_path).exists()
]
if not caption_paths:
return None
try:
from astrbot.core.astr_main_agent import _request_img_caption
plugin_context = getattr(self.run_context.context, "context", None)
return await _request_img_caption(
img_cap_prov_id,
cfg,
caption_paths,
plugin_context,
)
except Exception as exc: # noqa: BLE001
logger.error("Failed to caption tool images: %s", exc)
return None
```
This keeps `_caption_cached_tool_images` focused on assembling `TextPart`s and makes the config/captioning behavior easier to reuse or further centralize later (e.g., moving `_get_image_caption_config` and `_request_tool_image_captions` into a shared image captioning utility).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…otDevs#9675) Address automated review feedback: extract config resolution and the caption request into dedicated helpers, drop the redundant `if caption_parts:` guard (the method always returns at least one TextPart), and avoid a mutable default argument in the test helper.
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.
摘要 / Summary
当默认对话模型不支持图片模态输入时,工具返回的
ImageContent(如view_image返回的图片)此前会被静默丢弃,模型完全看不到图片内容。本 PR 在 Agent 工具循环中补齐降级逻辑:主模型不支持图片时,调用已配置的默认图片转述模型生成文字描述并注入上下文,行为与用户消息路径的_ensure_img_caption()保持一致。When the default chat model does not support image input, images returned by tools (e.g. via
ImageContentin aCallToolResult) were silently dropped, so the model never saw them. This PR adds the missing fallback in the agent tool loop: when the main model lacks the image modality, cached tool images are captioned by the configureddefault_image_caption_provider_idand injected as text, mirroring_ensure_img_caption()on the user-message path.问题 / Problem
tool_loop_agent_runner.py的if cached_images:块只在主模型modalities含image时把图片 base64 追加到上下文;主模型不支持图片时图片被静默丢弃——既不传 base64,也不调用转述模型。The
if cached_images:block intool_loop_agent_runner.pyonly appends base64 image parts when the main model'smodalitiesincludeimage. When the main model does not support images, the returned images were silently dropped — neither base64 data nor a caption was injected.改动 / Changes
在
if cached_images:块新增非多模态分支,调用default_image_caption_provider_id配置的转述模型,为该步返回的缓存图片生成一段<image_caption>文字描述,并以 user 消息注入上下文;转述调用与
_ensure_img_caption()一致:每步一次调用处理全部缓存图片,避免按图逐个调用导致长工具循环被多次串行转述拖慢;未配置转述模型或转述失败时注入占位文本(
[Image not visible to the current model]/[Image Captioning Failed]),保证图片内容不被静默丢弃且工具循环不被中断;新增 4 个单元测试,覆盖转述成功注入、未配置占位、转述失败占位、多图单次调用。
Added a non-multimodal branch to the
if cached_images:block that captions all cached images for a step with one call to the configureddefault_image_caption_provider_idand injects the<image_caption>text as a user message.The caption call mirrors
_ensure_img_caption(): a single call per step for all cached images, bounding the added LLM round-trips so long tool loops are not slowed by N serial caption calls.Injects placeholder text when no caption provider is configured or captioning fails, so image content is never silently dropped and the tool loop is not interrupted.
Added 4 unit tests covering successful captioning, no-provider placeholder, caption failure, and the single-call bound for multiple images.
验证 / Verification
相关测试套件全部通过。
Related test suites pass.
Fixes #9675
Summary by Sourcery
Fallback to caption tool-returned images when the main model lacks image modality, ensuring image information is preserved in the agent tool loop.
New Features:
Enhancements:
Tests: