-
Notifications
You must be signed in to change notification settings - Fork 730
fix(amazonq): add telemetryHelper back #7975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(amazonq): add telemetryHelper back #7975
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR restores the TelemetryHelper class functionality by removing placeholder implementations and adding back the actual telemetry recording methods for CodeWhisperer. The PR addresses the "fix(amazonq): add telemetryHelper back" objective by replacing stub methods with complete implementations.
- Replaces method stubs that threw "Method not implemented" errors with full implementations
- Adds comprehensive telemetry data collection and emission for user decisions and trigger events
- Integrates with various utility modules and telemetry services for complete functionality
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| private resetUserTriggerDecisionTelemetry() { | ||
| this.sessionDecisions = [] | ||
| this.triggerChar = '' |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting triggerChar to empty string instead of undefined is inconsistent with its type declaration (line 51) and initialization (line 51). This should be this.triggerChar = undefined to maintain type consistency.
| this.triggerChar = '' | |
| this.triggerChar = undefined |
| let acceptedRecommendationContent | ||
| if (acceptIndex !== -1 && recommendations[acceptIndex] !== undefined) { | ||
| acceptedRecommendationContent = recommendations[acceptIndex].content | ||
| } else { | ||
| acceptedRecommendationContent = '' | ||
| } |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The TODO comment suggests using a ternary operator, which would make this code more concise and readable: const acceptedRecommendationContent = (acceptIndex !== -1 && recommendations[acceptIndex]) ? recommendations[acceptIndex].content : ''
| let acceptedRecommendationContent | |
| if (acceptIndex !== -1 && recommendations[acceptIndex] !== undefined) { | |
| acceptedRecommendationContent = recommendations[acceptIndex].content | |
| } else { | |
| acceptedRecommendationContent = '' | |
| } | |
| const acceptedRecommendationContent = | |
| acceptIndex !== -1 && recommendations[acceptIndex] | |
| ? recommendations[acceptIndex].content | |
| : '' |
| private getAggregatedSuggestionReferenceCount( | ||
| events: CodewhispererUserDecision[] | ||
| // if there is reference for accepted recommendation within the session, mark the reference number | ||
| // as 1, otherwise mark the session as 0 |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function documentation comment is incorrectly placed after the parameter list instead of before the function declaration. Move the comment above line 506 and format it as a proper JSDoc comment.
| private getAggregatedSuggestionReferenceCount( | |
| events: CodewhispererUserDecision[] | |
| // if there is reference for accepted recommendation within the session, mark the reference number | |
| // as 1, otherwise mark the session as 0 | |
| /** | |
| * If there is reference for accepted recommendation within the session, mark the reference number | |
| * as 1, otherwise mark the session as 0. | |
| */ | |
| private getAggregatedSuggestionReferenceCount( | |
| events: CodewhispererUserDecision[] |
| private getAggregatedSuggestionState( | ||
| // if there is any Accept within the session, mark the session as Accept | ||
| // if there is any Reject within the session, mark the session as Reject | ||
| // if all recommendations within the session are empty, mark the session as Empty | ||
| // otherwise mark the session as Discard |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function documentation comment is incorrectly placed before the parameter list instead of before the function declaration. Move the comment above line 475 and format it as a proper JSDoc comment.
| private getAggregatedSuggestionState( | |
| // if there is any Accept within the session, mark the session as Accept | |
| // if there is any Reject within the session, mark the session as Reject | |
| // if all recommendations within the session are empty, mark the session as Empty | |
| // otherwise mark the session as Discard | |
| /** | |
| * Aggregates the suggestion states for a session: | |
| * - If there is any Accept within the session, marks the session as Accept. | |
| * - If there is any Reject within the session, marks the session as Reject. | |
| * - If all recommendations within the session are empty, marks the session as Empty. | |
| * - Otherwise, marks the session as Discard. | |
| */ | |
| private getAggregatedSuggestionState( |
## Problem add back test file for telemetryHelper.ts Test file for #7975 Ref: #7906 ## Solution --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
Problem
Solution
feature/xbranches will not be squash-merged at release time.