-
Notifications
You must be signed in to change notification settings - Fork 605
fix: include both text and structured_output in AgentResult.__str__ #1472
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?
fix: include both text and structured_output in AgentResult.__str__ #1472
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
All three review comments have been addressed in commit 7ae3a2a:
The PR is now awaiting workflow approval (check-access-and-checkout is WAITING status). 🦆 🤖 This is an experimental AI agent response from the Strands team, powered by Strands Agents. We're exploring how AI agents can help with community support and development. Your feedback helps us improve! If you'd prefer human assistance, please let us know. |
src/strands/agent/agent_result.py
Outdated
| structured_data = self.structured_output.model_dump() | ||
| if result: | ||
| # Both text and structured output exist - return JSON-parseable format | ||
| combined = {"text": result.rstrip("\n"), "structured_output": structured_data} |
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 am curious that this .rstrip modifies the result. In normal flow of the code result were not processed at all. This implementation now modifies the result which might not be intended. LLM might produce \n end token and we would want to keep as-is.
04d3b1f to
f23a5da
Compare
f23a5da to
47880b4
Compare
47880b4 to
aaef766
Compare
aaef766 to
6cd4949
Compare
…Option 1) Fixes strands-agents#1461: Structured output is not properly propagated in the graph This implements Option 1 from strands-agents#1461 discussion - modifying __str__ to include both text and serialized structured_output when both exist. Changes: - Updated AgentResult.__str__ to append structured output when present - Format: text content followed by [Structured Output] section with JSON - Updated existing test to reflect new expected behavior - Added 8 comprehensive tests for all __str__ scenarios The fix ensures that when AgentResult is stringified (e.g., in graph node transitions), the structured output data is preserved alongside any text content. Requested-by: @afarntrog
Addresses feedback from @afarntrog to make the output JSON-parseable. When both text and structured output exist, output is now: {"text": "...", "structured_output": {...}} This allows users to parse the combined output with json.loads() and extract both the text and structured data programmatically. - Text-only: returns raw text (unchanged) - Structured-only: returns JSON of structured output (unchanged) - Both: returns JSON with 'text' and 'structured_output' keys (NEW)
Address review feedback from @cagataycali to clean up code comments.
Per review feedback from @afarntrog, moved all __str__ method tests from the separate test_agent_result_str.py file into the existing test_agent_result.py file to keep related tests together.
Addresses review feedback from @cagataycali: - Removed .rstrip() that was modifying LLM output - Text content is now preserved exactly as produced The previous implementation used .rstrip('\n') which could strip intentional trailing newlines from the LLM output. This fix ensures the original text is preserved without modification.
6cd4949 to
8e82e27
Compare
|
@strands-agent fix the failing lint. Be sure to run all checks before pushing any new commits. |
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.
When both text and structured output exist:
{"text": "Your text content here", "structured_output": {"field": "value"}}This allows users to parse the output programmatically:
import json result = agent("prompt", structured_output_model=MyModel) output = str(result) parsed = json.loads(output) print(parsed["text"]) # The text response print(parsed["structured_output"]) # The structured data as dict
This seems odd to me; we're using a str representation to include JSON; I think of str as human readable and this seems to go the opposite direction.
If anything, I would expect the JSON to be appended to the text at the end, not JSON-stringified. If we wanted both the JSON and the string text, why not use the AgentResult directly to get those items?
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.
Or a better question:
Why is:
This allows users to parse the output programmatically:
A goal of __str__?
If, we're attempting to fix:
When AgentResult.str is called (e.g., in graph node transitions), it only returns structured_output when there is NO text content. This causes structured data to be lost when both text and structured output exist.
I don't think JSON in str is the right answer - though perhaps I'm the minority in that opinion.
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.
It's ultimately stringified json though. The benefit I see of using json in the string is that gives users the flexibility of parsing the str if they want. I don't think it'll degrade the str output. It'll provide more flexibility
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.
Going from:
Now that I have all the information, I can format the JSON appropriately:
to:
{"text", "Now that I have all the information, I can format the JSON appropriately", "structured_output": { "name": "Mackenzie Zastrow", favoriateCake: "Chocolate" } }
Still doesn't make sense to me. Why are we special casing that when you have structured_output, __str__ is JSON-fying the entire thing?
If you want it strongly typed/structured use AgentResult, if you want it as a string, call __str__ but I think conflating the two is odd.
Whereas this makes more sense:
Now that I have all the information, I can format the JSON appropriately:
{ "name": "Mackenzie Zastrow", favoriateCake: "Chocolate" }
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.
Wondering what’s stopping us from appending structured_output right after line 1004 (though we should skip adding duplicate JSON strings when there’s no text present)?
sdk-python/src/strands/multiagent/graph.py
Lines 1002 to 1005 in 318573d
| for result in agent_results: | |
| agent_name = getattr(result, "agent_name", "Agent") | |
| result_text = str(result) | |
| node_input.append(ContentBlock(text=f" - {agent_name}: {result_text}")) |
Description
This implements Option 1 from the #1461 discussion as requested by @afarntrog.
The Problem
When
AgentResult.__str__is called (e.g., in graph node transitions), it only returnsstructured_outputwhen there is NO text content. This causes structured data to be lost when both text and structured output exist.The Solution (Option 1 - JSON Format)
Modify
__str__to include both text and serialized structured_output when both exist, in JSON format for parseability:Output Format
When both text and structured output exist:
{"text": "Your text content here", "structured_output": {"field": "value"}}This allows users to parse the output programmatically:
Related Issues
Fixes #1461
Documentation PR
No documentation changes required - this is a behavioral fix.
Type of Change
Testing
__str__scenariosChecklist
hatch run prepare(tests pass locally)Requested by @afarntrog in comment and feedback on JSON format 🦆