-
Notifications
You must be signed in to change notification settings - Fork 479
[feat] Introduce partial results as part of progress notifications #383
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?
[feat] Introduce partial results as part of progress notifications #383
Conversation
"chunk": { | ||
"additionalProperties": {}, | ||
"description": "The partial result data chunk.", | ||
"type": "object" | ||
}, |
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.
question:
- there is nothing technically wrong with modeling this as a
dict[str, Any]
, but this is not howCallToolResult
models 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)
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.
1/ It was from perspective that progress is not tool specific, even server to client request can have a progress
2/ Yeah also totally good option, only reason is chunk intuitively gives a sense of something incomplete.
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 dont think the objects in content of CallToolResult are like tied to CallTool - they are generic content types. They're useful for consumer to be able to write logic against the various response modalities. If we return arbitrary JSON then we almost need a new schema for this. Also you could include DataContent from this PR: https://github.com/modelcontextprotocol/modelcontextprotocol/pull/356/files to achieve the JSON part too.
- IMO chunk is kind of specific to LLM nomenclature. I would suggest something more generic either borrowing form CallToolResult "content" or something like "partialResult"
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.
- Whole RPC result object can be in chunk/content, each result can have different structure. So intention of modeling it like dict is to model it like https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-03-26/schema.ts#L61
a. Headsup I think that PR got abandoned and RFC: addTool.outputSchema
andCallToolResult.structuredContent
#371 is merged. - Notice key just outside of this one is also called partialResult. But not tied to chunk, content is also fine.
@@ -844,6 +844,10 @@ | |||
"properties": { | |||
"_meta": { | |||
"properties": { | |||
"partialResults": { |
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.
NIT: I feel that renaming this streamPartialResults or notifyPartialResults would be more apt than adding a boolean value under partialResults.
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.
yes that makes sense, I can update
How should this be handled by clients when not all results are delivered, or they aren't all received in the order they were sent? As I've noted in #430, notifications aren't guaranteed to be received in any particular order, as there's no way for the client to acknowledge receipt (also noted in the JSON-RPC spec). |
The progress number in the message increments monotonically, making it possible to recognize out of order delivery. However progress doesn’t need to have a predictable step, so recognizing missed result chunks is a problem. It’s probably best to introduce a partial result ID that must increment monotonically by 1 when present. If out of order chunk is received, the client spec could be to disconnect and reconnect with resumption token. Unidirectional SSE is constraining in this regard… however even sync l task results don’t have a delivery receipt guarantee. |
Would reconnection be sufficient here? The messages would have already been sent successfully, so the server would have no reason to buffer them anymore. It'd make sense for the client to hold an ordered buffer and reorder messages correctly according to the server-provided ID instead of forcing in-order receipt, and use a "task complete" notification with the final ID as a cutoff. However, that still wouldn't handle the case where receipt doesn't happen at all, though. Not having any form of client acknowledgement is a fundamental flaw if partial results are intended to work as an alternative to receiving a response all at once. It'd work if partial results were represented with server->client requests instead of notifications, but that then introduces much more synchronization into the process, since the server needs to wait for an ack before sending the next part (it's also unintuitive, but setting that aside). I almost wonder if it'd be better to do this by implementing something like ranges as an optional parameter on resources, and use progress as a way of telling the client where it can read from 🤔 That way this can be best-effort and tolerant of dropped messages (e.g. client holds its last successful read position, reads to the server-provided latest byte on progress notifications - if the operation completes, just read to the end). |
I might have missed it, but why make partial results part of progress notifications? Why not have separate notifications for each? One feature that I would like to see is mergeable partial results. For example, if you have a partial result with
Do we think
Since the server will send an empty JSON-RPC response to cap things off, is this flag necessary? |
Added support for streaming partial results through progress notifications, allowing for incremental updates during long-running operations. This enables more responsive agent-based workflows where intermediate results can be shared while a top-level operation is still running. #111, #117, #314.
Motivation and Context
This enhancement addresses a key need identified in discussion #111 for "structured, formatted intermediate updates from server -> client, so a deep agent graph can provide information to the user even while a top-level tool is still being run."
Progress notifications were an ideal mechanism to extend for this purpose, as they already provide a communication channel during long-running operations. By adding support for partial results, we enable servers to stream incremental data to clients without waiting for operations to complete, creating a more responsive and interactive experience.
LSP's partial result progress has been referred for this implementation: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#partialResults, specially the recommendation around if server send partial results over progressNotification then final result should be empty.
How Has This Been Tested?
The implementation has been validated through:
Breaking Changes
None. This is a non-breaking change that adds new optional fields while maintaining backward compatibility with existing implementations. Clients that don't support partial results can simply ignore the new fields.
Types of changes
[x] New feature (non-breaking change which adds functionality)
[ ] Bug fix (non-breaking change which fixes an issue)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[x] Documentation update
Checklist
[x] I have read the MCP Documentation
[x] My code follows the repository's style guidelines
[x] New and existing tests pass locally
[x] I have added appropriate error handling
[x] I have added or updated documentation as needed
Additional context
The implementation adds three key components:
a. chunk: The partial result data (any JSON object)
b. append: Boolean flag indicating whether to append to previously received chunks
c. lastChunk: Boolean flag indicating the final chunk
Protocol
To request partial results, the client includes both a
progressToken
andpartialResults: true
in the request metadata:The server can then include partial results in progress notifications: