feat(eval): freeze cell identity and resume semantics before stable cut

docs/phase-eval-cell-identity.md

@@ -0,0 +1,95 @@
+# Eval cell identity and resume semantics
+
+This document is the contract for `brigade.eval_cell.v1` receipts produced by
+`src/brigade/model_trials.py`. It freezes the rules that become compatibility
+surfaces at the stable cut: how `cell_id` is derived, what `execute --resume`
+does for every recorded state, and how stale cells are handled.
+
+## Cell identity
+
+`cell_id` is the sha256 hex digest of a canonical JSON identity payload
+(`json.dumps` with sorted keys, `(",", ":")` separators, UTF-8) built in
+`expand_cells`. Exactly these fields participate, and nothing else:
+
+- `schema` — the `CELL_SCHEMA` tag (`brigade.eval_cell.v1`).
+- `case.id` — the case identifier from the manifest.
+- `case.prompt` — the inlined prompt text, after line-ending normalization
+  (see below).
+- `seat.seat` — the seat name.
+- `seat.cli` — the agent's CLI adapter.
+- `seat.model` — the pinned model.
+- `seat.reasoning` — the reasoning setting.
+- `seat.transport` — the transport, if any.
+- `seat.transport_version` — the transport version, if any.
+- `seat.env` — the agent's environment map, if any.
+- `seat.codex_transport` — the roster codex transport; set for codex seats
+  only, otherwise `null`.
+- `trial` — the 1-based trial number.
+- `graders` — the grader list for the case.
+- `execution_mode` — `read-only` or `writable-worktree`.
+
+The `coordinate` (`case:seat:trial`) is the human-stable axis and is **not**
+part of the identity payload; it is how staleness is detected (below).
+
+### Line-ending normalization
+
+Before prompt text (inline or from `prompt_file`) enters the identity payload,
+`\r\n` and bare `\r` are normalized to `\n`, so the same logical manifest
+hashes identically across checkouts with different line-ending conventions.
+This changes `cell_id` only for manifests that contained CRLF or CR line
+endings; that breakage is accepted because it lands before the stable cut.
+
+### Changing the identity payload
+
+Any change to the field set above — adding, removing, or reinterpreting a
+field — requires bumping `CELL_SCHEMA` and writing a migration note in this
+document. The identity lock test in `tests/test_model_trials.py` snapshots the
+exact payload keys and the resulting digest, so an accidental change fails CI.
+
+## Resume semantics
+
+`execute --resume` rebuilds the plan from the current manifest and decides per
+cell from the recorded `cell.json`:
+
+- `accepted`, `rejected`, `unscored`, `execution_error`, `adapter_error`,
+  `grader_error` (the terminal states): the cell is **skipped**; the existing
+  receipt stands.
+- `running`: the cell **re-runs as a new attempt**. `running` means the
+  previous process died mid-run (or, without a lock, is still executing in
+  another process). Resume treats it as a crash and starts the next attempt,
+  preserving the original `started_at`.
+- Missing, unreadable, or corrupt `cell.json`: the cell runs as a new attempt.
+- Any other state value: the cell re-runs (only exact terminal-state
+  membership causes a skip).
+
+Attempt numbers are `max(existing attempt numbers under attempts/) + 1`,
+tolerating gaps and non-`attempt-NNN` directories; a deleted attempt directory
+never causes a number to be reused.
+
+Every `cell.json` — both the `running` marker written before the run and the
+final receipt — records `manifest_digest`, the canonical digest of the
+manifest that produced the plan, so each cell stays attributable to its
+generation even after later manifest edits.
+
+## Stale cells
+
+A cell is stale when its `coordinate` exists in the previous `plan.json` with
+a different `cell_id` (for example after a manifest edit). Staleness is
+computed against the immediately previous `plan.json` only.
+
+The policy is **keep and report, no pruning**:
+
+- Stale cell directories are left on disk untouched.
+- The new `plan.json` lists them under `stale_cells` with the previous and
+  current ids.
+- `summarize` excludes them from the headline counts and reports them under
+  `stale_counts`.
+- On `execute --resume` with stale cells present, a one-line stale count is
+  printed to stderr.
+
+## Concurrency
+
+Two concurrent `execute` processes on one output directory are not guarded:
+both see a `running` cell and interleave writes. A lockfile-based guard is
+tracked separately and deliberately not part of this freeze; until it lands,
+do not run concurrent executes against the same output directory.

src/brigade/model_trials.py

@@ -73,17 +73,21 @@ def load_manifest(path: Path) -> dict[str, Any]:
     return data
 
 
+def _normalize_prompt(text: str) -> str:
+    return text.replace("\r\n", "\n").replace("\r", "\n")
+
+
 def _case_prompt(case: dict[str, Any], base_dir: Path) -> str:
     prompt = case.get("prompt")
     prompt_file = case.get("prompt_file")
     if isinstance(prompt, str) and prompt:
