refactor(cli,core): foundational layout, identity management, and type safety#23286
refactor(cli,core): foundational layout, identity management, and type safety#23286jwhelangoog merged 2 commits intomainfrom
Conversation
|
Hi @jwhelangoog, 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 establishes a robust structural foundation for upcoming compact tool output changes. It significantly enhances identity management across various components, refines UI layouts for improved user experience, and strengthens type safety throughout the codebase. These changes are designed to provide a more stable, predictable, and maintainable platform for future development, particularly in how tool outputs and user interactions are handled. 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: +4.09 kB (+0.02%) Total Size: 26.2 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 foundational layout, identity management, and type safety improvements to the Gemini CLI project. The changes include ensuring strictly increasing IDs for history items, preventing callId collisions, modernizing test rendering, fixing UI layout issues, refactoring IDE client initialization, improving diff stat derivation, adding foundational types, and introducing display name constants for tools. The review focuses on ensuring the correct implementation of the callId counter and the accuracy of the scroll arrows in the AskUserDialog component.
Note: Security Review did not run due to the size of the PR.
gemini-cli
bot
added
the
priority/p1
| } | ||
| terminalWidth={mainAreaWidth} | ||
| item={{ ...item, id: 0 }} | ||
| item={{ ...item, id: -(i + 1) }} |
There was a problem hiding this comment.
why are you making these ids negative? seems odd.
There was a problem hiding this comment.
This now assigns a sequential negative id to transient pending history items until they eventually are transitioned into a permanent (positive) history id. Previously pending items were based on an id of zero; potential for dup key errors.
jwhelangoog
force-pushed
the
dense-output-build-prep
branch
7 times, most recently
from
820c873 to
7ad2090
Compare
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
Approved whether these comments are addressed
Code Review for PR #23286 (Frontend & Core Focus)Review generated via This PR establishes an excellent structural foundation for the upcoming compact tool output changes. It cleanly extracts history management logic ( However, I've identified a few architectural and logical issues that should be addressed to ensure consistency and prevent subtle bugs in the future. Critical Findings
Improvements
Nitpicks
|
jwhelangoog
force-pushed
the
dense-output-build-prep
branch
from
7ad2090 to
2bd69fb
Compare
jwhelangoog
force-pushed
the
dense-output-build-prep
branch
2 times, most recently
from
04ede2e to
8360e50
Compare
|
Providing the following updates to address feedback from /review-frontend and Jacob Critical FindingsType Duplication (packages/cli/src/ui/types.ts):
Type Guard Robustness and Placement (packages/cli/src/ui/types.ts):
ImprovementsIdeClient State Initialization (packages/cli/src/ui/contexts/ToolActionsContext.tsx):
TOGGLE_ALL State Mismatch (packages/cli/src/ui/components/AskUserDialog.tsx):
|
jwhelangoog
force-pushed
the
dense-output-build-prep
branch
3 times, most recently
from
9ca0cb4 to
6c6e51e
Compare
jwhelangoog
force-pushed
the
dense-output-build-prep
branch
2 times, most recently
from
8f430ed to
9beee0d
Compare
|
@anj-s please review for google-gemini/gemini-cli-prompt-approvers approval. TIA! |
| DEFAULT_DIFF_OPTIONS, | ||
| ); | ||
|
|
||
| // Determine the full content as originally proposed by the AI to ensure accurate diff stats. |
There was a problem hiding this comment.
right bellow this we call getDiffStat. Why is htat not sufficient? Worry we are adding duplicate logic here and elsewhere in this pr tracking diffs.
There was a problem hiding this comment.
Added to alleviate the case when user's modify AI suggested changes. Will create an issue for further cleanup / refactor.
jwhelangoog
force-pushed
the
dense-output-build-prep
branch
2 times, most recently
from
d999ece to
5e65786
Compare
🧠 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. |
jwhelangoog
force-pushed
the
dense-output-build-prep
branch
from
5e65786 to
8507b97
Compare
…e safety This commit establishes the structural foundation and required infrastructure to support the upcoming compact tool output changes. It includes identity management improvements, layout fixes, and type-safety enhancements that stand independently. 1. Identity & History Management: - useHistoryManager: Ensure strictly increasing IDs for history items, even with identical timestamps. - acpClient: Introduced callIdCounter to prevent callId collisions during rapid execution. - MainContent: Implemented negative ID logic for pending items to ensure stable React keys and prevent collisions. - historyUtils: New file containing extracted history logic (isToolExecuting, getAllToolCalls) hoisted from AppContainer. 2. UI Infrastructure & Layout: - test-utils/render: Modernized renderWithProviders and removed legacy props. - AskUserDialog: Fixed layout, scroll visibility, and alignment issues. - toolLayoutUtils: Increased TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT for better spacing. - ShellToolMessage/ToolGroupMessage: Updated line-count expectations and snapshots to align with layout changes. 3. IDE & Diffing Improvements: - ToolActionsContext: Refactored IdeClient initialization to fix a race condition and potential memory leak. - edit/diffOptions: Improved accuracy of diff stat derivation, ensuring "full content" stats are available for the model. - coreToolScheduler: Automatically derive diff stats from patches if missing. - state-manager: Ensure diffStat preservation for rejected tool calls. 4. Type Safety & Constants: - types/tools: Added foundational types like DiffStat, FileDiff, and StructuredToolResult. - Type Guards: Added guards for isFileDiff, isTodoList, isAnsiOutput, and hasSummary. - CodeColorizer: Added function overloads to gracefully handle null language detection. - tool-names: Introduced DISPLAY_NAME constants for consistent tool labeling. 5. Changes from Review Feedback - consolidated tool-related interfaces and type guards within tools.js
…rnings from setInterval usage
jwhelangoog
force-pushed
the
dense-output-build-prep
branch
from
8507b97 to
87cc88b
Compare
3 participants
This commit establishes the structural foundation and required infrastructure to support the upcoming compact tool output changes. It includes identity management improvements, layout fixes, and type-safety enhancements that stand independently.
This commit passes all workspace tests and builds successfully. Feature-specific logic for compact output is excluded.
Summary
Details
Related Issues
Related to #20974
Related to #16190
How to Validate
Pre-Merge Checklist