fix(core): separate sub-agent thought headers with comma and space#21689
fix(core): separate sub-agent thought headers with comma and space#21689imadraude wants to merge 1 commit into
Conversation
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 significantly enhances the user experience within the Terminal User Interface (TUI) by refining how sub-agent thought processes are displayed. It addresses a formatting issue that previously made sequential thoughts difficult to read, ensuring that all agent activities are presented clearly and logically. The changes improve the clarity of real-time agent feedback, making it easier for users to follow the agent's progress and decision-making. Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request correctly identifies the need to separate sub-agent thought headers. The implementation adds the desired comma separator. However, the logic for handling incremental updates and preventing duplicates has some flaws that could lead to an incorrect display of thoughts in the UI. I've provided a suggestion to make this logic more robust, which also aligns with our guideline for using comma-separated strings for LLM-consumed data.
| if ( | ||
| text.startsWith(lastItem.content) && | ||
| lastItem.content.length > 0 | ||
| ) { | ||
| lastItem.content = text; | ||
| } else if (!lastItem.content.includes(text)) { | ||
| lastItem.content += (lastItem.content ? ', ' : '') + text; | ||
| } |
There was a problem hiding this comment.
The current logic for updating thought content has a couple of issues that can lead to incorrect display:
- Incremental Update Flaw: The
text.startsWith(lastItem.content)check only works for the very first thought in a sequence. IflastItem.contentis"Analyzing, Searching"and the next chunk is an update to the second thought like"Searching the web", this condition will fail, and it will be incorrectly appended as a new thought. - Duplicate Check Flaw: The
!lastItem.content.includes(text)check is not robust. It will incorrectly drop a new thought if it happens to be a substring of the existing content (e.g., a new thought"web"would be dropped if"Searching the web"is already present).
A more robust approach is to treat the content as a list of thoughts, operating on the last thought for incremental updates and checking for full-string existence for new thoughts.
Additionally, it would be beneficial to trim() the incoming text at the start of the THOUGHT_CHUNK case to prevent formatting issues like double spaces from chunks with leading/trailing whitespace.
const thoughts = lastItem.content.split(', ');
const lastThought = thoughts[thoughts.length - 1];
// Handle incremental streaming for the last thought
if (text.startsWith(lastThought) && lastThought.length > 0) {
thoughts[thoughts.length - 1] = text;
lastItem.content = thoughts.join(', ');
} else if (!thoughts.includes(text)) {
// Append a new, distinct thought
lastItem.content += (lastItem.content ? ', ' : '') + text;
}References
- For multi-select answers that will be consumed by an LLM, return a single comma-separated string instead of an array of strings to maintain a simpler API design. The suggested approach of joining thoughts with ', ' aligns with this principle for structured data.
Summary
Fixes the issue where sub-agent thought headers in the TUI were concatenated into a single string without spaces (e.g.,
Analyzing...Searching...). They are now separated by a comma and a space (Analyzing..., Searching...).Details
packages/core/src/agents/local-invocation.tsto handleTHOUGHT_CHUNKevents more intelligently.,as a separator.Related Issues
Fixes #21688
How to Validate
Alternatively, run the updated tests:
npm test -w @google/gemini-cli-core -- src/agents/local-invocation.test.tsPre-Merge Checklist