fix(swebench): namespace harness run_id by cell to stop concurrent collisions

Concurrent hybrid SWE cells were colliding on the swebench-harness
shared cache because run_id was keyed only on instance_id
(`oj-<instance_id>`). The second cell to score the same task hit
"1 instances already run, skipping..." and silently scored 0 with
`reason: no_report` (or read the first cell's verdict on a write race).
Previously papered over by manual cache wipes + rescores; this is the
root-cause fix.

run_id is now `oj-<cell>-<instance>` when a cell name is supplied,
falling back to the legacy `oj-<instance>` form for single-cell
callers. Same-cell resumes still hit the harness cache (run_id is
deterministic for a given cell + instance). Cell name is sanitized
to filesystem-safe `[A-Za-z0-9._-]+` since it lands in both the
`logs/run_evaluation/<run_id>/...` subtree and the
`<model>.<run_id>.json` summary filename.

Plumbed cell_name through `runner._process -> score -> _score_swebench
-> SWEBenchHarnessScorer -> _run_harness -> _build_run_id`. Regression
test under `tests/agents/hybrid/test_swebench_run_id_isolation.py`
pins: (a) different cells produce different run_ids, (b) same cell is
stable, (c) legacy `None` cell preserves old format, (d) hostile cell
names sanitize cleanly, (e) end-to-end wiring from scorer constructor
through to the run_id.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/openjarvis/agents/hybrid/runner.py

@@ -256,8 +256,19 @@ def _score_gaia(task: Dict[str, Any], answer: str) -> Dict[str, Any]:
     }
 
 
-def _score_swebench(task: Dict[str, Any], answer: str) -> Dict[str, Any]:
-    """Modal-backed SWE-bench Verified harness scorer."""
+def _score_swebench(
+    task: Dict[str, Any],
+    answer: str,
+    cell_name: Optional[str] = None,
+) -> Dict[str, Any]:
+    """Modal-backed SWE-bench Verified harness scorer.
+
+    ``cell_name`` is passed through to :class:`SWEBenchHarnessScorer` so the
+    underlying ``run_id`` is unique per (cell, instance). Without it,
+    concurrent hybrid cells scoring the same task collide on the shared
+    swebench harness cache and the second cell silently scores 0 with
+    ``reason: no_report`` (or reads the first cell's verdict).
+    """
     from openjarvis.evals.core.types import EvalRecord
     from openjarvis.evals.scorers.swebench_harness import (
         SWEBenchHarnessScorer,
@@ -275,7 +286,10 @@ def _score_swebench(task: Dict[str, Any], answer: str) -> Dict[str, Any]:
         category="agentic",
         metadata={"instance_id": task["task_id"]},
     )
-    scorer = SWEBenchHarnessScorer(timeout_s=int(os.environ.get("SWEBENCH_TIMEOUT_S", "1800")))
+    scorer = SWEBenchHarnessScorer(
+        timeout_s=int(os.environ.get("SWEBENCH_TIMEOUT_S", "1800")),
+        cell_name=cell_name,
+    )
     is_correct, details = scorer.score(record, answer)
     return {
         "success": bool(is_correct),
@@ -284,11 +298,16 @@ def _score_swebench(task: Dict[str, Any], answer: str) -> Dict[str, Any]:
     }
 
 
-def score(bench: str, task: Dict[str, Any], answer: str) -> Dict[str, Any]:
+def score(
+    bench: str,
+    task: Dict[str, Any],
+    answer: str,
+    cell_name: Optional[str] = None,
+) -> Dict[str, Any]:
     if bench == "gaia":
         return _score_gaia(task, answer)
     if bench in ("swebench-verified", "swebench_verified", "swebench"):
-        return _score_swebench(task, answer)
+        return _score_swebench(task, answer, cell_name=cell_name)
     raise ValueError(f"unknown bench: {bench!r}")
 
 
