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

Conversation

gagik
Copy link
Contributor

@gagik gagik commented Nov 18, 2024

Description

Sometimes, when using the /query command to build a query, and then at some point using the /schema to get information about the schema of a collection without opening a new chat window, Copilot keeps thinking that a user wants to build a query. This PR tries to fix this by introducing a blank message case handling and modifying the prompt to make it clearer the schema command should always output schemas.

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)

src/participant/prompts/schema.ts Show resolved Hide resolved
src/test/suite/participant/participant.test.ts Outdated Show resolved Hide resolved
@@ -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

@gagik gagik requested review from Anemy and alenakhineika November 18, 2024 14:15
@gagik gagik marked this pull request as ready for review November 18, 2024 14:15
@gagik gagik changed the title feat(chat): handle empty and short prompts with the schema command better feat(chat): handle empty and short prompts with the schema command better VSCODE-648 Nov 18, 2024
@gagik
Copy link
Contributor Author

gagik commented Nov 20, 2024

The new rule actually works pretty great even with empty prompts in my manual testing, so I'll revert the changes for empty prompt and just go ahead with the rule change.

@gagik gagik changed the title feat(chat): handle empty and short prompts with the schema command better VSCODE-648 feat(chat): update schema assistant prompt to handle empty and short prompts better VSCODE-648 Nov 20, 2024
@gagik gagik merged commit ce1f763 into main Nov 20, 2024
7 checks passed
@gagik gagik deleted the gagik/schema-corner-cases branch November 20, 2024 12:17
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