Limit grep result (#407)

* feat: implement result limiting for GrepTool to prevent context overflow
This commit is contained in:
ajiwo
2025-08-21 17:35:30 +07:00
committed by GitHub
parent 742337c390
commit ed5a2d0fa4
3 changed files with 176 additions and 3 deletions

View File

@@ -89,10 +89,13 @@ Qwen Code provides a comprehensive suite of tools for interacting with the local
- `pattern` (string, required): The regular expression (regex) to search for (e.g., `"function\s+myFunction"`). - `pattern` (string, required): The regular expression (regex) to search for (e.g., `"function\s+myFunction"`).
- `path` (string, optional): The absolute path to the directory to search within. Defaults to the current working directory. - `path` (string, optional): The absolute path to the directory to search within. Defaults to the current working directory.
- `include` (string, optional): A glob pattern to filter which files are searched (e.g., `"*.js"`, `"src/**/*.{ts,tsx}"`). If omitted, searches most files (respecting common ignores). - `include` (string, optional): A glob pattern to filter which files are searched (e.g., `"*.js"`, `"src/**/*.{ts,tsx}"`). If omitted, searches most files (respecting common ignores).
- `maxResults` (number, optional): Maximum number of matches to return to prevent context overflow (default: 20, max: 100). Use lower values for broad searches, higher for specific searches.
- **Behavior:** - **Behavior:**
- Uses `git grep` if available in a Git repository for speed; otherwise, falls back to system `grep` or a JavaScript-based search. - Uses `git grep` if available in a Git repository for speed; otherwise, falls back to system `grep` or a JavaScript-based search.
- Returns a list of matching lines, each prefixed with its file path (relative to the search directory) and line number. - Returns a list of matching lines, each prefixed with its file path (relative to the search directory) and line number.
- Limits results to a maximum of 20 matches by default to prevent context overflow. When results are truncated, shows a clear warning with guidance on refining searches.
- **Output (`llmContent`):** A formatted string of matches, e.g.: - **Output (`llmContent`):** A formatted string of matches, e.g.:
``` ```
Found 3 matches for pattern "myFunction" in path "." (filter: "*.ts"): Found 3 matches for pattern "myFunction" in path "." (filter: "*.ts"):
--- ---
@@ -103,9 +106,36 @@ Qwen Code provides a comprehensive suite of tools for interacting with the local
File: src/index.ts File: src/index.ts
L5: import { myFunction } from './utils'; L5: import { myFunction } from './utils';
--- ---
WARNING: Results truncated to prevent context overflow. To see more results:
- Use a more specific pattern to reduce matches
- Add file filters with the 'include' parameter (e.g., "*.js", "src/**")
- Specify a narrower 'path' to search in a subdirectory
- Increase 'maxResults' parameter if you need more matches (current: 20)
``` ```
- **Confirmation:** No. - **Confirmation:** No.
### `search_file_content` examples
Search for a pattern with default result limiting:
```
search_file_content(pattern="function\s+myFunction", path="src")
```
Search for a pattern with custom result limiting:
```
search_file_content(pattern="function", path="src", maxResults=50)
```
Search for a pattern with file filtering and custom result limiting:
```
search_file_content(pattern="function", include="*.js", maxResults=10)
```
## 6. `replace` (Edit) ## 6. `replace` (Edit)
`replace` replaces text within a file. By default, replaces a single occurrence, but can replace multiple occurrences when `expected_replacements` is specified. This tool is designed for precise, targeted changes and requires significant context around the `old_string` to ensure it modifies the correct location. `replace` replaces text within a file. By default, replaces a single occurrence, but can replace multiple occurrences when `expected_replacements` is specified. This tool is designed for precise, targeted changes and requires significant context around the `old_string` to ensure it modifies the correct location.

View File

@@ -439,4 +439,84 @@ describe('GrepTool', () => {
expect(invocation.getDescription()).toBe("'testPattern' within ./"); expect(invocation.getDescription()).toBe("'testPattern' within ./");
}); });
}); });
describe('Result limiting', () => {
beforeEach(async () => {
// Create many test files with matches to test limiting
for (let i = 1; i <= 30; i++) {
const fileName = `test${i}.txt`;
const content = `This is test file ${i} with the pattern testword in it.`;
await fs.writeFile(path.join(tempRootDir, fileName), content);
}
});
it('should limit results to default 20 matches', async () => {
const params: GrepToolParams = { pattern: 'testword' };
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 20 matches');
expect(result.llmContent).toContain(
'showing first 20 of 30+ total matches',
);
expect(result.llmContent).toContain('WARNING: Results truncated');
expect(result.returnDisplay).toContain(
'Found 20 matches (truncated from 30+)',
);
});
it('should respect custom maxResults parameter', async () => {
const params: GrepToolParams = { pattern: 'testword', maxResults: 5 };
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 5 matches');
expect(result.llmContent).toContain(
'showing first 5 of 30+ total matches',
);
expect(result.llmContent).toContain('current: 5');
expect(result.returnDisplay).toContain(
'Found 5 matches (truncated from 30+)',
);
});
it('should not show truncation warning when all results fit', async () => {
const params: GrepToolParams = { pattern: 'testword', maxResults: 50 };
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 30 matches');
expect(result.llmContent).not.toContain('WARNING: Results truncated');
expect(result.llmContent).not.toContain('showing first');
expect(result.returnDisplay).toBe('Found 30 matches');
});
it('should validate maxResults parameter', () => {
const invalidParams = [
{ pattern: 'test', maxResults: 0 },
{ pattern: 'test', maxResults: 101 },
{ pattern: 'test', maxResults: -1 },
{ pattern: 'test', maxResults: 1.5 },
];
invalidParams.forEach((params) => {
const error = grepTool.validateToolParams(params as GrepToolParams);
expect(error).toBeTruthy(); // Just check that validation fails
expect(error).toMatch(/maxResults|must be/); // Check it's about maxResults validation
});
});
it('should accept valid maxResults parameter', () => {
const validParams = [
{ pattern: 'test', maxResults: 1 },
{ pattern: 'test', maxResults: 50 },
{ pattern: 'test', maxResults: 100 },
];
validParams.forEach((params) => {
const error = grepTool.validateToolParams(params);
expect(error).toBeNull();
});
});
});
}); });

