fix(tools): serialize plain structured outputs as json#6312
Conversation
![corridor-security[bot]](https://avatars.githubusercontent.com/in/1177339?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: This PR changes tool output formatting to JSON-serialize plain mapping and sequence results before passing them back to agents.
Risk: Low risk. No exploitable security vulnerabilities were identified; the change does not add authentication, authorization, file, network, subprocess, or database attack surface.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesJSON serialization fallback for tool outputs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Per CONTRIBUTING.md, this PR should have the Please apply the label if needed. The PR body includes the AI assistance disclosure and validation evidence. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/crewai/tests/tools/test_base_tool.py (1)
463-472: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one regression test for non-serializable mapping fallback.
These tests cover successful JSON serialization, but not the new “fallback to
str(raw_result)” branch for mapping outputs whenjson.dumpsfails. A small test with a self-referential dict (or similarly unserializable payload) would lock that behavior.Also applies to: 512-526
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/tools/test_base_tool.py` around lines 463 - 472, The test coverage is missing a regression test for the fallback behavior when JSON serialization fails for mapping outputs. Add a new test method in the same test class that creates a non-serializable mapping (such as a self-referential dictionary) and verifies that the format_output_for_agent method falls back to using str(raw_result) instead of json.dumps when json.dumps fails with an exception.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/tools/structured_tool.py`:
- Around line 65-68: The exception handler in the try-except block around
json.dumps only catches TypeError, but json.dumps can also raise ValueError for
cases like circular references, which would bypass the fallback to
str(raw_result). Modify the except clause to catch both TypeError and ValueError
instead of just TypeError to ensure all serialization failures properly fall
back to the string representation of the raw_result.
---
Nitpick comments:
In `@lib/crewai/tests/tools/test_base_tool.py`:
- Around line 463-472: The test coverage is missing a regression test for the
fallback behavior when JSON serialization fails for mapping outputs. Add a new
test method in the same test class that creates a non-serializable mapping (such
as a self-referential dictionary) and verifies that the format_output_for_agent
method falls back to using str(raw_result) instead of json.dumps when json.dumps
fails with an exception.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 61cabb7d-c6c7-43dc-87db-00ded78ae45d
📒 Files selected for processing (4)
lib/crewai/src/crewai/tools/structured_tool.pylib/crewai/tests/tools/test_base_tool.pylib/crewai/tests/tools/test_structured_tool.pylib/crewai/tests/tools/test_tool_usage.py
fengjikui
force-pushed
the
fix/tool-usage-structured-output-json
branch
from
f25cf85 to
dd33b66
Compare
fengjikui
force-pushed
the
fix/tool-usage-structured-output-json
branch
from
dd33b66 to
3cc01e2
Compare
1 participant
Summary
Fixes #6267.
This makes plain JSON-like tool outputs (for example nested dict/list responses from LangChain-style tools) serialize to JSON before they are sent back to the agent. Non-JSON-serializable outputs still fall back to the existing
str(raw_result)behavior.Why
CrewAI already supports
format_output_for_agent()for Pydantic result schemas, but tools without a Pydanticresult_schemastill used Python repr for dict/list outputs. That leaves nested API responses as single-quoted Python dict strings instead of JSON the LLM can reliably parse.Validation
env -u HTTP_PROXY -u HTTPS_PROXY -u ALL_PROXY -u http_proxy -u https_proxy -u all_proxy uv run pytest lib/crewai/tests/tools/test_structured_tool.py lib/crewai/tests/tools/test_base_tool.py lib/crewai/tests/tools/test_tool_usage.py -q->96 passedenv -u HTTP_PROXY -u HTTPS_PROXY -u ALL_PROXY -u http_proxy -u https_proxy -u all_proxy uv run ruff check lib/crewai/src/crewai/tools/structured_tool.py lib/crewai/tests/tools/test_structured_tool.py lib/crewai/tests/tools/test_base_tool.py lib/crewai/tests/tools/test_tool_usage.py->All checks passed!mock_api_toolreturning{"status": "success", "data": {"items": [...]}}now producesjson.loads(result.result)-parseable agent text.AI assistance
AI assistance was used to inspect the issue and current tool execution path, implement this focused change, and run the validation above. I reviewed the final diff and test output.
Summary by CodeRabbit
New Features / Improvements
Tests