-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: keep all CallToolResult content items #6149
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
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 |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import os | ||
| import sys | ||
| from types import SimpleNamespace | ||
| from unittest.mock import AsyncMock | ||
|
|
||
| import pytest | ||
|
|
@@ -90,6 +91,29 @@ async def generator(): | |
| return generator() | ||
|
|
||
|
|
||
| class MockMixedContentToolExecutor: | ||
| """模拟返回图片 + 文本的工具执行器""" | ||
|
|
||
| @classmethod | ||
| def execute(cls, tool, run_context, **tool_args): | ||
| async def generator(): | ||
| from mcp.types import CallToolResult, ImageContent, TextContent | ||
|
|
||
| result = CallToolResult( | ||
| content=[ | ||
| ImageContent( | ||
| type="image", | ||
| data="dGVzdA==", | ||
| mimeType="image/png", | ||
| ), | ||
| TextContent(type="text", text="直播间标题:新游首发:零~红蝶~"), | ||
| ] | ||
| ) | ||
| yield result | ||
|
|
||
| return generator() | ||
|
|
||
|
|
||
| class MockFailingProvider(MockProvider): | ||
| async def text_chat(self, **kwargs) -> LLMResponse: | ||
| self.call_count += 1 | ||
|
|
@@ -372,6 +396,66 @@ async def test_hooks_called_with_max_step( | |
| assert mock_hooks.tool_end_called, "on_tool_end应该被调用" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_tool_result_includes_all_calltoolresult_content( | ||
|
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 (testing): 增加一个该测试的变体,在 CallToolResult.content 为空时,验证“无内容”分支 由于现在存在一个 建议实现如下: async def text_chat(self, **kwargs) -> LLMResponse:
self.call_count += 1
assert mock_hooks.tool_end_called, "on_tool_end应该被调用"
@pytest.mark.asyncio
async def test_tool_result_handles_empty_calltoolresult_content(
runner, mock_provider, provider_request, mock_hooks, monkeypatch
):
"""当工具返回空 content 时,应走 'no content' 分支且不保存图片。"""
from astrbot.core.agent.tool_image_cache import tool_image_cache
from astrbot.core.provider import CallToolResult
mock_provider.should_call_tools = True
mock_provider.max_calls_before_normal_response = 1
saved_images = []
def fake_save_image(*args, **kwargs):
saved_images.append((args, kwargs))
# monkeypatch 掉图片保存函数,方便断言不会被调用
monkeypatch.setattr(tool_image_cache, "save_image", fake_save_image)
# 让 mock 工具返回空的 CallToolResult.content,以覆盖 `if not res.content` 分支
async def fake_tool_callable(*args, **kwargs):
return CallToolResult(content=[])
# 根据现有测试/MockProvider 约定,挂上假的工具实现
mock_provider.tool_callable = fake_tool_callable
result = await runner.run(provider_request)
# 断言走到了 "no content" 分支
# (如果实现使用了其它提示文案,请同步更新下面的断言)
assert "The tool returned no content." in result.msgs[-1].content[0].text
# 不应保存任何图片
assert saved_images == []
# 工具生命周期 Hook 仍然应该被调用
assert mock_hooks.tool_end_called, "on_tool_end应该被调用"
@pytest.mark.asyncio
async def test_tool_result_includes_all_calltoolresult_content(
runner, mock_provider, provider_request, mock_hooks, monkeypatch
):
Original comment in Englishsuggestion (testing): Add a variant of this test where CallToolResult.content is empty to verify the 'no content' path Since there is now an Suggested implementation: async def text_chat(self, **kwargs) -> LLMResponse:
self.call_count += 1
assert mock_hooks.tool_end_called, "on_tool_end应该被调用"
@pytest.mark.asyncio
async def test_tool_result_handles_empty_calltoolresult_content(
runner, mock_provider, provider_request, mock_hooks, monkeypatch
):
"""当工具返回空 content 时,应走 'no content' 分支且不保存图片。"""
from astrbot.core.agent.tool_image_cache import tool_image_cache
from astrbot.core.provider import CallToolResult
mock_provider.should_call_tools = True
mock_provider.max_calls_before_normal_response = 1
saved_images = []
def fake_save_image(*args, **kwargs):
saved_images.append((args, kwargs))
# monkeypatch 掉图片保存函数,方便断言不会被调用
monkeypatch.setattr(tool_image_cache, "save_image", fake_save_image)
# 让 mock 工具返回空的 CallToolResult.content,以覆盖 `if not res.content` 分支
async def fake_tool_callable(*args, **kwargs):
return CallToolResult(content=[])
# 根据现有测试/MockProvider 约定,挂上假的工具实现
mock_provider.tool_callable = fake_tool_callable
result = await runner.run(provider_request)
# 断言走到了 "no content" 分支
# (如果实现使用了其它提示文案,请同步更新下面的断言)
assert "The tool returned no content." in result.msgs[-1].content[0].text
# 不应保存任何图片
assert saved_images == []
# 工具生命周期 Hook 仍然应该被调用
assert mock_hooks.tool_end_called, "on_tool_end应该被调用"
@pytest.mark.asyncio
async def test_tool_result_includes_all_calltoolresult_content(
runner, mock_provider, provider_request, mock_hooks, monkeypatch
):
|
||
| runner, mock_provider, provider_request, mock_hooks, monkeypatch | ||
| ): | ||
| """工具返回多个 content 项时,tool result 应包含全部内容。""" | ||
|
|
||
| from astrbot.core.agent.tool_image_cache import tool_image_cache | ||
|
|
||
| mock_provider.should_call_tools = True | ||
| mock_provider.max_calls_before_normal_response = 1 | ||
|
|
||
| saved_images = [] | ||
|
|
||
| def fake_save_image( | ||
| base64_data, tool_call_id, tool_name, index=0, mime_type="image/png" | ||
| ): | ||
| saved_images.append( | ||
| { | ||
| "base64_data": base64_data, | ||
| "tool_call_id": tool_call_id, | ||
| "tool_name": tool_name, | ||
| "index": index, | ||
| "mime_type": mime_type, | ||
| } | ||
| ) | ||
| return SimpleNamespace(file_path=f"/tmp/{tool_call_id}_{index}.png") | ||
|
|
||
| monkeypatch.setattr(tool_image_cache, "save_image", fake_save_image) | ||
|
|
||
| await runner.reset( | ||
| provider=mock_provider, | ||
| request=provider_request, | ||
| run_context=ContextWrapper(context=None), | ||
| tool_executor=MockMixedContentToolExecutor, | ||
| agent_hooks=mock_hooks, | ||
| streaming=False, | ||
| ) | ||
|
|
||
| async for _ in runner.step_until_done(3): | ||
| pass | ||
|
|
||
| tool_messages = [ | ||
| m for m in runner.run_context.messages if getattr(m, "role", None) == "tool" | ||
| ] | ||
| assert len(tool_messages) == 1 | ||
|
|
||
| content = str(tool_messages[0].content) | ||
| assert "Image returned and cached at path='/tmp/call_123_0.png'." in content | ||
| assert "直播间标题:新游首发:零~红蝶~" in content | ||
| assert saved_images == [ | ||
| { | ||
| "base64_data": "dGVzdA==", | ||
| "tool_call_id": "call_123", | ||
| "tool_name": "test_tool", | ||
| "index": 0, | ||
| "mime_type": "image/png", | ||
| } | ||
| ] | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_fallback_provider_used_when_primary_raises( | ||
| runner, provider_request, mock_tool_executor, mock_hooks | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (testing): 考虑添加一个混合内容的 mock,以同时覆盖 EmbeddedResource 分支
MockMixedContentToolExecutor目前很好地覆盖了ImageContent + TextContent路径,但实现中对EmbeddedResource(TextResourceContents和类图片的BlobResourceContents)也有专门的处理。请扩展这个 mock(或新增一个),使其同时返回文本和图片类型的EmbeddedResource,从而在测试中覆盖这些分支,减少资源处理相关回归的可能性。建议实现如下:
EmbeddedResource/TextResourceContents/BlobResourceContents有专门的类或字段名(例如EmbeddedResource,TextResourceContent,BlobResourceContent等),请将上面使用的kind,uri,mime_type,text,data、以及type="embedded_resource"等字段替换为项目中实际使用的类型和字段。SimpleNamespace(role=..., content=[...])、或封装在更高层对象中),如果有差异,请将yield [...]部分调整为与现有 mock 的结构完全一致,以确保新的 EmbeddedResource 分支被现有测试路径正确消费。Original comment in English
suggestion (testing): Consider adding a mixed content mock that also exercises EmbeddedResource branches
MockMixedContentToolExecutorusefully covers theImageContent + TextContentpath, but the implementation also has specific handling forEmbeddedResource(TextResourceContentsand image-likeBlobResourceContents). Please extend this mock (or add another) to return both text and imageEmbeddedResourcevalues so those branches are exercised by tests and resource-handling regressions are less likely.Suggested implementation:
EmbeddedResource/TextResourceContents/BlobResourceContents有专门的类或字段名(例如EmbeddedResource,TextResourceContent,BlobResourceContent等),请将上面使用的kind,uri,mime_type,text,data、以及type="embedded_resource"等字段替换为项目中实际使用的类型和字段。SimpleNamespace(role=..., content=[...])、或封装在更高层对象中),如果有差异,请将yield [...]部分调整为与现有 mock 的结构完全一致,以确保新的 EmbeddedResource 分支被现有测试路径正确消费。