Skip to content

feat: add partial results as part of progress notifications#669

Closed
cyccolin wants to merge 2 commits intomodelcontextprotocol:mainfrom
cyccolin:fastmcp-meta
Closed

feat: add partial results as part of progress notifications#669
cyccolin wants to merge 2 commits intomodelcontextprotocol:mainfrom
cyccolin:fastmcp-meta

Conversation

@cyccolin
Copy link
Copy Markdown

@cyccolin cyccolin commented May 8, 2025

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:

  1. Enables servers to stream intermediate results during execution of long-running operations, as specified in modelcontextprotocol/modelcontextprotocol#117
  2. Adds support for clients to send metadata during call_tool operations in the FastMCP implementation, as referenced in PR #631

Changes

  1. Add optional "meta" parameter to the ClientSession call_tool api to enable feature access from spec
  2. Add "partialResults" flag that allows clients to explicitly request streaming results.
  3. Add "partial_result" parameters in Server SendProgressNotification to enable notification with partial_result details.

How Has This Been Tested?

  1. Add unit tests for FastMCP reportProgress API.
  2. Manually test with custom client to receive partial results from long task server

Breaking Changes

No breaking changes as the new attributes and parameters are all optional.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

000-000-000-000-000
000-000-000-000-000 reviewed May 8, 2025
View reviewed changes
src/mcp/types.py
params: RequestParams | None = None

class PartialResult(BaseModel):
chunk: dict[str, Any]
Copy link
Copy Markdown

@000-000-000-000-000 000-000-000-000-000 May 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disregard below -- these points need to go on the spec PR. i will add them there.

question:

  1. there is nothing technically wrong with modeling this as a dict[str, Any], but this is not how CallToolResult models its content. Just curious if there is a specific reason not to keep same structure as CallToolResult? Intuitively i would expect them to be very similar.
  2. Also a nitpick on naming but should we call this chunk? why not "content" or something (similar to CallToolResult)

pantanurag555
pantanurag555 reviewed May 13, 2025
View reviewed changes
src/mcp/client/session.py
name: str,
arguments: dict[str, Any] | None = None,
read_timeout_seconds: timedelta | None = None,
**meta
Copy link
Copy Markdown

@pantanurag555 pantanurag555 May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

src/mcp/types.py
notifications. The receiver is not obligated to provide these notifications.
"""

partialResults: bool | None = None
Copy link
Copy Markdown

@pantanurag555 pantanurag555 May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@ihrpr
Copy link
Copy Markdown
Contributor

ihrpr commented May 23, 2025

Thank you for contributing to MCP Python SDK.

Closing this PR for now as the spec changes haven't been approved yet and there were changes in progress notification in SDK to include message.

Please re-open when the spec is approved or move to draft.

@ihrpr ihrpr closed this May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@000-000-000-000-000 000-000-000-000-000 000-000-000-000-000 left review comments

+1 more reviewer

@pantanurag555 pantanurag555 pantanurag555 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cyccolin @ihrpr @pantanurag555 @000-000-000-000-000