Add structural MCP eval loop (v0) - #6
Conversation
A hermetic, rule-graded eval for the gateway servers built on this
toolkit. It reads a live server's own wire surface as the spec
(tools/list + the describe action), generates deterministic, seedable
natural-language scenarios weighted toward the destructive and
idempotent classes, asks a cheap model to pick {tool, action, params},
and grades by rule: exact tool+action match, JSON-Schema param
validation, and a read-only safety check. No product backend, judge, or
cassette.
Because it speaks each catalog's own vocabulary over the wire, it is
product-agnostic and inherited by every server for free. It drives the
in-process fake catalog (CI, zero spend) or a real product stdio server
(fizzy shown), with the server run structurally-only against no backend.
Frugality is a first-class metric: the report prints per-scenario tokens
and a total dollar cost on every run, computed deterministically from the
prompt and answer at a published price table.
Includes generator + grader tests, a hermetic end-to-end smoke, a cheap
CI workflow with no live model calls, the cached fizzy corpus, and the
captured v0 run (12/12 pass, safety respected, ~$0.016).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Pull request overview
Adds a provisional structural MCP evaluation loop that derives specs from gateway servers, generates scenarios, evaluates model routing, and reports accuracy and cost.
Changes:
- Adds spec discovery, scenario generation, prompting, grading, and reporting.
- Supports oracle, Claude CLI, and Anthropic API backends.
- Adds fixtures, tests, documentation, and a hermetic CI smoke workflow.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Makefile |
Adds the eval smoke target. |
.github/workflows/eval.yml |
Runs the hermetic smoke in CI. |
eval/README.md |
Documents architecture, usage, results, and deferred work. |
eval/spec.go |
Derives action specifications over MCP. |
eval/scenario.go |
Generates deterministic weighted scenarios. |
eval/prompt.go |
Builds prompts and parses proposals. |
eval/model.go |
Implements model backends, token estimation, and pricing. |
eval/grade.go |
Grades routing, parameters, and safety. |
eval/run.go |
Coordinates evaluations and records results. |
eval/report.go |
Produces JSONL and aggregate reports. |
eval/fake.go |
Provides the in-process fake gateway. |
eval/eval_test.go |
Tests generation, grading, parsing, cost, and smoke behavior. |
eval/cmd/eval/main.go |
Adds the eval command-line runner. |
eval/testdata/scenarios/fizzy.json |
Captures the Fizzy scenario corpus. |
eval/results/fizzy-v0.jsonl |
Records the captured Fizzy run. |
Suppressed comments (2)
eval/scenario.go:188
syntheticValuehas noobjectcase, so a required object-valued parameter falls intodefaultand receives a string; the generated gold proposal then fails this package's own object type validation. Generate an object (and nil for a null schema) explicitly.
default:
eval/README.md:104
- Deferring Basecamp and HEY leaves this shared toolkit package proven against only Fizzy. That conflicts with the repository's two-instance proof/extraction rule; demonstrate the duplicated loop in at least two product servers before extracting it here, or keep v0 product-local until that proof exists.
- **The other two servers** (basecamp, hey) — same loop, their catalogs; and
**catalog-SHA regression triggers** computed from the toolkit's snapshot
testdata, so the eval reruns when a catalog changes.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31fb498b67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Package eval is a structural evaluation loop for the MCP gateway servers | ||
| // built on this toolkit (basecamp/hey/fizzy). It reads a live server's own | ||
| // wire surface — the tool listing and the gateway describe payloads — as the | ||
| // specification, generates deterministic natural-language scenarios from it, | ||
| // asks a cheap model to pick the right {tool, action, params}, and grades the |
There was a problem hiding this comment.
The new shared eval framework is demonstrated only with Fizzy—the committed corpus/results are Fizzy-specific, while the README explicitly defers Basecamp and HEY—so this lands a toolkit abstraction before two product instances have proven it by duplication. Keep the implementation in the product until a second instance exists, then extract the duplicated design.
AGENTS.md reference: AGENTS.md:L8-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining this one, with reasoning here so it isn't re-litigated.
The rule this cites guards against prematurely abstracting duplicated code before a second instance proves the shape. That's not what this is: the eval couples to the toolkit's own gateway contract — tools/list + the reserved describe action + the annotation surface — which every server built on this toolkit already emits by construction. It imports no product; SpecFromSession reads the wire, so there is nothing product-specific to duplicate or later de-duplicate.
It's also already exercised by two independent catalog instances, not one: the in-process fake catalog (the CI smoke + unit tests) and the real fizzy stdio server. Only the cached corpus and results are fizzy-specific, and those are namespaced (testdata/scenarios/fizzy.json, results/fizzy-v0.jsonl) exactly so basecamp/hey drop in beside them.
And the alternative the rule would point to — keep it in a product until a second appears — is the more-coupled option here: the harness lives below the products, so putting it inside fizzy would force hey and basecamp to import fizzy to reuse it (a module cycle against the toolkit). The brief's repo decision weighed this and chose in-toolkit deliberately. Happy to revisit if the coupling ever reaches into a product, but today it points the other way.
…l typing, mutation in write scenarios - Price the oracle at zero and stop fabricating a Haiku cost for unknown labels, so the zero-spend smoke reports $0.00 (costOf falls back to Haiku only for a genuinely unknown paid label). - On a failed model call, record only backend-reported usage, never a prompt-size estimate: a call that never reached a model is not paid. - JSON Schema type "null" now rejects any non-nil value instead of passing as unconstrained. - Write scenarios now name at least one body field, so an update is framed with a real change to make, not just its ids. - Correct the README cost wording ($0.0165 is under two cents) and refresh the captured run. Regenerated the fizzy corpus and re-ran: still 12/12 pass, safety respected, $0.0165.
…ing, safety-on-error - Preserve non-string enum constraints: ParamSpec.Enum is now []any, the prompt renders every member, and the grader validates by JSON-value equality — an integer or boolean enum now actually constrains. - Pin temperature 0 on API requests, matching the reproducible v0 config. - Generate an object value for object-typed params so a catalog with a required object param does not spuriously fail the oracle. - Parse --server-cmd with quote awareness and reject an empty command, so a quoted path with spaces launches and a blank value cannot panic. - Keep framings unique at generation, so the framing-keyed oracle never maps two distinct actions to one answer. - Treat an errored or unparseable cell as safe (no action was proposed), so a transient backend failure no longer lowers the safety rate. Added tests for non-string enums, object generation, null typing, oracle/unknown pricing, framing disambiguation, and command splitting. The fizzy corpus is byte-identical after these changes, so the captured v0 run stands.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a959b32f61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| valid, reasons := validateParams(spec, p.Params) | ||
| r.ParamsValid = valid |
There was a problem hiding this comment.
The new corpus now names concrete mutation values, but this call only validates p.Params against the action schema and never compares them with s.GoldParams. For the updated update_column scenario, a matching tool/action with different IDs—or only the required IDs and no requested color—still receives ParamsValid and a score of 1, so the headline rates can credit a call that ignores the requested mutation or targets the wrong resource.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Grading now requires the proposal to reproduce the requested values, not just be schema-valid: honorsGold (6b5d45a) fails a call that keeps only the ids, targets a different resource, or supplies a different value — compared in JSON value space so a gold int matches a decoded float64. This supersedes the old "validate against schema, not these values" note in scenario.go, which contradicted the mutation-value change (the framing now names concrete values). Recommendation: keep it — high confidence — because without it the mutation-value work is inert and params_match credits calls that ignore the request. Leaving this unresolved for your confirmation since it reverses a documented v0 decision; the code is in and green (TestGradeRequiresRequestedValues covers drop/wrong-value/wrong-resource).
Addresses the second Codex pass over the eval package: - grade.go: params_match now requires the proposal to reproduce the values the framing named (not just be schema-valid), so a call that keeps only the ids or targets a different resource no longer passes. Read-only-framed requests must resolve to a read-only action — any write, not just a destructive one, is now a safety violation. - scenario.go: colliding framings disambiguate with a neutral ordinal instead of the gold action name, which the catalog prompt exposes and would have leaked the answer. - prompt.go: proposal parsing scans every top-level brace block and returns the first that decodes, so an earlier aside in prose no longer masks the answer. - run.go / cmd: cached corpora carry their server; the CLI refuses to grade a corpus against a different live catalog. - Makefile / cmd: eval-smoke runs with --require-pass so a regression that makes the deterministic oracle emit FAIL/ERR? turns the smoke red.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b5d45a4a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| p, ok := PricingFor(label) | ||
| if !ok { | ||
| p = pricingTable["haiku"] | ||
| } |
There was a problem hiding this comment.
When --models contains a custom CLI/API model ID, costOf silently charges it at Haiku rates, so the report's advertised measured cost can substantially understate or overstate the experiment. Fresh evidence in this revision is that although PricingFor now rejects unknown labels, this caller immediately restores the Haiku fallback; reject unsupported pricing labels or require an explicit price instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not writing a third fix here — flagging the pattern instead. This is the third pass on one mechanism: unknown-label pricing (djBSy from Copilot, djCQf from Codex, now this). The prior rounds landed a deliberate, documented choice: PricingFor returns (Pricing, bool) so any caller CAN detect an unknown label and refuse it, while costOf keeps a Haiku floor purely so a custom model's cost line reads as a plausible non-zero figure instead of $0.00 (rationale is in the costOf doc comment). Codex wants the opposite — a hard reject. That is a genuine design call, not a bug: reject-unknown gives an honest error but breaks any ad-hoc '--models some-new-id' run until the table is updated; floor-to-Haiku keeps such runs working at a known-wrong-but-flagged price. Recommendation (medium confidence): for v0 keep the floor but make it visible — stamp the record/report as estimated pricing when the label is unknown, rather than silently substituting Haiku. That closes the understates/overstates concern without failing exploratory runs. Leaving this for @jeremy since it reverses a documented choice; I have deliberately not written a third selector on it.
| // The framing names the concrete ids and mutation values, so a correct answer | ||
| // carries them; a call that supplies only the required ids, or names a | ||
| // different resource, is schema-valid but has not done what was asked. The | ||
| // proposal may add other declared params — this checks only that the requested |
There was a problem hiding this comment.
When a proposal reproduces every gold value but also invents additional declared parameters, honorsGold still accepts it and the record can receive a full score. For an update request that names only one mutation, the model can therefore add another optional field and change data the user never mentioned; read actions can likewise add filters that alter the result set. Require proposal parameters to match the requested set rather than treating arbitrary declared extras as correct.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Recommendation, not a fix — this tightens the grading semantics I just introduced in honorsGold (thread on grade.go:80), which is itself still open for your confirmation, so I do not want to stack a second unilateral grading change on top. The finding is real: honorsGold checks gold is a subset of the proposal, so a model can add an optional field the request never mentioned (a stray mutation on a write, an extra filter on a read) and still score. But requiring an EXACT param set has real false-positive risk: a model that adds a schema default, a pagination cursor on a read, or an idempotency token would FAIL despite doing what was asked. Recommendation (medium confidence): do not require exact-set in v0. If we want to catch unrequested mutations, extend the SAFETY dimension instead — an unrequested body param on a write is an over-action — while leaving unrequested query/path params (filters, pagination) alone. That is a cleaner split than reject-all-extras. Bundling this with the honorsGold decision since they are the same knob: say how strict you want v0 params grading and I will set both.
Strict null grading (typeMatches("null") == nil) needs a matching generator:
a required null-typed param fell through syntheticValue's default to a string,
so the gold proposal failed its own validation. Emit nil and render it as
"null" in the framing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 619de0d77b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if corpus.Server != "" && corpus.Server != *server { | ||
| return fmt.Errorf("corpus %s was generated for server %q but --server is %q", *scenPath, corpus.Server, *server) | ||
| } | ||
| scenarios = corpus.Scenarios |
There was a problem hiding this comment.
When a valid corpus contains "scenarios": [], unmarshalling produces a non-nil empty slice, so both buildModels and Run skip generation, the oracle emits zero records, and --require-pass succeeds because FailingRecords is empty. Fresh evidence beyond the earlier smoke-check fix is that its new failure condition is vacuously satisfied when no cells execute; reject an empty loaded corpus so a corrupted or accidentally emptied cache cannot produce a green eval without evaluating anything.
Useful? React with 👍 / 👎.
What this is
The PROVISIONAL v0 MCP eval loop — a runnable end-to-end slice, not a plan. It reads a live gateway server's own wire surface as the spec, generates deterministic scenarios, asks a cheap model to pick
{tool, action, params}, grades by rule, and prints a scored table with a measured cost.Repo decision: in the toolkit as an
eval/package, per the brief — the catalog is the spec and it lives here, so the eval speaks each catalog's{action, params}vocabulary over the wire and is inherited by all three servers (basecamp / hey / fizzy) for free. No new repo, no catalog duplication.Scope (v0): structural-only — no backend, no judge, no cassettes. Proves the loop turns on the free deterministic layer.
The loop
spec.go) —tools/list+ the gatewaydescribeaction yield each action's identity, safety annotations, and param/body schema. Served from the catalog, never the product API, so it is hermetic.scenario.go) — a pure function of(specs, seed, N), sampling distinct actions weighted toward the destructive and idempotent classes, rendering an NL framing + gold resolution. Cached, hand-checkable corpus attestdata/scenarios/fizzy.json.model.go,prompt.go) — static catalog as the (cacheable) system prompt, the framing as the user turn. Backends:oracle(deterministic, no spend),cli(localclaude, no API key),api(Anthropic Messages API, exact usage).grade.go) — exact tool+action match, JSON-Schema param validation, and the read-only safety rule (a read/lookup framing must never resolve to a destructive action).report.go) — scenario×model table, per-model pass/params/safety rates, total$. Cost is deterministic (tokens × published price table), so the frugality metric is reproducible and free to compute.The captured v0 fizzy run
go run ./eval/cmd/eval --server fizzy --server-cmd "…/fizzy-mcp stdio --writes" --backend cli --models haiku --scenarios eval/testdata/scenarios/fizzy.json→eval/results/fizzy-v0.jsonl:~16 cents proves the loop turns and the cost story is real. A capable model clears this small, unambiguous corpus cleanly — v0 proves the machinery, not model discrimination (that's the hillclimb).
Frugality
Tokens and dollars print on every run. The only spend in the whole loop is the per-scenario model turn; spec, generation, grading, and reporting are free. CI (
make eval-smoke+eval.yml) runs the loop against the in-process fake with the oracle backend: no live model calls, no network, zero cost.Tests / CI
Generator tests (determinism, seed sensitivity, distinct-action sampling, gold validity) + grader tests (every dimension, types, enum, safety) + a hermetic end-to-end smoke, all in the normal
make test. Neweval.ymlruns the runnable smoke command with no model calls.Deferred (hillclimb, each independent)
Harder scenarios with distractors · a live-API layer (cassettes → real calls, grading the dispatched result) · an LLM judge · prompt caching on the API backend · the other two servers + catalog-SHA regression triggers · a gated tiny real-model CI set.
Caveat, flagged honestly: the captured run's answers came through the local
claudeCLI, which in this environment routes thehaikualias to a stronger model; the dollar figure is priced at Haiku list price as the intended cheap-model cost, and the input tokens are the uncached per-scenario upper bound (prompt caching, a hillclimb item, collapses them). Theapibackend records exact usage when a key is present.