-
Notifications
You must be signed in to change notification settings - Fork 11.7k
Update prompt and grep tool definition to limit context size #18780
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
Changes from all commits
8e9f430
93af8e5
7e32c32
5fa16e3
33e928f
7d7f77a
4b0c9ce
7b7089b
838ea7a
531fe49
20d7aac
a15553e
01a90f9
864e497
a98098c
93e282b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2026 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { describe, expect } from 'vitest'; | ||
| import { evalTest } from './test-helper.js'; | ||
|
|
||
| describe('Frugal Search', () => { | ||
| const getGrepParams = (call: any): any => { | ||
| let args = call.toolRequest.args; | ||
| if (typeof args === 'string') { | ||
| try { | ||
| args = JSON.parse(args); | ||
| } catch (e) { | ||
| // Ignore parse errors | ||
| } | ||
| } | ||
| return args; | ||
| }; | ||
|
|
||
| evalTest('USUALLY_PASSES', { | ||
| name: 'should use targeted search with limit', | ||
| prompt: 'find me a sample usage of path.resolve() in the codebase', | ||
| files: { | ||
| 'package.json': JSON.stringify({ | ||
| name: 'test-project', | ||
| version: '1.0.0', | ||
| main: 'dist/index.js', | ||
| scripts: { | ||
| build: 'tsc', | ||
| test: 'vitest', | ||
| }, | ||
| dependencies: { | ||
| typescript: '^5.0.0', | ||
| '@types/node': '^20.0.0', | ||
| vitest: '^1.0.0', | ||
| }, | ||
| }), | ||
| 'src/index.ts': ` | ||
| import { App } from './app.ts'; | ||
|
|
||
| const app = new App(); | ||
| app.start(); | ||
| `, | ||
| 'src/app.ts': ` | ||
| import * as path from 'path'; | ||
| import { UserController } from './controllers/user.ts'; | ||
|
|
||
| export class App { | ||
| constructor() { | ||
| console.log('App initialized'); | ||
| } | ||
|
|
||
| public start(): void { | ||
| const userController = new UserController(); | ||
| console.log('Static path:', path.resolve(__dirname, '../public')); | ||
| } | ||
| } | ||
| `, | ||
| 'src/utils.ts': ` | ||
| import * as path from 'path'; | ||
| import * as fs from 'fs'; | ||
|
|
||
| export function resolvePath(p: string): string { | ||
| return path.resolve(process.cwd(), p); | ||
| } | ||
|
|
||
| export function ensureDir(dirPath: string): void { | ||
| const absolutePath = path.resolve(dirPath); | ||
| if (!fs.existsSync(absolutePath)) { | ||
| fs.mkdirSync(absolutePath, { recursive: true }); | ||
| } | ||
| } | ||
| `, | ||
| 'src/config.ts': ` | ||
| import * as path from 'path'; | ||
|
|
||
| export const config = { | ||
| dbPath: path.resolve(process.cwd(), 'data/db.sqlite'), | ||
| logLevel: 'info', | ||
| }; | ||
| `, | ||
| 'src/controllers/user.ts': ` | ||
| import * as path from 'path'; | ||
|
|
||
| export class UserController { | ||
| public getUsers(): any[] { | ||
| console.log('Loading users from:', path.resolve('data/users.json')); | ||
| return [{ id: 1, name: 'Alice' }]; | ||
| } | ||
| } | ||
| `, | ||
| 'tests/app.test.ts': ` | ||
| import { describe, it, expect } from 'vitest'; | ||
| import * as path from 'path'; | ||
|
|
||
| describe('App', () => { | ||
| it('should resolve paths', () => { | ||
| const p = path.resolve('test'); | ||
| expect(p).toBeDefined(); | ||
| }); | ||
| }); | ||
| `, | ||
| }, | ||
| assert: async (rig) => { | ||
| const toolCalls = rig.readToolLogs(); | ||
| const grepCalls = toolCalls.filter( | ||
| (call) => call.toolRequest.name === 'grep_search', | ||
| ); | ||
|
|
||
| expect(grepCalls.length).toBeGreaterThan(0); | ||
|
|
||
| const hasFrugalLimit = grepCalls.some((call) => { | ||
| const params = getGrepParams(call); | ||
| // Check for explicitly set small limit for "sample" or "example" requests | ||
| return ( | ||
| params.total_max_matches !== undefined && | ||
| params.total_max_matches <= 100 | ||
| ); | ||
| }); | ||
|
|
||
| expect( | ||
| hasFrugalLimit, | ||
| `Expected agent to use a small total_max_matches for a sample usage request. Params used: ${JSON.stringify( | ||
| grepCalls.map(getGrepParams), | ||
| null, | ||
| 2, | ||
| )}`, | ||
| ).toBe(true); | ||
| }, | ||
| }); | ||
| }); | ||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,6 +164,9 @@ export function renderCoreMandates(options?: CoreMandatesOptions): string { | |
| - **Credential Protection:** Never log, print, or commit secrets, API keys, or sensitive credentials. Rigorously protect \`.env\` files, \`.git\`, and system configuration folders. | ||
| - **Source Control:** Do not stage or commit changes unless specifically requested by the user. | ||
|
|
||
| ## Context Efficiency: | ||
| - Always minimize wasted context window by scoping and limiting all of your ${GREP_TOOL_NAME} searches. e.g.: pass total_max_matches, include, and max_matches_per_file. | ||
joshualitt marked this conversation as resolved.
Show resolved
Hide resolved
joshualitt marked this conversation as resolved.
Show resolved
Hide resolved
joshualitt marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider updating the mandate with more refined language, an explicit "why," and a bit more instruction:
|
||
|
|
||
| ## Engineering Standards | ||
| - **Contextual Precedence:** Instructions found in ${formattedFilenames} files are foundational mandates. They take absolute precedence over the general workflows and tool defaults described in this system prompt. | ||
| - **Conventions & Style:** Rigorously adhere to existing workspace conventions, architectural patterns, and style (naming, formatting, typing, commenting). During the research phase, analyze surrounding files, tests, and configuration to ensure your changes are seamless, idiomatic, and consistent with the local context. Never compromise idiomatic quality or completeness (e.g., proper declarations, type safety, documentation) to minimize tool calls; all supporting changes required by local conventions are part of a surgical update. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,16 @@ export interface GrepToolParams { | |
| */ | ||
| include?: string; | ||
|
|
||
| /** | ||
| * Optional: A regular expression pattern to exclude from the search results. | ||
| */ | ||
| exclude_pattern?: string; | ||
|
|
||
| /** | ||
| * Optional: If true, only the file paths of the matches will be returned. | ||
| */ | ||
| names_only?: boolean; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: maybe
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't fixing this one. The model seems to understand this one. |
||
|
|
||
| /** | ||
| * Optional: Maximum number of matches to return per file. Use this to prevent being overwhelmed by repetitive matches in large files. | ||
| */ | ||
|
|
@@ -225,6 +235,7 @@ class GrepToolInvocation extends BaseToolInvocation< | |
| pattern: this.params.pattern, | ||
| path: searchDir, | ||
| include: this.params.include, | ||
| exclude_pattern: this.params.exclude_pattern, | ||
| maxMatches: remainingLimit, | ||
| max_matches_per_file: this.params.max_matches_per_file, | ||
| signal: timeoutController.signal, | ||
|
|
@@ -280,6 +291,16 @@ class GrepToolInvocation extends BaseToolInvocation< | |
| const matchCount = allMatches.length; | ||
| const matchTerm = matchCount === 1 ? 'match' : 'matches'; | ||
|
|
||
| if (this.params.names_only) { | ||
| const filePaths = Object.keys(matchesByFile).sort(); | ||
| let llmContent = `Found ${filePaths.length} files with matches for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}${wasTruncated ? ` (results limited to ${totalMaxMatches} matches for performance)` : ''}:\n`; | ||
| llmContent += filePaths.join('\n'); | ||
| return { | ||
| llmContent: llmContent.trim(), | ||
| returnDisplay: `Found ${filePaths.length} files${wasTruncated ? ' (limited)' : ''}`, | ||
| }; | ||
| } | ||
|
|
||
| let llmContent = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}`; | ||
|
|
||
| if (wasTruncated) { | ||
|
|
@@ -354,6 +375,7 @@ class GrepToolInvocation extends BaseToolInvocation< | |
| pattern: string; | ||
| path: string; // Expects absolute path | ||
| include?: string; | ||
| exclude_pattern?: string; | ||
| maxMatches: number; | ||
| max_matches_per_file?: number; | ||
| signal: AbortSignal; | ||
|
|
@@ -362,12 +384,18 @@ class GrepToolInvocation extends BaseToolInvocation< | |
| pattern, | ||
| path: absolutePath, | ||
| include, | ||
| exclude_pattern, | ||
| maxMatches, | ||
| max_matches_per_file, | ||
| } = options; | ||
| let strategyUsed = 'none'; | ||
|
|
||
| try { | ||
| let excludeRegex: RegExp | null = null; | ||
| if (exclude_pattern) { | ||
| excludeRegex = new RegExp(exclude_pattern, 'i'); | ||
| } | ||
|
|
||
| // --- Strategy 1: git grep --- | ||
| const isGit = isGitRepository(absolutePath); | ||
| const gitAvailable = isGit && (await this.isCommandAvailable('git')); | ||
|
|
@@ -400,6 +428,9 @@ class GrepToolInvocation extends BaseToolInvocation< | |
| for await (const line of generator) { | ||
| const match = this.parseGrepLine(line, absolutePath); | ||
| if (match) { | ||
| if (excludeRegex && excludeRegex.test(match.line)) { | ||
| continue; | ||
| } | ||
| results.push(match); | ||
| if (results.length >= maxMatches) { | ||
| break; | ||
|
|
@@ -467,6 +498,9 @@ class GrepToolInvocation extends BaseToolInvocation< | |
| for await (const line of generator) { | ||
| const match = this.parseGrepLine(line, absolutePath); | ||
| if (match) { | ||
| if (excludeRegex && excludeRegex.test(match.line)) { | ||
| continue; | ||
| } | ||
| results.push(match); | ||
| if (results.length >= maxMatches) { | ||
| break; | ||
|
|
@@ -528,6 +562,9 @@ class GrepToolInvocation extends BaseToolInvocation< | |
| for (let index = 0; index < lines.length; index++) { | ||
| const line = lines[index]; | ||
| if (regex.test(line)) { | ||
| if (excludeRegex && excludeRegex.test(line)) { | ||
| continue; | ||
| } | ||
| allMatches.push({ | ||
| filePath: | ||
| path.relative(absolutePath, fileAbsolutePath) || | ||
|
|
@@ -637,6 +674,14 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> { | |
| return `Invalid regular expression pattern provided: ${params.pattern}. Error: ${getErrorMessage(error)}`; | ||
| } | ||
|
|
||
| if (params.exclude_pattern) { | ||
| try { | ||
| new RegExp(params.exclude_pattern); | ||
| } catch (error) { | ||
| return `Invalid exclude regular expression pattern provided: ${params.exclude_pattern}. Error: ${getErrorMessage(error)}`; | ||
| } | ||
| } | ||
|
|
||
| if ( | ||
| params.max_matches_per_file !== undefined && | ||
| params.max_matches_per_file < 1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: for me it is not obvious what this evals do - we don't have frugal search tool or anything, so I imagine, it might be hard to remember in a year or too. I would add a comment or smth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback. In the interest of keeping momentum (particularly avoiding the need to re-run evals), I'm going to take these in an immediate followup.