feat(evals): split the reward into outcome and process - #26
Merged
Conversation
Every task scored one reward: is the answer right. That is not enough to gate an MCP plugin. The mock brokerage listens on localhost:8080 inside the container and its source sits in the checkout, so an agent can produce a perfect answer without ever calling a tool. A real gate run did exactly that. place-limit-order scored 0.0 only because the order never landed; the trajectory showed the agent searched for the MCP tools, never called them, read mock_api/app.py off disk, and drove the REST API with urllib. Had it written the right record it would have scored a clean 1.0 while never touching the server. The same hole let earnings-implied-move pass without loading the skill, with a trajectory of nothing but Read and Write. Each task now reports two rewards via rewardkit, following harbor-hub: outcome the answer is right process it came through the plugin For the twelve tool tasks `process` wants a mcp__tastytrade__* call and nothing reaching the mock directly. For the skill task it wants the skill or its script. Both fail closed: no trajectory is no evidence, which is also why the oracle scores outcome=1, process=0. validate_local.sh is rewritten around that. It used to run the oracle and check for reward 1, which cannot express the case that matters. It now scores every real verifier against three trajectories and asserts the matrix: solved oracle answer, intended route -> 1, 1 empty no answer, no trajectory -> 0, 0 bypassed oracle answer, round the server -> 1, 0 39 assertions, and writing them caught two bugs in the first draft of the checks I would not have found by reading. An empty run scored 0.5 because "did not bypass the server" is vacuously true when there are no tool calls at all. And the skill check matched the reference chain's own path, which contains the skill's name, so reading the input file counted as using the skill. It runs in the bench image now: rewardkit does not build on macOS, and scoring the same image CI runs means the verifier under test is the one that will grade a real run. So `make validate-tasks` needs Docker.
The first gate run under the split reward scored `outcome` 1.0 on all thirteen tasks and lost `process` on six: one at 0.0 and five at 0.5. Every one of those prompts said only "use the Tastytrade MCP tools" and never put anything out of bounds. `place-limit-order`, the single task that already spelt the rule out, scored a clean 1.0. So the prompt now says what the reward measures, on every task. The wording is `place-limit-order`'s, generalised: the tools are already connected, the work has to go through them, an answer reached any other way does not count, and a tool error is to be retried rather than worked around. It deliberately avoids the host, port, and module name the bypass check greps for, so an agent that echoes its instructions into a shell comment cannot fail the check by quoting it. `earnings-implied-move` gets the same move in its own terms: hand arithmetic on the chain is off the table, and finding the right skill is still the agent's job, since that is what the task measures. The bypass pattern now matches the mock's port rather than a list of hostnames. It binds every interface, so it answers on localhost, 127.0.0.1, 0.0.0.0, [::1], and the container's own name; naming two of those let the other three through, and a bypass that scores as good behaviour is worse than no check at all. `validate_in_container.sh` grows a fourth case per task to hold that shut: a bypass spelt the other way, which for the skill task means inline arithmetic that leaves no distinctive string at all. 52 assertions, up from 39. The skill picks up the question the eval actually asks. Its description covered comparing an implied move to history but not computing one, and the fit already produces exactly that number, separated from the front expiry's ordinary vol. It also now says that a chain file already in the documented shape can be fitted directly, which is the case the eval hands it. Diagnosis, last. The gate used to dump the verifier output with `cat`, which prints thirteen anonymous pairs of numbers: you could see that six tasks lost `process` and not which six, and recovering that meant downloading a CI artifact that expires in seven days. `explain_trials.py` names each trial with its rewards and, for anything short of a perfect `process`, lists the tool calls behind it. That list is the score, so it is the diagnosis. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6DtKz5F3fwaV9Yg2mPnXi
The trials name themselves now, and the second gate run says the failure was
never what it looked like. `process` went 0.731 -> 0.846 and nine tasks are
clean, but the four that still lost it all failed the same way, and it was not
a bypass:
== option-chain-atm: outcome=1.0, process=0.5
5 tool call(s), 0 through the MCP server:
! ToolSearch {"query": "tastytrade", "max_results": 20}
! Bash {"command": "mcp__tastytrade__get_option_chain --symbol SPY ..."}
Every one of the four opened with `ToolSearch {"query": "tastytrade"}` and then
never called a tool. One ran the tool name as a shell command. Two handed the
call to a subagent. `preview-vertical-spread` spent 44 calls trying to reach
the MCP over a socket, a subprocess, and an SDK import -- having already loaded
the schema with `select:mcp__tastytrade__preview_order` -- and hit the agent
timeout, which took `outcome` down with it, 1.0 to 0.0.
None of that is confusion about which tool or which arguments. It is confusion
about how to invoke an MCP tool at all when its schema is deferred rather than
listed, and the previous prompt made it worse: "you do not need to inspect
anything before calling one" is wrong in exactly the case where the agent
cannot see the tools yet. So the prompt now says the true thing. Load the
schema with ToolSearch, then call it the way you call any other tool. No
command, endpoint, or import reaches an MCP tool, and a run that spends its
budget trying will time out.
It also says to make the call rather than delegate it. A subagent keeps its own
transcript and returns only its result, so a delegated call leaves an `Agent`
entry and no tool in the trajectory; two trials fetched the right answer that
way and scored 0.5. That is the right verdict on the evidence rather than a gap
to paper over -- "a delegate says it called the server" is not a record of this
run calling it -- and `used_mcp_server` now carries that reasoning in its
docstring so the next reader does not mistake it for an oversight.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D6DtKz5F3fwaV9Yg2mPnXi
Delegation was scored as a bypass, and it is not one. Two trials handed the
lookup to a subagent, got the right answer through the real tools, and scored
`process` 0.5 because the trajectory showed an `Agent` call and nothing else.
harbor's source says why. `_get_session_dir` builds trajectory.json from the
main session only:
session_dirs = list(
{f.parent for f in jsonl_files if "subagents" not in f.parent.parts}
)
Modern Claude Code writes each subagent's transcript under exactly that
directory, so a delegated call is absent from the trajectory by construction.
The `isSidechain` handling further down is for older CLIs that inlined those
events; with the version CI runs, there is nothing to inline.
The transcripts themselves are right there, under CLAUDE_CONFIG_DIR, which
harbor points at /logs/agent/sessions -- the same mount the verifier reads. So
the checks now take the union of trajectory.json and every session jsonl, and
stop caring who placed the call. Whether the top-level agent called the tool or
routed it through a delegate is the harness's routing decision, not a fact
about this plugin, and the question this gate asks is whether a real agent can
drive the server to the answer. A delegated call is that.
Both criteria read the union, not just the crediting one. Counting a delegated
MCP call while missing a delegated `curl` would turn "ask a subagent" into an
invisible bypass, which is worse than not looking at all. `validate_local.sh`
grows a `delegated` / `delegated-bypass` pair to hold both directions shut,
with the subagent line written where a real one lands so the fixture inherits
the same invisibility. 78 assertions, up from 52.
The prompt keeps its line about calling the tools directly, demoted from a rule
to a preference: it is no longer scored, but a delegated one-line lookup still
spends a whole agent loop against a 300s budget.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D6DtKz5F3fwaV9Yg2mPnXi
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
2 participants
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.
Follow-up to #24, which flagged this.
Why
Every task scored one reward: is the answer right. That cannot gate an MCP plugin. The mock brokerage listens on
localhost:8080inside the container and its source sits in the checkout, so an agent can produce a perfect answer without ever calling a tool.A real gate run did exactly that.
place-limit-orderscored 0.0, but only because the order never landed. The trajectory showed the agent searched for the MCP tools, never called them, readmock_api/app.pyoff disk, and drove the REST API withurllib. Had it written the right record it would have scored a clean 1.0 while never touching the server. The same hole letearnings-implied-movepass without loading the skill, on a trajectory of nothing butReadandWrite.What
Each task now reports two rewards through
rewardkit, following harbor-hub:outcomethe answer is rightprocessit came through the pluginFor the twelve tool tasks,
processwants amcp__tastytrade__*call and nothing reaching the mock brokerage directly. Forearnings-implied-moveit wants the skill or the script it documents. Both fail closed: no trajectory is no evidence, which is also why the oracle scoresoutcome=1, process=0.The gate requires every reward at 1.0, so a bypass now fails the merge.
The validator is where the work is
validate_local.shused to run the oracle and check for reward 1. That cannot express the case that matters. It now scores every real verifier against three synthetic trajectories and asserts the matrix:The third row is the whole point, and it is what
harbor run -a oraclecannot tell you.Writing it paid for itself. It caught two bugs in my first draft of the checks that I would not have found by reading them:
process.earnings-calendars. Merely reading the input counted as using the skill.Verified
make validate-tasks: 39 passed, 0 failed.make check: lint, typecheck, selftest, 94 unit tests.check_reward.py --selftestcovers the new--onlyfilter and the split-reward shape.ruff checkandruff format --checkclean, so a rerun never dirties the tree.The live gate on this PR is the real test, since it is the first time these process checks meet an actual agent trajectory rather than a synthetic one.
Notes
make validate-tasksnow needs Docker. rewardkit does not build on macOS (itslitellmdependency wants a newer rustc than ships there), and scoring inside the bench image means the verifier under test is the one that will really grade a gate run.No version bump: everything here is under
evals/, which the version gate excludes because it is not shipped in the plugin cache.🤖 Generated with Claude Code