fix(validator): failed re-evaluation inherits the previous run's score from a stale results.json (#249)

docs/JOURNAL.md

@@ -18,6 +18,15 @@ protocol. **Newest entries at the top.** Tag each entry with one or more of:
 
 ---
 
+## 2026-07-24 — eval_runner scored a failed re-run from the previous run's results.json  #mistake #gotcha
+**Context:** `evaluate_submission` writes every run of a submission into `data/workspaces/submissions/<submission_id>/`. Submission rows are reused, not recreated — `create_pr_submission` updates the existing row when a miner pushes a new commit to the same PR, so every re-evaluation lands in the same workspace directory.
+**Expected:** an evaluation that produces no `results.json` is reported as failed (that is what `_is_missing_results_payload` and the two `rc != 0 and not results_path.exists()` guards are for).
+**Actual:** the second evaluation of a submission whose run crashed without writing anything was marked `completed` and carried the **first** run's score. The stale `cost_ledger.jsonl` was billed to it too.
+**Root cause:** nothing cleared the workspace between runs. `results.json` from the previous evaluation was still on disk, so `not local_results_path.exists()` was False (no raise), and `_prepare_results` parsed the old payload. The remote path had the same hole one level up: `_remote_attempt` rsyncs the remote workspace back **without `--delete`**, so a stale remote `results.json` would be copied over a freshly cleared local one.
+**Impact:** a wrong score propagates all the way through — `submission.latest_score`, `best_eval_id`, the public leaderboard, `update_king_score`, and the worker's `should_promote_submission` merge/close decision. A miner could bank one good score and have any later broken checkpoint inherit it.
+**Fix / decision:** `_clear_previous_run_files()` unlinks `results.json` and `cost_ledger.jsonl` at the top of `evaluate_submission`, and the remote command prologue now `rm -f`s the remote `results.json` alongside the ledger truncation it already did. Both existing "missing results" guards then work as written. Regression test: evaluate once with a scoring stub, then again with a stub that writes nothing — the second run must fail with `score is None`.
+**Follow-up:** provider-route evaluations are keyed by a unique `EvaluationRun.id`, so their workspace is never reused; left alone.
+
 ## 2026-07-12 — code grader: add resource limits on top of the HOME/secrets fix  #security #decision
 **Context:** issue #71 — the code grader (`run_pass_at_1`) runs untrusted miner/LLM candidate code. The core secret-leak fix (isolated throwaway HOME/cwd, scrubbed env, `python -I`) already landed on main.
 **Finding:** main's sandbox closes the HOME/secrets exfiltration vector but has **no resource limits** — an untrusted candidate can still exhaust host memory or fork-bomb the eval box within its wall-clock timeout (verified: a 4 GiB `bytearray` allocation runs to completion and "passes" on main).

validator/src/eval_backend/services/eval_runner.py

@@ -460,6 +460,21 @@ def _provider_eval_workspace(settings: Settings, evaluation_id: int) -> Path:
     return settings.workspace_root.expanduser() / "provider_evaluations" / str(evaluation_id)
 
 
