Skip to content

Commit 78d00b6

Browse files
committed
fix: harden Interactions collapser tool-call assembly from PR review
Address review findings on the SDK 2.x migration: - Flag assembled arguments_delta fragments that don't concatenate into valid JSON by step.stop via droppedChunks/firstDroppedSample, instead of silently writing a corrupt tool call into a recorded fixture. - Preserve the identity of a function_call step.start that arrives without an index by minting a synthetic key (matches the sibling collapsers) rather than dropping it silently. - Reword the emitter's arguments_delta comment: the mock emits one whole fragment (a valid degenerate case), it does not itself concatenate. - Add tests: multiple/interleaved tool calls collapse in step-index order, emitter assigns sequential step indices, invalid-JSON assembly is flagged, and an index-less function_call step.start keeps its identity.
1 parent 09c101d commit 78d00b6

3 files changed

Lines changed: 109 additions & 10 deletions

File tree

src/__tests__/gemini-interactions.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,28 @@ describe("SSE event builders", () => {
870870
expect(JSON.parse(delta.arguments as string)).toEqual({ city: "NYC" });
871871
});
872872

873+
it("assigns sequential step indices for multiple tool calls (SDK 2.x)", () => {
874+
const events = buildInteractionsToolCallSSEEvents(
875+
[
876+
{ name: "get_weather", arguments: '{"city":"NYC"}', id: "call_1" },
877+
{ name: "get_time", arguments: '{"tz":"UTC"}', id: "call_2" },
878+
],
879+
"aimock-int-0",
880+
logger,
881+
);
882+
const stepStarts = events.filter((e) => e.event_type === "step.start");
883+
expect(stepStarts.map((e) => e.index)).toEqual([0, 1]);
884+
expect((stepStarts[0].step as Record<string, unknown>).name).toBe("get_weather");
885+
expect((stepStarts[1].step as Record<string, unknown>).name).toBe("get_time");
886+
// Each call's arguments_delta is keyed to its own index.
887+
const argDeltas = events.filter(
888+
(e) =>
889+
e.event_type === "step.delta" &&
890+
(e.delta as Record<string, unknown>).type === "arguments_delta",
891+
);
892+
expect(argDeltas.map((e) => e.index)).toEqual([0, 1]);
893+
});
894+
873895
it("builds content+tools SSE with correct indices (SDK 2.x)", () => {
874896
const events = buildInteractionsContentWithToolCallsSSEEvents(
875897
"Text",
@@ -1349,6 +1371,58 @@ describe("collapseGeminiInteractionsSSE", () => {
13491371
expect(result.toolCalls![0].id).toBe("call_1");
13501372
});
13511373

1374+
it("collapses multiple interleaved tool calls in step-index order (SDK 2.x)", () => {
1375+
const sse = [
1376+
'data: {"event_type":"step.start","index":1,"step":{"type":"function_call","id":"call_a","name":"get_weather","arguments":{}},"event_id":"evt_1"}',
1377+
'data: {"event_type":"step.start","index":2,"step":{"type":"function_call","id":"call_b","name":"get_time","arguments":{}},"event_id":"evt_2"}',
1378+
// Interleaved argument fragments for both calls.
1379+
'data: {"event_type":"step.delta","index":2,"delta":{"type":"arguments_delta","arguments":"{\\"tz\\":"},"event_id":"evt_3"}',
1380+
'data: {"event_type":"step.delta","index":1,"delta":{"type":"arguments_delta","arguments":"{\\"city\\":\\"NYC\\"}"},"event_id":"evt_4"}',
1381+
'data: {"event_type":"step.delta","index":2,"delta":{"type":"arguments_delta","arguments":"\\"UTC\\"}"},"event_id":"evt_5"}',
1382+
'data: {"event_type":"step.stop","index":1,"event_id":"evt_6"}',
1383+
'data: {"event_type":"step.stop","index":2,"event_id":"evt_7"}',
1384+
].join("\n\n");
1385+
const result = collapseGeminiInteractionsSSE(sse);
1386+
expect(result.toolCalls).toHaveLength(2);
1387+
// Sorted by step index regardless of step.stop / delta arrival order.
1388+
expect(result.toolCalls![0]).toEqual({
1389+
name: "get_weather",
1390+
arguments: '{"city":"NYC"}',
1391+
id: "call_a",
1392+
});
1393+
expect(result.toolCalls![1]).toEqual({
1394+
name: "get_time",
1395+
arguments: '{"tz":"UTC"}',
1396+
id: "call_b",
1397+
});
1398+
});
1399+
1400+
it("flags assembled arguments_delta that is not valid JSON by step.stop", () => {
1401+
const sse = [
1402+
'data: {"event_type":"step.start","index":0,"step":{"type":"function_call","id":"call_1","name":"fn","arguments":{}},"event_id":"evt_1"}',
1403+
// Truncated/interrupted stream — fragment never closes into valid JSON.
1404+
'data: {"event_type":"step.delta","index":0,"delta":{"type":"arguments_delta","arguments":"{\\"city\\":\\"NY"},"event_id":"evt_2"}',
1405+
'data: {"event_type":"step.stop","index":0,"event_id":"evt_3"}',
1406+
].join("\n\n");
1407+
const result = collapseGeminiInteractionsSSE(sse);
1408+
// The (corrupt) call is still surfaced, but flagged so the recorder warns.
1409+
expect(result.toolCalls).toHaveLength(1);
1410+
expect(result.toolCalls![0].arguments).toBe('{"city":"NY');
1411+
expect(result.droppedChunks).toBe(1);
1412+
expect(result.firstDroppedSample).toMatch(/not valid JSON/);
1413+
});
1414+
1415+
it("preserves identity of a function_call step.start that arrives without an index", () => {
1416+
const sse = [
1417+
'data: {"event_type":"step.start","step":{"type":"function_call","id":"call_1","name":"fn","arguments":{"x":1}},"event_id":"evt_1"}',
1418+
'data: {"event_type":"step.stop","event_id":"evt_2"}',
1419+
].join("\n\n");
1420+
const result = collapseGeminiInteractionsSSE(sse);
1421+
expect(result.toolCalls).toHaveLength(1);
1422+
expect(result.toolCalls![0].name).toBe("fn");
1423+
expect(result.toolCalls![0].arguments).toBe('{"x":1}');
1424+
});
1425+
13521426
it("uses step.start arguments object when no arguments_delta streams (SDK 2.x)", () => {
13531427
const sse = [
13541428
'data: {"event_type":"step.start","index":0,"step":{"type":"function_call","id":"call_1","name":"fn","arguments":{"x":1}},"event_id":"evt_1"}',

src/gemini-interactions.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,8 +545,10 @@ export function buildInteractionsToolCallSSEEvents(
545545
event_id: nextEventId(),
546546
});
547547

548-
// arguments_delta.arguments is a string fragment; emitting the full
549-
// serialized object as one fragment keeps the concatenation valid JSON.
548+
// arguments_delta.arguments is a string fragment. The real SDK may split
549+
// the args across several fragments that concatenate into valid JSON; the
550+
// mock emits the whole serialized object as one fragment (a valid
551+
// degenerate case the collapser handles identically).
550552
events.push({
551553
event_type: "step.delta",
552554
index: idx,

src/stream-collapse.ts

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,6 +1215,10 @@ export function collapseGeminiInteractionsSSE(body: string): CollapseResult {
12151215
number,
12161216
{ id?: string; name: string; argsObj?: unknown; argsStr: string }
12171217
>();
1218+
// Synthetic keys for function_call step.start events that arrive without an
1219+
// index (matches the sibling collapsers); seeded high to avoid colliding with
1220+
// real step indices.
1221+
let nextSyntheticStepIndex = 1_000_000;
12181222

12191223
for (const line of lines) {
12201224
const data = extractSSEData(splitSSELines(line));
@@ -1242,8 +1246,12 @@ export function collapseGeminiInteractionsSSE(body: string): CollapseResult {
12421246
if (eventType === "step.start") {
12431247
// 2.x — tool-call identity lives on step.start, not in a delta.
12441248
const step = parsed.step as Record<string, unknown> | undefined;
1245-
if (step && step.type === "function_call" && index !== undefined) {
1246-
stepToolCalls.set(index, {
1249+
if (step && step.type === "function_call") {
1250+
// An index-less start can't correlate later arguments_delta fragments,
1251+
// but minting a synthetic key preserves the call's identity instead of
1252+
// dropping it silently.
1253+
const key = index ?? nextSyntheticStepIndex++;
1254+
stepToolCalls.set(key, {
12471255
id: step.id ? String(step.id) : undefined,
12481256
name: String(step.name ?? ""),
12491257
// step.start may carry a fully-populated `arguments` object (non-
@@ -1298,12 +1306,27 @@ export function collapseGeminiInteractionsSSE(body: string): CollapseResult {
12981306

12991307
// Finalize 2.x tool calls in step-index order.
13001308
for (const [, tc] of Array.from(stepToolCalls.entries()).sort(([a], [b]) => a - b)) {
1301-
const args =
1302-
tc.argsStr !== ""
1303-
? tc.argsStr
1304-
: typeof tc.argsObj === "string"
1305-
? tc.argsObj
1306-
: JSON.stringify(tc.argsObj ?? {});
1309+
let args: string;
1310+
if (tc.argsStr !== "") {
1311+
args = tc.argsStr;
1312+
// The arguments_delta fragments must concatenate into valid JSON by
1313+
// step.stop. A truncated/interrupted stream can leave them malformed;
1314+
// surface that via droppedChunks rather than writing a corrupt fixture
1315+
// silently (mirrors the per-chunk parse guard above).
1316+
try {
1317+
JSON.parse(args);
1318+
} catch {
1319+
droppedChunks++;
1320+
if (droppedChunks === 1) {
1321+
firstDroppedSample = `assembled arguments_delta not valid JSON for "${tc.name}": ${surrogateSafeSlice(
1322+
args,
1323+
200,
1324+
)}`;
1325+
}
1326+
}
1327+
} else {
1328+
args = typeof tc.argsObj === "string" ? tc.argsObj : JSON.stringify(tc.argsObj ?? {});
1329+
}
13071330
toolCalls.push({ name: tc.name, arguments: args, ...(tc.id ? { id: tc.id } : {}) });
13081331
}
13091332

0 commit comments

Comments
 (0)