-
Notifications
You must be signed in to change notification settings - Fork 1k
Python: MCP Improvements: improved connection loss behavior, pagination for loading and a param to control representation #3154
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
Conversation
Python Test Coverage Report •
Python Unit Test Overview
|
|||||||||||||||||||||||||||||||||||
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.
Pull request overview
This PR introduces improvements to the MCP (Model Context Protocol) integration in the Python Agent Framework, addressing three key issues:
Key Changes:
- Adds configurable result parsing with
parse_tool_resultsandparse_prompt_resultsparameters (True/Callable/None) - Implements pagination support for
load_tools()andload_prompts()methods to handle MCP servers with large tool/prompt catalogs - Introduces proactive connection validation via
_is_connection_valid()and_ensure_connected()to handle connection loss gracefully
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| python/uv.lock | Minor dependency updates (huggingface-hub, litellm, litellm-proxy-extras) |
| python/packages/core/tests/core/test_tools.py | Adds comprehensive test for nested object arrays in JSON schema validation |
| python/packages/core/tests/core/test_mcp.py | Extensive pagination tests, duplicate handling tests, and connection loss recovery tests |
| python/packages/core/agent_framework/_mcp.py | Implements pagination loops, connection validation logic, and configurable result parsing in call_tool/get_prompt methods |
61cf8aa to
f1f458a
Compare
794d763 to
4c3e659
Compare
giles17
left a 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 believe this should also fix this issue? #2708
maybe, we can have them check after it merged and released. |
…on for loading and a param to control representation (microsoft#3154) * pagination support (microsoft#2848) added a parse_tool_result param and connection loss (microsoft#2884) * fix microsoft#3153 * improved connection handling * improved logic
Motivation and Context
Introduces a parameter for tools and prompt to configure how the results are parsed.
Three values:
Fixes #2884 by doing a more robust check before attempting to use, also returns the ClosedResourceError to the user, so it's easier to decide on recovery by the user.
Fixes #2848 by introducing paging support in the load_tools and load_prompts method
Fixes #3153 to further enhance json parsing
Closes #3013 by adding a test to test_tools that validates that case
Description
Contribution Checklist