From f5315cf14ff0dc1ba907d7c23764256983da6ed8 Mon Sep 17 00:00:00 2001 From: shekharprateek Date: Mon, 26 Oct 2026 18:01:35 -0400 Subject: [PATCH] fix: enforce the no-run-output policy in .gitignore, and import AGENTS.md Two guardrails this repository states but does not enforce, plus the doc and comment claims that went with them. 1. Run output was committable, and four places said it was not. .gitignore only covered the cloned target repos (swe-benchmark-data/*/repo/) and replay.jsonl. Everything a run actually writes -- metrics.json, eval.json, patch.diff, the design artifacts and multi-megabyte *-stream.jsonl -- was committable by a plain `git add -A` after a benchmark. docs/repository- structure.md asserted "Gitignored - no run output is committed", and harness-reference.md, docs/omp-setup.md and the onboard-benchmark skill made the same promise for metrics.json, stream files and runner.yaml. The tree is now ignored with the directory's README re-included, which is the only file tracked under it. This is the one place the rule deliberately differs from upstream: upstream re-includes metrics.json, eval.json and run-summary.json so its runs stay inspectable from git, and here those are measured results this repository does not publish. benchmarks/config/runner.yaml is ignored too. It holds the endpoint and, unless api_key_env is used, a literal API key -- and two docs plus the onboard-benchmark skill walk every new user through creating it while stating it is gitignored. Also ported from upstream: a swe-clone-*/ backstop for a clone_dir pointed inside the repo, the three stray repo-root writes an agent produces when handed a bare relative filename instead of its absolute artifacts_dir, and the .hftoken spelling beside the existing .hf_token. 2. CLAUDE.md did not import AGENTS.md. Claude Code reads CLAUDE.md, not AGENTS.md, so it started with a single Markdown note and never saw this repository's two process guardrails: never commit directly to main, never merge without review. CLAUDE.md now imports AGENTS.md, which already carries the Git rules, the navigation guidance, the paths not to read and the same Markdown convention. 3. Claims that pointed the other way. harness-reference.md described "the committed run-summary.json", and three script docstrings and comments said the same. No run-summary.json is committed here and none was before this change -- only the README is tracked under swe-benchmark-data -- so the wording was inherited from upstream, where the rollup IS committed. Reworded to describe run-summary.json as the file a team keeps or publishes. No behaviour change: summarize_run.py still scrubs the judge's local temp path, it just no longer gives a reason that does not hold. Verified each case with git check-ignore: run output, stream files, runner.yaml and a stray /github-issue.md are ignored; swe-benchmark-data/README.md stays tracked. `git ls-files --ignored --exclude-standard -c` is empty, so no file that was tracked became ignored. 642 tests pass, ruff and ruff format clean. --- .gitignore | 45 ++++++++++++++++--- CLAUDE.md | 8 ++-- benchmarks/docs/harness-reference.md | 2 +- .../scripts/plot_complexity_breakdown.py | 2 +- benchmarks/scripts/plot_cost_quality.py | 2 +- benchmarks/scripts/summarize_run.py | 6 +-- 6 files changed, 49 insertions(+), 16 deletions(-) diff --git a/.gitignore b/.gitignore index 11c055f9..c4bcd746 100644 --- a/.gitignore +++ b/.gitignore @@ -19,12 +19,36 @@ self-hosted/vllm/logs/ .env.* .scratchpad/ -# Benchmark target repositories are cloned locally by contributors at a specific tag. -# Keep the README and the generated artifacts tracked, but never commit the cloned source. -# The double-glob form also catches nested copies a tool/cwd mistake might create -# (e.g. bedrock/benchmarks/swe-benchmark-data//repo/). -benchmarks/swe-benchmark-data/*/repo/ -**/benchmarks/swe-benchmark-data/*/repo/ +# Benchmark run output. This repository publishes NO run output: not the design +# artifacts, not the per-task metrics, not the judge scores, not the agent event +# streams. Only the directory's own README is tracked. The previous rules here +# ignored just the cloned target repos (/repo/), which left every artifact +# a run writes -- metrics.json, eval.json, patch.diff and multi-megabyte +# *-stream.jsonl -- committable by a plain `git add -A` after a benchmark, while +# three docs stated the opposite. Ignore the tree and re-include the README. +# (Upstream re-includes metrics.json, eval.json and run-summary.json so its runs +# are inspectable from git. That is the one place this rule deliberately differs: +# here those files are measured results, which this repository does not publish.) +benchmarks/swe-benchmark-data/** +!benchmarks/swe-benchmark-data/ +!benchmarks/swe-benchmark-data/README.md + +# Per-task target-repo clones. The harness clones into +# /swe-clone-/ (clone_dir defaults to /tmp, outside the +# repo) and deletes them after each task; this is the backstop for a config that +# points clone_dir inside the repo, so a leftover clone from a killed run can +# never be committed. The swe-clone- prefix is specific enough that this never +# matches the swe-benchmark-data output tree. +swe-clone-*/ +**/swe-clone-*/ + +# Stray repo-root writes from a benchmark session: an agent handed a bare +# relative filename instead of the absolute artifacts_dir drops the file here, +# because both harnesses run the agent with cwd=REPO_ROOT. Named individually +# rather than by glob so an ordinary new doc at the root is not silently ignored. +/completion_confirmation.md +/task_completion_report.txt +/github-issue.md __pycache__/ *.pyc credentials.json @@ -40,4 +64,11 @@ security-review-findings.xlsx .venv/ tmp/ .claude/worktrees/ -benchmarks/swe-benchmark-data/**/replay.jsonl +# The runner config holds the endpoint and, unless api_key_env is used, a literal +# API key. Two docs and the onboard-benchmark skill tell every new user to create +# this file and state that it is gitignored; until now it was not. +benchmarks/config/runner.yaml + +# HuggingFace token, second spelling (see .hf_token above). +.hftoken +**/.hftoken diff --git a/CLAUDE.md b/CLAUDE.md index 9e644be6..2fbe1ef2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,5 +1,7 @@ -# Project conventions +# Claude Coding Rules -## Markdown +The coding standards and agent instructions for this repository live in **[AGENTS.md](AGENTS.md)**, the single canonical guide every coding agent follows (Claude Code, opencode, Codex, Cursor, Aider and others). -- **Do not hard-wrap prose in Markdown files.** Write each paragraph as a single line and let the editor/renderer soft-wrap it. Tables, fenced code blocks, and list structure are unaffected. +Claude Code reads this file rather than AGENTS.md, so the line below imports it. The import pulls in the repository navigation rules, the paths not to read, the Markdown conventions, and the Git rules -- including **never commit directly to `main`** and **never merge a PR without review**, which Claude Code could not see while this file carried only a Markdown note. Keep substantive rules in AGENTS.md so the two never drift. + +@AGENTS.md diff --git a/benchmarks/docs/harness-reference.md b/benchmarks/docs/harness-reference.md index 1473ff43..fe01fa7e 100644 --- a/benchmarks/docs/harness-reference.md +++ b/benchmarks/docs/harness-reference.md @@ -470,7 +470,7 @@ A metric present in neither snapshot is reported as `null` (distinct from `0`, w Because the file records the task, model, and provider (endpoint or Amazon Bedrock) next to the token, latency, throughput, and turn-count numbers, the same task can be run against many models and the resulting `metrics.json` files compared side by side -- so you can see how each model differs in cost, speed, and how many turns it took to produce the artifacts. On a failed run the file also carries an `error` string and `api_error_status`, so a failure is diagnosable without re-running the task. -The per-task `metrics.json` is **gitignored** (it can be large and is machine-local). So the committed **`run-summary.json`** rollup (written by `summarize_run.py`) folds the *derived* signals up into each task row -- `input_tokens`, `output_tokens`, `num_turns`, `latency_seconds`, `cache_read_tokens`, `cache_write_tokens`, `prefix_cache_hit_rate`, `generation_tokens_per_sec`, `kv_cache_usage` (peak/mean), plus `agent_invocations` and `topped_up_artifacts` (top-up provenance) and the judge's `eval_scores`. This keeps the rollup self-contained and chartable: models can be compared on cache/KV efficiency, not just raw token counts, straight from the committed data -- without the raw `vllm_prometheus` counters/histograms (which are large and carry the single-tenant caveat). The human-readable `run-summary.md` surfaces a prefix-cache column per task and flags any task completed by a top-up. +The per-task `metrics.json` is **gitignored** (it can be large and is machine-local). So the **`run-summary.json`** rollup (written by `summarize_run.py`, and the file to keep or publish yourself if you want a run on the record) folds the *derived* signals up into each task row -- `input_tokens`, `output_tokens`, `num_turns`, `latency_seconds`, `cache_read_tokens`, `cache_write_tokens`, `prefix_cache_hit_rate`, `generation_tokens_per_sec`, `kv_cache_usage` (peak/mean), plus `agent_invocations` and `topped_up_artifacts` (top-up provenance) and the judge's `eval_scores`. This keeps the rollup self-contained and chartable: models can be compared on cache/KV efficiency, not just raw token counts, straight from the committed data -- without the raw `vllm_prometheus` counters/histograms (which are large and carry the single-tenant caveat). The human-readable `run-summary.md` surfaces a prefix-cache column per task and flags any task completed by a top-up. These metrics measure the *cost* of a run, not the *quality* of what it produced. The judge (next section) scores the four artifacts and adds those results to this same `metrics.json` under an `evaluation` key, so a single file holds both what a run cost and how good its output was -- the basis for ranking models on the benchmark. diff --git a/benchmarks/scripts/plot_complexity_breakdown.py b/benchmarks/scripts/plot_complexity_breakdown.py index 5a932514..d04d1486 100644 --- a/benchmarks/scripts/plot_complexity_breakdown.py +++ b/benchmarks/scripts/plot_complexity_breakdown.py @@ -111,7 +111,7 @@ def _load_summary(data_dir: Path, model: str, harness: str, skill: str, scope: str): - """Load a committed run-summary.json. + """Load a run's run-summary.json. Args: data_dir: The swe-benchmark-data root. diff --git a/benchmarks/scripts/plot_cost_quality.py b/benchmarks/scripts/plot_cost_quality.py index 427f572b..1f5042e8 100644 --- a/benchmarks/scripts/plot_cost_quality.py +++ b/benchmarks/scripts/plot_cost_quality.py @@ -253,7 +253,7 @@ def _task_score(eval_data: dict | None) -> float | None: def _point_from_summary( model_repo_dir: Path, model: str, arms: dict[str, str] | None = None ) -> ModelPoint | None: - """Build a ModelPoint from the committed run-summary.json, if present. + """Build a ModelPoint from a run's run-summary.json, if present. run-summary.json already carries the leaderboard-convention means (failed 0-score tasks excluded) and is committed for every model, so it is the diff --git a/benchmarks/scripts/summarize_run.py b/benchmarks/scripts/summarize_run.py index 1c22a140..f8209832 100644 --- a/benchmarks/scripts/summarize_run.py +++ b/benchmarks/scripts/summarize_run.py @@ -174,9 +174,9 @@ def _summarize(folder: Path, run_date: str | None) -> dict[str, Any]: # Identity/serving from the first task's metrics (uniform across a run). first = _read_json(folder / rows[0]["task"] / "metrics.json") or {} - # run-summary.json is committed to git, so drop the judge block's local-only - # temp path (repo_root, e.g. /tmp/swe-judge-repos/...); it is machine-specific - # noise, not provenance worth committing. + # Drop the judge block's local-only temp path (repo_root, e.g. + # /tmp/swe-judge-repos/...): it is machine-specific noise, not provenance, + # and run-summary.json is the file a team would keep or publish. judge = dict((first.get("evaluation") or {}).get("judge") or {}) judge.pop("repo_root", None) refs = sorted({r["ref"] for r in rows if r.get("ref")})