feat(evals): add local report command and developer documentation#28369
feat(evals): add local report command and developer documentation#28369ved015 wants to merge 1 commit into
Conversation
|
📊 PR Size: size/L
|
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 enhances the evaluation development kit (EDK) by adding tooling to analyze and report on behavioral evaluation results. It enables developers to easily aggregate pass rates across different models and provides clear documentation to streamline the contributor workflow for writing and maintaining behavioral evaluations. 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]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the Eval Development Kit (EDK) reporting tool, which includes a new CLI command (npm run eval:report), utility functions to parse and summarize Vitest test reports, comprehensive unit tests, and documentation for behavioral evaluations. The review feedback highlights critical security and robustness improvements, specifically recommending that file path inputs and parsed JSON properties be validated and sanitized to prevent path traversal vulnerabilities, null byte injections, and potential runtime TypeErrors.
| export function findReportFiles(dir: string): string[] { | ||
| const reports: string[] = []; | ||
| if (!fs.existsSync(dir)) return reports; | ||
|
|
||
| const entries = fs.readdirSync(dir, { withFileTypes: true }); |
There was a problem hiding this comment.
Utility functions that perform file system operations, such as findReportFiles, should validate their path inputs internally to prevent path traversal vulnerabilities, rather than relying solely on callers to perform validation. Additionally, if dir is a file rather than a directory, fs.readdirSync will throw an ENOTDIR error. We should gracefully handle cases where a user passes a direct file path or an invalid path type, while ensuring the path is validated.
| export function findReportFiles(dir: string): string[] { | |
| const reports: string[] = []; | |
| if (!fs.existsSync(dir)) return reports; | |
| const entries = fs.readdirSync(dir, { withFileTypes: true }); | |
| export function findReportFiles(dir: string): string[] { | |
| const reports: string[] = []; | |
| if (!dir || dir.includes('\0') || dir.includes('..')) { | |
| return reports; | |
| } | |
| if (!fs.existsSync(dir)) return reports; | |
| try { | |
| const stat = fs.statSync(dir); | |
| if (stat.isFile()) { | |
| if (path.basename(dir) === 'report.json') { | |
| return [dir]; | |
| } | |
| return reports; | |
| } | |
| } catch { | |
| return reports; | |
| } | |
| const entries = fs.readdirSync(dir, { withFileTypes: true }); |
References
- Utility functions that perform file system operations should validate their path inputs internally to prevent path traversal vulnerabilities, rather than relying solely on callers to perform validation.
| for (const fileResult of data.testResults) { | ||
| const filePath = fileResult.name; | ||
| const normalizedPath = path.resolve(filePath).replace(/\\/g, '/'); | ||
| if (Array.isArray(fileResult.assertionResults)) { | ||
| for (const assertion of fileResult.assertionResults) { | ||
| const testName = assertion.title; |
There was a problem hiding this comment.
The properties fileResult.name and assertion.title are parsed from untrusted JSON. If either is missing or null, path.resolve(filePath) or accessing assertion.title will throw a TypeError. Furthermore, since fileResult.name is extracted from an untrusted source, we must sanitize the file path to prevent path traversal (..), null byte injection (\0), and other vulnerabilities.
| for (const fileResult of data.testResults) { | |
| const filePath = fileResult.name; | |
| const normalizedPath = path.resolve(filePath).replace(/\\/g, '/'); | |
| if (Array.isArray(fileResult.assertionResults)) { | |
| for (const assertion of fileResult.assertionResults) { | |
| const testName = assertion.title; | |
| for (const fileResult of data.testResults) { | |
| if (!fileResult || typeof fileResult.name !== 'string') { | |
| continue; | |
| } | |
| const filePath = fileResult.name; | |
| if (filePath.includes('\0') || filePath.includes('..')) { | |
| continue; | |
| } | |
| const normalizedPath = path.resolve(filePath).replace(/\\/g, '/'); | |
| if (Array.isArray(fileResult.assertionResults)) { | |
| for (const assertion of fileResult.assertionResults) { | |
| if (!assertion || typeof assertion.title !== 'string') { | |
| continue; | |
| } | |
| const testName = assertion.title; |
References
- Sanitize file paths extracted from untrusted sources, such as command output, to prevent path traversal (
..), null byte injection (\0), and other vulnerabilities.
gemini-cli
Bot
added
the
status/need-issue
1 participant
Summary
Adds a local evaluation report summary utility and a developer guide for behavioral evaluations. Developers can run
npm run eval:reportto aggregate pass rates by model from Vitestreport.jsonfiles and map results back to their inventory policies, correctly supporting duplicate test names across different files via compound keys. Also adds a comprehensive contributor guide covering the EDK commands, validation rules, and CI setup.Details
scripts/utils/eval-report.ts— recursive file scanner, model-path parser, pass rate aggregator (using compound file + title keys), and JSON/human summary formattersscripts/eval-report-cli.ts— CLI wrapper for the report aggregator supporting positional paths,--json, and--rootscripts/tests/eval-report.test.ts— 11 unit tests covering model extraction, file scanning, formatting schemas, and duplicate test name non-collision checkspackage.json— wired the"eval:report"command scriptdocs/behavioral-evals.md— comprehensive developer guide for EDK commands, validation rules, PR checklists, and CI stepsdocs/sidebar.json— linked the guide under the Development sidebar sectionPre-Merge Checklist