Skip to content

Conversation

eneufeld
Copy link
Contributor

What it does

fixes #15467

How to test

Follow-ups

Breaking changes

  • This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

Attribution

Review checklist

Reminder for reviewers

@github-project-automation github-project-automation bot moved this to Waiting on reviewers in PR Backlog Apr 22, 2025
Copy link
Contributor

@planger planger left a comment

Choose a reason for hiding this comment

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

Very cool, thank you! Works already pretty well. I think we'll need a bit of prompt tweaking to help the LLM making best use of this. But this is something we probably need to play with in more detail over time (observing multiple search scenarios).

},
required: ['query', 'useRegExp']
},
handler: (argString: string) => this.handleSearch(argString)
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth looking for a cancellation token to ensure long running searches can be canceled by the user if the user cancels the response:

handler: async (args: string, ctx?: MutableChatRequestModel): Promise<string> => {
                const token = ctx?.response?.cancellationToken;
...

return {
id: SEARCH_IN_WORKSPACE_ID,
name: SEARCH_IN_WORKSPACE_ID,
description: 'Searches the workspace for files containing the provided query string or regular expression.',
Copy link
Contributor

Choose a reason for hiding this comment

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

I had a few cases where the AI tried to search for multi-word search strings. So we should probably guide the AI here a bit better how to use this search tool. Maybe the following works better?

Suggested change
description: 'Searches the workspace for files containing the provided query string or regular expression.',
description: 'Searches the workspace for files matching the given search term (parameter `query`), using case-insensitive full-string or regex matching. Multi-word patterns must match exactly (including spaces, but case-insensitive). For best results, prefer multiple queries with alternative search terms, over single long or complex ones.',

But I didn't deeply test this.

@github-project-automation github-project-automation bot moved this from Waiting on reviewers to Needs merge in PR Backlog Apr 22, 2025
@JonasHelming JonasHelming mentioned this pull request Apr 22, 2025
13 tasks
Copy link
Contributor

@JonasHelming JonasHelming left a comment

Choose a reason for hiding this comment

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

LGTM (with Philips feedback)

};
}

private async handleSearch(argString: string, cancellationToken: CancellationToken): Promise<string> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private async handleSearch(argString: string, cancellationToken: CancellationToken): Promise<string> {
private async handleSearch(argString: string, cancellationToken?: CancellationToken): Promise<string> {

this.searchService.search(args.query, callbacks, options)
.then(id => {
expectedSearchId = id;
cancellationToken.onCancellationRequested(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cancellationToken.onCancellationRequested(() => {
cancellationToken?.onCancellationRequested(() => {

@planger
Copy link
Contributor

planger commented Apr 24, 2025

Note that #15504 is based on this PR, so we only need to merge #15504

@eneufeld
Copy link
Contributor Author

fixed as part of #15504

@eneufeld eneufeld closed this Apr 25, 2025
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Allow Coder to search workspace
3 participants