-        return prompt
+        return _normalize_prompt(prompt)
     if isinstance(prompt_file, str) and prompt_file:
         candidate = (base_dir / prompt_file).resolve()
         if base_dir.resolve() not in candidate.parents and candidate != base_dir.resolve():
             raise ValueError(f"case {case.get('id')!r} prompt_file escapes the manifest directory")
         try:
-            return candidate.read_text()
+            return _normalize_prompt(candidate.read_text())
         except OSError as exc:
             raise ValueError(f"case {case.get('id')!r} prompt_file unreadable: {exc}") from exc
     raise ValueError(f"case {case.get('id')!r} needs prompt or prompt_file")
@@ -322,10 +326,19 @@ def _trial_worktree_path(
     )
 
 
+_ATTEMPT_DIR = re.compile(r"attempt-(\d+)")
+
+
 def _attempt_number(cell_dir: Path) -> int:
     attempts = cell_dir / "attempts"
-    existing = [p for p in attempts.iterdir() if p.is_dir()] if attempts.is_dir() else []
-    return len(existing) + 1
+    if not attempts.is_dir():
+        return 1
+    numbers = [
+        int(match.group(1))
+        for entry in attempts.iterdir()
+        if entry.is_dir() and (match := _ATTEMPT_DIR.fullmatch(entry.name)) is not None
+    ]
+    return max(numbers, default=0) + 1
 
 
 def _load_json(path: Path) -> dict[str, Any] | None:
@@ -385,6 +398,11 @@ def execute(
         print("error: writable-worktree trials require a git worktree target", file=sys.stderr)
         return 2
     localio.write_json(output_dir / "plan.json", plan)
+    if resume and plan["stale_cells"]:
+        print(
+            f"note: {len(plan['stale_cells'])} stale cell(s) from the previous plan kept and counted in summary",
+            file=sys.stderr,
+        )
     failures = 0
     for cell in cells:
         cell_dir = output_dir / "cells" / cell.cell_id
@@ -406,6 +424,7 @@ def execute(
                 "state": "running",
                 "attempt": attempt,
                 "started_at": started_at,
+                "manifest_digest": plan["manifest_digest"],
             },
         )
         run_dir = cell_dir / "attempts" / f"attempt-{attempt:03d}" / "run"
@@ -456,6 +475,7 @@ def execute(
             "state": state,
             "attempt": attempt,
             "started_at": started_at,
+            "manifest_digest": plan["manifest_digest"],
             "exit_code": rc,
             "duration_seconds": run_meta.get("duration_seconds"),
             "run_dir": str(run_dir),

tests/test_model_trials.py

@@ -28,6 +28,61 @@ def _roster() -> Roster:
     )
 
 
+def test_cell_identity_payload_is_locked():
+    # Snapshot of the frozen identity contract (docs/phase-eval-cell-identity.md).
+    # Any change to the field set below must come with a CELL_SCHEMA bump.
+    manifest = {
+        "schema": "brigade.eval_manifest.v1",
+        "name": "identity-lock",
+        "trials": 1,
+        "seats": ["cursor"],
+        "cases": [{"id": "hello", "prompt": "Say hello"}],
+        "graders": [{"type": "exact_output", "expected": "hello"}],
+    }
+    cell = model_trials.expand_cells(manifest, _roster())[0]
+    expected_identity = {
+        "schema": "brigade.eval_cell.v1",
+        "case": {"id": "hello", "prompt": "Say hello"},
+        "seat": {
+            "seat": "cursor",
+            "cli": "cursor",
+            "model": "composer-2.5",
+            "reasoning": None,
+            "transport": "direct",
+            "transport_version": None,
+            "env": None,
+            "codex_transport": None,
+        },
+        "trial": 1,
+        "graders": [{"type": "exact_output", "expected": "hello"}],
+        "execution_mode": "read-only",
+    }
+    assert model_trials._canonical_digest(expected_identity) == cell.cell_id
+    assert cell.cell_id == "55c07e87f401b5aa49f956b2cec1bfee87986088702bc0b1762cc722ff2638c1"
+
+
+def test_prompt_line_endings_do_not_change_identity():
+    crlf = _manifest()
+    crlf["cases"][0]["prompt"] = "Say hello\r\nagain\rnow"
+    unix = _manifest()
+    unix["cases"][0]["prompt"] = "Say hello\nagain\nnow"
+    crlf_cells = model_trials.expand_cells(crlf, _roster())
+    unix_cells = model_trials.expand_cells(unix, _roster())
+    assert [cell.cell_id for cell in crlf_cells] == [cell.cell_id for cell in unix_cells]
+    assert crlf_cells[0].prompt == "Say hello\nagain\nnow"
+
+
+def test_attempt_number_uses_max_plus_one_and_tolerates_gaps(tmp_path):
+    assert model_trials._attempt_number(tmp_path) == 1
+    attempts = tmp_path / "attempts"
+    attempts.mkdir()
+    (attempts / "attempt-001").mkdir()
+    (attempts / "attempt-003").mkdir()
+    (attempts / "scratch").mkdir()
+    (attempts / "attempt-002-partial").mkdir()
+    assert model_trials._attempt_number(tmp_path) == 4
+
+
 def test_expand_cells_is_stable_and_conditions_change_identity():
     first = model_trials.expand_cells(_manifest(), _roster())
     second = model_trials.expand_cells(_manifest(), _roster())
