From a54d67d4cb9f06d714dfac4b2aab82b2398f656c Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 29 Oct 2024 17:41:34 +0100 Subject: [PATCH 1/7] Filter long / invalid prompts --- src/participant/participant.ts | 2 +- src/participant/prompts/promptBase.ts | 9 +++ src/telemetry/telemetryService.ts | 9 +-- .../suite/participant/participant.test.ts | 56 +++++++++++++++++++ src/types/participantErrorTypes.ts | 7 +++ 5 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 src/types/participantErrorTypes.ts diff --git a/src/participant/participant.ts b/src/participant/participant.ts index ee5655467..f124d854b 100644 --- a/src/participant/participant.ts +++ b/src/participant/participant.ts @@ -35,7 +35,6 @@ import { } from './prompts/schema'; import { chatResultFeedbackKindToTelemetryValue, - ParticipantErrorTypes, TelemetryEventTypes, } from '../telemetry/telemetryService'; import { DocsChatbotAIService } from './docsChatbotAIService'; @@ -44,6 +43,7 @@ import formatError from '../utils/formatError'; import type { ModelInput } from './prompts/promptBase'; import { processStreamWithIdentifiers } from './streamParsing'; import type { PromptIntent } from './prompts/intent'; +import { ParticipantErrorTypes } from '../types/participantErrorTypes'; const log = createLogger('participant'); diff --git a/src/participant/prompts/promptBase.ts b/src/participant/prompts/promptBase.ts index a390770bb..bf95b4a22 100644 --- a/src/participant/prompts/promptBase.ts +++ b/src/participant/prompts/promptBase.ts @@ -4,6 +4,7 @@ import type { InternalPromptPurpose, ParticipantPromptProperties, } from '../../telemetry/telemetryService'; +import { ParticipantErrorTypes } from '../../types/participantErrorTypes'; export interface PromptArgsBase { request: { @@ -188,6 +189,14 @@ export abstract class PromptBase { } if (historyItem instanceof vscode.ChatResponseTurn) { + if ( + historyItem.result.errorDetails?.message === + ParticipantErrorTypes.FILTERED + ) { + messages.pop(); + continue; + } + let message = ''; // Skip a response to an empty user prompt message or connect message. diff --git a/src/telemetry/telemetryService.ts b/src/telemetry/telemetryService.ts index 24e4a8773..7bc389349 100644 --- a/src/telemetry/telemetryService.ts +++ b/src/telemetry/telemetryService.ts @@ -13,6 +13,7 @@ import type { NewConnectionTelemetryEventProperties } from './connectionTelemetr import type { ShellEvaluateResult } from '../types/playgroundType'; import type { StorageController } from '../storage'; import type { ParticipantResponseType } from '../participant/constants'; +import { ParticipantErrorTypes } from '../types/participantErrorTypes'; const log = createLogger('telemetry'); // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -193,14 +194,6 @@ export enum TelemetryEventTypes { PARTICIPANT_RESPONSE_GENERATED = 'Participant Response Generated', } -export enum ParticipantErrorTypes { - CHAT_MODEL_OFF_TOPIC = 'Chat Model Off Topic', - INVALID_PROMPT = 'Invalid Prompt', - FILTERED = 'Filtered by Responsible AI Service', - OTHER = 'Other', - DOCS_CHATBOT_API = 'Docs Chatbot API Issue', -} - /** * This controller manages telemetry. */ diff --git a/src/test/suite/participant/participant.test.ts b/src/test/suite/participant/participant.test.ts index 61eeb5c6f..bf4813fef 100644 --- a/src/test/suite/participant/participant.test.ts +++ b/src/test/suite/participant/participant.test.ts @@ -34,6 +34,7 @@ import { Prompts } from '../../../participant/prompts'; import { createMarkdownLink } from '../../../participant/markdown'; import EXTENSION_COMMANDS from '../../../commands'; import { getContentLength } from '../../../participant/prompts/promptBase'; +import { ParticipantErrorTypes } from '../../../types/participantErrorTypes'; // The Copilot's model in not available in tests, // therefore we need to mock its methods and returning values. @@ -2125,6 +2126,61 @@ Schema: getContentLength(messages[1]) ); }); + + suite('with invalid messages', function () { + test('filters disallowed messages', async function () { + const chatRequestMock = { + prompt: 'find all docs by a name example', + }; + + chatContextStub = { + history: [ + Object.assign(Object.create(vscode.ChatRequestTurn.prototype), { + prompt: 'give me the count of all people in the prod database', + command: 'query', + references: [], + participant: CHAT_PARTICIPANT_ID, + }), + Object.assign(Object.create(vscode.ChatRequestTurn.prototype), { + prompt: 'some disallowed message', + command: 'query', + references: [], + participant: CHAT_PARTICIPANT_ID, + }), + Object.assign(Object.create(vscode.ChatResponseTurn.prototype), { + result: { + errorDetails: { + message: ParticipantErrorTypes.FILTERED, + }, + }, + response: [], + participant: CHAT_PARTICIPANT_ID, + }), + Object.assign(Object.create(vscode.ChatRequestTurn.prototype), { + prompt: 'ok message', + references: [], + participant: CHAT_PARTICIPANT_ID, + }), + ], + }; + const { messages } = await Prompts.generic.buildMessages({ + context: chatContextStub, + request: chatRequestMock, + connectionNames: [], + }); + + expect(messages).to.have.lengthOf(4); + expect( + messages.find((message) => + message.content.includes('some disallowed message') + ) + ).to.be.undefined; + + expect( + messages.find((message) => message.content.includes('ok message')) + ).to.not.be.undefined; + }); + }); }); suite('telemetry', function () { diff --git a/src/types/participantErrorTypes.ts b/src/types/participantErrorTypes.ts new file mode 100644 index 000000000..1b2a6ce83 --- /dev/null +++ b/src/types/participantErrorTypes.ts @@ -0,0 +1,7 @@ +export enum ParticipantErrorTypes { + CHAT_MODEL_OFF_TOPIC = 'Chat Model Off Topic', + INVALID_PROMPT = 'Invalid Prompt', + FILTERED = 'Filtered by Responsible AI Service', + OTHER = 'Other', + DOCS_CHATBOT_API = 'Docs Chatbot API Issue', +} From 6ff0d8d5f2561fa5f37af15afe6fbd2750a3d300 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 30 Oct 2024 11:35:24 +0100 Subject: [PATCH 2/7] Try helper include --- src/test/suite/participant/participant.test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/test/suite/participant/participant.test.ts b/src/test/suite/participant/participant.test.ts index bf4813fef..01948b1ba 100644 --- a/src/test/suite/participant/participant.test.ts +++ b/src/test/suite/participant/participant.test.ts @@ -2170,14 +2170,28 @@ Schema: }); expect(messages).to.have.lengthOf(4); + + const contentIncludes = ( + message: unknown, + pattern: string + ): boolean => { + if (Array.isArray(message)) { + return message.find((sub) => sub.includes(pattern)); + } + if (message instanceof String) { + return message.includes(pattern); + } + throw new Error('Unhandled message content type'); + }; + expect( messages.find((message) => - message.content.includes('some disallowed message') + contentIncludes(message, 'some disallowed message') ) ).to.be.undefined; expect( - messages.find((message) => message.content.includes('ok message')) + messages.find((message) => contentIncludes(message, 'ok message')) ).to.not.be.undefined; }); }); From c9aa589bbead5297b2757127fc2a0864611e6713 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 30 Oct 2024 11:45:08 +0100 Subject: [PATCH 3/7] Use content value --- .../suite/participant/participant.test.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/test/suite/participant/participant.test.ts b/src/test/suite/participant/participant.test.ts index 01948b1ba..91088fab4 100644 --- a/src/test/suite/participant/participant.test.ts +++ b/src/test/suite/participant/participant.test.ts @@ -2171,17 +2171,18 @@ Schema: expect(messages).to.have.lengthOf(4); - const contentIncludes = ( - message: unknown, - pattern: string - ): boolean => { - if (Array.isArray(message)) { - return message.find((sub) => sub.includes(pattern)); + // VSCode's messages' content could either be array of values or a string + // This helper supports both. + const contentIncludes = (message: any, pattern: string): boolean => { + if (Array.isArray(message.content)) { + return message.content.find((sub: { value: string }) => + sub.value.includes(pattern) + ); } - if (message instanceof String) { - return message.includes(pattern); + if (message.content instanceof String) { + return message.content.includes(pattern); } - throw new Error('Unhandled message content type'); + throw new Error(`Unhandled message content type ${message}`); }; expect( From 698051f36fb1e3d7241f24084b4723970c0a263d Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 30 Oct 2024 11:56:49 +0100 Subject: [PATCH 4/7] Use a cleaner test --- .../suite/participant/participant.test.ts | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/test/suite/participant/participant.test.ts b/src/test/suite/participant/participant.test.ts index 91088fab4..5b8566abb 100644 --- a/src/test/suite/participant/participant.test.ts +++ b/src/test/suite/participant/participant.test.ts @@ -2185,15 +2185,21 @@ Schema: throw new Error(`Unhandled message content type ${message}`); }; - expect( - messages.find((message) => - contentIncludes(message, 'some disallowed message') - ) - ).to.be.undefined; - - expect( - messages.find((message) => contentIncludes(message, 'ok message')) - ).to.not.be.undefined; + const messageContents = messages.map((message) => { + // There may be different types for the messages' content + const content = Array.isArray(message.content) + ? message.content.flatMap((sub) => sub.value).join('') + : message.content; + + return content; + }); + + // Skip the preset prompt and check that the rest are correct. + expect(messageContents.slice(1)).deep.equal([ + 'give me the count of all people in the prod database', + 'ok message', + 'find all docs by a name example', + ]); }); }); }); From bf05722958daa41a53ea6e34e1590ccddfd61a61 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 30 Oct 2024 12:39:41 +0100 Subject: [PATCH 5/7] delete old helper --- src/test/suite/participant/participant.test.ts | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/test/suite/participant/participant.test.ts b/src/test/suite/participant/participant.test.ts index 5b8566abb..6c5c62d47 100644 --- a/src/test/suite/participant/participant.test.ts +++ b/src/test/suite/participant/participant.test.ts @@ -2171,24 +2171,10 @@ Schema: expect(messages).to.have.lengthOf(4); - // VSCode's messages' content could either be array of values or a string - // This helper supports both. - const contentIncludes = (message: any, pattern: string): boolean => { - if (Array.isArray(message.content)) { - return message.content.find((sub: { value: string }) => - sub.value.includes(pattern) - ); - } - if (message.content instanceof String) { - return message.content.includes(pattern); - } - throw new Error(`Unhandled message content type ${message}`); - }; - const messageContents = messages.map((message) => { // There may be different types for the messages' content const content = Array.isArray(message.content) - ? message.content.flatMap((sub) => sub.value).join('') + ? message.content.map((sub) => sub.value).join('') : message.content; return content; From 2445dc5449519a47fc708d3f2676abcd2cc3eedb Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 30 Oct 2024 13:00:01 +0100 Subject: [PATCH 6/7] Add explainer --- src/participant/prompts/promptBase.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/participant/prompts/promptBase.ts b/src/participant/prompts/promptBase.ts index bf95b4a22..240a3159f 100644 --- a/src/participant/prompts/promptBase.ts +++ b/src/participant/prompts/promptBase.ts @@ -193,6 +193,8 @@ export abstract class PromptBase { historyItem.result.errorDetails?.message === ParticipantErrorTypes.FILTERED ) { + // If the response led to a filtered error, we do not want the + // error-causing message to be sent again so we remove it. messages.pop(); continue; } From 53ffe8148ce3c6516ace9fa8e51061302491be51 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 6 Nov 2024 17:50:58 +0100 Subject: [PATCH 7/7] move error types inside participant --- src/participant/participant.ts | 2 +- src/participant/prompts/promptBase.ts | 2 +- src/telemetry/telemetryService.ts | 2 +- src/test/suite/participant/participant.test.ts | 2 +- src/{types => test/suite/participant}/participantErrorTypes.ts | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename src/{types => test/suite/participant}/participantErrorTypes.ts (100%) diff --git a/src/participant/participant.ts b/src/participant/participant.ts index f124d854b..19f7457d1 100644 --- a/src/participant/participant.ts +++ b/src/participant/participant.ts @@ -43,7 +43,7 @@ import formatError from '../utils/formatError'; import type { ModelInput } from './prompts/promptBase'; import { processStreamWithIdentifiers } from './streamParsing'; import type { PromptIntent } from './prompts/intent'; -import { ParticipantErrorTypes } from '../types/participantErrorTypes'; +import { ParticipantErrorTypes } from '../test/suite/participant/participantErrorTypes'; const log = createLogger('participant'); diff --git a/src/participant/prompts/promptBase.ts b/src/participant/prompts/promptBase.ts index 240a3159f..40c430eec 100644 --- a/src/participant/prompts/promptBase.ts +++ b/src/participant/prompts/promptBase.ts @@ -4,7 +4,7 @@ import type { InternalPromptPurpose, ParticipantPromptProperties, } from '../../telemetry/telemetryService'; -import { ParticipantErrorTypes } from '../../types/participantErrorTypes'; +import { ParticipantErrorTypes } from '../../test/suite/participant/participantErrorTypes'; export interface PromptArgsBase { request: { diff --git a/src/telemetry/telemetryService.ts b/src/telemetry/telemetryService.ts index 7bc389349..8e0482606 100644 --- a/src/telemetry/telemetryService.ts +++ b/src/telemetry/telemetryService.ts @@ -13,7 +13,7 @@ import type { NewConnectionTelemetryEventProperties } from './connectionTelemetr import type { ShellEvaluateResult } from '../types/playgroundType'; import type { StorageController } from '../storage'; import type { ParticipantResponseType } from '../participant/constants'; -import { ParticipantErrorTypes } from '../types/participantErrorTypes'; +import { ParticipantErrorTypes } from '../test/suite/participant/participantErrorTypes'; const log = createLogger('telemetry'); // eslint-disable-next-line @typescript-eslint/no-var-requires diff --git a/src/test/suite/participant/participant.test.ts b/src/test/suite/participant/participant.test.ts index 6c5c62d47..770e32ef9 100644 --- a/src/test/suite/participant/participant.test.ts +++ b/src/test/suite/participant/participant.test.ts @@ -34,7 +34,7 @@ import { Prompts } from '../../../participant/prompts'; import { createMarkdownLink } from '../../../participant/markdown'; import EXTENSION_COMMANDS from '../../../commands'; import { getContentLength } from '../../../participant/prompts/promptBase'; -import { ParticipantErrorTypes } from '../../../types/participantErrorTypes'; +import { ParticipantErrorTypes } from './participantErrorTypes'; // The Copilot's model in not available in tests, // therefore we need to mock its methods and returning values. diff --git a/src/types/participantErrorTypes.ts b/src/test/suite/participant/participantErrorTypes.ts similarity index 100% rename from src/types/participantErrorTypes.ts rename to src/test/suite/participant/participantErrorTypes.ts