fix(cli): bound memory growth in high-volume components#24862
fix(cli): bound memory growth in high-volume components#24862spencer426 wants to merge 3 commits intomainfrom
Conversation
|
Hi @spencer426, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
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 introduces memory usage optimizations across several CLI UI components by enforcing hard limits on the amount of data retained in memory. By capping the number of lines and activity items processed, the application becomes more resilient during high-volume operations. Additionally, a minor configuration update was applied to the test utilities package to resolve CI execution issues. 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 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. Footnotes
|
|
Size Change: +1.36 kB (0%) Total Size: 34 MB
ℹ️ View Unchanged
|
![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 line limits and truncation logic across several UI components and the text buffer to prevent memory issues and improve performance. Specifically, it caps ANSI output to 1000 lines, subagent progress activity to 100 items, and the text buffer to 10,000 lines. Feedback was provided regarding a logical inconsistency in the AnsiOutput truncation where disabling truncation could unexpectedly result in fewer lines than when enabled. Additionally, the textBufferReducer needs to be updated to correctly shift or clear the selectionAnchor when the buffer is truncated to avoid UI state corruption.
| const MAXIMUM_ANSI_LINES_RENDERED = 1000; | ||
| const lastLines = Array.isArray(data) | ||
| ? disableTruncation | ||
| ? data | ||
| ? data.slice(-MAXIMUM_ANSI_LINES_RENDERED) | ||
| : numLinesRetained === 0 | ||
| ? [] | ||
| : data.slice(-numLinesRetained) | ||
| : []; |
There was a problem hiding this comment.
The current implementation of MAXIMUM_ANSI_LINES_RENDERED introduces a logical inconsistency. When disableTruncation is true, the output is capped at 1000 lines. However, when disableTruncation is false, the number of lines is determined by numLinesRetained, which can exceed 1000 if maxLines is set to a higher value. This means that 'disabling' truncation could actually result in fewer lines being displayed than when it is enabled. The safety cap should be applied consistently to both cases to ensure the memory bound is respected without inverting the behavior of the disableTruncation prop.
| const MAXIMUM_ANSI_LINES_RENDERED = 1000; | |
| const lastLines = Array.isArray(data) | |
| ? disableTruncation | |
| ? data | |
| ? data.slice(-MAXIMUM_ANSI_LINES_RENDERED) | |
| : numLinesRetained === 0 | |
| ? [] | |
| : data.slice(-numLinesRetained) | |
| : []; | |
| const MAXIMUM_ANSI_LINES_RENDERED = 1000; | |
| const lastLines = Array.isArray(data) | |
| ? disableTruncation | |
| ? data.slice(-MAXIMUM_ANSI_LINES_RENDERED) | |
| : numLinesRetained === 0 | |
| ? [] | |
| : data.slice(-Math.min(numLinesRetained, MAXIMUM_ANSI_LINES_RENDERED)) | |
| : []; |
References
- When implementing truncation logic, a simpler approach is preferred over a more complex one if the potential inaccuracy is trivial compared to the overall buffer size.
- Maintain consistency with existing UI behavior across components.
| if (newState.lines.length > MAX_TEXT_BUFFER_LINES) { | ||
| const excess = newState.lines.length - MAX_TEXT_BUFFER_LINES; | ||
| const newLines = newState.lines.slice(excess); | ||
| const newCursorRow = Math.max(0, newState.cursorRow - excess); | ||
| let newExpandedPaste = newState.expandedPaste; | ||
| if (newExpandedPaste) { | ||
| const newStartLine = newExpandedPaste.startLine - excess; | ||
| if (newStartLine < 0) { | ||
| newExpandedPaste = null; | ||
| } else { | ||
| newExpandedPaste = { ...newExpandedPaste, startLine: newStartLine }; | ||
| } | ||
| } | ||
| newState = { | ||
| ...newState, | ||
| lines: newLines, | ||
| cursorRow: newCursorRow, | ||
| expandedPaste: newExpandedPaste, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The truncation logic correctly adjusts the cursorRow and expandedPaste regions, but it fails to update the selectionAnchor. If a selection exists when the buffer is truncated, the anchor row will point to the wrong line or become negative, leading to inconsistent state and UI bugs.
if (newState.lines.length > MAX_TEXT_BUFFER_LINES) {
const excess = newState.lines.length - MAX_TEXT_BUFFER_LINES;
const newLines = newState.lines.slice(excess);
const newCursorRow = Math.max(0, newState.cursorRow - excess);
let newExpandedPaste = newState.expandedPaste;
if (newExpandedPaste) {
const newStartLine = newExpandedPaste.startLine - excess;
if (newStartLine < 0) {
newExpandedPaste = null;
} else {
newExpandedPaste = { ...newExpandedPaste, startLine: newStartLine };
}
}
let newSelectionAnchor = newState.selectionAnchor;
if (newSelectionAnchor) {
const [anchorRow, anchorCol] = newSelectionAnchor;
const shiftedRow = anchorRow - excess;
newSelectionAnchor = shiftedRow < 0 ? null : [shiftedRow, anchorCol];
}
newState = {
...newState,
lines: newLines,
cursorRow: newCursorRow,
expandedPaste: newExpandedPaste,
selectionAnchor: newSelectionAnchor,
};
}References
- When implementing truncation logic, a simpler approach is preferred over a more complex one if the potential inaccuracy is trivial compared to the overall buffer size.
gemini-cli
Bot
added
the
priority/p1
|
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! |
1 participant
Summary
This PR addresses potential memory leaks in the CLI UI by bounding the maximum number of items/lines retained in high-volume components (
AnsiOutput,SubagentProgressDisplay, andtext-bufferreducers). It also adds a missingtest:ciscript topackages/test-utils/package.jsonto satisfy CI checks.Details
AnsiOutput: Limited the maximum number of ANSI lines rendered to 1000 when truncation is disabled.SubagentProgressDisplay: Bounded the recent activity history rendered to the last 100 items.text-buffer.ts: Capped the maximum text buffer lines at 10,000, adjusting the cursor and expanded paste regions accordingly.packages/test-utils: Addedtest:ciscript (echo 'no tests') to passnpm run test:ciacross workspaces.Related Issues
How to Validate
npm run preflightto verify all linters, type checks, and tests pass.Pre-Merge Checklist