Skip to content

fix: update fileSearch toolSpec and implementation #1320

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 13, 2025
Merged

fix: update fileSearch toolSpec and implementation #1320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b532932
fix: update fileSearch toolSpec and implementation
jguoamz May 13, 2025
2552aa6
fix: update unit test
jguoamz May 13, 2025
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
4 changes: 2 additions & 2 deletions core/aws-lsp-core/src/util/workspaceUtils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe('workspaceUtils', function () {
const result = (
await readDirectoryRecursively(testFeatures, tempFolder.path, {
customFormatCallback: getEntryPath,
excludeEntries: ['file4-bad', 'file2-bad'],
excludeFiles: ['file4-bad', 'file2-bad'],
})
).sort()
assert.deepStrictEqual(
Expand All @@ -209,7 +209,7 @@ describe('workspaceUtils', function () {
const result = (
await readDirectoryRecursively(testFeatures, tempFolder.path, {
customFormatCallback: getEntryPath,
excludeEntries: ['subdir12-bad'],
excludeDirs: ['subdir12-bad'],
})
).sort()
assert.deepStrictEqual(result, [subdir1.path, file1, subdir11.path, file3, subdir2.path, file2].sort())
Expand Down
19 changes: 14 additions & 5 deletions core/aws-lsp-core/src/util/workspaceUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ export async function readDirectoryRecursively(
folderPath: string,
options?: {
maxDepth?: number
excludeEntries?: string[]
excludeDirs?: string[]
excludeFiles?: string[]
customFormatCallback?: (entry: Dirent) => string
failOnError?: boolean
},
token?: CancellationToken
): Promise<string[]> {
const dirExists = await features.workspace.fs.exists(folderPath)
const excludeEntries = options?.excludeEntries ?? []
const excludeFiles = options?.excludeFiles ?? []
const excludeDirs = options?.excludeDirs ?? []
Comment on lines +23 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

is the listDirectory use the same workspaceUtils.ts, do we need to update the listDirectory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does not use the same method. It uses a new method that is already updated

if (!dirExists) {
throw new Error(`Directory does not exist: ${folderPath}`)
}
Expand Down Expand Up @@ -58,9 +60,16 @@ export async function readDirectoryRecursively(
}
for (const entry of entries) {
const childPath = getEntryPath(entry)
if (excludeEntries.includes(entry.name)) {
features.logging.log(`Skipping path ${childPath} due to match in [${excludeEntries.join(', ')}]`)
continue
if (entry.isDirectory()) {
if (excludeDirs.includes(entry.name)) {
features.logging.log(`Skipping directory ${childPath} due to match in [${excludeDirs.join(', ')}]`)
continue
}
} else {
if (excludeFiles.includes(entry.name)) {
features.logging.log(`Skipping file ${childPath} due to match in [${excludeFiles.join(', ')}]`)
continue
}
}
results.push(formatter(entry))
if (entry.isDirectory() && (options?.maxDepth === undefined || depth < options?.maxDepth)) {
Expand Down
28 changes: 14 additions & 14 deletions server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fileSearch.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ describe('FileSearch Tool', () => {

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

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

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

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

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

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

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

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

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

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

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

assert.ok(hasRegularFile, 'Should find regular.txt in root directory')
assert.ok(!hasExcludedFile, 'Should not find excluded.txt in node_modules directory')
Expand Down
34 changes: 26 additions & 8 deletions server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fileSearch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { CommandValidation, InvokeOutput, requiresPathAcceptance, validatePath }
import { workspaceUtils } from '@aws/lsp-core'
import { Features } from '@aws/language-server-runtimes/server-interface/server'
import { sanitize } from '@aws/lsp-core/out/util/path'
import { DEFAULT_EXCLUDE_ENTRIES } from '../../chat/constants'
import { DEFAULT_EXCLUDE_DIRS, DEFAULT_EXCLUDE_FILES } from '../../chat/constants'

export interface FileSearchParams {
path: string
Expand Down Expand Up @@ -78,14 +78,17 @@ export class FileSearch {
const listing = await workspaceUtils.readDirectoryRecursively(
{ workspace: this.workspace, logging: this.logging },
path,
{ maxDepth: params.maxDepth, excludeEntries: DEFAULT_EXCLUDE_ENTRIES }
{ maxDepth: params.maxDepth, excludeDirs: DEFAULT_EXCLUDE_DIRS, excludeFiles: DEFAULT_EXCLUDE_FILES }
)

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

// Filter the results based on the pattern
const filteredResults = listing.filter(item => regex.test(item))
// Filter the file results based on the pattern
const filteredResults = listing
.filter(item => item.startsWith('[F] '))
.map(item => item.substring(4))
.filter(item => regex.test(item))

if (filteredResults.length === 0) {
return this.createOutput(`No files matching pattern "${params.pattern}" found in ${path}`)
Expand All @@ -111,22 +114,37 @@ export class FileSearch {
return {
name: 'fileSearch',
description:
'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.',
'Search for files in a directory and its subdirectories using regex patterns.\n\n' +
'## Overview\n' +
'This tool searches for files matching a regex pattern, ignoring common build and dependency directories.\n\n' +
'## When to use\n' +
'- When you need to find files with specific naming patterns\n' +
'- When you need to locate files before using more targeted tools like fsRead\n' +
'- When you need to search across a project structure\n\n' +
'## When not to use\n' +
'- When you need to search file contents\n' +
'- When you already know the exact file path\n' +
'- When you need to list all files in a directory (use listDirectory instead)\n\n' +
'## Notes\n' +
'- This tool is more effective than running a command like `find` using `executeBash` tool\n' +
'- Case sensitivity can be controlled with the caseSensitive parameter\n' +
'- Use the `maxDepth` parameter to control how deep the directory traversal goes',
inputSchema: {
type: 'object',
properties: {
path: {
type: 'string',
description: 'Absolute path to a directory, e.g., `/repo`.',
description:
'Absolute path to a directory, e.g. `/repo` for Unix-like system including Unix/Linux/macOS or `d:\\repo\\` for Windows',
},
pattern: {
type: 'string',
description: 'Regex pattern to match against file and directory names.',
description: 'Regex pattern to match against file names.',
},
maxDepth: {
type: 'number',
description:
'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.',
'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.',
},
caseSensitive: {
type: 'boolean',
Expand Down
12 changes: 0 additions & 12 deletions server/aws-lsp-codewhisperer/src/language-server/chat/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,6 @@ export const HELP_MESSAGE = `I'm Amazon Q, a generative AI assistant. Learn more

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

// TO BE DEPRECATED, use DEFAULT_EXCLUDE_DIRS and DEFAULT_EXCLUDE_FILES instead
export const DEFAULT_EXCLUDE_ENTRIES = [
// Dependency directories
'node_modules',
// Build outputs
'dist',
'build',
'out',
// OS specific files
'.DS_Store',
]

export const DEFAULT_EXCLUDE_DIRS = [
// Dependency directories
'node_modules',
Expand Down
Loading