Change the type of ToolResult.responseParts (#6875)

This commit is contained in:
Tommaso Sciortino
2025-08-22 14:12:05 -07:00
committed by GitHub
parent 9a0722625b
commit 75822d3506
13 changed files with 205 additions and 324 deletions

View File

@@ -111,16 +111,7 @@ export async function runNonInteractive(
}
if (toolResponse.responseParts) {
const parts = Array.isArray(toolResponse.responseParts)
? toolResponse.responseParts
: [toolResponse.responseParts];
for (const part of parts) {
if (typeof part === 'string') {
toolResponseParts.push({ text: part });
} else if (part) {
toolResponseParts.push(part);
}
}
toolResponseParts.push(...toolResponse.responseParts);
}
}
currentMessages = [{ role: 'user', parts: toolResponseParts }];

View File

@@ -15,7 +15,7 @@ import {
MockInstance,
} from 'vitest';
import { renderHook, act, waitFor } from '@testing-library/react';
import { useGeminiStream, mergePartListUnions } from './useGeminiStream.js';
import { useGeminiStream } from './useGeminiStream.js';
import { useKeypress } from './useKeypress.js';
import * as atCommandProcessor from './atCommandProcessor.js';
import {
@@ -138,125 +138,6 @@ vi.mock('./slashCommandProcessor.js', () => ({
// --- END MOCKS ---
describe('mergePartListUnions', () => {
it('should merge multiple PartListUnion arrays', () => {
const list1: PartListUnion = [{ text: 'Hello' }];
const list2: PartListUnion = [
{ inlineData: { mimeType: 'image/png', data: 'abc' } },
];
const list3: PartListUnion = [{ text: 'World' }, { text: '!' }];
const result = mergePartListUnions([list1, list2, list3]);
expect(result).toEqual([
{ text: 'Hello' },
{ inlineData: { mimeType: 'image/png', data: 'abc' } },
{ text: 'World' },
{ text: '!' },
]);
});
it('should handle empty arrays in the input list', () => {
const list1: PartListUnion = [{ text: 'First' }];
const list2: PartListUnion = [];
const list3: PartListUnion = [{ text: 'Last' }];
const result = mergePartListUnions([list1, list2, list3]);
expect(result).toEqual([{ text: 'First' }, { text: 'Last' }]);
});
it('should handle a single PartListUnion array', () => {
const list1: PartListUnion = [
{ text: 'One' },
{ inlineData: { mimeType: 'image/jpeg', data: 'xyz' } },
];
const result = mergePartListUnions([list1]);
expect(result).toEqual(list1);
});
it('should return an empty array if all input arrays are empty', () => {
const list1: PartListUnion = [];
const list2: PartListUnion = [];
const result = mergePartListUnions([list1, list2]);
expect(result).toEqual([]);
});
it('should handle input list being empty', () => {
const result = mergePartListUnions([]);
expect(result).toEqual([]);
});
it('should correctly merge when PartListUnion items are single Parts not in arrays', () => {
const part1: Part = { text: 'Single part 1' };
const part2: Part = { inlineData: { mimeType: 'image/gif', data: 'gif' } };
const listContainingSingleParts: PartListUnion[] = [
part1,
[part2],
{ text: 'Another single part' },
];
const result = mergePartListUnions(listContainingSingleParts);
expect(result).toEqual([
{ text: 'Single part 1' },
{ inlineData: { mimeType: 'image/gif', data: 'gif' } },
{ text: 'Another single part' },
]);
});
it('should handle a mix of arrays and single parts, including empty arrays and undefined/null parts if they were possible (though PartListUnion typing restricts this)', () => {
const list1: PartListUnion = [{ text: 'A' }];
const list2: PartListUnion = [];
const part3: Part = { text: 'B' };
const list4: PartListUnion = [
{ text: 'C' },
{ inlineData: { mimeType: 'text/plain', data: 'D' } },
];
const result = mergePartListUnions([list1, list2, part3, list4]);
expect(result).toEqual([
{ text: 'A' },
{ text: 'B' },
{ text: 'C' },
{ inlineData: { mimeType: 'text/plain', data: 'D' } },
]);
});
it('should preserve the order of parts from the input arrays', () => {
const listA: PartListUnion = [{ text: '1' }, { text: '2' }];
const listB: PartListUnion = [{ text: '3' }];
const listC: PartListUnion = [{ text: '4' }, { text: '5' }];
const result = mergePartListUnions([listA, listB, listC]);
expect(result).toEqual([
{ text: '1' },
{ text: '2' },
{ text: '3' },
{ text: '4' },
{ text: '5' },
]);
});
it('should handle cases where some PartListUnion items are single Parts and others are arrays of Parts', () => {
const singlePart1: Part = { text: 'First single' };
const arrayPart1: Part[] = [
{ text: 'Array item 1' },
{ text: 'Array item 2' },
];
const singlePart2: Part = {
inlineData: { mimeType: 'application/json', data: 'e30=' },
}; // {}
const arrayPart2: Part[] = [{ text: 'Last array item' }];
const result = mergePartListUnions([
singlePart1,
arrayPart1,
singlePart2,
arrayPart2,
]);
expect(result).toEqual([
{ text: 'First single' },
{ text: 'Array item 1' },
{ text: 'Array item 2' },
{ inlineData: { mimeType: 'application/json', data: 'e30=' } },
{ text: 'Last array item' },
]);
});
});
// --- Tests for useGeminiStream Hook ---
describe('useGeminiStream', () => {
let mockAddItem: Mock;
@@ -505,12 +386,8 @@ describe('useGeminiStream', () => {
});
it('should submit tool responses when all tool calls are completed and ready', async () => {
const toolCall1ResponseParts: PartListUnion = [
{ text: 'tool 1 final response' },
];
const toolCall2ResponseParts: PartListUnion = [
{ text: 'tool 2 final response' },
];
const toolCall1ResponseParts: Part[] = [{ text: 'tool 1 final response' }];
const toolCall2ResponseParts: Part[] = [{ text: 'tool 2 final response' }];
const completedToolCalls: TrackedToolCall[] = [
{
request: {
@@ -593,10 +470,10 @@ describe('useGeminiStream', () => {
expect(mockSendMessageStream).toHaveBeenCalledTimes(1);
});
const expectedMergedResponse = mergePartListUnions([
toolCall1ResponseParts,
toolCall2ResponseParts,
]);
const expectedMergedResponse = [
...toolCall1ResponseParts,
...toolCall2ResponseParts,
];
expect(mockSendMessageStream).toHaveBeenCalledWith(
expectedMergedResponse,
expect.any(AbortSignal),

View File

@@ -56,18 +56,6 @@ import {
import { useSessionStats } from '../contexts/SessionContext.js';
import { useKeypress } from './useKeypress.js';
export function mergePartListUnions(list: PartListUnion[]): PartListUnion {
const resultParts: PartListUnion = [];
for (const item of list) {
if (Array.isArray(item)) {
resultParts.push(...item);
} else {
resultParts.push(item);
}
}
return resultParts;
}
enum StreamProcessingStatus {
Completed,
UserCancelled,
@@ -805,19 +793,9 @@ export const useGeminiStream = (
if (geminiClient) {
// We need to manually add the function responses to the history
// so the model knows the tools were cancelled.
const responsesToAdd = geminiTools.flatMap(
const combinedParts = geminiTools.flatMap(
(toolCall) => toolCall.response.responseParts,
);
const combinedParts: Part[] = [];
for (const response of responsesToAdd) {
if (Array.isArray(response)) {
combinedParts.push(...response);
} else if (typeof response === 'string') {
combinedParts.push({ text: response });
} else {
combinedParts.push(response);
}
}
geminiClient.addHistory({
role: 'user',
parts: combinedParts,
@@ -831,7 +809,7 @@ export const useGeminiStream = (
return;
}
const responsesToSend: PartListUnion[] = geminiTools.map(
const responsesToSend: Part[] = geminiTools.flatMap(
(toolCall) => toolCall.response.responseParts,
);
const callIdsToMarkAsSubmitted = geminiTools.map(
@@ -850,7 +828,7 @@ export const useGeminiStream = (
}
submitQuery(
mergePartListUnions(responsesToSend),
responsesToSend,
{
isContinuation: true,
},

View File

@@ -239,13 +239,15 @@ describe('useReactToolScheduler in YOLO Mode', () => {
request,
response: expect.objectContaining({
resultDisplay: 'YOLO Formatted tool output',
responseParts: {
functionResponse: {
id: 'yoloCall',
name: 'mockToolRequiresConfirmation',
response: { output: expectedOutput },
responseParts: [
{
functionResponse: {
id: 'yoloCall',
name: 'mockToolRequiresConfirmation',
response: { output: expectedOutput },
},
},
},
],
}),
}),
]);
@@ -388,13 +390,15 @@ describe('useReactToolScheduler', () => {
request,
response: expect.objectContaining({
resultDisplay: 'Formatted tool output',
responseParts: {
functionResponse: {
id: 'call1',
name: 'mockTool',
response: { output: 'Tool output' },
responseParts: [
{
functionResponse: {
id: 'call1',
name: 'mockTool',
response: { output: 'Tool output' },
},
},
},
],
}),
}),
]);
@@ -769,13 +773,15 @@ describe('useReactToolScheduler', () => {
request: requests[0],
response: expect.objectContaining({
resultDisplay: 'Display 1',
responseParts: {
functionResponse: {
id: 'multi1',
name: 'tool1',
response: { output: 'Output 1' },
responseParts: [
{
functionResponse: {
id: 'multi1',
name: 'tool1',
response: { output: 'Output 1' },
},
},
},
],
}),
});
expect(call2Result).toMatchObject({
@@ -783,13 +789,15 @@ describe('useReactToolScheduler', () => {
request: requests[1],
response: expect.objectContaining({
resultDisplay: 'Display 2',
responseParts: {
functionResponse: {
id: 'multi2',
name: 'tool2',
response: { output: 'Output 2' },
responseParts: [
{
functionResponse: {
id: 'multi2',
name: 'tool2',
response: { output: 'Output 2' },
},
},
},
],
}),
});
expect(result.current[0]).toEqual([]);

View File

@@ -26,7 +26,7 @@ import {
import * as acp from './acp.js';
import { AcpFileSystemService } from './fileSystemService.js';
import { Readable, Writable } from 'node:stream';
import { Content, Part, FunctionCall, PartListUnion } from '@google/genai';
import { Content, Part, FunctionCall } from '@google/genai';
import { LoadedSettings, SettingScope } from '../config/settings.js';
import * as fs from 'fs/promises';
import * as path from 'path';
@@ -300,16 +300,7 @@ class Session {
for (const fc of functionCalls) {
const response = await this.runTool(pendingSend.signal, promptId, fc);
const parts = Array.isArray(response) ? response : [response];
for (const part of parts) {
if (typeof part === 'string') {
toolResponseParts.push({ text: part });
} else if (part) {
toolResponseParts.push(part);
}
}
toolResponseParts.push(...response);
}
nextMessage = { role: 'user', parts: toolResponseParts };
@@ -332,7 +323,7 @@ class Session {
abortSignal: AbortSignal,
promptId: string,
fc: FunctionCall,
): Promise<PartListUnion> {
): Promise<Part[]> {
const callId = fc.id ?? `${fc.name}-${Date.now()}`;
const args = (fc.args ?? {}) as Record<string, unknown>;

View File

@@ -248,37 +248,43 @@ describe('convertToFunctionResponse', () => {
it('should handle simple string llmContent', () => {
const llmContent = 'Simple text output';
const result = convertToFunctionResponse(toolName, callId, llmContent);
expect(result).toEqual({
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Simple text output' },
expect(result).toEqual([
{
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Simple text output' },
},
},
});
]);
});
it('should handle llmContent as a single Part with text', () => {
const llmContent: Part = { text: 'Text from Part object' };
const result = convertToFunctionResponse(toolName, callId, llmContent);
expect(result).toEqual({
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Text from Part object' },
expect(result).toEqual([
{
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Text from Part object' },
},
},
});
]);
});
it('should handle llmContent as a PartListUnion array with a single text Part', () => {
const llmContent: PartListUnion = [{ text: 'Text from array' }];
const result = convertToFunctionResponse(toolName, callId, llmContent);
expect(result).toEqual({
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Text from array' },
expect(result).toEqual([
{
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Text from array' },
},
},
});
]);
});
it('should handle llmContent with inlineData', () => {
@@ -360,25 +366,29 @@ describe('convertToFunctionResponse', () => {
it('should handle llmContent as a generic Part (not text, inlineData, or fileData)', () => {
const llmContent: Part = { functionCall: { name: 'test', args: {} } };
const result = convertToFunctionResponse(toolName, callId, llmContent);
expect(result).toEqual({
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Tool execution succeeded.' },
expect(result).toEqual([
{
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Tool execution succeeded.' },
},
},
});
]);
});
it('should handle empty string llmContent', () => {
const llmContent = '';
const result = convertToFunctionResponse(toolName, callId, llmContent);
expect(result).toEqual({
functionResponse: {
name: toolName,
id: callId,
response: { output: '' },
expect(result).toEqual([
{
functionResponse: {
name: toolName,
id: callId,
response: { output: '' },
},
},
});
]);
});
it('should handle llmContent as an empty array', () => {
@@ -398,13 +408,15 @@ describe('convertToFunctionResponse', () => {
it('should handle llmContent as a Part with undefined inlineData/fileData/text', () => {
const llmContent: Part = {}; // An empty part object
const result = convertToFunctionResponse(toolName, callId, llmContent);
expect(result).toEqual({
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Tool execution succeeded.' },
expect(result).toEqual([
{
functionResponse: {
name: toolName,
id: callId,
response: { output: 'Tool execution succeeded.' },
},
},
});
]);
});
});

View File

@@ -150,14 +150,14 @@ export function convertToFunctionResponse(
toolName: string,
callId: string,
llmContent: PartListUnion,
): PartListUnion {
): Part[] {
const contentToProcess =
Array.isArray(llmContent) && llmContent.length === 1
? llmContent[0]
: llmContent;
if (typeof contentToProcess === 'string') {
return createFunctionResponsePart(callId, toolName, contentToProcess);
return [createFunctionResponsePart(callId, toolName, contentToProcess)];
}
if (Array.isArray(contentToProcess)) {
@@ -166,7 +166,7 @@ export function convertToFunctionResponse(
toolName,
'Tool execution succeeded.',
);
return [functionResponse, ...contentToProcess];
return [functionResponse, ...toParts(contentToProcess)];
}
// After this point, contentToProcess is a single Part object.
@@ -176,10 +176,10 @@ export function convertToFunctionResponse(
getResponseTextFromParts(
contentToProcess.functionResponse.response['content'] as Part[],
) || '';
return createFunctionResponsePart(callId, toolName, stringifiedOutput);
return [createFunctionResponsePart(callId, toolName, stringifiedOutput)];
}
// It's a functionResponse that we should pass through as is.
return contentToProcess;
return [contentToProcess];
}
if (contentToProcess.inlineData || contentToProcess.fileData) {
@@ -196,15 +196,27 @@ export function convertToFunctionResponse(
}
if (contentToProcess.text !== undefined) {
return createFunctionResponsePart(callId, toolName, contentToProcess.text);
return [
createFunctionResponsePart(callId, toolName, contentToProcess.text),
];
}
// Default case for other kinds of parts.
return createFunctionResponsePart(
callId,
toolName,
'Tool execution succeeded.',
);
return [
createFunctionResponsePart(callId, toolName, 'Tool execution succeeded.'),
];
}
function toParts(input: PartListUnion): Part[] {
const parts: Part[] = [];
for (const part of Array.isArray(input) ? input : [input]) {
if (typeof part === 'string') {
parts.push({ text: part });
} else if (part) {
parts.push(part);
}
}
return parts;
}
const createErrorResponse = (
@@ -214,13 +226,15 @@ const createErrorResponse = (
): ToolCallResponseInfo => ({
callId: request.callId,
error,
responseParts: {
functionResponse: {
id: request.callId,
name: request.name,
response: { error: error.message },
responseParts: [
{
functionResponse: {
id: request.callId,
name: request.name,
response: { error: error.message },
},
},
},
],
resultDisplay: error.message,
errorType,
});
@@ -382,15 +396,17 @@ export class CoreToolScheduler {
status: 'cancelled',
response: {
callId: currentCall.request.callId,
responseParts: {
functionResponse: {
id: currentCall.request.callId,
name: currentCall.request.name,
response: {
error: `[Operation Cancelled] Reason: ${auxiliaryData}`,
responseParts: [
{
functionResponse: {
id: currentCall.request.callId,
name: currentCall.request.name,
response: {
error: `[Operation Cancelled] Reason: ${auxiliaryData}`,
},
},
},
},
],
resultDisplay,
error: undefined,
errorType: undefined,

View File

@@ -73,13 +73,15 @@ describe('executeToolCall', () => {
error: undefined,
errorType: undefined,
resultDisplay: 'Success!',
responseParts: {
functionResponse: {
name: 'testTool',
id: 'call1',
response: { output: 'Tool executed successfully' },
responseParts: [
{
functionResponse: {
name: 'testTool',
id: 'call1',
response: { output: 'Tool executed successfully' },
},
},
},
],
});
});
@@ -104,13 +106,17 @@ describe('executeToolCall', () => {
error: new Error('Tool "nonexistentTool" not found in registry.'),
errorType: ToolErrorType.TOOL_NOT_REGISTERED,
resultDisplay: 'Tool "nonexistentTool" not found in registry.',
responseParts: {
functionResponse: {
name: 'nonexistentTool',
id: 'call2',
response: { error: 'Tool "nonexistentTool" not found in registry.' },
responseParts: [
{
functionResponse: {
name: 'nonexistentTool',
id: 'call2',
response: {
error: 'Tool "nonexistentTool" not found in registry.',
},
},
},
},
],
});
});
@@ -137,15 +143,17 @@ describe('executeToolCall', () => {
callId: 'call3',
error: new Error('Invalid parameters'),
errorType: ToolErrorType.INVALID_TOOL_PARAMS,
responseParts: {
functionResponse: {
id: 'call3',
name: 'testTool',
response: {
error: 'Invalid parameters',
responseParts: [
{
functionResponse: {
id: 'call3',
name: 'testTool',
response: {
error: 'Invalid parameters',
},
},
},
},
],
resultDisplay: 'Invalid parameters',
});
});
@@ -178,15 +186,17 @@ describe('executeToolCall', () => {
callId: 'call4',
error: new Error('Execution failed'),
errorType: ToolErrorType.EXECUTION_FAILED,
responseParts: {
functionResponse: {
id: 'call4',
name: 'testTool',
response: {
error: 'Execution failed',
responseParts: [
{
functionResponse: {
id: 'call4',
name: 'testTool',
response: {
error: 'Execution failed',
},
},
},
},
],
resultDisplay: 'Execution failed',
});
});
@@ -215,13 +225,15 @@ describe('executeToolCall', () => {
error: new Error('Something went very wrong'),
errorType: ToolErrorType.UNHANDLED_EXCEPTION,
resultDisplay: 'Something went very wrong',
responseParts: {
functionResponse: {
name: 'testTool',
id: 'call5',
response: { error: 'Something went very wrong' },
responseParts: [
{
functionResponse: {
name: 'testTool',
id: 'call5',
response: { error: 'Something went very wrong' },
},
},
},
],
});
});

View File

@@ -559,7 +559,7 @@ describe('subagent.ts', () => {
// Mock the tool execution result
vi.mocked(executeToolCall).mockResolvedValue({
callId: 'call_1',
responseParts: 'file1.txt\nfile2.ts',
responseParts: [{ text: 'file1.txt\nfile2.ts' }],
resultDisplay: 'Listed 2 files',
error: undefined,
errorType: undefined, // Or ToolErrorType.NONE if available and appropriate
@@ -614,7 +614,7 @@ describe('subagent.ts', () => {
// Mock the tool execution failure.
vi.mocked(executeToolCall).mockResolvedValue({
callId: 'call_fail',
responseParts: 'ERROR: Tool failed catastrophically', // This should be sent to the model
responseParts: [{ text: 'ERROR: Tool failed catastrophically' }], // This should be sent to the model
resultDisplay: 'Tool failed catastrophically',
error: new Error('Failure'),
errorType: ToolErrorType.INVALID_TOOL_PARAMS,

View File

@@ -502,7 +502,7 @@ export class SubAgentScope {
toolResponse = {
callId,
responseParts: `Emitted variable ${valName} successfully`,
responseParts: [{ text: `Emitted variable ${valName} successfully` }],
resultDisplay: `Emitted variable ${valName} successfully`,
error: undefined,
};
@@ -521,16 +521,7 @@ export class SubAgentScope {
}
if (toolResponse.responseParts) {
const parts = Array.isArray(toolResponse.responseParts)
? toolResponse.responseParts
: [toolResponse.responseParts];
for (const part of parts) {
if (typeof part === 'string') {
toolResponseParts.push({ text: part });
} else if (part) {
toolResponseParts.push(part);
}
}
toolResponseParts.push(...toolResponse.responseParts);
}
}
// If all tool calls failed, inform the model so it can re-evaluate.

View File

@@ -5,6 +5,7 @@
*/
import {
Part,
PartListUnion,
GenerateContentResponse,
FunctionCall,
@@ -74,7 +75,7 @@ export interface ToolCallRequestInfo {
export interface ToolCallResponseInfo {
callId: string;
responseParts: PartListUnion;
responseParts: Part[];
resultDisplay: ToolResultDisplay | undefined;
error: Error | undefined;
errorType: ToolErrorType | undefined;

View File

@@ -495,7 +495,7 @@ describe('loggers', () => {
},
response: {
callId: 'test-call-id',
responseParts: 'test-response',
responseParts: [{ text: 'test-response' }],
resultDisplay: undefined,
error: undefined,
errorType: undefined,
@@ -562,7 +562,7 @@ describe('loggers', () => {
},
response: {
callId: 'test-call-id',
responseParts: 'test-response',
responseParts: [{ text: 'test-response' }],
resultDisplay: undefined,
error: undefined,
errorType: undefined,
@@ -628,7 +628,7 @@ describe('loggers', () => {
},
response: {
callId: 'test-call-id',
responseParts: 'test-response',
responseParts: [{ text: 'test-response' }],
resultDisplay: undefined,
error: undefined,
errorType: undefined,
@@ -696,7 +696,7 @@ describe('loggers', () => {
},
response: {
callId: 'test-call-id',
responseParts: 'test-response',
responseParts: [{ text: 'test-response' }],
resultDisplay: undefined,
error: undefined,
errorType: undefined,
@@ -762,7 +762,7 @@ describe('loggers', () => {
},
response: {
callId: 'test-call-id',
responseParts: 'test-response',
responseParts: [{ text: 'test-response' }],
resultDisplay: undefined,
error: {
name: 'test-error-type',

View File

@@ -46,13 +46,15 @@ const createFakeCompletedToolCall = (
invocation: tool.build({ param: 'test' }),
response: {
callId: request.callId,
responseParts: {
functionResponse: {
id: request.callId,
name,
response: { output: 'Success!' },
responseParts: [
{
functionResponse: {
id: request.callId,
name,
response: { output: 'Success!' },
},
},
},
],
error: undefined,
errorType: undefined,
resultDisplay: 'Success!',
@@ -67,13 +69,15 @@ const createFakeCompletedToolCall = (
tool,
response: {
callId: request.callId,
responseParts: {
functionResponse: {
id: request.callId,
name,
response: { error: 'Tool failed' },
responseParts: [
{
functionResponse: {
id: request.callId,
name,
response: { error: 'Tool failed' },
},
},
},
],
error: error || new Error('Tool failed'),
errorType: ToolErrorType.UNKNOWN,
resultDisplay: 'Failure!',