Skip to content

fix(eval): stabilize GPT-5.5 native harness runs - #43

Draft
vincentkoc wants to merge 1 commit into
feat/native-shellbenchfrom
fix/native-gpt55-high
Draft

fix(eval): stabilize GPT-5.5 native harness runs#43
vincentkoc wants to merge 1 commit into
feat/native-shellbenchfrom
fix/native-gpt55-high

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

What does this PR do?

Stacks on #42 and upstreams the native-runner fixes proven by the July 28, 2026 full-suite OpenClaw and Hermes gpt-5.5 runs. Evidence is published in https://github.com/ShellBench/public-tasks/pull/40.

Why?

The first attempts exposed four concrete runner defects: GPT reasoning effort was not enforced or audited, OpenClaw could leave embedded gateways alive after a completed agent run, Hermes exported the wrong/empty session set, and images without an explicit WORKDIR were forced into /app. Superseded runs also needed an explicit aggregate exclusion path.

Changes

  • require low, medium, or high reasoning metadata in planned GPT runs and preserve it across fleet reruns
  • apply GPT reasoning effort in LiteLLM and record agent/judge effort in manifests
  • terminate OpenClaw after its final completion marker when child gateways keep the CLI alive
  • export the exact Hermes session with --yes --redact
  • discover /app, /workspace, or the container default when no image WORKDIR is configured
  • classify nonzero setup exits as infrastructure and honor explicit leaderboard exclusions
  • add focused regression coverage and an unreleased changelog entry

Tests

  • python -m pytest -q tests/test_native_eval_proxy.py tests/test_native_eval_runner.py tests/test_native_eval_fleet.py tests/test_native_eval_aggregate.py (54 passed)
  • python -m ruff check on all touched native runner and test files
  • bash -n scripts/native_eval/remote_run.sh
  • shellcheck scripts/native_eval/remote_run.sh
  • generated OpenClaw and Hermes setup/run/cleanup commands pass bash -n -c
  • git diff --check

@vincentkoc vincentkoc self-assigned this Jul 28, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. labels Jul 28, 2026
@clawsweeper

clawsweeper Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed July 27, 2026, 10:46 PM ET / July 28, 2026, 02:46 UTC.

ClawSweeper review

What this changes

This branch carries GPT-5.5 native-evaluation runner fixes for reasoning-effort metadata, OpenClaw shutdown, Hermes session export, container work-directory discovery, and leaderboard exclusion handling.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep this draft PR open. It is a member-authored follow-on to the still-open native matrix runner in #42, so it is neither implemented on current main nor eligible for conservative cleanup; the implementation also needs one release-owned changelog adjustment.

Priority: P2
Reviewed head: 822e8f9198b3f91cf3273c381d1181c0dd1aae1a

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused and accompanied by targeted tests, but it remains a draft stacked on an unmerged parent and needs an integration refresh.
Proof confidence 🌊 off-meta tidepool Not applicable: This is a member-authored PR, so the external-contributor proof gate does not apply; its claimed full-suite evidence should still be refreshed after the parent branch is integrated.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This is a member-authored PR, so the external-contributor proof gate does not apply; its claimed full-suite evidence should still be refreshed after the parent branch is integrated.
Evidence reviewed 4 items Stacked implementation dependency: The PR body states that this branch stacks on the open native matrix runner PR; the supplied metadata confirms its base is that branch rather than current main.
Current-main status: The supplied PR metadata identifies current main as 28ce18e, while the native runner itself is introduced by the separate open parent PR with 21 changed files and 7,867 additions. This follow-on work is therefore not already implemented on current main.
Focused patch coverage: The branch updates native-evaluation tests alongside planning, fleet dispatch, proxy, runtime, harness, manifest, and aggregation behavior; the PR body reports 54 focused pytest cases plus lint and shell validation.
Findings 1 actionable finding [P3] Remove the release-owned changelog entry
Security None None.

How this fits together

ShellBench’s native-evaluation subsystem turns a planned model-and-harness matrix into remote runs, then records manifests and aggregates results for ranking. This PR changes the planning, runtime, harness, and aggregation stages so GPT native runs preserve their intended execution settings and produce more reliable artifacts.

