feat(plan): create metrics for usage of AskUser tool#18820
Conversation
Summary of ChangesHello @Adib234, 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 application's telemetry capabilities by introducing detailed metrics for the 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
|
There was a problem hiding this comment.
Code Review
This pull request effectively adds valuable telemetry for the AskUser tool and enhances the overall telemetry system by including the active approval mode in all events. The changes are well-implemented across the board, with corresponding updates to documentation and comprehensive tests. I've identified one high-severity maintainability concern regarding how metadata is constructed in ToolCallEvent, which could lead to issues in the future. Otherwise, the implementation is solid.
|
Size Change: +1.83 kB (+0.01%) Total Size: 24.3 MB
ℹ️ View Unchanged
|
AskUser toolAskUser tool
| @@ -192,33 +192,49 @@ export class AskUserInvocation extends BaseToolInvocation< | |||
| } | |||
|
|
|||
| async execute(_signal: AbortSignal): Promise<ToolResult> { | |||
| const questionTypes = this.params.questions.map( | |||
| (q) => q.type ?? QuestionType.CHOICE, | |||
There was a problem hiding this comment.
is it defaulting to choice? the type should be a required field
There was a problem hiding this comment.
looks like question type can be undefined here, is there a better default value for this?
There was a problem hiding this comment.
ah i see, and it defaults to choice there as well - all good
cc @jackwotherspoon wonder if we should make type required
Summary
This PR implements comprehensive telemetry metrics for the AskUser tool and enhances the telemetry system to track the active approval mode across all events. These changes enable detailed analysis of user interaction patterns, question type usage, and tool effectiveness, specifically allowing for data dissection by modes like Plan mode.
Details
Related Issues
Fixes #18455
How to Validate
Searched for logs here https://cloudlogging.app.goo.gl/mr3SD483iVC3PqYGA, verified that it matches with what happened when we called
AskUsertoolPre-Merge Checklist