feat(cli): give visibility to /tools list command in the TUI and follow the subcommand pattern of other commands #21213
Conversation
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 significantly enhances the user experience of the Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request successfully exposes the /tools desc subcommand in the TUI by refactoring the /tools command to use subcommands. The implementation is good, but I've identified a couple of areas for improvement. There's some code duplication in the main command handler that could be refactored for better maintainability, aligning with best practices for CLI command structure. Additionally, some of the tests have been weakened by removing specific assertions, and I've recommended restoring them to maintain test quality.
|
Please fix the typo in the PR title? visibilty -> visibility as well as the existing Gemini Code Assist review comments. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the /tools command to introduce discoverable list and desc subcommands, improving the user experience by making the tool description functionality more visible. The implementation is well-structured, separating logic into a helper function and distinct subcommand definitions. My review focuses on strengthening the test suite to ensure the changes are robust. I've pointed out a couple of places in the tests where assertions for tool descriptions should be restored to prevent potential regressions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the /tools command to use subcommands (list and desc), improving discoverability of the tool description functionality. The changes are well-tested. However, I've found a bug and a maintainability issue in the parent command's action logic that manually dispatches to subcommands, which violates a repository guideline. My suggestion addresses both issues by implementing a more robust and scalable subcommand router.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the /tools command to use a subcommand structure, exposing the existing but hidden /tools desc functionality. The implementation introduces list and desc as formal subcommands, with list being the default action. The changes are well-tested. The previously identified critical issue regarding argument parsing was found to contradict established repository rules concerning argument handling and subcommand routing, and thus no specific code review comments are being provided.
|
thanks for the review @spencer426 i have addressed the issues by code-assist and corrected the title at present i am in confusion because even though i came up with the issue and made a PR first, there is also another PR linked to the issue ignoring my PR which is (#21241) and it seems to be approved by another maintainer so i really am confused by the status of this work just wanted to let you know. thanks for your time. and also there seem to be my mistake where i created the issue and linked my pr but forgot to assign myself, i will do it with my future PR's |
|
@JayadityaGit that's correct unfortunately. Since the issue is already being addressed and approved in pr #21241 we will have to close this PR. Going forward please assign to yourself if you plan on working on the issue. |
|
we cant even assign issues created by our own, until it get's a help wanted label, the moment it gets a help wanted label people just assign themselves and get a PR so fast in this case, creating the issue and attaching a PR and discussing with @jacob314 bringing this issue into light was the thing i did it is sad to see to happen like this. where people just assign themselves when there is already PR present in the first place. thanks for your time. |
|
and also that merged PR now makes /tools list subcommand hidden from the TUI, giving a new UX Problem, i just checked it right now. with this PR i wanted to bring light into /tools desc command where it was hidden in the tui, now after that PR merge it became inverse where /tools desc became visible /tools list became invisible this PR was aimed to bring both implementation into light so when the user goes into /tools command there will be two subcommands /tools list following the consistent subcommand pattern of other commands |
… feat/tools-desc-visibility
…lve conflicts in toolsCommand.ts
|
looking for a re-review @spencer426 , this PR solves the new UX problem which came up after the merge of that PR. |
spencer426
left a comment
There was a problem hiding this comment.
Hey @JayadityaGit, I'm really sorry about the frustration with how the issue assignment and labels played out here. You did everything exactly right—spotting the issue, starting the discussion, and putting up this PR. We need to get better at checking for active PRs before merging parallel fixes. Thanks for being patient with us.
LGTM
|
no problem it is hard to keep track on everything as a reviewer, thanks for the review @spencer426 ! |
…ow the subcommand pattern of other commands (google-gemini#21213)
…ow the subcommand pattern of other commands (google-gemini#21213)
…ow the subcommand pattern of other commands (google-gemini#21213)
…ow the subcommand pattern of other commands (#21213)
…ow the subcommand pattern of other commands (google-gemini#21213)
…ow the subcommand pattern of other commands (google-gemini#21213)
When i was deep researching about tools in the codebase documentation , i had realiesed there is an already an implementation which give descritpions on each tool which is /tool desc
But it was never visible in the TUI making it invisible to general user, who did not go into the codebase docs.
so this PR gives visibility to the hidden subcommand
so now when the user executes /tools command there will be two subcommands
this PR gives visibility to the hidden subcommand, where it is really hard to find.

issue :- #21831