feat(fred): Harbor eval benchmark and the merge gate - #42
Merged
Conversation
The design doc said a benchmark was worth adding once the tool surface settled. It has. Ten tasks over all five tools, following plugins/tastytrade/evals/ rather than inventing a second pattern. Two rewards per task: outcome (the answer is right) and process (it came through mcp__fred__*). The split exists because a tastytrade gate run once produced a perfect answer without calling a single tool, reading the mock's source off disk instead, and scored a clean 1.0 on outcome alone. This plugin has three ways round the server rather than tastytrade's two: the mock's port, the fixtures on disk, and the real FRED API, which is reachable because the benchmark runs with the network up for the agent's own model. Each gets a row in the reward matrix, and require-local-api refuses to start the server unless FRED_BASE_URL points at localhost, so a gate run cannot spend a real key. rate-history-max is the task that earns its keep: the fixture puts the maximum of a long daily series on a single day the downsampler does not sample, so the summary says 300.0 while the best returned point is 199.96, against a tolerance of 0.01. Reading the points is wrong by a hundred. An earlier fixture had a broad peak that the sampling grid happened to land on, which made the task prove nothing; a test now fails if that regresses. Two prerequisite bugs fixed along the way: - FRED_BASE_URL was documented in .env.example, never read by client.py, and absent from .mcp.json. All three now agree, and a test pins the passthrough set so an env var the code honours cannot silently do nothing once installed. - The mock was an httpx transport with no way to serve it. Routing is now a plain route() function with the transport and a stdlib HTTP server as thin callers, so unit tests, integration tests and the benchmark cannot disagree about what FRED returns. Verified: make validate-tasks is 70 passed, 0 failed, and the container answers all ten tasks correctly over stdio through the real launcher.
Harbor aggregates every trial into one mean per reward name, so a clean 10-task run reported "2 task(s)". That reads like eight tasks went missing, at exactly the moment someone is looking for a reason the gate failed.
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
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.
Closes #41. On top of #40.
The design doc said a benchmark was worth adding once the tool surface had settled. It has: five tools, all merged in the #30 stack. This adds the agent-loop evals and makes them a merge gate, following
plugins/tastytrade/evals/rather than inventing a second pattern.Note: this adds a token-spending CI job (
evals), gated behindcheckand restricted to same-repo PRs, billing to the Claude subscription exactly as tastytrade's does.Why the 291 unit tests were not enough
They prove the shaping is right and the correction table is right. They cannot prove an agent can drive the server, which is a different question with its own failure modes. Tastytrade's harness was built by hitting them: an agent that produced a perfect answer without ever calling a tool, four trials that loaded a schema and then never called it, two that delegated to a subagent whose calls
trajectory.jsondoes not record at all.Two rewards per task
outcomeis the answer.processis whether it came throughmcp__fred__*.This plugin has three ways round the server, not tastytrade's two:
:8080, not a hostname. The mock binds every interface, so it answers onlocalhost,127.0.0.1,0.0.0.0,[::1]and the container name; naming two lets the other three throughnetwork_mode: publicis required for the agent to reach its own modelThe third is new. Tastytrade's mock-only setup never had to consider it. It is also why
require-local-apiexists: the server refuses to start unlessFRED_BASE_URLpoints at localhost, verified to exit 1 against the real API host, so a gate run cannot spend a developer's real key even if one leaked into the environment.The task that earns its keep
rate-history-maxasks for the highest value a long daily series ever reached. The fixture puts that maximum on a single day at an index the downsampler does not sample:An agent reading the returned points is wrong by a hundred. This is the central claim of
get_observations(the summary covers every observation, the points are only a sample) turned into pass or fail.The first version of this task proved nothing. The fixture had a broad triangular peak and the sampling grid landed exactly on it, so reading the points gave the right answer too. Caught by checking rather than assuming, and
test_the_extremes_are_not_in_the_returned_pointsnow fails if it ever regresses.Two prerequisite bugs, both real
FRED_BASE_URLwas documented in.env.example, never read byclient.py, and absent from.mcp.json. Shipped that way in feat(fred): plugin scaffold, API client, guided errors, CI #36. All three now agree, and a test pins the passthrough set, because an env var the code honours but the plugin config drops does nothing once installed, which is the same class of bug.route(path, params) -> (status, body)with the transport and a stdlib HTTP server as thin callers. No new dependency, and the benchmark and the test suite cannot disagree about what FRED returns.Fixture changes
CPIAUCSLgains a real 26-month series, because year-over-year needs an observation twelve months back and a six-point series has none, sounits="yoy"would otherwise have been indistinguishable from passing no units at all. The mock now applies the units transforms for real rather than echoing the code back.Release dates are generated relative to a clock rather than written down. A calendar whose whole job is "what came out and what is next" stops straddling today the moment a hard-coded date ages, and CI runs on whatever day it runs. Tests pin the clock; the container uses the real one.
next-release's check and oracle both compute the date at run time, and the check accepts today's and yesterday's answer since a trial starting before midnight is graded after it, which the container run actually exercised.Verification
Everything below was run, not assumed.
The bench image, driven over stdio through the real
mcp-serverwrapper against the mock served over HTTP:Safety guard:
The
evalsgate itself has not been run locally, since it spends subscription tokens; the CI job on this PR is its first real run.Also noted, not fixed
start="5y"means five years ago, and there is no spelling for "the next 60 days", so a forward window needs an absolute end date. The calendar's default window already straddles today, which covers the common question. Recorded in the design doc's deferred work rather than expanded into this PR.