From c0817defbaf22fb9445bafe63ac489ed60e99d36 Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Thu, 29 Oct 2026 14:01:17 +0100 Subject: [PATCH 1/3] Enhance Vally evaluation output and reporting - Added detailed summaries for unresolved failures, including inline evidence and links to failed steps. - Improved error handling and logging for evaluation attempts. - Introduced new functions for summarizing evaluation results and extracting error messages. - Updated tests to cover new summary features and ensure accurate reporting of evaluation outcomes. --- .github/workflows/evals.yml | 38 +++++-- README.md | 6 + package.json | 2 +- scripts/eval-retry.mjs | 7 ++ scripts/eval-summary.mjs | 158 ++++++++++++++++++++++++++ scripts/eval-summary.test.mjs | 207 ++++++++++++++++++++++++++++++++++ scripts/eval.mjs | 39 +++---- 7 files changed, 426 insertions(+), 31 deletions(-) create mode 100644 scripts/eval-summary.mjs create mode 100644 scripts/eval-summary.test.mjs diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index 9db16558..c280197c 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -94,6 +94,7 @@ jobs: timeout-minutes: 180 permissions: contents: read + actions: read copilot-requests: write steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -106,20 +107,31 @@ jobs: - run: npm ci - run: npm run build - name: Run Vally + id: vally env: COPILOT_GITHUB_TOKEN: ${{ github.token }} GITHUB_TOKEN: ${{ github.token }} - run: npm run eval - - name: Publish Markdown reports - if: always() shell: bash run: | - if [ -d results ]; then - while IFS= read -r -d '' report; do - cat "$report" >> "$GITHUB_STEP_SUMMARY" - printf '\n' >> "$GITHUB_STEP_SUMMARY" - done < <(find results -name eval-results.md -type f -print0) + mkdir -p results + npm run eval 2>&1 | tee results/vally.log + - name: Publish failure summary + if: always() && steps.vally.outcome != 'skipped' + env: + GH_TOKEN: ${{ github.token }} + VALLY_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + VALLY_OUTCOME: ${{ steps.vally.outcome }} + VALLY_LOG: results/vally.log + shell: bash + run: | + if gh api --paginate --slurp \ + "repos/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID/attempts/$GITHUB_RUN_ATTEMPT/jobs?per_page=100" \ + > "$RUNNER_TEMP/vally-jobs.json"; then + export VALLY_JOBS="$RUNNER_TEMP/vally-jobs.json" + else + echo "::warning::Could not read pipeline steps; linking to the workflow run instead." fi + node scripts/eval-summary.mjs results - name: Upload reports and trajectories if: always() uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 @@ -135,10 +147,12 @@ jobs: if: always() runs-on: ubuntu-latest timeout-minutes: 5 - permissions: {} + permissions: + actions: read steps: - name: Report evaluation outcome env: + GH_TOKEN: ${{ github.token }} CHANGES_RESULT: ${{ needs.changes.result }} RELEVANT: ${{ needs.changes.outputs.relevant }} VALIDATION_RESULT: ${{ needs.validate.result }} @@ -153,6 +167,12 @@ jobs: fail() { echo "::error::$1" printf '%s\n' "$1" >> "$GITHUB_STEP_SUMMARY" + if ! gh api --paginate \ + "repos/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID/attempts/$GITHUB_RUN_ATTEMPT/jobs?per_page=100" \ + --jq '.jobs[] | select(.conclusion == "failure") | . as $job | .steps[] | select(.conclusion == "failure") | "- [\($job.name) / \(.name)](\($job.html_url)#step:\(.number):1)"' \ + >> "$GITHUB_STEP_SUMMARY"; then + echo "::warning::Could not link the failed pipeline steps." + fi exit 1 } [ "$CHANGES_RESULT" = success ] || fail "Change detection failed ($CHANGES_RESULT)." diff --git a/README.md b/README.md index 203579e1..cc103918 100644 --- a/README.md +++ b/README.md @@ -283,6 +283,12 @@ offline checks only. The aggregate check reports failures; branch protection det enforcement. See [Writing Tests](CONTRIBUTING.md#writing-tests) for authentication, grading, cost controls, and adding a rule test. +Actions summaries list only unresolved failures, grouped by loaded skills, executor, +model, and variant. Each row includes the failed graders on one line, a compact error +or grading explanation, and a link to the evaluation step. Passing and retry-recovered +stimuli appear only as counts. Full Markdown reports, unabridged grader evidence, +trajectories, the invocation manifest, and the console log remain in the results artifact. + ## Changelog See [CHANGELOG.md](CHANGELOG.md) for a dated history of updates to the agent kit, including the `cosmosdb-best-practices` skill and the testing framework. Each entry links to the PR that introduced the change. diff --git a/package.json b/package.json index 776af704..71a520a7 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "validate:manifests": "node scripts/validate-manifests.js", "eval": "cross-env VALLY_TELEMETRY_OPTOUT=1 node scripts/eval.mjs --output-dir results --workers 5 --max-retries 2 --require-pass", "eval:lint": "cross-env VALLY_TELEMETRY_OPTOUT=1 vally lint --eval-spec evals", - "eval:test": "node --test evals/cosmosdb-best-practices/graders.test.mjs scripts/eval-retry.test.mjs", + "eval:test": "node --test evals/cosmosdb-best-practices/graders.test.mjs scripts/eval-retry.test.mjs scripts/eval-summary.test.mjs", "version": "node scripts/version.js" }, "devDependencies": { diff --git a/scripts/eval-retry.mjs b/scripts/eval-retry.mjs index 6b9cafe7..db369ec0 100644 --- a/scripts/eval-retry.mjs +++ b/scripts/eval-retry.mjs @@ -18,6 +18,13 @@ function trialPassed(trial, threshold) { && resolveGradePass(trial.gradeResult, threshold); } +/** Apply the same verdict as stimulus aggregation when selecting report rows. */ +export function failedTrials(records) { + const threshold = thresholds(records); + return records.filter((trial) => trial.type === 'trial-result' + && !trialPassed(trial, threshold.get(trial.evalFilePath))); +} + /** Group trial results by stimulus; a stimulus passes only if every trial passes. */ export function summarizeStimuli(records) { const threshold = thresholds(records); diff --git a/scripts/eval-summary.mjs b/scripts/eval-summary.mjs new file mode 100644 index 00000000..b24eb7ee --- /dev/null +++ b/scripts/eval-summary.mjs @@ -0,0 +1,158 @@ +import { appendFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { stripVTControlCharacters } from 'node:util'; +import { failedTrials, summarizeStimuli, verifyRetry } from './eval-retry.mjs'; + +function compact(value, limit = 500) { + const text = stripVTControlCharacters(String(value)).replace(/\s+/g, ' ').trim(); + return text.length > limit ? `${text.slice(0, limit - 3)}...` : text; +} + +function cell(value) { + return compact(value).replace(/&/g, '&').replace(//g, '>') + .replace(/\|/g, '|').replace(/([\\`*_[\]])/g, '\\$1'); +} + +const matches = (trial, failure) => trial.evalFilePath === failure.evalFilePath + && trial.stimulus === failure.stimulus; + +function environment(trial) { + const metadata = trial.trajectory?.metadata; + const skills = metadata?.skillsLoaded; + return [ + skills?.length ? [...skills].sort().join(', ') : 'No loaded skills recorded', + metadata?.executor ?? 'executor unknown', + trial.model ?? metadata?.model ?? 'model unknown', + trial.variant ?? 'default', + ].join(' / '); +} + +function failureDetail(trial) { + if (trial.error) return { graders: '-', message: trial.error }; + if (trial.status !== 'success') { + return { graders: '-', message: trial.skipReason ?? `Trial ${trial.status}.` }; + } + if (!trial.gradeResult) return { graders: '-', message: 'No grade result was produced.' }; + const grade = trial.gradeResult; + const failed = (grade.details ?? []).filter((detail) => detail.status === 'error' || !detail.passed); + return { + graders: failed.map((detail) => detail.configuredName ?? detail.graderType ?? detail.name).join(', ') || 'aggregate', + message: failed.map((detail) => detail.evidence || `${detail.name} failed.`).join('; ') + || grade.evidence || `Score ${grade.score} did not meet the evaluation threshold.`, + }; +} + +/** Extract a useful crash message without dumping progress output or a stack trace. */ +export function extractLogError(log) { + const lines = stripVTControlCharacters(log).split(/\r?\n/).map((line) => line.trim()).filter(Boolean); + const errors = lines.filter((line) => /(?:\b[\w]*Error:|::error::|##\[error\])/i.test(line) + && !/Process completed with exit code/.test(line)); + return errors.length ? compact(errors.at(-1)) : undefined; +} + +export function findStepUrl(jobs, jobName = 'Run live evaluations', stepName = 'Run Vally') { + const job = jobs.find((entry) => entry.name === jobName); + const step = job?.steps?.find((entry) => entry.name === stepName); + return job?.html_url && step ? `${job.html_url}#step:${step.number}:1` : undefined; +} + +/** Failures only; successful and recovered stimuli remain counts, not table rows. */ +export function renderSummary(attempts, { stepUrl, runUrl, log = '', outcome } = {}) { + const first = attempts[0] ?? { status: 1, records: [] }; + const retryRuns = attempts.slice(1); + const initial = summarizeStimuli(first.records); + const failures = initial.filter((entry) => !entry.passed); + const { recovered, stillFailing } = verifyRetry(failures, retryRuns); + const lines = [ + '## Vally evaluation results', + '', + `**${stillFailing.length} failed | ${initial.length - failures.length} passed first attempt | ${recovered.length} recovered after retry**`, + '', + ]; + const link = stepUrl ? `[Run Vally](${stepUrl})` : runUrl ? `[Workflow run](${runUrl})` : 'Run Vally'; + const executionErrors = new Set(); + for (const [index, attempt] of attempts.entries()) { + if (attempt.error) executionErrors.add(`${index ? 'Retry' : 'First attempt'}: ${attempt.error}`); + for (const record of attempt.records) { + if (record.type !== 'run-summary') continue; + for (const entry of record.evals ?? []) { + if (entry.error) executionErrors.add(`${index ? 'Retry' : 'First attempt'}: ${entry.name}: ${entry.error}`); + } + } + if (!attempt.error && attempt.records.at(-1)?.type !== 'run-summary') { + executionErrors.add(`${index ? 'Retry' : 'First attempt'} did not produce a completed run summary (exit ${attempt.status}). ${extractLogError(log) ?? ''}`); + } + } + if (!attempts.length || (!executionErrors.size + && ((outcome === 'failure' && !stillFailing.length) || (first.status !== 0 && !failures.length)))) { + executionErrors.add(extractLogError(log) ?? 'Evaluation process failed without a retryable result; see the pipeline step.'); + } + for (const error of executionErrors) lines.push(`- **Error:** ${cell(error)} - ${link}`); + if (executionErrors.size) lines.push(''); + + const groups = new Map(); + for (const failure of stillFailing) { + const retry = retryRuns.findLast((run) => run.records.some((trial) => matches(trial, failure))); + const latestFailures = retry ? failedTrials(retry.records).filter((trial) => matches(trial, failure)) : []; + const trials = latestFailures.length ? latestFailures + : failedTrials(first.records).filter((trial) => matches(trial, failure)); + let note = ''; + if (retryRuns.length && !retry) note = 'Retry produced no matching result. '; + else if (retry && !latestFailures.length) { + note = `Retry did not confirm recovery (exit ${retry.status}, ${retry.records.at(-1)?.type === 'run-summary' ? 'complete' : 'incomplete'}); first failure: `; + } + for (const trial of trials) { + const key = environment(trial); + const rows = groups.get(key) ?? new Map(); + const detail = failureDetail(trial); + const rowKey = JSON.stringify([trial.evalFilePath, trial.stimulus]); + const row = rows.get(rowKey) ?? { stimulus: trial.stimulus, graders: new Set(), messages: new Set(), attempt: latestFailures.length ? 'Retry' : 'First' }; + row.graders.add(detail.graders); + row.messages.add(compact(note + detail.message)); + rows.set(rowKey, row); + groups.set(key, rows); + } + } + for (const [group, rows] of [...groups].sort(([a], [b]) => a.localeCompare(b))) { + lines.push(`### ${cell(group)}`, '', '| Failed stimulus | Attempt | Failed graders | Error | Step |', + '|---|---|---|---|---|'); + for (const row of [...rows.values()].sort((a, b) => a.stimulus.localeCompare(b.stimulus))) { + lines.push(`| ${cell(row.stimulus)} | ${row.attempt} | ${cell([...row.graders].join(', '))} | ${cell([...row.messages].join('; '))} | ${link} |`); + } + lines.push(''); + } + if (!stillFailing.length && !executionErrors.size) lines.push('No unresolved evaluation failures.', ''); + lines.push('Only unresolved failures are listed. Full reports, all grader details, and trajectories are in the results artifact.', ''); + return lines.join('\n'); +} + +export function readAttempts(outputDir) { + const manifest = join(outputDir, 'eval-attempts.json'); + if (!existsSync(manifest)) return []; + return JSON.parse(readFileSync(manifest, 'utf8')).map((attempt) => ({ + status: attempt.status, + error: attempt.error, + records: attempt.files.flatMap((file) => readFileSync(join(outputDir, file), 'utf8') + .split(/\r?\n/).filter(Boolean).map((line) => JSON.parse(line))), + })); +} + +if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + const outputDir = resolve(process.argv[2] ?? 'results'); + const logPath = process.env.VALLY_LOG; + const jobsPath = process.env.VALLY_JOBS; + const jobs = jobsPath ? JSON.parse(readFileSync(jobsPath, 'utf8')).flatMap((page) => page.jobs) : []; + const stepUrl = findStepUrl(jobs); + if (jobsPath && !stepUrl) console.warn('::warning::Could not locate the Run Vally step; linking to the workflow run instead.'); + const markdown = renderSummary(readAttempts(outputDir), { + stepUrl, + runUrl: process.env.VALLY_RUN_URL, + outcome: process.env.VALLY_OUTCOME, + log: logPath && existsSync(logPath) ? readFileSync(logPath, 'utf8') : '', + }); + mkdirSync(outputDir, { recursive: true }); + writeFileSync(join(outputDir, 'eval-summary.md'), markdown); + if (process.env.GITHUB_STEP_SUMMARY) appendFileSync(process.env.GITHUB_STEP_SUMMARY, markdown); + else console.log(markdown); +} diff --git a/scripts/eval-summary.test.mjs b/scripts/eval-summary.test.mjs new file mode 100644 index 00000000..7b7a3594 --- /dev/null +++ b/scripts/eval-summary.test.mjs @@ -0,0 +1,207 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { test } from 'node:test'; +import { extractLogError, findStepUrl, readAttempts, renderSummary } from './eval-summary.mjs'; + +const EVAL = '/repo/evals/query/eval.yaml'; +const STEP = 'https://github.com/owner/repo/actions/runs/10/job/20#step:6:1'; +const summary = (overrides = {}) => ({ + type: 'run-summary', + evals: [{ name: 'query', evalFilePath: EVAL, threshold: 1, ...overrides }], +}); +const trial = (stimulus, { passed = false, evidence = 'Missing partition filter.', ...overrides } = {}) => ({ + type: 'trial-result', + evalFilePath: EVAL, + stimulus, + status: 'success', + variant: 'main', + model: 'test-model', + gradeResult: { + passed, + score: passed ? 1 : 0.75, + evidence: '3/4 graders passed', + details: [ + { name: 'completed', graderType: 'completed', passed: true, evidence: 'Completed.' }, + { name: 'prompt "Long repeated rubric"', graderType: 'prompt', passed, evidence }, + ], + }, + trajectory: { metadata: { skillsLoaded: ['cosmosdb-best-practices'], executor: 'copilot-sdk' } }, + ...overrides, +}); +const run = (trials, status = 1) => ({ status, records: [...trials, summary()] }); + +test('shows only unresolved failures with inline evidence and a failed-step link', () => { + const markdown = renderSummary([run([trial('query-pass', { passed: true }), trial('query-fail')])], { stepUrl: STEP }); + assert.match(markdown, /1 failed \| 1 passed first attempt \| 0 recovered after retry/); + assert.match(markdown, /### cosmosdb-best-practices \/ copilot-sdk \/ test-model \/ main/); + assert.match(markdown, /\| query-fail \| First \| prompt \| Missing partition filter\./); + assert.ok(markdown.includes(`[Run Vally](${STEP})`)); + assert.doesNotMatch(markdown, /query-pass|completed|Long repeated rubric|
|
/); + assert.equal(markdown.split('\n').filter((line) => line.startsWith('|')).length, 3); +}); + +test('collapses category tables into one skill/environment group and keeps other environments separate', () => { + const markdown = renderSummary([run([ + trial('query-b'), + trial('index-a', { evalFilePath: '/repo/evals/index/eval.yaml' }), + trial('query-other-model', { model: 'other-model' }), + trial('query-other-variant', { variant: 'baseline' }), + trial('query-other-skills', { trajectory: { metadata: { skillsLoaded: ['other-skill'], executor: 'copilot-sdk' } } }), + ])]); + assert.equal(markdown.split('\n').filter((line) => line.startsWith('### ')).length, 4); + assert.ok(markdown.indexOf('| index-a |') < markdown.indexOf('| query-b |')); +}); + +test('omits recovered rows and uses retry evidence for persistent failures', () => { + const markdown = renderSummary([ + run([trial('flaky'), trial('persistent')]), + run([trial('flaky', { passed: true })], 0), + run([trial('persistent', { evidence: 'Retry still missed the filter.' })]), + ]); + assert.match(markdown, /1 failed \| 0 passed first attempt \| 1 recovered after retry/); + assert.doesNotMatch(markdown, /\| flaky \||Missing partition filter/); + assert.match(markdown, /\| persistent \| Retry \| prompt \| Retry still missed the filter/); +}); + +test('a passing record from an unsuccessful retry does not claim recovery', () => { + const markdown = renderSummary([run([trial('query-fail')]), run([trial('query-fail', { passed: true })])]); + assert.match(markdown, /1 failed/); + assert.match(markdown, /Retry did not confirm recovery \(exit 1, complete\); first failure: Missing partition filter/); +}); + +test('a retry with no matching result is explicit', () => { + const markdown = renderSummary([run([trial('query-fail')]), run([], 0)]); + assert.match(markdown, /Retry produced no matching result/); + assert.match(markdown, /1 failed/); +}); + +test('multiple failed trials share a row and graders stay on one line', () => { + const first = trial('query-fail'); + first.gradeResult.details.push({ name: 'output-matches', graderType: 'output-matches', passed: false, evidence: 'Output was empty.' }); + const markdown = renderSummary([run([first, { ...first, trialIndex: 1 }])]); + assert.equal(markdown.split('\n').filter((line) => line.startsWith('| query-fail |')).length, 1); + assert.match(markdown, /\| prompt, output-matches \| Missing partition filter\.; Output was empty\./); +}); + +test('uses the evaluation threshold rather than raw grader flags', () => { + const markdown = renderSummary([{ status: 0, records: [trial('lenient'), summary({ threshold: 0.5 })] }]); + assert.match(markdown, /0 failed \| 1 passed first attempt/); + assert.match(markdown, /No unresolved evaluation failures/); + assert.doesNotMatch(markdown, /\| lenient \|/); +}); + +test('reports execution errors, skipped trials, missing grades, and threshold failures without failed child graders', () => { + const markdown = renderSummary([run([ + trial('error', { status: 'error', error: 'Connection refused.', gradeResult: null, trajectory: null }), + trial('skip', { status: 'skipped', skipReason: 'Unsupported executor.' }), + trial('missing', { gradeResult: null }), + trial('aggregate', { gradeResult: { passed: false, score: 0.5 } }), + ])]); + for (const message of ['Connection refused.', 'Unsupported executor.', 'No grade result was produced.', 'Score 0.5 did not meet']) { + assert.ok(markdown.includes(message)); + } + assert.match(markdown, /No loaded skills recorded \/ executor unknown/); +}); + +test('escapes Markdown and HTML and bounds long multiline evidence', () => { + const markdown = renderSummary([run([trial('a|b', { evidence: '
\n[x](bad) | `code` & ' + 'x'.repeat(900) })])]); + const row = markdown.split('\n').find((line) => line.startsWith('| a')); + assert.match(row, /a|b/); + assert.match(row, /<details> \\\[x\\\]\(bad\) | \\`code\\` &/); + assert.ok(row.length < 650); + assert.match(row, /\.\.\./); + assert.doesNotMatch(markdown, /
/); +}); + +test('empty or crashed attempts expose errors rather than claiming success', () => { + for (const attempts of [[], [{ status: 1, records: [] }]]) { + const markdown = renderSummary(attempts, { log: '\u001b[31mError: Access denied.\u001b[0m\n at main()\n', stepUrl: STEP }); + assert.match(markdown, /Error: Access denied/); + assert.ok(markdown.includes(STEP)); + assert.doesNotMatch(markdown, /No unresolved evaluation failures/); + } +}); + +test('a terminated retry exposes its process error even when the first attempt had graded failures', () => { + const markdown = renderSummary([ + run([trial('query-fail')]), + { status: 1, records: [], error: 'Vally terminated by signal SIGTERM.' }, + ], { stepUrl: STEP, outcome: 'failure' }); + assert.match(markdown, /Retry: Vally terminated by signal SIGTERM/); + assert.match(markdown, /Retry produced no matching result/); + assert.match(markdown, /1 failed/); + assert.ok(markdown.includes(STEP)); +}); + +test('reports eval-level errors and an externally failed step even when trial records passed', () => { + const markdown = renderSummary([{ status: 1, records: [summary({ error: 'Invalid environment.' })] }]); + assert.match(markdown, /Invalid environment/); + assert.doesNotMatch(markdown, /without a retryable result/); + const external = renderSummary([run([trial('pass', { passed: true })], 0)], { + outcome: 'failure', log: 'Error: Process shutdown failed.', + }); + assert.match(external, /Process shutdown failed/); + assert.doesNotMatch(external, /No unresolved evaluation failures/); +}); + +test('all-pass and fully recovered runs have no tables', () => { + for (const attempts of [ + [run([trial('pass', { passed: true })], 0)], + [run([trial('flaky')]), run([trial('flaky', { passed: true })], 0)], + ]) { + const markdown = renderSummary(attempts); + assert.match(markdown, /No unresolved evaluation failures/); + assert.doesNotMatch(markdown, /Failed stimulus|\n\|/); + } +}); + +test('extracts error messages without generic exit-code noise', () => { + assert.equal(extractLogError('progress\nTypeError: Invalid input.\n at main\n##[error]Process completed with exit code 1.'), 'TypeError: Invalid input.'); + assert.equal(extractLogError('All done.'), undefined); +}); + +test('derives exact step anchors dynamically and falls back explicitly to the run', () => { + assert.equal(findStepUrl([{ name: 'Run live evaluations', html_url: STEP.split('#')[0], steps: [{ name: 'Run Vally', number: 6 }] }]), STEP); + assert.equal(findStepUrl([]), undefined); + const markdown = renderSummary([], { runUrl: 'https://github.com/owner/repo/actions/runs/10' }); + assert.match(markdown, /\[Workflow run\]\(https:\/\/github.com\/owner\/repo\/actions\/runs\/10\)/); +}); + +test('CLI loads only manifest-listed attempts and writes the artifact and Actions summary', () => { + const dir = mkdtempSync(join(tmpdir(), 'vally-summary-')); + try { + mkdirSync(join(dir, 'attempt-1')); + writeFileSync(join(dir, 'attempt-1', 'results.jsonl'), run([trial('query-fail')]).records.map((record) => JSON.stringify(record)).join('\n')); + writeFileSync(join(dir, 'stale.jsonl'), 'not valid JSON'); + writeFileSync(join(dir, 'eval-attempts.json'), JSON.stringify([{ status: 1, files: ['attempt-1/results.jsonl'] }])); + assert.equal(readAttempts(dir).length, 1); + const jobs = join(dir, 'jobs.json'); + writeFileSync(jobs, JSON.stringify([{ jobs: [] }, { jobs: [{ name: 'Run live evaluations', html_url: STEP.split('#')[0], steps: [{ name: 'Run Vally', number: 6 }] }] }])); + const stepSummary = join(dir, 'step-summary.md'); + const result = spawnSync(process.execPath, ['scripts/eval-summary.mjs', dir], { + encoding: 'utf8', + env: { ...process.env, GITHUB_STEP_SUMMARY: stepSummary, VALLY_JOBS: jobs, VALLY_OUTCOME: 'failure' }, + }); + assert.equal(result.status, 0, result.stderr); + const markdown = readFileSync(join(dir, 'eval-summary.md'), 'utf8'); + assert.equal(readFileSync(stepSummary, 'utf8'), markdown); + assert.ok(markdown.includes(STEP)); + assert.match(markdown, /Missing partition filter/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test('a malformed manifest is not silently treated as success', () => { + const dir = mkdtempSync(join(tmpdir(), 'vally-summary-invalid-')); + try { + assert.deepEqual(readAttempts(dir), []); + writeFileSync(join(dir, 'eval-attempts.json'), '{invalid'); + assert.throws(() => readAttempts(dir), SyntaxError); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/scripts/eval.mjs b/scripts/eval.mjs index 7710eacd..b45e133d 100644 --- a/scripts/eval.mjs +++ b/scripts/eval.mjs @@ -1,7 +1,7 @@ import { spawnSync } from 'node:child_process'; -import { appendFileSync, existsSync, readdirSync, readFileSync } from 'node:fs'; +import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; import { createRequire } from 'node:module'; -import { join, resolve } from 'node:path'; +import { join, relative, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { discoverEvals } from '@microsoft/vally'; import { planRetry, summarizeStimuli, verifyRetry } from './eval-retry.mjs'; @@ -32,6 +32,7 @@ const require = createRequire(import.meta.url); const cli = require.resolve('@microsoft/vally-cli/dist/index.js'); const listRuns = (dir) => existsSync(dir) ? new Set(readdirSync(dir)) : new Set(); +const attempts = []; // The Vally eval command accepts individual files, not directories or globs. function runVally(specs, tags, dir) { @@ -44,27 +45,23 @@ function runVally(specs, tags, dir) { '--output-dir', dir, ...passthrough, ], { stdio: 'inherit', env: process.env }); - if (result.error) { - throw result.error; - } - if (result.signal) { - throw new Error(`Vally terminated by signal ${result.signal}.`); - } - const records = [...listRuns(dir)].filter((name) => !before.has(name)) + const error = result.error ?? (result.signal ? new Error(`Vally terminated by signal ${result.signal}.`) : undefined); + const files = [...listRuns(dir)].filter((name) => !before.has(name)) .map((name) => join(dir, name, 'results.jsonl')) - .filter((file) => existsSync(file)) - .flatMap((file) => readFileSync(file, 'utf8').split(/\r?\n/).filter(Boolean) + .filter((file) => existsSync(file)); + attempts.push({ + status: result.status ?? 1, + ...(error ? { error: error.message } : {}), + files: files.map((file) => relative(resolve(outputDir), file)), + }); + mkdirSync(outputDir, { recursive: true }); + writeFileSync(join(outputDir, 'eval-attempts.json'), JSON.stringify(attempts)); + if (error) throw error; + const records = files.flatMap((file) => readFileSync(file, 'utf8').split(/\r?\n/).filter(Boolean) .map((line) => JSON.parse(line))); return { status: result.status ?? 1, records }; } -function report(lines) { - console.log(lines.join('\n')); - if (process.env.GITHUB_STEP_SUMMARY) { - appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${lines.join('\n')}\n\n`); - } -} - const first = runVally(evals.map(({ filePath }) => filePath), [], resolve(outputDir, 'attempt-1')); if (first.status === 0) { process.exit(0); @@ -85,14 +82,14 @@ const retryRuns = planRetry(failures) .map(({ specs, tags }) => runVally(specs, tags, resolve(outputDir, 'retry'))); const { recovered, stillFailing } = verifyRetry(failures, retryRuns); -report([ +console.log([ '## Vally retry summary', '', 'Each stimulus that failed the first attempt was retried once with unchanged grading.', '', '| Stimulus | First attempt | Retry |', '|---|---|---|', - ...recovered.map((f) => `| \`${f.stimulus}\` | ❌ | ✅ flaky |`), ...stillFailing.map((f) => `| \`${f.stimulus}\` | ❌ | ❌ |`), -]); + ...recovered.map((f) => `| \`${f.stimulus}\` | ❌ | ✅ flaky |`), +].join('\n')); process.exitCode = stillFailing.length === 0 ? 0 : 1; From 9db28f514398f3d470ecf4064529ff86171e44f4 Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Thu, 29 Oct 2026 14:48:18 +0100 Subject: [PATCH 2/3] Enhance Vally evaluation output with direct download links for results artifacts and improved test linking in summaries --- .github/workflows/evals.yml | 16 ++++++ README.md | 5 +- scripts/eval-summary.mjs | 40 +++++++++++++-- scripts/eval-summary.test.mjs | 93 +++++++++++++++++++++++++++++++++-- 4 files changed, 145 insertions(+), 9 deletions(-) diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index c280197c..ea2ed981 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -133,6 +133,7 @@ jobs: fi node scripts/eval-summary.mjs results - name: Upload reports and trajectories + id: reports if: always() uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: @@ -140,6 +141,21 @@ jobs: path: results/ if-no-files-found: warn retention-days: 14 + - name: Link results artifact + if: always() && steps.vally.outcome != 'skipped' + env: + ARTIFACT_URL: ${{ steps.reports.outputs.artifact-url }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + shell: bash + run: | + if [ -n "$ARTIFACT_URL" ]; then + printf '\n[Download full results artifact](%s) (reports, grader evidence, trajectories, and logs).\n' \ + "$ARTIFACT_URL" >> "$GITHUB_STEP_SUMMARY" + else + echo "::warning::Results artifact is unavailable; check the upload step." + printf '\n**Results artifact unavailable.** See the [workflow run](%s) for upload errors.\n' \ + "$RUN_URL" >> "$GITHUB_STEP_SUMMARY" + fi gate: name: Vally evaluation gate diff --git a/README.md b/README.md index cc103918..05ae0e8e 100644 --- a/README.md +++ b/README.md @@ -285,9 +285,12 @@ grading, cost controls, and adding a rule test. Actions summaries list only unresolved failures, grouped by loaded skills, executor, model, and variant. Each row includes the failed graders on one line, a compact error -or grading explanation, and a link to the evaluation step. Passing and retry-recovered +or grading explanation, and a link to the evaluation step. Failed test names link to +their definition and rubric at the tested commit. Passing and retry-recovered stimuli appear only as counts. Full Markdown reports, unabridged grader evidence, trajectories, the invocation manifest, and the console log remain in the results artifact. +The summary includes a direct download link after the artifact is uploaded, or an +explicit notice if the upload did not produce an artifact. ## Changelog diff --git a/scripts/eval-summary.mjs b/scripts/eval-summary.mjs index b24eb7ee..1bee9e55 100644 --- a/scripts/eval-summary.mjs +++ b/scripts/eval-summary.mjs @@ -1,5 +1,5 @@ import { appendFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; -import { join, resolve } from 'node:path'; +import { isAbsolute, join, relative, resolve, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import { stripVTControlCharacters } from 'node:util'; import { failedTrials, summarizeStimuli, verifyRetry } from './eval-retry.mjs'; @@ -57,8 +57,34 @@ export function findStepUrl(jobs, jobName = 'Run live evaluations', stepName = ' return job?.html_url && step ? `${job.html_url}#step:${step.number}:1` : undefined; } +export function findTestUrl(evalFilePath, stimulus, { workspace, sourceRootUrl } = {}) { + if (!workspace || !sourceRootUrl) return undefined; + const file = relative(workspace, evalFilePath); + if (!file || isAbsolute(file) || file === '..' || file.startsWith(`..${sep}`)) { + console.warn(`Cannot link test outside the repository: ${evalFilePath}`); + return undefined; + } + const url = `${sourceRootUrl}/${file.split(sep).map((part) => encodeURIComponent(part) + .replace(/[!'()*]/g, (character) => `%${character.charCodeAt(0).toString(16).toUpperCase()}`)).join('/')}`; + if (!existsSync(evalFilePath)) { + console.warn(`Test source is unavailable; linking to the spec without a line: ${evalFilePath}`); + return url; + } + const names = new Set([stimulus, JSON.stringify(stimulus), `'${stimulus.replace(/'/g, "''")}'`]); + const lines = readFileSync(evalFilePath, 'utf8').split(/\r?\n/); + const matches = lines.flatMap((line, index) => { + const name = line.match(/^\s*-\s+name:\s*(.*?)\s*$/)?.[1]; + return names.has(name) ? [index + 1] : []; + }); + if (matches.length !== 1) { + console.warn(`Cannot locate a unique test definition for ${stimulus}; linking to the spec: ${evalFilePath}`); + return url; + } + return `${url}#L${matches[0]}`; +} + /** Failures only; successful and recovered stimuli remain counts, not table rows. */ -export function renderSummary(attempts, { stepUrl, runUrl, log = '', outcome } = {}) { +export function renderSummary(attempts, { stepUrl, runUrl, log = '', outcome, testUrl = () => undefined } = {}) { const first = attempts[0] ?? { status: 1, records: [] }; const retryRuns = attempts.slice(1); const initial = summarizeStimuli(first.records); @@ -107,7 +133,7 @@ export function renderSummary(attempts, { stepUrl, runUrl, log = '', outcome } = const rows = groups.get(key) ?? new Map(); const detail = failureDetail(trial); const rowKey = JSON.stringify([trial.evalFilePath, trial.stimulus]); - const row = rows.get(rowKey) ?? { stimulus: trial.stimulus, graders: new Set(), messages: new Set(), attempt: latestFailures.length ? 'Retry' : 'First' }; + const row = rows.get(rowKey) ?? { evalFilePath: trial.evalFilePath, stimulus: trial.stimulus, graders: new Set(), messages: new Set(), attempt: latestFailures.length ? 'Retry' : 'First' }; row.graders.add(detail.graders); row.messages.add(compact(note + detail.message)); rows.set(rowKey, row); @@ -118,7 +144,9 @@ export function renderSummary(attempts, { stepUrl, runUrl, log = '', outcome } = lines.push(`### ${cell(group)}`, '', '| Failed stimulus | Attempt | Failed graders | Error | Step |', '|---|---|---|---|---|'); for (const row of [...rows.values()].sort((a, b) => a.stimulus.localeCompare(b.stimulus))) { - lines.push(`| ${cell(row.stimulus)} | ${row.attempt} | ${cell([...row.graders].join(', '))} | ${cell([...row.messages].join('; '))} | ${link} |`); + const url = testUrl(row.evalFilePath, row.stimulus); + const stimulus = url ? `[${cell(row.stimulus)}](${url})` : cell(row.stimulus); + lines.push(`| ${stimulus} | ${row.attempt} | ${cell([...row.graders].join(', '))} | ${cell([...row.messages].join('; '))} | ${link} |`); } lines.push(''); } @@ -145,11 +173,15 @@ if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.ur const jobs = jobsPath ? JSON.parse(readFileSync(jobsPath, 'utf8')).flatMap((page) => page.jobs) : []; const stepUrl = findStepUrl(jobs); if (jobsPath && !stepUrl) console.warn('::warning::Could not locate the Run Vally step; linking to the workflow run instead.'); + const { GITHUB_SERVER_URL, GITHUB_REPOSITORY, GITHUB_SHA, GITHUB_WORKSPACE } = process.env; + const sourceRootUrl = GITHUB_SERVER_URL && GITHUB_REPOSITORY && GITHUB_SHA + ? `${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/blob/${GITHUB_SHA}` : undefined; const markdown = renderSummary(readAttempts(outputDir), { stepUrl, runUrl: process.env.VALLY_RUN_URL, outcome: process.env.VALLY_OUTCOME, log: logPath && existsSync(logPath) ? readFileSync(logPath, 'utf8') : '', + testUrl: (file, stimulus) => findTestUrl(file, stimulus, { workspace: GITHUB_WORKSPACE, sourceRootUrl }), }); mkdirSync(outputDir, { recursive: true }); writeFileSync(join(outputDir, 'eval-summary.md'), markdown); diff --git a/scripts/eval-summary.test.mjs b/scripts/eval-summary.test.mjs index 7b7a3594..2ff4d592 100644 --- a/scripts/eval-summary.test.mjs +++ b/scripts/eval-summary.test.mjs @@ -2,12 +2,14 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { join, resolve } from 'node:path'; import { test } from 'node:test'; -import { extractLogError, findStepUrl, readAttempts, renderSummary } from './eval-summary.mjs'; +import matter from 'gray-matter'; +import { extractLogError, findStepUrl, findTestUrl, readAttempts, renderSummary } from './eval-summary.mjs'; const EVAL = '/repo/evals/query/eval.yaml'; const STEP = 'https://github.com/owner/repo/actions/runs/10/job/20#step:6:1'; +const SOURCE = 'https://github.com/owner/repo/blob/0123456789abcdef'; const summary = (overrides = {}) => ({ type: 'run-summary', evals: [{ name: 'query', evalFilePath: EVAL, threshold: 1, ...overrides }], @@ -170,11 +172,89 @@ test('derives exact step anchors dynamically and falls back explicitly to the ru assert.match(markdown, /\[Workflow run\]\(https:\/\/github.com\/owner\/repo\/actions\/runs\/10\)/); }); +test('links failed test names without adding columns or linking passing tests', () => { + const url = `${SOURCE}/evals/query/eval.yaml#L20`; + const markdown = renderSummary([run([trial('query-fail'), trial('query-pass', { passed: true })])], { + testUrl: (file, stimulus) => { + assert.equal(file, EVAL); + assert.equal(stimulus, 'query-fail'); + return url; + }, + }); + assert.ok(markdown.includes(`| [query-fail](${url}) | First |`)); + assert.equal(markdown.split('\n').filter((line) => line.startsWith('|')).length, 3); +}); + +test('test links use the checked-out path, commit, and exact definition line', () => { + const dir = mkdtempSync(join(tmpdir(), 'vally-test-link-')); + try { + const file = join(dir, 'eval [query](test).yaml'); + writeFileSync(file, [ + 'stimuli:', + ' - name: plain-name', + ' prompt: Explain this.', + ' - name: "quoted-name"', + " - name: 'single-quoted'", + ].join('\r\n')); + for (const [stimulus, line] of [['plain-name', 2], ['quoted-name', 4], ['single-quoted', 5]]) { + assert.equal(findTestUrl(file, stimulus, { workspace: dir, sourceRootUrl: SOURCE }), + `${SOURCE}/eval%20%5Bquery%5D%28test%29.yaml#L${line}`); + } + assert.equal(findTestUrl(file, 'plain-name'), undefined); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test('missing or ambiguous source locations warn and link the spec, never an invented line', (t) => { + const warn = t.mock.method(console, 'warn', () => {}); + const dir = mkdtempSync(join(tmpdir(), 'vally-test-link-missing-')); + try { + const file = join(dir, 'eval.yaml'); + const options = { workspace: dir, sourceRootUrl: SOURCE }; + assert.equal(findTestUrl(file, 'missing', options), `${SOURCE}/eval.yaml`); + writeFileSync(file, 'stimuli:\n - name: repeated\n - name: repeated\n'); + assert.equal(findTestUrl(file, 'missing', options), `${SOURCE}/eval.yaml`); + assert.equal(findTestUrl(file, 'repeated', options), `${SOURCE}/eval.yaml`); + assert.equal(findTestUrl(join(dir, '..', 'outside.yaml'), 'missing', options), undefined); + assert.equal(warn.mock.calls.length, 4); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test('the actual interrupt test link points at its definition and nearby rubric', () => { + const file = resolve('evals/cosmosdb-best-practices/pattern/eval.yaml'); + const url = findTestUrl(file, 'pattern-langgraph-interrupt-human', { workspace: process.cwd(), sourceRootUrl: SOURCE }); + assert.match(url, /\/evals\/cosmosdb-best-practices\/pattern\/eval.yaml#L\d+$/); + const index = Number(url.split('#L')[1]) - 1; + const lines = readFileSync(file, 'utf8').split(/\r?\n/); + assert.equal(lines[index].trim(), '- name: pattern-langgraph-interrupt-human'); + assert.ok(lines.slice(index, index + 8).some((line) => line.includes('resumption restarts the node'))); +}); + +test('the artifact link uses the upload output and is published even after a failed evaluation', () => { + const source = readFileSync(new URL('../.github/workflows/evals.yml', import.meta.url), 'utf8'); + const workflow = matter(`---\n${source}\n---`).data; + const steps = workflow.jobs.eval.steps; + const upload = steps.findIndex((step) => step.id === 'reports'); + const link = steps.findIndex((step) => step.name === 'Link results artifact'); + assert.ok(upload >= 0 && link > upload); + assert.match(steps[upload].uses, /^actions\/upload-artifact@/); + assert.equal(steps[link].if, "always() && steps.vally.outcome != 'skipped'"); + assert.equal(steps[link].env.ARTIFACT_URL, '${{ steps.reports.outputs.artifact-url }}'); + assert.match(steps[link].run, /Download full results artifact/); + assert.match(steps[link].run, /Results artifact unavailable/); +}); + test('CLI loads only manifest-listed attempts and writes the artifact and Actions summary', () => { const dir = mkdtempSync(join(tmpdir(), 'vally-summary-')); try { mkdirSync(join(dir, 'attempt-1')); - writeFileSync(join(dir, 'attempt-1', 'results.jsonl'), run([trial('query-fail')]).records.map((record) => JSON.stringify(record)).join('\n')); + const spec = join(dir, 'eval.yaml'); + writeFileSync(spec, 'stimuli:\n - name: query-fail\n'); + const records = [trial('query-fail', { evalFilePath: spec }), summary({ evalFilePath: spec })]; + writeFileSync(join(dir, 'attempt-1', 'results.jsonl'), records.map((record) => JSON.stringify(record)).join('\n')); writeFileSync(join(dir, 'stale.jsonl'), 'not valid JSON'); writeFileSync(join(dir, 'eval-attempts.json'), JSON.stringify([{ status: 1, files: ['attempt-1/results.jsonl'] }])); assert.equal(readAttempts(dir).length, 1); @@ -183,13 +263,18 @@ test('CLI loads only manifest-listed attempts and writes the artifact and Action const stepSummary = join(dir, 'step-summary.md'); const result = spawnSync(process.execPath, ['scripts/eval-summary.mjs', dir], { encoding: 'utf8', - env: { ...process.env, GITHUB_STEP_SUMMARY: stepSummary, VALLY_JOBS: jobs, VALLY_OUTCOME: 'failure' }, + env: { + ...process.env, GITHUB_STEP_SUMMARY: stepSummary, VALLY_JOBS: jobs, VALLY_OUTCOME: 'failure', + GITHUB_SERVER_URL: 'https://github.com', GITHUB_REPOSITORY: 'owner/repo', + GITHUB_SHA: '0123456789abcdef', GITHUB_WORKSPACE: dir, + }, }); assert.equal(result.status, 0, result.stderr); const markdown = readFileSync(join(dir, 'eval-summary.md'), 'utf8'); assert.equal(readFileSync(stepSummary, 'utf8'), markdown); assert.ok(markdown.includes(STEP)); assert.match(markdown, /Missing partition filter/); + assert.ok(markdown.includes(`[query-fail](${SOURCE}/eval.yaml#L2)`)); } finally { rmSync(dir, { recursive: true, force: true }); } From 79c394102190737c78f6ec94fffd0df579a610fe Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Thu, 29 Oct 2026 16:49:22 +0100 Subject: [PATCH 3/3] Improve error handling in readAttempts function Refactor readAttempts function to handle file read errors gracefully and collect multiple error messages. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Laurent Nguyen --- scripts/eval-summary.mjs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/scripts/eval-summary.mjs b/scripts/eval-summary.mjs index 1bee9e55..f0ed2aca 100644 --- a/scripts/eval-summary.mjs +++ b/scripts/eval-summary.mjs @@ -158,12 +158,20 @@ export function renderSummary(attempts, { stepUrl, runUrl, log = '', outcome, te export function readAttempts(outputDir) { const manifest = join(outputDir, 'eval-attempts.json'); if (!existsSync(manifest)) return []; - return JSON.parse(readFileSync(manifest, 'utf8')).map((attempt) => ({ - status: attempt.status, - error: attempt.error, - records: attempt.files.flatMap((file) => readFileSync(join(outputDir, file), 'utf8') - .split(/\r?\n/).filter(Boolean).map((line) => JSON.parse(line))), - })); + return JSON.parse(readFileSync(manifest, 'utf8')).map((attempt) => { + const records = []; + const errors = [attempt.error].filter(Boolean); + for (const file of attempt.files) { + try { + for (const line of readFileSync(join(outputDir, file), 'utf8').split(/\r?\n/).filter(Boolean)) { + records.push(JSON.parse(line)); + } + } catch (error) { + errors.push(`Could not read ${file}: ${error.message}`); + } + } + return { status: attempt.status, error: errors.length ? errors.join('; ') : undefined, records }; + }); } if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {