diff --git a/docs/cli/providers-openai-responses.md b/docs/cli/providers-openai-responses.md index 9a56b33a2..3305acbc7 100644 --- a/docs/cli/providers-openai-responses.md +++ b/docs/cli/providers-openai-responses.md @@ -321,7 +321,7 @@ const response = await provider.generateChatCompletion({ 2. **Tool calls not working** - Ensure tools are properly formatted for Responses API - Check that `tool_choice` is used instead of `function_call` - - Verify tool response format matches expected structure + - Verify tool response format matches expected structure (see [Tool output format](../tool-output-format.md)) 3. **Streaming issues** - Ensure SSE parsing is working correctly diff --git a/docs/cli/providers.md b/docs/cli/providers.md index 0f8067a0f..d79bd3055 100644 --- a/docs/cli/providers.md +++ b/docs/cli/providers.md @@ -92,7 +92,7 @@ Each provider offers different models. You can select a specific model using the ### OpenAI - Supports o3 (including o3-pro which REQUIRES Responses API), o1, GPT-4.1, GPT-4o, and other OpenAI models -- Tool calling support with JSON format +- Tool calling support (tool outputs are sent to the model as plain multi-line text; see [Tool output format](../tool-output-format.md)) - Responses API support for advanced models (o3, o1, gpt-4.1) ### Anthropic diff --git a/docs/tool-output-format.md b/docs/tool-output-format.md new file mode 100644 index 000000000..36a594b33 --- /dev/null +++ b/docs/tool-output-format.md @@ -0,0 +1,29 @@ +# Tool output format + +LLxprt Code sends tool outputs to the model as **plain multi-line text** (not JSON). + +This avoids double/triple JSON-encoding that produces heavily escaped strings, which are harder for the model to read (especially for code). + +## Format + +The tool output text is formatted as: + +```text +status: + + +toolName: + + +error: + + +output: + +``` + +Notes: + +- `output` is intended to be raw multi-line text (for example, code or logs). +- If there is no error, `error` is an empty string. +- When output is missing, it is replaced with `[no tool result]`. diff --git a/packages/core/src/providers/openai/OpenAIProvider.compressToolMessages.test.ts b/packages/core/src/providers/openai/OpenAIProvider.compressToolMessages.test.ts index 8cdb1e481..9a6076852 100644 --- a/packages/core/src/providers/openai/OpenAIProvider.compressToolMessages.test.ts +++ b/packages/core/src/providers/openai/OpenAIProvider.compressToolMessages.test.ts @@ -22,16 +22,14 @@ describe('OpenAIProvider compressToolMessages (Issue #894)', () => { it('should compress tool messages when provider limits require it', () => { const provider = new OpenAIProvider('test-key'); - const originalPayload = { - status: 'success', - toolName: 'read_file', - result: 'a'.repeat(5000), - }; + const originalContent = + 'status:\nsuccess\n\ntoolName:\nread_file\n\nerror:\n\n\noutput:\n' + + 'a'.repeat(5000); const messages: OpenAI.Chat.ChatCompletionMessageParam[] = [ { role: 'tool', - content: JSON.stringify(originalPayload), + content: originalContent, tool_call_id: 'call_abc', } as OpenAI.Chat.ChatCompletionToolMessageParam, ]; @@ -55,14 +53,8 @@ describe('OpenAIProvider compressToolMessages (Issue #894)', () => { const modified = messages[0] as { content?: unknown }; expect(typeof modified.content).toBe('string'); - const parsed = JSON.parse(modified.content as string) as { - result?: string; - truncated?: boolean; - originalLength?: number; - }; + const content = modified.content as string; - expect(parsed.truncated).toBe(true); - expect(typeof parsed.originalLength).toBe('number'); - expect(parsed.result).toContain('[omitted'); + expect(content).toContain('[truncated'); }); }); diff --git a/packages/core/src/providers/openai/OpenAIProvider.ts b/packages/core/src/providers/openai/OpenAIProvider.ts index 098048e2d..4dee32b19 100644 --- a/packages/core/src/providers/openai/OpenAIProvider.ts +++ b/packages/core/src/providers/openai/OpenAIProvider.ts @@ -62,7 +62,10 @@ import { resolveRuntimeAuthToken } from '../utils/authToken.js'; import { filterOpenAIRequestParams } from './openaiRequestParams.js'; import { ensureJsonSafe } from '../../utils/unicodeUtils.js'; import { ToolCallPipeline } from './ToolCallPipeline.js'; -import { buildToolResponsePayload } from '../utils/toolResponsePayload.js'; +import { + buildToolResponsePayload, + formatToolResponseText, +} from '../utils/toolResponsePayload.js'; import { isLocalEndpoint } from '../utils/localEndpoint.js'; import { filterThinkingForContext, @@ -1127,7 +1130,14 @@ export class OpenAIProvider extends BaseProvider implements IProvider { config?: Config, ): string { const payload = buildToolResponsePayload(block, config); - return ensureJsonSafe(JSON.stringify(payload)); + return ensureJsonSafe( + formatToolResponseText({ + status: payload.status, + toolName: payload.toolName ?? block.toolName, + error: payload.error, + output: payload.result, + }), + ); } private shouldCompressToolMessages( diff --git a/packages/core/src/providers/openai/buildResponsesRequest.stripToolCalls.test.ts b/packages/core/src/providers/openai/buildResponsesRequest.stripToolCalls.test.ts index 5e84fd5d0..e4df7eb17 100644 --- a/packages/core/src/providers/openai/buildResponsesRequest.stripToolCalls.test.ts +++ b/packages/core/src/providers/openai/buildResponsesRequest.stripToolCalls.test.ts @@ -68,7 +68,8 @@ describe('buildResponsesRequest - tool_calls stripping', () => { expect(request.input?.[3]).toEqual({ type: 'function_call_output', call_id: 'call_123', - output: 'Sunny, 72°F', + output: + 'status:\nsuccess\n\ntoolName:\nget_weather\n\nerror:\n\n\noutput:\nSunny, 72°F', }); }); diff --git a/packages/core/src/providers/openai/buildResponsesRequest.ts b/packages/core/src/providers/openai/buildResponsesRequest.ts index 5617db913..edfd4dd33 100644 --- a/packages/core/src/providers/openai/buildResponsesRequest.ts +++ b/packages/core/src/providers/openai/buildResponsesRequest.ts @@ -2,6 +2,11 @@ * @plan PLAN-20250120-DEBUGLOGGING.P15 * @requirement REQ-INT-001.1 */ +import { + buildToolResponsePayload, + formatToolResponseText, +} from '../utils/toolResponsePayload.js'; + import { DebugLogger } from '../../debug/index.js'; import { type IContent, @@ -223,7 +228,10 @@ export function buildResponsesRequest( type: 'function_call' as const, call_id: normalizeToOpenAIToolId(toolCallBlock.id), name: toolCallBlock.name, - arguments: JSON.stringify(toolCallBlock.parameters), + arguments: + typeof toolCallBlock.parameters === 'string' + ? toolCallBlock.parameters + : JSON.stringify(toolCallBlock.parameters), }); } }); @@ -234,18 +242,21 @@ export function buildResponsesRequest( msg.blocks.forEach((block) => { if (block.type === 'tool_response') { const toolResponseBlock = block as ToolResponseBlock; - // Sanitize content to ensure it's safe for JSON/API transmission - const resultStr = - typeof toolResponseBlock.result === 'string' - ? toolResponseBlock.result - : JSON.stringify(toolResponseBlock.result); - let sanitizedContent = resultStr; - if (hasUnicodeReplacements(resultStr)) { + + const payload = buildToolResponsePayload(toolResponseBlock); + let sanitizedContent = formatToolResponseText({ + status: payload.status, + toolName: payload.toolName ?? toolResponseBlock.toolName, + error: payload.error, + output: payload.result, + }); + + if (hasUnicodeReplacements(sanitizedContent)) { logger.debug( () => 'Tool output contains Unicode replacement characters (U+FFFD), sanitizing...', ); - sanitizedContent = ensureJsonSafe(resultStr); + sanitizedContent = ensureJsonSafe(sanitizedContent); } // Normalize tool IDs to OpenAI format (call_XXX) - fixes issue #825 diff --git a/packages/core/src/providers/utils/toolResponsePayload.ts b/packages/core/src/providers/utils/toolResponsePayload.ts index f4d26ba94..b75980ce7 100644 --- a/packages/core/src/providers/utils/toolResponsePayload.ts +++ b/packages/core/src/providers/utils/toolResponsePayload.ts @@ -12,6 +12,32 @@ import { hasUnicodeReplacements, } from '../../utils/unicodeUtils.js'; +export function formatToolResponseText(params: { + status: 'success' | 'error'; + toolName?: string; + error?: string; + output?: string; +}): string { + const blocks: string[] = []; + + blocks.push('status:'); + blocks.push(params.status); + + blocks.push(''); + blocks.push('toolName:'); + blocks.push(params.toolName ?? ''); + + blocks.push(''); + blocks.push('error:'); + blocks.push(params.error ?? ''); + + blocks.push(''); + blocks.push('output:'); + blocks.push(params.output ?? ''); + + return blocks.join('\n'); +} + export interface ToolResponsePayload { status: 'success' | 'error'; toolName?: string; @@ -62,13 +88,17 @@ function formatToolResult(result: unknown): { const formatted = formatToolResultValue(result); return { ...formatted, raw: result }; } - try { - const serialized = JSON.stringify(result); - return { value: serialized, raw: serialized }; - } catch { - const coerced = coerceToString(result); - return { value: coerced, raw: coerced }; + + if (typeof result === 'object') { + const output = (result as { output?: unknown }).output; + if (typeof output === 'string') { + const formatted = formatToolResultValue(output); + return { ...formatted, raw: output }; + } } + + const coerced = coerceToString(result); + return { value: coerced, raw: coerced }; } function limitToolPayload(