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: filter long and invalid prompts in future messages VSCODE-614 #861

Merged
merged 7 commits into from
Nov 7, 2024

Conversation

gagik
Copy link
Contributor

@gagik gagik commented Oct 29, 2024

Description

When someone sends a long message (or some message which could cause it to be i.e. filtered by the responsible AI service), we keep resending this in future prompts, causing the chat to be broken for the whole session. This PR excludes them from the results.

Checklist

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@@ -193,14 +194,6 @@ export enum TelemetryEventTypes {
PARTICIPANT_RESPONSE_GENERATED = 'Participant Response Generated',
}

export enum ParticipantErrorTypes {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured it'd be better to get the error types out of the scope of telemetry since it might be useful to handle specific errors.

src/participant/prompts/promptBase.ts Show resolved Hide resolved
@gagik gagik force-pushed the gagik/fix-long-prompts branch from 6a0a0c9 to bf05722 Compare October 30, 2024 11:40
@gagik gagik marked this pull request as ready for review October 30, 2024 11:48
@gagik gagik requested a review from alenakhineika October 30, 2024 11:48
@alenakhineika
Copy link
Contributor

I am thinking, what if instead of removing the long message from the history we should introduce a max prompt size, similar like we do in compass, and inform users after submission, that their prompt is too long. We have even mentioned doing something like this in tech design in the "Building Chat Participant" section. @Anemy @GaurabAryal do you maybe have some thoughts?

@gagik
Copy link
Contributor Author

gagik commented Oct 30, 2024

I am thinking, what if instead of removing the long message from the history we should introduce a max prompt size, similar like we do in compass, and inform users after submission, that their prompt is too long. We have even mentioned doing something like this in tech design in the "Building Chat Participant" section. @Anemy @GaurabAryal do you maybe have some thoughts?

That'd be good for that specific case, which should be common enough to be worth handling.

Beyond that I assume we may still end up in scenario where i.e. maybe the user has some bad words which are filtered by the Responsible AI Service (which I think does do filtering beyond just prompt size), in which case we'd probably want to dismiss its contents in the following messages; otherwise again we'll leave the user stuck for the whole chat just because they made a mistake. I read people complain about the filter before, so this might be quite annoying.

@nirinchev
Copy link
Contributor

I agree - we probably want to do both. Note that the max prompt size concerns both the user prompt, but also the history, so if a user has a very long conversation, they may end up exceeding the limit, even if their last message was a short one. That means we'll need to build out a system for dropping the oldest history messages.

@gagik gagik changed the title fix: Filter long and invalid prompts in future messages VSCODE-614 fix: filter long and invalid prompts in future messages VSCODE-614 Nov 1, 2024
Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

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

lgtm, nice test add

src/types/participantErrorTypes.ts Outdated Show resolved Hide resolved
@gagik gagik merged commit 6ba9dd2 into main Nov 7, 2024
6 checks passed
@gagik gagik deleted the gagik/fix-long-prompts branch November 7, 2024 10:02
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.

5 participants