Skip to content

fix(native-eval): benchmark genuine OpenClaw code mode - #62

Open
vincentkoc wants to merge 5 commits into
mainfrom
fix/openclaw-tool-search-explicit-off
Open

fix(native-eval): benchmark genuine OpenClaw code mode#62
vincentkoc wants to merge 5 commits into
mainfrom
fix/openclaw-tool-search-explicit-off

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jul 29, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Makes the OpenClaw native-eval arms represent genuine direct, directory, and
Code Mode execution, then exports and validates their public trajectory bundles
before a run can be scored.

Why?

Fixes #61.

The earlier code arm selected the legacy tool_search_code bridge rather than
OpenClaw Code Mode. The harness also masked setup failures, forced thinking off,
terminated the runtime before lifecycle cleanup settled, and reconstructed
delegated traces from private session files. Those failures made the released
direct/code comparison invalid and disproportionately erased Code Mode's nested
tool calls.

Changes

  • Map direct, directory, and code to explicit, mutually exclusive
    OpenClaw tool surfaces.
  • Propagate the requested reasoning effort through planning, dispatch,
    OpenClaw defaults, subagents, CLI execution, and manifests.
  • Fail setup immediately and let openclaw agent exit naturally.
  • Export root and child trajectories through
    openclaw sessions export-trajectory.
  • Capture child runs at the canonical pre-cleanup terminal hook, including
    nested and repeated runs, without blocking the Gateway.
  • Validate audit initialization, trace/session identity, event counts, terminal
    status, complete Code Mode snapshots, and the exact provider-visible
    exec/wait surface.
  • Reconstruct Code Mode nested calls and complete session-tree usage from the
    public export bundles.
  • Bound child-export stabilization and reject missing or failed evidence rather
    than publishing partial traces.

Tests

  • Blacksmith Testbox full suite: 454 passed, 5 skipped
  • Blacksmith Testbox focused runner suite after final fixes: 61 passed
  • Ruff clean
  • Python compile, git diff --check, generated shell syntax for all three
    modes, and generated audit-plugin node --check
  • Fresh Codex autoreview: clean
  • Fresh matched direct/Code Mode ShellBench batch

The July 29, 2026 released code result remains legacy Tool Search bridge
evidence only. It is not evidence about genuine OpenClaw Code Mode.

@clawsweeper clawsweeper Bot added 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. 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 29, 2026
@clawsweeper

clawsweeper Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed August 3, 2026, 12:09 AM ET / 04:09 UTC.

ClawSweeper review

What this changes

The branch makes ShellBench explicitly configure OpenClaw direct, directory, and Code Mode runs, export public root and child trajectories, and validate those trajectories before scoring benchmark results.

Merge readiness

⚠️ Ready for maintainer review - 6 items remain

This PR addresses a real ShellBench native-eval correctness problem, but the current head turns two existing legacy configuration inputs into immediate failures without an upgrade path. Because the author is a repository member and the remaining choice is whether to preserve those interfaces or intentionally break them, it should stay open for maintainer direction and a compatibility repair before merge.

Priority: P2
Reviewed head: f5b31cb1260b631b811bf0ede9f4eba51a94b98c
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The branch contains substantial targeted validation and passing CI, but two P1 upgrade-compatibility blockers prevent a merge-ready rating.
Proof confidence 🌊 off-meta tidepool Not applicable: The PR author is a repository member, so the external-contributor proof gate does not apply; however, the PR body still identifies a fresh matched direct/Code Mode batch as pending validation.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The PR author is a repository member, so the external-contributor proof gate does not apply; however, the PR body still identifies a fresh matched direct/Code Mode batch as pending validation.
Evidence reviewed 6 items Legacy manifest configuration is rejected: The proposed fleet-plan validation aborts any nonempty legacy openclaw_tool_search_mode value instead of translating a known existing setting to the new explicit mode.
Legacy environment invocation is rejected: The proposed runner rejects SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE before constructing a run, so existing scripted invocations fail rather than receiving a compatibility translation or deprecation warning.
Current tests encode the breaking behavior: The added tests assert rejection of nonempty legacy manifest and environment values, so green CI does not establish upgrade compatibility.
Findings 2 actionable findings [P1] Preserve retired manifest tool-mode configuration
[P1] Translate the legacy environment tool-mode input
Security None None.

