-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Added a mechanism to extract metadata from MCP tool call response #3339
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
base: main
Are you sure you want to change the base?
Conversation
feat: Added a new MCP tool that attaches metadata to MCP TextContent. test: Added a test to call the aforementioned tool (failing as of now).
|
Hi @DouweM, quoting you from #3323 (comment), I am pondering over whether it is better to edit the At present, if you take a look at the |
todo: Exhaustive tests to improve coverage.
|
Quoting myself (#3339 (comment))
Note that I just did that @DouweM. Need to add more tests to improve coverage. |
| return structured | ||
| if isinstance(structured, dict) and ( | ||
| (len(structured) == 1 and 'result' in structured) | ||
| or (len(structured) == 2 and 'result' in structured and '_meta' in structured) |
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.
In the example at https://gofastmcp.com/servers/tools#toolresult-and-metadata, wouldn't the metadata be on result.meta? I don't think we should try to parse it directly from the result.structuredData
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.
I agree with you regarding not modifying the structured result.
We may be, though, not thinking of the same metadata.
What you are referring to is a FastMCP tool call result metadata. In addition, the meta is an attribute of FastMCP ToolResult only from version 2.13.1 (according to https://gofastmcp.com/servers/tools#toolresult-and-metadata) while the version currently in use with Pydantic AI is 2.12.4.
What I have been referring to is the _meta in the latest MCP standard https://modelcontextprotocol.io/specification/2025-06-18/basic/index#meta. FastMCP wraps this in TextContent and other types of content too.
If we go with the FastMCP-specific meta then there is a possibility that a MCP server implemented without using that specific version (> 2.13.1) of FastMCP or implemented in a different language will not return the metadata in the expected ToolResult style object.
Having said that, there is a possibility that FastMCP is implementing what the upcoming MCP standard will be, as they seem to typically do. (I haven't dug through this in details.)
In summary:
- for structured content, I think we could go with
metaofToolResultbut I need to upgrade FastMCP for Pydantic AI to 2.13.1 or above; - however, we ought to support
_metaaliasedmetafor each content type.
Regarding (1), is this something I should do by myself?
What are your thoughts?
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.
Actually, I noticed that in my original issue #3323, I had referred to both the FastMCP metadata and the standards _meta. Sorry for the confusion.
Ideally, we should support both.
| async def _map_tool_result_part( | ||
| self, part: mcp_types.ContentBlock | ||
| ) -> str | messages.BinaryContent | dict[str, Any] | list[Any]: | ||
| ) -> str | messages.ToolReturn | messages.BinaryContent | dict[str, Any] | list[Any]: |
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.
This is not going to work, because now the tool call could return a list of ToolReturns which is not supported: the tool needs to itself return a ToolReturn object.
I think we should build the list of output contents as we used to, and then if there's result.meta, return a ToolReturn with that metadata + the output content, instead of returning the output content directly
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.
Okay, but as I mentioned in my comment above, what I have been referring to is the _meta in the latest MCP standard https://modelcontextprotocol.io/specification/2025-06-18/basic/index#meta. This can be present in each content block, it seems.
If we return a single meta, there is no way to know how to merge multiple _meta that may be present in the content blocks.
| # See https://github.com/jlowin/fastmcp/blob/main/docs/servers/tools.mdx#return-values | ||
|
|
||
| # Let's also check for metadata but it can be present in not just TextContent | ||
| metadata: dict[str, Any] | None = part.meta |
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.
Do we want to get the metadata off of the ContentBlock, or from the CallToolResult itself? I was assuming this PR was going to be about tool call result metadata as in https://gofastmcp.com/servers/tools#toolresult-and-metadata, not about metadata on specific text/binary parts.
Note that we have a separate issue about embedded resource metadata: #2288 (comment).
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.
I see your point in #2288 (comment) -- the same applies to tool calls.
However, maybe, we still should not ignore the _meta in content blocks because maybe the Pydantic AI user has some use of them (other than passing these to the LLM), such as logging.
💡 That makes me think that maybe there is a need to support a user-specified MCP tool/feature call handler, using which, the user could figure out a way to handle tool call in a way they want while Pydantic AI's default handler will ignore _meta but support FastMCP ToolResult.meta.
What do you think @DouweM?
|
@anirbanbasu I'll respond here at the top level with some thoughts because the 3 threads touch on overlapping topics:
Let me know what you think. The fact that there can be metadata at multiple levels in MCP but not currently in Pydantic AI makes this tricky! |
Fixes: #3323
TextContentand todict[str, Any]as a_metakey.TextContent._metain content blocks; or both.