fix(agent_tool): ensure text output when skip_summarization is True#4390
fix(agent_tool): ensure text output when skip_summarization is True#4390ItsMacto wants to merge 5 commits intogoogle:mainfrom
Conversation
When skip_summarization=True, AgentTool previously terminated the flow with a FunctionResponse event that contained no text, causing UIs to appear stuck. This change appends a Text part to the response event when skipping summarization so tool output is displayed. The implementation avoids duplication, safely serializes non-string results, and preserves existing behavior for normal flows. Added regression tests to verify text output is present for both string and JSON-like tool results.
Summary of ChangesHello @ItsMacto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical user experience issue where the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where UIs could get stuck when skip_summarization=True by ensuring a text part is always included in the final event. The implementation correctly adds a text representation of the tool's output, and includes new regression tests.
My review includes a suggestion to refine the serialization logic for non-string results to provide a cleaner output. Additionally, I've recommended strengthening the new test assertions and expanding test coverage to better validate the fix.
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where AgentTool would not produce a displayable text output when skip_summarization is enabled, causing UIs to appear stuck. The fix correctly appends a Text part to the response event, serializing non-string results to ensure there's always a visible output. The added regression tests effectively cover the new functionality. My review includes suggestions to strengthen the assertions in these new tests to make them more precise and less prone to false positives.
…the 'result' key for JSON display. - assert the exact string content - new test case for a non-string dictionary - more precise assertion checking for the exact text content
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where the UI would hang when skip_summarization=True by ensuring a text part is always included in the final event. The implementation correctly handles both string and non-string results from the tool. The accompanying tests are thorough and cover the key scenarios. My review includes a suggestion to refactor the implementation for better readability and another to reduce code duplication in the new tests using a pytest fixture.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of UIs hanging when skip_summarization=True by ensuring a text part is always included in the function response event. The implementation correctly handles both string and non-string (JSON-serializable) tool outputs. The new unit tests are comprehensive and validate the fix across different scenarios. The changes are well-implemented and solve the problem described. I have one minor suggestion to improve code clarity and maintainability.
…zation is skipped.
When skip_summarization=True, AgentTool previously terminated the flow with a FunctionResponse event that contained no text, leaving UIs stuck.
This change appends a Text part to the response event when summarization is skipped, so the tool's output is displayed. The implementation avoids duplication, safely serializes non-string results, and preserves existing behavior for normal flows.
Added regression tests to verify text output is present for both string and JSON-like tool results.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
When
skip_summarization=True, the final response event emitted by AgentTool contains only a FunctionResponse and no displayable text. Most UIs do not render function responses, making the agent appear to hang or return an empty result.Solution:
When skip summarization is enabled, the function result is additionally included as a Text part in the response event. If the result is a string, it is used directly; otherwise, it is safely serialized to JSON for display. This ensures the final event remains displayable while preserving existing behavior for non-skip paths.
Testing Plan
Unit Tests:
Passed tests:
pytest tests/unittests/tools/test_agent_tool.pyManual End-to-End (E2E) Tests:
Not required. The change affects event construction and is fully covered by unit tests validating the final response content.
Checklist