How this fits together

ShellBench’s native evaluator plans benchmark runs, dispatches them to a remote OpenClaw harness, then exports and validates trajectories for scoring. This PR changes the configuration passed through that pipeline and the evidence used to decide whether a run is valid.

flowchart LR
  A[Benchmark plan] --> B[Fleet dispatcher]
  B --> C[Run environment and manifest]
  C --> D[OpenClaw native harness]
  D --> E[Public trajectory exports]
  E --> F[Trajectory validation]
  F --> G[Scored benchmark results]
Loading

Decision needed

Question Recommendation
Should ShellBench preserve recognized openclaw_tool_search_mode manifest and environment values during the migration to openclaw_tool_mode, or intentionally make this a breaking configuration change? Translate recognized legacy values: Map supported legacy values to the equivalent explicit mode, emit a deprecation warning, and keep rejection for unknown values.

Why: The patch mechanically supports compatibility translation, but whether existing benchmark automation may be broken is a maintainer-owned contract decision.

Before merge

  • Preserve retired manifest tool-mode configuration (P1) - Translate recognized openclaw_tool_search_mode values to the corresponding explicit mode with a deprecation warning. Raising here aborts queued existing fleet plans before dispatch, turning a configuration migration into a runtime-breaking upgrade. This repeats the unresolved blocker from the prior review at the same head.
  • Translate the legacy environment tool-mode input (P1) - Accept recognized SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE values long enough to map them to SHELLBENCH_OPENCLAW_TOOL_MODE, then warn. Current scripted benchmark invocations otherwise fail before RunSpec construction; this is the unresolved prior-review blocker at the unchanged head.
  • Resolve merge risk (P1) - Existing fleet manifests containing a recognized nonempty openclaw_tool_search_mode will fail before dispatch after merge.
  • Resolve merge risk (P1) - Existing automation that exports SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE will fail before a benchmark starts; the PR body also leaves a fresh matched direct/Code Mode batch unchecked.
  • Complete next step (P2) - A maintainer must choose the legacy-configuration contract before a narrow compatibility repair can be applied safely.

Findings

  • [P1] Preserve retired manifest tool-mode configuration — scripts/native_eval/fleet.py:447-450
  • [P1] Translate the legacy environment tool-mode input — scripts/native_eval/run_job.py:370-373
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 9 files affected; production +699/-477, tests +1116/-475 The migration changes the runner, fleet dispatch, harness, trajectory export, and a large regression-test surface, so compatibility behavior needs deliberate review.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #61
Summary: This PR is the linked candidate implementation for the open native-eval tool-mode correctness report; it should remain paired with that issue until a compatible fix is merged.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Add a compatibility shim before merge (recommended)
    Translate recognized legacy manifest and environment values to the equivalent explicit tool mode, warn users, and cover both paths with focused tests.
  2. Accept an intentional breaking migration
    Merge the failures only after a maintainer explicitly accepts the upgrade break and the PR documents the required replacement configuration.
  3. Pause for contract direction
    Keep the PR open until the native-eval configuration migration policy is decided rather than landing an implicit breaking change.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Preserve recognized openclaw_tool_search_mode and SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE values through a deprecation translation to openclaw_tool_mode; retain rejection for invalid values and add focused migration tests.

Technical review

Best possible solution:

Preserve recognized legacy tool-mode values through an explicit deprecation translation to the new direct, directory, or Code Mode contract, reject only invalid values, and publish a fresh matched direct/Code Mode batch using the validated public exports.

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

Yes, from source: set a recognized nonempty legacy manifest field or SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE and the proposed validation raises before dispatch or run construction. The issue does not need a live remote benchmark to establish that upgrade regression.

Is this the best way to solve the issue?

No. Explicit direct, directory, and Code Mode selection is the right repair direction, but rejecting known legacy values without translation or explicit maintainer approval is not the narrowest safe migration.

