benchmarking/swebench: fix the review findings from #94-#96 (dataset builder rejects all rows, delete-park leak, cluster script), and a flaky test

.github/workflows/ci.yaml

@@ -61,5 +61,6 @@ jobs:
       - run: python -m pip install -e 'clients/python[dev]' -e 'integrations/nemo-gym[test]'
       - run: python -m pytest clients/python/tests -q
       - run: python -m pytest internal/benchmarking/swebench/trajectories -q
+      - run: python -m pytest benchmarking/swebench -q
       - run: ruff check clients/python
       - run: python -m pytest integrations/nemo-gym/tests -q

benchmarking/swebench/README.md

@@ -86,8 +86,10 @@ between runs.
 
    `--image-ref` sets how an instance maps to an image reference (default:
    one tag per instance, `sweb.eval.x86_64.<instance_id>`, in one
-   repository); the script lists instances whose image it cannot resolve and
-   writes the dataset revision and swebench version next to the manifest.
+   repository); the script lists instances whose image it cannot resolve. It
+   loads the dataset at a resolved commit (`--revision`, else the current
+   head) and writes that commit (`resolved_revision`; the file's sha256 for a
+   local dataset) and the swebench version to `OUT.meta.json`.
 
 3. Install ate-env with a base template (`ate-env manifest template`). Per
    image, the API derives one template from it on first use and bakes the

benchmarking/swebench/make-dataset.py

@@ -23,10 +23,13 @@
 registry the cluster can pull from.
 
 The eval script comes from the installed swebench's make_test_spec, which
-wraps the dataset row's own eval_script with the harness header and the
-"Test Exit Code" marker; the two are checked to agree. The dataset revision
-and the swebench version are recorded in OUT.meta.json so two operators can
-build the same manifest; pass --revision to pin the dataset.
+wraps the dataset row's own eval_script with the harness header and adds the
+two lines that record the test command's exit status ("Test Exit Code"); the
+two scripts are checked to agree line for line apart from those additions.
+The dataset is loaded at a resolved commit (the --revision given, else the
+current head), and that commit, the swebench version and the file hash of a
+local dataset are recorded in OUT.meta.json, so two operators build the same
+manifest.
 
     pip install datasets 'swebench>=5'
     ./make-dataset.py --registry us-central1-docker.pkg.dev/PROJECT/swebench \
@@ -45,10 +48,60 @@
 
 import argparse
 import datetime
+import hashlib
 import json
 import subprocess
 import sys
 
