From 8721b41db20c989c09f1c2cdb5fde2c72e2eb3df Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 2 May 2026 17:52:39 +0800 Subject: [PATCH] fix(core): narrow DeepSeek thinking injection to tool_use turns + subdomain test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR review round 2: 1. Narrow injection scope to assistant turns containing tool_use. Live verification against api.deepseek.com/anthropic showed plain-text assistant turns without thinking are accepted unchanged — only tool_use turns trigger the HTTP 400. Injecting on every assistant turn unnecessarily bloats replay history with synthetic blocks the API does not require. Existing thinking blocks on any turn are still preserved untouched. 2. Add test coverage for the subdomain hostname branch (us.api.deepseek.com → matches), addressing the gap noted in review. 3. Update existing negative-case tests (non-deepseek / spoofed / reasoning=false / includeThoughts=false) to use tool_use scenarios so they actually exercise the gating logic instead of trivially passing under the narrowed scope. --- .../anthropicContentGenerator.test.ts | 142 +++++++++++------- .../converter.test.ts | 60 ++++---- .../anthropicContentGenerator/converter.ts | 48 +++--- 3 files changed, 146 insertions(+), 104 deletions(-) diff --git a/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts b/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts index 5f6050ae5..9b9587ea1 100644 --- a/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts +++ b/packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts @@ -498,7 +498,29 @@ describe('AnthropicContentGenerator', () => { // anthropic-compatible API rejects subsequent requests when any prior // assistant turn omits a thinking block while thinking mode is on. describe('DeepSeek anthropic-compatible provider', () => { - it('injects empty thinking blocks on prior assistant turns when baseUrl points to api.deepseek.com', async () => { + // Helper: tool-use assistant turn missing thinking — the only shape that + // actually triggers DeepSeek's HTTP 400. + const toolUseConversation = [ + { role: 'user' as const, parts: [{ text: 'Run tool' }] }, + { + role: 'model' as const, + parts: [{ functionCall: { id: 't1', name: 'tool', args: {} } }], + }, + { + role: 'user' as const, + parts: [ + { + functionResponse: { + id: 't1', + name: 'tool', + response: { output: 'ok' }, + }, + }, + ], + }, + ]; + + it('injects empty thinking blocks on tool-use assistant turns when baseUrl is api.deepseek.com', async () => { const { AnthropicContentGenerator } = await importGenerator(); anthropicState.createImpl.mockResolvedValue({ id: 'msg-1', @@ -521,23 +543,18 @@ describe('AnthropicContentGenerator', () => { await generator.generateContent({ model: 'models/ignored', - contents: [ - { role: 'user', parts: [{ text: 'Hi' }] }, - { role: 'model', parts: [{ text: 'Hello!' }] }, - { role: 'user', parts: [{ text: 'How are you?' }] }, - ], + contents: toolUseConversation, } as unknown as GenerateContentParameters); const [anthropicRequest] = anthropicState.lastCreateArgs as AnthropicCreateArgs; const messages = (anthropicRequest as { messages: unknown[] }).messages; - // Assistant turn should now have an empty thinking block prepended. expect(messages[1]).toEqual({ role: 'assistant', content: [ { type: 'thinking', thinking: '', signature: '' }, - { type: 'text', text: 'Hello!' }, + { type: 'tool_use', id: 't1', name: 'tool', input: {} }, ], }); }); @@ -565,11 +582,7 @@ describe('AnthropicContentGenerator', () => { await generator.generateContent({ model: 'models/ignored', - contents: [ - { role: 'user', parts: [{ text: 'Hi' }] }, - { role: 'model', parts: [{ text: 'Hello!' }] }, - { role: 'user', parts: [{ text: 'How are you?' }] }, - ], + contents: toolUseConversation, } as unknown as GenerateContentParameters); const [anthropicRequest] = @@ -580,11 +593,55 @@ describe('AnthropicContentGenerator', () => { role: 'assistant', content: [ { type: 'thinking', thinking: '', signature: '' }, - { type: 'text', text: 'Hello!' }, + { type: 'tool_use', id: 't1', name: 'tool', input: {} }, ], }); }); + it('matches regional DeepSeek subdomains (e.g. us.api.deepseek.com)', async () => { + const { AnthropicContentGenerator } = await importGenerator(); + anthropicState.createImpl.mockResolvedValue({ + id: 'msg-1', + model: 'unrelated-model', + content: [{ type: 'text', text: 'ok' }], + }); + + const generator = new AnthropicContentGenerator( + { + model: 'unrelated-model', + apiKey: 'test-key', + baseUrl: 'https://us.api.deepseek.com/anthropic', + timeout: 10_000, + maxRetries: 2, + samplingParams: { max_tokens: 500 }, + schemaCompliance: 'auto', + }, + mockConfig, + ); + + await generator.generateContent({ + model: 'models/ignored', + contents: toolUseConversation, + } as unknown as GenerateContentParameters); + + const [anthropicRequest] = + anthropicState.lastCreateArgs as AnthropicCreateArgs; + const messages = (anthropicRequest as { messages: unknown[] }).messages; + + expect(messages[1]).toEqual({ + role: 'assistant', + content: [ + { type: 'thinking', thinking: '', signature: '' }, + { type: 'tool_use', id: 't1', name: 'tool', input: {} }, + ], + }); + }); + + const toolOnlyAssistant = { + role: 'assistant', + content: [{ type: 'tool_use', id: 't1', name: 'tool', input: {} }], + }; + it('does not inject empty thinking blocks for non-deepseek providers', async () => { const { AnthropicContentGenerator } = await importGenerator(); anthropicState.createImpl.mockResolvedValue({ @@ -608,22 +665,15 @@ describe('AnthropicContentGenerator', () => { await generator.generateContent({ model: 'models/ignored', - contents: [ - { role: 'user', parts: [{ text: 'Hi' }] }, - { role: 'model', parts: [{ text: 'Hello!' }] }, - { role: 'user', parts: [{ text: 'How are you?' }] }, - ], + contents: toolUseConversation, } as unknown as GenerateContentParameters); const [anthropicRequest] = anthropicState.lastCreateArgs as AnthropicCreateArgs; const messages = (anthropicRequest as { messages: unknown[] }).messages; - // No thinking block injected for non-deepseek providers. - expect(messages[1]).toEqual({ - role: 'assistant', - content: [{ type: 'text', text: 'Hello!' }], - }); + // Non-deepseek provider: even tool_use turns get no injection. + expect(messages[1]).toEqual(toolOnlyAssistant); }); it('does not match spoofed hostnames like api.deepseek.com.evil.com', async () => { @@ -649,28 +699,23 @@ describe('AnthropicContentGenerator', () => { await generator.generateContent({ model: 'models/ignored', - contents: [ - { role: 'user', parts: [{ text: 'Hi' }] }, - { role: 'model', parts: [{ text: 'Hello!' }] }, - { role: 'user', parts: [{ text: 'How are you?' }] }, - ], + contents: toolUseConversation, } as unknown as GenerateContentParameters); const [anthropicRequest] = anthropicState.lastCreateArgs as AnthropicCreateArgs; const messages = (anthropicRequest as { messages: unknown[] }).messages; - // Hostname differs from api.deepseek.com — must not inject. - expect(messages[1]).toEqual({ - role: 'assistant', - content: [{ type: 'text', text: 'Hello!' }], - }); + // Hostname differs from api.deepseek.com — must not inject even on + // tool_use turns. + expect(messages[1]).toEqual(toolOnlyAssistant); }); it('does not inject when reasoning is explicitly disabled', async () => { - // Even on a confirmed-DeepSeek provider, if the request omits the - // top-level `thinking` parameter (because reasoning=false), shipping - // synthetic thinking blocks would be a protocol violation. + // Even on a confirmed-DeepSeek provider with a tool-use turn, if the + // request omits the top-level `thinking` parameter (because + // reasoning=false), shipping synthetic thinking blocks would be a + // protocol violation. const { AnthropicContentGenerator } = await importGenerator(); anthropicState.createImpl.mockResolvedValue({ id: 'msg-1', @@ -694,25 +739,17 @@ describe('AnthropicContentGenerator', () => { await generator.generateContent({ model: 'models/ignored', - contents: [ - { role: 'user', parts: [{ text: 'Hi' }] }, - { role: 'model', parts: [{ text: 'Hello!' }] }, - { role: 'user', parts: [{ text: 'How are you?' }] }, - ], + contents: toolUseConversation, } as unknown as GenerateContentParameters); const [anthropicRequest] = anthropicState.lastCreateArgs as AnthropicCreateArgs; const messages = (anthropicRequest as { messages: unknown[] }).messages; - // No `thinking` field in the request body, no injected blocks either. expect(anthropicRequest).toEqual( expect.not.objectContaining({ thinking: expect.anything() }), ); - expect(messages[1]).toEqual({ - role: 'assistant', - content: [{ type: 'text', text: 'Hello!' }], - }); + expect(messages[1]).toEqual(toolOnlyAssistant); }); it('does not inject when request sets thinkingConfig.includeThoughts=false', async () => { @@ -741,11 +778,7 @@ describe('AnthropicContentGenerator', () => { await generator.generateContent({ model: 'models/ignored', - contents: [ - { role: 'user', parts: [{ text: 'Hi' }] }, - { role: 'model', parts: [{ text: 'Hello!' }] }, - { role: 'user', parts: [{ text: 'How are you?' }] }, - ], + contents: toolUseConversation, config: { thinkingConfig: { includeThoughts: false } }, } as unknown as GenerateContentParameters); @@ -756,10 +789,7 @@ describe('AnthropicContentGenerator', () => { expect(anthropicRequest).toEqual( expect.not.objectContaining({ thinking: expect.anything() }), ); - expect(messages[1]).toEqual({ - role: 'assistant', - content: [{ type: 'text', text: 'Hello!' }], - }); + expect(messages[1]).toEqual(toolOnlyAssistant); }); }); diff --git a/packages/core/src/core/anthropicContentGenerator/converter.test.ts b/packages/core/src/core/anthropicContentGenerator/converter.test.ts index 382f1072d..7459d8e52 100644 --- a/packages/core/src/core/anthropicContentGenerator/converter.test.ts +++ b/packages/core/src/core/anthropicContentGenerator/converter.test.ts @@ -654,7 +654,10 @@ describe('AnthropicContentConverter', () => { describe('ensureAssistantThinking', () => { const enableThinking = { ensureAssistantThinking: true }; - it('injects an empty thinking block on assistant turns missing one', () => { + it('does not inject on plain-text assistant turns (DeepSeek tolerates them)', () => { + // Verified against api.deepseek.com/anthropic: plain-text assistant + // turns without thinking are accepted. Avoid bloating replay history + // with synthetic blocks the API does not require. const { messages } = converter.convertGeminiRequestToAnthropic( { model: 'models/test', @@ -666,21 +669,10 @@ describe('AnthropicContentConverter', () => { enableThinking, ); - expect(messages).toEqual([ - { - role: 'user', - content: [ - { type: 'text', text: 'Hi', cache_control: { type: 'ephemeral' } }, - ], - }, - { - role: 'assistant', - content: [ - { type: 'thinking', thinking: '', signature: '' }, - { type: 'text', text: 'Hello!' }, - ], - }, - ]); + expect(messages[1]).toEqual({ + role: 'assistant', + content: [{ type: 'text', text: 'Hello!' }], + }); }); it('injects an empty thinking block on tool-calling assistant turns missing one', () => { @@ -720,12 +712,12 @@ describe('AnthropicContentConverter', () => { }); }); - it('preserves existing thinking blocks on assistant turns', () => { + it('preserves existing thinking blocks on tool-use assistant turns', () => { const { messages } = converter.convertGeminiRequestToAnthropic( { model: 'models/test', contents: [ - { role: 'user', parts: [{ text: 'Hi' }] }, + { role: 'user', parts: [{ text: 'Run tool' }] }, { role: 'model', parts: [ @@ -734,7 +726,7 @@ describe('AnthropicContentConverter', () => { thought: true, thoughtSignature: 'sig', }, - { text: 'Hello!' }, + { functionCall: { id: 't1', name: 'tool', args: {} } }, ], }, ], @@ -746,7 +738,7 @@ describe('AnthropicContentConverter', () => { role: 'assistant', content: [ { type: 'thinking', thinking: 'Let me think', signature: 'sig' }, - { type: 'text', text: 'Hello!' }, + { type: 'tool_use', id: 't1', name: 'tool', input: {} }, ], }); }); @@ -785,16 +777,23 @@ describe('AnthropicContentConverter', () => { }); }); - it('injects thinking blocks on every prior assistant turn in a multi-turn history', () => { + it('injects thinking blocks on every tool-using assistant turn in a multi-turn history', () => { + const toolUse = (id: string) => ({ + functionCall: { id, name: 'tool', args: {} }, + }); + const toolResult = (id: string) => ({ + functionResponse: { id, name: 'tool', response: { output: 'ok' } }, + }); + const { messages } = converter.convertGeminiRequestToAnthropic( { model: 'models/test', contents: [ { role: 'user', parts: [{ text: 'Q1' }] }, - { role: 'model', parts: [{ text: 'A1' }] }, - { role: 'user', parts: [{ text: 'Q2' }] }, - { role: 'model', parts: [{ text: 'A2' }] }, - { role: 'user', parts: [{ text: 'Q3' }] }, + { role: 'model', parts: [toolUse('t1')] }, + { role: 'user', parts: [toolResult('t1')] }, + { role: 'model', parts: [toolUse('t2')] }, + { role: 'user', parts: [toolResult('t2')] }, ], }, enableThinking, @@ -814,7 +813,7 @@ describe('AnthropicContentConverter', () => { }); }); - it('does not inject a duplicate thinking block when one already exists (even without signature)', () => { + it('does not inject a duplicate thinking block when one already exists on a tool-use turn', () => { // A part `{ text: '', thought: true }` (e.g. from a redacted_thinking // response or a turn whose thinking stream had no content) converts to // a `{ type: 'thinking', thinking: '' }` block without a signature. The @@ -823,10 +822,13 @@ describe('AnthropicContentConverter', () => { { model: 'models/test', contents: [ - { role: 'user', parts: [{ text: 'Hi' }] }, + { role: 'user', parts: [{ text: 'Run tool' }] }, { role: 'model', - parts: [{ text: '', thought: true }, { text: 'Hello!' }], + parts: [ + { text: '', thought: true }, + { functionCall: { id: 't1', name: 'tool', args: {} } }, + ], }, ], }, @@ -837,7 +839,7 @@ describe('AnthropicContentConverter', () => { role: 'assistant', content: [ { type: 'thinking', thinking: '' }, - { type: 'text', text: 'Hello!' }, + { type: 'tool_use', id: 't1', name: 'tool', input: {} }, ], }); }); diff --git a/packages/core/src/core/anthropicContentGenerator/converter.ts b/packages/core/src/core/anthropicContentGenerator/converter.ts index 4d266cbd6..547dba465 100644 --- a/packages/core/src/core/anthropicContentGenerator/converter.ts +++ b/packages/core/src/core/anthropicContentGenerator/converter.ts @@ -563,12 +563,18 @@ export class AnthropicContentConverter { } /** - * DeepSeek's anthropic-compatible API rejects follow-up requests when any - * prior assistant turn omits a thinking block while thinking mode is on, - * returning HTTP 400 ("The content[].thinking in the thinking mode must be - * passed back to the API."). The model can legitimately return a turn - * without thinking content, so inject an empty thinking block whenever one - * is missing. https://github.com/QwenLM/qwen-code/issues/3786 + * DeepSeek's anthropic-compatible API rejects follow-up requests when an + * assistant turn carrying `tool_use` omits a thinking block while thinking + * mode is on, returning HTTP 400 ("The content[].thinking in the thinking + * mode must be passed back to the API."). The model can legitimately + * return a tool round without thinking content, so inject an empty thinking + * block when one is missing. + * + * Live verification against api.deepseek.com/anthropic confirmed the + * trigger is specific to tool_use turns — plain-text assistant turns + * without thinking are accepted unchanged. We mirror that boundary here to + * avoid bloating replay history with synthetic blocks for turns the API + * already accepts. https://github.com/QwenLM/qwen-code/issues/3786 */ private applyEmptyThinkingToAssistantMessages( messages: AnthropicMessageParam[], @@ -583,25 +589,29 @@ export class AnthropicContentConverter { ? message.content : []; + const hasToolUse = blocks.some( + (block) => (block as { type?: string }).type === 'tool_use', + ); + if (!hasToolUse) continue; + const hasThinking = blocks.some( (block) => (block as { type?: string }).type === 'thinking' || (block as { type?: string }).type === 'redacted_thinking', ); + if (hasThinking) continue; - if (!hasThinking) { - // DeepSeek currently accepts an empty `signature` for synthetic - // thinking blocks. The `signature` field is an opaque token in the - // Anthropic spec, so this is a workaround — if DeepSeek tightens - // validation in the future, we may need to switch to - // `redacted_thinking` or another approach. - const emptyThinking = { - type: 'thinking', - thinking: '', - signature: '', - } as unknown as AnthropicContentBlockParam; - message.content = [emptyThinking, ...blocks]; - } + // DeepSeek currently accepts an empty `signature` for synthetic + // thinking blocks. The `signature` field is an opaque token in the + // Anthropic spec, so this is a workaround — if DeepSeek tightens + // validation in the future, we may need to switch to + // `redacted_thinking` or another approach. + const emptyThinking = { + type: 'thinking', + thinking: '', + signature: '', + } as unknown as AnthropicContentBlockParam; + message.content = [emptyThinking, ...blocks]; } }