View File

@@ -43,6 +43,11 @@ export interface GrepToolParams {
* File pattern to include in the search (e.g. "*.js", "*.{ts,tsx}") * File pattern to include in the search (e.g. "*.js", "*.{ts,tsx}")
*/ */
include?: string; include?: string;
/**
* Maximum number of matches to return (optional, defaults to 20)
*/
maxResults?: number;
} }
/** /**
@@ -124,6 +129,10 @@ class GrepToolInvocation extends BaseToolInvocation<
// Collect matches from all search directories // Collect matches from all search directories
let allMatches: GrepMatch[] = []; let allMatches: GrepMatch[] = [];
const maxResults = this.params.maxResults ?? 20; // Default to 20 results
let totalMatchesFound = 0;
let searchTruncated = false;
for (const searchDir of searchDirectories) { for (const searchDir of searchDirectories) {
const matches = await this.performGrepSearch({ const matches = await this.performGrepSearch({
pattern: this.params.pattern, pattern: this.params.pattern,
@@ -132,6 +141,8 @@ class GrepToolInvocation extends BaseToolInvocation<
signal, signal,
}); });
totalMatchesFound += matches.length;
// Add directory prefix if searching multiple directories // Add directory prefix if searching multiple directories
if (searchDirectories.length > 1) { if (searchDirectories.length > 1) {
const dirName = path.basename(searchDir); const dirName = path.basename(searchDir);
@@ -140,8 +151,21 @@ class GrepToolInvocation extends BaseToolInvocation<
}); });
} }
// Apply result limiting
const remainingSlots = maxResults - allMatches.length;
if (remainingSlots <= 0) {
searchTruncated = true;
break;
}
if (matches.length > remainingSlots) {
allMatches = allMatches.concat(matches.slice(0, remainingSlots));
searchTruncated = true;
break;
} else {
allMatches = allMatches.concat(matches); allMatches = allMatches.concat(matches);
} }
}
let searchLocationDescription: string; let searchLocationDescription: string;
if (searchDirAbs === null) { if (searchDirAbs === null) {
@@ -176,7 +200,14 @@ class GrepToolInvocation extends BaseToolInvocation<
const matchCount = allMatches.length; const matchCount = allMatches.length;
const matchTerm = matchCount === 1 ? 'match' : 'matches'; const matchTerm = matchCount === 1 ? 'match' : 'matches';
let llmContent = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}: // Build the header with truncation info if needed
let headerText = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}`;
if (searchTruncated) {
headerText += ` (showing first ${matchCount} of ${totalMatchesFound}+ total matches)`;
}
let llmContent = `${headerText}:
--- ---
`; `;
@@ -189,9 +220,23 @@ class GrepToolInvocation extends BaseToolInvocation<
llmContent += '---\n'; llmContent += '---\n';
} }
// Add truncation guidance if results were limited
if (searchTruncated) {
llmContent += `\nWARNING: Results truncated to prevent context overflow. To see more results:
- Use a more specific pattern to reduce matches
- Add file filters with the 'include' parameter (e.g., "*.js", "src/**")
- Specify a narrower 'path' to search in a subdirectory
- Increase 'maxResults' parameter if you need more matches (current: ${maxResults})`;
}
let displayText = `Found ${matchCount} ${matchTerm}`;
if (searchTruncated) {
displayText += ` (truncated from ${totalMatchesFound}+)`;
}
return { return {
llmContent: llmContent.trim(), llmContent: llmContent.trim(),
returnDisplay: `Found ${matchCount} ${matchTerm}`, returnDisplay: displayText,
}; };
} catch (error) { } catch (error) {
console.error(`Error during GrepLogic execution: ${error}`); console.error(`Error during GrepLogic execution: ${error}`);
@@ -567,6 +612,13 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
"Optional: A glob pattern to filter which files are searched (e.g., '*.js', '*.{ts,tsx}', 'src/**'). If omitted, searches all files (respecting potential global ignores).", "Optional: A glob pattern to filter which files are searched (e.g., '*.js', '*.{ts,tsx}', 'src/**'). If omitted, searches all files (respecting potential global ignores).",
type: 'string', type: 'string',
}, },
maxResults: {
description:
'Optional: Maximum number of matches to return to prevent context overflow (default: 20, max: 100). Use lower values for broad searches, higher for specific searches.',
type: 'number',
minimum: 1,
maximum: 100,
},
}, },
required: ['pattern'], required: ['pattern'],
type: 'object', type: 'object',
@@ -635,6 +687,17 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
return `Invalid regular expression pattern provided: ${params.pattern}. Error: ${getErrorMessage(error)}`; return `Invalid regular expression pattern provided: ${params.pattern}. Error: ${getErrorMessage(error)}`;
} }
// Validate maxResults if provided
if (params.maxResults !== undefined) {
if (
!Number.isInteger(params.maxResults) ||
params.maxResults < 1 ||
params.maxResults > 100
) {
return `maxResults must be an integer between 1 and 100, got: ${params.maxResults}`;
}
}
// Only validate path if one is provided // Only validate path if one is provided
if (params.path) { if (params.path) {
try { try {