Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
import {
desanitizeFromConvexTransport,
sanitizeForConvexTransport,
toPersistedToolCalls,
} from "../convex-sanitize.js";

describe("sanitizeForConvexTransport", () => {
Expand Down Expand Up @@ -47,3 +48,68 @@ describe("sanitizeForConvexTransport", () => {
).toEqual(original);
});
});

describe("toPersistedToolCalls", () => {
it("returns an empty array for an iteration that called no tools", () => {
// The common case: `actualToolCalls: []` is what a no-tool iteration
// persists, and the validator requires an array, not a missing field.
expect(toPersistedToolCalls([])).toEqual([]);
});

it("keeps exactly the fields the backend validator accepts", () => {
expect(
toPersistedToolCalls([
{
toolName: "connector_list",
arguments: { includeSourceConnectors: true },
},
{
toolName: "search",
arguments: { q: "coffee" },
toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM",
},
]),
).toEqual([
{
toolName: "connector_list",
arguments: { includeSourceConnectors: true },
},
{
toolName: "search",
arguments: { q: "coffee" },
toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM",
},
]);
});

it("drops a field the strict validator would reject (CONVEX-1QF)", () => {
// `updateTestIteration.actualToolCalls` is a strict `v.object`: an
// unrecognized key fails the whole finalize call, so the boundary has to
// project rather than trust whatever the runner attached upstream.
const calls = toPersistedToolCalls([
{
toolName: "connector_list",
arguments: { includeSourceConnectors: true },
toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM",
providerExecuted: true,
state: "output-available",
} as never,
]);

expect(Object.keys(calls[0]!).sort()).toEqual([
"arguments",
"toolCallId",
"toolName",
]);
});

it("omits toolCallId rather than sending undefined when absent", () => {
// `v.optional(v.string())` accepts a missing key; an explicit `undefined`
// is what Convex serialization rejects.
const [call] = toPersistedToolCalls([
{ toolName: "echo", arguments: {}, toolCallId: undefined },
]);

expect("toolCallId" in call!).toBe(false);
});
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,41 @@ describe("finalizeEvalIteration", () => {
expect(update!.args.messages).toBeDefined();
});

test("actualToolCalls reaches the wire projected to the persisted shape", async () => {
// CONVEX-1QF: `updateTestIteration.actualToolCalls` is a strict Convex
// `v.object`, so one unrecognized field fails the whole finalize rather
// than being dropped. The mapper's unit tests cover the projection; this
// pins that finalize actually applies it to what it sends.
const { client, calls } = makeClient({});
await finalizeEvalIteration({
convexClient: client,
iterationId: "iter1",
passed: true,
toolsCalled: [
{
toolName: "connector_list",
arguments: { includeSourceConnectors: true },
toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM",
// Not in the validator — the shape the runner could drift into.
providerExecuted: true,
} as never,
],
usage: usageZero,
messages,
});
const update = calls.find(
(c) => c.ref === "testSuites:updateTestIteration",
);
expect(update).toBeDefined();
expect(update!.args.actualToolCalls).toEqual([
{
toolName: "connector_list",
arguments: { includeSourceConnectors: true },
toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM",
},
]);
});

test("W1 fallback omits systemPrompt when unset", async () => {
const { client, calls } = makeClient({
appendThrows: new Error("fanout pre-turn failure"),
Expand Down
44 changes: 44 additions & 0 deletions mcpjam-inspector/server/services/evals/convex-sanitize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,47 @@ export {
sanitizeForConvexTransport,
desanitizeFromConvexTransport,
} from "@/shared/convex-sanitize";

/**
* The exact persisted shape of one `testIteration.actualToolCalls` entry —
* mirrors the backend's `evalIterationToolCallValidator` (mcpjam-backend
* `convex/lib/evalAnalysis.ts`), which is a strict `v.object`.
*/
export type PersistedToolCall = {
toolName: string;
arguments: Record<string, unknown>;
toolCallId?: string;
};

/**
* Project tool calls onto exactly the fields that
* `updateTestIteration.actualToolCalls` accepts, dropping anything else.
*
* Convex object validators are STRICT: one unrecognized field is a hard
* `ArgumentValidationError`, not a silent drop — so an extra key here doesn't
* degrade a write, it fails the whole iteration finalize. That is what
* CONVEX-1QF was: the runner began attaching `toolCallId` (inspector #4308, to
* filter policy-blocked calls by id) and every eval iteration carrying a tool
* call stopped persisting until the validator was widened (backend #1134).
*
* Each producer currently builds these objects field-by-field, so today's
* payload is already clean. This makes that a property of the boundary rather
* than of every producer remembering: the runner reads tool calls out of
* `any`-typed AI SDK step objects, so the next field added upstream would
* otherwise reach the validator the same way.
*/
export function toPersistedToolCalls(
toolCalls: ReadonlyArray<{
toolName: string;
arguments: Record<string, unknown>;
toolCallId?: string;
}>,
): PersistedToolCall[] {
return toolCalls.map((call) => ({
toolName: call.toolName,
arguments: call.arguments,
...(typeof call.toolCallId === "string"
? { toolCallId: call.toolCallId }
: {}),
}));
}
9 changes: 7 additions & 2 deletions mcpjam-inspector/server/services/evals/finalize-iteration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import type {
import { logger } from "../../utils/logger.js";
import { uploadVideoBlob } from "../../utils/mcp-app-widget-capture.js";
import type { UsageTotals } from "./types.js";
import { sanitizeForConvexTransport } from "./convex-sanitize.js";
import {
sanitizeForConvexTransport,
toPersistedToolCalls,
} from "./convex-sanitize.js";
import { emitBrowserEvalMetrics } from "./browser-eval-metrics.js";
import {
serializeBrowserStepsForBackend,
Expand Down Expand Up @@ -964,7 +967,9 @@ export async function finalizeEvalIteration(
iterationId,
status: iterationStatus === "completed" ? "completed" : iterationStatus,
result,
actualToolCalls: sanitizeForConvexTransport(toolsCalled),
actualToolCalls: sanitizeForConvexTransport(
toPersistedToolCalls(toolsCalled),
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
tokensUsed: usage.totalTokens ?? 0,
...(useW1Fallback
? {
Expand Down
Loading