Full review comments:

  • [P1] Preserve retired manifest tool-mode configuration — scripts/native_eval/fleet.py:447-450
    Translate recognized openclaw_tool_search_mode values to the corresponding explicit mode with a deprecation warning. Raising here aborts queued existing fleet plans before dispatch, turning a configuration migration into a runtime-breaking upgrade. This repeats the unresolved blocker from the prior review at the same head.
    Confidence: 0.98
  • [P1] Translate the legacy environment tool-mode input — scripts/native_eval/run_job.py:370-373
    Accept recognized SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE values long enough to map them to SHELLBENCH_OPENCLAW_TOOL_MODE, then warn. Current scripted benchmark invocations otherwise fail before RunSpec construction; this is the unresolved prior-review blocker at the unchanged head.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

  • P2: The patch corrects benchmark-validity behavior but does not indicate an active production outage or security incident.
  • merge-risk: 🚨 compatibility: The proposed validation makes existing nonempty manifest and environment configuration fail before benchmark execution.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The PR author is a repository member, so the external-contributor proof gate does not apply; however, the PR body still identifies a fresh matched direct/Code Mode batch as pending validation.

Evidence

What I checked:

  • Legacy manifest configuration is rejected: The proposed fleet-plan validation aborts any nonempty legacy openclaw_tool_search_mode value instead of translating a known existing setting to the new explicit mode. (scripts/native_eval/fleet.py:447, f5b31cb1260b)
  • Legacy environment invocation is rejected: The proposed runner rejects SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE before constructing a run, so existing scripted invocations fail rather than receiving a compatibility translation or deprecation warning. (scripts/native_eval/run_job.py:370, f5b31cb1260b)
  • Current tests encode the breaking behavior: The added tests assert rejection of nonempty legacy manifest and environment values, so green CI does not establish upgrade compatibility. (tests/test_native_eval_runner.py:687, f5b31cb1260b)
  • Prior blocker remains at the same head: The prior ClawSweeper review identified both compatibility blockers at this exact head; the focused diff from that reviewed SHA to the current head is empty for the affected files. (scripts/native_eval/fleet.py:447, f5b31cb1260b)
  • Current-main ownership provenance: Current main’s related native-eval trajectory implementation was merged as the delegated-trace reconstruction work, and the available path history attributes the central native-eval files to Vincent Koc. (scripts/native_eval/harnesses.py:18, 884dd1bb5511)
  • PR-head provenance: The current head’s latest commit is the Code Mode surface-validation change, dated July 30, 2026; no later PR commit addresses the legacy-input behavior. (scripts/native_eval/harnesses.py:358, f5b31cb1260b)

Likely related people:

  • vincentkoc: Available history shows Vincent Koc authored the current-main delegated-trajectory implementation and every commit in this PR’s native-eval series. (role: native-eval feature owner and recent area contributor; confidence: high; commits: 884dd1bb5511, f5b31cb1260b; files: scripts/native_eval/harnesses.py, scripts/native_eval/fleet.py, scripts/native_eval/run_job.py)

Rank-up moves

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

  • Preserve recognized legacy manifest and environment values with a deprecation translation and focused regression coverage.
  • Run and attach a redacted fresh matched direct/Code Mode batch before treating the new benchmark comparison as publishable evidence.

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 (21 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-01T12:21:15.559Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-01T17:56:46.557Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-01T19:16:46.445Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-02T13:16:59.157Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-02T19:15:27.999Z sha f5b31cb :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T20:38:31.284Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-02T22:18:15.271Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-03T01:49:10.537Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration | [P1] Translate legacy environment-based benchmark invocation

@vincentkoc
vincentkoc marked this pull request as ready for review July 29, 2026 20:56
@vincentkoc
vincentkoc requested a review from a team as a code owner July 29, 2026 20:56
@vincentkoc vincentkoc changed the title fix(native-eval): make OpenClaw Tool Search controls explicit fix(native-eval): benchmark genuine OpenClaw code mode Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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: 🐚 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. and removed 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. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. 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. and removed merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. 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. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(native-eval): make OpenClaw Tool Search controls explicit

1 participant