flowchart LR
  A[Run plan] --> B[Fleet dispatcher]
  B --> C[Remote native runner]
  C --> D[Model proxy]
  C --> E[Agent harnesses]
  D --> F[Run manifest]
  E --> F
  F --> G[Result aggregation]
  G --> H[Leaderboard report]
Loading

Before merge

  • Remove the release-owned changelog entry (P3) - CHANGELOG.md is release-owned under the review policy, so this normal implementation PR should leave release-note selection and wording to the release process. Keep the user-visible summary in the PR body or commit message instead.
  • Resolve merge risk (P1) - This branch is based on the still-open draft implementation in feat(eval): add auditable native matrix runner #42, so its final merge result and native-run behavior need a refresh after that parent lands or changes.
  • Complete next step (P2) - Review the stacked native-runner change after its parent PR lands; the remaining action is integration validation and ordinary member-authored PR review, not a safe automated repair.

Findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7-8
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 235 added, 9 removed, 12 files affected The changes span the end-to-end native-evaluation path, so the stacked base and final integration result matter more than isolated unit coverage.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Land and validate the native matrix runner first, then rebase this focused follow-on branch onto current main, remove the release-owned changelog edit, and rerun the documented native harness validation against the rebased result.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land and validate the native matrix runner first, then rebase this focused follow-on branch onto current main, remove the release-owned changelog edit, and rerun the documented native harness validation against the rebased result.

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

Unclear. The PR describes concrete July 28, 2026 full-suite failures and provides focused tests, but the linked live-run artifacts were not available for direct inspection in this review.

Is this the best way to solve the issue?

Yes, with one adjustment. Carrying the fixes as a focused follow-on to the native matrix runner is coherent, but it should be rebased after the parent PR lands and should not modify the release-owned changelog.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7-8
    CHANGELOG.md is release-owned under the review policy, so this normal implementation PR should leave release-note selection and wording to the release process. Keep the user-visible summary in the PR body or commit message instead.
    Confidence: 0.93

Overall correctness: patch is correct
Overall confidence: 0.72

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against 28ce18e5dfc8.

Labels

Label changes:

  • add P2: This fixes evaluation-run integrity and cleanup behavior with a bounded blast radius in the native harness subsystem.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a member-authored PR, so the external-contributor proof gate does not apply; its claimed full-suite evidence should still be refreshed after the parent branch is integrated.

Label justifications:

  • P2: This fixes evaluation-run integrity and cleanup behavior with a bounded blast radius in the native harness subsystem.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a member-authored PR, so the external-contributor proof gate does not apply; its claimed full-suite evidence should still be refreshed after the parent branch is integrated.

Evidence

What I checked:

  • Stacked implementation dependency: The PR body states that this branch stacks on the open native matrix runner PR; the supplied metadata confirms its base is that branch rather than current main. (scripts/native_eval/fleet.py:1, 682b6c7847b4)
  • Current-main status: The supplied PR metadata identifies current main as 28ce18e, while the native runner itself is introduced by the separate open parent PR with 21 changed files and 7,867 additions. This follow-on work is therefore not already implemented on current main. (scripts/native_eval/plan.py:1, 28ce18e5dfc8)
  • Focused patch coverage: The branch updates native-evaluation tests alongside planning, fleet dispatch, proxy, runtime, harness, manifest, and aggregation behavior; the PR body reports 54 focused pytest cases plus lint and shell validation. (tests/test_native_eval_runner.py:1, 822e8f9198b3)
  • Repository-policy check: A read-only repository inspection command was blocked by the execution sandbox before it could determine whether a target AGENTS.md exists, so no repository-authored policy was applied beyond the supplied review contract.

Likely related people:

  • vincentkoc: The supplied history shows this person authored both the parent native matrix runner branch and this focused follow-on commit, making them the clearest available routing contact for the stacked implementation. (role: native-evaluation feature contributor; confidence: medium; commits: 682b6c7847b4, 822e8f9198b3; files: scripts/native_eval/fleet.py, scripts/native_eval/runtime.py, scripts/native_eval/harnesses.py)

Rank-up moves

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant