fix(ai): emit concat-safe minimax object-args delta on toolcall_end
Codex review on #2082 caught that the merged-args fix still emitted `delta = JSON.stringify(rawArgs)` per object chunk. Every downstream consumer that follows the OpenAI `toolcall_delta` contract by concatenating deltas — `packages/agent/src/proxy.ts` reconstructing `partialJson` (lines 286-290), `openai-chat-server` forwarding `tool_calls[].function.arguments`, `openai-responses-server` appending to `cur.argsText`, `anthropic-messages-server` emitting `input_json_delta` — would have seen an invalid sequence like `{"input":"a"}{"input":"b"}` once a host fragmented the args across deltas, and `parseStreamingJson`'s repair-then-partial-parse fallback collapses that to `{}` or just the first object. The source-side `.result()` was correct because `block.arguments` already held the merged result, but every concat-based reader downstream lost the args. Suppress object-chunk wire deltas during streaming (the merge still runs into `block.partialArgs`/`block.arguments`) and flush the full merged JSON as a single concat-safe delta in `finishToolCallBlock` right before `toolcall_end`. Concat consumers now reconstruct the args unconditionally — single-chunk case is still `"" + full_json`, multi-chunk case is `"" + "" + … + full_json`, both parse to the same merged object. Two new regression tests in `issue-2080-repro.test.ts` accumulate `event.delta` the way `proxy.ts` does and assert `JSON.parse(accum)` matches the source-side merged args, covering both the multi-chunk fragmented-string shape and the single-chunk shape (no #1776 regression). The three pre-existing tests for the merge behaviour itself still pass unchanged.
This commit is contained in:
roboomp
@@ -14,7 +14,7 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed duplicate upstream `tool_call_id` values collapsing distinct tool calls during message transformation, preserving one call/result pairing per emitted tool call before provider replay and keeping generated duplicate IDs distinct after OpenAI/Mistral wire-length caps. ([#2055](https://github.com/can1357/oh-my-pi/issues/2055))
|
||||
- Fixed MiniMax-compatible OpenAI-completions hosts losing tool-call argument content when `function.arguments` is streamed as an object across more than one delta. The accumulator added in #1776 wrote `block.partialArgs = rawArgs` per chunk, so every chunk but the last was overwritten — for an `edit` call this surfaced as a tail-slice of the patch text being applied (e.g. a single-line `replace 91..91:` body extending the deletion across the surrounding rows). Chunks are now shallow-merged; for shared string keys, `startsWith` distinguishes cumulative restatements (take the latest) from per-chunk-delta fragments (concatenate), and the single-chunk shape covered by the existing #1776 regression test stays a no-op. ([#2080](https://github.com/can1357/oh-my-pi/issues/2080))
|
||||
- Fixed MiniMax-compatible OpenAI-completions hosts losing tool-call argument content when `function.arguments` is streamed as an object across more than one delta. The accumulator added in #1776 wrote `block.partialArgs = rawArgs` per chunk, so every chunk but the last was overwritten — for an `edit` call this surfaced as a tail-slice of the patch text being applied (e.g. a single-line `replace 91..91:` body extending the deletion across the surrounding rows). Chunks are now shallow-merged; for shared string keys, `startsWith` distinguishes cumulative restatements (take the latest) from per-chunk-delta fragments (concatenate). Per-chunk `toolcall_delta` emission for the object branch is suppressed (the previous code emitted `JSON.stringify(rawArgs)` per chunk, which fed downstream concat consumers — `packages/agent/src/proxy.ts`, `openai-chat-server`, `openai-responses-server`, `anthropic-messages-server` — an invalid sequence like `{"input":"a"}{"input":"b"}`); the merged object is flushed instead as a single concat-safe delta in `finishToolCallBlock` before `toolcall_end`, so accumulators reconstruct the args correctly. The single-chunk shape covered by the existing #1776 regression test stays correct end-to-end. ([#2080](https://github.com/can1357/oh-my-pi/issues/2080))
|
||||
|
||||
## [15.10.1] - 2026-06-07
|
||||
|
||||
|
||||
@@ -560,6 +560,20 @@ export const streamOpenAICompletions: StreamFunction<"openai-completions"> = (
|
||||
if (block.partialArgs === undefined) return;
|
||||
const contentIndex = blockIndex(block);
|
||||
if (contentIndex < 0) return;
|
||||
// Object-shaped `partialArgs` came from MiniMax-compatible hosts that stream
|
||||
// `function.arguments` as an object. The per-chunk handler holds them with an
|
||||
// empty wire delta (see the object branch below) because emitting each chunk's
|
||||
// `JSON.stringify(rawArgs)` would feed concat-based downstream consumers
|
||||
// (proxy.ts, openai-chat-server, openai-responses-server, anthropic-messages-server)
|
||||
// an invalid concatenation like `{"input":"a"}{"input":"b"}`. Flush the final
|
||||
// merged object as one concat-safe delta now so those consumers reconstruct the
|
||||
// args correctly before observing `toolcall_end`.
|
||||
if (typeof block.partialArgs === "object" && !Array.isArray(block.partialArgs)) {
|
||||
const fullJson = JSON.stringify(block.partialArgs);
|
||||
if (fullJson.length > 0 && fullJson !== "{}") {
|
||||
stream.push({ type: "toolcall_delta", contentIndex, delta: fullJson, partial: output });
|
||||
}
|
||||
}
|
||||
block.arguments =
|
||||
typeof block.partialArgs === "string" ? parseStreamingJson(block.partialArgs) : block.partialArgs;
|
||||
delete block.partialArgs;
|
||||
@@ -877,6 +891,12 @@ export const streamOpenAICompletions: StreamFunction<"openai-completions"> = (
|
||||
// cumulative-vs-delta semantics with `startsWith` so we neither duplicate cumulative
|
||||
// payloads nor lose delta fragments. Degenerates to the previous "last wins"
|
||||
// behaviour for the common single-chunk shape (no prior value to merge with).
|
||||
//
|
||||
// `delta` stays empty here: emitting `JSON.stringify(rawArgs)` per chunk feeds
|
||||
// downstream concat-based accumulators (proxy.ts, openai-chat-server,
|
||||
// openai-responses-server, anthropic-messages-server) an invalid sequence like
|
||||
// `{"input":"a"}{"input":"b"}`. The merged object is flushed as a single
|
||||
// concat-safe delta in `finishToolCallBlock` before `toolcall_end` instead.
|
||||
const prev =
|
||||
block.partialArgs &&
|
||||
typeof block.partialArgs === "object" &&
|
||||
@@ -894,7 +914,6 @@ export const streamOpenAICompletions: StreamFunction<"openai-completions"> = (
|
||||
}
|
||||
block.partialArgs = merged;
|
||||
block.arguments = merged;
|
||||
delta = JSON.stringify(rawArgs);
|
||||
}
|
||||
stream.push({
|
||||
type: "toolcall_delta",
|
||||
|
||||
@@ -159,4 +159,62 @@ describe("issue #2080 - MiniMax multi-chunk object tool arguments", () => {
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("emits a concat-safe `toolcall_delta` sequence — accumulated deltas parse to the merged args", async () => {
|
||||
// Codex review on PR #2082 caught that emitting `JSON.stringify(rawArgs)` per chunk
|
||||
// feeds downstream concat consumers (proxy.ts, openai-chat-server, etc.) an invalid
|
||||
// sequence like `{"input":"a"}{"input":"b"}` even when the merged source-side args
|
||||
// are correct. The fix defers object-chunk emission to `finishToolCallBlock`, which
|
||||
// flushes one delta carrying the full merged JSON. Verify that contract by
|
||||
// reconstructing the args the way the proxy does (concat + parse) and comparing
|
||||
// against the source-side merged result.
|
||||
const model = getBundledModel<"openai-completions">("minimax-code-cn", "MiniMax-M3");
|
||||
global.fetch = createMockFetch([
|
||||
toolCallChunk(model, {
|
||||
name: "edit",
|
||||
arguments: { input: "[foo.ts#A1B2]\nreplace 91..91:\n+ " },
|
||||
}),
|
||||
toolCallChunk(model, {
|
||||
arguments: { input: 'const out = await executeTool("nuke", { path: "x" }, ctx);' },
|
||||
}),
|
||||
stopChunk(model),
|
||||
"[DONE]",
|
||||
]);
|
||||
|
||||
const s = streamOpenAICompletions(model, baseContext(), { apiKey: "test-key" });
|
||||
let accumulated = "";
|
||||
let toolCallEndArgs: unknown;
|
||||
for await (const event of s) {
|
||||
if (event.type === "toolcall_delta") accumulated += event.delta;
|
||||
else if (event.type === "toolcall_end") toolCallEndArgs = event.toolCall.arguments;
|
||||
}
|
||||
|
||||
const expected = {
|
||||
input: '[foo.ts#A1B2]\nreplace 91..91:\n+ const out = await executeTool("nuke", { path: "x" }, ctx);',
|
||||
};
|
||||
// Source-side merged result (what `block.arguments` is set to in `finishToolCallBlock`).
|
||||
expect(toolCallEndArgs).toEqual(expected);
|
||||
// Concat consumers must observe the same args by parsing the accumulated delta string —
|
||||
// this is the contract proxy.ts:286-290 reconstructs against.
|
||||
expect(JSON.parse(accumulated)).toEqual(expected);
|
||||
});
|
||||
|
||||
it("keeps the single-chunk object case concat-safe (no #1776 regression)", async () => {
|
||||
// The #1776 fix sent the full JSON as one delta during streaming. The PR #2082 follow-up
|
||||
// moves emission to `finishToolCallBlock`. The single-chunk path stays correct end-to-end:
|
||||
// the proxy still concatenates ("" then the final delta) and parses to the same args.
|
||||
const model = getBundledModel<"openai-completions">("minimax-code-cn", "MiniMax-M3");
|
||||
global.fetch = createMockFetch([
|
||||
toolCallChunk(model, { name: "edit", arguments: { input: "[foo.ts#A1B2]\ndelete 5" } }),
|
||||
stopChunk(model),
|
||||
"[DONE]",
|
||||
]);
|
||||
|
||||
const s = streamOpenAICompletions(model, baseContext(), { apiKey: "test-key" });
|
||||
let accumulated = "";
|
||||
for await (const event of s) {
|
||||
if (event.type === "toolcall_delta") accumulated += event.delta;
|
||||
}
|
||||
expect(JSON.parse(accumulated)).toEqual({ input: "[foo.ts#A1B2]\ndelete 5" });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user