fix(native-eval): separate execution validity from diagnostic rewards - #73
fix(native-eval): separate execution validity from diagnostic rewards#73steipete wants to merge 1 commit into
Conversation
Integrate PR #64 on current main and retain its structured outcomes, run acceptance, legacy classification, and fail-closed recovery. Archive the terminal job exit code before packaging so recovery can prove completion. Keep the first exception for diagnostics while classifying later verifier or infrastructure failures for acceptance. Cover real shell exports for exits 0 and 2, post-agent failures, aggregation validity, and explicit versus unknown recovery status. Fixes #63 Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs real behavior proof before merge. Reviewed August 28, 2026, 5:21 AM ET / 09:21 UTC. ClawSweeper reviewWhat this changesThe PR adds structured native-evaluation execution outcomes, rejects wholly invalid runs from scoring eligibility, archives terminal exit status, and expands related tests. Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked until stronger real behavior proof is added - 5 items remain Keep open: an introduced P2 classification-precedence defect can hide a later verifier or infrastructure failure behind OpenClaw exit 70/71, and the external PR lacks inspectable real-behavior proof. Priority: P2 Review scores
Verification
How this fits togetherNative evaluation executes agent tasks, captures verifier rewards and artifacts, then aggregates them into leaderboard eligibility. Fleet recovery uses exported archives and an explicit exit status to determine whether a remote run completed. flowchart LR
A[Native task plan] --> B[Harness execution]
B --> C[Trial result and verifier reward]
C --> D[Execution outcome classification]
D --> E[Run acceptance decision]
E --> F[Aggregate leaderboard]
B --> G[Remote artifact archive]
G --> H[Fleet recovery status]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Classify the final verifier or infrastructure exception before applying the legacy 70/71 harness mapping, cover that exact sequence, and attach redacted live validation evidence before merge. Do we have a high-confidence way to reproduce the issue? Yes, by source: an agent exit of 70/71 followed by a verifier or infrastructure exception reaches the new classifier with both values, and its early exit-code branch returns harness_error rather than the later failure. Is this the best way to solve the issue? No: the legacy 70/71 mapping must apply only when the final exception is the agent-exit error, or after later verifier and infrastructure failures have been classified. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against 643c395f6793. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Replaced by the rebased branch (see the new PR) — branch protection blocked force-pushing the rebase here. Same reviewed integration, now on current main. |
Reviewed integration of #64: aggregation no longer retains fully invalid agent-exit runs, recovery no longer infers exit zero without an archived exit status — plus two additional defects found in review (missing exit status in final archives; later execution failures hidden behind earlier agent errors). 134 focused + 471 full-suite tests pass; real shell exports preserved exits 0 and 2; the aggregation CLI retained diagnostic score 0.5 while rejecting the invalid run. Credit to @vincentkoc for the original repair in #64. Fixes #63.