mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(ai): report malformed final tool-call arguments instead of keeping the parsed prefix (#10944)
When a finished tool call's arguments text was not valid JSON, the
streaming parser kept only the fields before the error and the tool ran
with them. Finished calls are now parsed strictly (with the existing
string repair); on failure arguments are {} and argumentsError holds the
reason and raw text, which validateToolArguments reports to the model.
fixes #10897
This commit is contained in:
parent
b45e0ebcc5
commit
87f9a36419
12 changed files with 125 additions and 11 deletions
|
|
@ -21,6 +21,7 @@
|
|||
- Fixed Gemini 4 Argon requests on Google Vertex failing with "Thinking budget is not supported", and its thinking level being ignored on the Gemini API, by sending `thinkingLevel` for Gemini 4 and later models ([#10890](https://github.com/earendil-works/pi/issues/10890))
|
||||
- Fixed Xiaomi MiMo models failing with a 400 at the `minimal` thinking level by sending only the documented thinking on/off toggle and exposing only the `off` and `high` levels ([#10818](https://github.com/earendil-works/pi/issues/10818))
|
||||
- Fixed `openai-codex/gpt-6.1-sol` using a 272K context window instead of the 500K window Codex now provides ([#10840](https://github.com/earendil-works/pi/issues/10840))
|
||||
- Fixed tool calls whose final arguments are malformed JSON silently running with only the fields parsed before the error; such calls now carry `argumentsError` and fail validation with the raw arguments text, so the model sees what it sent ([#10897](https://github.com/earendil-works/pi/issues/10897))
|
||||
|
||||
## [1.2.0] - 2026-10-12
|
||||
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ import type {
|
|||
import { appendAssistantMessageDiagnostic } from "../utils/diagnostics.ts";
|
||||
import { AssistantMessageEventStream } from "../utils/event-stream.ts";
|
||||
import { headersToRecord } from "../utils/headers.ts";
|
||||
import { parseJsonWithRepair, parseStreamingJson } from "../utils/json-parse.ts";
|
||||
import { parseJsonWithRepair, parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts";
|
||||
import { getPiUserAgent } from "../utils/pi-user-agent.ts";
|
||||
import { getProviderEnvValue } from "../utils/provider-env.ts";
|
||||
import { retryProviderRequest } from "../utils/provider-retry.ts";
|
||||
|
|
@ -833,7 +833,7 @@ export const stream: StreamFunction<"anthropic-messages", AnthropicOptions> = (
|
|||
partial: output,
|
||||
});
|
||||
} else if (block.type === "toolCall") {
|
||||
block.arguments = parseStreamingJson(block.partialJson);
|
||||
Object.assign(block, parseToolCallArguments(block.partialJson));
|
||||
// Finalize in-place and strip the scratch buffer so replay only
|
||||
// carries parsed arguments.
|
||||
delete (block as { partialJson?: string }).partialJson;
|
||||
|
|
|
|||
|
|
@ -53,7 +53,7 @@ import { appendAssistantMessageDiagnostic } from "../utils/diagnostics.ts";
|
|||
import { normalizeProviderError } from "../utils/error-body.ts";
|
||||
import { AssistantMessageEventStream } from "../utils/event-stream.ts";
|
||||
import { providerHeadersToRecord } from "../utils/headers.ts";
|
||||
import { parseStreamingJson } from "../utils/json-parse.ts";
|
||||
import { parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts";
|
||||
import { resolveHttpProxyUrlForTarget } from "../utils/node-http-proxy.ts";
|
||||
import { getProviderEnvValue } from "../utils/provider-env.ts";
|
||||
import { sanitizeSurrogates } from "../utils/sanitize-unicode.ts";
|
||||
|
|
@ -741,7 +741,7 @@ function handleContentBlockStop(
|
|||
stream.push({ type: "thinking_end", contentIndex: index, content: block.thinking, partial: output });
|
||||
break;
|
||||
case "toolCall":
|
||||
block.arguments = parseStreamingJson(block.partialJson);
|
||||
Object.assign(block, parseToolCallArguments(block.partialJson));
|
||||
// Finalize in-place and strip the scratch buffer so replay only
|
||||
// carries parsed arguments.
|
||||
delete (block as Block).partialJson;
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ import { combineAbortSignals } from "../utils/abort-signals.ts";
|
|||
import { AssistantMessageEventStream } from "../utils/event-stream.ts";
|
||||
import { shortHash } from "../utils/hash.ts";
|
||||
import { headersToRecord } from "../utils/headers.ts";
|
||||
import { parseStreamingJson } from "../utils/json-parse.ts";
|
||||
import { parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts";
|
||||
import { getPiUserAgent } from "../utils/pi-user-agent.ts";
|
||||
import { sanitizeSurrogates } from "../utils/sanitize-unicode.ts";
|
||||
import { getSystemMessageText, renderSystemMessageUpdate } from "../utils/text.ts";
|
||||
|
|
@ -761,7 +761,7 @@ async function consumeChatStream(
|
|||
const block = output.content[index];
|
||||
if (block.type !== "toolCall") continue;
|
||||
const toolBlock = block as ToolCall & { partialArgs?: string };
|
||||
toolBlock.arguments = parseStreamingJson<ToolCall["arguments"]>(toolBlock.partialArgs);
|
||||
Object.assign(toolBlock, parseToolCallArguments(toolBlock.partialArgs));
|
||||
// Finalize in-place and strip the scratch buffer so replay only
|
||||
// carries parsed arguments.
|
||||
delete toolBlock.partialArgs;
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@ import { formatProviderError, normalizeProviderError } from "../utils/error-body
|
|||
import { AssistantMessageEventStream } from "../utils/event-stream.ts";
|
||||
import { shortHash } from "../utils/hash.ts";
|
||||
import { headersToRecord } from "../utils/headers.ts";
|
||||
import { parseStreamingJson } from "../utils/json-parse.ts";
|
||||
import { parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts";
|
||||
import { getPiUserAgent } from "../utils/pi-user-agent.ts";
|
||||
import { getProviderEnvValue } from "../utils/provider-env.ts";
|
||||
import { retryProviderRequest } from "../utils/provider-retry.ts";
|
||||
|
|
@ -461,7 +461,7 @@ export const stream: StreamFunction<"openai-completions", OpenAICompletionsOptio
|
|||
});
|
||||
}
|
||||
} else {
|
||||
block.arguments = parseStreamingJson(block.partialArgs);
|
||||
Object.assign(block, parseToolCallArguments(block.partialArgs));
|
||||
}
|
||||
// Finalize in-place and strip the scratch buffers so replay only
|
||||
// carries parsed arguments.
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ import type {
|
|||
} from "../types.ts";
|
||||
import type { AssistantMessageEventStream } from "../utils/event-stream.ts";
|
||||
import { shortHash } from "../utils/hash.ts";
|
||||
import { parseStreamingJson } from "../utils/json-parse.ts";
|
||||
import { parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts";
|
||||
import { sanitizeSurrogates } from "../utils/sanitize-unicode.ts";
|
||||
import { getSystemMessageText, renderSystemMessageUpdate } from "../utils/text.ts";
|
||||
import { resolveTranscript, resolveTranscriptTools } from "../utils/transcript.ts";
|
||||
|
|
@ -738,7 +738,7 @@ export async function processResponsesStream<TApi extends Api>(
|
|||
slot?.type === "toolCall" &&
|
||||
slot.block.partialJson !== undefined
|
||||
) {
|
||||
slot.block.arguments = parseStreamingJson(item.arguments || slot.block.partialJson || "{}");
|
||||
Object.assign(slot.block, parseToolCallArguments(item.arguments || slot.block.partialJson));
|
||||
if (item.namespace !== undefined) slot.block.namespace = item.namespace;
|
||||
// Finalize in-place and strip the scratch buffer so replay only
|
||||
// carries parsed arguments.
|
||||
|
|
|
|||
|
|
@ -441,6 +441,11 @@ export interface ToolCall {
|
|||
thoughtSignature?: string; // Google-specific: opaque signature for reusing thought context
|
||||
/** OpenAI Responses namespace for calls to dynamically loaded or namespaced tools. */
|
||||
namespace?: string;
|
||||
/**
|
||||
* Set when the model's complete arguments text was not a valid JSON object. `arguments` is then `{}`.
|
||||
* Describes the problem and includes the text the model sent; tool argument validation fails with it.
|
||||
*/
|
||||
argumentsError?: string;
|
||||
}
|
||||
|
||||
export interface Usage {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { parse as partialParse } from "partial-json";
|
||||
import type { ToolCall } from "../types.ts";
|
||||
|
||||
const VALID_JSON_ESCAPES = new Set(['"', "\\", "/", "b", "f", "n", "r", "t"]);
|
||||
|
||||
|
|
@ -94,6 +95,30 @@ export function parseJsonWithRepair<T>(json: string): T {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parses the model's complete arguments text of a finished tool call.
|
||||
*
|
||||
* Unlike {@link parseStreamingJson}, this never keeps a partially parsed prefix: once the call is
|
||||
* finished, text that is not a valid JSON object means the model sent malformed arguments. In that
|
||||
* case `arguments` is `{}` and `argumentsError` describes the problem, including the text the
|
||||
* model sent, so tool validation can report it back to the model.
|
||||
*/
|
||||
export function parseToolCallArguments(json: string | undefined): Pick<ToolCall, "arguments" | "argumentsError"> {
|
||||
if (!json?.trim()) return { arguments: {} };
|
||||
try {
|
||||
const parsed = parseJsonWithRepair<ToolCall["arguments"] | null>(json);
|
||||
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
|
||||
throw new Error("Tool arguments must be a JSON object");
|
||||
}
|
||||
return { arguments: parsed };
|
||||
} catch (error) {
|
||||
return {
|
||||
arguments: {},
|
||||
argumentsError: `${error instanceof Error ? error.message : String(error)}\n\nReceived arguments:\n${json}`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Attempts to parse potentially incomplete JSON during streaming.
|
||||
* Always returns a valid object, even if the JSON is incomplete.
|
||||
|
|
|
|||
|
|
@ -315,6 +315,9 @@ export function validateToolCall(tools: Tool[], toolCall: ToolCall): any {
|
|||
* @throws Error with formatted message if validation fails
|
||||
*/
|
||||
export function validateToolArguments(tool: Tool, toolCall: ToolCall): any {
|
||||
if (toolCall.argumentsError) {
|
||||
throw new Error(`Invalid arguments for tool "${toolCall.name}": ${toolCall.argumentsError}`);
|
||||
}
|
||||
const args = structuredClone(toolCall.arguments);
|
||||
normalizeOptionalNulls(args, tool.parameters as JsonSchemaObject);
|
||||
Value.Convert(tool.parameters, args);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { parseStreamingJson } from "../src/utils/json-parse.ts";
|
||||
import { parseStreamingJson, parseToolCallArguments } from "../src/utils/json-parse.ts";
|
||||
|
||||
describe("parseStreamingJson", () => {
|
||||
// #10868
|
||||
|
|
@ -20,3 +20,38 @@ describe("parseStreamingJson", () => {
|
|||
expect(parseStreamingJson(String.raw`{"text":"ab\u00`)).toEqual({ text: "ab" });
|
||||
});
|
||||
});
|
||||
|
||||
// #10897
|
||||
describe("parseToolCallArguments", () => {
|
||||
it("parses complete JSON and repairs invalid escapes", () => {
|
||||
expect(parseToolCallArguments(String.raw`{"kind":"seek","note":"\url"}`)).toEqual({
|
||||
arguments: { kind: "seek", note: "\\url" },
|
||||
});
|
||||
});
|
||||
|
||||
it("uses empty arguments for empty text", () => {
|
||||
expect(parseToolCallArguments("")).toEqual({ arguments: {} });
|
||||
});
|
||||
|
||||
it("does not keep the valid prefix of malformed JSON", () => {
|
||||
const raw = '{"action":"apply","actions":[{"kind":"seek","time":.5}]}';
|
||||
const result = parseToolCallArguments(raw);
|
||||
expect(result.arguments).toEqual({});
|
||||
expect(result.argumentsError).toMatch(/\n\nReceived arguments:\n/);
|
||||
expect(result.argumentsError?.endsWith(raw)).toBe(true);
|
||||
});
|
||||
|
||||
it("does not complete cut-off JSON", () => {
|
||||
const raw = '{"kind":"seek","clipId":"clip-fl';
|
||||
const result = parseToolCallArguments(raw);
|
||||
expect(result.arguments).toEqual({});
|
||||
expect(result.argumentsError?.endsWith(raw)).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects JSON that is not an object", () => {
|
||||
expect(parseToolCallArguments("[1,2]")).toEqual({
|
||||
arguments: {},
|
||||
argumentsError: "Tool arguments must be a JSON object\n\nReceived arguments:\n[1,2]",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -103,4 +103,30 @@ describe("openai responses partialJson cleanup", () => {
|
|||
expect(toolCallEnd.toolCall).toBe(persistedToolCall);
|
||||
expect("partialJson" in toolCallEnd.toolCall).toBe(false);
|
||||
});
|
||||
|
||||
// #10897
|
||||
it("marks malformed final arguments instead of keeping the parsed prefix", async () => {
|
||||
const model: Model<"openai-responses"> = {
|
||||
id: "gpt-5-mini",
|
||||
name: "GPT-5 Mini",
|
||||
api: "openai-responses",
|
||||
provider: "openai",
|
||||
baseUrl: "https://api.openai.com/v1",
|
||||
reasoning: true,
|
||||
input: ["text"],
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||
contextWindow: 400000,
|
||||
maxTokens: 128000,
|
||||
};
|
||||
const output = createOutput(model);
|
||||
const stream = new AssistantMessageEventStream();
|
||||
const argumentsJson = '{"path":"README.md","content":"updated","mode":.5}';
|
||||
|
||||
await processResponsesStream(createFunctionCallEvents(argumentsJson), output, stream, model);
|
||||
|
||||
const toolCall = output.content[0];
|
||||
if (!toolCall || toolCall.type !== "toolCall") throw new Error("Expected toolCall block");
|
||||
expect(toolCall.arguments).toEqual({});
|
||||
expect(toolCall.argumentsError?.endsWith(`Received arguments:\n${argumentsJson}`)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -207,4 +207,23 @@ describe("validateToolArguments", () => {
|
|||
expect(() => validateToolArguments(tool, toolCall)).toThrow("Validation failed");
|
||||
}
|
||||
});
|
||||
// #10897
|
||||
it("reports malformed arguments JSON instead of validating empty arguments", () => {
|
||||
const tool: Tool = {
|
||||
name: "echo",
|
||||
description: "Echo tool",
|
||||
parameters: Type.Object({ value: Type.Optional(Type.String()) }),
|
||||
};
|
||||
const toolCall: ToolCall = {
|
||||
type: "toolCall",
|
||||
id: "tool-1",
|
||||
name: "echo",
|
||||
arguments: {},
|
||||
argumentsError: "Unexpected token",
|
||||
};
|
||||
|
||||
expect(() => validateToolArguments(tool, toolCall)).toThrow(
|
||||
'Invalid arguments for tool "echo": Unexpected token',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue