diff --git a/src/fastmcp/tools/tool.py b/src/fastmcp/tools/tool.py index 484bb2615..d9c74b5d8 100644 --- a/src/fastmcp/tools/tool.py +++ b/src/fastmcp/tools/tool.py @@ -502,26 +502,59 @@ def _convert_to_content( # if the result is a list, then it could either be a list of MCP types, # or a "regular" list that the tool is returning, or a mix of both. # - # so we extract all the MCP types / images and convert them as individual content elements, - # and aggregate the rest as a single content element + # Group adjacent non-MCP types together while preserving order - mcp_types = [] - other_content = [] + content_items = [] + non_mcp_batch = [] + + def flush_non_mcp_batch(): + """Convert accumulated non-MCP items to a single TextContent block.""" + if non_mcp_batch: + if len(non_mcp_batch) == 1: + # Single item - convert directly to avoid combining when not needed + content_items.extend( + _convert_to_content( + non_mcp_batch[0], + serializer=serializer, + _process_as_single_item=True, + ) + ) + else: + # Multiple items - combine into a single text block + combined_text = "" + for item in non_mcp_batch: + if isinstance(item, str): + combined_text += item + else: + if serializer is None: + combined_text += default_serializer(item) + else: + try: + combined_text += serializer(item) + except Exception as e: + logger.warning( + "Error serializing tool result: %s", + e, + exc_info=True, + ) + combined_text += default_serializer(item) + content_items.append(TextContent(type="text", text=combined_text)) + non_mcp_batch.clear() for item in result: if isinstance(item, ContentBlock | Image | Audio | File): - mcp_types.append(_convert_to_content(item)[0]) + # Flush any accumulated non-MCP items first + flush_non_mcp_batch() + # Add the MCP item + content_items.extend(_convert_to_content(item)) else: - other_content.append(item) + # Accumulate non-MCP items + non_mcp_batch.append(item) - if other_content: - other_content = _convert_to_content( - other_content, - serializer=serializer, - _process_as_single_item=True, - ) + # Flush any remaining non-MCP items + flush_non_mcp_batch() - return other_content + mcp_types + return content_items if not isinstance(result, str): if serializer is None: diff --git a/tests/server/test_server_interactions.py b/tests/server/test_server_interactions.py index 370210d61..501ad418d 100644 --- a/tests/server/test_server_interactions.py +++ b/tests/server/test_server_interactions.py @@ -185,7 +185,9 @@ class TestTools: async def test_tool_returns_list(self, tool_server: FastMCP): async with Client(tool_server) as client: result = await client.call_tool("list_tool", {}) - assert result.content[0].text == '["x",2]' # type: ignore[attr-defined] + # Adjacent non-MCP list items are combined into single content block + assert len(result.content) == 1 + assert result.content[0].text == "x2" # type: ignore[attr-defined] assert result.data == ["x", 2] async def test_file_text_tool(self, tool_server: FastMCP): @@ -426,7 +428,7 @@ class TestToolReturnTypes: self, tool_server: FastMCP, tmp_path: Path ): """Test that lists containing Image objects and other types are handled - correctly. Note that the non-MCP content will be grouped together.""" + correctly. Items now preserve their original order.""" # Create a test image image_path = tmp_path / "test.png" image_path.write_bytes(b"test image data") @@ -435,26 +437,30 @@ class TestToolReturnTypes: result = await client.call_tool( "mixed_list_fn", {"image_path": str(image_path)} ) - assert len(result.content) == 3 - # Check text conversion + assert len(result.content) == 4 # Now each item is separate + # Check text message (first item) content1 = result.content[0] assert isinstance(content1, TextContent) - assert json.loads(content1.text) == ["text message", {"key": "value"}] - # Check image conversion + assert content1.text == "text message" + # Check image conversion (second item) content2 = result.content[1] assert isinstance(content2, ImageContent) assert content2.mimeType == "image/png" assert base64.b64decode(content2.data) == b"test image data" - # Check direct TextContent + # Check dict content (third item) content3 = result.content[2] assert isinstance(content3, TextContent) - assert content3.text == "direct content" + assert json.loads(content3.text) == {"key": "value"} + # Check direct TextContent (fourth item) + content4 = result.content[3] + assert isinstance(content4, TextContent) + assert content4.text == "direct content" async def test_tool_mixed_list_with_audio( self, tool_server: FastMCP, tmp_path: Path ): """Test that lists containing Audio objects and other types are handled - correctly. Note that the non-MCP content will be grouped together.""" + correctly. Items now preserve their original order.""" # Create a test audio file audio_path = tmp_path / "test.wav" audio_path.write_bytes(b"test audio data") @@ -463,26 +469,30 @@ class TestToolReturnTypes: result = await client.call_tool( "mixed_audio_list_fn", {"audio_path": str(audio_path)} ) - assert len(result.content) == 3 - # Check text conversion + assert len(result.content) == 4 # Now each item is separate + # Check text message (first item) content1 = result.content[0] assert isinstance(content1, TextContent) - assert json.loads(content1.text) == ["text message", {"key": "value"}] - # Check audio conversion + assert content1.text == "text message" + # Check audio conversion (second item) content2 = result.content[1] assert isinstance(content2, AudioContent) assert content2.mimeType == "audio/wav" assert base64.b64decode(content2.data) == b"test audio data" - # Check direct TextContent + # Check dict content (third item) content3 = result.content[2] assert isinstance(content3, TextContent) - assert content3.text == "direct content" + assert json.loads(content3.text) == {"key": "value"} + # Check direct TextContent (fourth item) + content4 = result.content[3] + assert isinstance(content4, TextContent) + assert content4.text == "direct content" async def test_tool_mixed_list_with_file( self, tool_server: FastMCP, tmp_path: Path ): """Test that lists containing File objects and other types are handled - correctly. Note that the non-MCP content will be grouped together.""" + correctly. Items now preserve their original order.""" # Create a test file file_path = tmp_path / "test.bin" file_path.write_bytes(b"test file data") @@ -491,12 +501,12 @@ class TestToolReturnTypes: result = await client.call_tool( "mixed_file_list_fn", {"file_path": str(file_path)} ) - assert len(result.content) == 3 - # Check text conversion + assert len(result.content) == 4 # Now each item is separate + # Check text message (first item) content1 = result.content[0] assert isinstance(content1, TextContent) - assert json.loads(content1.text) == ["text message", {"key": "value"}] - # Check file conversion + assert content1.text == "text message" + # Check file conversion (second item) content2 = result.content[1] assert isinstance(content2, EmbeddedResource) assert content2.type == "resource" @@ -505,10 +515,14 @@ class TestToolReturnTypes: assert hasattr(resource, "blob") blob_data = getattr(resource, "blob") assert base64.b64decode(blob_data) == b"test file data" - # Check direct TextContent + # Check dict content (third item) content3 = result.content[2] assert isinstance(content3, TextContent) - assert content3.text == "direct content" + assert json.loads(content3.text) == {"key": "value"} + # Check direct TextContent (fourth item) + content4 = result.content[3] + assert isinstance(content4, TextContent) + assert content4.text == "direct content" class TestToolParameters: diff --git a/tests/test_examples.py b/tests/test_examples.py index 62fe38f28..19ba99cfe 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -24,8 +24,9 @@ async def test_complex_inputs(): result = await client.call_tool_mcp( "name_shrimp", {"tank": tank, "extra_names": ["charlie"]} ) + # Adjacent non-MCP list items are combined into single content assert len(result.content) == 1 - assert result.content[0].text == '["bob","alice","charlie"]' # type: ignore[attr-defined] + assert result.content[0].text == "bobalicecharlie" # type: ignore[attr-defined] async def test_desktop(monkeypatch): diff --git a/tests/tools/test_tool.py b/tests/tools/test_tool.py index 7f8f2077e..778d6ca07 100644 --- a/tests/tools/test_tool.py +++ b/tests/tools/test_tool.py @@ -1056,12 +1056,12 @@ class TestConvertResultToContent: assert result[0].text == '{"a":1,"b":2}' def test_list_of_basic_types(self): - """Test that a list of basic types is converted to a single TextContent.""" + """Test that a list of basic types is converted to a single combined TextContent item.""" result = _convert_to_content([1, "two", {"c": 3}]) assert isinstance(result, list) - assert len(result) == 1 + assert len(result) == 1 # Adjacent non-MCP types are combined assert isinstance(result[0], TextContent) - assert result[0].text == '[1,"two",{"c":3}]' + assert result[0].text == '1two{"c":3}' # All adjacent basic types combined def test_list_of_mcp_types(self): """Test that a list of MCP types is returned as a list of those types.""" @@ -1091,11 +1091,60 @@ class TestConvertResultToContent: assert text_content_count == 2 assert image_content_count == 1 - text_item = next(item for item in result if isinstance(item, TextContent)) - assert text_item.text == '[{"a":1}]' + # Verify order is preserved: text, image, text + assert isinstance(result[0], TextContent) + assert result[0].text == "hello" + assert isinstance(result[1], ImageContent) + assert result[1].data == "ZmFrZWltYWdlZGF0YQ==" + assert isinstance(result[2], TextContent) + assert result[2].text == '{"a":1}' - image_item = next(item for item in result if isinstance(item, ImageContent)) - assert image_item.data == "ZmFrZWltYWdlZGF0YQ==" + def test_mixed_text_and_images_preserve_order(self): + """Test that mixed text and images preserve their original order (GitHub issue #1656).""" + img_obj1 = Image(data=b"imagedata1") + img_obj2 = Image(data=b"imagedata2") + + # Test the exact pattern from the issue: [text1, img1, text2, img2] + result = _convert_to_content(["text1", img_obj1, "text2", img_obj2]) + + assert isinstance(result, list) + assert len(result) == 4 + + # Verify exact order is preserved + assert isinstance(result[0], TextContent) + assert result[0].text == "text1" + + assert isinstance(result[1], ImageContent) + assert result[1].data == "aW1hZ2VkYXRhMQ==" # base64 of "imagedata1" + + assert isinstance(result[2], TextContent) + assert result[2].text == "text2" + + assert isinstance(result[3], ImageContent) + assert result[3].data == "aW1hZ2VkYXRhMg==" # base64 of "imagedata2" + + def test_adjacent_non_mcp_types_combined(self): + """Test strawgate's example: image, 'x', 'y', image should be 2 image blocks and 1 content block.""" + img_obj1 = Image(data=b"imagedata1") + img_obj2 = Image(data=b"imagedata2") + + # Test the exact pattern from strawgate's request: [image, 'x', 'y', image] + result = _convert_to_content([img_obj1, "x", "y", img_obj2]) + + assert isinstance(result, list) + assert len(result) == 3 # 2 image blocks + 1 combined text block + + # First image + assert isinstance(result[0], ImageContent) + assert result[0].data == "aW1hZ2VkYXRhMQ==" # base64 of "imagedata1" + + # Combined text content for adjacent 'x' and 'y' + assert isinstance(result[1], TextContent) + assert result[1].text == "xy" # Adjacent strings combined + + # Second image + assert isinstance(result[2], ImageContent) + assert result[2].data == "aW1hZ2VkYXRhMg==" # base64 of "imagedata2" def test_list_of_mixed_types_list(self): """Test that a list of mixed types, including a list as one of the elements, is converted correctly.""" @@ -1105,19 +1154,23 @@ class TestConvertResultToContent: result = _convert_to_content([content1, image_obj, basic_data]) assert isinstance(result, list) - assert len(result) == 3 + assert ( + len(result) == 3 + ) # Adjacent non-MCP types are combined: hello (TextContent) + image + basic_data (single TextContent) text_content_count = sum(isinstance(item, TextContent) for item in result) image_content_count = sum(isinstance(item, ImageContent) for item in result) - assert text_content_count == 2 + assert text_content_count == 2 # hello + serialized basic_data assert image_content_count == 1 - text_item = next(item for item in result if isinstance(item, TextContent)) - assert text_item.text == '[[{"a":1},{"b":2}]]' - - image_item = next(item for item in result if isinstance(item, ImageContent)) - assert image_item.data == "ZmFrZWltYWdlZGF0YQ==" + # Verify order: hello, image, serialized basic_data + assert isinstance(result[0], TextContent) + assert result[0].text == "hello" + assert isinstance(result[1], ImageContent) + assert result[1].data == "ZmFrZWltYWdlZGF0YQ==" + assert isinstance(result[2], TextContent) + assert result[2].text == '[{"a":1},{"b":2}]' # basic_data serialized as JSON def test_list_of_mixed_types_with_audio(self): """Test that a list of mixed types including Audio is converted correctly.""" @@ -1135,11 +1188,13 @@ class TestConvertResultToContent: assert text_content_count == 2 assert audio_content_count == 1 - text_item = next(item for item in result if isinstance(item, TextContent)) - assert text_item.text == '[{"a":1}]' - - audio_item = next(item for item in result if isinstance(item, AudioContent)) - assert audio_item.data == "ZmFrZWF1ZGlvZGF0YQ==" + # Verify order is preserved: text, audio, text + assert isinstance(result[0], TextContent) + assert result[0].text == "hello" + assert isinstance(result[1], AudioContent) + assert result[1].data == "ZmFrZWF1ZGlvZGF0YQ==" + assert isinstance(result[2], TextContent) + assert result[2].text == '{"a":1}' def test_list_of_mixed_types_with_file(self): """Test that a list of mixed types including File is converted correctly.""" @@ -1160,14 +1215,15 @@ class TestConvertResultToContent: assert text_content_count == 2 assert embedded_content_count == 1 - text_item = next(item for item in result if isinstance(item, TextContent)) - assert text_item.text == '[{"a":1}]' + # Verify order is preserved: text, file, text + assert isinstance(result[0], TextContent) + assert result[0].text == "hello" + assert isinstance(result[1], EmbeddedResource) + assert result[1].type == "resource" + assert isinstance(result[2], TextContent) + assert result[2].text == '{"a":1}' - embedded_item = next( - item - for item in result - if isinstance(item, EmbeddedResource) and item.type == "resource" - ) + embedded_item = result[1] resource = embedded_item.resource assert resource.mimeType == "application/octet-stream" # Check for blob attribute and its value @@ -1239,28 +1295,28 @@ class TestConvertResultToContent: ] def test_single_element_list_preserves_structure(self): - """Test that single-element lists preserve their list structure.""" + """Test that single-element lists are converted to individual TextContent items.""" - # Test with a single integer + # Test with a single integer - now returns the integer as individual content result = _convert_to_content([1]) assert isinstance(result, list) assert len(result) == 1 assert isinstance(result[0], TextContent) - assert result[0].text == "[1]" # Should be "[1]", not "1" + assert result[0].text == "1" # Individual item, not wrapped in list - # Test with a single string + # Test with a single string - now returns the string as individual content result = _convert_to_content(["hello"]) assert isinstance(result, list) assert len(result) == 1 assert isinstance(result[0], TextContent) - assert result[0].text == '["hello"]' # Should be ["hello"], not "hello" + assert result[0].text == "hello" # Individual string, not wrapped in list - # Test with a single dict + # Test with a single dict - now returns the dict as individual content result = _convert_to_content([{"a": 1}]) assert isinstance(result, list) assert len(result) == 1 assert isinstance(result[0], TextContent) - assert result[0].text == '[{"a":1}]' # Should be wrapped in a list + assert result[0].text == '{"a":1}' # Individual dict, not wrapped in list class TestAutomaticStructuredContent: @@ -1385,9 +1441,10 @@ class TestAutomaticStructuredContent: result = await tool.run({}) - # Should only have content, no structured content + # Adjacent non-MCP types should be combined into single content block, no structured content assert len(result.content) == 1 - assert isinstance(result.content[0], TextContent) + assert all(isinstance(item, TextContent) for item in result.content) + assert result.content[0].text == "12345" # All numbers combined assert result.structured_content is None async def test_int_return_with_schema_creates_structured_content(self): diff --git a/tests/tools/test_tool_manager.py b/tests/tools/test_tool_manager.py index 3d74e82ae..1ab362a0d 100644 --- a/tests/tools/test_tool_manager.py +++ b/tests/tools/test_tool_manager.py @@ -569,7 +569,9 @@ class TestCallTools: }, ) - assert result.content[0].text == '["rex","gertrude"]' # type: ignore[attr-defined] + # Adjacent non-MCP list items are combined into single content block + assert len(result.content) == 1 + assert result.content[0].text == "rexgertrude" # type: ignore[attr-defined] assert result.structured_content == {"result": ["rex", "gertrude"]} async def test_call_tool_with_custom_serializer(self): @@ -611,9 +613,11 @@ class TestCallTools: ] result = await manager.call_tool("get_data", {}) + # Adjacent non-MCP list items get combined with custom serializer applied to each + assert len(result.content) == 1 assert ( result.content[0].text # type: ignore[attr-defined] - == 'CUSTOM:[{"key": "value", "number": 123}, {"key": "value2", "number": 456}]' # type: ignore[attr-defined] + == '{"key": "value", "number": 123}{"key": "value2", "number": 456}' # Adjacent items combined after individual serialization ) assert result.structured_content == { "result": [