fix(core): throttle shell text output and bound live UI buffer#26955
fix(core): throttle shell text output and bound live UI buffer#26955emersonbusson wants to merge 4 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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 optimizes the shell tool's UI responsiveness by throttling text output and bounding the live buffer size. By preventing excessive React re-renders during high-volume command execution, the changes ensure the interface remains interactive. The implementation guarantees that the initial output is shown immediately, subsequent updates are throttled, and the final state is correctly flushed upon command completion, all while preserving full performance for PTY-based terminal outputs. 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
|
|
@googlebot I signed it! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces output throttling and a bounded buffer for live shell output in the ShellTool to improve performance. It adds logic to manage a rolling output buffer and throttles UI updates to a fixed interval. Review feedback suggests optimizing data transfer by emitting only incremental deltas instead of the full buffer and implementing a timer-based trailing flush to handle silent but active processes.
| let lastUpdateTime = 0; | ||
| let hasFlushedOutput = false; | ||
| let hasPendingOutput = false; | ||
| let isBinaryStream = false; | ||
|
|
||
| const appendToLiveOutputBuffer = (chunk: string) => { | ||
| const currentOutput = | ||
| typeof cumulativeOutput === 'string' ? cumulativeOutput : ''; | ||
| if (chunk.length >= LIVE_OUTPUT_MAX_BUFFER_CHARS) { | ||
| cumulativeOutput = chunk.slice(-LIVE_OUTPUT_MAX_BUFFER_CHARS); | ||
| return; | ||
| } | ||
|
|
||
| const nextOutput = currentOutput + chunk; | ||
| cumulativeOutput = | ||
| nextOutput.length > LIVE_OUTPUT_MAX_BUFFER_CHARS | ||
| ? nextOutput.slice(-LIVE_OUTPUT_MAX_BUFFER_CHARS) | ||
| : nextOutput; | ||
| }; | ||
|
|
||
| const flushOutput = () => { | ||
| if (!hasPendingOutput || !updateOutput || this.params.is_background) { | ||
| return; | ||
| } | ||
|
|
||
| updateOutput(cumulativeOutput); | ||
| hasPendingOutput = false; | ||
| hasFlushedOutput = true; | ||
| lastUpdateTime = Date.now(); | ||
| }; |
There was a problem hiding this comment.
To ensure that the last chunk of output is not stuck in the buffer when a command becomes silent but remains active, we should implement a trailing edge flush using a timer. This ensures that the UI is updated even if no further output events are received within the throttle interval. When implementing this, ensure the flush emits only the incremental delta instead of the full accumulated text to avoid redundant data transfer. The check for signal.aborted in flushOutput correctly utilizes the AbortSignal for cancellation safety. Note that while this check provides safety, it is still recommended to clear the timer in the finally block of the execute method to avoid unnecessary resource usage, although that block is outside the current diff scope.
References
- When implementing streaming message events, emit only the incremental delta instead of the full accumulated text to avoid redundant data transfer and potential display issues, particularly when a higher-level component is responsible for content accumulation.
- Asynchronous operations waiting for user input via the MessageBus should rely on the provided AbortSignal for cancellation, rather than implementing a separate timeout, to maintain consistency with existing patterns.
| if (shouldUpdate && !this.params.is_background) { | ||
| updateOutput(cumulativeOutput); | ||
| lastUpdateTime = Date.now(); | ||
| flushOutput(); | ||
| } |
There was a problem hiding this comment.
Update the data handler to schedule a trailing flush if the current event is throttled. This ensures that sporadic output is eventually displayed as an incremental delta even if the process doesn't exit immediately.
References
- When implementing streaming message events, emit only the incremental delta instead of the full accumulated text to avoid redundant data transfer and potential display issues, particularly when a higher-level component is responsible for content accumulation.
gemini-cli
Bot
added
priority/p2
Adds a setTimeout-based trailing flush so buffered text chunks are rendered when a command goes silent but remains active. Addresses the gemini-code-assist review on google-gemini#25461 / google-gemini#26955. - New trailingFlushTimer hoisted to execute() scope and cleared in the existing finally block alongside timeoutTimer. - scheduleTrailingFlush() is invoked whenever a text data event is throttle-blocked (within OUTPUT_UPDATE_INTERVAL_MS of the previous flush). - flushOutput() now cancels any pending trailing timer up front, so synchronous flushes and the exit-path flush always win the race. - Two new tests in shell.test.ts: * trailing flush fires when the command is silent for the throttle interval * exit cancels a scheduled trailing flush (no duplicate update) - Existing "leading + throttle" test simplified to no longer depend on timer advances; final-state assertion now uses the exit flush.
Follow-up: trailing-edge flush (commit 011fdf0)Pushed a follow-up commit that addresses both HIGH priority items from the What changed in 011fdf0:
The "incremental delta vs full buffer" suggestion in the review is a broader End-to-end validationBuilt the bundle locally and ran it against a real workload (44 shell tool |
|
Follow-up hardening pushed for the shell live-output path:
Local validation:
The remaining expected external blocker is maintainer approval for |
|
✅ 80 tests passed successfully on gemini-3-flash-preview. 🧠 Model Steering GuidanceThis PR modifies files that affect the model's behavior (prompts, tools, or instructions).
This is an automated guidance message triggered by steering logic signatures. |
|
Don't touch the PR while the tests are running |
2 participants
Summary
Throttles shell tool text
dataevents toOUTPUT_UPDATE_INTERVAL_MS(1s), matchingthe existing
binary_progresscadence, and caps the live UI buffer toLIVE_OUTPUT_MAX_BUFFER_CHARS(100k chars). Without throttling, every chunktriggered a full React re-render of the shell output panel; commands emitting
thousands of lines (build warnings, manifest generation, verbose test runs)
pinned the UI until the command exited.
Details
This PR builds on the feedback @jacob314 left on the previous attempt (#25461)
and resolves the three concerns raised there:
lastUpdateTimestarts at0and the first textchunk renders immediately. Subsequent chunks are throttled.
appendToLiveOutputBufferkeeps only thelast
LIVE_OUTPUT_MAX_BUFFER_CHARScharacters for the live preview, removingthe unbounded-string growth risk. The complete output is unaffected: it is
still rendered from the full result returned by
shellExecutionServiceafterthe command exits.
AnsiOutput(PTY mode) chunks bypass thethrottle entirely, preserving responsiveness for progress bars and
curses-style UIs.
Final-state guarantee:
flushOutput()runs on theexitevent and again afterresultPromiseresolves (non-background path), so the last frame is alwaysflushed without delay.
Related Issues
Fixes #25459
Supersedes #22843 (closed under the help-wanted policy) and #25461 (closed for
inactivity after change requests).
How to Validate
npm run bundlefor i in $(seq 1 20000); do echo "line $i"; donestays interactive) and the final output is complete and correct.
updates still appear at full rate (no throttle on PTY).
npm test -w @google/gemini-cli-core -- src/tools/shell.test.tsPre-Merge Checklist
user-facing API change)
shell.test.ts)