fix(translator): coerce submit_pr_review functionalChanges/findings to arrays (#2242)

Integrated into release/v3.8.0 — surgical streaming translator shim for submit_pr_review functionalChanges/findings array fields.
This commit is contained in:
Anton
2026-05-14 13:08:02 +02:00
committed by GitHub
parent 1b14b5b012
commit 52285d8a7a
3 changed files with 361 additions and 5 deletions

View File

@@ -0,0 +1,67 @@
// Defensive shims for tool calls whose strict-schema fields can be malformed
// by upstream models (e.g. MiMo emitting empty objects/strings instead of
// arrays for Capy's submit_pr_review).
//
// Applied on the assembled OpenAI tool-call arguments after streaming, just
// before they are re-emitted as a single Claude input_json_delta.
//
// To add a new shim: register a (input) => input transformer in TOOL_SHIMS
// keyed by the tool name. The transformer must accept arbitrary input and
// return a JSON-safe value.
type ShimFn = (input: unknown) => unknown;
function coerceToArray(v: unknown): unknown[] {
if (Array.isArray(v)) return v;
if (v == null) return [];
if (typeof v === "string") {
if (v === "") return [];
try {
const parsed = JSON.parse(v);
return Array.isArray(parsed) ? parsed : [];
} catch {
return [];
}
}
// Plain object or other non-array → empty
return [];
}
const TOOL_SHIMS: Record<string, ShimFn> = {
submit_pr_review: (input) => {
if (typeof input !== "object" || input === null || Array.isArray(input)) return input;
const patched = { ...(input as Record<string, unknown>) };
for (const key of ["functionalChanges", "findings"]) {
patched[key] = coerceToArray(patched[key]);
}
return patched;
},
};
export function hasToolCallShim(name: string | undefined | null): boolean {
return typeof name === "string" && Object.prototype.hasOwnProperty.call(TOOL_SHIMS, name);
}
/**
* Apply the registered shim for a tool call's raw assembled arguments string.
* Returns a stringified JSON value safe to emit as input_json_delta.partial_json.
* If the buffer is unparseable, returns the empty-object JSON `{}` after applying
* the shim with `{}` as input (so required arrays still get injected).
*/
export function applyToolCallShimToBuffer(name: string, raw: string): string {
const shim = TOOL_SHIMS[name];
if (!shim) return raw;
let parsed: unknown;
try {
parsed = raw && raw.length > 0 ? JSON.parse(raw) : {};
} catch {
parsed = {};
}
const patched = shim(parsed);
return JSON.stringify(patched);
}
// Exposed for unit tests only.
export const __test = { coerceToArray, TOOL_SHIMS };

View File

@@ -1,6 +1,7 @@
import { register } from "../registry.ts";
import { FORMATS } from "../formats.ts";
import { CLAUDE_OAUTH_TOOL_PREFIX } from "../request/openai-to-claude.ts";
import { hasToolCallShim, applyToolCallShimToBuffer } from "../helpers/toolCallShim.ts";
// Helper: stop thinking block if started
function stopThinkingBlock(state, results) {
@@ -157,11 +158,6 @@ export function openaiToClaudeResponse(chunk, state) {
stopTextBlock(state, results);
const toolBlockIndex = state.nextBlockIndex++;
state.toolCalls.set(idx, {
id: tc.id,
name: tc.function?.name || "",
blockIndex: toolBlockIndex,
});
// Strip prefix from tool name for response
let toolName = tc.function?.name || "";
@@ -169,6 +165,16 @@ export function openaiToClaudeResponse(chunk, state) {
toolName = toolName.slice(CLAUDE_OAUTH_TOOL_PREFIX.length);
}
state.toolCalls.set(idx, {
id: tc.id,
name: toolName,
blockIndex: toolBlockIndex,
// Shimmed tools buffer their raw args and emit a single corrected
// input_json_delta at content_block_stop time (see finish handler).
shimmed: hasToolCallShim(toolName),
argBuffer: "",
});
results.push({
type: "content_block_start",
index: toolBlockIndex,
@@ -184,6 +190,15 @@ export function openaiToClaudeResponse(chunk, state) {
if (tc.function?.arguments) {
const toolInfo = state.toolCalls.get(idx);
if (toolInfo) {
// Always buffer the raw stream so shimmed tools can re-emit a
// corrected JSON at stop time.
toolInfo.argBuffer = (toolInfo.argBuffer || "") + tc.function.arguments;
if (toolInfo.shimmed) {
// Suppress passthrough; we emit one corrective delta at finish.
continue;
}
let deltaStr = tc.function.arguments;
// Fix #1852: Strip empty string and array placeholders from streaming tool arguments
@@ -211,6 +226,17 @@ export function openaiToClaudeResponse(chunk, state) {
stopTextBlock(state, results);
for (const [, toolInfo] of state.toolCalls) {
// For shimmed tools, emit one corrective input_json_delta with the
// fully patched JSON before closing the block.
if (toolInfo.shimmed) {
const patched = applyToolCallShimToBuffer(toolInfo.name, toolInfo.argBuffer || "");
results.push({
type: "content_block_delta",
index: toolInfo.blockIndex,
delta: { type: "input_json_delta", partial_json: patched },
});
}
results.push({
type: "content_block_stop",
index: toolInfo.blockIndex,

View File

@@ -0,0 +1,263 @@
import test from "node:test";
import assert from "node:assert/strict";
const { applyToolCallShimToBuffer, hasToolCallShim, __test } = await import(
"../../open-sse/translator/helpers/toolCallShim.ts"
);
const { openaiToClaudeResponse } = await import(
"../../open-sse/translator/response/openai-to-claude.ts"
);
const { coerceToArray } = __test as { coerceToArray: (v: unknown) => unknown[] };
// -------- Helper-level tests --------
test("hasToolCallShim: returns true for submit_pr_review only", () => {
assert.equal(hasToolCallShim("submit_pr_review"), true);
assert.equal(hasToolCallShim("some_other_tool"), false);
assert.equal(hasToolCallShim(""), false);
assert.equal(hasToolCallShim(undefined), false);
assert.equal(hasToolCallShim(null), false);
});
test("coerceToArray: passes arrays through unchanged", () => {
assert.deepEqual(coerceToArray([]), []);
assert.deepEqual(coerceToArray([{ a: 1 }]), [{ a: 1 }]);
});
test("coerceToArray: null/undefined -> []", () => {
assert.deepEqual(coerceToArray(null), []);
assert.deepEqual(coerceToArray(undefined), []);
});
test("coerceToArray: plain object -> []", () => {
assert.deepEqual(coerceToArray({}), []);
assert.deepEqual(coerceToArray({ a: 1 }), []);
});
test("coerceToArray: empty string -> []", () => {
assert.deepEqual(coerceToArray(""), []);
});
test("coerceToArray: stringified array parsed", () => {
assert.deepEqual(coerceToArray("[]"), []);
assert.deepEqual(coerceToArray('[{"title":"x"}]'), [{ title: "x" }]);
});
test("coerceToArray: unparseable string -> []", () => {
assert.deepEqual(coerceToArray("not json"), []);
assert.deepEqual(coerceToArray("{"), []);
});
test("coerceToArray: stringified non-array -> []", () => {
assert.deepEqual(coerceToArray('{"a":1}'), []);
assert.deepEqual(coerceToArray('"a string"'), []);
});
test("applyToolCallShimToBuffer: submit_pr_review with valid arrays preserved", () => {
const raw = JSON.stringify({
summary: "ok",
functionalChanges: [{ description: "x" }],
findings: [{ title: "y" }],
});
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", raw));
assert.equal(out.summary, "ok");
assert.deepEqual(out.functionalChanges, [{ description: "x" }]);
assert.deepEqual(out.findings, [{ title: "y" }]);
});
test("applyToolCallShimToBuffer: submit_pr_review missing both keys -> arrays injected", () => {
const raw = JSON.stringify({ summary: "no findings" });
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", raw));
assert.equal(out.summary, "no findings");
assert.deepEqual(out.functionalChanges, []);
assert.deepEqual(out.findings, []);
});
test("applyToolCallShimToBuffer: submit_pr_review with functionalChanges=null replaced", () => {
const raw = JSON.stringify({ functionalChanges: null, findings: [] });
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", raw));
assert.deepEqual(out.functionalChanges, []);
assert.deepEqual(out.findings, []);
});
test("applyToolCallShimToBuffer: submit_pr_review with findings={} replaced", () => {
const raw = JSON.stringify({ functionalChanges: [], findings: {} });
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", raw));
assert.deepEqual(out.findings, []);
});
test("applyToolCallShimToBuffer: submit_pr_review with findings='' replaced", () => {
const raw = JSON.stringify({ functionalChanges: [], findings: "" });
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", raw));
assert.deepEqual(out.findings, []);
});
test("applyToolCallShimToBuffer: submit_pr_review with findings='[]' parsed", () => {
const raw = JSON.stringify({ functionalChanges: [], findings: "[]" });
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", raw));
assert.deepEqual(out.findings, []);
});
test("applyToolCallShimToBuffer: submit_pr_review with stringified array of objects parsed", () => {
const raw = JSON.stringify({
functionalChanges: [],
findings: '[{"title":"x"}]',
});
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", raw));
assert.deepEqual(out.findings, [{ title: "x" }]);
});
test("applyToolCallShimToBuffer: submit_pr_review with empty buffer -> empty arrays injected", () => {
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", ""));
assert.deepEqual(out.functionalChanges, []);
assert.deepEqual(out.findings, []);
});
test("applyToolCallShimToBuffer: submit_pr_review with unparseable buffer -> empty arrays", () => {
const out = JSON.parse(applyToolCallShimToBuffer("submit_pr_review", "{broken"));
assert.deepEqual(out.functionalChanges, []);
assert.deepEqual(out.findings, []);
});
test("applyToolCallShimToBuffer: non-shimmed tool passes raw through", () => {
const raw = '{"x":1}';
assert.equal(applyToolCallShimToBuffer("some_other_tool", raw), raw);
});
// -------- Streaming integration tests --------
function freshState() {
return {
messageStartSent: false,
nextBlockIndex: 0,
toolCalls: new Map(),
thinkingBlockStarted: false,
textBlockStarted: false,
textBlockClosed: false,
};
}
function streamChunks(chunks: any[], state: any): any[] {
const all: any[] = [];
for (const c of chunks) {
const out = openaiToClaudeResponse(c, state);
if (out) all.push(...out);
}
return all;
}
test("streaming: submit_pr_review with missing arrays gets corrective delta at finish", () => {
const state = freshState();
const chunks = [
// chunk 1: message start + tool call start with name
{
id: "chatcmpl-1",
model: "xiaomi-mimo/mimo-v2.5-pro",
choices: [
{
delta: {
tool_calls: [
{
index: 0,
id: "call_1",
function: { name: "submit_pr_review", arguments: "" },
},
],
},
},
],
},
// chunk 2: argument fragment (summary only — no findings/functionalChanges)
{
choices: [
{
delta: {
tool_calls: [
{
index: 0,
function: { arguments: '{"summary":"no findings"}' },
},
],
},
},
],
},
// chunk 3: finish
{
choices: [{ delta: {}, finish_reason: "tool_calls" }],
},
];
const events = streamChunks(chunks, state);
// No passthrough input_json_delta for shimmed tool
const passthroughDeltas = events.filter(
(e) =>
e.type === "content_block_delta" &&
e.delta?.type === "input_json_delta" &&
e.delta?.partial_json === '{"summary":"no findings"}'
);
assert.equal(passthroughDeltas.length, 0, "raw passthrough delta should be suppressed");
// Exactly one corrective input_json_delta on the tool block
const correctiveDeltas = events.filter(
(e) => e.type === "content_block_delta" && e.delta?.type === "input_json_delta"
);
assert.equal(correctiveDeltas.length, 1, "expected exactly one corrective delta");
const finalInput = JSON.parse(correctiveDeltas[0].delta.partial_json);
assert.equal(finalInput.summary, "no findings");
assert.deepEqual(finalInput.functionalChanges, []);
assert.deepEqual(finalInput.findings, []);
// Corrective delta MUST come before the content_block_stop for that tool block
const correctiveIdx = events.findIndex(
(e) => e.type === "content_block_delta" && e.delta?.type === "input_json_delta"
);
const stopIdx = events.findIndex(
(e) => e.type === "content_block_stop" && e.index === correctiveDeltas[0].index
);
assert.ok(correctiveIdx < stopIdx, "corrective delta must precede content_block_stop");
});
test("streaming: non-shimmed tool still streams partials through", () => {
const state = freshState();
const chunks = [
{
id: "chatcmpl-1",
model: "x",
choices: [
{
delta: {
tool_calls: [
{
index: 0,
id: "call_1",
function: { name: "some_other_tool", arguments: "" },
},
],
},
},
],
},
{
choices: [
{
delta: {
tool_calls: [{ index: 0, function: { arguments: '{"x":1}' } }],
},
},
],
},
{ choices: [{ delta: {}, finish_reason: "tool_calls" }] },
];
const events = streamChunks(chunks, state);
const deltas = events.filter(
(e) => e.type === "content_block_delta" && e.delta?.type === "input_json_delta"
);
// For non-shimmed tools, the original passthrough delta survives (and no extra corrective delta).
assert.equal(deltas.length, 1);
assert.equal(deltas[0].delta.partial_json, '{"x":1}');
});