Skip to content

track latency from request -> showing terminal completions, indicate first shown for shell type and for window #252007

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

Merged
merged 22 commits into from
Jun 20, 2025

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Jun 20, 2025

part of #251994

Screenshot 2025-06-20 at 4 09 34 PM

@meganrogge meganrogge requested a review from Tyriar June 20, 2025 17:27
@meganrogge meganrogge self-assigned this Jun 20, 2025
@meganrogge meganrogge added this to the June 2025 milestone Jun 20, 2025
Tyriar
Tyriar previously requested changes Jun 20, 2025
@meganrogge meganrogge requested a review from Tyriar June 20, 2025 20:10
@meganrogge meganrogge changed the title track latency from request -> showing terminal completions track latency from request -> showing terminal completions, indicate first shown for shell type and for window Jun 20, 2025
@meganrogge meganrogge enabled auto-merge (squash) June 20, 2025 20:15
@meganrogge meganrogge requested a review from Copilot June 20, 2025 21:07
Copy link

@Copilot Copilot AI left a 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 adds telemetry to measure latency from a terminal completion request to when suggestions are shown and flags the first time completions are displayed per window and per shell type.

  • Introduces logCompletionLatency in TerminalSuggestTelemetry to send latency and first-shown flags.
  • Updates SuggestAddon to timestamp requests, compute/display latency, and track first-shown state.
  • Adds a new FirstShown storage key in terminalStorageKeys.ts.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestTelemetry.ts Added logCompletionLatency method with its telemetry schema.
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestAddon.ts Implemented request timestamping, first-shown determination, and telemetry logging.
src/vs/workbench/contrib/terminal/common/terminalStorageKeys.ts Added FirstShown enum entry for storing first-shown state.
Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestTelemetry.ts:58

  • The new logCompletionLatency method adds critical telemetry logic but lacks accompanying unit or integration tests. Please add tests to verify the correct payload is sent under various conditions (e.g., first vs. subsequent invocations).
	logCompletionLatency(sessionId: string, latency: number, firstShownFor: { window: boolean; shell: boolean }): void {

meganrogge and others added 2 commits June 20, 2025 17:11
…nalSuggestAddon.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@meganrogge meganrogge disabled auto-merge June 20, 2025 21:15
@meganrogge meganrogge enabled auto-merge (squash) June 20, 2025 21:22
@meganrogge
Copy link
Contributor Author

Just realized a set for the shell types might be better

Tyriar
Tyriar previously approved these changes Jun 20, 2025
@meganrogge meganrogge merged commit e55ba96 into main Jun 20, 2025
8 checks passed
@meganrogge meganrogge deleted the merogge/first-show branch June 20, 2025 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants