-
Notifications
You must be signed in to change notification settings - Fork 14.5k
fix(evals): update eval tests for invoke_agent telemetry and project-scoped memory #25502
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
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 |
|---|---|---|
|
|
@@ -145,22 +145,30 @@ describe('save_memory', () => { | |
| }, | ||
| }); | ||
|
|
||
| const ignoringDbSchemaLocation = | ||
| "Agent ignores workspace's database schema location"; | ||
| const savingDbSchemaLocationAsProjectMemory = | ||
| 'Agent saves workspace database schema location as project memory'; | ||
| evalTest('USUALLY_PASSES', { | ||
| suiteName: 'default', | ||
| suiteType: 'behavioral', | ||
| name: ignoringDbSchemaLocation, | ||
| name: savingDbSchemaLocationAsProjectMemory, | ||
| prompt: `The database schema for this workspace is located in \`db/schema.sql\`.`, | ||
| assert: async (rig, result) => { | ||
| await rig.waitForTelemetryReady(); | ||
| const wasToolCalled = rig | ||
| .readToolLogs() | ||
| .some((log) => log.toolRequest.name === 'save_memory'); | ||
| const wasToolCalled = await rig.waitForToolCall( | ||
| 'save_memory', | ||
| undefined, | ||
| (args) => { | ||
| try { | ||
| const params = JSON.parse(args); | ||
| return params.scope === 'project'; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }, | ||
| ); | ||
| expect( | ||
| wasToolCalled, | ||
| 'save_memory should not be called for workspace-specific information', | ||
| ).toBe(false); | ||
| 'Expected save_memory to be called with scope="project" for workspace-specific information', | ||
| ).toBe(true); | ||
|
|
||
| assertModelHasOutput(result); | ||
| }, | ||
|
|
@@ -188,42 +196,59 @@ describe('save_memory', () => { | |
| }, | ||
| }); | ||
|
|
||
| const ignoringBuildArtifactLocation = | ||
| 'Agent ignores workspace build artifact location'; | ||
| const savingBuildArtifactLocationAsProjectMemory = | ||
| 'Agent saves workspace build artifact location as project memory'; | ||
| evalTest('USUALLY_PASSES', { | ||
| suiteName: 'default', | ||
| suiteType: 'behavioral', | ||
| name: ignoringBuildArtifactLocation, | ||
| name: savingBuildArtifactLocationAsProjectMemory, | ||
| prompt: `In this workspace, build artifacts are stored in the \`dist/artifacts\` directory.`, | ||
| assert: async (rig, result) => { | ||
| await rig.waitForTelemetryReady(); | ||
| const wasToolCalled = rig | ||
| .readToolLogs() | ||
| .some((log) => log.toolRequest.name === 'save_memory'); | ||
| const wasToolCalled = await rig.waitForToolCall( | ||
| 'save_memory', | ||
| undefined, | ||
| (args) => { | ||
| try { | ||
| const params = JSON.parse(args); | ||
| return params.scope === 'project'; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }, | ||
| ); | ||
| expect( | ||
| wasToolCalled, | ||
| 'save_memory should not be called for workspace-specific information', | ||
| ).toBe(false); | ||
| 'Expected save_memory to be called with scope="project" for workspace-specific information', | ||
| ).toBe(true); | ||
|
|
||
| assertModelHasOutput(result); | ||
| }, | ||
| }); | ||
|
|
||
| const ignoringMainEntryPoint = "Agent ignores workspace's main entry point"; | ||
| const savingMainEntryPointAsProjectMemory = | ||
| 'Agent saves workspace main entry point as project memory'; | ||
| evalTest('USUALLY_PASSES', { | ||
| suiteName: 'default', | ||
| suiteType: 'behavioral', | ||
| name: ignoringMainEntryPoint, | ||
| name: savingMainEntryPointAsProjectMemory, | ||
| prompt: `The main entry point for this workspace is \`src/index.js\`.`, | ||
| assert: async (rig, result) => { | ||
| await rig.waitForTelemetryReady(); | ||
| const wasToolCalled = rig | ||
| .readToolLogs() | ||
| .some((log) => log.toolRequest.name === 'save_memory'); | ||
| const wasToolCalled = await rig.waitForToolCall( | ||
| 'save_memory', | ||
| undefined, | ||
| (args) => { | ||
| try { | ||
| const params = JSON.parse(args); | ||
| return params.scope === 'project'; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }, | ||
| ); | ||
| expect( | ||
| wasToolCalled, | ||
| 'save_memory should not be called for workspace-specific information', | ||
| ).toBe(false); | ||
| 'Expected save_memory to be called with scope="project" for workspace-specific information', | ||
| ).toBe(true); | ||
|
|
||
| assertModelHasOutput(result); | ||
| }, | ||
|
|
@@ -317,13 +342,13 @@ describe('save_memory', () => { | |
| 'Please save any persistent preferences or facts about me from our conversation to memory.', | ||
| assert: async (rig, result) => { | ||
| const wasToolCalled = await rig.waitForToolCall( | ||
| 'save_memory', | ||
| 'invoke_agent', | ||
| undefined, | ||
| (args) => /vitest/i.test(args), | ||
| (args) => /save_memory/i.test(args) && /vitest/i.test(args), | ||
| ); | ||
| expect( | ||
| wasToolCalled, | ||
| 'Expected save_memory to be called with the Vitest preference from the conversation history', | ||
| 'Expected invoke_agent to be called with save_memory agent and the Vitest preference from the conversation history', | ||
| ).toBe(true); | ||
|
|
||
| assertModelHasOutput(result); | ||
|
|
@@ -379,8 +404,15 @@ describe('save_memory', () => { | |
| ], | ||
| prompt: 'Please save the preferences I mentioned earlier to memory.', | ||
| assert: async (rig, result) => { | ||
| const wasToolCalled = await rig.waitForToolCall('save_memory'); | ||
| expect(wasToolCalled, 'Expected save_memory to be called').toBe(true); | ||
| const wasToolCalled = await rig.waitForToolCall( | ||
| 'invoke_agent', | ||
| undefined, | ||
| (args) => /save_memory/i.test(args), | ||
|
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. Similar to the previous test, using a simple regex on the raw arguments string for invoke_agent is imprecise. Following the repository rule that toolRequest.args is a JSON string, it should be explicitly parsed to verify that the agent_name is save_memory to avoid false positives from other subagent calls. (args) => {
try {
const parsed = JSON.parse(args);
return parsed.agent_name === 'save_memory';
} catch {
return false;
}
},References
|
||
| ); | ||
| expect( | ||
| wasToolCalled, | ||
| 'Expected invoke_agent to be called with save_memory agent', | ||
| ).toBe(true); | ||
|
|
||
| assertModelHasOutput(result); | ||
| }, | ||
|
|
||
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.
The current predicate uses a loose regex check on the entire stringified arguments of the invoke_agent tool. This can lead to false positives if the agent name is different but the request string happens to contain 'save_memory'. As per repository rules, toolRequest.args is a JSON string and must be parsed using JSON.parse() before its properties can be accessed. It is safer to parse the JSON and explicitly check the agent_name property.
References