diff --git a/.github/scripts/eval-report.mjs b/.github/scripts/eval-report.mjs deleted file mode 100644 index 92ec880..0000000 --- a/.github/scripts/eval-report.mjs +++ /dev/null @@ -1,546 +0,0 @@ -import { existsSync, readFileSync, readdirSync, realpathSync, statSync } from "node:fs"; -import { relative, resolve, sep } from "node:path"; -import { fileURLToPath } from "node:url"; -import { parseArgs, stripVTControlCharacters } from "node:util"; -import { loadProjectConfig } from "@microsoft/vally/config"; - -const hasGraderError = (grade) => grade?.status === "error" || (grade?.details ?? []).some(hasGraderError); -const stripTerminalControls = (text) => stripVTControlCharacters(String(text ?? "")) - // eslint-disable-next-line no-control-regex - .replace(/[\u0000-\u001f\u007f-\u009f\u202a-\u202e\u2066-\u2069]+/gu, " ") - .trim(); -const escapeCell = (text) => stripTerminalControls(text).replaceAll("|", "\\|").replaceAll("<", "<").replaceAll(">", ">"); -const finiteNonNegative = (value) => typeof value === "number" && Number.isFinite(value) && value >= 0 ? value : undefined; -const sanitizeIdentifier = (value, fallback = "check") => { - const cleaned = String(value ?? fallback).replace(/[^A-Za-z0-9._:-]+/g, "-").replace(/^-+|-+$/g, ""); - return cleaned || fallback; -}; -const sanitizeGraderId = (detail) => sanitizeIdentifier( - typeof detail === "string" ? detail : (detail?.configuredName ?? detail?.name ?? detail?.graderType ?? "check"), - "check", -); - -export function extractTrialMetrics(record) { - const durationMs = finiteNonNegative(record?.durationMs) ?? finiteNonNegative(record?.trajectory?.metrics?.wallTimeMs); - const rawUsage = record?.trajectory?.metrics?.tokenUsage; - let tokenUsage; - if (rawUsage && typeof rawUsage === "object" && !Array.isArray(rawUsage)) { - const inputTokens = finiteNonNegative(rawUsage.inputTokens); - const outputTokens = finiteNonNegative(rawUsage.outputTokens); - const totalTokens = finiteNonNegative(rawUsage.totalTokens) - ?? (inputTokens !== undefined && outputTokens !== undefined ? inputTokens + outputTokens : undefined); - if (totalTokens !== undefined) { - tokenUsage = { - totalTokens, - ...(inputTokens !== undefined ? { inputTokens } : {}), - ...(outputTokens !== undefined ? { outputTokens } : {}), - }; - } - } - return { durationMs, tokenUsage }; -} - -export function formatTokens(tokenUsage) { - if (!tokenUsage || typeof tokenUsage.totalTokens !== "number") return "—"; - const total = tokenUsage.totalTokens.toLocaleString("en-US"); - if (typeof tokenUsage.inputTokens === "number" && typeof tokenUsage.outputTokens === "number") { - return `${total} (in: ${tokenUsage.inputTokens.toLocaleString("en-US")}, out: ${tokenUsage.outputTokens.toLocaleString("en-US")})`; - } - return total; -} - -export function formatDuration(durationMs) { - if (typeof durationMs !== "number" || !Number.isFinite(durationMs) || durationMs < 0) return "—"; - if (durationMs < 1000) return `${Math.round(durationMs)}ms`; - return `${(durationMs / 1000).toFixed(1)}s`; -} - -export function formatScore(score) { - if (typeof score !== "number" || !Number.isFinite(score)) return "Not scored"; - return `${(score * 100).toFixed(1)}%`; -} - -export function summarizeRun(records) { - if (!Array.isArray(records) || !records.length || records.at(-1)?.type !== "run-summary") { - throw new Error("The run has no completion summary."); - } - const runIds = new Set(records.map((record) => record?.runId)); - if (runIds.size !== 1 || typeof records[0].runId !== "string") { - throw new Error("Missing or mixed run identities."); - } - - const caseGroups = new Map(); - const errors = []; - let passedCount = 0; - let failedCount = 0; - let errorCount = 0; - let scoredCount = 0; - let scoreSum = 0; - let totalDurationMs = 0; - let allTrialsHaveDuration = true; - let totalInputTokens = 0; - let totalOutputTokens = 0; - let totalTokens = 0; - let allTrialsHaveTokens = true; - let hasInOutTokens = true; - let trialCount = 0; - - for (const record of records) { - if (record.type === "run-summary") { - if (typeof record.hadExecutionErrors !== "boolean") throw new Error("Invalid run summary."); - if (record.hadExecutionErrors) errors.push("execution-error"); - continue; - } - if (record.type !== "trial-result" || typeof record.stimulus !== "string" || !record.stimulus.trim()) { - throw new Error("Unexpected trial identity."); - } - if (!["success", "error", "skipped"].includes(record.status)) { - throw new Error("Unknown trial status."); - } - trialCount++; - - const grade = record.gradeResult; - const runtimeErrors = (record.trajectory?.events ?? []).filter((event) => event.type === "error"); - const fatalCode = record.status === "skipped" - ? "skipped" - : record.status !== "success" - ? "execution-error" - : !grade || hasGraderError(grade) - ? "grader-error" - : runtimeErrors.length > 0 - ? "runtime-error" - : undefined; - const fatal = fatalCode !== undefined; - if (!fatal && (typeof grade.passed !== "boolean" || !Number.isFinite(grade.score) || grade.score < 0 || grade.score > 1)) { - throw new Error(`Invalid grade: ${sanitizeIdentifier(record.stimulus, "trial")}`); - } - if (fatal) { - errorCount++; - errors.push(`${sanitizeIdentifier(record.stimulus, "trial")}: ${fatalCode}`); - } else if (grade.passed) { - passedCount++; - scoredCount++; - scoreSum += grade.score; - } else { - failedCount++; - scoredCount++; - scoreSum += grade.score; - } - - const { durationMs, tokenUsage } = extractTrialMetrics(record); - if (durationMs !== undefined) { - totalDurationMs += durationMs; - } else { - allTrialsHaveDuration = false; - } - if (tokenUsage !== undefined) { - totalTokens += tokenUsage.totalTokens; - if (typeof tokenUsage.inputTokens === "number" && typeof tokenUsage.outputTokens === "number") { - totalInputTokens += tokenUsage.inputTokens; - totalOutputTokens += tokenUsage.outputTokens; - } else { - hasInOutTokens = false; - } - } else { - allTrialsHaveTokens = false; - } - - const evalName = typeof record.evalName === "string" && record.evalName.trim() ? record.evalName.trim() : undefined; - const groupKey = `${evalName ?? ""}\0${record.stimulus}`; - let group = caseGroups.get(groupKey); - if (!group) { - group = { - evalName, - name: record.stimulus, - trials: 0, - passedCount: 0, - hasFatal: false, - allPassed: true, - allScored: true, - scoreSum: 0, - allHaveDuration: true, - durationSum: 0, - allHaveTokens: true, - allHaveInOutTokens: true, - totalTokensSum: 0, - inputTokensSum: 0, - outputTokensSum: 0, - failures: new Set(), - }; - caseGroups.set(groupKey, group); - } - - group.trials++; - if (fatal) { - group.hasFatal = true; - group.allPassed = false; - group.allScored = false; - } else { - if (grade.passed) { - group.passedCount++; - } else { - group.allPassed = false; - } - group.scoreSum += grade.score; - } - - if (durationMs !== undefined) { - group.durationSum += durationMs; - } else { - group.allHaveDuration = false; - } - - if (tokenUsage !== undefined) { - group.totalTokensSum += tokenUsage.totalTokens; - if (typeof tokenUsage.inputTokens === "number" && typeof tokenUsage.outputTokens === "number") { - group.inputTokensSum += tokenUsage.inputTokens; - group.outputTokensSum += tokenUsage.outputTokens; - } else { - group.allHaveInOutTokens = false; - } - } else { - group.allHaveTokens = false; - group.allHaveInOutTokens = false; - } - - for (const detail of (grade?.details ?? [])) { - if (!detail.passed) { - group.failures.add(sanitizeGraderId(detail)); - } - } - } - - if (!trialCount) throw new Error("No cases were evaluated."); - - const rows = [...caseGroups.values()].map((group) => { - const passed = !group.hasFatal && group.allPassed; - const status = group.hasFatal ? "ERROR / NOT EVALUATED" : passed ? "PASS" : "BEHAVIORAL FAILURE"; - return { - evalName: group.evalName, - name: group.name, - trials: group.trials, - passedCount: group.passedCount, - status, - passed, - score: !group.hasFatal && group.allScored ? group.scoreSum / group.trials : undefined, - durationMs: group.allHaveDuration ? group.durationSum : undefined, - tokenUsage: group.allHaveTokens - ? { - totalTokens: group.totalTokensSum, - ...(group.allHaveInOutTokens - ? { inputTokens: group.inputTokensSum, outputTokens: group.outputTokensSum } - : {}), - } - : undefined, - failures: [...group.failures], - }; - }); - - return { - runId: records[0].runId, - rows, - errors, - passedCount, - failedCount, - errorCount, - averageScore: scoredCount > 0 ? scoreSum / scoredCount : undefined, - totalDurationMs: allTrialsHaveDuration ? totalDurationMs : undefined, - tokenUsage: allTrialsHaveTokens - ? { - totalTokens, - ...(hasInOutTokens ? { inputTokens: totalInputTokens, outputTokens: totalOutputTokens } : {}), - } - : undefined, - }; -} - -function caseKey(row, useEvalPrefix) { - return useEvalPrefix && row.evalName ? `${row.evalName} / ${row.name}` : row.name; -} - -export function classifyTransition(baselineRow, candidateRow) { - if (!baselineRow) return "Added"; - if (!candidateRow) return "Removed"; - if (baselineRow.status !== "PASS" && candidateRow.status === "PASS") return "Fixed"; - if (baselineRow.status === "PASS" && candidateRow.status !== "PASS") return "Regressed"; - if (baselineRow.status === "BEHAVIORAL FAILURE" && candidateRow.status === "ERROR / NOT EVALUATED") return "Regressed"; - if (baselineRow.status === "ERROR / NOT EVALUATED" || candidateRow.status === "ERROR / NOT EVALUATED") return "Unchanged"; - if (typeof baselineRow.score !== "number" || !Number.isFinite(baselineRow.score) - || typeof candidateRow.score !== "number" || !Number.isFinite(candidateRow.score)) { - return "Unchanged"; - } - if (candidateRow.score > baselineRow.score) return "Improved score"; - if (candidateRow.score < baselineRow.score) return "Regressed"; - if (baselineRow.status === candidateRow.status && (baselineRow.trials ?? 1) === (candidateRow.trials ?? 1)) { - const baseTokens = baselineRow.tokenUsage?.totalTokens; - const candTokens = candidateRow.tokenUsage?.totalTokens; - if (typeof baseTokens === "number" && typeof candTokens === "number") { - if (candTokens < baseTokens) return "Reduced tokens at parity"; - if (candTokens > baseTokens) return "Increased tokens at parity"; - } - } - return "Unchanged"; -} - -export function compareSummaries(baseline, candidate) { - const useEvalPrefix = baseline.rows.every((row) => Boolean(row.evalName)) - && candidate.rows.every((row) => Boolean(row.evalName)); - const baselineMap = new Map(baseline.rows.map((row) => [caseKey(row, useEvalPrefix), row])); - const candidateMap = new Map(candidate.rows.map((row) => [caseKey(row, useEvalPrefix), row])); - const keys = [...new Set([...baselineMap.keys(), ...candidateMap.keys()])]; - - const cases = keys.map((key) => { - const baseRow = baselineMap.get(key); - const candRow = candidateMap.get(key); - const sameTrialCount = (baseRow?.trials ?? 1) === (candRow?.trials ?? 1); - const scoreDelta = typeof baseRow?.score === "number" && typeof candRow?.score === "number" - ? candRow.score - baseRow.score - : undefined; - const baseTokens = baseRow?.tokenUsage?.totalTokens; - const candTokens = candRow?.tokenUsage?.totalTokens; - const tokenDelta = sameTrialCount && typeof baseTokens === "number" && typeof candTokens === "number" - ? candTokens - baseTokens - : undefined; - const tokenDeltaPct = tokenDelta !== undefined && baseTokens > 0 - ? (tokenDelta / baseTokens) * 100 - : undefined; - const durationDeltaMs = sameTrialCount && typeof baseRow?.durationMs === "number" && typeof candRow?.durationMs === "number" - ? candRow.durationMs - baseRow.durationMs - : undefined; - - return { - key, - baseline: baseRow, - candidate: candRow, - scoreDelta, - tokenDelta, - tokenDeltaPct, - durationDeltaMs, - transition: classifyTransition(baseRow, candRow), - }; - }); - - const workloadsMatch = - baseline.rows.length === candidate.rows.length && - cases.every((item) => item.baseline && item.candidate && item.baseline.trials === item.candidate.trials); - const baseTotalTokens = baseline.tokenUsage?.totalTokens; - const candTotalTokens = candidate.tokenUsage?.totalTokens; - const totalTokenDelta = workloadsMatch && typeof baseTotalTokens === "number" && typeof candTotalTokens === "number" - ? candTotalTokens - baseTotalTokens - : undefined; - const totalTokenDeltaPct = totalTokenDelta !== undefined && baseTotalTokens > 0 - ? (totalTokenDelta / baseTotalTokens) * 100 - : undefined; - - return { - baseline, - candidate, - cases, - deltas: { - passed: candidate.passedCount - baseline.passedCount, - failed: (candidate.failedCount + candidate.errorCount) - (baseline.failedCount + baseline.errorCount), - averageScore: typeof baseline.averageScore === "number" && typeof candidate.averageScore === "number" - ? candidate.averageScore - baseline.averageScore - : undefined, - totalTokens: totalTokenDelta, - totalTokensPct: totalTokenDeltaPct, - totalDurationMs: workloadsMatch && typeof baseline.totalDurationMs === "number" && typeof candidate.totalDurationMs === "number" - ? candidate.totalDurationMs - baseline.totalDurationMs - : undefined, - }, - }; -} - -function formatSignedInt(value) { - if (typeof value !== "number" || !Number.isFinite(value)) return "—"; - if (value === 0) return "0"; - return `${value > 0 ? "+" : ""}${value.toLocaleString("en-US")}`; -} - -function formatScoreDelta(delta) { - if (typeof delta !== "number" || !Number.isFinite(delta)) return "—"; - const points = delta * 100; - if (Math.abs(points) < 0.05) return "0.0%"; - return `${points > 0 ? "+" : ""}${points.toFixed(1)}%`; -} - -function formatTokenDelta(delta, pct) { - if (typeof delta !== "number" || !Number.isFinite(delta)) return "—"; - if (delta === 0) return "0 (0.0%)"; - const sign = delta > 0 ? "+" : ""; - const pctText = typeof pct === "number" && Number.isFinite(pct) - ? ` (${pct > 0 ? "+" : ""}${pct.toFixed(1)}%)` - : ""; - return `${sign}${delta.toLocaleString("en-US")}${pctText}`; -} - -function formatDurationDelta(deltaMs) { - if (typeof deltaMs !== "number" || !Number.isFinite(deltaMs)) return "—"; - if (deltaMs === 0) return "0.0s"; - const sign = deltaMs > 0 ? "+" : "-"; - return `${sign}${formatDuration(Math.abs(deltaMs))}`; -} - -export function renderReport(summary, path, { baselineSummary, baselinePath } = {}) { - const lines = [ - "## Behavioral eval report", - "", - `Saved run: \`${escapeCell(path)}\``, - "", - "| Passed | Failed | Avg Score | Total Tokens | Total Duration |", - "| --- | --- | --- | --- | --- |", - `| ${summary.passedCount} | ${summary.failedCount + summary.errorCount} | ${formatScore(summary.averageScore)} | ${formatTokens(summary.tokenUsage)} | ${formatDuration(summary.totalDurationMs)} |`, - "", - "| Case | Verdict | Score | Tokens | Duration |", - "| --- | --- | --- | --- | --- |", - ...summary.rows.map((row) => - `| ${escapeCell(row.name)} | ${row.status} | ${formatScore(row.score)} | ${formatTokens(row.tokenUsage)} | ${formatDuration(row.durationMs)} |`), - "", - ]; - - const failureLines = summary.rows.flatMap((row) => - row.failures.map((grader) => `- ${escapeCell(row.name)} / ${escapeCell(sanitizeGraderId(grader))}: check failed`)); - const errorLines = summary.errors.map((error) => `- ERROR: ${escapeCell(error)}`); - if (failureLines.length || errorLines.length) { - lines.push(...failureLines, ...errorLines, ""); - } - - if (baselineSummary) { - const comparison = compareSummaries(baselineSummary, summary); - lines.push( - "## Baseline vs. Candidate Comparison", - "", - ...(baselinePath ? [`Baseline: \`${escapeCell(baselinePath)}\` | Candidate: \`${escapeCell(path)}\``, ""] : []), - "| Metric | Baseline | Candidate | Delta |", - "| --- | --- | --- | --- |", - `| Passed | ${baselineSummary.passedCount} | ${summary.passedCount} | ${formatSignedInt(comparison.deltas.passed)} |`, - `| Failed | ${baselineSummary.failedCount + baselineSummary.errorCount} | ${summary.failedCount + summary.errorCount} | ${formatSignedInt(comparison.deltas.failed)} |`, - `| Avg Score | ${formatScore(baselineSummary.averageScore)} | ${formatScore(summary.averageScore)} | ${formatScoreDelta(comparison.deltas.averageScore)} |`, - `| Total Tokens | ${formatTokens(baselineSummary.tokenUsage)} | ${formatTokens(summary.tokenUsage)} | ${formatTokenDelta(comparison.deltas.totalTokens, comparison.deltas.totalTokensPct)} |`, - `| Total Duration | ${formatDuration(baselineSummary.totalDurationMs)} | ${formatDuration(summary.totalDurationMs)} | ${formatDurationDelta(comparison.deltas.totalDurationMs)} |`, - "", - "| Case | Baseline | Candidate | Score Delta | Token Delta | Transition |", - "| --- | --- | --- | --- | --- | --- |", - ...comparison.cases.map((item) => { - const baseCell = item.baseline ? `${item.baseline.status} (${formatScore(item.baseline.score)})` : "—"; - const candCell = item.candidate ? `${item.candidate.status} (${formatScore(item.candidate.score)})` : "—"; - return `| ${escapeCell(item.key)} | ${baseCell} | ${candCell} | ${formatScoreDelta(item.scoreDelta)} | ${formatTokenDelta(item.tokenDelta, item.tokenDeltaPct)} | ${item.transition} |`; - }), - "", - "> **Note:** Single-run score, token, and duration deltas are observed signals for review rather than multi-trial statistical proof.", - "", - ); - } - - return lines.join("\n"); -} - -export async function resolveResultsPath(input, root = process.cwd()) { - if (!input) { - const config = await loadProjectConfig(root); - const base = resolve(config.dir || root, config.paths.results); - const display = relative(root, base).split(sep).join("/") || "."; - if (!existsSync(base)) { - throw new Error(`No saved eval runs under ${display}/. Run evals first, or pass --results .`); - } - const runs = readdirSync(base, { withFileTypes: true }) - .filter((entry) => entry.isDirectory() && existsSync(resolve(base, entry.name, "results.jsonl"))) - .map((entry) => entry.name) - .sort(); - if (!runs.length) { - throw new Error(`No saved eval runs under ${display}/. Run evals first, or pass --results .`); - } - return resolve(base, runs.at(-1), "results.jsonl"); - } - const resolved = resolve(root, input); - if (!existsSync(resolved)) { - throw new Error(`${relative(root, resolved).split(sep).join("/")} doesn't exist.`); - } - if (statSync(resolved).isDirectory()) { - const file = resolve(resolved, "results.jsonl"); - if (!existsSync(file)) { - throw new Error(`${relative(root, file).split(sep).join("/")} doesn't exist.`); - } - return file; - } - return resolved; -} - -export async function loadResults(path, root = process.cwd()) { - const file = await resolveResultsPath(path, root); - const lines = readFileSync(file, "utf8").split(/\r?\n/).filter((line) => line.trim()); - const records = lines.map((line) => JSON.parse(line)); - const displayPath = relative(root, file).split(sep).join("/") || file; - return { file, displayPath, summary: summarizeRun(records) }; -} - -export const usage = `Usage: node .github/scripts/eval-report.mjs [options] - -Reads Vally results.jsonl files and prints a Markdown report with per-case and -aggregate verdicts, scores, token usage (input, output, total), and duration. -When --baseline is supplied, also prints a Baseline vs. Candidate comparison -table highlighting fixed failures, regressions, and token deltas. - -Options: - --results, --candidate, --run-dir - Candidate results.jsonl file or run directory. Defaults to - the newest run under .vally.yaml's paths.results (Vally's - default is vally-results/ when not configured). - --baseline Baseline results.jsonl file or run directory to compare - against the candidate run. - -h, --help Show this help message. - -Examples: - npm run eval:report - npm run eval:compare -- --baseline evals/_shared/fixtures/pr-description/baseline-results.jsonl --candidate evals/_shared/fixtures/pr-description/candidate-results.jsonl`; - -async function main(argv = process.argv.slice(2)) { - let parsed; - try { - parsed = parseArgs({ - args: argv, - allowPositionals: true, - options: { - results: { type: "string" }, - candidate: { type: "string" }, - "run-dir": { type: "string" }, - baseline: { type: "string" }, - help: { type: "boolean", short: "h" }, - }, - }); - } catch (error) { - throw new Error(`${error.message}. Run npm run eval:report -- --help for options.`); - } - - const { values, positionals } = parsed; - if (values.help) { - console.log(usage); - return; - } - if (positionals.length > 2) { - throw new Error("Pass at most two positional paths ([results] or [baseline] [candidate]). Run npm run eval:report -- --help for options."); - } - - const baselineInput = values.baseline ?? (positionals.length === 2 ? positionals[0] : undefined); - const candidateInput = values.candidate ?? values.results ?? values["run-dir"] - ?? (positionals.length === 2 ? positionals[1] : positionals[0]); - const candidate = await loadResults(candidateInput); - const baseline = baselineInput ? await loadResults(baselineInput) : undefined; - const markdown = renderReport(candidate.summary, candidate.displayPath, { - baselineSummary: baseline?.summary, - baselinePath: baseline?.displayPath, - }); - console.log(markdown); - if (candidate.summary.errors.length || baseline?.summary.errors.length) { - process.exitCode = 1; - } -} - -if (process.argv[1] && realpathSync(fileURLToPath(import.meta.url)) === realpathSync(resolve(process.argv[1]))) { - try { - await main(); - } catch (error) { - console.error(error instanceof Error ? error.message : String(error)); - process.exitCode = 1; - } -} diff --git a/.github/scripts/eval-report.test.mjs b/.github/scripts/eval-report.test.mjs deleted file mode 100644 index 303c157..0000000 --- a/.github/scripts/eval-report.test.mjs +++ /dev/null @@ -1,563 +0,0 @@ -import test from "node:test"; -import assert from "node:assert/strict"; -import { spawnSync } from "node:child_process"; -import { randomUUID } from "node:crypto"; -import { copyFileSync, existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { resolve } from "node:path"; -import { fileURLToPath, pathToFileURL } from "node:url"; -import { createDefaultGraderRegistry } from "@microsoft/vally"; -import { parse } from "yaml"; -import { - classifyTransition, - compareSummaries, - extractTrialMetrics, - formatDuration, - formatTokens, - loadResults, - renderReport, - summarizeRun, -} from "./eval-report.mjs"; - -const root = fileURLToPath(new URL("../../", import.meta.url)); -const hasPrDescription = existsSync(resolve(root, "plugins/pr-description")); -const prDescriptionTest = hasPrDescription ? test : test.skip; - -const makeTrial = ({ - runId = "run-1", - evalName = "pr-description", - stimulus = "case-1", - status = "success", - passed = true, - gradeScore = passed ? 1 : 0, - durationMs = 2400, - inputTokens = 1100, - outputTokens = 320, - totalTokens = inputTokens + outputTokens, - details = [], - events = [], -} = {}) => ({ - type: "trial-result", - runId, - evalName, - stimulus, - status, - durationMs, - gradeResult: { passed, score: gradeScore, details }, - trajectory: { - output: "Done.", - events, - metrics: { - wallTimeMs: durationMs, - tokenUsage: { - inputTokens, - outputTokens, - totalTokens, - }, - }, - }, -}); - -const makeFooter = (runId = "run-1", hadExecutionErrors = false) => ({ - type: "run-summary", - runId, - passed: true, - hadExecutionErrors, - evals: [], -}); - -test("eval-report follows native project results configuration and retains explicit paths and defaults", async (t) => { - const directory = resolve(root, ".github/scripts", `.report-path-test-${randomUUID()}`); - mkdirSync(directory); - t.after(() => rmSync(directory, { recursive: true, force: true })); - const configured = resolve(directory, "evals/results/2026-01-01/results.jsonl"); - const legacy = resolve(directory, "vally-results/2026-02-01/results.jsonl"); - for (const [file, stimulus] of [[configured, "configured-results"], [legacy, "native-default"]]) { - mkdirSync(resolve(file, ".."), { recursive: true }); - writeFileSync(file, `${JSON.stringify(makeTrial({ stimulus }))}\n${JSON.stringify(makeFooter())}\n`); - } - const configuration = resolve(directory, ".vally.yaml"); - writeFileSync(configuration, "paths:\n results: evals/results\n"); - assert.equal((await loadResults(undefined, directory)).file, configured); - assert.equal((await loadResults("vally-results/2026-02-01", directory)).file, legacy); - const cli = spawnSync(process.execPath, [resolve(root, ".github/scripts/eval-report.mjs")], { - cwd: directory, encoding: "utf8", timeout: 10000, - }); - assert.ifError(cli.error); - assert.equal(cli.status, 0, cli.stderr); - assert.match(cli.stdout, /configured-results/); - assert.doesNotMatch(cli.stdout, /native-default/); - writeFileSync(configuration, "{}\n"); - assert.equal((await loadResults(undefined, directory)).file, legacy); - writeFileSync(configuration, "paths: [\n"); - await assert.rejects(loadResults(undefined, directory), /parse|YAML|flow sequence/i); -}); - -test("extractTrialMetrics and summarizeRun extract per-trial and aggregate tokens and duration", () => { - const trial1 = makeTrial({ stimulus: "case-a", durationMs: 1500, inputTokens: 800, outputTokens: 200, totalTokens: 1000 }); - const trial2 = makeTrial({ stimulus: "case-b", durationMs: 2500, inputTokens: 1200, outputTokens: 300, totalTokens: 1500, passed: false, gradeScore: 0.5 }); - - assert.deepEqual(extractTrialMetrics(trial1), { - durationMs: 1500, - tokenUsage: { inputTokens: 800, outputTokens: 200, totalTokens: 1000 }, - }); - - const summary = summarizeRun([trial1, trial2, makeFooter()]); - assert.equal(summary.passedCount, 1); - assert.equal(summary.failedCount, 1); - assert.equal(summary.errorCount, 0); - assert.equal(summary.averageScore, 0.75); - assert.equal(summary.totalDurationMs, 4000); - assert.deepEqual(summary.tokenUsage, { - inputTokens: 2000, - outputTokens: 500, - totalTokens: 2500, - }); - assert.equal(formatTokens(summary.tokenUsage), "2,500 (in: 2,000, out: 500)"); - assert.equal(formatDuration(summary.totalDurationMs), "4.0s"); - assert.equal(formatDuration(450), "450ms"); - - const markdown = renderReport(summary, "vally-results/run-1/results.jsonl"); - assert.match(markdown, /\| Passed \| Failed \| Avg Score \| Total Tokens \| Total Duration \|/); - assert.match(markdown, /\| 1 \| 1 \| 75\.0% \| 2,500 \(in: 2,000, out: 500\) \| 4\.0s \|/); - assert.match(markdown, /\| case-a \| PASS \| 100\.0% \| 1,000 \(in: 800, out: 200\) \| 1\.5s \|/); - assert.match(markdown, /\| case-b \| BEHAVIORAL FAILURE \| 50\.0% \| 1,500 \(in: 1,200, out: 300\) \| 2\.5s \|/); -}); - -test("compareSummaries and renderReport surface fixed failures, regressions, and token diet reductions at parity", () => { - const baseline = summarizeRun([ - makeTrial({ - runId: "base", - stimulus: "pr-description-drafts-structured-pr-body", - passed: false, - gradeScore: 0.5, - durationMs: 6840, - inputTokens: 3420, - outputTokens: 980, - totalTokens: 4400, - }), - makeTrial({ - runId: "base", - stimulus: "pr-description-ignores-bug-and-review-question", - passed: true, - gradeScore: 1, - durationMs: 4120, - inputTokens: 2150, - outputTokens: 410, - totalTokens: 2560, - }), - makeFooter("base"), - ]); - - const candidate = summarizeRun([ - makeTrial({ - runId: "cand", - stimulus: "pr-description-drafts-structured-pr-body", - passed: true, - gradeScore: 1, - durationMs: 3910, - inputTokens: 1680, - outputTokens: 310, - totalTokens: 1990, - }), - makeTrial({ - runId: "cand", - stimulus: "pr-description-ignores-bug-and-review-question", - passed: true, - gradeScore: 1, - durationMs: 2450, - inputTokens: 1120, - outputTokens: 190, - totalTokens: 1310, - }), - makeFooter("cand"), - ]); - - const comparison = compareSummaries(baseline, candidate); - assert.equal(comparison.deltas.passed, 1); - assert.equal(comparison.deltas.failed, -1); - assert.equal(comparison.deltas.averageScore, 0.25); - assert.equal(comparison.deltas.totalTokens, -3660); - assert.equal(comparison.cases[0].transition, "Fixed"); - assert.equal(comparison.cases[1].transition, "Reduced tokens at parity"); - - assert.equal( - classifyTransition( - { status: "PASS", score: 1, tokenUsage: { totalTokens: 1000 } }, - { status: "BEHAVIORAL FAILURE", score: 0, tokenUsage: { totalTokens: 900 } }, - ), - "Regressed", - ); - - const markdown = renderReport(candidate, "candidate/results.jsonl", { - baselineSummary: baseline, - baselinePath: "baseline/results.jsonl", - }); - assert.match(markdown, /## Baseline vs\. Candidate Comparison/); - assert.match(markdown, /\| Total Tokens \| 6,960 \(in: 5,570, out: 1,390\) \| 3,300 \(in: 2,800, out: 500\) \| -3,660 \(-52\.6%\) \|/); - assert.match(markdown, /\| pr-description \/ pr-description-drafts-structured-pr-body \| BEHAVIORAL FAILURE \(50\.0%\) \| PASS \(100\.0%\) \| \+50\.0% \| -2,410 \(-54\.8%\) \| Fixed \|/); - assert.match(markdown, /\| pr-description \/ pr-description-ignores-bug-and-review-question \| PASS \(100\.0%\) \| PASS \(100\.0%\) \| 0\.0% \| -1,250 \(-48\.8%\) \| Reduced tokens at parity \|/); - assert.match(markdown, /Single-run score, token, and duration deltas are observed signals for review/); -}); - -prDescriptionTest("eval-report CLI compares the shared pr-description baseline and candidate fixtures without writing GITHUB_STEP_SUMMARY", async (t) => { - const tempDir = resolve(root, ".github/scripts", `.report-test-${randomUUID()}`); - mkdirSync(tempDir, { recursive: true }); - t.after(() => rmSync(tempDir, { recursive: true, force: true })); - const summaryFile = resolve(tempDir, "step-summary.md"); - writeFileSync(summaryFile, ""); - - const baselinePath = "evals/_shared/fixtures/pr-description/baseline-results.jsonl"; - const candidatePath = "evals/_shared/fixtures/pr-description/candidate-results.jsonl"; - const loadedBase = await loadResults(baselinePath, root); - const loadedCand = await loadResults(candidatePath, root); - assert.equal(loadedBase.summary.passedCount, 1); - assert.equal(loadedCand.summary.passedCount, 2); - - const result = spawnSync( - process.execPath, - [ - resolve(root, ".github/scripts/eval-report.mjs"), - "--baseline", - baselinePath, - "--candidate", - candidatePath, - ], - { - cwd: root, - encoding: "utf8", - timeout: 10000, - env: { ...process.env, GITHUB_STEP_SUMMARY: summaryFile }, - }, - ); - assert.ifError(result.error); - assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /## Baseline vs\. Candidate Comparison/); - assert.match(result.stdout, /Fixed/); - assert.match(result.stdout, /Reduced tokens at parity/); - assert.equal(readFileSync(summaryFile, "utf8"), ""); -}); - -test("eval-report redacts raw grader evidence and error strings, requires complete metric coverage, aggregates multi-trial cases, and guards unscored transitions", () => { - const secretEvidence = "SECRET_TOKEN_EVIDENCE_12345"; - const secretError = "RAW_EXECUTION_ERROR_67890"; - const secretSkip = "RAW_SKIP_REASON_ABCDE"; - const secretRuntime = "RAW_RUNTIME_EVENT_FGHIJ"; - - const leakedRecords = [ - makeTrial({ - runId: "leak-run", - stimulus: "failed-case", - passed: false, - gradeScore: 0.5, - details: [{ name: "pr-description-contract", passed: false, score: 0, evidence: secretEvidence }], - }), - { - ...makeTrial({ runId: "leak-run", stimulus: "error-case", status: "error" }), - error: secretError, - }, - { - ...makeTrial({ runId: "leak-run", stimulus: "skip-case", status: "skipped" }), - skipReason: secretSkip, - }, - makeTrial({ - runId: "leak-run", - stimulus: "runtime-error-case", - events: [{ type: "error", data: { message: secretRuntime } }], - }), - makeFooter("leak-run", true), - ]; - - const leakedSummary = summarizeRun(leakedRecords); - const leakedReport = renderReport(leakedSummary, "vally-results/leak-run/results.jsonl"); - for (const secret of [secretEvidence, secretError, secretSkip, secretRuntime]) { - assert.ok(!JSON.stringify(leakedSummary).includes(secret), `summary leaked ${secret}`); - assert.ok(!leakedReport.includes(secret), `report leaked ${secret}`); - } - assert.deepEqual(leakedSummary.rows[0].failures, ["pr-description-contract"]); - assert.deepEqual(leakedSummary.errors, [ - "error-case: execution-error", - "skip-case: skipped", - "runtime-error-case: runtime-error", - "execution-error", - ]); - - // Partial duration or token coverage must yield undefined run totals - const missingSecondMetrics = makeTrial({ runId: "partial-run", stimulus: "case-2" }); - delete missingSecondMetrics.durationMs; - delete missingSecondMetrics.trajectory.metrics; - const partialSummary = summarizeRun([ - makeTrial({ runId: "partial-run", stimulus: "case-1", durationMs: 1200, inputTokens: 500, outputTokens: 100, totalTokens: 600 }), - missingSecondMetrics, - makeFooter("partial-run"), - ]); - assert.equal(partialSummary.totalDurationMs, undefined); - assert.equal(partialSummary.tokenUsage, undefined); - - // Multi-trial aggregation per case and mismatched trial count comparison - const multiBase = summarizeRun([ - makeTrial({ runId: "m-base", stimulus: "repeat-case", passed: true, gradeScore: 1, durationMs: 1000, inputTokens: 400, outputTokens: 100, totalTokens: 500 }), - makeTrial({ runId: "m-base", stimulus: "repeat-case", passed: true, gradeScore: 1, durationMs: 1200, inputTokens: 500, outputTokens: 100, totalTokens: 600 }), - makeFooter("m-base"), - ]); - const multiCand = summarizeRun([ - makeTrial({ - runId: "m-cand", - stimulus: "repeat-case", - passed: false, - gradeScore: 0, - durationMs: 900, - inputTokens: 300, - outputTokens: 100, - totalTokens: 400, - details: [{ name: "contract-check", passed: false, evidence: "ignored" }], - }), - makeTrial({ - runId: "m-cand", - stimulus: "repeat-case", - passed: true, - gradeScore: 1, - durationMs: 900, - inputTokens: 300, - outputTokens: 100, - totalTokens: 400, - details: [{ name: "contract-check", passed: false, evidence: "ignored" }], - }), - makeFooter("m-cand"), - ]); - assert.equal(multiBase.rows.length, 1); - assert.equal(multiBase.rows[0].trials, 2); - assert.equal(multiBase.rows[0].passedCount, 2); - assert.equal(multiBase.rows[0].status, "PASS"); - assert.equal(multiBase.rows[0].score, 1); - assert.equal(multiBase.rows[0].durationMs, 2200); - assert.equal(multiBase.rows[0].tokenUsage.totalTokens, 1100); - - assert.equal(multiCand.rows.length, 1); - assert.equal(multiCand.rows[0].trials, 2); - assert.equal(multiCand.rows[0].passedCount, 1); - assert.equal(multiCand.rows[0].status, "BEHAVIORAL FAILURE"); - assert.equal(multiCand.rows[0].passed, false); - assert.equal(multiCand.rows[0].score, 0.5); - assert.deepEqual(multiCand.rows[0].failures, ["contract-check"]); - - const multiComp = compareSummaries(multiBase, multiCand); - assert.equal(multiComp.cases[0].transition, "Regressed"); - assert.equal(multiComp.cases[0].scoreDelta, -0.5); - assert.equal(multiComp.cases[0].tokenDelta, -300); - - const singleTrialCand = summarizeRun([ - makeTrial({ runId: "s-cand", stimulus: "repeat-case", passed: true, gradeScore: 1, durationMs: 900, inputTokens: 300, outputTokens: 100, totalTokens: 400 }), - makeFooter("s-cand"), - ]); - const mismatchedTrialsComp = compareSummaries(multiBase, singleTrialCand); - assert.equal(mismatchedTrialsComp.cases[0].tokenDelta, undefined); - assert.equal(mismatchedTrialsComp.cases[0].durationDeltaMs, undefined); - assert.equal(mismatchedTrialsComp.cases[0].transition, "Unchanged"); - assert.equal(mismatchedTrialsComp.deltas.totalTokens, undefined); - assert.equal(mismatchedTrialsComp.deltas.totalTokensPct, undefined); - assert.equal(mismatchedTrialsComp.deltas.totalDurationMs, undefined); - const mismatchedTrialsReport = renderReport(singleTrialCand, "cand/results.jsonl", { - baselineSummary: multiBase, - baselinePath: "base/results.jsonl", - }); - assert.match(mismatchedTrialsReport, /\| Total Tokens \| 1,100 \(in: 900, out: 200\) \| 400 \(in: 300, out: 100\) \| — \|/); - assert.match(mismatchedTrialsReport, /\| Total Duration \| 2\.2s \| 900ms \| — \|/); - - const extraCaseBase = summarizeRun([ - makeTrial({ runId: "e-base", stimulus: "repeat-case", passed: true, gradeScore: 1, durationMs: 900, inputTokens: 300, outputTokens: 100, totalTokens: 400 }), - makeTrial({ runId: "e-base", stimulus: "extra-case", passed: true, gradeScore: 1, durationMs: 900, inputTokens: 300, outputTokens: 100, totalTokens: 400 }), - makeFooter("e-base"), - ]); - const mismatchedCasesComp = compareSummaries(extraCaseBase, singleTrialCand); - assert.equal(mismatchedCasesComp.deltas.totalTokens, undefined); - assert.equal(mismatchedCasesComp.deltas.totalTokensPct, undefined); - assert.equal(mismatchedCasesComp.deltas.totalDurationMs, undefined); - - // Unscored / errored transitions - assert.equal( - classifyTransition( - { status: "BEHAVIORAL FAILURE", score: 0.5, tokenUsage: { totalTokens: 2000 } }, - { status: "ERROR / NOT EVALUATED", score: undefined, tokenUsage: { totalTokens: 500 } }, - ), - "Regressed", - ); - assert.equal( - classifyTransition( - { status: "ERROR / NOT EVALUATED", score: undefined, tokenUsage: { totalTokens: 2000 } }, - { status: "ERROR / NOT EVALUATED", score: undefined, tokenUsage: { totalTokens: 500 } }, - ), - "Unchanged", - ); - assert.equal( - classifyTransition( - { status: "ERROR / NOT EVALUATED", score: undefined, tokenUsage: { totalTokens: 2000 } }, - { status: "BEHAVIORAL FAILURE", score: 0.5, tokenUsage: { totalTokens: 500 } }, - ), - "Unchanged", - ); - - // Strip terminal escape sequences (including OSC 52) from report fields and invalid-grade errors - const osc52Stimulus = "case-\u001b]52;c;ZWNobyBpw6g=\u0007\u001b[31mred\u001b[0m"; - const escapedSummary = summarizeRun([ - makeTrial({ runId: "osc-run", stimulus: osc52Stimulus, passed: true, gradeScore: 1 }), - makeFooter("osc-run"), - ]); - const escapedReport = renderReport(escapedSummary, "vally-results/\u001b]52;c;bad\u0007/results.jsonl"); - assert.doesNotMatch(escapedReport, /\u001b|\u0007/); - assert.throws( - () => summarizeRun([ - { - ...makeTrial({ runId: "bad-grade-osc", stimulus: osc52Stimulus }), - gradeResult: { status: "success", passed: true, score: 2 }, - }, - makeFooter("bad-grade-osc"), - ]), - (err) => err instanceof Error && /^Invalid grade: /.test(err.message) && !/[\u001b\u0007]/.test(err.message), - ); -}); - -prDescriptionTest("pr-description eval stages the multi-skill marketplace cohort and enforces non-collision", async () => { - const evalDir = resolve(root, "evals/pr-description/skills/pr-description"); - const spec = parse(readFileSync(resolve(evalDir, "eval.yaml"), "utf8")); - assert.equal(spec.scoring.threshold, 1); - const project = parse(readFileSync(resolve(root, ".vally.yaml"), "utf8")); - assert.equal(spec.agent_environment, "marketplace"); - const skills = project.environments.marketplace.skills; - for (const configuredSkill of skills) { - assert.ok(existsSync(resolve(evalDir, configuredSkill)), `configured cohort skill path is missing: ${configuredSkill}`); - } - for (const expectedSkill of [ - "../../../../plugins/pr-description/skills/pr-description", - "../../../../plugins/plugin-authoring/skills/add-plugin", - "../../../../plugins/plugin-authoring/skills/try-plugin-locally", - "../../../../plugins/plugin-authoring/skills/submit-plugin", - "../../../../plugins/marketplace-maintainer/skills/update-remote-pins", - "../../../../plugins/marketplace-maintainer/skills/release-plugins", - ].filter((skillPath) => existsSync(resolve(evalDir, skillPath)))) { - assert.ok(skills.includes(expectedSkill), `missing cohort skill: ${expectedSkill}`); - } - - const grader = createDefaultGraderRegistry().get("skill-invocation"); - const positive = spec.stimuli.find((item) => item.tags?.control === "positive"); - const negative = spec.stimuli.find((item) => item.tags?.control === "negative"); - const posConfig = positive.graders.find((item) => item.type === "skill-invocation").config; - const negConfig = negative.graders.find((item) => item.type === "skill-invocation").config; - - const exactMatch = await grader.grade({ - stimulus: positive, - config: posConfig, - trajectory: { output: "ok", events: [{ type: "skill_activation", data: { name: "pr-description" } }] }, - }); - assert.equal(exactMatch.passed, true); - - const neighborCollision = await grader.grade({ - stimulus: positive, - config: posConfig, - trajectory: { - output: "ok", - events: [ - { type: "skill_activation", data: { name: "pr-description" } }, - { type: "skill_activation", data: { name: "submit-plugin" } }, - ], - }, - }); - assert.equal(neighborCollision.passed, false, "activating a neighbor skill alongside pr-description must fail"); - - const negativeTriggered = await grader.grade({ - stimulus: negative, - config: negConfig, - trajectory: { output: "ok", events: [{ type: "skill_activation", data: { name: "pr-description" } }] }, - }); - assert.equal(negativeTriggered.passed, false, "activating pr-description on the negative case must fail"); -}); - -prDescriptionTest("pr-description program grader enforces ordered headings, factual changes, and unshipped backlog exclusion", async (t) => { - const gradingDir = resolve(root, ".github/scripts", `.pr-grader-test-${randomUUID()}`); - mkdirSync(gradingDir, { recursive: true }); - t.after(() => rmSync(gradingDir, { recursive: true, force: true })); - copyFileSync(resolve(root, "evals/pr-description/skills/pr-description/grader.mjs"), resolve(gradingDir, "grader.mjs")); - copyFileSync(resolve(root, "evals/_shared/graders/common.mjs"), resolve(gradingDir, "common.mjs")); - const { evaluate: evaluatePrDescription } = await import(pathToFileURL(resolve(gradingDir, "grader.mjs"))); - const { score } = await import(pathToFileURL(resolve(gradingDir, "common.mjs"))); - - const validOutput = [ - "## Summary", - "Validate `--timeout` as a positive integer in `src/cli.mjs` and add unit tests. Retry backoff (#42) remains open and out of scope.", - "", - "## Changes", - "- Added positive integer validation for `--timeout` in `src/cli.mjs`.", - "- Added unit tests for `--timeout 0` and `--timeout -5` in `test/cli.test.mjs`.", - "", - "## Verification", - "- `npm test` passed (14 tests).", - "", - "## Risks / Rollback", - "- Low risk; revert the commit if needed.", - ].join("\n"); - - assert.equal(score("pr-description-contract", evaluatePrDescription({ output: validOutput, events: [] })).passed, true); - - for (const passingLine of [ - "- `npm test` (14 tests): passed.", - "- `npm test` — 14 tests, all passing.", - "- Ran `npm test`; 14/14 tests passed.", - "- `npm test`: 14 tests passed, 0 failed, 0 errors.", - "- `npm test` passed (14 tests, no errors).", - "- `npm test`: 14 passed, # fail 0, 0 failures.", - ]) { - const equivalentOutput = validOutput.replace("- `npm test` passed (14 tests).", passingLine); - assert.equal( - score("pr-description-contract", evaluatePrDescription({ output: equivalentOutput, events: [] })).passed, - true, - `expected passing verification format: ${passingLine}`, - ); - } - - for (const negatedLine of [ - "- `npm test` (14 tests).", - "- `npm test` did not pass (14 tests).", - "- `npm test` (14 tests): not passing.", - "- `npm test`: 14 tests; 12 passed, 2 failures.", - ]) { - const negatedOutput = validOutput.replace("- `npm test` passed (14 tests).", negatedLine); - const negatedResult = score("pr-description-contract", evaluatePrDescription({ output: negatedOutput, events: [] })); - assert.equal(negatedResult.passed, false, `expected negated verification to fail: ${negatedLine}`); - assert.match(negatedResult.evidence, /Recorded the supplied npm test verification result/); - } - - for (const leakedChanges of [ - "- Added unit tests for `--timeout 0` and `--timeout -5` in `test/cli.test.mjs`.\n- Implemented exponential retry backoff (#42).", - "- Rejects `--timeout` values that are not positive integers.\n- Adds exponential retry backoff (#42).", - ]) { - const leaksBacklogIntoChanges = validOutput.replace( - "- Added unit tests for `--timeout 0` and `--timeout -5` in `test/cli.test.mjs`.", - leakedChanges, - ); - const leakedResult = score("pr-description-contract", evaluatePrDescription({ output: leaksBacklogIntoChanges, events: [] })); - assert.equal(leakedResult.passed, false); - assert.match(leakedResult.evidence, /Excluded unshipped retry backoff/); - } - - const missingRisks = validOutput.split("## Risks / Rollback")[0]; - assert.equal(score("pr-description-contract", evaluatePrDescription({ output: missingRisks, events: [] })).passed, false); - - const failedTestsClaim = validOutput.replace( - "- `npm test` passed (14 tests).", - "- `npm test` failed (14 tests).", - ); - const failedTestsResult = score("pr-description-contract", evaluatePrDescription({ output: failedTestsClaim, events: [] })); - assert.equal(failedTestsResult.passed, false); - assert.match(failedTestsResult.evidence, /Recorded the supplied npm test verification result/); - - for (const prohibitedTool of ["bash", "functions.bash", "functions.powershell", "mcp:edit", "tools/write_file"]) { - const toolResult = score( - "pr-description-contract", - evaluatePrDescription({ - output: validOutput, - events: [{ type: "tool_call", data: { toolCallId: "1", toolName: prohibitedTool, arguments: { command: "git status" } } }], - }), - ); - assert.equal(toolResult.passed, false, `must reject prohibited tool ${prohibitedTool}`); - assert.match(toolResult.evidence, /Did not run shell commands or create a pull request/); - } -}); diff --git a/.github/scripts/pr-description.test.mjs b/.github/scripts/pr-description.test.mjs new file mode 100644 index 0000000..7cc96ce --- /dev/null +++ b/.github/scripts/pr-description.test.mjs @@ -0,0 +1,130 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { randomUUID } from "node:crypto"; +import { copyFileSync, existsSync, mkdirSync, readFileSync, readdirSync, rmSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { createDefaultGraderRegistry } from "@microsoft/vally"; +import { parse } from "yaml"; + +const root = fileURLToPath(new URL("../../", import.meta.url)); +const prDescriptionTest = existsSync(resolve(root, "plugins/pr-description")) ? test : test.skip; + +prDescriptionTest("pr-description eval stages catalog-local skills and enforces non-collision", async () => { + const evalDir = resolve(root, "evals/pr-description/skills/pr-description"); + const spec = parse(readFileSync(resolve(evalDir, "eval.yaml"), "utf8")); + assert.equal(spec.scoring.threshold, 1); + const project = parse(readFileSync(resolve(root, ".vally.yaml"), "utf8")); + assert.equal(spec.agent_environment, "marketplace"); + const skills = project.environments.marketplace.skills; + for (const configuredSkill of skills) { + assert.ok(existsSync(resolve(evalDir, configuredSkill)), `configured cohort skill path is missing: ${configuredSkill}`); + } + const staged = new Set(skills.map((skill) => resolve(evalDir, skill))); + const catalog = JSON.parse(readFileSync(resolve(root, "marketplace.json"), "utf8")); + for (const plugin of catalog.plugins.filter((plugin) => typeof plugin.source === "string")) { + const directory = resolve(root, plugin.source, "skills"); + if (!existsSync(directory)) continue; + for (const skill of readdirSync(directory)) { + const source = resolve(directory, skill); + if (existsSync(resolve(source, "SKILL.md"))) { + assert.ok(staged.has(source), `Add catalog skill ${plugin.name}/${skill} to the native marketplace environment.`); + } + } + } + + const grader = createDefaultGraderRegistry().get("skill-invocation"); + const positive = spec.stimuli.find((item) => item.tags?.control === "positive"); + const negative = spec.stimuli.find((item) => item.tags?.control === "negative"); + const posConfig = positive.graders.find((item) => item.type === "skill-invocation").config; + const negConfig = negative.graders.find((item) => item.type === "skill-invocation").config; + const activation = (name) => ({ type: "skill_activation", data: { name } }); + + const exactMatch = await grader.grade({ + stimulus: positive, config: posConfig, + trajectory: { output: "ok", events: [activation("pr-description")] }, + }); + assert.equal(exactMatch.passed, true); + + if (posConfig.disallowed?.length) { + const neighborCollision = await grader.grade({ + stimulus: positive, config: posConfig, + trajectory: { output: "ok", events: [activation("pr-description"), activation(posConfig.disallowed[0])] }, + }); + assert.equal(neighborCollision.passed, false, "activating a configured neighbor must fail"); + } + + const negativeTriggered = await grader.grade({ + stimulus: negative, config: negConfig, + trajectory: { output: "ok", events: [activation("pr-description")] }, + }); + assert.equal(negativeTriggered.passed, false, "activating pr-description on the negative case must fail"); +}); + +prDescriptionTest("pr-description program grader enforces ordered headings, factual changes, and unshipped backlog exclusion", async (t) => { + const gradingDir = resolve(root, ".github/scripts", `.pr-grader-test-${randomUUID()}`); + mkdirSync(gradingDir, { recursive: true }); + t.after(() => rmSync(gradingDir, { recursive: true, force: true })); + copyFileSync(resolve(root, "evals/pr-description/skills/pr-description/grader.mjs"), resolve(gradingDir, "grader.mjs")); + copyFileSync(resolve(root, "evals/_shared/graders/common.mjs"), resolve(gradingDir, "common.mjs")); + const { evaluate: evaluatePrDescription } = await import(pathToFileURL(resolve(gradingDir, "grader.mjs"))); + const { score } = await import(pathToFileURL(resolve(gradingDir, "common.mjs"))); + const validOutput = [ + "## Summary", + "Validate `--timeout` as a positive integer in `src/cli.mjs` and add unit tests. Retry backoff (#42) remains open and out of scope.", + "", + "## Changes", + "- Added positive integer validation for `--timeout` in `src/cli.mjs`.", + "- Added unit tests for `--timeout 0` and `--timeout -5` in `test/cli.test.mjs`.", + "", + "## Verification", + "- `npm test` passed (14 tests).", + "", + "## Risks / Rollback", + "- Low risk; revert the commit if needed.", + ].join("\n"); + assert.equal(score("pr-description-contract", evaluatePrDescription({ output: validOutput, events: [] })).passed, true); + for (const passingLine of [ + "- `npm test` (14 tests): passed.", + "- `npm test` \u2014 14 tests, all passing.", + "- Ran `npm test`; 14/14 tests passed.", + "- `npm test`: 14 tests passed, 0 failed, 0 errors.", + "- `npm test` passed (14 tests, no errors).", + "- `npm test`: 14 passed, # fail 0, 0 failures.", + ]) { + const output = validOutput.replace("- `npm test` passed (14 tests).", passingLine); + assert.equal(score("pr-description-contract", evaluatePrDescription({ output, events: [] })).passed, true, passingLine); + } + for (const negatedLine of [ + "- `npm test` (14 tests).", + "- `npm test` did not pass (14 tests).", + "- `npm test` (14 tests): not passing.", + "- `npm test`: 14 tests; 12 passed, 2 failures.", + "- `npm test` failed (14 tests).", + ]) { + const output = validOutput.replace("- `npm test` passed (14 tests).", negatedLine); + const result = score("pr-description-contract", evaluatePrDescription({ output, events: [] })); + assert.equal(result.passed, false, negatedLine); + assert.match(result.evidence, /Recorded the supplied npm test verification result/); + } + for (const leakedChanges of [ + "- Added unit tests for `--timeout 0` and `--timeout -5` in `test/cli.test.mjs`.\n- Implemented exponential retry backoff (#42).", + "- Rejects `--timeout` values that are not positive integers.\n- Adds exponential retry backoff (#42).", + ]) { + const output = validOutput.replace( + "- Added unit tests for `--timeout 0` and `--timeout -5` in `test/cli.test.mjs`.", leakedChanges); + const result = score("pr-description-contract", evaluatePrDescription({ output, events: [] })); + assert.equal(result.passed, false); + assert.match(result.evidence, /Excluded unshipped retry backoff/); + } + const missingRisks = validOutput.split("## Risks / Rollback")[0]; + assert.equal(score("pr-description-contract", evaluatePrDescription({ output: missingRisks, events: [] })).passed, false); + for (const prohibitedTool of ["bash", "functions.bash", "functions.powershell", "mcp:edit", "tools/write_file"]) { + const result = score("pr-description-contract", evaluatePrDescription({ + output: validOutput, + events: [{ type: "tool_call", data: { toolCallId: "1", toolName: prohibitedTool, arguments: { command: "git status" } } }], + })); + assert.equal(result.passed, false, `must reject prohibited tool ${prohibitedTool}`); + assert.match(result.evidence, /Did not run shell commands or create a pull request/); + } +}); diff --git a/.github/scripts/select-skill-evals.mjs b/.github/scripts/select-skill-evals.mjs new file mode 100644 index 0000000..5817d96 --- /dev/null +++ b/.github/scripts/select-skill-evals.mjs @@ -0,0 +1,108 @@ +import { execFileSync, spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { appendFileSync, existsSync, readFileSync, readdirSync, realpathSync, writeFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { githubApi } from "./check-remote-pins.mjs"; + +const name = /^[a-z0-9][a-z0-9._-]*$/; +const sha = /^[a-f0-9]{40}$/; +const parse = (contents) => { + try { return JSON.parse(contents); } catch { throw new Error("Invalid skill selection JSON."); } +}; +const json = (path) => parse(readFileSync(path, "utf8")); + +function pinnedTree(repo, commit) { + const response = githubApi(`repos/${repo}/git/trees/${commit}?recursive=1`, (command, args, options) => + spawnSync(command, args, { ...options, timeout: 30000, maxBuffer: 8 * 1024 * 1024 })); + if (response.status !== 200) throw new Error(`Pinned skill inventory returned GitHub HTTP ${response.status}.`); + return parse(response.body); +} + +export function inventorySkills(root, readTree = pinnedTree) { + const skills = []; + for (const plugin of json(resolve(root, "marketplace.json")).plugins) { + if (!name.test(plugin.name)) throw new Error("Unsupported catalog plugin name."); + let names; + if (typeof plugin.source === "string") { + if (plugin.source !== `./plugins/${plugin.name}`) throw new Error("Expected a self-contained local plugin path."); + const directory = resolve(root, plugin.source, "skills"); + names = existsSync(directory) ? readdirSync(directory).filter((skill) => + name.test(skill) && existsSync(resolve(directory, skill, "SKILL.md"))) : []; + } else { + const source = plugin.source; + if (source?.source !== "github" || !/^[\w.-]+\/[\w.-]+$/.test(source.repo) || !sha.test(source.sha)) { + throw new Error("Remote skill inventory requires a pinned GitHub source."); + } + const prefix = source.path ? `${source.path}/` : ""; + if (prefix.split("/").includes("..")) throw new Error("Unsupported remote plugin path."); + const tree = readTree(source.repo, source.sha); + if (tree.truncated || !Array.isArray(tree.tree)) throw new Error("Incomplete pinned remote skill inventory."); + names = tree.tree.flatMap((entry) => { + if (entry.type !== "blob" || !entry.path.startsWith(`${prefix}skills/`)) return []; + const parts = entry.path.slice(`${prefix}skills/`.length).split("/"); + return parts.length === 2 && parts[1] === "SKILL.md" && name.test(parts[0]) ? [parts[0]] : []; + }); + } + for (const skill of names) skills.push({ + key: `${plugin.name}/${skill}`, + hasEval: existsSync(resolve(root, "evals", plugin.name, "skills", skill, "eval.yaml")), + }); + } + return skills.sort((a, b) => a.key.localeCompare(b.key)); +} + +export function selectSkillEvals(root, context, environment, readTree = pinnedTree) { + if (!environment.protection_rules?.some((rule) => rule.type === "required_reviewers" && rule.reviewers?.length)) { + throw new Error("Configure the evals environment with required maintainer reviewers before enabling paid PR evals."); + } + const git = (...args) => execFileSync("git", args, { cwd: root, encoding: "utf8" }).trim(); + if (![context.base, context.head, context.merge].every((value) => sha.test(value ?? "")) || + git("rev-parse", "HEAD") !== context.merge || git("rev-parse", "HEAD^1") !== context.base || + git("rev-parse", "HEAD^2") !== context.head) { + throw new Error("Expected the verified PR merge and its exact parents."); + } + const changed = execFileSync("git", ["diff", "--no-renames", "--name-only", "-z", context.base, "HEAD", "--", "plugins"], + { cwd: root, encoding: "utf8" }); + const keys = [...new Set(changed.split("\0").flatMap((file) => { + const match = /^plugins\/([^/]+)\/skills\/([^/]+)\//.exec(file); + return match ? [`${match[1]}/${match[2]}`] : []; + }))].sort(); + const include = []; + for (const key of keys) { + if (key.split("/").length !== 2 || !key.split("/").every((part) => name.test(part))) { + throw new Error("Unsupported plugin or skill path in PR changes."); + } + const [plugin, skill] = key.split("/"); + const directory = `plugins/${plugin}/skills/${skill}`; + const evalPath = `evals/${plugin}/skills/${skill}/eval.yaml`; + const present = (ref) => { + const result = spawnSync("git", ["cat-file", "-e", `${ref}:${directory}/SKILL.md`], { cwd: root, stdio: "ignore" }); + if (result.error) throw result.error; + if (result.status !== 0 && result.status !== 128) throw new Error("Could not inspect the skill source tree."); + return result.status === 0; + }; + if (present(context.base) && present(context.head) && existsSync(resolve(root, evalPath))) { + include.push({ key, id: `skill-${createHash("sha256").update(key).digest("hex").slice(0, 12)}`, + directory, evalPath, base: context.base, head: context.head }); + } + } + return { ...context, include, changed: keys, skills: inventorySkills(root, readTree) }; +} + +if (process.argv[1] && realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url))) { + try { + const [contextFile, environmentFile, outputFile] = process.argv.slice(2); + if (!contextFile || !environmentFile || !outputFile) throw new Error("Expected context, environment and selection file paths."); + const selection = selectSkillEvals(process.cwd(), json(contextFile), json(environmentFile)); + for (const key of selection.changed.filter((key) => !selection.include.some((entry) => entry.key === key))) { + console.log(`::warning::No before/after pair for ${key}: both skill versions and its repo eval are required. No eval was run.`); + } + writeFileSync(outputFile, `${JSON.stringify(selection)}\n`); + appendFileSync(process.env.GITHUB_OUTPUT, + `matrix=${JSON.stringify({ include: selection.include })}\ncount=${selection.include.length}\nattempt=${selection.attempt}\nselection=${JSON.stringify(selection)}\n`); + } catch (error) { + console.error(error.message); + process.exitCode = 1; + } +} diff --git a/.github/scripts/select-skill-evals.test.mjs b/.github/scripts/select-skill-evals.test.mjs new file mode 100644 index 0000000..dd21ffa --- /dev/null +++ b/.github/scripts/select-skill-evals.test.mjs @@ -0,0 +1,149 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { copyFileSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { parse } from "yaml"; +import { inventorySkills, selectSkillEvals } from "./select-skill-evals.mjs"; + +test("marketplace inventory reads local skills and exact pinned remote trees without installing plugins", (t) => { + const root = mkdtempSync(join(tmpdir(), "skill-inventory-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + mkdirSync(join(root, "plugins/local/skills/draft"), { recursive: true }); + writeFileSync(join(root, "plugins/local/skills/draft/SKILL.md"), "synthetic"); + mkdirSync(join(root, "evals/local/skills/draft"), { recursive: true }); + writeFileSync(join(root, "evals/local/skills/draft/eval.yaml"), "synthetic"); + const local = { name: "local", source: "./plugins/local" }; + const remote = { name: "remote", source: { source: "github", repo: "example/plugin", sha: "c".repeat(40), path: "package" } }; + const catalog = (plugins) => writeFileSync(join(root, "marketplace.json"), JSON.stringify({ plugins })); + catalog([local, remote]); + const readTree = (repo, commit) => { + assert.equal(repo, "example/plugin"); + assert.equal(commit, "c".repeat(40)); + return { truncated: false, tree: [ + { type: "blob", path: "package/skills/run/SKILL.md" }, + { type: "blob", path: "other/skills/unrelated/SKILL.md" }, + ] }; + }; + assert.deepEqual(inventorySkills(root, readTree), [ + { key: "local/draft", hasEval: true }, { key: "remote/run", hasEval: false }, + ]); + assert.throws(() => inventorySkills(root, () => ({ truncated: true, tree: [] })), /Incomplete/); + mkdirSync(join(root, "plugins/custom/skills/triage"), { recursive: true }); + writeFileSync(join(root, "plugins/custom/skills/triage/SKILL.md"), "synthetic customization"); + catalog([local, { name: "custom", source: "./plugins/custom" }, remote]); + assert.deepEqual(inventorySkills(root, readTree).map((skill) => skill.key), + ["custom/triage", "local/draft", "remote/run"]); + mkdirSync(join(root, "evals/custom/skills/triage"), { recursive: true }); + const customEval = join(root, "evals/custom/skills/triage/eval.yaml"); + writeFileSync(customEval, "synthetic"); + assert.equal(inventorySkills(root, readTree).find((skill) => skill.key === "custom/triage").hasEval, true); + rmSync(customEval); + assert.equal(inventorySkills(root, readTree).find((skill) => skill.key === "custom/triage").hasEval, false); + catalog([local]); + assert.deepEqual(inventorySkills(root, readTree), [{ key: "local/draft", hasEval: true }]); + rmSync(join(root, "plugins"), { recursive: true }); + catalog([remote]); + assert.deepEqual(inventorySkills(root, readTree), [{ key: "remote/run", hasEval: false }]); +}); + +test("selection requires approval protection and existing paired skills with repo evals at the verified merge", (t) => { + const directory = mkdtempSync(join(tmpdir(), "skill selection-")); + t.after(() => rmSync(directory, { recursive: true, force: true })); + const root = resolve(directory, "repository"), hooks = resolve(directory, "hooks"); + mkdirSync(root); + mkdirSync(hooks); + const config = resolve(directory, "gitconfig"); + writeFileSync(config, ""); + const env = { ...process.env, GIT_CONFIG_GLOBAL: config, GIT_CONFIG_NOSYSTEM: "1" }; + const git = (...args) => { + const result = spawnSync("git", [ + "-c", "user.name=Fixture", "-c", "user.email=fixture@example.test", + "-c", "commit.gpgsign=false", "-c", `core.hooksPath=${hooks}`, ...args, + ], { cwd: root, env, encoding: "utf8", timeout: 10000 }); + assert.ifError(result.error); + assert.equal(result.status, 0, result.stderr); + return result.stdout.trim(); + }; + const file = (path, content) => { + mkdirSync(resolve(root, path, ".."), { recursive: true }); + writeFileSync(resolve(root, path), content); + }; + const target = "plugins/example/skills/changed"; + git("init", "--initial-branch=main"); + file(`${target}/SKILL.md`, "base"); + file(`${target}/r\u00e9sum\u00e9.md`, "base non-ASCII path"); + file("plugins/example/skills/neighbor/SKILL.md", "base neighbor"); + file("plugins/example/skills/deleted/SKILL.md", "deleted"); + file("plugins/custom/skills/triage/SKILL.md", "base custom skill"); + file("plugins/custom/skills/no-eval/SKILL.md", "base skill with eval"); + file("evals/custom/skills/no-eval/eval.yaml", "removed measurement"); + file("evals/example/skills/changed/eval.yaml", "fixed measurement"); + file("marketplace.json", JSON.stringify({ plugins: [ + { name: "example", source: "./plugins/example" }, { name: "custom", source: "./plugins/custom" }, + ] })); + file(".github/scripts/select-skill-evals.mjs", "throw new Error('Candidate selector must not run');"); + git("add", "."); git("commit", "-m", "Base fixture"); + git("checkout", "-b", "candidate"); + file(`${target}/SKILL.md`, "head"); + file(`${target}/r\u00e9sum\u00e9.md`, "head non-ASCII path"); + file("plugins/example/skills/neighbor/SKILL.md", "head neighbor"); + file("plugins/example/skills/added/SKILL.md", "added"); + file("plugins/custom/skills/triage/SKILL.md", "head custom skill"); + file("evals/custom/skills/triage/eval.yaml", "new custom measurement"); + file("plugins/custom/skills/no-eval/SKILL.md", "head skill without eval"); + rmSync(resolve(root, "evals/custom/skills/no-eval/eval.yaml")); + rmSync(resolve(root, "plugins/example/skills/deleted"), { recursive: true }); + git("add", "."); git("commit", "-m", "Candidate fixture"); + const head = git("rev-parse", "HEAD"); + git("checkout", "main"); + file(`${target}/main.md`, "current-main-only file"); + git("add", "."); git("commit", "-m", "Advance main fixture"); + const base = git("rev-parse", "HEAD"); + git("merge", "--no-ff", "candidate", "-m", "Merge fixture"); + const context = { + base, head, merge: git("rev-parse", "HEAD"), trusted: "d".repeat(40), + runHead: head, + repository: "fixture/repo", run: "2", attempt: "1", sourceRun: "1", sourceAttempt: "1", + sourceWorkflow: ".github/workflows/skill-evals.yml", + }; + const protection = { protection_rules: [{ type: "required_reviewers", reviewers: [{ type: "Team" }] }] }; + assert.throws(() => selectSkillEvals(root, context, {}), /required maintainer reviewers/); + assert.throws(() => selectSkillEvals(root, { ...context, head: "f".repeat(40) }, protection), /exact parents/); + const selection = selectSkillEvals(root, context, protection); + assert.deepEqual(selection.changed, + ["custom/no-eval", "custom/triage", "example/added", "example/changed", "example/deleted", "example/neighbor"]); + assert.deepEqual(selection.include.map((entry) => entry.key), ["custom/triage", "example/changed"]); + const selected = selection.include.find((entry) => entry.key === "example/changed"); + assert.deepEqual(selected, { + key: "example/changed", id: selected.id, directory: target, + evalPath: "evals/example/skills/changed/eval.yaml", base, head, + }); + assert.match(selection.include[0].id, /^skill-[a-f0-9]{12}$/); + assert.equal(readFileSync(resolve(root, `${target}/SKILL.md`), "utf8"), "head", "selection must not switch source files"); + const contextFile = resolve(directory, "context.json"), environmentFile = resolve(directory, "evals-environment.json"); + const selectionFile = resolve(directory, "selection.json"), outputFile = resolve(directory, "outputs"); + writeFileSync(contextFile, JSON.stringify(context)); + writeFileSync(environmentFile, JSON.stringify(protection)); + const trusted = resolve(directory, "trusted/.github/scripts"); + mkdirSync(trusted, { recursive: true }); + for (const script of ["select-skill-evals.mjs", "check-remote-pins.mjs"]) { + copyFileSync(new URL(`./${script}`, import.meta.url), resolve(trusted, script)); + } + const workflow = parse(readFileSync(new URL("../workflows/skill-eval-comment.yml", import.meta.url), "utf8")); + const command = workflow.jobs.select.steps.find((step) => step.id === "select").run; + const result = spawnSync("bash", ["-c", command], { + cwd: root, encoding: "utf8", timeout: 10000, + env: { ...env, GITHUB_OUTPUT: outputFile, CONTEXT_FILE: contextFile, + GITHUB_WORKSPACE: directory.replaceAll("\\", "/"), RUNNER_TEMP: directory.replaceAll("\\", "/") }, + }); + assert.ifError(result.error); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /No before\/after pair for example\/added/); + assert.match(result.stdout, /No before\/after pair for example\/deleted/); + assert.match(result.stdout, /No before\/after pair for example\/neighbor/); + assert.match(result.stdout, /No before\/after pair for custom\/no-eval/); + assert.deepEqual(JSON.parse(readFileSync(selectionFile, "utf8")), selection); + assert.match(readFileSync(outputFile, "utf8"), /\ncount=2\nattempt=1\n/); +}); diff --git a/.github/scripts/validation.test.mjs b/.github/scripts/validation.test.mjs index 6063235..c08cab4 100644 --- a/.github/scripts/validation.test.mjs +++ b/.github/scripts/validation.test.mjs @@ -2,7 +2,8 @@ import test from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; -import { copyFileSync, existsSync, mkdirSync, readFileSync, readdirSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { parse } from "yaml"; @@ -361,7 +362,7 @@ test("schema validation skips absent local manifests without downloading or inst } }); -test("validation calls CI scripts, and paid eval activation is manual-only", () => { +test("validation stays model-free and the smoke workflow remains manual-only", () => { const validate = readFileSync(resolve(root, ".github/workflows/validate.yml"), "utf8"); assert.doesNotMatch(validate, /node -e|run: \|/); for (const script of ["validate-schema.sh", "check-local-catalog.mjs", "check-remote-pins.mjs"]) { @@ -370,7 +371,7 @@ test("validation calls CI scripts, and paid eval activation is manual-only", () assert.match(validate, /node --test \.github\/scripts\/\*\.test\.mjs/); const evals = readFileSync(resolve(root, ".github/workflows/evals.yml"), "utf8"); assert.match(evals, /^on:\n workflow_dispatch:\npermissions:/m); - assert.doesNotMatch(evals, /pull_request:|ENABLE_EVALS|^\s+if:|eval:report/m); + assert.doesNotMatch(evals, /pull_request:|ENABLE_EVALS|^\s+if:|eval:(?:serve|compare)/m); assert.match(evals, /^\s+environment:\s*evals$/m); assert.match(evals, /copilot-requests: write/); assert.match(evals, /npm run eval -- --suite smoke --require-pass/); @@ -387,11 +388,116 @@ test("validation calls CI scripts, and paid eval activation is manual-only", () lint: "node .github/scripts/lint.mjs", eval: "vally eval --workers 1 --max-retries 1", "eval:models": "vally experiment run evals/standard-models.experiment.yaml --output-dir evals/results/standard-models --require-pass", - "eval:report": "node .github/scripts/eval-report.mjs", - "eval:compare": "node .github/scripts/eval-report.mjs", + "eval:serve": "vally serve evals/results", + "eval:compare": "vally compare", }); }); + test("PR comparisons keep repository approval and permissions around three pinned Action calls", () => { + const workflow = parse(readFileSync(resolve(root, ".github/workflows/skill-evals.yml"), "utf8")); + assert.deepEqual(Object.keys(workflow.on), ["pull_request"]); + assert.deepEqual(workflow.on.pull_request.paths, ["plugins/*/skills/**"]); + assert.equal(workflow.jobs.request.if, "github.event.pull_request.head.repo.full_name == github.repository"); + assert.deepEqual(workflow.permissions, { contents: "read" }); + assert.deepEqual(workflow.concurrency, { + group: "skill-evals-${{ github.event.pull_request.number }}", "cancel-in-progress": true, + }); + assert.deepEqual(Object.keys(workflow.jobs), ["request"]); + assert.doesNotMatch(JSON.stringify(workflow), /copilot-requests|pull-requests|evals-action/); + assert.equal(workflow.jobs.request.environment, undefined); + const publisher = parse(readFileSync(resolve(root, ".github/workflows/skill-eval-comment.yml"), "utf8")); + assert.deepEqual(publisher.on, { workflow_run: { workflows: ["Skill evals (before and after)"], types: ["completed"] } }); + assert.deepEqual(publisher.permissions, {}); + assert.equal(publisher.concurrency, undefined, "unverified source events must not cancel privileged work"); + assert.deepEqual(publisher.jobs.select.permissions, { contents: "read", actions: "read", "pull-requests": "read" }); + assert.match(publisher.jobs.select.if, /conclusion != 'cancelled'/); + assert.match(publisher.jobs.select.if, /head_repository\.full_name == github\.repository/); + const calls = Object.values(publisher.jobs).flatMap((job) => job.steps) + .filter((step) => step.uses?.startsWith("github/evals-action/")); + assert.deepEqual(calls.map((step) => step.with.command), ["context", "run", "report"]); + for (const step of calls) { + assert.equal(step.uses, "github/evals-action/comparison@3c8cdb79a1e55b149950627a21e8723817d0e772"); + assert.equal(step.with["source-workflow"], ".github/workflows/skill-evals.yml"); + } + const context = calls[0]; + assert.equal(context.id, "context"); + const select = publisher.jobs.select; + assert.equal(select.outputs.context, "${{ steps.context.outputs.context }}"); + assert.equal(select.outputs.selection, "${{ steps.select.outputs.selection }}"); + const trusted = select.steps.find((step) => step.with?.path === "trusted"); + assert.equal(trusted.with.ref, "${{ github.sha }}"); + assert.equal(trusted.with["persist-credentials"], false); + const candidate = select.steps.find((step) => step.with?.path === "candidate"); + assert.equal(candidate.with.ref, "${{ steps.context.outputs.merge }}"); + assert.equal(candidate.with["fetch-depth"], 2); + assert.equal(candidate.with["persist-credentials"], false); + const selection = select.steps.find((step) => step.id === "select"); + assert.equal(selection["working-directory"], "candidate"); + assert.equal(selection.env.CONTEXT_FILE, "${{ steps.context.outputs.context-path }}"); + assert.match(selection.run, /node "\$GITHUB_WORKSPACE\/trusted\/\.github\/scripts\/select-skill-evals\.mjs"/); + assert.match(selection.run, /"\$CONTEXT_FILE" "\$RUNNER_TEMP\/evals-environment\.json" "\$RUNNER_TEMP\/selection\.json"/); + const job = publisher.jobs.evals; + assert.match(job.if, /needs\.select\.result == 'success'/); + assert.equal(job.concurrency["cancel-in-progress"], true); + assert.equal(job.concurrency.group, + "skill-eval-run-${{ github.repository }}-${{ fromJSON(needs.select.outputs.context).pullNumber }}-${{ fromJSON(needs.select.outputs.context).head }}-${{ matrix.id }}"); + assert.equal(job.environment, "evals"); + assert.equal(job.strategy["fail-fast"], false); + assert.equal(job.name, "${{ matrix.key }}"); + assert.equal(job.strategy.matrix, "${{ fromJSON(needs.select.outputs.matrix) }}"); + assert.deepEqual(job.permissions, { contents: "read", actions: "read", "copilot-requests": "write", "pull-requests": "read" }); + const run = calls[1]; + assert.equal(run.id, "comparison"); + assert.equal(run.with["working-directory"], "candidate"); + assert.equal(run.with["vally-version"], "0.21.0"); + assert.equal(run.with["comparison-id"], "${{ matrix.id }}"); + assert.equal(run.with["copilot-token"], "${{ github.token }}"); + assert.equal(run["continue-on-error"], undefined, "native failures must keep the paid job failed"); + for (const step of [run, calls[2]]) { + assert.equal(step.with.context, "${{ needs.select.outputs.context }}"); + assert.equal(step.with.selection, "${{ needs.select.outputs.selection }}"); + assert.equal(step.with["producing-attempt"], "${{ needs.select.outputs.attempt }}"); + } + assert.equal(workflow.jobs.comment, undefined, "PR-controlled jobs must not publish with write permission"); + for (const step of Object.values(workflow.jobs)) assert.equal(step.permissions?.["pull-requests"], undefined); + const comment = publisher.jobs.publish; + assert.equal(comment.concurrency["cancel-in-progress"], false); + assert.equal(comment.concurrency.group, + "skill-eval-comment-${{ github.repository }}-${{ fromJSON(needs.select.outputs.context).pullNumber }}"); + assert.deepEqual(comment.permissions, { contents: "read", actions: "read", "pull-requests": "write" }); + assert.equal(comment.permissions["copilot-requests"], undefined); + assert.deepEqual(comment.needs, ["select", "evals"]); + assert.match(comment.if, /!cancelled\(\)/); + assert.match(comment.if, /needs\.select\.result == 'success'/); + assert.equal(calls[2].with["artifact-directory"], "${{ runner.temp }}/skill-comparison"); + assert.equal(calls[2].with["copilot-token"], undefined); + assert.equal(context.with["copilot-token"], undefined); + const download = comment.steps.find((step) => step.uses?.startsWith("actions/download-artifact@")).with; + assert.equal(download.pattern, "skill-comparison-${{ needs.select.outputs.attempt }}-*"); + assert.equal(download["run-id"], undefined, "publication reads this trusted paid run, not request-run artifacts"); + assert.equal(download.path, "${{ runner.temp }}/skill-comparison"); + for (const step of [...publisher.jobs.select.steps, ...job.steps].filter((step) => step.uses?.startsWith("actions/upload-artifact@"))) { + assert.match(step.with.name, /github\.run_attempt/, "reruns must not collide with or import earlier-attempt artifacts"); + } + const evidence = job.steps.find((step) => step.uses?.startsWith("actions/upload-artifact@")); + assert.deepEqual(evidence.with.path.trim().split("\n"), [ + "${{ steps.comparison.outputs.before-path }}", + "${{ steps.comparison.outputs.after-path }}", + ], "paid jobs may upload only the exact expected verdict files, not extra candidate-created JSON"); + assert.equal(evidence.if, "${{ !cancelled() }}"); + assert.equal(evidence.with["if-no-files-found"], "error"); + assert.equal(evidence.with["retention-days"], 7); + assert.equal(job.permissions["pull-requests"], "read"); + assert.ok(comment.steps.every((step) => !step.run && !step.uses?.startsWith("actions/checkout@")), + "the fresh publisher executes only packaged Actions, never a candidate checkout or local script"); + const checkout = job.steps.filter((step) => step.uses?.startsWith("actions/checkout@")); + assert.equal(checkout.length, 1); + assert.equal(checkout[0].with.path, "candidate"); + assert.equal(checkout[0].with["persist-credentials"], false); + assert.equal(checkout[0].with["fetch-depth"], 2); + assert.equal(checkout[0].with.ref, "${{ fromJSON(needs.select.outputs.context).merge }}"); + }); + for (const [plugin, skillName, readInstruction] of [ ["marketplace-maintainer", "update-remote-pins", "Read its `AGENTS.md` and `marketplace.json`, and this skill's [remote pin rules](references/remote-pins.md)."], ["plugin-authoring", "submit-plugin", "Read its `AGENTS.md` and `CONTRIBUTING.md`."], diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index ed3cae3..d62ee5d 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -1,6 +1,7 @@ name: Evals # Runs the smoke evals with real Copilot sessions, which cost AI credits. -# Start by explicit manual dispatch only; pull requests never start paid runs. +# This smoke workflow starts only by explicit manual dispatch. +# Skill comparisons use the separate approval-gated skill-evals workflow. on: workflow_dispatch: permissions: diff --git a/.github/workflows/skill-eval-comment.yml b/.github/workflows/skill-eval-comment.yml new file mode 100644 index 0000000..ee24d12 --- /dev/null +++ b/.github/workflows/skill-eval-comment.yml @@ -0,0 +1,137 @@ +name: Run and publish skill eval comparison +on: + workflow_run: + workflows: ["Skill evals (before and after)"] + types: [completed] +permissions: {} +jobs: + select: + if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion != 'cancelled' && github.event.workflow_run.head_repository.full_name == github.repository + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + actions: read + pull-requests: read + outputs: + matrix: ${{ steps.select.outputs.matrix }} + count: ${{ steps.select.outputs.count }} + attempt: ${{ steps.select.outputs.attempt }} + context: ${{ steps.context.outputs.context }} + selection: ${{ steps.select.outputs.selection }} + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + ref: ${{ github.sha }} + path: trusted + persist-credentials: false + - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 + with: + node-version: 24 + - name: Authenticate the source request and select the current PR merge revision + id: context + uses: github/evals-action/comparison@3c8cdb79a1e55b149950627a21e8723817d0e772 + with: + command: context + source-workflow: .github/workflows/skill-evals.yml + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + ref: ${{ steps.context.outputs.merge }} + path: candidate + fetch-depth: 2 + persist-credentials: false + - name: Read approval protection without creating an environment + env: + GH_TOKEN: ${{ github.token }} + run: gh api "repos/$GITHUB_REPOSITORY/environments/evals" > "$RUNNER_TEMP/evals-environment.json" + - name: Require approval protection and select changed skills with trusted code + id: select + working-directory: candidate + env: + GH_TOKEN: ${{ github.token }} + CONTEXT_FILE: ${{ steps.context.outputs.context-path }} + run: | + node "$GITHUB_WORKSPACE/trusted/.github/scripts/select-skill-evals.mjs" \ + "$CONTEXT_FILE" "$RUNNER_TEMP/evals-environment.json" "$RUNNER_TEMP/selection.json" + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: skill-comparison-${{ github.run_attempt }}-selection + path: ${{ runner.temp }}/selection.json + if-no-files-found: error + retention-days: 7 + + evals: + needs: select + if: needs.select.result == 'success' && needs.select.outputs.count != '0' + name: ${{ matrix.key }} + runs-on: ubuntu-latest + timeout-minutes: 60 + environment: evals + permissions: + contents: read + actions: read + copilot-requests: write + pull-requests: read + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.select.outputs.matrix) }} + concurrency: + group: skill-eval-run-${{ github.repository }}-${{ fromJSON(needs.select.outputs.context).pullNumber }}-${{ fromJSON(needs.select.outputs.context).head }}-${{ matrix.id }} + cancel-in-progress: true + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + ref: ${{ fromJSON(needs.select.outputs.context).merge }} + path: candidate + fetch-depth: 2 + persist-credentials: false + - name: Compare selected skill with native Vally + id: comparison + uses: github/evals-action/comparison@3c8cdb79a1e55b149950627a21e8723817d0e772 + with: + command: run + source-workflow: .github/workflows/skill-evals.yml + context: ${{ needs.select.outputs.context }} + selection: ${{ needs.select.outputs.selection }} + producing-attempt: ${{ needs.select.outputs.attempt }} + comparison-id: ${{ matrix.id }} + working-directory: candidate + vally-version: '0.21.0' + copilot-token: ${{ github.token }} + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + if: ${{ !cancelled() }} + with: + name: skill-comparison-${{ github.run_attempt }}-${{ matrix.id }} + path: | + ${{ steps.comparison.outputs.before-path }} + ${{ steps.comparison.outputs.after-path }} + if-no-files-found: error + retention-days: 7 + + publish: + needs: [select, evals] + if: ${{ !cancelled() && needs.select.result == 'success' }} + runs-on: ubuntu-latest + timeout-minutes: 5 + concurrency: + group: skill-eval-comment-${{ github.repository }}-${{ fromJSON(needs.select.outputs.context).pullNumber }} + cancel-in-progress: false + permissions: + contents: read + actions: read + pull-requests: write + steps: + - uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5 + with: + pattern: skill-comparison-${{ needs.select.outputs.attempt }}-* + merge-multiple: true + path: ${{ runner.temp }}/skill-comparison + - name: Validate source provenance and publish with trusted code + uses: github/evals-action/comparison@3c8cdb79a1e55b149950627a21e8723817d0e772 + with: + command: report + source-workflow: .github/workflows/skill-evals.yml + context: ${{ needs.select.outputs.context }} + selection: ${{ needs.select.outputs.selection }} + producing-attempt: ${{ needs.select.outputs.attempt }} + artifact-directory: ${{ runner.temp }}/skill-comparison diff --git a/.github/workflows/skill-evals.yml b/.github/workflows/skill-evals.yml new file mode 100644 index 0000000..1355e67 --- /dev/null +++ b/.github/workflows/skill-evals.yml @@ -0,0 +1,18 @@ +name: Skill evals (before and after) +on: + pull_request: + paths: + - plugins/*/skills/** +permissions: + contents: read +concurrency: + group: skill-evals-${{ github.event.pull_request.number }} + cancel-in-progress: true +jobs: + request: + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Request comparison in the trusted workflow + run: echo "The default-branch workflow_run evaluates eligible skills after protected-environment approval." diff --git a/README.md b/README.md index 323183f..4371ee9 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,8 @@ Then add the plugin's skills, agents, or other parts to its folder. The lists what a plugin can contain. Put any evals for it in `evals//`. Eval files that several plugins share live in `evals/_shared/`, which can't clash with a plugin because plugin names start with a lowercase letter or digit. +For shared eval context and PR comparison layout, follow +[customizing evals and marketplace context](docs/checks-and-evals.md#customizing-evals-and-marketplace-context). For changes since a known released base, bump the plugin's version in `plugin.json` and `marketplace.json` together and add a `CHANGELOG.md` entry. @@ -178,8 +180,9 @@ The `release-plugins` skill can retire a plugin for you. By hand, delete `plugins//` and `evals//`, then remove the plugin's entry from `marketplace.json` and its row and install command from this README. If it's one of the two plugins in `.github/copilot/settings.json`, remove its key there -too. If surviving cohort evals under `evals/` list the retired plugin's skills -in `agent_environment.skills` or `disallowed`, remove those entries as well. +too. Remove its skill paths from `.vally.yaml`'s shared marketplace environment +and any surviving eval's explicit `agent_environment.skills` or `disallowed` +entries. You can keep only remote entries by removing both bundled local plugins and their eval folders. No empty `plugins/` or `evals/` directory is needed. @@ -196,13 +199,32 @@ npm run lint validation, catalog consistency checks, and remote-pin checks on every pull request. +Use Copilot to help author evals, choose native commands, and interpret results. +The npm eval aliases call Vally directly: `eval` runs cases, `eval:models` runs +the model experiment, `eval:serve` serves its native results dashboard, and +`eval:compare` invokes its comparison judge. +See [native saved-results viewing and comparison](docs/checks-and-evals.md#saved-results) +for paths and costs; serving saved results does not call a model, but comparing +them does. + **Paid evals:** first complete the [Docker and scoped authentication setup](docs/checks-and-evals.md#native-docker-evals). Local examples use a dedicated Copilot Requests token with public-repository access for the synthetic fixtures, not a broad GitHub CLI login credential. Then `npm run eval` runs real Copilot sessions and costs AI credits. [Evals](.github/workflows/evals.yml) runs only when manually started; pull -requests never trigger paid evals. +requests that change a plugin skill use the separate +[before/after workflow](.github/workflows/skill-evals.yml), with maintainer approval +required in the protected `evals` environment. Fork PRs do not run paid evals. +The workflow updates one PR comment with per-case grader counts, failure +details, and an all-skills coverage table. Unmeasured skills are marked not run; +the comment does not imply a full-marketplace or multi-model run. +Paid execution and publication use a separate +[trusted `workflow_run` workflow](.github/workflows/skill-eval-comment.yml): +the pinned Evals Action comparison adapter runs native Vally behind approval, +then a different job gets comment-write access. The +[comparison Action integration and trust limits](docs/checks-and-evals.md#pr-comparison-comment) +explain what stays in this repository and the upstream deployment dependency. Repository evals **default to Vally's native Docker backend** in [.vally.yaml](.vally.yaml), including `npm run eval` and the manually dispatched @@ -215,8 +237,8 @@ default image locally. See [Native Docker evals and isolation limits](docs/checks-and-evals.md#native-docker-evals) before running. The pinned Evals plugin's Run entrypoint cannot read the new project backend default and currently rejects this configuration; use the native -Vally CLI rather than assuming plugin Run is container-isolated. Paid CI remains -manual-only. +Vally CLI rather than assuming plugin Run is container-isolated. The smoke-suite +workflow remains manual-only; PR comparisons require protected approval. **Standard models:** `npm run eval:models` uses the native [model experiment](evals/standard-models.experiment.yaml) to test every retained diff --git a/docs/checks-and-evals.md b/docs/checks-and-evals.md index 5377092..cdb2a07 100644 --- a/docs/checks-and-evals.md +++ b/docs/checks-and-evals.md @@ -81,6 +81,25 @@ follow the skill's which uses its bundled Node helper before invoking Copilot. Installing doesn't call a model or prove how a plugin behaves in a model session. +### Customizing evals and marketplace context + +The checks and PR selector discover catalog entries rather than assuming the +bundled plugin names. Local skill comparisons use +`plugins//skills//SKILL.md` and +`evals//skills//eval.yaml`; both source versions must exist. +Other eval names can be discovered by native Vally, but are not automatically +paired by this PR integration. Agents and newly added/deleted skills are not +Before/After pairs under this policy. + +The native shared context is explicit, not generated from the catalog. When +adding/removing local skills, update `environments.marketplace.skills` in +`.vally.yaml` and affected `disallowed` lists. Remote context uses the reviewed +pinned staging commands there; update them when changing the remote context. +The comment inventory reads standard `skills//SKILL.md` folders from exact +pinned GitHub trees, without installing plugins. A remote-pin-only change does +not trigger the local-skill comparison workflow. This is not arbitrary plugin +layout support or automatic activation of hooks, agents or MCP tools. + ## Paid evals **These runs start real Copilot sessions and spend AI credits.** @@ -314,19 +333,137 @@ location, update the command's `--output-dir` too. ### Saved results For these direct Vally runs, results follow `paths.results` in -[.vally.yaml](../.vally.yaml), currently `evals/results/`. The report command -uses the same configured directory unless you supply an explicit results path. +[.vally.yaml](../.vally.yaml), currently `evals/results/`. Native run directories +contain `results.jsonl` and the saved trajectory artifacts. Copilot can help +inspect a selected run or choose the appropriate native command. -Summarize the newest saved run (verdicts, scores, input/output/total tokens, -and duration) or compare a baseline run against a candidate run without calling -a model: +View reports in Vally's built-in dashboard without calling a model: ```sh -npm run eval:report -npm run eval:compare -- --baseline evals/_shared/fixtures/pr-description/baseline-results.jsonl --candidate evals/_shared/fixtures/pr-description/candidate-results.jsonl +npm run eval:serve ``` +This is just `vally serve evals/results`, using the native +[`serve`](https://microsoft.github.io/vally/reference/cli/serve/) command. +Open `http://127.0.0.1:3200`; keep the default loopback binding because saved +trajectories and grader evidence can contain sensitive data. The pinned +`@microsoft/vally-cli` 0.21.0 includes `@microsoft/vally-server` as a dependency: +`npm ci` installs it, with no separate server package required. +If you change the results directory, update the alias or run +`vally serve ` directly. Experiments use their explicit output +directory, as described above. + +For native judged comparison, supply real baseline and treatment run directories +or JSONL files: + +```sh +npm run eval:compare -- --baseline path/to/baseline-run --treatment path/to/treatment-run +``` + +This is `vally compare`, not a custom result parser. It pairs saved trajectories +and calls a comparison judge using their rubrics, so **it spends AI credits**. +You can instead supply a native experiment output directory, or use +`--judge-model` and `--output` as documented in +[`compare`](https://microsoft.github.io/vally/reference/cli/compare/). +Viewing results and PR comment publication do not automatically invoke this +judge. No local Markdown report implementation is maintained here. + [Evals](../.github/workflows/evals.yml) runs the smoke suite only when you explicitly start it with **Run workflow** in GitHub Actions -(`workflow_dispatch`). Pull requests never start paid evals. Review the -selected branch and its eval code before dispatching a run. +(`workflow_dispatch`). Review the selected branch and its eval code before +dispatching a run. + +[Skill evals](../.github/workflows/skill-evals.yml) is a read-only PR notification +for plugin skill changes. The +[trusted default-branch workflow](../.github/workflows/skill-eval-comment.yml) +uses the standard +[unprivileged PR / trusted `workflow_run` pattern](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/). +It calls the pinned Evals Action `comparison` subaction for authentication, +paired native execution and publication. Repository-specific selection and +approval policy stay here; the generic runner, projection and comment machinery +do not. + +### PR comparison comment + +All three calls use `github/evals-action/comparison` at +`3c8cdb79a1e55b149950627a21e8723817d0e772`. The +[pinned Action contract](https://github.com/github/evals-action/blob/3c8cdb79a1e55b149950627a21e8723817d0e772/comparison/README.md) +owns source-workflow/run/attempt authentication, verified merge parents, +source switching, native verdict projection, bounded artifact intake and the +bot-owned comment. The existing root Action's event policy is unchanged. +This pin is the accessible implementation from +[github/evals-action#28](https://github.com/github/evals-action/pull/28), addressing +[github/evals-action#26](https://github.com/github/evals-action/issues/26); +it is an **unmerged upstream dependency**, not a released or live-qualified +integration. Review that dependency before deployment. Both workflows and the +trusted selector must reach the default branch before automatic activation. + +The [repository selector](../.github/scripts/select-skill-evals.mjs) verifies the +merge checkout, required environment reviewers and changed skill paths without +executing candidate scripts. Before uses the current merge's first parent, not +possibly stale PR base metadata. Eligible pairs require both skill versions and +`evals//skills//eval.yaml`. New, deleted and unpaired skills are +explicitly not run. Local skill folders and exact pinned remote GitHub trees +supply the display inventory without installing plugins. Loading neighboring +skill context does not evaluate those skills or enable full plugin hooks/MCP. +Skills, evals, fixtures, graders, catalog, models and native configuration remain +repository-owned. Local results use Vally's native dashboard and comparison +commands above. + +The protected job runs native Vally **0.21.0** sequentially with the saved model, +one trial/worker and zero retries. Only the target skill switches between +base/head sources; measurements and neighbors stay at the selected merge. +Before failure does not prevent After, and either failed/incomplete arm keeps +the job failed. The Action's Node 24 runtime acquires native tools outside the +candidate checkout with install scripts disabled and a job-private generated +integrity lock. Both arms share one acquired dependency graph; recorded +CLI/lock/dependency digests are in the job summary. Unlike the previous trusted +committed-lock bootstrap, transitive resolution can differ **across jobs**. +Candidate manifests are not bootstrap inputs. Native backend configuration, +including this repository's Docker defaults, is honored; container bootstrap, +the Copilot Evals plugin's separate Run entrypoint and runtime parity retain the +[earlier documented limits](#native-docker-evals). + +Repository policy stays visible in the workflow: same-repository PRs only, +read-only selection, the protected `evals` environment, job permissions, +timeouts, retention and concurrency. Owners must configure required reviewers +and permit CI-token billing. Missing/unprotected environments fail preflight. +The paid job has read-only repository/PR/Actions access and +`copilot-requests: write`, but no PR-write access. The fresh publisher has +PR-write access but no billing permission and performs no checkout or candidate +execution. Approval intentionally allows candidate eval/setup/grader code to +execute with the model credential. Fresh HOME/TMP and a narrow environment are +process hygiene, not credential, network or shared-kernel isolation. +Environment reviewers protect this job, not arbitrary workflows authorized +editors could add; a workflow-token permission is not an environment secret or +an organization-wide billing ceiling. No settings/authentication policy changes +or `pull_request_target` are introduced. + +Paid concurrency uses authenticated PR/head/skill outputs. Same-head duplicates +can cancel; a newer head does not immediately cancel an old in-flight arm. +The Action rechecks current PR/head after approval and before each paid arm, +blocking the next stale arm and stale publication, not undoing requests already +billed. Publishers serialize by verified PR without cancellation; producing +run/attempt order prevents older comments overwriting newer results. +**Partial paid retries are unsupported: use Re-run all jobs.** Publisher-only +retries reuse the original selection producing attempt without repeating models. + +The approved comment retains per-changed-skill case/model tables showing +Before → After **passing/total graders**, visible After failures, collapsed +Before failures and collapsed marketplace **passing/total recorded cases**. +Missing checks, grading errors and incomplete coverage remain unknown rather +than resolved failures; known failures remain visible. These single-trial +observations do not establish causation or full-cohort/multi-model coverage. +The Action contract documents the complete evidence and rendering rules. + +Only selection metadata and the Action's exact Before/After JSON projection +paths are uploaded, with seven-day retention subject to repository policy. +Never upload its private `projection-directory` recursively: it also holds raw +diagnostics. The publisher independently authenticates context and the exact +trusted selection/producing attempt; artifacts remain untrusted data, not +independent attestations of candidate code. Raw trajectories, model output, +grader evidence, exceptions and transcripts are not posted or uploaded. +Displayed eval/model/check names must themselves be suitable for publication. +Model-free checks do not prove live `workflow_run`, token acceptance, protected +approval, Docker execution or billing; those need separately authorized +qualification. diff --git a/evals/_shared/fixtures/pr-description/baseline-results.jsonl b/evals/_shared/fixtures/pr-description/baseline-results.jsonl deleted file mode 100644 index eb4e3b6..0000000 --- a/evals/_shared/fixtures/pr-description/baseline-results.jsonl +++ /dev/null @@ -1,3 +0,0 @@ -{"type":"trial-result","runId":"baseline-run-001","evalName":"pr-description","stimulus":"pr-description-drafts-structured-pr-body","status":"success","durationMs":6840,"gradeResult":{"passed":false,"score":0.5,"details":[{"name":"completed","passed":true,"score":1,"evidence":"Completed"},{"name":"skill-invocation","passed":true,"score":1,"evidence":"Invoked pr-description"},{"name":"pr-description-contract","passed":false,"score":0.5,"evidence":"Wrote ## Summary, ## Changes, ## Verification, and ## Risks / Rollback in order; Excluded unshipped retry backoff (#42) from shipped ## Changes"},{"name":"diff-empty","passed":true,"score":1,"evidence":"No workspace changes"}]},"trajectory":{"output":"## Summary\nAdded timeout validation and retry backoff.\n\n## Changes\n- Added `--timeout` validation in `src/cli.mjs` and tests in `test/cli.test.mjs`\n- Implemented exponential retry backoff (#42)\n\n## Verification\n- Ran `npm test` (14 passing)\n","events":[{"type":"skill_activation","data":{"name":"pr-description"}}],"metrics":{"wallTimeMs":6840,"turnCount":2,"toolCallCount":1,"errorCount":0,"tokenUsage":{"inputTokens":3420,"outputTokens":980,"totalTokens":4400,"cacheReadTokens":0,"cacheWriteTokens":0,"callCount":2,"byModel":{}}}}} -{"type":"trial-result","runId":"baseline-run-001","evalName":"pr-description","stimulus":"pr-description-ignores-bug-and-review-question","status":"success","durationMs":4120,"gradeResult":{"passed":true,"score":1,"details":[{"name":"completed","passed":true,"score":1,"evidence":"Completed"},{"name":"skill-invocation","passed":true,"score":1,"evidence":"No disallowed skills invoked"},{"name":"output-matches","passed":true,"score":1,"evidence":"Matched off-by-one explanation"},{"name":"diff-empty","passed":true,"score":1,"evidence":"No workspace changes"}]},"trajectory":{"output":"`slice(0, size - 1)` uses an exclusive end index, so it returns `size - 1` items and drops the final element on the page.","events":[],"metrics":{"wallTimeMs":4120,"turnCount":1,"toolCallCount":0,"errorCount":0,"tokenUsage":{"inputTokens":2150,"outputTokens":410,"totalTokens":2560,"cacheReadTokens":0,"cacheWriteTokens":0,"callCount":1,"byModel":{}}}}} -{"type":"run-summary","runId":"baseline-run-001","passed":false,"hadExecutionErrors":false,"evals":[{"name":"pr-description","passed":false,"scoringApplied":true,"overallScore":0.75,"threshold":1,"stimuliRun":2,"stimuliTotal":2,"durationMs":10960}]} diff --git a/evals/_shared/fixtures/pr-description/candidate-results.jsonl b/evals/_shared/fixtures/pr-description/candidate-results.jsonl deleted file mode 100644 index fe40069..0000000 --- a/evals/_shared/fixtures/pr-description/candidate-results.jsonl +++ /dev/null @@ -1,3 +0,0 @@ -{"type":"trial-result","runId":"candidate-run-002","evalName":"pr-description","stimulus":"pr-description-drafts-structured-pr-body","status":"success","durationMs":3910,"gradeResult":{"passed":true,"score":1,"details":[{"name":"completed","passed":true,"score":1,"evidence":"Completed"},{"name":"skill-invocation","passed":true,"score":1,"evidence":"Invoked pr-description"},{"name":"pr-description-contract","passed":true,"score":1,"evidence":"All contract assertions passed."},{"name":"diff-empty","passed":true,"score":1,"evidence":"No workspace changes"}]},"trajectory":{"output":"## Summary\nValidate `--timeout` as a positive integer in `src/cli.mjs` and cover invalid values with unit tests. Exponential retry backoff (#42) remains out of scope in the backlog.\n\n## Changes\n- Added positive integer validation for `--timeout` in `src/cli.mjs` (exits with code 2 on invalid input).\n- Added unit tests for `--timeout 0` and `--timeout -5` in `test/cli.test.mjs`.\n\n## Verification\n- `npm test` passed (14 tests).\n\n## Risks / Rollback\n- Low risk; callers passing invalid non-positive `--timeout` values now exit with code 2. Revert the commit if needed.","events":[{"type":"skill_activation","data":{"name":"pr-description"}}],"metrics":{"wallTimeMs":3910,"turnCount":1,"toolCallCount":1,"errorCount":0,"tokenUsage":{"inputTokens":1680,"outputTokens":310,"totalTokens":1990,"cacheReadTokens":0,"cacheWriteTokens":0,"callCount":1,"byModel":{}}}}} -{"type":"trial-result","runId":"candidate-run-002","evalName":"pr-description","stimulus":"pr-description-ignores-bug-and-review-question","status":"success","durationMs":2450,"gradeResult":{"passed":true,"score":1,"details":[{"name":"completed","passed":true,"score":1,"evidence":"Completed"},{"name":"skill-invocation","passed":true,"score":1,"evidence":"No disallowed skills invoked"},{"name":"output-matches","passed":true,"score":1,"evidence":"Matched off-by-one explanation"},{"name":"diff-empty","passed":true,"score":1,"evidence":"No workspace changes"}]},"trajectory":{"output":"`Array.prototype.slice(start, end)` excludes `end`, so `slice(0, size - 1)` returns at most `size - 1` items and drops the last item. Report expected (`size` items) vs. actual (`size - 1` items) with the `slice(0, size - 1)` call site.","events":[],"metrics":{"wallTimeMs":2450,"turnCount":1,"toolCallCount":0,"errorCount":0,"tokenUsage":{"inputTokens":1120,"outputTokens":190,"totalTokens":1310,"cacheReadTokens":0,"cacheWriteTokens":0,"callCount":1,"byModel":{}}}}} -{"type":"run-summary","runId":"candidate-run-002","passed":true,"hadExecutionErrors":false,"evals":[{"name":"pr-description","passed":true,"scoringApplied":true,"overallScore":1,"threshold":1,"stimuliRun":2,"stimuliTotal":2,"durationMs":6360}]} diff --git a/package.json b/package.json index 075180d..352af22 100644 --- a/package.json +++ b/package.json @@ -8,8 +8,8 @@ "lint": "node .github/scripts/lint.mjs", "eval": "vally eval --workers 1 --max-retries 1", "eval:models": "vally experiment run evals/standard-models.experiment.yaml --output-dir evals/results/standard-models --require-pass", - "eval:report": "node .github/scripts/eval-report.mjs", - "eval:compare": "node .github/scripts/eval-report.mjs" + "eval:serve": "vally serve evals/results", + "eval:compare": "vally compare" }, "devDependencies": { "@github/copilot": "1.0.98",