feat(tool): retain ask_user question in text history - #29022
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 adds a new configuration setting, 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 the 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 counterproductive. 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
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
📊 PR Size: size/L
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new configuration option, ui.keepAskUserQuestionsInHistory, which allows users to keep questions from the ask_user tool visible in their chat history. The changes span documentation, CLI settings schema, configuration loading, tool invocation logic, and comprehensive unit tests. The reviewer's feedback correctly points out that the optional chaining and nullish coalescing operator used when loading this setting in packages/cli/src/config/config.ts are redundant, as the configuration schema already guarantees the default value is populated.
Note: Security Review did not run due to the size of the PR.
| keepAskUserQuestionsInHistory: | ||
| settings.ui?.keepAskUserQuestionsInHistory ?? false, |
There was a problem hiding this comment.
According to the repository guidelines, we should rely on the schema as the single source of truth for configuration defaults and avoid redundant nullish coalescing operators. Since settings is a MergedSettings object, all properties defined in the schema (including settings.ui.keepAskUserQuestionsInHistory) are guaranteed to be populated with their default values. Therefore, the optional chaining and nullish coalescing operator are redundant here.
keepAskUserQuestionsInHistory: settings.ui.keepAskUserQuestionsInHistory,References
- Rely on the schema as the single source of truth for configuration defaults, avoiding redundant nullish coalescing operators.
There was a problem hiding this comment.
@gemini-code-assist I followed the other pre-existing options such as toolSandboxing, loadMemoryFromIncludeDirectories and others which either have || false or ?? false. Should I change nevertheless?
There was a problem hiding this comment.
Consistency with the existing codebase is important. If the current pattern in packages/cli/src/config/config.ts relies on these defensive checks, it is acceptable to follow that pattern for now to maintain consistency. While relying solely on the schema is the ideal state, refactoring existing patterns should ideally be done as a separate, codebase-wide improvement rather than within a feature PR. Please feel free to align with the existing style.
Summary
Implements
ui.keepAskUserQuestionsInHistorysettings option.Details
When ask_user tool is used, we have a nice question GUI, but after answering, the question is lost and if/when we resume a session or simply want to remember what were our choices during a session, it's impossible as there's no question visible. Gemini might still have it in it's context, but the human context is completely lost.
Related Issues
Closes #29021
How to Validate
Set the following in .gemini/settings.json
Prompt gemini to ask you a random question using
ask_usertool, examples:Pre-Merge Checklist
Examples
Current behavior/keepAskUserQuestionsInHistory=false or not set
keepAskUserQuestionsInHistory=true