@@ -228,6 +283,92 @@ def fake_run(task, roster, **kwargs):
     assert summary["stale_counts"] == {"accepted": 2}
 
 
+def test_resume_after_manifest_edit_reruns_only_changed_cells(tmp_path, monkeypatch, capsys):
+    manifest = _manifest()
+    manifest["trials"] = 1
+    manifest["cases"] = [
+        {"id": "alpha", "prompt": "Say alpha"},
+        {"id": "beta", "prompt": "Say beta"},
+    ]
+    manifest_path = tmp_path / "eval.json"
+    manifest_path.write_text(json.dumps(manifest))
+    tasks: list[str] = []
+
+    def fake_run(task, roster, **kwargs):
+        tasks.append(task)
+        out = kwargs["output_dir"]
+        out.mkdir(parents=True, exist_ok=True)
+        (out / "final.txt").write_text("hello\n")
+        (out / "run.json").write_text(json.dumps({"status": "ok", "duration_seconds": 0.5}))
+        return 0
+
+    monkeypatch.setattr(model_trials.aboyeur, "run", fake_run)
+    root = tmp_path / "results"
+    assert model_trials.execute(manifest_path, _roster(), workspace=tmp_path, output_dir=root, resume=False) == 0
+    assert sorted(tasks) == ["Say alpha", "Say beta"]
+
+    original_ids = {cell.case_id: cell.cell_id for cell in model_trials.expand_cells(manifest, _roster())}
+    alpha_path = root / "cells" / original_ids["alpha"] / "cell.json"
+
+    edited = json.loads(manifest_path.read_text())
+    edited["cases"][1]["prompt"] = "Say beta differently"
+    manifest_path.write_text(json.dumps(edited))
+    tasks.clear()
+    capsys.readouterr()
+    assert model_trials.execute(manifest_path, _roster(), workspace=tmp_path, output_dir=root, resume=True) == 0
+
+    # The unchanged cell is skipped; the edited cell re-runs under a new cell_id.
+    assert tasks == ["Say beta differently"]
+    alpha = json.loads(alpha_path.read_text())
+    assert alpha["state"] == "accepted"
+    assert alpha["attempt"] == 1
+    edited_ids = {cell.case_id: cell.cell_id for cell in model_trials.expand_cells(edited, _roster())}
+    assert edited_ids["alpha"] == original_ids["alpha"]
+    assert edited_ids["beta"] != original_ids["beta"]
+    new_beta = json.loads((root / "cells" / edited_ids["beta"] / "cell.json").read_text())
+    assert new_beta["state"] == "accepted"
+    plan = json.loads((root / "plan.json").read_text())
+    assert new_beta["manifest_digest"] == plan["manifest_digest"]
+
+    # The old cell is kept and reported, not pruned; resume warns on stderr.
+    assert (root / "cells" / original_ids["beta"] / "cell.json").is_file()
+    summary = json.loads((root / "summary.json").read_text())
+    assert summary["counts"] == {"accepted": 2}
+    assert summary["stale_counts"] == {"accepted": 1}
+    assert "1 stale cell(s)" in capsys.readouterr().err
+
+
+def test_resume_reruns_killed_running_cell_as_new_attempt(tmp_path, monkeypatch):
+    manifest = _manifest()
+    manifest["trials"] = 1
+    manifest_path = tmp_path / "eval.json"
+    manifest_path.write_text(json.dumps(manifest))
+
+    def fake_run(task, roster, **kwargs):
+        out = kwargs["output_dir"]
+        out.mkdir(parents=True, exist_ok=True)
+        (out / "final.txt").write_text("hello\n")
+        (out / "run.json").write_text(json.dumps({"status": "ok", "duration_seconds": 0.5}))
+        return 0
+
+    monkeypatch.setattr(model_trials.aboyeur, "run", fake_run)
+    root = tmp_path / "results"
+    assert model_trials.execute(manifest_path, _roster(), workspace=tmp_path, output_dir=root, resume=False) == 0
+    cell_path = next((root / "cells").glob("*/cell.json"))
+
+    # Simulate a kill mid-run: the last durable state is "running".
+    killed = json.loads(cell_path.read_text())
+    killed["state"] = "running"
+    cell_path.write_text(json.dumps(killed))
+
+    assert model_trials.execute(manifest_path, _roster(), workspace=tmp_path, output_dir=root, resume=True) == 0
+    final = json.loads(cell_path.read_text())
+    assert final["state"] == "accepted"
+    assert final["attempt"] == 2
+    attempts = sorted(p.name for p in (cell_path.parent / "attempts").iterdir())
+    assert attempts == ["attempt-001", "attempt-002"]
+
+
 def test_grader_envelope_links_digested_output(tmp_path, monkeypatch):
     manifest_path = tmp_path / "eval.json"
     manifest_path.write_text(json.dumps(_manifest()))