fix(native-eval): resolve planned reasoning effort before proxy startup - #75
Merged
Merged
Conversation
Integrate PR #54's native harness effort controls and preserve the planned effort in the fleet RunSpec and run manifest. Resolve CLI/planned effort over environment defaults before generating the proxy configuration. Export that canonical value before starting the proxy or runner, and reject conflicting late CLI overrides. Cover all four harnesses through the real remote launcher with controlled proxy and trial processes. Keep event-driven scheduler test waits bounded without treating host scheduling latency as a performance contract. Fixes #53 Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs real behavior proof before merge. Reviewed August 28, 2026, 12:19 PM ET / 16:19 UTC. ClawSweeper reviewWhat this changesThe PR carries planned reasoning effort through native fleet runs, resolves proxy and runner precedence before startup, maps the value to four harnesses, and adds coverage for the launch path. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 3 items remain Keep this PR open: the source changes coherently fix the linked native-evaluation mismatch, but the supplied after-fix evidence is limited to controlled tests with a fake proxy and monkey-patched trial, not a real harness run. Priority: P1 Review scores
Verification
How this fits togetherShellBench’s native-evaluation runner turns a planned model configuration into a LiteLLM proxy plus a chosen agent harness. The resolved reasoning setting must match in the proxy, harness command, and saved run manifest so cross-harness results are comparable. flowchart LR
Plan[Native evaluation plan] --> Fleet[Fleet dispatch]
Fleet --> Launcher[Remote launcher]
Provider[Provider environment defaults] --> Launcher
Launcher --> Resolve[Resolve and validate effort]
Resolve --> Proxy[LiteLLM proxy]
Resolve --> Harness[Agent harness]
Resolve --> Manifest[Run manifest]
Proxy --> Trial[Native evaluation trial]
Harness --> Trial
Before merge
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:
Technical reviewBest possible solution: Add redacted after-fix evidence from real pinned harness launches showing the resolved value in the proxy, each harness invocation, and the resulting manifest, then land this as the canonical fix for the linked issue. Do we have a high-confidence way to reproduce the issue? Yes, from source: the current-main base starts the proxy from environment configuration before constructing a runner that had no reasoning-effort propagation, while OpenClaw remained hard-coded to thinking off. Is this the best way to solve the issue? Yes for the implementation approach: resolving one validated value before startup is the narrowest way to prevent proxy, harness, and manifest drift; real client compatibility proof is still needed. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against 01211da61b64. LabelsLabel changes:
Label justifications:
EvidenceWhat 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
|
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Native plans could say
highwhile harnesses used their defaults (including OpenClaw's hard-coded--thinking off). The candidate fix in #54 added harness controls, but its CLI override could disagree with a proxy already configured from the environment. The remote launcher also let the provider environment file overwrite fleet's planned effort.This integrates @vincentkoc's #54 changes and resolves effort before proxy startup. A preparation invocation validates the complete runner arguments, gives planned/CLI effort precedence over environment defaults, writes the proxy configuration, and returns the canonical value. The launcher exports that value before starting the proxy or runner. Ordinary runner invocations reject conflicting late overrides, and manifests record the RunSpec value. All four harness mappings and unspecified defaults are covered; Claude Code receives
maxfor canonicalxhigh.Regression proof independently reproduced the original proxy=
lowversus harness/manifest=highsplit. The new startup tests run the real Bash launcher, Python preparation/job lifecycle, manifest writer, and tar packaging with controlled proxy/trial processes: 12 successful starts agree across all four harnesses, and four invalid inputs start neither process. They do not make paid inference calls or launch real agent binaries.Two pre-existing event-driven fleet tests also exceeded their two-second waits under heavy host load. Their deadlock/cleanup guards now allow 30 seconds; blocked-job events and every ordering/capacity assertion are unchanged.
Validation on the final tree:
Fixes #53