Repository navigation
fix: serialize tool outputs to JSON, remove trailing assistant messages, resolve Bedrock model IDs - #8144
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
json.dumps on arbitrary dict/list contents can raise TypeError, potentially breaking tool execution paths that should not fail due to result stringification/logging, and the new tests introduce strict type-checking violations via unparameterized dict/list annotations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes structured tool return-value serialization in autogen-core so dict/list outputs are emitted as valid JSON (via json.dumps) rather than Python repr strings, reducing downstream parsing fragility in agent pipelines.
Changes:
- Updated
BaseTool.return_value_as_stringto JSON-serializedict/listreturn values. - Added regression tests asserting
FunctionTooldict/list return values serialize as JSON. - Kept existing Pydantic
BaseModelserialization behavior (already JSON-based for dict dumps).
File summaries
| File | Description |
|---|---|
| python/packages/autogen-core/src/autogen_core/tools/_base.py | Switches dict/list stringification from str(...) to json.dumps(...) in tool result serialization. |
| python/packages/autogen-core/tests/test_tools.py | Adds regression tests covering JSON serialization for dict/list tool outputs. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if isinstance(value, (dict, list)): | ||
| return json.dumps(value) | ||
|
|
| def my_function() -> dict: | ||
| return {"status": "success", "count": 42} |
| def my_function() -> list: | ||
| return ["a", "b", "c"] |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
I've handled the |
…es, resolve Bedrock model IDs
d55f0a0 to
b93b0a9
Compare
|
Hello maintainers, Checking in on this pull request (fix: serialize tool outputs to JSON, remove trailing assistant messages, resolve Bedrock model IDs) for microsoft/autogen. Contributing around multi-agent orchestration, agent communication, and tool execution in AutoGen has been exciting, and I'd love to help out with additional improvements and features across microsoft/autogen. I'm available for both freelance/contract work and full-time software engineering roles, or joining the project as an active contributor. Feel free to check out my profile and other work at https://github.com/wasim-builds. |
Summary
Fixes #7867, #7768, and #7833
#7867: Tool output serialization
BaseTool.return_value_as_string used str() for dicts/lists, producing Python repr instead of valid JSON.
#7768: Anthropic trailing assistant messages
_rstrip_last_assistant_message only stripped whitespace but didn't remove trailing assistant messages.
#7833: Bedrock model ID resolution
get_info/get_token_limit could not resolve Bedrock model IDs with provider prefixes.
Changes
Test plan