From 167fb716096c327be28343ab23383975e1c4c3ed Mon Sep 17 00:00:00 2001 From: 404oops Date: Thu, 27 Aug 2026 17:32:28 +0200 Subject: [PATCH] fix(ai): scope the normalized flag, gate the Mistral stream split, correct four doc claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acts on a second triple-check audit. `normalized` means one thing. The driver stamped it on both the OpenAI-shape path and the legacy `response.normalize` path, which converts toward Anthropic blocks — so a caller could get `normalized: true` alongside array content, and the flag told them nothing they could branch on. The legacy branch no longer sets it. That branch is reachable only by a direct driver call with `response.normalize` and no `normalize`; the four wire routes pin `normalize: false`, which skips it. Mistral streaming, split by concern. Flattening a reasoning model's chunked `delta.content` to a string is a correctness floor and stays ungated — the shared handler passes the value straight to `addText`, so an array reaches the caller as stringified objects. Splitting the thinking text out into a `reasoning` delta is the dialect change and now sits behind the policy gate like the non-streaming remap. On the native path the thinking text is kept inline rather than dropped. Mistral `finishReason` is deleted only once its value carried over. The delete ran unconditionally, so a non-string `finishReason` with no `finish_reason` left the choice with no finish reason at all. Four doc claims corrected against the code paths they cover: `content` is string-or-null on normalized responses (tool-only turns carry no text, and the `// always a string` example comment was wrong); `reasoning_details` is not scoped to normalized responses, since Responses models emit it either way; and the release-date rule depends on the serving provider's own dates — OpenRouter derives them from its live API, so models newly listed there from 2026-09-01 normalize by default. Adds the test the copy-on-write fix was actually for: one messages array sent through two sequential calls, asserting the caller's array is untouched and both attempts carried the thinking signature. That is the fallback hazard; the harness wires one provider per model, so the fallback loop itself cannot be driven from a provider test. Co-Authored-By: Claude Opus 5 (1M context) --- .../ai-chat/ChatCompletionDriver.test.ts | 14 ++-- .../drivers/ai-chat/ChatCompletionDriver.ts | 7 +- .../providers/claude/ClaudeProvider.test.ts | 58 ++++++++++++++++ .../mistral/MistralAiProvider.test.ts | 69 +++++++++++++++++++ .../providers/mistral/MistralAiProvider.ts | 38 +++++++--- .../drivers/ai-chat/utils/OpenAIUtil.js | 4 ++ src/docs/src/AI/chat.md | 4 +- src/docs/src/Objects/chatresponse.md | 4 +- 8 files changed, 178 insertions(+), 20 deletions(-) diff --git a/src/backend/drivers/ai-chat/ChatCompletionDriver.test.ts b/src/backend/drivers/ai-chat/ChatCompletionDriver.test.ts index af97792fa..1de8b036f 100644 --- a/src/backend/drivers/ai-chat/ChatCompletionDriver.test.ts +++ b/src/backend/drivers/ai-chat/ChatCompletionDriver.test.ts @@ -813,9 +813,14 @@ describe('ChatCompletionDriver.complete normalization', () => { messages: [{ role: 'user', content: 'hi' }], response: { normalize: true }, }), - )) as { message: { role: string; content: unknown[] }; normalized: boolean }; + )) as { + message: { role: string; content: unknown[] }; + normalized?: boolean; + }; - expect(res.normalized).toBe(true); + // `normalized` is the caller's signal that the message is in the + // OpenAI shape; this branch produces Anthropic blocks, so it is absent. + expect(res.normalized).toBeUndefined(); expect(res.message.role).toBe('user'); // default role from normalize expect(res.message.content).toEqual([ { type: 'text', text: 'plain text reply' }, @@ -1015,8 +1020,9 @@ describe('ChatCompletionDriver.complete OpenAI-shape normalization', () => { }), )) as NormalizedResult; - // Legacy block shape, not the OpenAI string shape. - expect(res.normalized).toBe(true); + // Legacy block shape, not the OpenAI string shape — and therefore + // not flagged `normalized`, which means "OpenAI shape" specifically. + expect(res.normalized).toBeUndefined(); expect(res.message.content).toEqual([ { type: 'text', text: 'hi there' }, ]); diff --git a/src/backend/drivers/ai-chat/ChatCompletionDriver.ts b/src/backend/drivers/ai-chat/ChatCompletionDriver.ts index 8f1dc8a2b..eff5194ee 100644 --- a/src/backend/drivers/ai-chat/ChatCompletionDriver.ts +++ b/src/backend/drivers/ai-chat/ChatCompletionDriver.ts @@ -693,12 +693,15 @@ export class ChatCompletionDriver extends PuterDriver { }; } // The legacy flag normalizes the other way — to Anthropic blocks — - // and only when the new flag is absent. + // and only when the new flag is absent. It deliberately does NOT + // set `normalized`: that field is the caller's signal that the + // message is in the OpenAI shape, and this branch produces the + // opposite. Stamping both made the flag mean "some normalization + // happened", which no consumer can act on. if (args.normalize !== false && args.response?.normalize) { return { ...messageRes, message: normalize_single_message(messageRes.message), - normalized: true, via_ai_chat_service: true, }; } diff --git a/src/backend/drivers/ai-chat/providers/claude/ClaudeProvider.test.ts b/src/backend/drivers/ai-chat/providers/claude/ClaudeProvider.test.ts index c513f67ae..e85115e22 100644 --- a/src/backend/drivers/ai-chat/providers/claude/ClaudeProvider.test.ts +++ b/src/backend/drivers/ai-chat/providers/claude/ClaudeProvider.test.ts @@ -466,6 +466,64 @@ describe('ClaudeProvider.complete request shape', () => { ).toMatchObject({ type: 'thinking', signature: 'sig_1' }); }); + it('survives the same messages array being sent twice', async () => { + // This is the fallback hazard the copy-on-write exists for: the driver + // reuses one messages array across attempts, so if attempt 1 strips + // `reasoning_details` in place, attempt 2 sends a message with no + // thinking blocks and Anthropic rejects the continuation. Two + // sequential calls over one shared array reproduce that directly — the + // harness wires only one provider per model, so the real fallback loop + // cannot be driven from here. + const { provider } = makeProvider(); + messagesCreateMock + .mockResolvedValueOnce(baseResponse) + .mockResolvedValueOnce(baseResponse); + + const messages = [ + { role: 'user', content: 'think then call a tool' }, + { + role: 'assistant', + content: 'here you go', + reasoning: 'step one', + refusal: null, + reasoning_details: [ + { + type: 'thinking', + thinking: 'step one', + signature: 'sig_1', + }, + ], + }, + ]; + const before = structuredClone(messages); + + await withTestActor(() => + provider.complete({ + model: 'claude-haiku-4-5-20251001', + messages: messages as never, + }), + ); + await withTestActor(() => + provider.complete({ + model: 'claude-haiku-4-5-20251001', + messages: messages as never, + }), + ); + + // The caller's array is untouched by either attempt... + expect(messages).toEqual(before); + // ...so both attempts sent the thinking block with its signature. + for (const call of messagesCreateMock.mock.calls.slice(0, 2)) { + const sent = call[0].messages[1] as Record; + const content = sent.content as Array>; + expect(content[0]).toEqual({ + type: 'thinking', + thinking: 'step one', + signature: 'sig_1', + }); + } + }); + it('strips output-only reasoning fields even with no reasoning_details', async () => { const { provider } = makeProvider(); messagesCreateMock.mockResolvedValueOnce(baseResponse); diff --git a/src/backend/drivers/ai-chat/providers/mistral/MistralAiProvider.test.ts b/src/backend/drivers/ai-chat/providers/mistral/MistralAiProvider.test.ts index 9a62e5860..50d373bdf 100644 --- a/src/backend/drivers/ai-chat/providers/mistral/MistralAiProvider.test.ts +++ b/src/backend/drivers/ai-chat/providers/mistral/MistralAiProvider.test.ts @@ -732,6 +732,7 @@ describe('MistralAIProvider.complete streaming', () => { model: 'magistral-small-latest', messages: [{ role: 'user', content: 'think' }], stream: true, + normalize: true, }), ); @@ -752,6 +753,74 @@ describe('MistralAIProvider.complete streaming', () => { ); }); + it('never hands a chunk array to addText on the native path', async () => { + // Splitting thinking into a `reasoning` delta is the dialect change and + // is gated. Flattening the chunk array is not: the shared stream + // handler passes `delta.content` straight to addText, so leaving an + // array there would put stringified objects in the caller's text + // stream. Native path keeps every chunk's text, thinking included. + const { provider } = makeProvider(); + streamMock.mockReturnValueOnce( + asAsyncIterable([ + { + data: { + choices: [ + { + delta: { + content: [ + { + type: 'thinking', + thinking: [ + { + type: 'text', + text: 'thinking…', + }, + ], + }, + { type: 'text', text: 'answer' }, + ], + }, + }, + ], + }, + }, + { + data: { + choices: [{ delta: {} }], + usage: { promptTokens: 1, completionTokens: 1 }, + }, + }, + ]), + ); + + const result = await withTestActor(() => + provider.complete({ + model: 'magistral-small-latest', + messages: [{ role: 'user', content: 'think' }], + stream: true, + }), + ); + + const harness = makeCapturingChatStream(); + await ( + result as { + init_chat_stream: (p: { chatStream: unknown }) => Promise; + } + ).init_chat_stream({ chatStream: harness.chatStream }); + + const events = harness.events(); + // No reasoning channel on the native path... + expect(events.some((e) => e.type === 'reasoning')).toBe(false); + // ...and the text is text, not '[object Object]' or raw JSON. + const text = events + .filter((e) => e.type === 'text') + .map((e) => e.text) + .join(''); + expect(text).toBe('thinking…\n\nanswer'); + expect(text).not.toContain('object'); + expect(text).not.toContain('{'); + }); + it('builds a tool_use block from camelCase delta.toolCalls deltas', async () => { const { provider } = makeProvider(); streamMock.mockReturnValueOnce( diff --git a/src/backend/drivers/ai-chat/providers/mistral/MistralAiProvider.ts b/src/backend/drivers/ai-chat/providers/mistral/MistralAiProvider.ts index 904af4d1d..616850c76 100644 --- a/src/backend/drivers/ai-chat/providers/mistral/MistralAiProvider.ts +++ b/src/backend/drivers/ai-chat/providers/mistral/MistralAiProvider.ts @@ -194,13 +194,11 @@ export class MistralAIProvider implements IChatProvider { // for the equalized shape. (The streaming half is not gated: feeding a // chunk array to `addText` is a plain bug, and streamed chunks are // provider-uniform by design.) - if ( - !stream && - shouldPresentAsOpenAI( - { normalize, response }, - selectedModel.release_date, - ) - ) { + const presentAsOpenAI = shouldPresentAsOpenAI( + { normalize, response }, + selectedModel.release_date, + ); + if (!stream && presentAsOpenAI) { const choices = (completion as ChatCompletionResponse).choices ?? []; for (const choice of choices as unknown as Record< @@ -214,8 +212,11 @@ export class MistralAIProvider implements IChatProvider { choice.finish_reason = MISTRAL_FINISH_REASON_MAP[choice.finishReason] ?? choice.finishReason; + // Dropped only once its value carried over. Deleting + // unconditionally left a choice with neither key when + // `finishReason` was not a string. + delete choice.finishReason; } - delete choice.finishReason; const message = choice.message as | (Record & { toolCalls?: { @@ -276,9 +277,15 @@ export class MistralAIProvider implements IChatProvider { return snake_usage; }, // Mistral wraps each event; unwrap it, then flatten a - // reasoning model's chunked `delta.content` so the shared - // stream handler sees the string content + `reasoning` delta - // it expects from every other provider. + // reasoning model's chunked `delta.content`. + // + // Two concerns, gated differently. The shared stream handler + // passes `delta.content` straight to `addText`, so a chunk + // array would reach the caller as stringified objects — + // flattening it to text is a correctness floor that applies on + // every path. Splitting thinking out into a `reasoning` delta + // is the dialect change, and that sits behind the policy gate + // like the non-streaming remap. chunk_but_like_actually: (chunk: unknown) => { const data = (chunk as { data?: unknown }).data as | { @@ -294,6 +301,15 @@ export class MistralAIProvider implements IChatProvider { const { text, reasoning } = splitMistralContentChunks( delta.content, ); + if (!presentAsOpenAI) { + // Native path: keep every chunk's text, thinking + // included, so nothing is silently dropped — but + // never hand an array to `addText`. + delta.content = reasoning + ? [reasoning, text].filter(Boolean).join('\n\n') + : text; + continue; + } delta.content = text; if (reasoning && delta.reasoning === undefined) { delta.reasoning = reasoning; diff --git a/src/backend/drivers/ai-chat/utils/OpenAIUtil.js b/src/backend/drivers/ai-chat/utils/OpenAIUtil.js index 036639511..98405466a 100644 --- a/src/backend/drivers/ai-chat/utils/OpenAIUtil.js +++ b/src/backend/drivers/ai-chat/utils/OpenAIUtil.js @@ -314,6 +314,10 @@ const renameReasoningContent = (obj) => { if (obj.reasoning === undefined && obj.reasoning_content !== undefined) { obj.reasoning = obj.reasoning_content; } + // Dropped even when `reasoning` already won: a provider sending both + // means the same thing twice, and the vendor key is the one Puter does not + // expose. Pinned by BytePlusProvider.test.ts / ZAIProvider.test.ts, whose + // fixtures name the value 'should-be-dropped'. delete obj.reasoning_content; }; diff --git a/src/docs/src/AI/chat.md b/src/docs/src/AI/chat.md index 85bac5deb..a5193a46e 100755 --- a/src/docs/src/AI/chat.md +++ b/src/docs/src/AI/chat.md @@ -126,7 +126,7 @@ You can control this per call with the `normalize` option: ```js // Force the OpenAI format on any model, old or new: const response = await puter.ai.chat("Hello", { model: "claude-sonnet-5", normalize: true }); -console.log(response.message.content); // always a string +console.log(response.message.content); // a string, or null on a tool-only turn console.log(response.finish_reason); // "stop" | "length" | "tool_calls" | "content_filter" | vendor value // Force the vendor-native format, even on a post-cutoff model: @@ -148,6 +148,8 @@ Normalization does not cost you the ability to continue a reasoning turn. The op One caveat. The release-date rule applies to the model that actually serves the request — if a request is rerouted to a fallback provider, the served model's release date decides. +One thing to know about the release-date rule: a model's release date comes from the catalog of whichever provider serves it, and some providers report it from their own live listing. Models served through OpenRouter carry the date OpenRouter itself assigns, so a model newly listed there on or after September 1, 2026 is normalized by default without Puter shipping any change. Pin `normalize: false` if your code depends on a provider's native shape. + Streaming is unaffected by normalization: streamed [`ChatResponseChunk`](/Objects/chatresponsechunk) objects already share one format across all vendors. ## Function Calling diff --git a/src/docs/src/Objects/chatresponse.md b/src/docs/src/Objects/chatresponse.md index a37b0cfc6..c38363d84 100644 --- a/src/docs/src/Objects/chatresponse.md +++ b/src/docs/src/Objects/chatresponse.md @@ -13,13 +13,13 @@ An object containing the chat message data. - `role` (String) - The role of the message sender. -- `content` (String | Array) - The content of the message. A string on normalized (OpenAI-format) responses — which includes all models released on or after September 1, 2026 and any call made with `normalize: true`. On older Anthropic models without `normalize: true`, this is the vendor-native array of content blocks such as `[{ type: "text", text: "..." }]`. See [Response Normalization](/AI/chat#response-normalization). +- `content` (String | Array) - The content of the message. On normalized (OpenAI-format) responses — which includes all models released on or after September 1, 2026 and any call made with `normalize: true` — this is a string, or `null` when the model returned only tool calls and no text. On older Anthropic models without `normalize: true`, this is the vendor-native array of content blocks such as `[{ type: "text", text: "..." }]`. See [Response Normalization](/AI/chat#response-normalization). - `tool_calls` (Array) - An optional array of [`ToolCall`](/Objects/toolcall) objects if the model wants to call tools. - `reasoning` (String) - Optional extended-thinking output, when the model exposes it. Multiple reasoning segments are joined with a blank line between them. -- `reasoning_details` (Array) - Optional opaque reasoning artifacts, present on normalized responses from models that expose them: Anthropic `thinking`/`redacted_thinking` blocks with their `signature`, or OpenAI reasoning items with their `id` and `encrypted_content`. Treat the contents as opaque and resend the array verbatim to continue an extended-thinking turn — providers reject a continuation whose reasoning lost its signature. The human-readable text is in `reasoning`; this field is only for the round trip. +- `reasoning_details` (Array) - Optional opaque reasoning artifacts from models that expose them. Present on normalized Anthropic responses, and on OpenAI Responses-API models whether or not the response was normalized. Contents: Anthropic `thinking`/`redacted_thinking` blocks with their `signature`, or OpenAI reasoning items with their `id` and `encrypted_content`. Treat the contents as opaque and resend the array verbatim to continue an extended-thinking turn — providers reject a continuation whose reasoning lost its signature. The human-readable text is in `reasoning`; this field is only for the round trip. - `tool_call_id` (String) - An optional identifier linking this message to the tool call it responds to.