Skip to content

fix(native-eval): reject invalid harness runs - #64

Draft
vincentkoc wants to merge 1 commit into
mainfrom
fix/native-eval-run-acceptance
Draft

fix(native-eval): reject invalid harness runs#64
vincentkoc wants to merge 1 commit into
mainfrom
fix/native-eval-run-acceptance

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jul 30, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Records structured native-eval execution outcomes and rejects runs where every
trial failed at the harness, infrastructure, or verifier layer.

Why?

Verifier rewards are useful diagnostics, but they are not proof that the
benchmark harness executed correctly. Fully invalid runs were being presented
as completed score data, and fleet recovery could infer success without an
archived exit status.

Fixes #63

Changes

  • classify clean, agent, harness, infrastructure, and verifier outcomes
  • preserve per-trial rewards while recording run-level execution acceptance
  • exclude rejected runs from score-eligible aggregates
  • infer historical OpenClaw exit 70/71 outcomes correctly
  • remove fleet success inference based only on result count and checkpoint text

Tests

  • python -m pytest -q passes on Blacksmith Testbox
  • python -m ruff check clawbench app.py scripts tests passes on Blacksmith Testbox

Focused proof: 117 tests passed; Ruff clean. Autoreview reported no actionable
findings.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 30, 2026
@clawsweeper

clawsweeper Bot commented Jul 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 6, 2026, 9:17 AM ET / 13:17 UTC.

ClawSweeper review

What this changes

The branch records native-evaluation execution outcomes, rejects fully invalid runs from score eligibility, and requires an archived exit status during fleet recovery.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open: current main lacks this distinct native-evaluation fix, and the member-authored PR needs maintainer approval of its intentional stricter recovery behavior.

Priority: P1
Reviewed head: 9f4490e8e4370cbc752404a1becce42d5a6a69f8
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused patch and regression coverage are sound, with the remaining blocker being an intentional recovery-policy decision.
Proof confidence 🌊 off-meta tidepool Not applicable: The author is a repository member, so the external-contributor real-behavior-proof gate does not apply; focused test and lint claims remain supplemental evidence.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The author is a repository member, so the external-contributor real-behavior-proof gate does not apply; focused test and lint claims remain supplemental evidence.
Evidence reviewed 5 items Current main lacks acceptance gating: Current main's eligibility calculation checks completeness, infrastructure dominance, harness-wide failure, identity, and trajectory state, but has no run-acceptance condition.
Proposed implementation: The branch adds persisted execution acceptance and makes it an eligibility condition while retaining diagnostic rewards.
Recovery behavior change: Main inferred a successful exit from full result coverage plus a final checkpoint; the branch removes that inference and only accepts an archived exit status.
Findings None None.
Security None None.

How this fits together

ShellBench runs benchmark tasks through native harnesses, then aggregates trial artifacts into eligibility and score reports. Its fleet controller also recovers remote runs, determining whether archived work is completed, failed, or retried.

flowchart LR
  A[Benchmark task plan] --> B[Native harness run]
  B --> C[Trial results and artifacts]
  C --> D[Execution outcome classification]
  D --> E[Aggregate eligibility decision]
  C --> F[Final exported archive]
  F --> G[Fleet recovery]
  G --> H[Completed or retry state]
Loading

Decision needed

Question Recommendation
Should verified legacy exports without an archived exit status now fail and rerun rather than retain the prior inferred-success behavior? Adopt strict archival proof: Require an archived exit status for completion so uncertain exports retry or fail rather than appear successful.

Why: The patch deliberately changes recovery semantics for existing archives; source review cannot choose the acceptable operational trade-off.

Before merge

  • Resolve merge risk (P1) - Existing full-coverage exports without an archived exit status will now fail and schedule a rerun instead of being inferred successful, potentially increasing fleet work for legacy archives.
  • Complete next step (P2) - A maintainer must choose the intended legacy recovery policy before this member-authored PR can land.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Code delta production +186/-28; tests +93/-7 The added production state supports a narrowly scoped reliability correction and is accompanied by focused regression coverage.
Touched surface 4 production files, 3 test files Aggregation and fleet recovery both change, making upgrade semantics relevant to review.

Merge-risk options

Maintainer options:

  1. Approve strict recovery semantics (recommended)
    Confirm that unknown archived exit status must result in failure and rerun before landing this recovery behavior.
  2. Retain a compatibility fallback
    Restore a documented legacy completion rule if old exported runs must remain complete without archived exit status.

Technical review

Best possible solution:

Adopt explicit execution outcomes and archived-exit recovery after confirming that fail-closed handling of legacy exports is the intended operational policy.

Do we have a high-confidence way to reproduce the issue?

Yes in source: focused fixtures demonstrate a full OpenClaw exit-71 result set with rewards, and current main's eligibility path has no execution-acceptance gate. This read-only review did not run a live harness campaign.

Is this the best way to solve the issue?

Yes: recording execution status at runtime and enforcing it during aggregation avoids using verifier rewards or result count as proof of valid benchmark execution. The recovery-policy change still requires maintainer approval.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 884dd1bb5511.

Labels

Label justifications:

  • P1: The current behavior can publish fully invalid native benchmark runs as eligible score data.
  • merge-risk: 🚨 compatibility: The fleet controller changes completion handling for existing archives that have no recorded exit status.
  • merge-risk: 🚨 availability: The strict recovery path can schedule reruns where prior logic completed the run.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The author is a repository member, so the external-contributor real-behavior-proof gate does not apply; focused test and lint claims remain supplemental evidence.

Evidence

What I checked:

Likely related people:

  • vincentkoc: Authored current main's native-evaluation trace work and this follow-up across the aggregation, runtime, and fleet paths. (role: recent native-evaluation contributor; confidence: high; commits: 884dd1bb5511, 9f4490e8e437, 9afe6a4f1746; files: scripts/native_eval/aggregate.py, scripts/native_eval/runtime.py, scripts/native_eval/fleet.py)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Confirm the strict recovery behavior for legacy exported runs before merge.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (35 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-05T00:21:07.718Z sha 9f4490e :: needs changes before merge. :: [P1] Reject unknown exit status in the checkpoint path
  • reviewed 2026-08-05T08:36:07.196Z sha 9f4490e :: needs changes before merge. :: [P1] Reject unknown exit status in the checkpoint path
  • reviewed 2026-08-05T12:08:16.922Z sha 9f4490e :: needs changes before merge. :: [P1] Reject unknown exit status in the checkpoint path
  • reviewed 2026-08-05T19:41:21.141Z sha 9f4490e :: needs changes before merge. :: [P1] Require archived exit status after the checkpoint loop
  • reviewed 2026-08-05T20:52:00.706Z sha 9f4490e :: needs changes before merge. :: [P1] Reject missing exit status in the normal checkpoint path | [P1] Archive the explicit run status before strict recovery
  • reviewed 2026-08-05T23:38:52.265Z sha 9f4490e :: needs changes before merge. :: [P1] Reject a missing checkpoint exit status | [P1] Archive the run status used for recovery
  • reviewed 2026-08-06T07:18:44.096Z sha 9f4490e :: needs changes before merge. :: [P1] Reject an absent checkpoint exit status
  • reviewed 2026-08-06T10:16:41.240Z sha 9f4490e :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot mentioned this pull request Jul 30, 2026
Open
@clawsweeper clawsweeper Bot added status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P1 Urgent regression or broken agent/channel workflow affecting real users now. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Native eval accepts fully invalid harness runs

1 participant

@vincentkoc