refactor(cli): centralize tool mapping and decouple legacy scheduler #17044
Conversation
- Introduces toolMapping.ts as a pure projection layer from Core ToolCall to UI IndividualToolCallDisplay. - Decouples useReactToolScheduler from shared mapping logic by moving legacy "tracked" types locally. - Updates useGeminiStream to use the new mapping utility. - Provides a clean, verified foundation for the event-driven scheduler migration.
|
Hi @abhipatel12, 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 @abhipatel12, 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 refactors the CLI UI to centralize the logic for mapping tool calls to their display representations and to decouple the legacy tool scheduler from UI concerns. This foundational work establishes a clear data contract, which is crucial for the upcoming Phase 2 of the Event-Driven ReAct loop migration. The changes aim to improve maintainability, promote consistent UI rendering, and prepare the codebase for future architectural enhancements. 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. 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
|
|
Size Change: -1.19 kB (-0.01%) Total Size: 23.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 is a nice refactoring that centralizes the tool call mapping logic into a new toolMapping.ts module. This decouples the UI mapping from the tool scheduling logic in useReactToolScheduler.ts, improving modularity and separation of concerns. The introduction of comprehensive unit tests for the new mapping logic in toolMapping.test.ts is also a great addition.
My review focuses on improving the type safety in the new mapping function. I've suggested a small change to avoid using an any type cast, which makes the code cleaner and more robust.
gemini-cli
bot
added
the
area/agent
|
|
||
| if (call.status === 'error') { | ||
| displayName = | ||
| call.tool === undefined ? call.request.name : call.tool.displayName; |
There was a problem hiding this comment.
Why diverge the display name in error scenarios (compared to the else block below)?
There was a problem hiding this comment.
We end up needing it b/c in some error states (like a
tool not being found), call.tool or call.invocation could be undefined.
I changed some things to unify the display name but keeping the description check.
| export type CancelAllFn = (signal: AbortSignal) => void; | ||
|
|
||
| /** | ||
| * Legacy scheduler implementation based on CoreToolScheduler callbacks. |
There was a problem hiding this comment.
What still uses this?
There was a problem hiding this comment.
This is still the primary hook. It's just labeled legacy b/c there's a chain and we have another PR after this that switches it.
Added a comment to clarify this!
2 participants
Summary
Refactor the CLI UI to centralize tool mapping logic and decouple the legacy
scheduler from UI concerns. This establishes the necessary data contract for
Phase 2 of the Event-Driven ReAct loop migration.
Details
toolMapping.tsas a pure projectionlayer. It transforms Core
ToolCallobjects into UIHistoryItemToolGroupobjects, separating execution state from presentation.
useReactToolScheduler.tsto remove internal UImapping logic, making it a pure execution bridge.
useGeminiStream.tsto utilize the newcentralized mapper, ensuring consistent rendering regardless of the underlying
scheduler implementation.
toolMapping.test.tswith comprehensive unit tests for alltool state transitions (validating, success, error, etc.).
Related Issues
Related to #14306
How to Validate
npm test -w @google/gemini-cli -- src/ui/hooks/toolMapping.test.tsnpm test -w @google/gemini-cli -- src/ui/hooks/useReactToolScheduler.test.tslist files).in the history.
Pre-Merge Checklist