Skip to content

Commit 4b18f25

Browse files
authored
fix: update fileSearch toolSpec and implementation (#1320)
* fix: update fileSearch toolSpec and implementation * fix: update unit test
1 parent aaa08a4 commit 4b18f25

File tree

5 files changed

+56
-41
lines changed

5 files changed

+56
-41
lines changed

core/aws-lsp-core/src/util/workspaceUtils.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ describe('workspaceUtils', function () {
184184
const result = (
185185
await readDirectoryRecursively(testFeatures, tempFolder.path, {
186186
customFormatCallback: getEntryPath,
187-
excludeEntries: ['file4-bad', 'file2-bad'],
187+
excludeFiles: ['file4-bad', 'file2-bad'],
188188
})
189189
).sort()
190190
assert.deepStrictEqual(
@@ -209,7 +209,7 @@ describe('workspaceUtils', function () {
209209
const result = (
210210
await readDirectoryRecursively(testFeatures, tempFolder.path, {
211211
customFormatCallback: getEntryPath,
212-
excludeEntries: ['subdir12-bad'],
212+
excludeDirs: ['subdir12-bad'],
213213
})
214214
).sort()
215215
assert.deepStrictEqual(result, [subdir1.path, file1, subdir11.path, file3, subdir2.path, file2].sort())

core/aws-lsp-core/src/util/workspaceUtils.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ export async function readDirectoryRecursively(
1212
folderPath: string,
1313
options?: {
1414
maxDepth?: number
15-
excludeEntries?: string[]
15+
excludeDirs?: string[]
16+
excludeFiles?: string[]
1617
customFormatCallback?: (entry: Dirent) => string
1718
failOnError?: boolean
1819
},
1920
token?: CancellationToken
2021
): Promise<string[]> {
2122
const dirExists = await features.workspace.fs.exists(folderPath)
22-
const excludeEntries = options?.excludeEntries ?? []
23+
const excludeFiles = options?.excludeFiles ?? []
24+
const excludeDirs = options?.excludeDirs ?? []
2325
if (!dirExists) {
2426
throw new Error(`Directory does not exist: ${folderPath}`)
2527
}
@@ -58,9 +60,16 @@ export async function readDirectoryRecursively(
5860
}
5961
for (const entry of entries) {
6062
const childPath = getEntryPath(entry)
61-
if (excludeEntries.includes(entry.name)) {
62-
features.logging.log(`Skipping path ${childPath} due to match in [${excludeEntries.join(', ')}]`)
63-
continue
63+
if (entry.isDirectory()) {
64+
if (excludeDirs.includes(entry.name)) {
65+
features.logging.log(`Skipping directory ${childPath} due to match in [${excludeDirs.join(', ')}]`)
66+
continue
67+
}
68+
} else {
69+
if (excludeFiles.includes(entry.name)) {
70+
features.logging.log(`Skipping file ${childPath} due to match in [${excludeFiles.join(', ')}]`)
71+
continue
72+
}
6473
}
6574
results.push(formatter(entry))
6675
if (entry.isDirectory() && (options?.maxDepth === undefined || depth < options?.maxDepth)) {

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fileSearch.test.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ describe('FileSearch Tool', () => {
6969

7070
assert.strictEqual(result.output.kind, 'text')
7171
const lines = result.output.content.split('\n')
72-
const hasFileA = lines.some(line => line.includes('[F] ') && line.includes('fileA.txt'))
73-
const hasFileB = lines.some(line => line.includes('[F] ') && line.includes('fileB.md'))
72+
const hasFileA = lines.some(line => !line.includes('[F] ') && line.includes('fileA.txt'))
73+
const hasFileB = lines.some(line => !line.includes('[F] ') && line.includes('fileB.md'))
7474

7575
assert.ok(hasFileA, 'Should find fileA.txt matching the pattern')
7676
assert.ok(!hasFileB, 'Should not find fileB.md as it does not match the pattern')
@@ -90,9 +90,9 @@ describe('FileSearch Tool', () => {
9090

9191
assert.strictEqual(result.output.kind, 'text')
9292
const lines = result.output.content.split('\n')
93-
const hasFileA = lines.some(line => line.includes('[F] ') && line.includes('fileA.txt'))
94-
const hasFileB = lines.some(line => line.includes('[F] ') && line.includes('fileB.txt'))
95-
const hasFileC = lines.some(line => line.includes('[F] ') && line.includes('fileC.md'))
93+
const hasFileA = lines.some(line => !line.includes('[F] ') && line.includes('fileA.txt'))
94+
const hasFileB = lines.some(line => !line.includes('[F] ') && line.includes('fileB.txt'))
95+
const hasFileC = lines.some(line => !line.includes('[F] ') && line.includes('fileC.md'))
9696

9797
assert.ok(hasFileA, 'Should find fileA.txt in root directory')
9898
assert.ok(hasFileB, 'Should find fileB.txt in subfolder')
@@ -116,9 +116,9 @@ describe('FileSearch Tool', () => {
116116

117117
assert.strictEqual(result.output.kind, 'text')
118118
const lines = result.output.content.split('\n')
119-
const hasRootFile = lines.some(line => line.includes('[F] ') && line.includes('root.txt'))
120-
const hasLevel1File = lines.some(line => line.includes('[F] ') && line.includes('level1.txt'))
121-
const hasLevel2File = lines.some(line => line.includes('[F] ') && line.includes('level2.txt'))
119+
const hasRootFile = lines.some(line => !line.includes('[F] ') && line.includes('root.txt'))
120+
const hasLevel1File = lines.some(line => !line.includes('[F] ') && line.includes('level1.txt'))
121+
const hasLevel2File = lines.some(line => !line.includes('[F] ') && line.includes('level2.txt'))
122122

123123
assert.ok(hasRootFile, 'Should find root.txt in root directory')
124124
assert.ok(hasLevel1File, 'Should find level1.txt in subfolder1')
@@ -138,8 +138,8 @@ describe('FileSearch Tool', () => {
138138

139139
assert.strictEqual(result.output.kind, 'text')
140140
const lines = result.output.content.split('\n')
141-
const hasUpperFile = lines.some(line => line.includes('[F] ') && line.includes('FileUpper.txt'))
142-
const hasLowerFile = lines.some(line => line.includes('[F] ') && line.includes('fileLower.txt'))
141+
const hasUpperFile = lines.some(line => !line.includes('[F] ') && line.includes('FileUpper.txt'))
142+
const hasLowerFile = lines.some(line => !line.includes('[F] ') && line.includes('fileLower.txt'))
143143

144144
assert.ok(hasUpperFile, 'Should find FileUpper.txt with case-insensitive search')
145145
assert.ok(hasLowerFile, 'Should find fileLower.txt with case-insensitive search')
@@ -159,8 +159,8 @@ describe('FileSearch Tool', () => {
159159

160160
assert.strictEqual(result.output.kind, 'text')
161161
const lines = result.output.content.split('\n')
162-
const hasUpperFile = lines.some(line => line.includes('[F] ') && line.includes('FileUpper.txt'))
163-
const hasLowerFile = lines.some(line => line.includes('[F] ') && line.includes('fileLower.txt'))
162+
const hasUpperFile = lines.some(line => !line.includes('[F] ') && line.includes('FileUpper.txt'))
163+
const hasLowerFile = lines.some(line => !line.includes('[F] ') && line.includes('fileLower.txt'))
164164

165165
assert.ok(!hasUpperFile, 'Should not find FileUpper.txt with case-sensitive search')
166166
assert.ok(hasLowerFile, 'Should find fileLower.txt with case-sensitive search')
@@ -179,8 +179,8 @@ describe('FileSearch Tool', () => {
179179

180180
assert.strictEqual(result.output.kind, 'text')
181181
const lines = result.output.content.split('\n')
182-
const hasRegularFile = lines.some(line => line.includes('[F] ') && line.includes('regular.txt'))
183-
const hasExcludedFile = lines.some(line => line.includes('[F] ') && line.includes('excluded.txt'))
182+
const hasRegularFile = lines.some(line => !line.includes('[F] ') && line.includes('regular.txt'))
183+
const hasExcludedFile = lines.some(line => !line.includes('[F] ') && line.includes('excluded.txt'))
184184

185185
assert.ok(hasRegularFile, 'Should find regular.txt in root directory')
186186
assert.ok(!hasExcludedFile, 'Should not find excluded.txt in node_modules directory')

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fileSearch.ts

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { CommandValidation, InvokeOutput, requiresPathAcceptance, validatePath }
33
import { workspaceUtils } from '@aws/lsp-core'
44
import { Features } from '@aws/language-server-runtimes/server-interface/server'
55
import { sanitize } from '@aws/lsp-core/out/util/path'
6-
import { DEFAULT_EXCLUDE_ENTRIES } from '../../chat/constants'
6+
import { DEFAULT_EXCLUDE_DIRS, DEFAULT_EXCLUDE_FILES } from '../../chat/constants'
77

88
export interface FileSearchParams {
99
path: string
@@ -78,14 +78,17 @@ export class FileSearch {
7878
const listing = await workspaceUtils.readDirectoryRecursively(
7979
{ workspace: this.workspace, logging: this.logging },
8080
path,
81-
{ maxDepth: params.maxDepth, excludeEntries: DEFAULT_EXCLUDE_ENTRIES }
81+
{ maxDepth: params.maxDepth, excludeDirs: DEFAULT_EXCLUDE_DIRS, excludeFiles: DEFAULT_EXCLUDE_FILES }
8282
)
8383

8484
// Create regex pattern for filtering
8585
const regex = new RegExp(params.pattern, params.caseSensitive ? '' : 'i')
8686

87-
// Filter the results based on the pattern
88-
const filteredResults = listing.filter(item => regex.test(item))
87+
// Filter the file results based on the pattern
88+
const filteredResults = listing
89+
.filter(item => item.startsWith('[F] '))
90+
.map(item => item.substring(4))
91+
.filter(item => regex.test(item))
8992

9093
if (filteredResults.length === 0) {
9194
return this.createOutput(`No files matching pattern "${params.pattern}" found in ${path}`)
@@ -111,22 +114,37 @@ export class FileSearch {
111114
return {
112115
name: 'fileSearch',
113116
description:
114-
'Search for files in a directory and its subdirectories using regex patterns. It filters out build outputs such as `build/`, `out/` and `dist` and dependency directories such as `node_modules/`.\n * Results are filtered by the provided regex pattern.\n * Case sensitivity can be controlled with the caseSensitive parameter.\n * Results clearly distinguish between files, directories or symlinks with [F], [D] and [L] prefixes.',
117+
'Search for files in a directory and its subdirectories using regex patterns.\n\n' +
118+
'## Overview\n' +
119+
'This tool searches for files matching a regex pattern, ignoring common build and dependency directories.\n\n' +
120+
'## When to use\n' +
121+
'- When you need to find files with specific naming patterns\n' +
122+
'- When you need to locate files before using more targeted tools like fsRead\n' +
123+
'- When you need to search across a project structure\n\n' +
124+
'## When not to use\n' +
125+
'- When you need to search file contents\n' +
126+
'- When you already know the exact file path\n' +
127+
'- When you need to list all files in a directory (use listDirectory instead)\n\n' +
128+
'## Notes\n' +
129+
'- This tool is more effective than running a command like `find` using `executeBash` tool\n' +
130+
'- Case sensitivity can be controlled with the caseSensitive parameter\n' +
131+
'- Use the `maxDepth` parameter to control how deep the directory traversal goes',
115132
inputSchema: {
116133
type: 'object',
117134
properties: {
118135
path: {
119136
type: 'string',
120-
description: 'Absolute path to a directory, e.g., `/repo`.',
137+
description:
138+
'Absolute path to a directory, e.g. `/repo` for Unix-like system including Unix/Linux/macOS or `d:\\repo\\` for Windows',
121139
},
122140
pattern: {
123141
type: 'string',
124-
description: 'Regex pattern to match against file and directory names.',
142+
description: 'Regex pattern to match against file names.',
125143
},
126144
maxDepth: {
127145
type: 'number',
128146
description:
129-
'Maximum depth to traverse when searching directories. Use `0` to search only the specified directory, `1` to include immediate subdirectories, etc. If it is not provided, it will search all subdirectories recursively.',
147+
'Maximum depth to traverse when searching files. Use `0` to search only under the specified directory, `1` to include immediate subdirectories, etc. If it is not provided, it will search all subdirectories recursively.',
130148
},
131149
caseSensitive: {
132150
type: 'boolean',

server/aws-lsp-codewhisperer/src/language-server/chat/constants.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,6 @@ export const HELP_MESSAGE = `I'm Amazon Q, a generative AI assistant. Learn more
3131

3232
export const DEFAULT_HELP_FOLLOW_UP_PROMPT = 'How can Amazon Q help me?'
3333

34-
// TO BE DEPRECATED, use DEFAULT_EXCLUDE_DIRS and DEFAULT_EXCLUDE_FILES instead
35-
export const DEFAULT_EXCLUDE_ENTRIES = [
36-
// Dependency directories
37-
'node_modules',
38-
// Build outputs
39-
'dist',
40-
'build',
41-
'out',
42-
// OS specific files
43-
'.DS_Store',
44-
]
45-
4634
export const DEFAULT_EXCLUDE_DIRS = [
4735
// Dependency directories
4836
'node_modules',

0 commit comments

Comments
 (0)