{{ message }}
feat: add partial results as part of progress notifications - #669
Closed
cyccolin wants to merge 2 commits into
Closed
feat: add partial results as part of progress notifications#669cyccolin wants to merge 2 commits into
cyccolin wants to merge 2 commits into
Conversation
added 2 commits
May 8, 2025 13:56
There was a problem hiding this comment.
disregard below -- these points need to go on the spec PR. i will add them there.
question:
- there is nothing technically wrong with modeling this as a
dict[str, Any], but this is not howCallToolResultmodels its content. Just curious if there is a specific reason not to keep same structure asCallToolResult? Intuitively i would expect them to be very similar. - Also a nitpick on naming but should we call this chunk? why not "content" or something (similar to
CallToolResult)
| name: str, | ||
| arguments: dict[str, Any] | None = None, | ||
| read_timeout_seconds: timedelta | None = None, | ||
| **meta |
There was a problem hiding this comment.
Can we model this as an argument with Meta type rather than treating it as **kwargs? If the idea is not to put all extra keyword arguments, that don't match other call_tool arguments, in meta then we should use an argument with type Meta instead.
| notifications. The receiver is not obligated to provide these notifications. | ||
| """ | ||
|
|
||
| partialResults: bool | None = None |
There was a problem hiding this comment.
Might be a comment for the spec PR.
Nit: I wonder if streamPartialResults or notifyPartialResults would be more apt than adding a boolean value under partialResults?
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements the specifications defined in modelcontextprotocol/modelcontextprotocol#383, introducing partial result streaming capabilities within progress notifications from Server to Client.
Motivation and Context
This enhancement addresses two key improvements to the protocol:
Changes
call_toolapi to enable feature access from specHow Has This Been Tested?
Breaking Changes
No breaking changes as the new attributes and parameters are all optional.
Types of changes
Checklist
Additional context