@@ -573,7 +592,9 @@ def _run_cell_locked(
         scored: Optional[Dict[str, Any]] = None
         if do_score and row.get("error") is None:
             try:
-                scored = score(cell["bench"], task, row["answer"])
+                scored = score(
+                    cell["bench"], task, row["answer"], cell_name=cell_name,
+                )
             except Exception as e:
                 scored = {
                     "success": False, "score": 0.0,

src/openjarvis/evals/scorers/swebench_harness.py

@@ -271,8 +271,8 @@ def _find_report(cache: Path, instance_id: str, run_id: str) -> Optional[Dict[st
     """Find the harness's report JSON for one instance.
 
     swebench writes ``<model_name_or_path>.<run_id>.json`` inside the
-    subprocess CWD. We use ``model_name_or_path="openjarvis-harness"``,
-    ``run_id=f"oj-{instance_id}"`` in :func:`_run_harness`.
+    subprocess CWD. We use ``model_name_or_path="openjarvis-harness"``;
+    ``run_id`` is built by :func:`_build_run_id`.
     """
     fname = f"openjarvis-harness.{run_id}.json"
     p = cache / fname
@@ -284,7 +284,53 @@ def _find_report(cache: Path, instance_id: str, run_id: str) -> Optional[Dict[st
         return None
 
 
-def _run_harness(instance_id: str, patch: str, timeout_s: int) -> Dict[str, Any]:
+_RUN_ID_SAFE_RE = re.compile(r"[^A-Za-z0-9._-]+")
+
+
+def _sanitize_run_id_part(s: str) -> str:
+    """Reduce a free-form string to filesystem-safe ``[A-Za-z0-9._-]+``.
+
+    Both the harness summary filename (``<model>.<run_id>.json``) and the
+    per-instance log subtree (``logs/run_evaluation/<run_id>/...``) are
+    keyed on ``run_id``, so any character that breaks paths or globs will
+    silently corrupt the score. Strip leading/trailing dashes too — those
+    look fine but make filenames awkward to manage by hand.
+    """
+    return _RUN_ID_SAFE_RE.sub("-", s).strip("-")
+
+
+def _build_run_id(instance_id: str, cell_name: Optional[str]) -> str:
+    """Construct a swebench ``run_id`` unique per (cell, instance).
+
+    The harness keys both its "already run, skipping" cache and its report
+    file path on ``run_id`` alone, so two concurrent cells scoring the
+    same ``instance_id`` with the same ``run_id`` collide: the second
+    cell's harness invocation finds the first's report on disk, skips
+    actual execution, and our caller silently reads the wrong verdict (or
+    ``no_report`` if the two cells race on the summary file write). See
+    :func:`_run_harness` for the full failure mode.
+
+    With ``cell_name`` we emit ``oj-<cell>-<instance>``, which keeps the
+    intra-cell resume cache working (same cell + same instance → same
+    run_id → harness cache hit) while making inter-cell collisions
+    impossible. Without ``cell_name`` we fall back to the legacy
+    ``oj-<instance>`` form for backwards compat with single-cell callers.
+    """
+    safe_instance = _sanitize_run_id_part(instance_id)
+    if not cell_name:
+        return f"oj-{safe_instance}"
+    safe_cell = _sanitize_run_id_part(cell_name)
+    if not safe_cell:
+        return f"oj-{safe_instance}"
+    return f"oj-{safe_cell}-{safe_instance}"
+
+
+def _run_harness(
+    instance_id: str,
+    patch: str,
+    timeout_s: int,
+    cell_name: Optional[str] = None,
+) -> Dict[str, Any]:
     """Hand one prediction to ``python -m swebench.harness.run_evaluation``.
 
     Returns ``{"success": bool, "score": float, "details": dict}``.
@@ -292,7 +338,7 @@ def _run_harness(instance_id: str, patch: str, timeout_s: int) -> Dict[str, Any]
     _apply_patches_once()
     backend = os.environ.get("SWEBENCH_BACKEND", "modal").lower()
     cache = _harness_cache_dir()
-    run_id = f"oj-{instance_id}"
+    run_id = _build_run_id(instance_id, cell_name)
 
     # Defend against stale reports: ``run_id`` is deterministic per
     # instance, the cache dir is shared across runs, and ``_find_report``
@@ -381,10 +427,17 @@ class SWEBenchHarnessScorer(Scorer):
         self,
         *,
         timeout_s: int = 1800,
+        cell_name: Optional[str] = None,
         judge_backend: object = None,  # noqa: ARG002 — CLI factory compat
         judge_model: str = "",         # noqa: ARG002 — CLI factory compat
     ) -> None:
         self._timeout_s = int(timeout_s)
+        # ``cell_name`` namespaces the ``run_id`` so concurrent cells scoring
+        # the same SWE instance don't collide on the harness's shared cache.
+        # See :func:`_build_run_id` for the failure mode this prevents. Pass
+        # the hybrid cell name (e.g. ``"skillorchestra-qwen36-opus47-swe-n100"``)
+        # or leave as ``None`` for single-cell callers.
+        self._cell_name = cell_name
 
     def score(
         self,
@@ -406,10 +459,17 @@ class SWEBenchHarnessScorer(Scorer):
         if not instance_id:
             return False, {"reason": "missing_instance_id"}
 
-        result = _run_harness(instance_id, patch, self._timeout_s)
+        result = _run_harness(
+            instance_id, patch, self._timeout_s, cell_name=self._cell_name,
+        )
         details = dict(result.get("details", {}))
         details["patch"] = patch
         return bool(result["success"]), details
 
 
-__all__ = ["SWEBenchHarnessScorer", "extract_patch"]
+__all__ = [
+    "SWEBenchHarnessScorer",
+    "extract_patch",
+    "_build_run_id",
+    "_sanitize_run_id_part",
+]

tests/agents/hybrid/test_swebench_run_id_isolation.py

@@ -0,0 +1,143 @@
+"""Regression test for SWE-bench ``run_id`` collisions across concurrent cells.
+
+Failure mode (observed 2026-05-18, twice):
+    Multiple hybrid SWE cells running concurrently against the Modal
+    ``swebench-harness`` shared its per-instance cache via a ``run_id``
+    keyed only on ``instance_id``. Two cells scoring the same task hit
+    "1 instances already run, skipping..." on the second call; the
+    runner saw ``reason: no_report`` and scored that row 0 even when
+    the patch was correct. First seen with
+    ``minions-qwen27b-opus47-swe-n100`` vs the advisors cell; recurred
+    on 2026-05-18 across three of four ``qwen36`` SWE cells.
+
+Fix: ``_build_run_id(instance_id, cell_name)`` namespaces the run_id by
+cell so concurrent cells can't collide while same-cell resumes still
+hit the harness cache.
+
+This test pins the contract:
+
+1. Different cells + same instance → different run_ids (the bug).
+2. Same cell + same instance → identical run_id (resume still works).
+3. Both forms are filesystem-safe (no chars that would break the
+   ``logs/run_evaluation/<run_id>/...`` subtree or the
+   ``<model>.<run_id>.json`` summary glob).
+4. Cell name flows from :class:`SWEBenchHarnessScorer` into the run_id —
+   wiring regression guard, in case someone refactors and drops the
+   ``cell_name`` kwarg.
+"""
+
+from __future__ import annotations
+
+import re
+
+import pytest
+
+from openjarvis.evals.scorers.swebench_harness import (
+    SWEBenchHarnessScorer,
+    _build_run_id,
+    _sanitize_run_id_part,
+)
+
+
+# Filenames + harness log paths must match this. Slashes, spaces, colons,
+# and other shell-hostile chars would silently corrupt scoring.
+_SAFE_RE = re.compile(r"^[A-Za-z0-9._-]+$")
+
+
+INSTANCE = "astropy__astropy-12907"
+CELL_A = "advisors-qwen36-opus47-swe-n100"
+CELL_B = "skillorchestra-qwen36-opus47-swe-n100"
+
+
+def test_different_cells_produce_different_run_ids():
+    """The core bug: two cells, same instance → must not collide."""
+    a = _build_run_id(INSTANCE, CELL_A)
+    b = _build_run_id(INSTANCE, CELL_B)
+    assert a != b, (
+        f"run_id collision between cells {CELL_A!r} and {CELL_B!r} "
+        f"on instance {INSTANCE!r}: both produced {a!r}"
+    )
+    # Both must still contain the instance id so the harness logs are
+    # greppable by instance.
+    assert INSTANCE in a
+    assert INSTANCE in b
+
+
+def test_same_cell_same_instance_is_stable():
+    """Resume semantics: re-running the same cell must hit the cache."""
+    assert _build_run_id(INSTANCE, CELL_A) == _build_run_id(INSTANCE, CELL_A)
+
+
+def test_legacy_no_cell_name_preserves_old_format():
+    """Single-cell callers (no ``cell_name``) keep the legacy ``oj-<id>``."""
+    assert _build_run_id(INSTANCE, None) == f"oj-{INSTANCE}"
+    assert _build_run_id(INSTANCE, "") == f"oj-{INSTANCE}"
+
+
+@pytest.mark.parametrize(
+    "cell_name",
+    [
+        CELL_A,
+        CELL_B,
+        "minions-qwen27b-opus47-swe-n100",
+        # Hostile inputs — must still produce something path-safe.
+        "weird/cell name with spaces",
+        "cell:with:colons",
+        "---leading-and-trailing---",
+    ],
+)
+def test_run_id_is_filesystem_safe(cell_name: str):
+    """Whatever we feed in, the run_id must be a clean path component."""
+    rid = _build_run_id(INSTANCE, cell_name)
+    assert _SAFE_RE.match(rid), f"run_id {rid!r} contains unsafe chars"
+
+
+def test_sanitizer_strips_unsafe_chars():
+    assert _sanitize_run_id_part("a/b c:d") == "a-b-c-d"
+    assert _sanitize_run_id_part("---x---") == "x"
+    assert _sanitize_run_id_part("ok-cell.1_2") == "ok-cell.1_2"
+
+
+def test_scorer_threads_cell_name_into_run_id(monkeypatch):
+    """End-to-end wiring: scorer constructor → ``_run_harness`` → run_id.
+
+    Guards against a refactor that quietly drops the ``cell_name`` kwarg
+    on the scorer or stops forwarding it to ``_run_harness``.
+    """
+    from openjarvis.evals.core.types import EvalRecord
+    from openjarvis.evals.scorers import swebench_harness as mod
+
+    captured: dict = {}
+
+    def fake_run_harness(instance_id, patch, timeout_s, cell_name=None):
+        captured["instance_id"] = instance_id
+        captured["cell_name"] = cell_name
+        captured["run_id"] = mod._build_run_id(instance_id, cell_name)
+        return {"success": True, "score": 1.0, "details": {}}
+
+    monkeypatch.setattr(mod, "_run_harness", fake_run_harness)
+
+    scorer = SWEBenchHarnessScorer(timeout_s=60, cell_name=CELL_A)
+    record = EvalRecord(
+        record_id=INSTANCE,
+        problem="",
+        reference="",
+        category="agentic",
+        metadata={"instance_id": INSTANCE},
+    )
+    # Minimal patch text that ``extract_patch`` will accept.
+    answer = "```diff\ndiff --git a/x b/x\n--- a/x\n+++ b/x\n@@ -0,0 +1 @@\n+x\n```"
+    scorer.score(record, answer)
+
+    assert captured["cell_name"] == CELL_A
+    assert captured["instance_id"] == INSTANCE
+    assert CELL_A in captured["run_id"]
+    assert INSTANCE in captured["run_id"]
+
+    # And the other cell name produces a *different* run_id end-to-end.
+    captured.clear()
+    scorer_b = SWEBenchHarnessScorer(timeout_s=60, cell_name=CELL_B)
+    scorer_b.score(record, answer)
+    assert captured["cell_name"] == CELL_B
+    assert CELL_B in captured["run_id"]
+    assert CELL_A not in captured["run_id"]