From ee2c6b970cf7697b49c9e16dcb046193268fb165 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:04:03 +0000 Subject: [PATCH 1/2] Add checkout intents to the bench and report tool failures A prepare intent walks a site's flow part way and may register named action tools as well as read-only ones. Once the agent is done, the runner reads the end state with the intent's read-only check tool, in both arms, and grades it, so the result comes from the site rather than the agent's answer. The first one puts the cheapest Delta Main Classic fare in the cart and stops on Trip Summary. Each WebMCP invocation now records the tool's own status and error message. invoke ok counted calls whose tool returned status error as successes; it no longer does. The report adds a per-tool failure table with the most common errors. --- bench/README.md | 16 +++++++-- bench/grade.mjs | 18 +++++++--- bench/lib/agent.mjs | 22 +++++++++++-- bench/lib/intents.mjs | 10 ++++-- bench/lib/tools.mjs | 12 ++++--- bench/prompts/system.md | 2 +- bench/report.mjs | 33 ++++++++++++++++++- bench/run.mjs | 26 +++++++++++++-- bench/schemas/cart-flight.json | 8 +++++ .../delta.com/delta-cart-main-classic.json | 33 +++++++++++++++++++ bench/test/agent.test.mjs | 24 ++++++++++++-- bench/test/grade.test.mjs | 11 +++++++ bench/test/report.test.mjs | 12 ++++++- bench/test/tasks.test.mjs | 19 ++++++++++- 14 files changed, 219 insertions(+), 27 deletions(-) create mode 100644 bench/schemas/cart-flight.json create mode 100644 bench/tasks/delta.com/delta-cart-main-classic.json diff --git a/bench/README.md b/bench/README.md index 750842e2..53629ce3 100644 --- a/bench/README.md +++ b/bench/README.md @@ -127,6 +127,12 @@ and deletes every browser. - `stage` is `find`, `inspect`, `compare`, `prepare` or `gap`. A `gap` intent is one no tool covers; it names no tools and serves as the placebo check. +- A `prepare` intent walks a site's flow part way, such as putting a fare in Delta's cart and + stopping on Trip Summary. `actions` lists the tools that change site state that it may + register as well as the read-only ones. `check` names a read-only tool, an optional `input` and + `rules`: once the agent is done, the runner invokes that tool in the browser, in both arms + (control gets the tools registered only then), and grades its output, so the end state is read + from the site rather than taken from the agent's answer. - `tools` lists kernel/xwebmcp refs (or full `sha`s) to register from. Treatment gets every tool `registry.json` lists for the domain at those commits whose file declares `readOnlyHint: true`; later sources replace earlier tools of the same name, so a PR to an @@ -178,6 +184,8 @@ by name. 1. Extract an answer from the final message: the last fenced JSON block, then a trailing object, then `key: value` lines naming schema properties. Nothing extracted is `malformed`. 2. A schema or rule failure is `wrong`; otherwise `correct`, which means plausible, not verified. +3. For a `prepare` intent, the end state must also pass the check's rules, or it is `wrong`. The + check tool's output is saved as `end-state.json` next to the transcript. A successful call outside the allowlist, a pi error or a missing cost is `crashed`; hitting a cap is `timeout`. `node bench/grade.mjs bench/results//trials//` regrades @@ -201,9 +209,11 @@ scored: slower arm is `mixed` when it was correct in 2 fewer pairs, gave the worse `better` answer in 2 more, or the arms' `same` answers differed in 2 pairs. -Then the worst label per PR, and per run: spend over every attempt including smoke gates, the -label histogram (noting labels from `shape only` intents), median speedup, each arm's -agent-minutes, turns and spend over complete pairs, the pooled gap-intent +A tool failures table lists, per tool, the treatment invocations that did not complete or whose +tool returned `status: "error"`, with their most common error messages. Then the worst label per +PR, and per run: spend over every attempt including smoke gates, the label histogram (noting +labels from `shape only` intents), median speedup, each arm's agent-minutes, turns and spend +over complete pairs, the pooled gap-intent placebo, an order check, a start delay check, a native tools check (control trials that started with the page's own WebMCP tools), cap calibration, and how many labels to expect from noise. diff --git a/bench/grade.mjs b/bench/grade.mjs index faacae5c..cb20a247 100644 --- a/bench/grade.mjs +++ b/bench/grade.mjs @@ -3,7 +3,7 @@ // rules. The first failing step decides the outcome. // Usage: node bench/grade.mjs bench/results//trials// -import { readFileSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import { basename, dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { summarize } from "./lib/agent.mjs"; @@ -119,8 +119,10 @@ export function checkRule(rule, answer) { return null; } -export function gradeAnswer(intent, schema, text) { - const grade = { outcome: "malformed", extracted: false, shape: false, rules: false, answer: null, problems: [] }; +// `endState` is what the runner read from the browser with a prepare intent's check tool: +// { output } or { error }. +export function gradeAnswer(intent, schema, text, endState = null) { + const grade = { outcome: "malformed", extracted: false, shape: false, rules: false, end_state: null, answer: null, problems: [] }; const answer = extractAnswer(text, schema); if (!answer) { grade.problems.push("no answer object in the final message"); @@ -135,6 +137,12 @@ export function gradeAnswer(intent, schema, text) { grade.problems = fillRules(intent.rules, intent.fixture).map(rule => checkRule(rule, answer)).filter(Boolean); if (grade.problems.length) return grade; grade.rules = true; + if (intent.check) { + grade.problems = endState?.error ? [`end state: ${endState.error}`] + : fillRules(intent.check.rules, intent.fixture).map(rule => checkRule(rule, endState?.output)).filter(Boolean).map(problem => `end state: ${problem}`); + grade.end_state = !grade.problems.length; + if (grade.problems.length) return grade; + } grade.outcome = "correct"; return grade; } @@ -148,5 +156,7 @@ if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.ur const run = JSON.parse(readFileSync(join(trialDir, "../../../run.json"), "utf8")); const { intent, schema } = run.intents[basename(dirname(trialDir))]; const events = readFileSync(join(trialDir, "transcript.jsonl"), "utf8").split("\n").filter(Boolean).map(line => JSON.parse(line)); - console.log(JSON.stringify(gradeAnswer(intent, schema, summarize(events, []).answer), null, 2)); + const endStateFile = join(trialDir, "end-state.json"); + const endState = existsSync(endStateFile) ? JSON.parse(readFileSync(endStateFile, "utf8")) : null; + console.log(JSON.stringify(gradeAnswer(intent, schema, summarize(events, []).answer, endState), null, 2)); } diff --git a/bench/lib/agent.mjs b/bench/lib/agent.mjs index 9081b4b0..4361e05f 100644 --- a/bench/lib/agent.mjs +++ b/bench/lib/agent.mjs @@ -140,6 +140,23 @@ function collectRefs(value, refs, depth = 0) { for (const child of Object.values(value)) collectRefs(child, refs, depth + 1); } +// The invocation's status, the tool's own status, and an error message when either failed. A +// call the MCP server rejected, such as one with a stale tool_ref, comes back as plain text. +function invocationResult(call) { + if (!call.done) return { status: "no_result", tool_status: null, error: null }; + const result = parseJson(call.text ?? ""); + if (result === null || typeof result !== "object") { + return { status: call.isError || /^error\b/i.test(call.text ?? "") ? "error" : "unknown", tool_status: null, error: (call.text ?? "").slice(0, 300) || null }; + } + const output = result.output !== null && typeof result.output === "object" ? result.output : {}; + const toolStatus = typeof output.status === "string" ? output.status : null; + const failed = result.status !== "completed" || toolStatus === "error"; + const error = failed ? result.error_text ?? output.error_message ?? output.message ?? output.error_kind ?? result.status ?? null : null; + return { status: result.status ?? "unknown", tool_status: toolStatus, error: typeof error === "string" ? error.slice(0, 300) : null }; +} + +export const invocationFailed = (invocation) => invocation.status !== "completed" || invocation.tool_status === "error"; + const WEBMCP = "mcp__kernel__webmcp"; const CODE_PATHS = { mcp__kernel__execute_playwright_code: "playwright", mcp__kernel__browser_repl: "repl" }; @@ -182,17 +199,16 @@ export function summarize(events, allow) { if (call.name === WEBMCP && args.action === "list") { webmcp.listed = true; } else if (call.name === WEBMCP && args.action === "invoke") { - const output = parseJson(call.text ?? ""); webmcp.invocations.push({ path: "mcp", tool: refs.get(args.tool_ref) ?? args.tool_ref ?? null, - status: output?.status ?? (call.done ? (call.isError ? "error" : "unknown") : "no_result"), + ...invocationResult(call), duration_ms: call.done && call.startedAt !== undefined ? call.endedAt - call.startedAt : null, }); } else if (CODE_PATHS[call.name] && typeof args.code === "string") { if (/\blistTools\s*\(/.test(args.code)) webmcp.listed = true; if (/\binvokeTool\s*\(/.test(args.code)) { - webmcp.invocations.push({ path: CODE_PATHS[call.name], tool: null, status: null, duration_ms: null }); + webmcp.invocations.push({ path: CODE_PATHS[call.name], tool: null, status: null, tool_status: null, error: null, duration_ms: null }); } } } diff --git a/bench/lib/intents.mjs b/bench/lib/intents.mjs index 65fdbf45..641b2372 100644 --- a/bench/lib/intents.mjs +++ b/bench/lib/intents.mjs @@ -76,7 +76,13 @@ export function validateIntent(intent, benchDir) { need(intent.stage === "gap" ? named.length === 0 : named.length > 0, intent.stage === "gap" ? "gap intents name no tools" : "non-gap intents name the tools a treatment agent would use"); need(typeof intent.entry_tool === "string", "entry_tool is required"); need(typeof intent.answer_schema === "string" && existsSync(join(benchDir, intent.answer_schema)), "answer_schema must point at a file under bench/"); - for (const rule of intent.rules ?? []) { + // A prepare intent changes site state, so the runner reads where the agent left the browser. + const { check, actions } = intent; + need(intent.stage === "prepare" ? check !== undefined : check === undefined && actions === undefined, "check, and actions, belong to prepare intents, and a prepare intent needs a check"); + need(actions === undefined || (Array.isArray(actions) && actions.every(name => typeof name === "string")), "actions must list tool names"); + need(check === undefined || (typeof check?.tool === "string" && Array.isArray(check.rules) && check.rules.length > 0 && (check.input === undefined || (check.input !== null && typeof check.input === "object"))), + "check needs a tool, rules and, optionally, an input object"); + for (const rule of [...(intent.rules ?? []), ...(Array.isArray(check?.rules) ? check.rules : [])]) { const checks = Object.keys(rule).filter(key => key !== "path"); need(typeof rule.path === "string" && checks.length === 1 && RULE_CHECKS.includes(checks[0]), `rule ${JSON.stringify(rule)} needs a path and one of ${RULE_CHECKS.join(", ")}`); if ("matches" in rule) need(typeof rule.matches === "string" && compiles(fillRules([rule], intent.fixture)[0].matches, "iu"), `rule ${JSON.stringify(rule)} needs a valid regex`); @@ -89,7 +95,7 @@ export function validateIntent(intent, benchDir) { for (const [key, value] of Object.entries(intent.fixture ?? {})) { need(typeof value === "string" || typeof value === "number" || relativeDate(value), `fixture ${key} must be a string, a number or {"days_from_start": n}`); } - const text = JSON.stringify([intent.prompt, intent.rules ?? []]); + const text = JSON.stringify([intent.prompt, intent.rules ?? [], check?.rules ?? [], check?.input ?? {}]); for (const [, key] of text.matchAll(/\{\{(\w+)\}\}/g)) need(Object.hasOwn(intent.fixture ?? {}, key), `{{${key}}} has no fixture value`); for (const key of ["pairs", "trial_timeout_sec", "max_turns"]) need(Number.isInteger(intent[key]) && intent[key] > 0, `${key} must be a positive integer`); return problems; diff --git a/bench/lib/tools.mjs b/bench/lib/tools.mjs index a37c7d66..a4e043a3 100644 --- a/bench/lib/tools.mjs +++ b/bench/lib/tools.mjs @@ -74,7 +74,9 @@ function namespaceTools(show, sha, namespace) { } // Sources apply in order, so a branch listed after main replaces main's tool of the same name. +// A prepare intent's actions are tools that change site state, registered as well. export async function registeredTools(intent, shas, show) { + const actions = intent.actions ?? []; const byName = new Map(); const known = new Set(); const problems = []; @@ -85,15 +87,15 @@ export async function registeredTools(intent, shas, show) { if (!all.some(tool => tool.name === name)) problems.push(`${name} is not in registry.json at ${source.ref} (${shas[i].slice(0, 7)})`); } for (const tool of all) { - if (tool.readOnly && !Object.hasOwn(intent.exclude, tool.name)) byName.set(tool.name, tool); + if ((tool.readOnly || actions.includes(tool.name)) && !Object.hasOwn(intent.exclude, tool.name)) byName.set(tool.name, tool); else byName.delete(tool.name); } } - for (const name of Object.keys(intent.exclude)) { - if (!known.has(name)) problems.push(`exclude names ${name}, which no source has`); + for (const [list, name] of [...Object.keys(intent.exclude).map(name => ["exclude", name]), ...actions.map(name => ["actions", name])]) { + if (!known.has(name)) problems.push(`${list} names ${name}, which no source has`); } - for (const name of [...intent.tools.flatMap(s => s.names), intent.entry_tool]) { - if (!byName.has(name) && !problems.some(p => p.startsWith(`${name} `))) problems.push(`${name} is not read-only or is excluded, so it would not be registered`); + for (const name of [...intent.tools.flatMap(s => s.names), intent.entry_tool, ...(intent.check ? [intent.check.tool] : [])]) { + if (!byName.has(name) && !problems.some(p => p.startsWith(`${name} `))) problems.push(`${name} is excluded or neither read-only nor an action, so it would not be registered`); } if (problems.length) throw new Error(`${intent.id}: ${problems.join("; ")}`); return [...byName.values()].sort((a, b) => a.name.localeCompare(b.name)); diff --git a/bench/prompts/system.md b/bench/prompts/system.md index 68140dd4..961f87e8 100644 --- a/bench/prompts/system.md +++ b/bench/prompts/system.md @@ -10,7 +10,7 @@ Use the page's WebMCP tools first: Rules: -- Do not check out, pay, place an order, or complete a booking or reservation. +- Do not pay, place an order, or complete a booking or reservation, and go no further into a checkout than the task asks. - Do not add items or tickets to a cart unless the task says you may. Selecting seats and ticket types to see their prices is allowed. - Do not log in, sign up, or enter personal or payment details. - Do not post, comment, send messages, star, follow, or change anything else on the site. diff --git a/bench/report.mjs b/bench/report.mjs index 37204fae..f6c6f4a8 100644 --- a/bench/report.mjs +++ b/bench/report.mjs @@ -8,6 +8,7 @@ import { fileURLToPath } from "node:url"; import { completePairRows, latestRows, readRows } from "./lib/rows.mjs"; import { median, newcombe, signTest, signTestRate, wilson } from "./lib/stats.mjs"; import { resolvePath } from "./grade.mjs"; +import { invocationFailed } from "./lib/agent.mjs"; // The speedup ratios are recalibrated after the pilot. const THRESHOLDS = { adoption: 0.5, minBothCorrect: 5, p: 0.1, faster: 1.5, slower: 0.67, mixedDelta: 2 }; @@ -44,7 +45,7 @@ export function armStats(rows) { min_per_correct: correct.length ? sum(all) / 60_000 / correct.length : null, adoption: rows.length ? rows.filter(r => r.webmcp?.adopted).length / rows.length : null, invocations: invocations.length, - invoke_success: withStatus.length ? withStatus.filter(i => i.status === "completed").length / withStatus.length : null, + invoke_success: withStatus.length ? withStatus.filter(i => !invocationFailed(i)).length / withStatus.length : null, tool_median: median(withStatus.map(i => i.duration_ms).filter(d => d !== null)), turns: median(rows.map(r => r.turns).filter(t => typeof t === "number")), tokens_in: median(rows.map(r => r.tokens_in).filter(t => typeof t === "number")), @@ -106,6 +107,7 @@ export function adoptedPairStats(rows, agreement = null) { // A rule that only repeats the fixture back pins nothing. export function answerCheck(intent) { + if (intent.check) return "end state"; const pinned = (intent.rules ?? []).some(rule => ("equals" in rule || "in" in rule) && !JSON.stringify(rule.equals ?? rule.in).includes("{{")); return pinned ? "pinned" : intent.agreement ? "agreement" : "shape only"; } @@ -139,6 +141,27 @@ export function compare(control, treatment, pairs, stage) { }; } +// Failed invocations per tool, with their most common errors, over treatment trials. +export function toolFailures(rows, domainOf) { + const byTool = new Map(); + for (const row of rows.filter(r => r.arm === "treatment")) { + for (const invocation of row.webmcp?.invocations ?? []) { + if (invocation.status === null) continue; + const key = `${domainOf(row.intent)}/${invocation.tool ?? "unknown"}`; + const entry = byTool.get(key) ?? { tool: key, calls: 0, failed: 0, errors: new Map() }; + entry.calls++; + if (invocationFailed(invocation)) { + entry.failed++; + const error = invocation.error ?? invocation.status; + entry.errors.set(error, (entry.errors.get(error) ?? 0) + 1); + } + byTool.set(key, entry); + } + } + return [...byTool.values()].filter(e => e.failed).sort((a, b) => b.failed - a.failed || a.tool.localeCompare(b.tool)) + .map(e => ({ ...e, errors: [...e.errors].sort((a, b) => b[1] - a[1]) })); +} + // --- formatting ----------------------------------------------------------------------------- const na = "n/a"; @@ -313,6 +336,14 @@ export function buildReport(run, allRows, smokeRows = []) { + "intents, so up to that many faster or slower labels may be noise."); } + const failures = toolFailures(scored, id => run.intents[id]?.intent.domain ?? id); + if (failures.length) { + out.push("", "## Tool failures", "", "Invocations that did not complete or whose tool returned `status: \"error\"`, in scored treatment trials.", ""); + out.push(table(["tool", "calls", "failed", "most common errors"], failures.map(f => [ + f.tool, f.calls, f.failed, f.errors.slice(0, 3).map(([error, n]) => `${n}× ${error.replace(/\|/g, "\\|").replace(/\n/g, " ").slice(0, 160)}`).join("
"), + ]))); + } + const flagged = rows.filter(r => r.outcome === "aborted" || r.outcome === "crashed" || r.registration_changed); if (flagged.length) { out.push("", "## Trials to look at", ""); diff --git a/bench/run.mjs b/bench/run.mjs index 604960f0..6eb4bf22 100644 --- a/bench/run.mjs +++ b/bench/run.mjs @@ -355,9 +355,11 @@ const registrationFingerprint = (tools) => JSON.stringify(tools.map(t => `${t.na // A fresh browser on a signed-out profile is still signed out, so setup does not retry. class SignedOut extends Error {} +const liveRef = (live, intent, tool) => live.find(t => t.tool?.name === tool && t.source?.custom?.namespace === intent.domain)?.tool_ref; + async function checkSignedIn(browser, intent, live) { const { tool, input = {} } = intent.signed_in_check; - const ref = live.find(t => t.tool?.name === tool && t.source?.custom?.namespace === intent.domain)?.tool_ref; + const ref = liveRef(live, intent, tool); if (!ref) throw new Error(`${tool} is not live on ${intent.start_url}`); const output = await invokeTool(browser, ref, input); if (output?.status === "error" || output?.error) throw new SignedOut(`signed out: ${tool} returned ${JSON.stringify(output).slice(0, 300)}`); @@ -408,6 +410,22 @@ async function setUpBrowser(plan, intentPlan, arm, label) { } } +// Reads where a prepare intent's agent left the browser, with the intent's check tool. Control +// gets the batch registered only now, after its agent is done. +async function readEndState(browser, intentPlan, arm) { + const { intent } = intentPlan; + if (arm === "control") await addCustomTools(browser, intent.domain, join(runDir, "sources", `${intent.id}.js`)); + const live = await listLiveTools(browser); + const ref = liveRef(live, intent, intent.check.tool); + const pages = [...new Set(live.map(t => t.source?.page_url).filter(Boolean))]; + if (!ref) return { error: `${intent.check.tool} is not live; the browser is on ${pages.join(", ") || "no page with tools"}` }; + try { + return { output: await invokeTool(browser, ref, intent.check.input ?? {}) }; + } catch (error) { + return { error: `${intent.check.tool} failed: ${error.message}` }; + } +} + // Returns the outcome, or null when the run is stopping and no row was written. async function runTrial(plan, intentPlan, arm, key) { if (stopping) return null; @@ -462,11 +480,13 @@ async function runTrial(plan, intentPlan, arm, key) { if (verdict) { [row.outcome, row.reason] = verdict; } else { - const grade = gradeAnswer(intent, intentPlan.schema, summary.answer); + const endState = intent.check ? await readEndState(browser, intentPlan, arm) : null; + if (endState) writeFileSync(join(dir, "end-state.json"), JSON.stringify(endState, null, 2) + "\n"); + const grade = gradeAnswer(intent, intentPlan.schema, summary.answer, endState); writeFileSync(join(dir, "grade.json"), JSON.stringify(grade, null, 2) + "\n"); row.outcome = grade.outcome; row.reason = grade.problems.slice(0, 3).join("; ") || null; - row.grade = { extracted: grade.extracted, shape: grade.shape, rules: grade.rules, answer: grade.answer }; + row.grade = { extracted: grade.extracted, shape: grade.shape, rules: grade.rules, end_state: grade.end_state, answer: grade.answer }; } } catch (error) { row.outcome = "aborted"; diff --git a/bench/schemas/cart-flight.json b/bench/schemas/cart-flight.json new file mode 100644 index 00000000..82446506 --- /dev/null +++ b/bench/schemas/cart-flight.json @@ -0,0 +1,8 @@ +{ + "type": "object", + "required": ["flight_numbers", "amount_due"], + "properties": { + "flight_numbers": { "type": "array", "minItems": 1, "items": { "type": "string", "minLength": 1 } }, + "amount_due": { "type": "number", "exclusiveMinimum": 0 } + } +} diff --git a/bench/tasks/delta.com/delta-cart-main-classic.json b/bench/tasks/delta.com/delta-cart-main-classic.json new file mode 100644 index 00000000..64b40ec7 --- /dev/null +++ b/bench/tasks/delta.com/delta-cart-main-classic.json @@ -0,0 +1,33 @@ +{ + "id": "delta-cart-main-classic", + "domain": "delta.com", + "stage": "prepare", + "prompt": "On delta.com, put the cheapest Delta Main Classic one-way fare for 1 adult from {{origin}} to {{destination}} on {{date}} in the cart, and stop on Delta's Trip Summary page with that flight in the cart. You may select the flight and its fare; do not go on to passenger details, seats or payment. Reply with JSON: {\"flight_numbers\": [string], \"amount_due\": number in US dollars, the total Delta shows on the Trip Summary page}.", + "fixture": { + "origin": "Atlanta (ATL)", + "destination": "Orlando (MCO)", + "date": {"days_from_start": 28} + }, + "start_url": "https://www.delta.com/", + "tools": [{"repo": "kernel/xwebmcp", "ref": "main", "names": ["search_flights", "list_flight_results", "select_fare", "select_fare_experience", "get_trip_summary"]}], + "actions": ["select_fare", "select_fare_experience"], + "entry_tool": "search_flights", + "answer_schema": "schemas/cart-flight.json", + "rules": [{"path": "amount_due", "range": [1, 3000]}], + "check": { + "tool": "get_trip_summary", + "rules": [ + {"path": "status", "equals": "ok"}, + {"path": "step", "equals": "trip_summary"}, + {"path": "passengers", "equals": 1}, + {"path": "slices", "count": [1, 1]}, + {"path": "slices[].origin", "matches": "ATL|Atlanta"}, + {"path": "slices[].destination", "matches": "MCO|Orlando"}, + {"path": "slices[].cabin", "matches": "classic"} + ] + }, + "agreement": {"path": "amount_due", "better": "lower", "tolerance": 1}, + "pairs": 5, + "trial_timeout_sec": 900, + "max_turns": 60 +} diff --git a/bench/test/agent.test.mjs b/bench/test/agent.test.mjs index e7b2f5ef..a475ef3a 100644 --- a/bench/test/agent.test.mjs +++ b/bench/test/agent.test.mjs @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { classifyTrial, parseRecord, piArgs, summarize } from "../lib/agent.mjs"; +import { classifyTrial, invocationFailed, parseRecord, piArgs, summarize } from "../lib/agent.mjs"; const allow = ["mcp__kernel__browser_repl", "mcp__kernel__computer_action", "mcp__kernel__execute_playwright_code", "mcp__kernel__webmcp"]; @@ -57,8 +57,8 @@ test("the transcript yields tools, the answer, metrics, leaks and WebMCP usage", listed: true, adopted: true, invocations: [ - { path: "mcp", tool: "lookup_ip", status: "completed", duration_ms: 1500 }, - { path: "playwright", tool: null, status: null, duration_ms: null }, + { path: "mcp", tool: "lookup_ip", status: "completed", tool_status: null, error: null, duration_ms: 1500 }, + { path: "playwright", tool: null, status: null, tool_status: null, error: null, duration_ms: null }, ], }); }); @@ -155,3 +155,21 @@ test("a provider error pi gave up retrying is reported, and a retry that recover assert.deepEqual(classifyTrial(summary, { limit: null, exitCode: 1 }, allow, ""), ["aborted", "provider error after 3 retries: 429 rate limited"]); assert.equal(summarize([...failed.slice(0, 2), { type: "auto_retry_end", success: true, attempt: 1 }], allow).retries_exhausted, null); }); + +test("a failed invocation keeps its error, whether the call, the tool or the MCP server failed", () => { + const invoke = (id) => call(id, "mcp__kernel__webmcp", { action: "invoke", tool_ref: "wmcp_a" }); + const summary = summarize([ + assistant([invoke("1"), invoke("2"), invoke("3"), invoke("4")]), + end("1", "mcp__kernel__webmcp", JSON.stringify({ status: "completed", output: { status: "error", message: "UI_CHANGED: missing button" } }), 10), + end("2", "mcp__kernel__webmcp", JSON.stringify({ status: "error", error_text: "input failed JSON Schema validation" }), 20), + end("3", "mcp__kernel__webmcp", 'error in webmcp (invoke): {"code":"not_found"}', 30), + end("4", "mcp__kernel__webmcp", JSON.stringify({ status: "completed", output: { status: "no_results", message: "nothing matched" } }), 40), + ], allow); + const shown = summary.webmcp.invocations.map(i => [i.status, i.tool_status, i.error, invocationFailed(i)]); + assert.deepEqual(shown, [ + ["completed", "error", "UI_CHANGED: missing button", true], + ["error", null, "input failed JSON Schema validation", true], + ["error", null, 'error in webmcp (invoke): {"code":"not_found"}', true], + ["completed", "no_results", null, false], + ]); +}); diff --git a/bench/test/grade.test.mjs b/bench/test/grade.test.mjs index 9da6fa10..0cf47dae 100644 --- a/bench/test/grade.test.mjs +++ b/bench/test/grade.test.mjs @@ -75,3 +75,14 @@ test("a matches rule takes fixture values literally", () => { assert.equal(grade("verity (2026)"), "correct"); assert.equal(grade("Verity 2026"), "wrong"); }); + +test("a prepare intent is correct only when the end state passes its check", () => { + const prepare = { ...intent, check: { tool: "get_trip_summary", rules: [{ path: "step", equals: "trip_summary" }, { path: "slices[].origin", matches: "ATL" }] } }; + const text = '{"ip": "8.8.8.8", "country_code": "US", "asn": "AS15169"}'; + const grade = (endState) => gradeAnswer(prepare, schema, text, endState); + assert.deepEqual([grade({ output: { step: "trip_summary", slices: [{ origin: "ATL" }] } }).outcome, grade({ output: { step: "trip_summary", slices: [{ origin: "ATL" }] } }).end_state], ["correct", true]); + const elsewhere = grade({ output: { step: "other", slices: [] } }); + assert.deepEqual([elsewhere.outcome, elsewhere.end_state, elsewhere.problems], ["wrong", false, ['end state: step: "other" is not "trip_summary"', "end state: slices[].origin: no value"]]); + assert.deepEqual(grade({ error: "get_trip_summary is not live" }).problems, ["end state: get_trip_summary is not live"]); + assert.equal(gradeAnswer(intent, schema, text).end_state, null); +}); diff --git a/bench/test/report.test.mjs b/bench/test/report.test.mjs index 37b7ca58..5de944ae 100644 --- a/bench/test/report.test.mjs +++ b/bench/test/report.test.mjs @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { adoptedPairStats, answerCheck, armStats, buildReport, compare, pairStats, startDelaySkews } from "../report.mjs"; +import { adoptedPairStats, answerCheck, armStats, buildReport, compare, pairStats, startDelaySkews, toolFailures } from "../report.mjs"; import { newcombe, signTest, signTestRate, wilson } from "../lib/stats.mjs"; const round = (iv) => iv.map(x => Math.round(x * 100)); @@ -231,3 +231,13 @@ test("the report gives a paired test of correctness and each arm's turns, tokens assert.match(report, /per trial, median: turns 12 control, 5 treatment; input tokens 90000 and 40000; cost \$0\.400 and \$0\.400\./); assert.match(report, /over complete pairs: control 39 agent-minutes, 120 turns and \$4\.00; treatment 22 agent-minutes, 50 turns and \$4\.00\./); }); + +test("tool failures are counted per tool, with the most common errors first", () => { + const invocation = (tool, status, tool_status, error) => ({ path: "mcp", tool, status, tool_status, error, duration_ms: 1 }); + const rows = [ + { intent: "d", arm: "treatment", webmcp: { invocations: [invocation("search", "completed", "error", "UI_CHANGED"), invocation("search", "completed", "ok", null), invocation("read", "completed", null, null)] } }, + { intent: "d", arm: "treatment", webmcp: { invocations: [invocation("search", "completed", "error", "UI_CHANGED"), invocation("search", "error", null, "schema")] } }, + { intent: "d", arm: "control", webmcp: { invocations: [invocation("search", "error", null, "control never counts")] } }, + ]; + assert.deepEqual(toolFailures(rows, () => "x.com"), [{ tool: "x.com/search", calls: 4, failed: 3, errors: [["UI_CHANGED", 2], ["schema", 1]] }]); +}); diff --git a/bench/test/tasks.test.mjs b/bench/test/tasks.test.mjs index 3390904f..a3ad3698 100644 --- a/bench/test/tasks.test.mjs +++ b/bench/test/tasks.test.mjs @@ -176,5 +176,22 @@ test("a file that declares more than one tool is refused", async () => { test("registration fails on a name that no source has or that would not be registered", async () => { await assert.rejects(registeredTools({ ...base, exclude: { whoamii: "typo" }, tools: [{ ref: "branch", names: [] }] }, ["branch"], show), /exclude names whoamii, which no source has/); await assert.rejects(registeredTools({ ...base, tools: [{ ref: "main", names: ["get_item"] }] }, ["main"], show), /get_item is not in registry.json at main/); - await assert.rejects(registeredTools({ ...base, tools: [{ ref: "main", names: ["save"] }] }, ["main"], show), /save is not read-only or is excluded/); + await assert.rejects(registeredTools({ ...base, tools: [{ ref: "main", names: ["save"] }] }, ["main"], show), /save is excluded or neither read-only nor an action/); +}); + +test("a prepare intent needs a check, and only prepare intents take checks and actions", () => { + const intent = JSON.parse(readFileSync(join(benchDir, "tasks/delta.com/delta-cart-main-classic.json"), "utf8")); + assert.deepEqual(validateIntent(intent, benchDir), []); + const { check, ...unchecked } = intent; + assert.ok(validateIntent(unchecked, benchDir).some(p => p.startsWith("check, and actions"))); + assert.ok(validateIntent({ ...intent, stage: "find" }, benchDir).some(p => p.startsWith("check, and actions"))); + assert.ok(validateIntent({ ...intent, check: { tool: "get_trip_summary", rules: [] } }, benchDir).some(p => p.startsWith("check needs"))); + assert.ok(validateIntent({ ...intent, check: { ...check, rules: [{ path: "step", equals: "{{cabin}}" }] } }, benchDir).includes("{{cabin}} has no fixture value")); +}); + +test("a prepare intent's actions are registered, and must exist", async () => { + const intent = { ...base, exclude: {}, stage: "prepare", actions: ["save"], check: { tool: "search", rules: [] }, tools: [{ ref: "main", names: ["save"] }] }; + assert.deepEqual((await registeredTools(intent, ["main"], show)).map(t => t.name), ["save", "search"]); + await assert.rejects(registeredTools({ ...intent, actions: ["delete"] }, ["main"], show), /actions names delete, which no source has/); + await assert.rejects(registeredTools({ ...intent, actions: [], check: { tool: "save", rules: [] }, tools: [{ ref: "main", names: [] }] }, ["main"], show), /save is excluded or neither/); }); From 5d6e62642a4331aa20f00cf598e9a93df079b6e5 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:49:18 +0000 Subject: [PATCH 2/2] Check a prepare intent's answer against the end state, and count unrecorded invocations --- bench/README.md | 8 +++++--- bench/grade.mjs | 15 ++++++++++++++- bench/lib/intents.mjs | 3 +++ bench/report.mjs | 4 +++- .../delta.com/delta-cart-main-classic.json | 3 ++- bench/test/grade.test.mjs | 18 +++++++++++++++++- bench/test/report.test.mjs | 8 ++++++++ bench/test/tasks.test.mjs | 1 + 8 files changed, 53 insertions(+), 7 deletions(-) diff --git a/bench/README.md b/bench/README.md index 53629ce3..e47dc433 100644 --- a/bench/README.md +++ b/bench/README.md @@ -132,7 +132,8 @@ and deletes every browser. register as well as the read-only ones. `check` names a read-only tool, an optional `input` and `rules`: once the agent is done, the runner invokes that tool in the browser, in both arms (control gets the tools registered only then), and grades its output, so the end state is read - from the site rather than taken from the agent's answer. + from the site rather than taken from the agent's answer. `same_as_answer` lists `{answer, + end_state}` path pairs whose values must match, so the answer reports what is in the cart. - `tools` lists kernel/xwebmcp refs (or full `sha`s) to register from. Treatment gets every tool `registry.json` lists for the domain at those commits whose file declares `readOnlyHint: true`; later sources replace earlier tools of the same name, so a PR to an @@ -184,8 +185,9 @@ by name. 1. Extract an answer from the final message: the last fenced JSON block, then a trailing object, then `key: value` lines naming schema properties. Nothing extracted is `malformed`. 2. A schema or rule failure is `wrong`; otherwise `correct`, which means plausible, not verified. -3. For a `prepare` intent, the end state must also pass the check's rules, or it is `wrong`. The - check tool's output is saved as `end-state.json` next to the transcript. +3. For a `prepare` intent, the end state must also pass the check's rules and match the answer + at each `same_as_answer` pair, or it is `wrong`. The check tool's output is saved as + `end-state.json` next to the transcript. A successful call outside the allowlist, a pi error or a missing cost is `crashed`; hitting a cap is `timeout`. `node bench/grade.mjs bench/results//trials//` regrades diff --git a/bench/grade.mjs b/bench/grade.mjs index cb20a247..eabdaa10 100644 --- a/bench/grade.mjs +++ b/bench/grade.mjs @@ -99,6 +99,16 @@ function isDate(value) { return date.toISOString().slice(0, 10) === m[0]; } +// Compares what the agent reported with what the check tool read, as sets: strings ignoring case +// and whitespace, amounts to the cent. +export function checkSameAsAnswer({ answer: answerPath, end_state: endStatePath }, answer, output) { + const canonical = (value) => typeof value === "string" ? value.toLowerCase().replace(/\s+/g, "") : typeof value === "number" ? Math.round(value * 100) / 100 : value; + const values = (value, path) => resolvePath(value, path).map(canonical).map(v => JSON.stringify(v)).sort(); + const [reported, read] = [values(answer, answerPath), values(output, endStatePath)]; + if (JSON.stringify(reported) === JSON.stringify(read)) return null; + return `${answerPath} is [${reported.join(", ")}] in the answer but ${endStatePath} is [${read.join(", ")}] on the site`; +} + export function checkRule(rule, answer) { const values = resolvePath(answer, rule.path); if (!values.length) return `${rule.path}: no value`; @@ -139,7 +149,10 @@ export function gradeAnswer(intent, schema, text, endState = null) { grade.rules = true; if (intent.check) { grade.problems = endState?.error ? [`end state: ${endState.error}`] - : fillRules(intent.check.rules, intent.fixture).map(rule => checkRule(rule, endState?.output)).filter(Boolean).map(problem => `end state: ${problem}`); + : [ + ...fillRules(intent.check.rules, intent.fixture).map(rule => checkRule(rule, endState?.output)), + ...(intent.check.same_as_answer ?? []).map(pair => checkSameAsAnswer(pair, answer, endState?.output)), + ].filter(Boolean).map(problem => `end state: ${problem}`); grade.end_state = !grade.problems.length; if (grade.problems.length) return grade; } diff --git a/bench/lib/intents.mjs b/bench/lib/intents.mjs index 641b2372..37f17d34 100644 --- a/bench/lib/intents.mjs +++ b/bench/lib/intents.mjs @@ -82,6 +82,9 @@ export function validateIntent(intent, benchDir) { need(actions === undefined || (Array.isArray(actions) && actions.every(name => typeof name === "string")), "actions must list tool names"); need(check === undefined || (typeof check?.tool === "string" && Array.isArray(check.rules) && check.rules.length > 0 && (check.input === undefined || (check.input !== null && typeof check.input === "object"))), "check needs a tool, rules and, optionally, an input object"); + const sameAsAnswer = check?.same_as_answer; + need(sameAsAnswer === undefined || (Array.isArray(sameAsAnswer) && sameAsAnswer.every(pair => typeof pair?.answer === "string" && typeof pair?.end_state === "string")), + "check.same_as_answer must list {answer, end_state} paths"); for (const rule of [...(intent.rules ?? []), ...(Array.isArray(check?.rules) ? check.rules : [])]) { const checks = Object.keys(rule).filter(key => key !== "path"); need(typeof rule.path === "string" && checks.length === 1 && RULE_CHECKS.includes(checks[0]), `rule ${JSON.stringify(rule)} needs a path and one of ${RULE_CHECKS.join(", ")}`); diff --git a/bench/report.mjs b/bench/report.mjs index f6c6f4a8..229c3694 100644 --- a/bench/report.mjs +++ b/bench/report.mjs @@ -338,7 +338,9 @@ export function buildReport(run, allRows, smokeRows = []) { const failures = toolFailures(scored, id => run.intents[id]?.intent.domain ?? id); if (failures.length) { - out.push("", "## Tool failures", "", "Invocations that did not complete or whose tool returned `status: \"error\"`, in scored treatment trials.", ""); + const unrecorded = scored.filter(r => r.arm === "treatment").flatMap(r => r.webmcp?.invocations ?? []).filter(i => i.status === null).length; + const unrecordedText = unrecorded ? ` ${unrecorded} invocation(s) made through Playwright or the REPL have no recorded result and are not counted.` : ""; + out.push("", "## Tool failures", "", `Invocations that did not complete or whose tool returned \`status: "error"\`, in scored treatment trials.${unrecordedText}`, ""); out.push(table(["tool", "calls", "failed", "most common errors"], failures.map(f => [ f.tool, f.calls, f.failed, f.errors.slice(0, 3).map(([error, n]) => `${n}× ${error.replace(/\|/g, "\\|").replace(/\n/g, " ").slice(0, 160)}`).join("
"), ]))); diff --git a/bench/tasks/delta.com/delta-cart-main-classic.json b/bench/tasks/delta.com/delta-cart-main-classic.json index 64b40ec7..8f2f25df 100644 --- a/bench/tasks/delta.com/delta-cart-main-classic.json +++ b/bench/tasks/delta.com/delta-cart-main-classic.json @@ -24,7 +24,8 @@ {"path": "slices[].origin", "matches": "ATL|Atlanta"}, {"path": "slices[].destination", "matches": "MCO|Orlando"}, {"path": "slices[].cabin", "matches": "classic"} - ] + ], + "same_as_answer": [{"answer": "amount_due", "end_state": "amount_due"}] }, "agreement": {"path": "amount_due", "better": "lower", "tolerance": 1}, "pairs": 5, diff --git a/bench/test/grade.test.mjs b/bench/test/grade.test.mjs index 0cf47dae..d2f7e4f5 100644 --- a/bench/test/grade.test.mjs +++ b/bench/test/grade.test.mjs @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { checkRule, extractAnswer, gradeAnswer, resolvePath } from "../grade.mjs"; +import { checkRule, checkSameAsAnswer, extractAnswer, gradeAnswer, resolvePath } from "../grade.mjs"; import { fill } from "../lib/intents.mjs"; const schema = { @@ -86,3 +86,19 @@ test("a prepare intent is correct only when the end state passes its check", () assert.deepEqual(grade({ error: "get_trip_summary is not live" }).problems, ["end state: get_trip_summary is not live"]); assert.equal(gradeAnswer(intent, schema, text).end_state, null); }); + +test("a prepare intent's answer must report what the check tool read on the site", () => { + const prepare = { ...intent, check: { tool: "get_trip_summary", rules: [], same_as_answer: [{ answer: "asn", end_state: "connection.asn" }, { answer: "ip", end_state: "ips[]" }] } }; + const text = '{"ip": "8.8.8.8", "country_code": "US", "asn": "AS15169"}'; + const grade = (output) => gradeAnswer(prepare, schema, text, { output }); + assert.equal(grade({ connection: { asn: "as 15169" }, ips: ["8.8.8.8"] }).outcome, "correct"); + const elsewhere = grade({ connection: { asn: "AS15170" }, ips: ["8.8.8.8"] }); + assert.deepEqual([elsewhere.outcome, elsewhere.end_state, elsewhere.problems], ["wrong", false, ['end state: asn is ["as15169"] in the answer but connection.asn is ["as15170"] on the site']]); + assert.equal(grade({ connection: { asn: "AS15169" }, ips: [] }).outcome, "wrong"); +}); + +test("amounts match to the cent", () => { + assert.equal(checkSameAsAnswer({ answer: "total", end_state: "amount_due" }, { total: 143.4 }, { amount_due: 143.40000001 }), null); + assert.match(checkSameAsAnswer({ answer: "total", end_state: "amount_due" }, { total: 143.4 }, { amount_due: 150.2 }), /\[143\.4\] in the answer but amount_due is \[150\.2\]/); + assert.match(checkSameAsAnswer({ answer: "total", end_state: "amount_due" }, { total: 143.4 }, { amount_due: null }), /\[null\] on the site/); +}); diff --git a/bench/test/report.test.mjs b/bench/test/report.test.mjs index 5de944ae..3b5ab0c7 100644 --- a/bench/test/report.test.mjs +++ b/bench/test/report.test.mjs @@ -240,4 +240,12 @@ test("tool failures are counted per tool, with the most common errors first", () { intent: "d", arm: "control", webmcp: { invocations: [invocation("search", "error", null, "control never counts")] } }, ]; assert.deepEqual(toolFailures(rows, () => "x.com"), [{ tool: "x.com/search", calls: 4, failed: 3, errors: [["UI_CHANGED", 2], ["schema", 1]] }]); + + const reportRows = exampleRows(); + reportRows[10].webmcp = { adopted: true, invocations: [invocation("get_theater_showtimes", "completed", "error", "UI_CHANGED"), { path: "playwright", tool: null, status: null, tool_status: null, error: null, duration_ms: null }] }; + const run = { + run_id: "t", model: "m", thinking: null, agent: "pi 0", created_at: "2026-10-07T00:00:00Z", + intents: { "fandango-theater-showtimes": { intent: { id: "fandango-theater-showtimes", domain: "fandango.com", stage: "inspect" }, sources: [{ ref: "main", sha: "0".repeat(40) }], cap_sec: 600 } }, + }; + assert.match(buildReport(run, reportRows), /in scored treatment trials\. 1 invocation\(s\) made through Playwright or the REPL have no recorded result and are not counted\.\n\n\| tool \|[^\n]*\n[^\n]*\n\| fandango\.com\/get_theater_showtimes \| 9 \| 1 \| 1× UI_CHANGED \|/); }); diff --git a/bench/test/tasks.test.mjs b/bench/test/tasks.test.mjs index a3ad3698..f49b421a 100644 --- a/bench/test/tasks.test.mjs +++ b/bench/test/tasks.test.mjs @@ -187,6 +187,7 @@ test("a prepare intent needs a check, and only prepare intents take checks and a assert.ok(validateIntent({ ...intent, stage: "find" }, benchDir).some(p => p.startsWith("check, and actions"))); assert.ok(validateIntent({ ...intent, check: { tool: "get_trip_summary", rules: [] } }, benchDir).some(p => p.startsWith("check needs"))); assert.ok(validateIntent({ ...intent, check: { ...check, rules: [{ path: "step", equals: "{{cabin}}" }] } }, benchDir).includes("{{cabin}} has no fixture value")); + assert.ok(validateIntent({ ...intent, check: { ...check, same_as_answer: [{ answer: "amount_due" }] } }, benchDir).includes("check.same_as_answer must list {answer, end_state} paths")); }); test("a prepare intent's actions are registered, and must exist", async () => {