+def _clear_previous_run_files(*paths: Path) -> None:
+    """Delete run outputs left behind by an earlier evaluation of the same submission.
+
+    Submission workspaces are keyed by submission id and reused on every
+    re-evaluation (pushing a new commit to a submission PR updates the existing
+    row instead of creating a new one). Nothing truncated the previous
+    ``results.json``, so a re-run that produced no results at all still found the
+    old file: the ``rc != 0 and not results_path.exists()`` guards did not fire
+    and ``_prepare_results`` happily parsed the stale payload, reporting the
+    previous run's score for the new checkpoint.
+    """
+    for path in paths:
+        path.unlink(missing_ok=True)
+
+
 def _prepare_results(
     results_path: Path,
     *,
@@ -590,6 +605,10 @@ def _build_remote_command(
     )
     return (
         f"mkdir -p {shlex.quote(str(ledger_path.parent))} && : > {shlex.quote(str(ledger_path))} && "
+        # The remote workspace is reused across evaluations of the same submission
+        # and is rsynced back without --delete, so a stale results.json would be
+        # copied over the freshly cleared local one when this run produces none.
+        f"rm -f {shlex.quote(str(results_path))} && "
         f"export TRINITY_COST_LEDGER={shlex.quote(str(ledger_path))}; "
         f"export TRINITY_REMOTE_DIR={repo_dir}; "
         f"export TRINITY_GPU_INDEX={shlex.quote(str(getattr(settings, 'trinity_gpu_index', 5)))}; "
@@ -876,6 +895,7 @@ def evaluate_submission(
     local_workspace.mkdir(parents=True, exist_ok=True)
     local_results_path = local_workspace / "results.json"
     local_cost_ledger_path = local_workspace / "cost_ledger.jsonl"
+    _clear_previous_run_files(local_results_path, local_cost_ledger_path)
 
     effective_allow_local_fallback = (
         settings.eval_allow_local_fallback if allow_local_fallback is None else allow_local_fallback

validator/tests/test_eval_runner.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+import shlex
 from pathlib import Path
 
 from eval_backend.core.config import Settings
@@ -137,6 +138,65 @@ def _fake_local_attempt(
     assert result.score == 0.88
 
 
+def test_reevaluation_does_not_reuse_previous_results(validator_session, tmp_path, monkeypatch):
+    """A re-run that produces no results must fail, not inherit the old score."""
+    session = validator_session
+    settings = _build_settings(tmp_path)
+    checkpoint_path = tmp_path / "theta.npy"
+    checkpoint_path.write_bytes(b"theta")
+    submission = _add_submission(session, checkpoint_path)
+
+    def _scoring_local_attempt(
+        settings,
+        checkpoint_path,
+        local_results_path,
+        local_ledger_path,
+        submission_id,
+        env,
+    ):
+        local_results_path.write_text(
+            json.dumps({"results": {"TRINITY": {"accuracy": 0.91}}}),
+            encoding="utf-8",
+        )
+        local_ledger_path.write_text(
+            json.dumps({"provider": "chutes", "m": "google/gemma-4-31B-turbo-TEE", "p": 100, "c": 50})
+            + "\n",
+            encoding="utf-8",
+        )
+        return ("fake-eval-command", 0, "stdout", "")
+
+    monkeypatch.setattr(eval_runner, "_local_attempt", _scoring_local_attempt)
+    first = eval_runner.evaluate_submission(session, submission, settings)
+    assert first.score == 0.91
+
+    # The miner pushes a new commit: the same submission row (and therefore the
+    # same workspace) is evaluated again, but this run writes nothing at all.
+    def _crashing_local_attempt(*args, **kwargs):
+        return ("fake-eval-command", 1, "stdout", "boom")
+
+    monkeypatch.setattr(eval_runner, "_local_attempt", _crashing_local_attempt)
+    second = eval_runner.evaluate_submission(session, submission, settings)
+
+    assert second.run.status == "failed"
+    assert second.score is None
+    assert submission.status == "failed"
+    assert submission.latest_score is None
+    assert second.metrics["cost_usd"] == 0.0
+
+
+def test_remote_command_clears_stale_remote_results(tmp_path):
+    settings = _build_settings(tmp_path)
+    results_path = tmp_path / "workspace" / "results.json"
+    command = eval_runner._build_remote_command(
+        settings,
+        tmp_path / "theta.npy",
+        results_path,
+        tmp_path / "workspace" / "ledger.jsonl",
+        tmp_path / "workspace",
+    )
+    assert f"rm -f {shlex.quote(str(results_path))}" in command
+
+
 def test_ledger_cost_report_prices_current_openrouter_models(tmp_path):
     ledger = tmp_path / "cost_ledger.jsonl"
     ledger.write_text(