+# The lines swebench >= 5 adds around the end marker to record the test
+# command's own exit status (swebench.harness.utils.record_test_exit_code).
+# They are not in the dataset row's eval_script.
+_EXIT_CODE_VAR = "SWEBENCH_TEST_EXIT_CODE"
+_EXIT_CODE_MARKER = ">>>>> Test Exit Code"
+_HEADER = ("#!/bin/bash", "set -uxo pipefail")
+
+
+def _exit_code_names() -> tuple[str, str]:
+    try:
+        from swebench.harness.constants import TEST_EXIT_CODE, TEST_EXIT_CODE_VAR
+        return TEST_EXIT_CODE_VAR, TEST_EXIT_CODE
+    except ImportError:
+        return _EXIT_CODE_VAR, _EXIT_CODE_MARKER
+
+
+def script_lines(script: str, *, harness: bool = False) -> list[str]:
+    """The command lines of an eval script, without the shell header.
+
+    With harness=True the exit-code lines the harness adds are dropped too.
+    """
+    var, marker = _exit_code_names()
+    lines = []
+    for line in script.strip().splitlines():
+        if line in _HEADER:
+            continue
+        if harness and (line == f"{var}=$?" or line.startswith(f'echo "{marker}:')):
+            continue
+        lines.append(line)
+    return lines
+
+
+def row_script_matches(row_script: str, harness_script: str) -> bool:
+    """Whether the harness runs the dataset row's eval script, apart from the
+    header and the exit-code lines it adds."""
+    return script_lines(harness_script, harness=True) == script_lines(row_script)
+
+
+def resolve_revision(dataset: str, revision: str | None) -> str | None:
+    """The dataset's commit (Hugging Face), or the sha256 of a local file."""
+    if dataset.endswith((".json", ".jsonl")):
+        with open(dataset, "rb") as f:
+            return "sha256:" + hashlib.sha256(f.read()).hexdigest()
+    try:
+        from huggingface_hub import HfApi
+    except ImportError:
+        return None
+    return HfApi().dataset_info(dataset, revision=revision).sha
+
 
 def image_ref(registry: str, instance_id: str, fmt: str) -> str:
     return fmt.format(
@@ -75,7 +128,7 @@ def main() -> int:
     ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
     ap.add_argument("--dataset", default="SWE-bench/SWE-bench_Verified", help="Hugging Face dataset name or a local JSON/JSONL path. The rows must carry eval_script (the SWE-bench org copies do; the older princeton-nlp copies do not).")
     ap.add_argument("--split", default="test")
-    ap.add_argument("--revision", default=None, help="Dataset revision (commit or tag) to pin; recorded in the meta file.")
+    ap.add_argument("--revision", default=None, help="Dataset revision (commit, tag or branch) to load; default the current head. The resolved commit is what is loaded and recorded in the meta file.")
     ap.add_argument("--registry", required=True, help="Registry path that holds the task images, e.g. us-docker.pkg.dev/PROJECT/swebench-mirror/swebench-verified.")
     ap.add_argument("--image-ref", default="{registry}:sweb.eval.x86_64.{instance_id}", help="Image reference per instance; fields: {registry}, {instance_id}, {instance_id_1776}.")
     ap.add_argument("--limit", type=int, default=0, help="Stop after N instances (0 = all).")
@@ -94,10 +147,19 @@ def main() -> int:
         print(f"cannot import make_test_spec from swebench: {e}; pip install -U 'swebench>=5'", file=sys.stderr)
         return 2
 
+    try:
+        resolved = resolve_revision(args.dataset, args.revision)
+    except Exception as e:  # network, auth, unknown revision
+        print(f"cannot resolve the revision of {args.dataset}: {e}", file=sys.stderr)
+        return 2
+    if resolved is None:
+        print("cannot resolve the dataset revision: huggingface_hub is missing; pip install datasets", file=sys.stderr)
+        return 2
     if args.dataset.endswith((".json", ".jsonl")):
         rows = load_dataset("json", data_files=args.dataset, split="train")
     else:
-        rows = load_dataset(args.dataset, split=args.split, revision=args.revision)
+        # Load the commit that is recorded, not whatever the head is by then.
+        rows = load_dataset(args.dataset, split=args.split, revision=resolved)
     import swebench
     swebench_version = getattr(swebench, "__version__", "unknown")
     want = {s for s in args.instances.split(",") if s}
@@ -122,9 +184,10 @@ def main() -> int:
                 print(f"{iid}: dataset row lacks field {e} that this swebench release needs; "
                       "use a dataset revision that carries eval_script (swebench >= 5) or an older swebench", file=sys.stderr)
                 return 2
-            if row.get("eval_script") and row["eval_script"].strip() not in spec.eval_script:
-                print(f"{iid}: the installed swebench produced an eval script that does not contain the dataset row's; "
-                      "check the swebench version against the dataset revision", file=sys.stderr)
+            if row.get("eval_script") and not row_script_matches(row["eval_script"], spec.eval_script):
+                print(f"{iid}: the installed swebench's eval script differs from the dataset row's "
+                      "beyond the header and the exit-code lines; check the swebench version against "
+                      "the dataset revision", file=sys.stderr)
                 return 2
             rec = {
                 "instance_id": iid,
@@ -144,6 +207,7 @@ def main() -> int:
         "dataset": args.dataset,
         "split": args.split,
         "revision": args.revision,
+        "resolved_revision": resolved,
         "swebench_version": swebench_version,
         "registry": args.registry,
         "image_ref": args.image_ref,

benchmarking/swebench/run-in-cluster.sh

@@ -28,7 +28,12 @@
 # characters). Results land in ./RUN_ID.jsonl and ./RUN_ID.json in the
 # directory the script is run from, which can be outside the repository.
 # NAMESPACE defaults to ate-env; START_TIMEOUT (seconds, default 600) bounds
-# the wait for the pod to start.
+# the wait for the pod to start. A failed kubectl call while waiting for the
+# loader's exit code is retried; only a pod that is gone (NotFound) or Failed
+# ends the wait early. If the script is killed before its trap runs, the Job
+# ends by itself after 48 hours (activeDeadlineSeconds) and is removed an hour
+# after it finishes; Ctrl-C runs the trap and deletes the Job at once, results
+# included.
 
 set -o errexit -o nounset -o pipefail
 
@@ -66,8 +71,11 @@ ARGS_JSON="$(python3 -c 'import json, sys; print(json.dumps(sys.argv[1:]))' \
   run "-endpoint=ate-env-api.${NS}.svc:7777" "-dataset=/var/run/ko/dataset.jsonl" \
   "-exclude-file=/var/run/ko/offline-exclusions.txt" "-run-id=${RUN_ID}" "-out=/work/${RUN_ID}.jsonl" "$@")"
 
+KUBECTL_ERR="$(mktemp)"
+# shellcheck disable=SC2329 # invoked by the EXIT trap
 cleanup() {
   kubectl -n "${NS}" delete job "${JOB}" --ignore-not-found --wait=false >/dev/null 2>&1 || true
+  rm -f "${KUBECTL_ERR}"
 }
 trap cleanup EXIT
 
@@ -81,6 +89,10 @@ metadata:
   namespace: ${NS}
 spec:
   backoffLimit: 0
+  # Bounds for a Job whose script died before its trap ran: the keep sidecar
+  # sleeps 48 hours, and a finished Job is removed after an hour.
+  activeDeadlineSeconds: 172800
+  ttlSecondsAfterFinished: 3600
   template:
     metadata:
       labels:
@@ -143,23 +155,48 @@ while (( SECONDS < deadline )); do
 done
 if [[ -z "${POD}" ]] || ! kubectl -n "${NS}" get pod "${POD}" -o jsonpath='{.status.containerStatuses[?(@.name=="loader")].state}' 2>/dev/null | grep -q 'running\|terminated'; then
   echo "loader did not start within ${START_TIMEOUT}s" >&2
-  [[ -n "${POD}" ]] && kubectl -n "${NS}" describe pod "${POD}" | tail -15 >&2
+  if [[ -n "${POD}" ]]; then
+    kubectl -n "${NS}" describe pod "${POD}" | tail -15 >&2 || true
+  else
+    echo "no pod was created for job ${JOB}; its events:" >&2
+    kubectl -n "${NS}" describe job "${JOB}" | tail -15 >&2 || true
+  fi
   exit 1
 fi
 echo "pod ${POD}; following the loader log"
 kubectl -n "${NS}" logs -f "${POD}" -c loader || true
 
-# The log stream can end before the container does; wait for the exit code,
-# with the pod's own state as the deadline check.
-until EXIT="$(kubectl -n "${NS}" get pod "${POD}" -o jsonpath='{.status.containerStatuses[?(@.name=="loader")].state.terminated.exitCode}')" && [[ -n "${EXIT}" ]]; do
-  PHASE="$(kubectl -n "${NS}" get pod "${POD}" -o jsonpath='{.status.phase}' 2>/dev/null || echo gone)"
+# The log stream can end before the container does (or drop with the
+# connection); wait for the exit code, with the pod's own state as the
+# deadline check. One failed kubectl call is not a lost pod: only NotFound
+# means gone, and other errors are retried, MAX_API_ERRORS times in a row.
+MAX_API_ERRORS="${MAX_API_ERRORS:-30}"
+api_errors=0
+until EXIT="$(kubectl -n "${NS}" get pod "${POD}" -o jsonpath='{.status.containerStatuses[?(@.name=="loader")].state.terminated.exitCode}' 2>/dev/null)" && [[ -n "${EXIT}" ]]; do
+  if PHASE="$(kubectl -n "${NS}" get pod "${POD}" -o jsonpath='{.status.phase}' 2>"${KUBECTL_ERR}")"; then
+    api_errors=0
+  elif grep -q 'NotFound' "${KUBECTL_ERR}"; then
+    PHASE=gone
+  else
+    api_errors=$((api_errors + 1))
+    echo "kubectl get pod failed (${api_errors}/${MAX_API_ERRORS}): $(head -c 300 "${KUBECTL_ERR}")" >&2
+    if (( api_errors >= MAX_API_ERRORS )); then
+      exit 1
+    fi
+    sleep 10
+    continue
+  fi
   if [[ "${PHASE}" == "gone" || "${PHASE}" == "Failed" ]]; then
     echo "pod ${POD} is ${PHASE} before the loader reported an exit code" >&2
     exit 1
   fi
   sleep 10
 done
-kubectl -n "${NS}" cp -c keep "${POD}:/work/${RUN_ID}.jsonl" "${OUT_DIR}/${RUN_ID}.jsonl"
-echo "loader exit code ${EXIT}; events in ${OUT_DIR}/${RUN_ID}.jsonl"
+echo "loader exit code ${EXIT}"
+if ! kubectl -n "${NS}" cp -c keep "${POD}:/work/${RUN_ID}.jsonl" "${OUT_DIR}/${RUN_ID}.jsonl"; then
+  echo "no events file to copy; the loader exited before writing it" >&2
+  exit "$(( EXIT == 0 ? 1 : EXIT ))"
+fi
+echo "events in ${OUT_DIR}/${RUN_ID}.jsonl"
 go run ./cmd/benchmarking/swebench-loader report "${OUT_DIR}/${RUN_ID}.jsonl" -json "${OUT_DIR}/${RUN_ID}.json"
 exit "${EXIT}"

benchmarking/swebench/test_make_dataset.py

@@ -0,0 +1,77 @@
+# Copyright 2026 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""make-dataset.py's eval-script check and revision resolution, without a network."""
+
+from __future__ import annotations
+
+import importlib.util
+import pathlib
+
+_SPEC = importlib.util.spec_from_file_location(
+    "make_dataset", pathlib.Path(__file__).with_name("make-dataset.py"))
+md = importlib.util.module_from_spec(_SPEC)
+_SPEC.loader.exec_module(md)
+
+# The shape of a SWE-bench Verified row's eval_script (astropy__astropy-12907,
+# shortened) and what swebench 5.x's make_test_spec turns it into.
+ROW = """#!/bin/bash
+set -uxo pipefail
+source /opt/miniconda3/bin/activate
+cd /testbed
+git checkout d16bfe05a744909de4b27f5875fe0d4ed41ce607 astropy/modeling/tests/test_separable.py
+git apply -v - <<'EOF_114329324912'
+diff --git a/astropy/modeling/tests/test_separable.py b/astropy/modeling/tests/test_separable.py
+EOF_114329324912
+: '>>>>> Start Test Output'
+pytest -rA astropy/modeling/tests/test_separable.py
+: '>>>>> End Test Output'
+git checkout d16bfe05a744909de4b27f5875fe0d4ed41ce607 astropy/modeling/tests/test_separable.py
+"""
+
+HARNESS = ROW.replace(
+    "pytest -rA astropy/modeling/tests/test_separable.py\n: '>>>>> End Test Output'\n",
+    "pytest -rA astropy/modeling/tests/test_separable.py\nSWEBENCH_TEST_EXIT_CODE=$?\n"
+    ": '>>>>> End Test Output'\necho \">>>>> Test Exit Code: $SWEBENCH_TEST_EXIT_CODE\"\n",
+)
+
+
+def test_the_harness_additions_are_accepted():
+    # The substring check this replaces rejected each row of the dataset.
+    assert ROW.strip() not in HARNESS
+    assert md.row_script_matches(ROW, HARNESS)
+
+
+def test_the_header_may_be_absent_on_either_side():
+    body = ROW.replace("#!/bin/bash\nset -uxo pipefail\n", "")
+    assert md.row_script_matches(body, HARNESS)
+    assert md.row_script_matches(ROW, HARNESS.replace("#!/bin/bash\nset -uxo pipefail\n", ""))
+
+
+def test_a_changed_test_command_is_rejected():
+    assert not md.row_script_matches(ROW, HARNESS.replace("pytest -rA", "pytest -rA -k nothing"))
+
+
+def test_a_missing_or_extra_line_is_rejected():
+    assert not md.row_script_matches(ROW, HARNESS.replace("cd /testbed\n", ""))
+    assert not md.row_script_matches(ROW, HARNESS + "rm -rf /testbed\n")
+
+
+def test_a_local_dataset_resolves_to_its_file_hash(tmp_path):
+    f = tmp_path / "rows.jsonl"
+    f.write_text('{"instance_id": "a"}\n')
+    rev = md.resolve_revision(str(f), None)
+    assert rev.startswith("sha256:") and len(rev) == len("sha256:") + 64
+    f.write_text('{"instance_id": "b"}\n')
+    assert md.resolve_revision(str(f), None) != rev

guest/process/service_test.go

@@ -455,7 +455,7 @@ func TestNotFound(t *testing.T) {
 }
 
 func TestConcurrencyLimiter(t *testing.T) {
-	cfg := DefaultConfig("")
+	cfg := DefaultConfig(t.TempDir()) // not the shared default: the cleanup removes LogDir
 	cfg.MaxConcurrentProcesses = 2
 	client := setupTestServerWithConfig(t, cfg)
 	ctx := context.Background()
@@ -481,7 +481,7 @@ func TestConcurrencyLimiter(t *testing.T) {
 }
 
 func TestLogCapping(t *testing.T) {
-	cfg := DefaultConfig("")
+	cfg := DefaultConfig(t.TempDir()) // not the shared default: the cleanup removes LogDir
 	cfg.MaxLogBytes = 256
 	client := setupTestServerWithConfig(t, cfg)
 
@@ -497,7 +497,7 @@ func TestLogCapping(t *testing.T) {
 }
 
 func TestDefaultTimeoutKills(t *testing.T) {
-	cfg := DefaultConfig("")
+	cfg := DefaultConfig(t.TempDir()) // not the shared default: the cleanup removes LogDir
 	cfg.DefaultProcessTimeout = 100 * time.Millisecond
 	client := setupTestServerWithConfig(t, cfg)
 

internal/benchmarking/swebench/dataset.go

@@ -86,7 +86,20 @@ func LoadDataset(source string) ([]Instance, error) {
 		return nil, fmt.Errorf("dataset %s: remote manifests must use https", source)
 	}
 	if strings.HasPrefix(source, "https://") {
-		client := &http.Client{Timeout: datasetFetchTimeout}
+		client := &http.Client{
+			Timeout: datasetFetchTimeout,
+			// The https rule holds for each hop: a redirect to http would
+			// fetch the scripts in clear text.
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				if req.URL.Scheme != "https" {
+					return fmt.Errorf("redirect to %s: remote manifests must use https", req.URL.Redacted())
+				}
+				if len(via) >= 10 {
+					return errors.New("stopped after 10 redirects")
+				}
+				return nil
+			},
+		}
 		resp, err := client.Get(source)
 		if err != nil {
 			return nil, fmt.Errorf("fetch dataset: %w", err)