diff --git a/bench/README.md b/bench/README.md index 750842e2..e47dc433 100644 --- a/bench/README.md +++ b/bench/README.md @@ -127,6 +127,13 @@ 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. `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 @@ -178,6 +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 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 @@ -201,9 +211,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..eabdaa10 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"; @@ -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`; @@ -119,8 +129,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 +147,15 @@ 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)), + ...(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; + } grade.outcome = "correct"; return grade; } @@ -148,5 +169,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..37f17d34 100644 --- a/bench/lib/intents.mjs +++ b/bench/lib/intents.mjs @@ -76,7 +76,16 @@ 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"); + 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(", ")}`); 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 +98,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..229c3694 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,16 @@ 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) { + 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("
"), + ]))); + } + 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..8f2f25df --- /dev/null +++ b/bench/tasks/delta.com/delta-cart-main-classic.json @@ -0,0 +1,34 @@ +{ + "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"} + ], + "same_as_answer": [{"answer": "amount_due", "end_state": "amount_due"}] + }, + "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..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 = { @@ -75,3 +75,30 @@ 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); +}); + +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 37b7ca58..3b5ab0c7 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,21 @@ 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]] }]); + + 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 3390904f..f49b421a 100644 --- a/bench/test/tasks.test.mjs +++ b/bench/test/tasks.test.mjs @@ -176,5 +176,23 @@ 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")); + 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 () => { + 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/); });