Repository navigation
Conversation
…out-independently
…s behavioral evaluations
…ehavioral evaluations
|
📊 PR Size: size/XL
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust suite of behavioral evaluations designed to verify agent tool usage and decision-making logic. By adding these tests, the codebase gains better coverage for critical workflows. Additionally, the PR upgrades the testing infrastructure with improved inspection helpers and a new path normalization utility, which resolves cross-platform path issues in evaluation reporting and ensures more reliable test results. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds several behavioral evaluation tests to verify agent capabilities and tool usage, enhances the AppRig test utility, and refactors the evaluation reporting script to use checkout-independent relative paths. The review feedback suggests improving Windows path resolution in getRelativePath by normalizing drive letter casing, adding a test case for casing mismatches, and using the CoreToolCallStatus.Success enum instead of a hardcoded string in AppRig.
| export function getRelativePath(filePath: string, rootDir: string): string { | ||
| const normalizedPath = filePath.replace(/\\/g, '/'); | ||
| const root = path.resolve(rootDir).replace(/\\/g, '/'); | ||
|
|
||
| // Handle Windows absolute paths with drive letters on POSIX systems | ||
| if (/^[a-zA-Z]:\//.test(normalizedPath)) { | ||
| if (normalizedPath.startsWith(root + '/')) { | ||
| return normalizedPath.slice(root.length + 1); | ||
| } | ||
| const match = normalizedPath.match( | ||
| /(?:^|\/)(evals|packages|scripts|integration-tests|memory-tests)\/(.+)$/, | ||
| ); | ||
| if (match) { | ||
| return `${match[1]}/${match[2]}`; | ||
| } | ||
| return normalizedPath.split('/').pop() || ''; | ||
| } | ||
|
|
||
| const absolute = path.resolve(rootDir, filePath).replace(/\\/g, '/'); | ||
| if (absolute.startsWith(root + '/')) { | ||
| return absolute.slice(root.length + 1); | ||
| } else if (absolute === root) { | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
Windows drive letters can have inconsistent casing (e.g., C:/ vs c:/) depending on how the path was resolved or retrieved. This casing mismatch can cause normalizedPath.startsWith(root + '/') to evaluate to false, failing to correctly resolve relative paths on Windows developer machines. Normalizing the drive letter casing to lowercase for both paths ensures robust cross-platform path resolution.
export function getRelativePath(filePath: string, rootDir: string): string {
let normalizedPath = filePath.replace(/\\/g, '/');
let root = path.resolve(rootDir).replace(/\\/g, '/');
// Normalize Windows drive letter casing to avoid mismatch (e.g. C:/ vs c:/)
if (/^[a-zA-Z]:/.test(normalizedPath)) {
normalizedPath = normalizedPath[0].toLowerCase() + normalizedPath.slice(1);
}
if (/^[a-zA-Z]:/.test(root)) {
root = root[0].toLowerCase() + root.slice(1);
}
// Handle Windows absolute paths with drive letters on POSIX systems
if (/^[a-zA-Z]:\//.test(normalizedPath)) {
if (normalizedPath.startsWith(root + '/')) {
return normalizedPath.slice(root.length + 1);
}
const match = normalizedPath.match(
/(?:^|\/)(evals|packages|scripts|integration-tests|memory-tests)\/(.+)$/,
);
if (match) {
return `${match[1]}/${match[2]}`;
}
return normalizedPath.split('/').pop() || '';
}
let absolute = path.resolve(rootDir, filePath).replace(/\\/g, '/');
if (/^[a-zA-Z]:/.test(absolute)) {
absolute = absolute[0].toLowerCase() + absolute.slice(1);
}
if (absolute.startsWith(root + '/')) {
return absolute.slice(root.length + 1);
} else if (absolute === root) {
return '';
}| describe('getRelativePath', () => { | ||
| it('correctly handles Windows drive letter paths on any platform', () => { | ||
| const winPath = 'C:\\coding\\gemini-cli\\evals\\my_test.eval.ts'; | ||
| expect(getRelativePath(winPath, '/home/runner/work/repo')).toBe( | ||
| 'evals/my_test.eval.ts', | ||
| ); | ||
| }); | ||
|
|
||
| it('extracts relative path when starts with rootDir', () => { | ||
| const fullPath = '/home/runner/work/repo/evals/my_test.eval.ts'; | ||
| expect(getRelativePath(fullPath, '/home/runner/work/repo')).toBe( | ||
| 'evals/my_test.eval.ts', | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Add a unit test to verify that getRelativePath correctly handles Windows drive letter casing mismatches (e.g., c:\... vs C:\...).
describe('getRelativePath', () => {
it('correctly handles Windows drive letter paths on any platform', () => {
const winPath = 'C:\\coding\\gemini-cli\\evals\\my_test.eval.ts';
expect(getRelativePath(winPath, '/home/runner/work/repo')).toBe(
'evals/my_test.eval.ts',
);
});
it('handles Windows drive letter casing mismatches gracefully', () => {
const winPath = 'c:\\coding\\gemini-cli\\evals\\my_test.eval.ts';
expect(getRelativePath(winPath, 'C:\\coding\\gemini-cli')).toBe(
'evals/my_test.eval.ts',
);
});
it('extracts relative path when starts with rootDir', () => {
const fullPath = '/home/runner/work/repo/evals/my_test.eval.ts';
expect(getRelativePath(fullPath, '/home/runner/work/repo')).toBe(
'evals/my_test.eval.ts',
);
});
});| async waitForCompletedToolCall(toolName: string, timeout = 10000) { | ||
| await this.waitUntil( | ||
| () => this.toolCalls.some( | ||
| (call) => call.request.name === toolName && call.status === 'success', | ||
| ), | ||
| { | ||
| timeout, | ||
| message: `Timed out waiting for tool call "${toolName}" to complete with success status`, | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Avoid using the hardcoded string 'success' when checking the tool call status. Instead, use the imported CoreToolCallStatus.Success enum to maintain type safety and consistency with the rest of the file.
| async waitForCompletedToolCall(toolName: string, timeout = 10000) { | |
| await this.waitUntil( | |
| () => this.toolCalls.some( | |
| (call) => call.request.name === toolName && call.status === 'success', | |
| ), | |
| { | |
| timeout, | |
| message: `Timed out waiting for tool call "${toolName}" to complete with success status`, | |
| }, | |
| ); | |
| } | |
| async waitForCompletedToolCall(toolName: string, timeout = 10000) { | |
| await this.waitUntil( | |
| () => this.toolCalls.some( | |
| (call) => call.request.name === toolName && call.status === CoreToolCallStatus.Success, | |
| ), | |
| { | |
| timeout, | |
| message: `Timed out waiting for tool call "${toolName}" to complete with success status`, | |
| }, | |
| ); | |
| } |
gemini-cli
Bot
added
the
status/need-issue
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
This branch had an error being deployed
1 participant
Summary
Adds behavioral evaluations for task planning (
write_todos), task completion signaling (complete_task), and task tracker status querying (tracker_list_tasksandtracker_get_task).Details
evals/write_todos.eval.ts— [NEW] Behavioral evaluation asserting thatwrite_todosis invoked to structure TODO items when given a multi-step refactoring prompt.evals/complete_task.eval.ts— [NEW] Behavioral evaluation asserting thatcomplete_taskis invoked to submit final findings upon completing a task flow.evals/tracker_queries.eval.ts— [NEW] Behavioral evaluations for task tracker read queries:tracker_list_tasksis called to list active tasks whenexperimental.taskTrackeris enabled.tracker_get_taskis called with the target task ID to fetch specific task details.Pre-Merge Checklist