Skip to content

Commit e6deb8e

Browse files
committed
refactor: improve structured content handling and code quality
- Change verbose info logging to debug level for tool responses to reduce production log noise while maintaining debug capability - Remove redundant isinstance check in plugin response handling - Add comprehensive docstring explaining structured content return types and MCP SDK behavior in call_tool function - Add test case for structured content validation with tuple returns - Improve code maintainability and documentation clarity These changes enhance code quality without altering functionality, improving pylint score from 9.64 to 9.77. Signed-off-by: Mihai Criveti <[email protected]>
1 parent 3ea179c commit e6deb8e

File tree

3 files changed

+68
-12
lines changed

3 files changed

+68
-12
lines changed

mcpgateway/services/tool_service.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,7 +1240,7 @@ async def invoke_tool(self, db: Session, name: str, arguments: Dict[str, Any], r
12401240
# Don't mark as successful for error responses - success remains False
12411241
else:
12421242
result = response.json()
1243-
logger.info(f" Rest API Tool response: {result}")
1243+
logger.debug(f"REST API tool response: {result}")
12441244
filtered_response = extract_using_jq(result, tool.jsonpath_filter)
12451245
tool_result = ToolResult(content=[TextContent(type="text", text=json.dumps(filtered_response, indent=2))])
12461246
success = True
@@ -1405,7 +1405,7 @@ async def connect_to_streamablehttp_server(server_url: str, headers: dict = head
14051405
elif transport == "streamablehttp":
14061406
tool_call_result = await connect_to_streamablehttp_server(tool_gateway.url, headers=headers)
14071407
dump = tool_call_result.model_dump(by_alias=True)
1408-
logger.info(f"Tool call result dump: {dump}")
1408+
logger.debug(f"Tool call result dump: {dump}")
14091409
content = dump.get("content", [])
14101410
# Accept both alias and pythonic names for structured content
14111411
structured = dump.get("structuredContent") or dump.get("structured_content")
@@ -1415,7 +1415,7 @@ async def connect_to_streamablehttp_server(server_url: str, headers: dict = head
14151415
if is_err is None:
14161416
is_err = getattr(tool_call_result, "isError", False)
14171417
tool_result = ToolResult(content=filtered_response, structured_content=structured, is_error=is_err, meta=getattr(tool_call_result, "meta", None))
1418-
logger.info(f"Final tool_result: {tool_result}")
1418+
logger.debug(f"Final tool_result: {tool_result}")
14191419
else:
14201420
tool_result = ToolResult(content=[TextContent(type="text", text="Invalid tool type")])
14211421

@@ -1435,14 +1435,9 @@ async def connect_to_streamablehttp_server(server_url: str, headers: dict = head
14351435
if isinstance(modified_result, dict) and "content" in modified_result:
14361436
# Safely obtain structured content using .get() to avoid KeyError when
14371437
# plugins provide only the content without structured content fields.
1438-
structured = None
1439-
if isinstance(modified_result, dict):
1440-
structured = modified_result.get("structuredContent") if "structuredContent" in modified_result else modified_result.get("structured_content")
1441-
1442-
tool_result = ToolResult(
1443-
content=modified_result["content"],
1444-
structured_content=structured
1445-
)
1438+
structured = modified_result.get("structuredContent") if "structuredContent" in modified_result else modified_result.get("structured_content")
1439+
1440+
tool_result = ToolResult(content=modified_result["content"], structured_content=structured)
14461441
else:
14471442
# If result is not in expected format, convert it to text content
14481443
tool_result = ToolResult(content=[TextContent(type="text", text=str(modified_result))])

mcpgateway/transports/streamablehttp_transport.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,12 +359,25 @@ async def call_tool(name: str, arguments: dict) -> List[Union[types.TextContent,
359359
"""
360360
Handles tool invocation via the MCP Server.
361361
362+
This function supports the MCP protocol's tool calling with structured content validation.
363+
It can return either unstructured content only, or both unstructured and structured content
364+
when the tool defines an outputSchema.
365+
362366
Args:
363367
name (str): The name of the tool to invoke.
364368
arguments (dict): A dictionary of arguments to pass to the tool.
365369
366370
Returns:
367-
List of content (TextContent, ImageContent, or EmbeddedResource) from the tool response.
371+
Union[List[ContentBlock], Tuple[List[ContentBlock], Dict[str, Any]]]:
372+
- If structured content is not present: Returns a list of content blocks
373+
(TextContent, ImageContent, or EmbeddedResource)
374+
- If structured content is present: Returns a tuple of (unstructured_content, structured_content)
375+
where structured_content is a dictionary that will be validated against the tool's outputSchema
376+
377+
The MCP SDK's call_tool decorator automatically handles both return types:
378+
- List return → CallToolResult with content only
379+
- Tuple return → CallToolResult with both content and structuredContent fields
380+
368381
Logs and returns an empty list on failure.
369382
370383
Examples:

tests/unit/mcpgateway/transports/test_streamablehttp_transport.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,54 @@ async def fake_get_db():
211211
assert result[0].text == "hello"
212212

213213

214+
@pytest.mark.asyncio
215+
async def test_call_tool_with_structured_content(monkeypatch):
216+
"""Test call_tool returns tuple with both unstructured and structured content."""
217+
# First-Party
218+
from mcpgateway.transports.streamablehttp_transport import call_tool, tool_service, types
219+
220+
mock_db = MagicMock()
221+
mock_result = MagicMock()
222+
mock_content = MagicMock()
223+
mock_content.type = "text"
224+
mock_content.text = '{"result": "success"}'
225+
mock_result.content = [mock_content]
226+
227+
# Simulate structured content being present
228+
mock_structured = {"status": "ok", "data": {"value": 42}}
229+
mock_result.structured_content = mock_structured
230+
mock_result.model_dump = lambda by_alias=True: {
231+
"content": [{"type": "text", "text": '{"result": "success"}'}],
232+
"structuredContent": mock_structured
233+
}
234+
235+
@asynccontextmanager
236+
async def fake_get_db():
237+
yield mock_db
238+
239+
monkeypatch.setattr("mcpgateway.transports.streamablehttp_transport.get_db", fake_get_db)
240+
monkeypatch.setattr(tool_service, "invoke_tool", AsyncMock(return_value=mock_result))
241+
242+
result = await call_tool("mytool", {"foo": "bar"})
243+
244+
# When structured content is present, result should be a tuple
245+
assert isinstance(result, tuple)
246+
assert len(result) == 2
247+
248+
# First element should be the unstructured content list
249+
unstructured, structured = result
250+
assert isinstance(unstructured, list)
251+
assert len(unstructured) == 1
252+
assert isinstance(unstructured[0], types.TextContent)
253+
assert unstructured[0].text == '{"result": "success"}'
254+
255+
# Second element should be the structured content dict
256+
assert isinstance(structured, dict)
257+
assert structured == mock_structured
258+
assert structured["status"] == "ok"
259+
assert structured["data"]["value"] == 42
260+
261+
214262
@pytest.mark.asyncio
215263
async def test_call_tool_no_content(monkeypatch, caplog):
216264
"""Test call_tool returns [] and logs warning if no content."""

0 commit comments

Comments
 (0)