Skip to content
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 intent telemetry (#6779) #6795

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

janhartman
Copy link
Contributor

@janhartman janhartman commented Jan 24, 2025

This fixes a regression in the intent telemetry (described here, originally fixed in #6637). Two fixes:

  • we weren't using the right fields in commonProps, meaning that the values weren't assigned to the message and thus weren't accessible in the chat controller where we emit metadata.
  • we were accidentally overriding the userSpecifiedIntent field with auto because of operator precedence.

We now get the correct data emitted in the chat/executed telemetry events.

I want to backport this to vscode-1.64 and jb-7.12. Why? Telemetry data will help us improve intent detection significantly and it's important we have the pipeline functioning well at launch. Furthermore, the changes are quite low-risk.

Test plan

Tested manually.

(cherry picked from commit 1049a20)

This fixes a regression in the intent telemetry (described
[here](https://linear.app/sourcegraph/issue/SPLF-330), originally fixed
in #6637).
Two fixes:
- we weren't using the right fields in `commonProps`, meaning that the
values weren't assigned to the message and thus weren't accessible in
the chat controller where we emit metadata.
- we were accidentally overriding the `userSpecifiedIntent` field with
`auto` because of operator precedence.

We now get the correct data emitted in the `chat/executed` telemetry
events.

I want to backport this to vscode-1.64 and jb-7.12.
Why? Telemetry data will help us improve intent detection significantly
and it's important we have the pipeline functioning well at launch.
Furthermore, the changes are quite low-risk.

Tested manually.

(cherry picked from commit 1049a20)
@janhartman janhartman requested review from ErikaRS and umpox January 24, 2025 10:21
Copy link
Contributor

@umpox umpox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I'll let @beyang or @ErikaRS approve here before we backport

Copy link

@ErikaRS ErikaRS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved for backport. Since this is fixing a regression, we don't need review from Beyang.

@janhartman janhartman merged commit 1e9fffe into vscode-v1.64.x Jan 24, 2025
21 of 23 checks passed
@janhartman janhartman deleted the backport-6779-to-vscode-v1.64.x branch January 24, 2025 16:45
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.

3 participants