refactor(core): adopt CoreToolCallStatus enum for type safety#18998
refactor(core): adopt CoreToolCallStatus enum for type safety#18998
CoreToolCallStatus enum for type safety#18998Conversation
Summary of ChangesHello @jerop, 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 a significant refactoring across the core and CLI packages by replacing all instances of hardcoded string literals for tool call statuses with a new Highlights
Changelog
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
|
This change replaces hardcoded tool call status strings ('validating',
'scheduled', 'executing', 'success', 'error', 'cancelled') with a
centralized 'CoreToolCallStatus' enum in the core package.
Benefits:
- Type Safety: Prevents runtime errors and logic bugs caused by typos in
status strings across the core and cli packages.
- Maintainability: Provides a single source of truth for all possible
tool call states, making it easier to track and update state transitions.
- Developer Experience: Enables IDE autocomplete and better discoverability
of tool call statuses.
- Code Quality: Resolves several unsafe type assertions and clarifies
state handling in the Scheduler and CoreToolScheduler.
This refactor establishes a more robust foundation for tool execution
tracking and UI mapping.
16f3465 to
cf6d866
Compare
There was a problem hiding this comment.
Code Review
This pull request is a great refactoring that replaces hardcoded tool call status strings with a CoreToolCallStatus enum. This significantly improves type safety and maintainability across the codebase. The changes are applied consistently across core logic, tests, and UI components.
I've found one critical issue related to a type mismatch that will likely cause a compilation error. Please see my comment for details.
Overall, this is a valuable improvement to the codebase.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/cli/src/ui/hooks/toolMapping.ts (124)
The property approvalMode does not seem to exist on the call object, which is of type ToolCall from @google/gemini-cli-core. The ToolCall type and its constituent types (ValidatingToolCall, ScheduledToolCall, etc.) in packages/core/src/scheduler/types.ts do not define an approvalMode property. This will likely cause a compilation error.
If approvalMode is a property of an extended type (e.g., TrackedToolCall from useToolScheduler), the type of the call parameter in the map function should be adjusted accordingly, or the mapToDisplay function signature should be updated to accept the more specific type.
|
Size Change: +3.89 kB (+0.02%) Total Size: 24.4 MB
ℹ️ View Unchanged
|
…allDisplay This refactor updates the CLI to store the high-fidelity 'CoreToolCallStatus' in the history data model (ToolCallEvent and IndividualToolCallDisplay) instead of the simplified 'ToolCallStatus'. The mapping to the display-oriented 'ToolCallStatus' is now deferred to the view layer (React components), ensuring that internal logic and hooks have access to the precise state of tool execution. Follow-up to #18998 Related to #18918
…allDisplay This refactor updates the CLI to store the high-fidelity 'CoreToolCallStatus' in the history data model (ToolCallEvent and IndividualToolCallDisplay) instead of the simplified 'ToolCallStatus'. The mapping to the display-oriented 'ToolCallStatus' is now deferred to the view layer (React components), ensuring that internal logic and hooks have access to the precise state of tool execution. Follow-up to #18998 Related to #18918
This change replaces hardcoded tool call status strings ('validating', 'scheduled', 'executing', 'success', 'error', 'cancelled') with a centralized 'CoreToolCallStatus' enum in the core package.
Benefits:
This refactor establishes a more robust foundation for tool execution tracking and UI mapping.
Related to #18918.