diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 0663337..e1349c5 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -20,6 +20,9 @@ jobs: run: pip install "git+https://github.com/agentskills/agentskills.git#subdirectory=skills-ref" - name: Validate SKILL.md run: skills-ref validate ./practical-coding + - name: Run benchmark harness tests + working-directory: practical-coding + run: python -m unittest benchmarks.test_benchmarks benchmarks.test_stability - name: Check Codex default_prompt references the skill as $skill-name run: grep -qF '$practical-coding' practical-coding/agents/openai.yaml - name: Ensure legacy local graph runtime is not reintroduced diff --git a/benchmarks/README.md b/benchmarks/README.md index bdb2056..994f66c 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -15,6 +15,21 @@ pwsh -File benchmarks/run.ps1 -Profile smoke -Suite router -Case direct-artifact pwsh -File benchmarks/run.ps1 -Rescore D:\path\to\benchmark-results\20260824-203839 ``` +For a result that will be presented as a stable ranking, opt into the evidence gate: + +```powershell +pwsh -File benchmarks/run.ps1 -Profile standard -Runs 3 -Workers 3 -RequireStableRanking +pwsh -File benchmarks/run.ps1 -Profile full -Suite delivery -Runs 3 -BaselineSkill docs\evaluations\snapshots\practical-v1.10 -RequireStableRanking +``` + +`-RequireStableRanking` refuses an effective run count below three, keeps production builds enabled for Delivery, writes to an explicit output directory when needed, and validates the completed result with `benchmarks/check_stability.py`. Existing artifacts can be checked directly: + +```powershell +python benchmarks/check_stability.py benchmark-results\v111-delivery-n1-core-reverted --suite delivery +``` + +That command intentionally reports the published v1.11 Delivery `n=1` artifact as `PROVISIONAL`; it must not be used for a stable ranking until the same cells are rerun with at least three distinct repetitions. + Profiles: - `smoke`: one run by default; quick harness and model sanity check. @@ -28,17 +43,18 @@ Useful options: - `-Suite`, `-Case`, and `-Arm` select repeatable subsets for diagnosis or focused regression gates. - `-SourcesRoot ` reuses pinned competitor checkouts. Without it, sources are cached under the user-local application data directory and cloned as needed. - `-IncludeBaseline` adds a no-skill delivery arm. -- `-NoBuilds` skips runner-owned frontend production builds. +- `-NoBuilds` skips runner-owned frontend production builds. It is rejected for a stable Delivery ranking. - `-SelfTest` runs the local harness regression tests and validates fixtures, upstream scorers, source pins, and reporting without model calls. -- `-Rescore ` reapplies the current mechanical graders to saved workspaces/transcripts without another model call; the manifest records the new runner hash and rescore time. - `-FailOnCellFailure` makes any behavioral cell failure return exit code 2. By default only harness/infrastructure failures are non-zero, because a valid comparison may intentionally expose competitor or candidate failures. +- `-RequireStableRanking` requires at least three distinct repetitions per selected suite/case/arm and rejects incomplete or infrastructure-failed runs before they are called stable. +- `-Rescore ` reapplies the current mechanical graders to saved workspaces/transcripts without another model call; the manifest records the new runner hash and rescore time. By default, run artifacts are written under `benchmark-results/` and ignored by Git, so transcripts and generated workspaces remain inspectable across commands without entering commits. Use `-Output` for an explicit location. The fast harness regression suite is also runnable without sources or model access: ```powershell -python -m unittest benchmarks.test_benchmarks +python -m unittest benchmarks.test_benchmarks benchmarks.test_stability ``` The output directory contains: @@ -66,3 +82,5 @@ cells/ prompt, raw JSONL, stderr, answer, and code workspace per ## Acceptance Use repeated paired results. A candidate is not accepted merely because its prose matches a Skill contract. Require no correctness/build regression, then compare delivered code and behavior. Treat LOC, tokens, and time as secondary within equally correct artifacts. `n=1` is a smoke result, not a stable ranking. + +A published stable ranking must pass `benchmarks/check_stability.py` with the default minimum `n=3`. The gate checks distinct repetition IDs, complete-run metadata, and infrastructure errors. Behavioral or build failures remain valid benchmark observations and therefore do not invalidate the sample by themselves. diff --git a/benchmarks/check_stability.py b/benchmarks/check_stability.py new file mode 100644 index 0000000..467b975 --- /dev/null +++ b/benchmarks/check_stability.py @@ -0,0 +1,155 @@ +#!/usr/bin/env python3 +"""Gate benchmark reports that are presented as stable rankings.""" + +from __future__ import annotations + +import argparse +import json +from collections import defaultdict +from pathlib import Path +from typing import Any + +MIN_STABLE_RUNS = 3 + + +def load_run(run_dir: Path) -> tuple[dict[str, Any], list[dict[str, Any]]]: + manifest_path = run_dir / "manifest.json" + results_path = run_dir / "results.json" + if not manifest_path.is_file() or not results_path.is_file(): + raise ValueError("run directory must contain manifest.json and results.json") + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + results = json.loads(results_path.read_text(encoding="utf-8")) + if not isinstance(manifest, dict) or not isinstance(results, list): + raise ValueError("invalid benchmark manifest/results format") + return manifest, results + + +def stability_rows( + records: list[dict[str, Any]], + *, + min_runs: int = MIN_STABLE_RUNS, + suites: set[str] | None = None, +) -> list[dict[str, Any]]: + if min_runs < 1: + raise ValueError("min_runs must be positive") + + groups: dict[tuple[str, str, str], list[dict[str, Any]]] = defaultdict(list) + for record in records: + suite = record.get("suite") + if suites and suite not in suites: + continue + groups[(str(suite), str(record.get("case")), str(record.get("arm")))].append(record) + + rows: list[dict[str, Any]] = [] + for (suite, case, arm), cells in sorted(groups.items()): + repetitions = [cell.get("repetition") for cell in cells] + valid_repetitions = [rep for rep in repetitions if isinstance(rep, int) and rep > 0] + unique_repetitions = set(valid_repetitions) + duplicate_repetitions = len(unique_repetitions) != len(valid_repetitions) + invalid_repetitions = len(valid_repetitions) != len(repetitions) + infrastructure_errors = sum(bool(cell.get("error")) for cell in cells) + stable = ( + len(unique_repetitions) >= min_runs + and not duplicate_repetitions + and not invalid_repetitions + and infrastructure_errors == 0 + ) + rows.append( + { + "suite": suite, + "case": case, + "arm": arm, + "runs": len(unique_repetitions), + "records": len(cells), + "infrastructure_errors": infrastructure_errors, + "duplicate_repetitions": duplicate_repetitions, + "invalid_repetitions": invalid_repetitions, + "stable": stable, + } + ) + return rows + + +def assess_run( + manifest: dict[str, Any], + records: list[dict[str, Any]], + *, + min_runs: int = MIN_STABLE_RUNS, + suites: set[str] | None = None, +) -> tuple[bool, list[str], list[dict[str, Any]]]: + reasons: list[str] = [] + if not manifest.get("completed_at"): + reasons.append("run is incomplete: manifest has no completed_at") + + expected_cells = manifest.get("cells") + if isinstance(expected_cells, int) and expected_cells != len(records): + reasons.append(f"manifest cells={expected_cells}, results records={len(records)}") + + rows = stability_rows(records, min_runs=min_runs, suites=suites) + if not rows: + reasons.append("no benchmark cells match the selected suites") + for row in rows: + if row["runs"] < min_runs: + reasons.append( + f"{row['suite']}/{row['case']}/{row['arm']} has n={row['runs']} < {min_runs}" + ) + if row["duplicate_repetitions"]: + reasons.append( + f"{row['suite']}/{row['case']}/{row['arm']} contains duplicate repetitions" + ) + if row["invalid_repetitions"]: + reasons.append( + f"{row['suite']}/{row['case']}/{row['arm']} contains invalid repetition ids" + ) + if row["infrastructure_errors"]: + reasons.append( + f"{row['suite']}/{row['case']}/{row['arm']} has " + f"{row['infrastructure_errors']} infrastructure error(s)" + ) + + return not reasons, reasons, rows + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description="Require repeated, complete benchmark evidence before calling a result a stable ranking." + ) + parser.add_argument("run_dir", type=Path) + parser.add_argument("--suite", action="append", help="limit the gate to one or more suites") + parser.add_argument("--min-runs", type=int, default=MIN_STABLE_RUNS) + return parser.parse_args() + + +def main() -> int: + args = parse_args() + if args.min_runs < 1: + raise SystemExit("--min-runs must be positive") + try: + manifest, records = load_run(args.run_dir.resolve()) + stable, reasons, rows = assess_run( + manifest, + records, + min_runs=args.min_runs, + suites=set(args.suite) if args.suite else None, + ) + except (OSError, ValueError, json.JSONDecodeError) as error: + print(f"INVALID: {error}") + return 1 + + status = "STABLE" if stable else "PROVISIONAL" + print(f"{status}: minimum n={args.min_runs}") + for row in rows: + marker = "ok" if row["stable"] else "not-stable" + print( + f"- {marker}: {row['suite']}/{row['case']}/{row['arm']} " + f"n={row['runs']} errors={row['infrastructure_errors']}" + ) + if reasons: + print("Reasons:") + for reason in reasons: + print(f"- {reason}") + return 0 if stable else 2 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/benchmarks/run.ps1 b/benchmarks/run.ps1 index c9092cb..0bf3668 100644 --- a/benchmarks/run.ps1 +++ b/benchmarks/run.ps1 @@ -15,6 +15,7 @@ param( [switch]$NoBuilds, [switch]$SelfTest, [switch]$FailOnCellFailure, + [switch]$RequireStableRanking, [string]$Rescore = "" ) @@ -26,7 +27,7 @@ $repoRoot = Split-Path -Parent $scriptDir if ($SelfTest) { Push-Location $repoRoot try { - & python -m unittest benchmarks.test_benchmarks + & python -m unittest benchmarks.test_benchmarks benchmarks.test_stability if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } } finally { @@ -34,6 +35,33 @@ if ($SelfTest) { } } +$effectiveRuns = if ($Runs -gt 0) { + $Runs +} +elseif ($Profile -eq "smoke") { + 1 +} +else { + 3 +} + +if ($RequireStableRanking -and $Rescore) { + Write-Error "-RequireStableRanking cannot be combined with -Rescore; gate the rescored run directly with benchmarks/check_stability.py." + exit 2 +} +if ($RequireStableRanking -and $effectiveRuns -lt 3) { + Write-Error "Stable ranking requires at least 3 runs per cell; effective runs=$effectiveRuns." + exit 2 +} +if ($RequireStableRanking -and $NoBuilds -and (($Suite.Count -eq 0) -or ($Suite -contains "delivery"))) { + Write-Error "Stable Delivery ranking requires production build evidence; remove -NoBuilds." + exit 2 +} +if ($RequireStableRanking -and -not $Output) { + $stamp = Get-Date -Format "yyyyMMdd-HHmmss" + $Output = Join-Path $repoRoot "benchmark-results/stable-$stamp" +} + $arguments = @( (Join-Path $scriptDir "run_benchmarks.py"), "--profile", $Profile, @@ -55,4 +83,18 @@ if ($FailOnCellFailure) { $arguments += "--fail-on-cell-failure" } if ($Rescore) { $arguments += @("--rescore", $Rescore) } & python @arguments -exit $LASTEXITCODE +$benchmarkExit = $LASTEXITCODE +if ($benchmarkExit -ne 0) { exit $benchmarkExit } + +if ($RequireStableRanking) { + $gateArguments = @( + (Join-Path $scriptDir "check_stability.py"), + $Output, + "--min-runs", "3" + ) + foreach ($item in $Suite) { $gateArguments += @("--suite", $item) } + & python @gateArguments + exit $LASTEXITCODE +} + +exit 0 diff --git a/benchmarks/test_stability.py b/benchmarks/test_stability.py new file mode 100644 index 0000000..0c3ffe0 --- /dev/null +++ b/benchmarks/test_stability.py @@ -0,0 +1,66 @@ +import unittest + +from benchmarks import check_stability as gate + + +def record(repetition, *, error=None): + item = { + "suite": "delivery", + "case": "tmpl-fe-command", + "arm": "practical-current", + "repetition": repetition, + "passed": False, + } + if error is not None: + item["error"] = error + return item + + +class StabilityGateTests(unittest.TestCase): + def test_n1_is_provisional(self): + stable, reasons, rows = gate.assess_run( + {"completed_at": "2026-08-24T00:00:00Z", "cells": 1}, + [record(1)], + ) + self.assertFalse(stable) + self.assertEqual(rows[0]["runs"], 1) + self.assertTrue(any("n=1 < 3" in reason for reason in reasons)) + + def test_three_distinct_repetitions_are_stable(self): + stable, reasons, rows = gate.assess_run( + {"completed_at": "2026-08-24T00:00:00Z", "cells": 3}, + [record(1), record(2), record(3)], + ) + self.assertTrue(stable) + self.assertEqual(reasons, []) + self.assertTrue(rows[0]["stable"]) + + def test_duplicate_repetition_does_not_inflate_n(self): + stable, reasons, rows = gate.assess_run( + {"completed_at": "2026-08-24T00:00:00Z", "cells": 3}, + [record(1), record(1), record(2)], + ) + self.assertFalse(stable) + self.assertEqual(rows[0]["runs"], 2) + self.assertTrue(any("duplicate repetitions" in reason for reason in reasons)) + + def test_infrastructure_error_blocks_stable_ranking(self): + stable, reasons, rows = gate.assess_run( + {"completed_at": "2026-08-24T00:00:00Z", "cells": 3}, + [record(1), record(2, error="timeout"), record(3)], + ) + self.assertFalse(stable) + self.assertEqual(rows[0]["infrastructure_errors"], 1) + self.assertTrue(any("infrastructure error" in reason for reason in reasons)) + + def test_incomplete_run_is_not_stable(self): + stable, reasons, _ = gate.assess_run( + {"cells": 3}, + [record(1), record(2), record(3)], + ) + self.assertFalse(stable) + self.assertTrue(any("run is incomplete" in reason for reason in reasons)) + + +if __name__ == "__main__": + unittest.main()