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

feat(chat): update schema assistant prompt to handle empty and short prompts better VSCODE-648 #874

Merged
merged 5 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions src/participant/participant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1187,15 +1187,17 @@ export default class ParticipantController {
});
}

if (
Prompts.isPromptEmpty(request) &&
this._doesLastMessageAskForNamespace(context.history)
) {
return this.handleEmptyNamespaceMessage({
command: '/schema',
context,
stream,
});
if (Prompts.isPromptEmpty(request)) {
if (this._doesLastMessageAskForNamespace(context.history)) {
return this.handleEmptyNamespaceMessage({
command: '/schema',
context,
stream,
});
}

stream.markdown(Prompts.schema.emptyRequestResponse);
return emptyRequestChatResult(context.history);
}

const namespace = await this._getNamespaceFromChat({
Expand Down
14 changes: 11 additions & 3 deletions src/participant/prompts/schema.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { UserPromptResponse } from './promptBase';
import { PromptBase, type PromptArgsBase } from './promptBase';
import * as vscode from 'vscode';

export const DOCUMENTS_TO_SAMPLE_FOR_SCHEMA_PROMPT = 100;

Expand All @@ -21,9 +22,10 @@ export class SchemaPrompt extends PromptBase<SchemaPromptArgs> {
return `You are a senior engineer who describes the schema of documents in a MongoDB database.
The schema is generated from a sample of documents in the user's collection.
You must follow these rules.
Rule 1: Try to be as concise as possible.
Rule 2: Pay attention to the JSON schema.
Rule 3: Mention the amount of documents sampled in your response.
Rule 1: Your answer should always describe the schema of documents in the collection.
gagik marked this conversation as resolved.
Show resolved Hide resolved
Rule 2: Try to be as concise as possible.
Rule 3: Pay attention to the JSON schema.
Rule 4: Mention the amount of documents sampled in your response.
Amount of documents sampled: ${amountOfDocumentsSampled}.`;
}

Expand All @@ -44,4 +46,10 @@ ${schema}`,
hasSampleDocs: false,
});
}

get emptyRequestResponse(): string {
return vscode.l10n.t(
'Please specify a question when using this command. Usage: @MongoDB /schema what is the schema for the sample_mflix.users collection?'
);
}
}
82 changes: 26 additions & 56 deletions src/test/suite/participant/participant.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,19 @@ suite('Participant Controller Test Suite', function () {
});
});

test('returns a special response with an empty prompt', async function () {
await invokeChatHandler({
prompt: '',
command: 'query',
references: [],
});

expect(chatStreamStub.markdown.calledOnce).is.true;
expect(chatStreamStub.markdown.getCall(0).firstArg).equals(
Prompts.query.emptyRequestResponse
);
});

test('generates a query', async function () {
const chatRequestMock = {
prompt: 'find all docs by a name example',
Expand Down Expand Up @@ -1300,21 +1313,6 @@ suite('Participant Controller Test Suite', function () {
});
});

test('without a prompt it asks for the database name without pinging ai', async function () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this behavior becomes outdated; unless I am overlooking something, we do not gain too much by asking for the namespace if the user just runs /schema. It'd be nicer to show an explainer first to help them discover the syntax.

Copy link
Member

Choose a reason for hiding this comment

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

If I remember correctly this was intentional. We want the user to be able to get the schema of a collection by just writing /schema and then clicking their namespace.

Copy link
Contributor Author

@gagik gagik Nov 19, 2024

Choose a reason for hiding this comment

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

Hm I see. I can see how it's convenient but this does also make it hard for a user to actually discover what the /schema command expects as an argument (something which we could definitely instead handle by introducing something like /help or equivalent for internal extension info or questions). Seems like the only way to discover that you can also mention another collection or ask a question, etc. with the /schema is through reading documentation.

I will remove the empty message response and see if the new rule works good enough when the prompt is empty

const chatRequestMock = {
prompt: '',
command: 'schema',
references: [],
};
await invokeChatHandler(chatRequestMock);

expect(sendRequestStub.called).to.be.false;
const askForDBMessage = chatStreamStub.markdown.getCall(0).args[0];
expect(askForDBMessage).to.include(
'Which database would you like to use? Select one by either clicking on an item in the list or typing the name manually in the chat.\n\n'
);
});

test('with a prompt it asks the ai for the namespace', async function () {
const chatRequestMock = {
prompt: 'pineapple',
Expand All @@ -1335,47 +1333,6 @@ suite('Participant Controller Test Suite', function () {
'Which database would you like to use? Select one by either clicking on an item in the list or typing the name manually in the chat.\n\n'
);
});

test('with history, and a blank prompt, it sets a message so it does not cause model error (VSCODE-626)', async function () {
gagik marked this conversation as resolved.
Show resolved Hide resolved
const chatRequestMock = {
prompt: '',
command: 'schema',
references: [],
};
chatContextStub = {
history: [
createChatRequestTurn(
'/query',
'how do I make a find request vs favorite_fruits.pineapple?'
),
createChatResponseTurn('/query', {
response: [
{
value: { value: 'some code' } as vscode.MarkdownString,
},
],
result: {
metadata: {
intent: 'query',
chatId: 'abc',
},
},
}),
],
};
await invokeChatHandler(chatRequestMock);

expect(sendRequestStub.calledOnce).to.be.true;

const messages = sendRequestStub.firstCall
.args[0] as vscode.LanguageModelChatMessage[];
expect(getMessageContent(messages[0])).to.include(
'Parse all user messages to find a database name and a collection name.'
);
expect(getMessageContent(messages[3])).to.include(
'see previous messages'
);
});
});

suite(
Expand All @@ -1387,6 +1344,19 @@ suite('Participant Controller Test Suite', function () {
});
});

test('returns a special response with an empty prompt', async function () {
await invokeChatHandler({
prompt: '',
command: 'schema',
references: [],
});

expect(chatStreamStub.markdown.calledOnce).is.true;
expect(chatStreamStub.markdown.getCall(0).firstArg).equals(
Prompts.schema.emptyRequestResponse
);
});

test('shows a button to view the json output', async function () {
const chatRequestMock = {
prompt: 'what is my schema',
Expand Down
Loading