@W-23558172: Enable coding-agent eval harness in main + admin tool-coverage cases#623
@W-23558172: Enable coding-agent eval harness in main + admin tool-coverage cases#623Alon-ST-DATA wants to merge 12 commits into
Conversation
…nia schools data and official test cases
Builds on Joe Constant's `joecon/eval` branch — the eval test infrastructure (coding-agent runner + LangSmith-traced grading) that this work rebases onto current main so the admin/mutation tools are actually in the build under test. Changes: - evals/cases/admin/ — 5 tool-coverage cases for the admin tools: list-users, query-admin-insights (read-only) + update-user, delete-content, update-cloud-extract-refresh-task (mutation, preview-only, zero blast radius). Renamed from cases/mutation/ since the read-only cases are not mutations. - evals/run-case.ts — forward ADMIN_TOOLS_ENABLED / INSIGHTS_TOOLS_ENABLED / FEATURE_GATE_PROVIDER[_CONFIG] to the MCP stdio subprocess; without it the admin tools never register and every admin case fails missing_tools. - evals/ADMIN_EVALS.md — onboarding guide (rebase, gating, two-phase preview strategy, run/grade commands, failure triage). - evals/README.md — npm install deps prerequisite + admin-cases section. - env.example.list — admin gate + eval-target LUID vars. - evals/DESIGN_local-grading.md, DESIGN_eval-probe-helper.md — deferred designs for LangSmith-free tool-coverage grading and an eval:probe helper. Verified: list-users case runs live (agent calls mcp__tableau__list-users, 9 users returned, exit 0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rebased harness carried joecon/eval's stale package-lock.json, which
pinned eslint 9.28.0 and downgraded ~40 other deps vs main. eslint 9.28.0
flags the `declare global { var __telemetryProvider }` idiom in
src/telemetry/init.ts as no-var (that file is unchanged from main and lints
clean under main's eslint 9.39.4).
Restore main's package-lock.json as the base and reconcile with npm install,
so the only net-new dependency is langsmith (pinned ^0.8.3; 0.8.4 is outside
the registry date window). Lock now diverges from main by ~80 lines instead
of ~3900.
Verified: npm run lint clean, npm run build succeeds.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
[Alon's agent] CI status — one failure fixed, one is pre-existing on 1. Lint ( 2. E2E
A fresh build from byte-identical source can't produce a different tool list, so this is snapshot drift between the registered tools and the live E2E test site — a maintainer/fixture issue that is red on This PR stays draft; result reports for the remaining 4 admin cases ( |
|
[Alon's agent] Admin eval results — all 5 cases PASS (full LangSmith grading). Ran the complete
Trace IDs (LangSmith project
Notes:
With this, the eval coverage in the PR is complete: 5/5 admin cases green. The remaining CI note is unchanged — the |
|
[Alon's agent] Note on Working captions confirmed from the successful payload: This is expected agent behavior against an undocumented live schema, so we're leaving the result as-is in this PR (not bumping the budget to paper over it) and tracking the optimization — enrich the case prompt with the TS Events schema hint so the agent goes straight to a working query (~2 calls) — as a separate follow-up: W-23559510 (under epic [266] Tableau MCP for Admins). |
server.test.ts's flowTools filter only stripped list-flows and get-flow when FLOW_TOOLS_ENABLED is unset, but the flow gate group (toolName.ts) also gates list-flow-runs and list-flow-tasks. The default/combined variants therefore expected two flow tools the server never registers, failing the length assertion. Add the two missing names to both filter arrays. Pre-existing failure on main; unblocks build (24.x) on this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Grading correctness: - grade.ts: an errored expected-tool call no longer counts as coverage. Add 'tool_error' outcome; return it when any tool matching expected_tools carries a non-null trace error (e.g. admin-gate rejection) so a rejected call can't false-pass. - run-case.ts: timed_out now reflects an actual runner kill (error.killed / SIGTERM), not merely finishing over budget — the adapter kill fires at maxWallMs + 10s, so wallMs >= maxWallMs was a false positive. Phantom submodule: - Remove the committed evals/bird_mini gitlink (mode 160000, no .gitmodules) that anyone checking out the branch inherited as a broken submodule. gitignore evals/bird_mini/ and document how to obtain the BIRD snapshot. Docs realigned to code: - claude-code-harness.md + env.example.list: the semantic judge runs headless via GRADER_HARNESS/GRADER_MODEL, not the OpenAI API — drop dead OPENAI_API_KEY / BIRD_GRADE_MODEL. Metrics are trace-sourced (total_tokens, not total_context_tokens); output-dir + LangSmith sections match reality; fix "provided at within the project" typo. - ADMIN_EVALS.md: the run-case.ts env passthrough is already on this branch (not a patch to apply); frame the rebase sections as historical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
[Alon's agent] Self code review — findings + fixes (pushed in
|
| # | Finding | Fix |
|---|---|---|
| M1 | Docs + env.example.list describe an OpenAI / GPT-4o-mini judge (OPENAI_API_KEY, BIRD_GRADE_MODEL) — neither var is read anywhere in code. The judge actually runs headless through a coding-agent harness. |
Replaced with the real knobs GRADER_HARNESS / GRADER_MODEL across env.example.list and claude-code-harness.md (Prereqs, Env table, grading signals, verdicts). |
| M2 | evals/bird_mini was committed as a phantom submodule (gitlink mode 160000, no .gitmodules, empty on disk) — a broken checkout for anyone pulling the branch. |
git rm --cached the gitlink; gitignored evals/bird_mini/; documented how to obtain the BIRD Mini-Dev snapshot in evals/README.md + the harness doc. |
| M3 | grade.ts could report pass when an expected tool call actually errored (e.g. admin-gate rejection) — an errored call still satisfied observed ⊇ expected. |
Added a tool_error outcome; the grader now flags any expected-tool call carrying a non-null trace error before returning pass. |
Minor
| # | Finding | Fix |
|---|---|---|
| m4 | ADMIN_EVALS.md §2 instructs the reader to apply a run-case.ts env-passthrough patch that is already applied on this branch (double-apply footgun). |
Reframed §2 as "already wired up"; §1/§5 rebase steps marked historical. |
| m5 | Metrics doc stale: sources listed as hook.jsonl/Claude-stream (now trace-sourced), field total_context_tokens (code emits total_tokens), and output-dir + LangSmith-hooks sections described artifacts no longer written. |
Rewrote to trace-sourced metrics, correct total_tokens, real output dirs, and a grading-requires-a-trace LangSmith section. |
| m6 | timed_out: wallMs >= budget.maxWallMs is a false positive — the adapter only kills at maxWallMs + 10s, so a run that merely finished slightly over budget was mislabeled as a timeout. |
Now keys off an actual runner kill (error.killed / SIGTERM). |
| m7 | ADMIN_EVALS branch-divergence framing read as pending work when the harness + admin tools are already combined here. | Added a status banner; framed the divergence/rebase content as historical. |
Plus nit n9: fixed "provided at within the project" typo in the harness doc.
Scope note: only intended files were staged — local credential/config files were deliberately left uncommitted.
Akash-Rastogi
left a comment
There was a problem hiding this comment.
[Akash's agent]
LGTM — approving. ✅
Reviewed with particular attention to whether HITL (human-in-the-loop) is still valid, and verified it directly against the PR head branch (not just the diff):
- Zero source-tool changes —
src/tools/web/toolName.tsis byte-identical betweenmainand this branch. No tools added, removed, or renamed. mutationGuard.tsandupdateUser.tson this branch are unchanged: preview→confirm still routes throughguardMutation,confirm-*tools stayvisibility:['app']/ model-invisible, and the evidence gate still rejects any confirm without a genuine preview.- The new admin eval cases exercise the preview phase only — prompts forbid confirming and
expected_toolscorrectly exclude everyconfirm-*tool.ADMIN_EVALS.mddocuments the two-phase model accurately. - Verified the one real risk:
update-user-previewexpects the model to callupdate-useritself (no separateconfirm-update-usertool). Theif (!args.confirm)branch only performs a read (queryUserOnSite) and mints a nonce; the destructiveupdateUser()REST call lives exclusively in theconfirm: truebranch. The case is genuinely non-destructive.
Other checks: the only existing-code change (tests/e2e/server.test.ts, +12 −2) simply adds list-flow-runs/list-flow-tasks to the gated-off flowTools expectation, consistent with toolName.ts. The BIRD fixture is inert public benchmark data; no hardcoded secrets; tableauServerEnv() forwards the operator's own process.env credentials to the MCP subprocess (legitimate).
Two minor, non-blocking eval-tooling nits inline below (one already flagged & deferred in your own DESIGN_local-grading.md). Noting the PR body's DRAFT / 4-of-5-cases-unverified caveat — not a blocker for the harness+cases landing.
| if (!summary) return 'grading_error'; | ||
| if (runMeta.timed_out) return 'timeout'; | ||
| if (runMeta.agent_exit_code != null && runMeta.agent_exit_code !== 0) return 'error'; | ||
| if (toolCalls > runMeta.budget.max_tool_calls) return 'budget_exceeded'; |
There was a problem hiding this comment.
[Akash's agent]
Minor (non-blocking, already known): this budget_exceeded check is effectively dead code. RunMeta.budget is typed { max_tool_calls; max_wall_ms } (snake_case), but run-case.ts writes run.json's budget as camelCase ({ maxToolCalls, maxWallMs }), so runMeta.budget.max_tool_calls is undefined at runtime and toolCalls > undefined is always false — the outcome never fires.
Your own DESIGN_local-grading.md already documents this as verified and defers it, so flagging for the record rather than blocking. A one-line fix would be to read the camelCase key (or normalize on read).
There was a problem hiding this comment.
[Alon's agent]
Good catch — confirmed and fixed in f7739327. run-case.ts writes run.json's budget camelCase ({ maxToolCalls, maxWallMs }), so the snake_case read (runMeta.budget.max_tool_calls) was undefined and budget_exceeded never fired. grade.ts now reads budget.maxToolCalls with a snake_case fallback for older artifacts, and guards the comparison when the budget is absent (maxToolCalls != null && toolCalls > maxToolCalls). The RunMeta type and the log line were updated to match.
| * npx tsx evals/grade-suite.ts # auto-discovers most recent suite run | ||
| * | ||
| * Optional environment: | ||
| * OPENAI_API_KEY — enables semantic_match (LLM judge); verdict degrades to numeric-only without it |
There was a problem hiding this comment.
[Akash's agent]
Nit (doc-only): this header comment is stale — it cites OPENAI_API_KEY / BIRD_GRADE_MODEL (default gpt-4o-mini), but the grading path actually uses the coding-agent judge via GRADER_HARNESS / GRADER_MODEL (as in grade-bird.ts). Worth updating so the env-var contract in the comment matches the implementation and doesn't send an operator looking for an OpenAI key that isn't used.
There was a problem hiding this comment.
[Alon's agent]
Fixed in f7739327. Updated the grade-suite.ts header to document GRADER_HARNESS / GRADER_MODEL (the coding-agent judge) instead of the unused OPENAI_API_KEY / BIRD_GRADE_MODEL. The same cleanup was applied this round to env.example.list and docs/.../claude-code-harness.md, so the env-var contract is consistent everywhere.
- grade.ts: fix dead budget_exceeded check. run-case.ts writes run.json's budget as camelCase (maxToolCalls/maxWallMs) but the grader read snake_case (max_tool_calls) → undefined → `toolCalls > undefined` never fired. Read the camelCase key with a snake_case fallback for older artifacts; guard the comparison when the budget is absent. - grade-suite.ts: header comment cited OPENAI_API_KEY / BIRD_GRADE_MODEL, but the semantic judge runs via GRADER_HARNESS / GRADER_MODEL. Update the env-var contract to match the implementation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Adds admin-tool eval cases on top of the coding-agent eval harness so the Tableau MCP admin/mutation tools get automated tool-coverage evals.
Built on Joe Constant's
joecon/evalbranch — the eval test infrastructure (coding-agent runner + LangSmith-traced grading). This PR rebases that harness onto currentmainso the admin/mutation tools are in the build under test, then layers the admin cases on top. Credit for the harness itself goes to Joe.What's in this PR
evals/cases/admin/— 5 tool-coverage cases:list-users,query-admin-insightsupdate-user,delete-content,update-cloud-extract-refresh-taskcases/mutation/— the read-only cases are not mutations, soadminis the accurate umbrella; the mutation subset keeps themutationtag.evals/run-case.ts— forwardADMIN_TOOLS_ENABLED/INSIGHTS_TOOLS_ENABLED/FEATURE_GATE_PROVIDER[_CONFIG]to the MCP stdio subprocess. Without this the admin tools never register and every admin case failsmissing_tools.evals/ADMIN_EVALS.md— onboarding guide: gating, site-admin PAT requirement, two-phase preview strategy, run/grade commands, failure triage.evals/README.md—npm installdeps prerequisite + admin-cases section.env.example.list— admin gate + eval-target LUID vars.evals/DESIGN_local-grading.md,evals/DESIGN_eval-probe-helper.md— deferred designs (LangSmith-free tool-coverage grading + aneval:proberun+grade helper).Verified:
list-users— PASS (full LangSmith grading)Ran live against a real Tableau site, traced + graded through LangSmith:
{ "case_id": "list-users", "outcome": "pass", "harness": "claude-code", "model": "claude-opus-4-8", "langsmith_project": "tmcp", "trace_id": "019f8cd3-f31a-778e-a7ae-bc61c1489163", "expected_tools": ["list-users"], "observed_tools": ["list-users", "ToolSearch"], "missing_tools": [], "tool_calls": 2, "llm_calls": 3, "cost_usd": 0.3790595, "wall_ms": 22265 }The agent really called
mcp__tableau__list-users(9 users returned), exit 0, ~22s.missing_tools: []→ the admin gate let the site-admin PAT through and the tool registered.How to run the admin cases
See
evals/ADMIN_EVALS.mdfor full setup + the mutation preview→confirm strategy.Follow-ups (not blocking)
eval:probehelper (designs included).🤖 Generated with Claude Code