Skip to content

Commit fd24f00

Browse files
Mofefjulian-risch
andauthored
test: Extract method to extract text from mcp result (#3154)
* Reduce code duplicated between tool invoke methods * Add test for mcp result text extraction * Add docstring, fix type and fallback * Address reviewer comments * skip tests if OPENAI_API_KEY exists but is empty * skip tests if OPENAI_API_KEY exists but is empty --------- Co-authored-by: Julian Risch <julian.risch@deepset.ai>
1 parent 5684e43 commit fd24f00

File tree

3 files changed

+46
-34
lines changed

3 files changed

+46
-34
lines changed

integrations/mcp/src/haystack_integrations/tools/mcp/mcp_tool.py

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,27 @@ def _resolve_headers(headers: dict[str, str | Secret] | None) -> dict[str, str]
6363
return resolved_headers
6464

6565

66+
def _extract_first_text_element(tool_call_result: str) -> str | dict[str, Any]:
67+
"""
68+
Return the first text content block from an MCP tool call result.
69+
70+
MCP tool call results may include mixed content types such as text, image, or
71+
audio blocks. This helper extracts the first text block because the tool
72+
invoker expects a single parsed payload rather than the full content list.
73+
"""
74+
parsed: dict = json.loads(tool_call_result)
75+
content: list = parsed.get("content", [])
76+
for block in content:
77+
if isinstance(block, dict) and block.get("type") == "text":
78+
text = block.get("text", "")
79+
try:
80+
return json.loads(text)
81+
except (json.JSONDecodeError, TypeError):
82+
return text
83+
# No TextContent found, return full parsed response as fallback
84+
return parsed
85+
86+
6687
class AsyncExecutor:
6788
"""Thread-safe event loop executor for running async code from sync contexts."""
6889

@@ -1088,21 +1109,7 @@ async def invoke() -> Any:
10881109
# Parse JSON to dict only when outputs_to_state is configured.
10891110
# ToolInvoker requires dict for _merge_tool_outputs(); ToolCallResult.result expects str otherwise.
10901111
if self.outputs_to_state:
1091-
parsed = json.loads(result)
1092-
1093-
# Per MCP spec, content[] may contain TextContent, ImageContent, AudioContent, etc.
1094-
# Parse only first TextContent block (ToolInvoker requires dict, not list).
1095-
content = parsed.get("content", [])
1096-
for block in content:
1097-
if isinstance(block, dict) and block.get("type") == "text":
1098-
text = block.get("text", "")
1099-
try:
1100-
return json.loads(text)
1101-
except (json.JSONDecodeError, TypeError):
1102-
return text
1103-
1104-
# No TextContent found, return full parsed response as fallback
1105-
return parsed
1112+
return _extract_first_text_element(result)
11061113

11071114
return result
11081115
except (MCPError, TimeoutError) as e:
@@ -1133,21 +1140,7 @@ async def ainvoke(self, **kwargs: Any) -> str | dict[str, Any]:
11331140
# Parse JSON to dict only when outputs_to_state is configured.
11341141
# ToolInvoker requires dict for _merge_tool_outputs(); ToolCallResult.result expects str otherwise.
11351142
if self.outputs_to_state:
1136-
parsed = json.loads(result)
1137-
1138-
# Per MCP spec, content[] may contain TextContent, ImageContent, AudioContent, etc.
1139-
# Parse only first TextContent block (ToolInvoker requires dict, not list).
1140-
content = parsed.get("content", [])
1141-
for block in content:
1142-
if isinstance(block, dict) and block.get("type") == "text":
1143-
text = block.get("text", "")
1144-
try:
1145-
return json.loads(text)
1146-
except (json.JSONDecodeError, TypeError):
1147-
return text
1148-
1149-
# No TextContent found, return full parsed response as fallback
1150-
return parsed
1143+
return _extract_first_text_element(result)
11511144

11521145
return result
11531146
except asyncio.TimeoutError as e:

integrations/mcp/tests/test_mcp_tool.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
MCPTool,
1616
StdioServerInfo,
1717
)
18-
from haystack_integrations.tools.mcp.mcp_tool import StdioClient
18+
from haystack_integrations.tools.mcp.mcp_tool import StdioClient, _extract_first_text_element
1919

2020
from .mcp_memory_transport import InMemoryServerInfo
2121
from .mcp_servers_fixtures import calculator_mcp, echo_mcp
@@ -27,6 +27,25 @@ def simple_haystack_tool(name: str) -> str:
2727
return f"Hello, {name}!"
2828

2929

30+
# from https://modelcontextprotocol.io/specification/draft/server/tools#output-schema
31+
EXAMPLE_MCP_TOOL_CALL_RESULT = {
32+
"content": [{"type": "text", "text": '{"temperature": 22.5, "conditions": "Partly cloudy", "humidity": 65}'}],
33+
"structuredContent": {"temperature": 22.5, "conditions": "Partly cloudy", "humidity": 65},
34+
}
35+
36+
37+
def test_extract_first_text_element():
38+
"""Test that extract_first_text skips non-text blocks and parses the first text block."""
39+
tool_call_result = EXAMPLE_MCP_TOOL_CALL_RESULT
40+
tool_call_result["content"].insert(0, {"type": "image", "data": "ignored"})
41+
tool_call_result["content"].insert(1, {"type": "text", "text": '{"answer": 42}'}) # target
42+
tool_call_result = json.dumps(tool_call_result)
43+
44+
extracted = _extract_first_text_element(tool_call_result)
45+
46+
assert extracted == {"answer": 42}
47+
48+
3049
class TestMCPTool:
3150
"""Tests for the MCPTool class using in-memory servers."""
3251

@@ -236,7 +255,7 @@ async def test_stdio_client_stderr_handling(self, fileno_side_effect, fileno_ret
236255
else:
237256
assert errlog is mock_stderr
238257

239-
@pytest.mark.skipif("OPENAI_API_KEY" not in os.environ, reason="OPENAI_API_KEY not set")
258+
@pytest.mark.skipif(not os.environ.get("OPENAI_API_KEY"), reason="OPENAI_API_KEY not set")
240259
@pytest.mark.integration
241260
def test_pipeline_warmup_with_mcp_tool(self):
242261
"""Test lazy connection with Pipeline.warm_up() - replicates time_pipeline.py."""
@@ -259,7 +278,7 @@ def test_pipeline_warmup_with_mcp_tool(self):
259278
if tool:
260279
tool.close()
261280

262-
@pytest.mark.skipif("OPENAI_API_KEY" not in os.environ, reason="OPENAI_API_KEY not set")
281+
@pytest.mark.skipif(not os.environ.get("OPENAI_API_KEY"), reason="OPENAI_API_KEY not set")
263282
@pytest.mark.integration
264283
def test_agent_with_state_mapping(self):
265284
"""Test Agent with MCPTool using state-mapping to inject location from state."""

integrations/mcp/tests/test_mcp_toolset.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ async def test_toolset_no_state_config(self, calculator_toolset):
382382
assert tool.outputs_to_state is None
383383
assert tool.outputs_to_string is None
384384

385-
@pytest.mark.skipif("OPENAI_API_KEY" not in os.environ, reason="OPENAI_API_KEY not set")
385+
@pytest.mark.skipif(not os.environ.get("OPENAI_API_KEY"), reason="OPENAI_API_KEY not set")
386386
@pytest.mark.integration
387387
async def test_pipeline_warmup_with_mcp_toolset(self):
388388
"""Test lazy connection with Pipeline.warm_up() - replicates time_pipeline.py."""

0 commit comments

Comments
 (0)