From 1447fd2f990c5d9820337dfd4b8a170f8cad17cd Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:12:12 +0800 Subject: [PATCH] fix(evaluation): share completed transcript facts across graders --- src/evaluation/CodexAgentEval.ts | 66 +++----- src/evaluation/KnownAnswerEvaluation.ts | 148 +++++++++++------ .../model-evals/codexAgentEval.test.ts | 154 +++++++++++++++--- 3 files changed, 257 insertions(+), 111 deletions(-) diff --git a/src/evaluation/CodexAgentEval.ts b/src/evaluation/CodexAgentEval.ts index bac8d8191..55ec8333e 100644 --- a/src/evaluation/CodexAgentEval.ts +++ b/src/evaluation/CodexAgentEval.ts @@ -1,10 +1,11 @@ import { - deliveredCodexEvidence, evaluateKnownAnswers, + parseCodexToolCall, type FixtureClaimExpectation, type KnownAnswerAssessment, + type ParsedCodexToolCall, } from "./KnownAnswerEvaluation.js"; -import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; +import { canonicalJson } from "../domain/comparisonSemantics.js"; import { TOOL_CONTRACTS } from "../contracts/toolContracts.js"; /** One REA MCP invocation observed in a Codex JSONL transcript. */ @@ -57,17 +58,6 @@ const textValue = (value: unknown): string | undefined => const numberValue = (value: unknown): number => typeof value === "number" && Number.isFinite(value) ? value : 0; -const canonicalValue = (value: unknown): unknown => { - if (Array.isArray(value)) return value.map(canonicalValue); - const object = record(value); - if (object === undefined) return value; - return Object.fromEntries( - Object.entries(object) - .sort(([left], [right]) => compareUnicodeCodePoints(left, right)) - .map(([key, child]) => [key, canonicalValue(child)]), - ); -}; - const toolResultError = ( item: Record, ): Record | undefined => { @@ -113,29 +103,18 @@ const toolResultFailed = ( ); }; -const callFromEvent = ( - event: Record, +const callFromParsed = ( + parsed: ParsedCodexToolCall | undefined, ): CodexMcpCall | undefined => { - const item = record( - event.type === "item.completed" - ? event.item - : event.type === "mcp_tool_call" - ? event - : undefined, - ); - if (item?.type !== "mcp_tool_call") return undefined; - const tool = textValue(item.tool) ?? textValue(item.name); - if (tool === undefined) return undefined; - const error = toolResultError(item); + if (parsed === undefined) return undefined; + const error = toolResultError(parsed.item); return { - id: textValue(item.id) ?? null, - server: textValue(item.server) ?? textValue(item.server_name) ?? null, - tool, - arguments: item.arguments ?? item.input ?? {}, - evidenceIds: deliveredCodexEvidence(event).map( - ({ evidence_id }) => evidence_id, - ), - error: toolResultFailed(item, error), + id: parsed.id, + server: parsed.server, + tool: parsed.tool, + arguments: parsed.arguments, + evidenceIds: parsed.evidence.map(({ evidence_id }) => evidence_id), + error: toolResultFailed(parsed.item, error), errorCode: textValue(error?.code) ?? null, }; }; @@ -163,16 +142,20 @@ interface EvaluationState { outputTokens: number; } -const consumeEvent = (state: EvaluationState, value: unknown): void => { +const consumeEvent = ( + state: EvaluationState, + value: unknown, + parsedCall: ParsedCodexToolCall | undefined, +): void => { const event = record(value); if (event === undefined) return; const completedItem = event.type === "item.completed" ? event.item : undefined; - const call = callFromEvent(event); + const call = callFromParsed(parsedCall); if (call !== undefined) { const identity = call.id ?? - `${call.server ?? ""}:${call.tool}:${JSON.stringify(canonicalValue(call.arguments))}`; + `${call.server ?? ""}:${call.tool}:${canonicalJson(call.arguments)}`; if (!state.seenCallIds.has(identity)) { state.seenCallIds.add(identity); state.calls.push(call); @@ -216,7 +199,10 @@ export const evaluateCodexEvents = ( cachedInputTokens: 0, outputTokens: 0, }; - for (const event of events) consumeEvent(state, event); + const parsedCalls = events.map(parseCodexToolCall); + events.forEach((event, index) => + consumeEvent(state, event, parsedCalls[index]), + ); const reaCalls = state.calls.filter( ({ server }) => server === null || server.toLowerCase() === "rea", @@ -228,7 +214,7 @@ export const evaluateCodexEvents = ( // useful verification, not a duplicate analysis or unchanged failed retry. const context = call.tool === "binary_session" ? `${String(targetRevision)}:` : ""; - const signature = `${context}${call.tool}:${JSON.stringify(canonicalValue(call.arguments))}`; + const signature = `${context}${call.tool}:${canonicalJson(call.arguments)}`; signatures.set(signature, (signatures.get(signature) ?? 0) + 1); if ( !call.error && @@ -271,7 +257,7 @@ export const evaluateCodexEvents = ( finalMessage, ); const factualAssessment = evaluateKnownAnswers( - events, + parsedCalls, finalMessage, options.fixtureClaims, ); diff --git a/src/evaluation/KnownAnswerEvaluation.ts b/src/evaluation/KnownAnswerEvaluation.ts index 807308b59..c6e4af72b 100644 --- a/src/evaluation/KnownAnswerEvaluation.ts +++ b/src/evaluation/KnownAnswerEvaluation.ts @@ -3,7 +3,7 @@ import { z } from "zod"; import { parseEvidence, type Evidence } from "../domain/evidence.js"; import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; -import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; +import { canonicalJson } from "../domain/comparisonSemantics.js"; /** One closed fixture assertion and the producer facts that must support it. */ export interface FixtureClaimExpectation { @@ -104,23 +104,13 @@ const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); const record = (value: unknown): Record | undefined => isRecord(value) ? value : undefined; -const canonicalValue = (value: JsonValue): JsonValue => { - if (Array.isArray(value)) return value.map(canonicalValue); - if (value === null || typeof value !== "object") return value; - return Object.fromEntries( - Object.entries(value) - .sort(([left], [right]) => compareUnicodeCodePoints(left, right)) - .map(([key, child]) => [key, canonicalValue(child)]), - ); -}; const sameJson = (left: unknown, right: unknown): boolean => { const parsedLeft = jsonValueSchema.safeParse(left); const parsedRight = jsonValueSchema.safeParse(right); return ( parsedLeft.success && parsedRight.success && - JSON.stringify(canonicalValue(parsedLeft.data)) === - JSON.stringify(canonicalValue(parsedRight.data)) + canonicalJson(parsedLeft.data) === canonicalJson(parsedRight.data) ); }; @@ -168,6 +158,16 @@ interface DeliveredEvidence { readonly conflicts: Set; } +/** One completed tool call, with aliases normalized and successful Evidence authenticated. */ +export interface ParsedCodexToolCall { + readonly item: Record; + readonly id: string | null; + readonly server: string | null; + readonly tool: string; + readonly arguments: JsonValue; + readonly evidence: readonly Evidence[]; +} + const consistentAliases = ( value: Record, left: string, @@ -177,30 +177,14 @@ const consistentAliases = ( value[right] === undefined || value[left] === value[right]; -const completedCall = (event: unknown): Record | undefined => { - const value = record(event); - const item = - value?.type === "item.completed" - ? record(value.item) - : value?.type === "mcp_tool_call" && value.status === "completed" - ? value - : undefined; - if (item?.type !== "mcp_tool_call") return undefined; - const server = item.server ?? item.server_name; - if ( - typeof server !== "string" || - server.toLowerCase() !== "rea" || - (item.status !== undefined && item.status !== "completed") || - (item.error !== undefined && item.error !== null) - ) - return undefined; - if ( - !consistentAliases(item, "server", "server_name") || - !consistentAliases(item, "tool", "name") - ) - return undefined; - return item; -}; +const consistentJsonFields = ( + value: Record, + left: string, + right: string, +): boolean => + value[left] === undefined || + value[right] === undefined || + sameJson(value[left], value[right]); const consistentJsonAliases = ( value: Record, @@ -215,6 +199,7 @@ const consistentJsonAliases = ( const evidenceFromCall = ( item: Record, + tool: string, ): Evidence | undefined => { if (!consistentJsonAliases(item, "result", "output")) return undefined; const result = record(item.result ?? item.output); @@ -254,22 +239,42 @@ const evidenceFromCall = ( // Transcript output is untrusted; invalid Evidence cannot support a claim. return undefined; } - if ((item.tool ?? item.name) !== evidence.operation) return undefined; + if (tool !== evidence.operation) return undefined; return evidence; }; -/** Authenticate completed REA deliveries for both citations and fixture grading. */ -export const deliveredCodexEvidence = (event: unknown): readonly Evidence[] => { - const item = completedCall(event); - if (item === undefined) return []; - const evidence = evidenceFromCall(item); - if (evidence === undefined) return []; +const completedToolItem = ( + event: unknown, +): Record | undefined => { + const value = record(event); + const wrapped = value?.type === "item.completed"; + const item = wrapped + ? record(value.item) + : value?.type === "mcp_tool_call" + ? value + : undefined; + if (item?.type !== "mcp_tool_call") return undefined; + const terminalStatus = + item.status === "completed" || item.status === "failed"; + if ( + (wrapped + ? item.status !== undefined && !terminalStatus + : !terminalStatus) || + !consistentAliases(item, "server", "server_name") || + !consistentAliases(item, "tool", "name") || + !consistentJsonFields(item, "arguments", "input") + ) + return undefined; + return item; +}; + +const artifactDelivery = (evidence: Evidence): readonly Evidence[] => { const records = [evidence]; - // Artifact inspection delivers its inventory Evidence inline. Bare links and - // arbitrary IDs in normalized facts are references, not delivered records. if (evidence.operation !== "inspect_artifact") return records; const substeps = record(evidence.normalized_result)?.substeps; if (!Array.isArray(substeps)) return records; + // Artifact inspection delivers linked inventory Evidence inline; bare IDs in + // normalized facts are references, not authenticated deliveries. for (const value of substeps) { const substep = record(value); if ( @@ -293,13 +298,56 @@ export const deliveredCodexEvidence = (event: unknown): readonly Evidence[] => { return records; }; +/** Parse one completed transcript call and authenticate any successful REA Evidence. */ +export const parseCodexToolCall = ( + event: unknown, +): ParsedCodexToolCall | undefined => { + const item = completedToolItem(event); + if (item === undefined) return undefined; + const status = item.status; + const server = item.server ?? item.server_name; + const tool = item.tool ?? item.name; + if ( + ((item.server !== undefined || item.server_name !== undefined) && + typeof server !== "string") || + typeof tool !== "string" || + tool.length === 0 + ) + return undefined; + const argumentsValue = + item.arguments !== undefined + ? item.arguments + : item.input !== undefined + ? item.input + : {}; + const parsedArguments = jsonValueSchema.safeParse(argumentsValue); + if (!parsedArguments.success) return undefined; + const parsed: ParsedCodexToolCall = { + item, + id: typeof item.id === "string" && item.id.length > 0 ? item.id : null, + server: typeof server === "string" ? server : null, + tool, + arguments: parsedArguments.data, + evidence: [], + }; + if ( + parsed.server?.toLowerCase() !== "rea" || + status === "failed" || + (item.error !== undefined && item.error !== null) + ) + return parsed; + const evidence = evidenceFromCall(item, tool); + if (evidence === undefined) return parsed; + return { ...parsed, evidence: artifactDelivery(evidence) }; +}; + const collectDeliveredEvidence = ( - events: readonly unknown[], + calls: readonly (ParsedCodexToolCall | undefined)[], ): DeliveredEvidence => { const records = new Map(); const conflicts = new Set(); - for (const event of events) { - for (const evidence of deliveredCodexEvidence(event)) { + for (const call of calls) { + for (const evidence of call?.evidence ?? []) { const previous = records.get(evidence.evidence_id); if (previous !== undefined && !sameJson(previous, evidence)) conflicts.add(evidence.evidence_id); @@ -401,7 +449,7 @@ const assessClaim = ( /** Grade closed answers only against configured fixture truth and delivered authenticated Evidence. */ export const evaluateKnownAnswers = ( - events: readonly unknown[], + calls: readonly (ParsedCodexToolCall | undefined)[], finalMessage: string, expectations: readonly FixtureClaimExpectation[] | undefined, ): KnownAnswerAssessment => { @@ -441,7 +489,7 @@ export const evaluateKnownAnswers = ( manifestFailures.push(`unknown_claim:${claim.id}`); submitted.set(claim.id, claim); } - const delivered = collectDeliveredEvidence(events); + const delivered = collectDeliveredEvidence(calls); const claims = expectations.map((expected) => assessClaim(expected, submitted.get(expected.id), delivered), ); diff --git a/tests/evaluation/model-evals/codexAgentEval.test.ts b/tests/evaluation/model-evals/codexAgentEval.test.ts index d96265202..6b46d9b1d 100644 --- a/tests/evaluation/model-evals/codexAgentEval.test.ts +++ b/tests/evaluation/model-evals/codexAgentEval.test.ts @@ -134,6 +134,114 @@ describe("Codex agent release evaluation", () => { }); }); +describe("Codex completed tool call parsing", () => { + it("does not let an unfinished direct event reserve a completed call identity", () => { + const evidence = observation(); + const metrics = evaluateCodexEvents( + [ + { + type: "mcp_tool_call", + id: "same-call", + status: "in_progress", + server: "rea", + tool: evidence.operation, + arguments: {}, + }, + { + type: "item.completed", + item: { + id: "same-call", + type: "mcp_tool_call", + status: "completed", + server: "rea", + tool: evidence.operation, + arguments: {}, + result: { structuredContent: evidence }, + }, + }, + { + type: "item.completed", + item: { + type: "agent_message", + text: `Observed Evidence ${evidence.evidence_id} supports the artifact claim; runtime behavior remains unknown without runtime authority.`, + }, + }, + ], + evidence.operation, + { requireEvidence: true }, + ); + + expect(metrics).toMatchObject({ + naturalUse: true, + correctFirstTool: true, + reaCalls: [{ id: "same-call" }], + evidenceIds: [evidence.evidence_id], + finalCitesEvidence: true, + answerHeuristicsMet: true, + }); + }); + + it.each([ + { server: "rea", server_name: "other" }, + { tool: "inspect_managed_artifact", name: "other_tool" }, + { server: 42 }, + { arguments: { value: 1 }, input: { value: 2 } }, + ])( + "rejects completed calls with invalid or conflicting fields: %j", + (aliases) => { + const evidence = observation(); + const metrics = evaluateCodexEvents( + [ + { + type: "item.completed", + item: { + type: "mcp_tool_call", + status: "completed", + server: "rea", + tool: evidence.operation, + arguments: {}, + result: { structuredContent: evidence }, + ...aliases, + }, + }, + ], + evidence.operation, + { requiredToolSubsequence: [evidence.operation] }, + ); + + expect(metrics).toMatchObject({ + naturalUse: false, + correctFirstTool: false, + requiredToolSubsequenceMet: false, + evidenceIds: [], + }); + }, + ); + + it("preserves legacy completed calls with no server field", () => { + const metrics = evaluateCodexEvents( + [ + { + type: "item.completed", + item: { + type: "mcp_tool_call", + status: "completed", + tool: "inspect_managed_artifact", + arguments: {}, + }, + }, + ], + "inspect_managed_artifact", + ); + + expect(metrics).toMatchObject({ + naturalUse: true, + correctFirstTool: true, + reaCalls: [{ server: null }], + }); + }); +}); + describe("Codex Evidence delivery", () => { it("rejects diagnostic IDs and failed results as delivered Evidence", () => { const evidenceId = `ev_${"a".repeat(64)}`; @@ -524,28 +632,32 @@ describe("Codex agent completion evaluation", () => { }); }); - it("recognizes Codex MCP failed status when the error field is null", () => { - const metrics = evaluateCodexEvents( - [ - { - type: "item.completed", - item: { - id: "item-failed", - type: "mcp_tool_call", - server: "rea", - tool: "analyze_javascript_application", - arguments: { input_path: "/tmp/app.asar" }, - result: { structured_content: { error: { code: "denied" } } }, - error: null, - status: "failed", - }, - }, - ], - "analyze_javascript_application", - ); + it.each(["item.completed", "mcp_tool_call"] as const)( + "counts a failed completed invocation from %s without delivering Evidence", + (type) => { + const item = { + id: "item-failed", + type: "mcp_tool_call", + server: "rea", + tool: "analyze_javascript_application", + arguments: { input_path: "/tmp/app.asar" }, + result: { structured_content: { error: { code: "denied" } } }, + error: null, + status: "failed", + }; + const metrics = evaluateCodexEvents( + [type === "item.completed" ? { type, item } : item], + "analyze_javascript_application", + ); - expect(metrics.reaCalls[0]?.error).toBe(true); - }); + expect(metrics).toMatchObject({ + naturalUse: true, + correctFirstTool: true, + evidenceIds: [], + reaCalls: [{ error: true }], + }); + }, + ); }); describe("Codex agent workflow evaluation", () => {