fix(core): throttle shell text output UI updates - #22843
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 interface's responsiveness when handling shell commands that produce a large amount of text output. By introducing a throttling mechanism for text data events, it prevents the UI from freezing due to excessive re-renders, thereby improving the overall user experience without compromising the accuracy or completeness of the displayed information. 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 aims to improve UI performance by throttling shell output updates, which is a valuable enhancement. However, the current implementation introduces a functional regression for non-PTY shell executions. By overwriting the output with each chunk instead of accumulating it, intermediate output is lost during the throttled live updates. I've provided a critical review comment with a code suggestion to address this issue by correctly handling both string-based (cumulative) and object-based (overwrite) output types.
| @@ -241,7 +241,13 @@ export class ShellToolInvocation extends BaseToolInvocation< | |||
| case 'data': | |||
| if (isBinaryStream) break; | |||
| cumulativeOutput = event.chunk; | |||
There was a problem hiding this comment.
The current implementation overwrites cumulativeOutput with each new event.chunk. While this is correct for PTY executions where event.chunk represents the full terminal state (AnsiOutput), it is incorrect for non-PTY (child_process) executions where event.chunk is a partial string of the output. With the new throttling logic, any intermediate chunks received between UI updates will be lost, leading to an incomplete and choppy live view of the command's output.
To fix this, you should accumulate the output for string-based chunks, while continuing to overwrite for AnsiOutput chunks.
| cumulativeOutput = event.chunk; | |
| if (typeof event.chunk === 'string') { | |
| cumulativeOutput = (cumulativeOutput as string) + event.chunk; | |
| } else { | |
| cumulativeOutput = event.chunk; | |
| } |
There was a problem hiding this comment.
This was fixed in the latest commit
8cfd982 to
1af4345
Compare
Improves shell tool output accumulation. By accurately buffering the text stream and accumulating chunks, it prevents the terminal UI from freezing or lagging during high-volume text streams (e.g. verbose build commands) without losing intermediate output chunks.
9502704 to
7f69803
Compare
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we have updated our contribution policy (see Discussion #17383). We only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. All other community pull requests are subject to closure after 14 days if they do not align with our current focus areas. For this reason, we strongly recommend that contributors only submit pull requests against issues explicitly labeled as 'help-wanted'. This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding and for being part of our community! |
|
Tracking issue filed: #25459. Cannot reopen this PR (GitHub blocks it — branch is far behind base). Will open a fresh PR against current main referencing the issue. |
Summary
dataevents) triggered a React re-render on every chunk, whilebinary_progressalready throttled to 1s intervals viaOUTPUT_UPDATE_INTERVAL_MSOUTPUT_UPDATE_INTERVAL_MSthrottle to text data events. The final output is unaffected — it is rendered from the complete result after the command exitsTest plan
shell.test.tstests are unaffected (only test usingtype: 'data'is for background mode which already skipsupdateOutput)for i in $(seq 1 5000); do echo "line $i"; done) and verify the UI remains responsive🤖 Generated with Claude Code