Python: Prefer MCP structuredContent over duplicate content - #7897
Shivani . (Shivani767) wants to merge 2 commits into
Conversation
When CallToolResult includes both content and structuredContent, return only the structured payload so agents are not charged for duplicated tokens from servers that echo the same result in both fields.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
There was a problem hiding this comment.
Pull request overview
Updates MCP tool-result parsing to avoid duplicated structured output while propagating server metadata.
Changes:
- Prefer
structuredContentover parallel content blocks. - Add regression tests for deduplication and metadata propagation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
python/packages/core/agent_framework/_mcp.py |
Changes MCP result parsing precedence. |
python/packages/core/tests/core/test_mcp.py |
Updates and adds parser regression tests. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| # each newly constructed Content; empty when the server provided no meta. | ||
| additional_kwargs: dict[str, Any] = {"additional_properties": {"_meta": meta}} if meta else {} | ||
|
|
||
| if mcp_type.structuredContent is not None: |
There was a problem hiding this comment.
Agreed — dropping all content was too aggressive.
Updated to emit structuredContent first, skip only text (or embedded text) that demonstrably echoes the structured payload, and keep complementary text plus images/audio/resources. Added a mixed image + structured regression test.
GitHub Actions
Inactive
Skip only text content that echoes structuredContent; keep images, audio, resources, and complementary summaries. Add a mixed-content regression test.
GitHub Actions
Inactive
GitHub Actions
Inactive
| with contextlib.suppress(json.JSONDecodeError, TypeError): | ||
| if json.loads(text) == structured: | ||
| return True |
There was a problem hiding this comment.
Could this comparison preserve JSON type identity? Python treats 1 == True, so content='{"approved": 1}' with structuredContent={"approved": true} reaches this branch and silently drops the numeric result even though they are distinct JSON documents. Comparing canonical JSON with type-aware values, or avoiding heuristic suppression through explicit modes, would prevent the data loss.
| if isinstance(structured, Mapping): | ||
| return any(_structured_content_contains_text(value, text) for value in structured.values()) | ||
| if isinstance(structured, Sequence) and not isinstance(structured, (str, bytes, bytearray)): | ||
| return any(_structured_content_contains_text(item, text) for item in structured) |
There was a problem hiding this comment.
Could we avoid rescanning the full structured tree for every text block? An MCP server controls both payloads, and this traversal runs once per content item, so a roughly 248 KB result with 10,000 leaves and 10,000 nonmatching text blocks performs 100 million comparisons and blocked the parser for about four seconds. Precomputing the string leaves once, or using a bounded selection policy, would keep one tool response from stalling the event loop.
| """Return whether a text content block is an echo of ``structuredContent``. | ||
|
|
||
| MCP servers often return the same payload as both a text ``content`` block and | ||
| ``structuredContent`` (for example ``{"result": "<same text>"}``). Treat those as | ||
| duplicates so agents are not charged twice. Complementary text (a human-readable | ||
| summary that is not present in the structured payload) is kept. | ||
| """ | ||
| if text == structured_json: | ||
| return True | ||
| with contextlib.suppress(json.JSONDecodeError, TypeError): | ||
| if json.loads(text) == structured: | ||
| return True | ||
| return _structured_content_contains_text(structured, text) |
There was a problem hiding this comment.
Should the selection behavior be caller-controlled instead of inferred globally from payload values? Every MCP caller now inherits three hidden rules: serialized JSON equality, Python decoded equality, and equality with any nested string, while parse_tool_results requires replacing the entire rich-content and metadata parser. A result_content_mode on MCPTool, forwarded by its transport constructors, could provide the structured-first, content-first, content-only, structured-only, and both policies discussed in #7866 without making callers reimplement parsing.
|
Re-open when wanting to move forward. No need to keep in a draft next time. |
|
Evan Mattson (@moonbox3) Sorry for the late response, and thanks for the detailed feedback. Could you please reopen this PR so I can address the review comments and push the required changes? I’ll update the implementation and add the requested regression tests. Thanks! |
|
Implemented in ab28f74. What changed
Test coverage
Validation |
Motivation & Context
MCP
CallToolResultmay include bothcontentandstructuredContent. Many servers (MS Learn, DeepWiki) echo an equivalent payload in both fields. Agent Framework previously appended both, so agents saw duplicated text and paid ~2x tokens.Fixes #7866
Note: Pavlo Natalenko (@Pavnat) reported this and shared a custom-parser workaround that prefers
structuredContent; there was no open PR when this was started. Happy to coordinate if preferred.Description & Review Guide
What are the major changes?
structuredContentfirst when present.contentblocks that demonstrably echo the structured payload (exact JSON match or string value present in the structured tree)._metaonto structured-contentContentitems.What is the impact of these changes?
contentis no longer dropped whenstructuredContentexists.parse_tool_results.What do you want reviewers to focus on?
structuredContentas an echo is the right equivalence check.Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.