diff --git a/.github/workflows/evaluation.yml b/.github/workflows/evaluation.yml index 2e4ded669a..1f44bc087e 100644 --- a/.github/workflows/evaluation.yml +++ b/.github/workflows/evaluation.yml @@ -160,9 +160,40 @@ jobs: } shell: pwsh - evaluate: + build-validator: + runs-on: ubuntu-latest needs: discover if: needs.discover.outputs.has_entries == 'true' + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'npm' + cache-dependency-path: eng/skill-validator/package-lock.json + + - name: Build skill-validator + run: cd eng/skill-validator && npm ci && npm run build + + - name: Prune dev dependencies + run: cd eng/skill-validator && npm prune --omit=dev + + - name: Upload built validator + uses: actions/upload-artifact@v4 + with: + name: skill-validator-dist + path: | + eng/skill-validator/dist/ + eng/skill-validator/node_modules/ + eng/skill-validator/package.json + retention-days: 1 + + evaluate: + needs: [discover, build-validator] + if: needs.discover.outputs.has_entries == 'true' runs-on: ubuntu-latest timeout-minutes: 30 name: evaluate (${{ matrix.entry.name }}) @@ -185,103 +216,97 @@ jobs: with: dotnet-version: ${{ env.DOTNET_VERSION }} - - name: Build skill-validator - run: cd eng/skill-validator && npm ci && npm run build + - name: Download built validator + uses: actions/download-artifact@v4 + with: + name: skill-validator-dist + path: artifacts/skill-validator/ - name: Run skill-validator if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository continue-on-error: true env: GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN_2 }} + RESULTS_PATH: artifacts/TestResults/skill-validator/${{ matrix.entry.name }} run: | ARGS="--strict --require-evals" - ARGS="$ARGS --reporter console --reporter json:.skill-validator-results/results.json" + ARGS="$ARGS --results-dir $RESULTS_PATH --reporter console --reporter json --reporter markdown" + ARGS="$ARGS --model ${{ github.event.inputs.model || env.DEFAULT_MODEL }}" + ARGS="$ARGS --judge-model ${{ github.event.inputs.judge-model || env.DEFAULT_JUDGE_MODEL }}" + ARGS="$ARGS --runs ${{ github.event.inputs.runs || env.DEFAULT_RUNS }}" + ARGS="$ARGS --parallel-skills ${{ github.event.inputs.parallel-skills || env.DEFAULT_PARALLEL_SKILLS }}" + ARGS="$ARGS --parallel-scenarios ${{ github.event.inputs.parallel-scenarios || env.DEFAULT_PARALLEL_SCENARIOS }}" + ARGS="$ARGS --parallel-runs ${{ github.event.inputs.parallel-runs || env.DEFAULT_PARALLEL_RUNS }}" - MODEL="${{ github.event.inputs.model || env.DEFAULT_MODEL }}" - if [ -n "$MODEL" ]; then - ARGS="$ARGS --model $MODEL" - fi - RUNS="${{ github.event.inputs.runs || env.DEFAULT_RUNS }}" - if [ -n "$RUNS" ]; then - ARGS="$ARGS --runs $RUNS" - fi - PARALLEL_SKILLS="${{ github.event.inputs.parallel-skills || env.DEFAULT_PARALLEL_SKILLS }}" - if [ -n "$PARALLEL_SKILLS" ]; then - ARGS="$ARGS --parallel-skills $PARALLEL_SKILLS" - fi - PARALLEL_SCENARIOS="${{ github.event.inputs.parallel-scenarios || env.DEFAULT_PARALLEL_SCENARIOS }}" - if [ -n "$PARALLEL_SCENARIOS" ]; then - ARGS="$ARGS --parallel-scenarios $PARALLEL_SCENARIOS" - fi - PARALLEL_RUNS="${{ github.event.inputs.parallel-runs || env.DEFAULT_PARALLEL_RUNS }}" - if [ -n "$PARALLEL_RUNS" ]; then - ARGS="$ARGS --parallel-runs $PARALLEL_RUNS" - fi if [ "${{ github.event.inputs.verbose }}" = "true" ]; then ARGS="$ARGS --verbose" fi - JUDGE_MODEL="${{ github.event.inputs.judge-model || env.DEFAULT_JUDGE_MODEL }}" - if [ -n "$JUDGE_MODEL" ]; then - ARGS="$ARGS --judge-model $JUDGE_MODEL" - fi - node eng/skill-validator/dist/index.js $ARGS --tests-dir ./src/${{ matrix.entry.component }}/tests ./${{ matrix.entry.skills_path }} + node artifacts/skill-validator/dist/index.js $ARGS --tests-dir ./src/${{ matrix.entry.component }}/tests ./${{ matrix.entry.skills_path }} - name: Upload results if: always() uses: actions/upload-artifact@v4 with: name: skill-validator-results-${{ matrix.entry.name }} - path: .skill-validator-results/ + path: artifacts/TestResults/skill-validator/${{ matrix.entry.name }}/ include-hidden-files: true retention-days: 30 - - name: Generate Summary - if: always() + comment-on-pr: + needs: [discover, evaluate] + if: always() && needs.discover.outputs.has_entries == 'true' + runs-on: ubuntu-latest + steps: + - name: Download all result artifacts + uses: actions/download-artifact@v4 + with: + pattern: skill-validator-results-* + path: all-results/ + merge-multiple: false + + - name: Consolidate summaries run: | - RESULTS_DIR=$(ls -d .skill-validator-results/run-* 2>/dev/null | head -1) - if [ -z "$RESULTS_DIR" ]; then - echo "## Skill Validation Results — ${{ matrix.entry.name }}" >> $GITHUB_STEP_SUMMARY - echo "No results found." >> $GITHUB_STEP_SUMMARY - exit 0 + COMMENT_MARKER="" + BODY="$COMMENT_MARKER"$'\n' + FOUND=false + + for COMPONENT_DIR in all-results/skill-validator-results-*/; do + SUMMARY="$COMPONENT_DIR/summary.md" + if [ -f "$SUMMARY" ]; then + BODY+=$(cat "$SUMMARY")$'\n\n' + FOUND=true + fi + done + + if [ "$FOUND" = "false" ]; then + BODY+="## Skill Validation Results"$'\n'"No results were produced."$'\n' fi - RESULTS_FILE="$RESULTS_DIR/results.json" - if [ ! -f "$RESULTS_FILE" ]; then - echo "## Skill Validation Results — ${{ matrix.entry.name }}" >> $GITHUB_STEP_SUMMARY - echo "No results.json found." >> $GITHUB_STEP_SUMMARY - exit 0 - fi + BODY+=$'\n'"[Full results](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})"$'\n' - # Generate markdown summary from results - node -e " - const fs = require('fs'); - const results = JSON.parse(fs.readFileSync('$RESULTS_FILE', 'utf8')); - let md = '## Skill Validation Results — ${{ matrix.entry.name }}\n\n'; - md += '| Skill | Test | Baseline | With Skill | Δ | Verdict |\n'; - md += '|-------|----------|----------|------------|---|--------|\n'; - for (const v of results.verdicts) { - for (const s of v.scenarios) { - const base = s.baseline?.judgeResult?.overallScore?.toFixed(1) ?? '—'; - const skill = s.withSkill?.judgeResult?.overallScore?.toFixed(1) ?? '—'; - const delta = (s.withSkill?.judgeResult?.overallScore - s.baseline?.judgeResult?.overallScore)?.toFixed(1); - const deltaStr = delta > 0 ? '+' + delta : delta; - const icon = v.passed ? '✅' : '❌'; - md += '| ' + v.skillName + ' | ' + s.scenarioName + ' | ' + base + '/5 | ' + skill + '/5 | ' + deltaStr + ' | ' + icon + ' |\n'; - } - } - md += '\nModel: ' + results.model + ' | Judge: ' + results.judgeModel + '\n'; - md += '\n[Full results](' + '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' + ')\n'; - fs.writeFileSync('.skill-validator-results/summary.md', md); - process.stdout.write(md); - " >> $GITHUB_STEP_SUMMARY - - - name: Comment on PR - if: always() && github.event_name == 'pull_request' + echo "$BODY" > consolidated-comment.md + cat consolidated-comment.md >> $GITHUB_STEP_SUMMARY + + - name: Post or update PR comment + if: github.event_name == 'pull_request' continue-on-error: true env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: gh pr comment ${{ github.event.pull_request.number }} --body-file .skill-validator-results/summary.md + run: | + PR_NUMBER=${{ github.event.pull_request.number }} + MARKER="" + + # Find existing comment with our marker + COMMENT_ID=$(gh api "repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ + --paginate --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" | head -1) + + if [ -n "$COMMENT_ID" ]; then + gh api "repos/${{ github.repository }}/issues/comments/${COMMENT_ID}" \ + -X PATCH -F "body=@consolidated-comment.md" + else + gh pr comment "$PR_NUMBER" --body-file consolidated-comment.md + fi publish-benchmark: needs: [discover, evaluate] diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3b3ab9cf71..cbe68f36af 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -213,21 +213,19 @@ Prerequisites: Node.js >= 20 and `gh auth login`. ```bash # Build the validator -cd eng/skill-validator -npm ci -npm run build +cd eng/skill-validator && npm ci && npm run build && cd ../.. # Run tests for a single component -node dist/index.js --tests-dir ./src/dotnet-msbuild/tests ./src/dotnet-msbuild/skills +node eng/skill-validator/dist/index.js --tests-dir src/dotnet-msbuild/tests src/dotnet-msbuild/skills # Run tests for a single skill (pass the skill directory directly) -node dist/index.js --tests-dir ./src/dotnet-msbuild/tests ./src/dotnet-msbuild/skills/common-build-errors +node eng/skill-validator/dist/index.js --tests-dir src/dotnet-msbuild/tests src/dotnet-msbuild/skills/common-build-errors # Fewer runs for faster iteration (default is 5) -node dist/index.js --runs 3 --tests-dir ./src/dotnet-msbuild/tests ./src/dotnet-msbuild/skills +node eng/skill-validator/dist/index.js --runs 1 --tests-dir src/dotnet-msbuild/tests src/dotnet-msbuild/skills # Use a specific model -node dist/index.js --model claude-opus-4.6 --tests-dir ./src/dotnet-msbuild/tests ./src/dotnet-msbuild/skills +node eng/skill-validator/dist/index.js --model claude-sonnet-4.5 --tests-dir src/dotnet-msbuild/tests src/dotnet-msbuild/skills ``` > [!WARNING] diff --git a/eng/skill-validator/README.md b/eng/skill-validator/README.md index 4b6ca32bd7..0617824f77 100644 --- a/eng/skill-validator/README.md +++ b/eng/skill-validator/README.md @@ -19,7 +19,7 @@ Plugging into your CI, it ensures every new skill adds real value, and existing ## Prerequisites -- Node.js >= 20 +- Node.js >= 22 - Authenticated with GitHub via `gh auth login` (the SDK picks up your credentials automatically) ## Install @@ -51,15 +51,14 @@ skill-validator --model gpt-5.3-codex --judge-model claude-opus-4.6-fast ./skill # Multiple runs for stability skill-validator --runs 5 ./skills/ -# Output as JSON or JUnit XML -skill-validator --reporter json:results.json ./skills/ -skill-validator --reporter junit:results.xml ./skills/ +# Override the default results directory (.skill-validator-results) +skill-validator --results-dir ./my-results ./skills/ + +# File reporters can also be specified explicitly. +skill-validator --reporter junit ./skills/ # Strict mode (require all skills to have evals) skill-validator --strict ./skills/ - -# Custom results directory -skill-validator --results-dir ./my-results ./skills/ ``` ## Writing eval files @@ -230,18 +229,18 @@ The default of 5 runs provides sufficient precision for significance testing (va | `--require-evals` | `false` | Fail if skill has no tests/eval.yaml | | `--strict` | `false` | Enable --require-evals and strict checking | | `--verbose` | `false` | Show tool calls and agent events during runs | -| `--reporter ` | `console` | Output format: `console`, `json:path`, `junit:path` | -| `--results-dir ` | `.skill-validator-results` | Directory for saved run results | -| `--no-save-results` | | Disable saving run results to disk | +| `--reporter ` | `console`, `json`, `markdown` | Output format: `console`, `json`, `junit`, `markdown`. | +| `--results-dir ` | `.skill-validator-results` | Directory for file reporter output. | Models are validated on startup — invalid model names fail fast with a list of available models. ## Output -Results are displayed in the console with color-coded scores and metric deltas. Run results are also auto-saved to `.skill-validator-results/run-{timestamp}/` containing: +Results are displayed in the console with color-coded scores and metric deltas. By default, `json` and `markdown` reporters are enabled and write to `.skill-validator-results/` (override with `--results-dir`). File reporters write to that directory: -- `results.json` — full results with model, timestamp, and all verdicts -- Per-skill directories with `verdict.json` and per-scenario markdown files +- `json` — `results.json` with model, timestamp, and all verdicts +- `junit` — `results.xml` with JUnit XML test results +- `markdown` — `summary.md` with a results table, plus per-skill directories with per-scenario judge reports ## CI integration diff --git a/eng/skill-validator/src/cli.ts b/eng/skill-validator/src/cli.ts index 38294c67d4..3fe711d31a 100644 --- a/eng/skill-validator/src/cli.ts +++ b/eng/skill-validator/src/cli.ts @@ -2,12 +2,12 @@ import { Command } from "commander"; import chalk from "chalk"; import pLimit from "p-limit"; import { discoverSkills } from "./discovery.js"; -import { runAgent, stopSharedClient, getSharedClient } from "./runner.js"; +import { runAgent, stopSharedClient, getSharedClient, cleanupWorkDirs } from "./runner.js"; import { evaluateAssertions, evaluateConstraints } from "./assertions.js"; import { judgeRun } from "./judge.js"; import { pairwiseJudge } from "./pairwise-judge.js"; import { compareScenario, computeVerdict } from "./comparator.js"; -import { reportResults, saveRunResults } from "./reporter.js"; +import { reportResults } from "./reporter.js"; import { analyzeSkill, formatProfileLine, formatProfileWarnings } from "./skill-profile.js"; import type { ValidatorConfig, @@ -86,11 +86,11 @@ class Spinner { } function parseReporter(value: string): ReporterSpec { - const [type, outputPath] = value.split(":"); - if (type !== "console" && type !== "json" && type !== "junit") { + const type = value; + if (type !== "console" && type !== "json" && type !== "junit" && type !== "markdown") { throw new Error(`Unknown reporter type: ${type}`); } - return { type, outputPath }; + return { type }; } export function createProgram(): Command { @@ -123,7 +123,7 @@ export function createProgram(): Command { .option("--confidence-level ", "Confidence level for statistical intervals (0-1)", "0.95") .option( "--results-dir ", - "Directory to save run results", + "Directory to save results to (default: .skill-validator-results). Used by file-based reporters (json, junit, markdown).", ".skill-validator-results" ) .option( @@ -132,11 +132,24 @@ export function createProgram(): Command { ) .option( "--reporter ", - "Reporter (console, json:path, junit:path). Can be repeated.", + "Reporter (console, json, junit, markdown). Can be repeated.", (val: string, prev: ReporterSpec[]) => [...prev, parseReporter(val)], [] as ReporterSpec[] ) .action(async (paths: string[], opts) => { + const reporters: ReporterSpec[] = + opts.reporter.length > 0 + ? opts.reporter + : [{ type: "console" as const }, { type: "json" as const }, { type: "markdown" as const }]; + + const fileReporters = reporters.filter((r) => r.type !== "console"); + if (fileReporters.length > 0 && !opts.resultsDir) { + const names = fileReporters.map((r) => r.type).join(", "); + throw new Error( + `--results-dir is required when using file-based reporters: ${names}` + ); + } + const config: ValidatorConfig = { minImprovement: parseFloat(opts.minImprovement), requireCompletion: opts.requireCompletion, @@ -152,12 +165,8 @@ export function createProgram(): Command { parallelRuns: Math.max(1, parseInt(opts.parallelRuns, 10) || 1), judgeTimeout: parseInt(opts.judgeTimeout, 10) * 1000, confidenceLevel: parseFloat(opts.confidenceLevel || "0.95"), - reporters: - opts.reporter.length > 0 - ? opts.reporter - : [{ type: "console" as const }], + reporters, skillPaths: paths, - saveResults: opts.saveResults !== false, resultsDir: opts.resultsDir, testsDir: opts.testsDir, }; @@ -471,14 +480,14 @@ export async function run(config: ValidatorConfig): Promise { } } - await reportResults(verdicts, config.reporters, config.verbose); - - if (config.saveResults) { - const runDir = await saveRunResults(verdicts, config.resultsDir, config.model, config.judgeModel); - console.log(chalk.dim(`Run results saved to ${runDir}`)); - } + await reportResults(verdicts, config.reporters, config.verbose, { + model: config.model, + judgeModel: config.judgeModel, + resultsDir: config.resultsDir, + }); await stopSharedClient(); + await cleanupWorkDirs(); const allPassed = verdicts.every((v) => v.passed); return allPassed ? 0 : 1; diff --git a/eng/skill-validator/src/discovery.ts b/eng/skill-validator/src/discovery.ts index af43262502..de2790d18e 100644 --- a/eng/skill-validator/src/discovery.ts +++ b/eng/skill-validator/src/discovery.ts @@ -81,9 +81,12 @@ export async function discoverSkills(targetPath: string, testsDir?: string): Pro if (!(await isDirectory(targetPath))) return skills; const entries = await readdir(targetPath, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory() || entry.name.startsWith(".")) continue; - const skill = await discoverSkillAt(join(targetPath, entry.name), testsDir); + const promises = entries + .filter((entry) => entry.isDirectory() && !entry.name.startsWith(".")) + .map((entry) => discoverSkillAt(join(targetPath, entry.name), testsDir)); + + const results = await Promise.all(promises); + for (const skill of results) { if (skill) skills.push(skill); } diff --git a/eng/skill-validator/src/reporter.ts b/eng/skill-validator/src/reporter.ts index d5fd64dc80..e2c5fde376 100644 --- a/eng/skill-validator/src/reporter.ts +++ b/eng/skill-validator/src/reporter.ts @@ -1,109 +1,45 @@ import chalk from "chalk"; import { writeFile, mkdir } from "node:fs/promises"; -import { join, dirname } from "node:path"; +import { join } from "node:path"; import type { SkillVerdict, ReporterSpec, ScenarioComparison } from "./types.js"; export async function reportResults( verdicts: SkillVerdict[], reporters: ReporterSpec[], - verbose: boolean + verbose: boolean, + config?: { model?: string; judgeModel?: string; resultsDir?: string } ): Promise { + const resultsDir = config?.resultsDir; + if (resultsDir) { + await mkdir(resultsDir, { recursive: true }); + } for (const reporter of reporters) { switch (reporter.type) { case "console": reportConsole(verdicts, verbose); break; case "json": - await reportJson(verdicts, reporter.outputPath); + if (!resultsDir) { + throw new Error("--results-dir is required for the json reporter"); + } + await reportJson(verdicts, resultsDir, config); break; case "junit": - await reportJunit(verdicts, reporter.outputPath); + if (!resultsDir) { + throw new Error("--results-dir is required for the junit reporter"); + } + await reportJunit(verdicts, resultsDir); + break; + case "markdown": + if (!resultsDir) { + throw new Error("--results-dir is required for the markdown reporter"); + } + await reportMarkdown(verdicts, resultsDir, config); break; } } } -export async function saveRunResults( - verdicts: SkillVerdict[], - resultsDir: string, - model?: string, - judgeModel?: string -): Promise { - const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); - const runDir = join(resultsDir, `run-${timestamp}`); - await mkdir(runDir, { recursive: true }); - - // Save full results JSON with metadata - const output = { - model: model ?? "unknown", - judgeModel: judgeModel ?? model ?? "unknown", - timestamp: new Date().toISOString(), - verdicts, - }; - await writeFile( - join(runDir, "results.json"), - JSON.stringify(output, null, 2), - "utf-8" - ); - - // Save per-skill detail files - for (const verdict of verdicts) { - const skillDir = join(runDir, verdict.skillName); - await mkdir(skillDir, { recursive: true }); - - await writeFile( - join(skillDir, "verdict.json"), - JSON.stringify(verdict, null, 2), - "utf-8" - ); - - for (const scenario of verdict.scenarios) { - const scenarioSlug = scenario.scenarioName - .toLowerCase() - .replace(/[^a-z0-9]+/g, "-"); - - // Save full judge output for both runs - const judgeReport = [ - `# Judge Report: ${scenario.scenarioName}`, - "", - `## Baseline Judge`, - `Overall Score: ${scenario.baseline.judgeResult.overallScore}/5`, - `Reasoning: ${scenario.baseline.judgeResult.overallReasoning}`, - "", - ...scenario.baseline.judgeResult.rubricScores.map( - (s) => `- **${s.criterion}**: ${s.score}/5 — ${s.reasoning}` - ), - "", - `## With-Skill Judge`, - `Overall Score: ${scenario.withSkill.judgeResult.overallScore}/5`, - `Reasoning: ${scenario.withSkill.judgeResult.overallReasoning}`, - "", - ...scenario.withSkill.judgeResult.rubricScores.map( - (s) => `- **${s.criterion}**: ${s.score}/5 — ${s.reasoning}` - ), - "", - `## Baseline Agent Output`, - "```", - scenario.baseline.metrics.agentOutput || "(no output)", - "```", - "", - `## With-Skill Agent Output`, - "```", - scenario.withSkill.metrics.agentOutput || "(no output)", - "```", - ].join("\n"); - - await writeFile( - join(skillDir, `${scenarioSlug}.md`), - judgeReport, - "utf-8" - ); - } - } - - return runDir; -} - function reportConsole(verdicts: SkillVerdict[], verbose: boolean): void { console.log("\n" + chalk.bold("═══ Skill Validation Results ═══") + "\n"); @@ -344,23 +280,100 @@ function truncate(s: string, max: number): string { return s.length > max ? s.slice(0, max - 3) + "..." : s; } -async function reportJson( +async function reportMarkdown( verdicts: SkillVerdict[], - outputPath?: string + resultsDir: string, + config?: { model?: string; judgeModel?: string } ): Promise { - const json = JSON.stringify(verdicts, null, 2); - if (outputPath) { - await mkdir(dirname(outputPath), { recursive: true }); - await writeFile(outputPath, json, "utf-8"); - console.log(`JSON results written to ${outputPath}`); - } else { - console.log(json); + let md= "## Skill Validation Results\n\n"; + md += "| Skill | Scenario | Baseline | With Skill | Δ | Verdict |\n"; + md += "|-------|----------|----------|------------|---|---------|\n"; + for (const v of verdicts) { + for (const s of v.scenarios) { + const base = s.baseline?.judgeResult?.overallScore?.toFixed(1) ?? "—"; + const skill = s.withSkill?.judgeResult?.overallScore?.toFixed(1) ?? "—"; + const delta = ( + (s.withSkill?.judgeResult?.overallScore ?? 0) - + (s.baseline?.judgeResult?.overallScore ?? 0) + ).toFixed(1); + const deltaStr = Number(delta) > 0 ? `+${delta}` : delta; + const icon = v.passed ? "✅" : "❌"; + md += `| ${v.skillName} | ${s.scenarioName} | ${base}/5 | ${skill}/5 | ${deltaStr} | ${icon} |\n`; + } + } + md += `\nModel: ${config?.model ?? "unknown"} | Judge: ${config?.judgeModel ?? "unknown"}\n`; + + await writeFile(join(resultsDir, "summary.md"), md, "utf-8"); + console.log(`Markdown summary written to ${join(resultsDir, "summary.md")}`); + + // Write per-scenario judge reports + for (const verdict of verdicts) { + const skillDir = join(resultsDir, verdict.skillName); + await mkdir(skillDir, { recursive: true }); + + for (const scenario of verdict.scenarios) { + const scenarioSlug = scenario.scenarioName + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-"); + + const judgeReport = [ + `# Judge Report: ${scenario.scenarioName}`, + "", + `## Baseline Judge`, + `Overall Score: ${scenario.baseline.judgeResult.overallScore}/5`, + `Reasoning: ${scenario.baseline.judgeResult.overallReasoning}`, + "", + ...scenario.baseline.judgeResult.rubricScores.map( + (s) => `- **${s.criterion}**: ${s.score}/5 — ${s.reasoning}` + ), + "", + `## With-Skill Judge`, + `Overall Score: ${scenario.withSkill.judgeResult.overallScore}/5`, + `Reasoning: ${scenario.withSkill.judgeResult.overallReasoning}`, + "", + ...scenario.withSkill.judgeResult.rubricScores.map( + (s) => `- **${s.criterion}**: ${s.score}/5 — ${s.reasoning}` + ), + "", + `## Baseline Agent Output`, + "```", + scenario.baseline.metrics.agentOutput || "(no output)", + "```", + "", + `## With-Skill Agent Output`, + "```", + scenario.withSkill.metrics.agentOutput || "(no output)", + "```", + ].join("\n"); + + await writeFile( + join(skillDir, `${scenarioSlug}.md`), + judgeReport, + "utf-8" + ); + } } } +async function reportJson( + verdicts: SkillVerdict[], + resultsDir: string, + config?: { model?: string; judgeModel?: string } +): Promise { + const output = { + model: config?.model ?? "unknown", + judgeModel: config?.judgeModel ?? config?.model ?? "unknown", + timestamp: new Date().toISOString(), + verdicts, + }; + const json= JSON.stringify(output, null, 2); + await writeFile(join(resultsDir, "results.json"), json, "utf-8"); + console.log(`JSON results written to ${join(resultsDir, "results.json")}`); +} + async function reportJunit( verdicts: SkillVerdict[], - outputPath?: string + resultsDir: string ): Promise { const testcases = verdicts.flatMap((verdict) => { if (verdict.scenarios.length === 0) { @@ -385,12 +398,8 @@ ${testcases.join("\n")} `; - if (outputPath) { - await writeFile(outputPath, xml, "utf-8"); - console.log(`JUnit results written to ${outputPath}`); - } else { - console.log(xml); - } + await writeFile(join(resultsDir, "results.xml"), xml, "utf-8"); + console.log(`JUnit results written to ${join(resultsDir, "results.xml")}`); } function escapeXml(s: string): string { diff --git a/eng/skill-validator/src/runner.ts b/eng/skill-validator/src/runner.ts index 7f215b2e00..825228281e 100644 --- a/eng/skill-validator/src/runner.ts +++ b/eng/skill-validator/src/runner.ts @@ -1,4 +1,4 @@ -import { mkdtemp, cp, writeFile, mkdir, readdir } from "node:fs/promises"; +import { mkdtemp, cp, writeFile, mkdir, readdir, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, dirname, resolve, sep } from "node:path"; import type { @@ -33,6 +33,7 @@ async function setupWorkDir( evalPath: string | null ): Promise { const workDir = await mkdtemp(join(tmpdir(), "skill-validator-")); + _workDirs.push(workDir); // Copy all sibling files from the eval directory when opted in if (evalPath && scenario.setup?.copy_test_files) { @@ -65,6 +66,7 @@ async function setupWorkDir( } let _sharedClient: CopilotClient | null = null; +const _workDirs: string[] = []; export async function getSharedClient(verbose: boolean): Promise { if (_sharedClient) return _sharedClient; @@ -85,6 +87,14 @@ export async function stopSharedClient(): Promise { } } +/** Remove all temporary working directories created during runs. */ +export async function cleanupWorkDirs(): Promise { + const dirs = _workDirs.splice(0); + await Promise.all( + dirs.map((dir) => rm(dir, { recursive: true, force: true }).catch(() => {})) + ); +} + export function checkPermission( req: PermissionRequest, workDir: string, @@ -142,59 +152,61 @@ export async function runAgent(options: RunOptions): Promise { buildSessionConfig(skill, model, workDir) ); - const idlePromise = new Promise((resolve, reject) => { - const timer = setTimeout(() => { - reject(new Error(`Scenario timed out after ${scenario.timeout}s`)); - }, (scenario.timeout ?? 120) * 1000); - - session.on((event: SessionEvent) => { - const agentEvent: AgentEvent = { - type: event.type, - timestamp: Date.now(), - data: event.data as Record, - }; - events.push(agentEvent); - - if ( - event.type === "assistant.message_delta" && - typeof event.data.deltaContent === "string" - ) { - agentOutput += event.data.deltaContent; - } - - if ( - event.type === "assistant.message" && - typeof event.data.content === "string" && - event.data.content !== "" - ) { - agentOutput = event.data.content; - } - - if (verbose) { - const write = options.log ?? ((msg: string) => process.stderr.write(`${msg}\n`)); - if (event.type === "tool.execution_start") { - write(` 🔧 ${event.data.toolName}`); - } else if (event.type === "assistant.message") { - write(` 💬 Response received`); + try { + const idlePromise = new Promise((resolve, reject) => { + const timer = setTimeout(() => { + reject(new Error(`Scenario timed out after ${scenario.timeout}s`)); + }, (scenario.timeout ?? 120) * 1000); + + session.on((event: SessionEvent) => { + const agentEvent: AgentEvent = { + type: event.type, + timestamp: Date.now(), + data: event.data as Record, + }; + events.push(agentEvent); + + if ( + event.type === "assistant.message_delta" && + typeof event.data.deltaContent === "string" + ) { + agentOutput += event.data.deltaContent; } - } - if (event.type === "session.idle") { - clearTimeout(timer); - resolve(); - } + if ( + event.type === "assistant.message" && + typeof event.data.content === "string" && + event.data.content !== "" + ) { + agentOutput = event.data.content; + } - if (event.type === "session.error") { - clearTimeout(timer); - reject(new Error(String(event.data.message || "Session error"))); - } - }); - }); + if (verbose) { + const write = options.log ?? ((msg: string) => process.stderr.write(`${msg}\n`)); + if (event.type === "tool.execution_start") { + write(` 🔧 ${event.data.toolName}`); + } else if (event.type === "assistant.message") { + write(` 💬 Response received`); + } + } - await session.send({ prompt: scenario.prompt }); - await idlePromise; + if (event.type === "session.idle") { + clearTimeout(timer); + resolve(); + } - await session.destroy(); + if (event.type === "session.error") { + clearTimeout(timer); + reject(new Error(String(event.data.message || "Session error"))); + } + }); + }); + + await session.send({ prompt: scenario.prompt }); + await idlePromise; + } finally { + await session.destroy(); + } } catch (error) { events.push({ type: "runner.error", diff --git a/eng/skill-validator/src/types.ts b/eng/skill-validator/src/types.ts index 953c2a1745..18dee32d54 100644 --- a/eng/skill-validator/src/types.ts +++ b/eng/skill-validator/src/types.ts @@ -190,14 +190,12 @@ export interface ValidatorConfig { confidenceLevel: number; reporters: ReporterSpec[]; skillPaths: string[]; - saveResults: boolean; - resultsDir: string; + resultsDir?: string; testsDir?: string; } export interface ReporterSpec { - type: "console" | "json" | "junit"; - outputPath?: string; + type: "console" | "json" | "junit" | "markdown"; } export const DEFAULT_WEIGHTS: Record = {