Validate runtime repeatability - #1247
burtenshaw wants to merge 10 commits into
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Seed check fails on unrelated replays
- Seed control now evaluates only the baseline and different-seed replay evidence, with regression coverage for unrelated replay failures.
- ✅ Fixed: Replay timeouts fail determinism
- Failed replay samples now count as incomplete while primary collection failures still fail, with regression coverage for timeout evidence.
Or push these changes by commenting:
@cursor push 6ad5aac095
Preview (6ad5aac095)
diff --git a/src/openenv/validation/graders/runtime/repeatability.py b/src/openenv/validation/graders/runtime/repeatability.py
--- a/src/openenv/validation/graders/runtime/repeatability.py
+++ b/src/openenv/validation/graders/runtime/repeatability.py
@@ -110,9 +110,12 @@
def check(self, subject, evidence):
problems = []
original_seed = None
- for index, sample in enumerate(
- [evidence] + [replay.evidence for replay in evidence.replays]
- ):
+ samples = [(0, evidence)] + [
+ (index, replay.evidence)
+ for index, replay in enumerate(evidence.replays, 1)
+ if replay.scope == "seed"
+ ]
+ for index, sample in samples:
if sample.failure_reason or sample.telemetry_error:
problems.append(f"replay {index}: reset or telemetry collection failed")
continue
@@ -129,7 +132,7 @@
continue
if index == 0:
original_seed = seed
- elif evidence.replays[index - 1].scope == "seed" and seed == original_seed:
+ elif seed == original_seed:
problems.append(f"replay {index}: scheduled seed was not changed")
observed = _telemetry(sample).get("seed")
if (
@@ -213,11 +216,13 @@
}
for index, sample in enumerate(samples):
if sample.failure_reason:
- return (
- CheckStatus.FAIL,
- [f"replay {index}: collection failed"],
- measured,
- )
+ if index == 0:
+ return (
+ CheckStatus.FAIL,
+ [f"replay {index}: collection failed"],
+ measured,
+ )
+ continue
if not any(row.operation == "step" for row in sample.exchanges):
return (
CheckStatus.SKIP,
@@ -229,7 +234,9 @@
)
judged = subject.manifest.capabilities.llm_judged
required = JUDGED_REPLAYS if judged else 3
- if len(samples) < required or {row.scope for row in replays} != {
+ if measured["completed_replays"] < required or {
+ row.scope for row in replays
+ } != {
"session",
"container",
}:
diff --git a/tests/test_validation/test_runtime_repeatability.py b/tests/test_validation/test_runtime_repeatability.py
--- a/tests/test_validation/test_runtime_repeatability.py
+++ b/tests/test_validation/test_runtime_repeatability.py
@@ -124,6 +124,20 @@
)
+def test_seed_control_ignores_unrelated_replay_failure(tmp_path):
+ subject = subject_with_replays(tmp_path)
+ replays = list(subject.runtime_evidence.replays)
+ replays[0] = replace(
+ replays[0],
+ evidence=replace(
+ replays[0].evidence, failure_reason="step failed (TimeoutError)"
+ ),
+ )
+ evidence = replace(subject.runtime_evidence, replays=tuple(replays))
+ result = SeedControlGrader().run(replace(subject, runtime_evidence=evidence))
+ assert result.status is CheckStatus.PASS
+
+
@pytest.mark.parametrize(
"field,value", [("trajectory", None), ("schema_version", True)]
)
@@ -201,6 +215,25 @@
assert result.measured["completed_replays"] == 19
+def test_failed_replay_is_incomplete(tmp_path):
+ subject = subject_with_replays(tmp_path)
+ replays = list(subject.runtime_evidence.replays)
+ replays[1] = replace(
+ replays[1],
+ evidence=replace(
+ replays[1].evidence, failure_reason="step failed (TimeoutError)"
+ ),
+ )
+ evidence = replace(
+ subject.runtime_evidence,
+ replays=tuple(replays),
+ replay_failure_reason="fresh container replay failed (RuntimeError)",
+ )
+ result = EpisodeDeterminismGrader().run(replace(subject, runtime_evidence=evidence))
+ assert result.status is CheckStatus.SKIP
+ assert result.measured["completed_replays"] == 2
+
+
@pytest.mark.parametrize(
"low,bound,status", [(0.5, 0.1, CheckStatus.PASS), (0.0, 0.2, CheckStatus.FAIL)]
)You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Alignment Review Report
Two-tier review of the runtime-repeatability slice (RFC 008 PR5). Scoped to the PR diff f1575e9…96f3fc6e.
Automated Checks
- Lint: FAIL —
ruff format --checkwould reformat 1 file:src/openenv/validation/providers/docker.py:212(the newcapabilities = frozenset({…})block).usort check✅ andruff check✅ both pass. (Thelint.shhook itself errored only becauseuvwasn't on PATH in this runner; I installeduvand ran the three underlying commands directly.) - Debug code: CLEAN — no
print/breakpoint/pdb/TODOin any changed file. Thecheck-debug.shhits are all pre-existing docstrings and CLI output in unrelated modules (auto/,cli/,core/,harbor/), none in this diff. - Tests (supporting): full
tests/test_validationsuite passes locally — 476 passed, 27 skipped (the Docker/network integration cases), covering the 99 new/changed non-Docker cases.
Open RFCs Context
- RFC 008 — Environment Auto-Validation · Status In Review · author @zkwentz. This PR is slice PR5 ("Runtime replay evidence") of the RFC 008 stack and also edits the RFC's own prose. No other open RFC (010/011/012 …) touches this area.
Tier 1: Fixes Required
-
src/openenv/validation/providers/docker.py:212—ruff formatviolation on the newfrozenset({…})set literal. As-is this fails the CI lint gate. Fix:uv run ruff format src/openenv/validation/providers/docker.py(see inline comment for the exact reformatted block).
Tier 2: Alignment Discussion
Principle Conflicts
None identified. The slice stays inside existing boundaries:
- Dual API boundary (INVARIANTS → Architectural #1): the replay/telemetry path is orchestrator-only — session telemetry is gated behind
OPENENV_VALIDATION_TOKENon the simulation/wsconnection and is never exposed as an MCP/agent tool; the fresh-container replay mints a new token per container (replay.py). No simulation control leaks to the agent surface. - Rewards inside environment (PRINCIPLES / INVARIANTS #3): the judged-determinism check only reads environment-emitted rewards and computes population variance over them (reward-squared units); it introduces no external reward computation.
- Client–server separation: changes are confined to the validation orchestration layer; no client↔server imports are added.
RFC Conflicts
ALIGNMENT FLAG: PR rewrites normative prose of an In-Review RFC
- Principle/RFC at stake: RFC 008 (In Review)
- The concern: This PR renames "Later runtime evidence contracts" → "Runtime replay evidence (PR5)" and rewrites that section to describe the shipped behavior (300 s shared deadline, 32 MiB retention cap, session/container/seed scopes, exactly 20 judged samples incl. the baseline, no volatile-field exclusions, and "the protocol authorizes …" replacing "a future protocol slice must …"). The implementation matches the amended prose, so this is not a code↔spec conflict — but because RFC 008 is still In Review and owned by another author, the wording change is worth an explicit ack from the RFC owner rather than landing implicitly inside an implementation PR.
- Suggested reviewer: @zkwentz (RFC 008 author) for the prose change; @Darktex (Davide Testuggine — author of the dual-API-boundary & rewards invariants this path exercises) for the boundary check.
Summary
- 1 mechanical issue to fix (ruff format — Tier 1)
- 0 principle conflicts
- 1 RFC point to confirm with the owner (In-Review RFC 008 prose update)
Sent by Cursor Automation: Pre-review
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Cleanup failure skips independent graders
- Runtime graders now inspect retained evidence before replay cleanup failure changes startup to ERROR, with regression coverage confirming dependent graders execute.
Or push these changes by commenting:
@cursor push 34c64e6954
Preview (34c64e6954)
diff --git a/src/openenv/validation/runner.py b/src/openenv/validation/runner.py
--- a/src/openenv/validation/runner.py
+++ b/src/openenv/validation/runner.py
@@ -189,9 +189,6 @@ def _runtime(subject, *, skip_build, provider):
capabilities=manifest.capabilities,
deadline=replay_deadline,
)
- if any(replay.cleanup_complete is False for replay in evidence.replays):
- result.status = CheckStatus.ERROR
- result.evidence.append("replay subject teardown failed")
subject = replace(
subject, image_ref=image_ref, running=running, runtime_evidence=evidence
)
@@ -189,9 +189,6 @@ def _runtime(subject, *, skip_build, provider):
capabilities=manifest.capabilities,
deadline=replay_deadline,
)
- if any(replay.cleanup_complete is False for replay in evidence.replays):
- result.status = CheckStatus.ERROR
- result.evidence.append("replay subject teardown failed")
subject = replace(
subject, image_ref=image_ref, running=running, runtime_evidence=evidence
)
@@ -208,6 +205,9 @@ def _runtime(subject, *, skip_build, provider):
provider_capabilities=provider.capabilities,
prior=[result],
)
+ if any(replay.cleanup_complete is False for replay in evidence.replays):
+ result.status = CheckStatus.ERROR
+ result.evidence.append("replay subject teardown failed")
except UnsupportedCapability as exc:
result = _outcome(
"runtime.startup", CheckStatus.SKIP, str(exc), started=started
@@ -208,6 +205,9 @@ def _runtime(subject, *, skip_build, provider):
provider_capabilities=provider.capabilities,
prior=[result],
)
+ if any(replay.cleanup_complete is False for replay in evidence.replays):
+ result.status = CheckStatus.ERROR
+ result.evidence.append("replay subject teardown failed")
except UnsupportedCapability as exc:
result = _outcome(
"runtime.startup", CheckStatus.SKIP, str(exc), started=started
diff --git a/tests/test_validation/test_runtime_execution.py b/tests/test_validation/test_runtime_execution.py
--- a/tests/test_validation/test_runtime_execution.py
+++ b/tests/test_validation/test_runtime_execution.py
@@ -462,6 +462,10 @@ def replays(*args, **kwargs):
next(row for row in result.results if row.check_id == "runtime.startup").status
is CheckStatus.ERROR
)
+ if not interrupted:
+ checks = {row.check_id: row for row in result.results}
+ assert checks["runtime.reward_well_formed"].status is CheckStatus.PASS
+ assert "completed_replays" in checks["runtime.episode_determinism"].measured
assert json.loads((bundle / "cleanup.json").read_text()) == {
"required": True,
"completed": False,
@@ -462,6 +462,10 @@ def replays(*args, **kwargs):
next(row for row in result.results if row.check_id == "runtime.startup").status
is CheckStatus.ERROR
)
+ if not interrupted:
+ checks = {row.check_id: row for row in result.results}
+ assert checks["runtime.reward_well_formed"].status is CheckStatus.PASS
+ assert "completed_replays" in checks["runtime.episode_determinism"].measured
assert json.loads((bundle / "cleanup.json").read_text()) == {
"required": True,
"completed": False,You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 2ba5adb. Configure here.
There was a problem hiding this comment.
Please fix one issue before merge.
_difference() in src/openenv/validation/graders/runtime/repeatability.py:50 copies subject-controlled JSON keys into the public divergence path. Object keys in reset, step, and state payloads may contain credentials, private text, newlines, or other diagnostic control characters.
A minimal probe against this head, _difference({"private-subject-key": 1}, {}), returns $.private-subject-key, and that string reaches the public report. Please replace raw keys with a non-disclosing path segment, such as a redacted or hashed value, and add a regression for a mismatched secret-bearing key.
At 9f9b780d, the focused repeatability, replay, execution, artifact, process, and CLI suite otherwise passed with 119 tests passing and 15 skipped. Ruff, Ruff format, usort, and git diff --check passed for the changed files. CI is green at this head, including Linux Docker.
RFC 008 is still in review, so the RFC or core owner should also confirm the replay procedure and prose changes.
|
Addressed the diagnostic-key review in 26f6bb1. Both replay and trajectory mismatches now report field ordinals without exposing subject-provided key text; four regressions cover secret/control-character keys. All 12 CI checks pass at this head. Fresh HF Jobs and Linux Docker CI evidence was independently verified: 479 fast, 27 protocol and 19 Docker tests, zero failures or skips. Ready for re-review; RFC/core-owner signoff remains outstanding. |
|
The stack-review cleanup finding is fixed in 7dc4d96. Determinism now requires confirmed fresh-container cleanup before PASS; failed or unknown cleanup yields SKIP. Malformed evidence, divergence and excessive judged variance still produce FAIL, and cleanup failure keeps the overall run failed. 129 focused tests pass. Fresh HF Jobs evidence is independently verified: 494 fast and 27 protocol tests, zero failures or skips. Linux Docker CI also passed all 19 Docker cases; its artifacts and exact source hashes are independently verified. All 12 CI checks are green at this head. |
This branch has not been deployed
2 participants
This PR checks seed handling, fresh-session and fresh-container replay, and subject-emitted records. Part 5 of #1177; builds on #1246.