fix(acp): move tool explanation from thought stream to tool call content#26554
fix(acp): move tool explanation from thought stream to tool call content#26554
Conversation
Address an issue where raw input parameters for MCP tools were emitted as JSON strings in the agent's thought stream, creating noise in the chat UI. Details: - Stop sending `invocation.getExplanation()` as an `agent_thought_chunk` in `acpSession.ts`. - Include the explanation in the `content` field of the `toolCall` payload in both `RequestPermissionRequest` and `tool_call` (in_progress) updates, provided the content is not already populated (e.g., by a diff). - This ensures that tool parameters remain visible to the client (e.g., JetBrains or Xcode) in the structured payload without cluttering the chat stream or breaking the title field, thus avoiding regression for issue #1572. - Add unit tests in `acpSession.test.ts` to verify that the explanation is correctly placed in the content and not emitted as a thought chunk.
Summary of ChangesHello, 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 improves the MCP tool invocation experience by cleaning up the agent's thought stream. By moving tool explanations from raw thought chunks into the structured tool call content, the changes ensure that critical information remains accessible to clients like JetBrains or Xcode while significantly reducing visual clutter in the chat interface. This approach maintains backward compatibility and prevents regressions for structured tool data handling. Highlights
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 the 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 counterproductive. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of tool execution explanations in acpSession.ts. Explanations are no longer sent as separate agent_thought_chunk updates; instead, they are now included within the tool_call content or the permission request content when the content array is empty. Corresponding unit tests have been added to acpSession.test.ts to ensure correct behavior in both permission-required and automatic execution scenarios. I have no feedback to provide as there were no review comments to evaluate.
|
Size Change: +167 B (0%) Total Size: 34 MB
ℹ️ View Unchanged
|
Summary
Address an issue where raw input parameters for MCP tools were emitted as JSON strings in the agent's thought stream, creating noise in the chat UI.
Details:
invocation.getExplanation()as anagent_thought_chunkinacpSession.ts.contentfield of thetoolCallpayload in bothRequestPermissionRequestandtool_call(in_progress) updates, provided the content is not already populated (e.g., by a diff).acpSession.test.tsto verify that the explanation is correctly placed in the content and not emitted as a thought chunk.Related Issues
google-gemini/maintainers-gemini-cli#1659
How to Validate
Ensure no regressions. Issue testing is being done by an external team.
The PR that introduced this issue was meant for JB - as the explanation was appearing as part of the command run before original fix. Checked that - it's still not part of the actual run command tool box. The slight change is that it's no longer part of the thinking. It is shown in the main message area. (not part of the commmand run) as shown below:
Pre-Merge Checklist