Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-openai-legacy-thinking-off.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

Honor an explicit thinking "off" on OpenAI-compatible (chat completions) providers: it used to be indistinguishable from "never configured", so the history-based auto `reasoning_effort` injection kept the model reasoning (and could leak the field to models that reject it). The provider now also reports the actual current thinking effort ("on"/"off") instead of recording "off" for both.
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ export class OpenAILegacyChatProvider implements ChatProvider {
private _baseUrl: string | undefined;
private _defaultHeaders: Record<string, string> | undefined;
private _reasoningKey: string | undefined;
private _reasoningEffort: string | undefined;
private _thinkingEffort: ThinkingEffort | undefined;
private _generationKwargs: OpenAILegacyGenerationKwargs;
private _toolMessageConversion: ToolMessageConversion;
private _client: OpenAI | undefined;
Expand All @@ -452,7 +452,7 @@ export class OpenAILegacyChatProvider implements ChatProvider {
normalizedReasoningKey !== undefined && normalizedReasoningKey.length > 0
? normalizedReasoningKey
: undefined;
this._reasoningEffort = undefined;
this._thinkingEffort = undefined;
this._generationKwargs =
options.maxTokens !== undefined ? completionTokenKwargs(this._model, options.maxTokens) : {};
this._toolMessageConversion = options.toolMessageConversion ?? null;
Expand All @@ -467,8 +467,7 @@ export class OpenAILegacyChatProvider implements ChatProvider {
}

get thinkingEffort(): ThinkingEffort | null {
if (this._reasoningEffort === undefined) return null;
return this._reasoningEffort === 'none' ? 'off' : this._reasoningEffort;
return this._thinkingEffort ?? null;
}

get maxCompletionTokens(): number | undefined {
Expand Down Expand Up @@ -506,9 +505,19 @@ export class OpenAILegacyChatProvider implements ChatProvider {
this._generationKwargs,
);

let reasoningEffort: string | undefined = this._reasoningEffort;
// 'off' and 'on' have no wire encoding; only a concrete effort is passed
// through. An explicit 'off' must also suppress the history-based
// auto-enable below (issue #1616), so it stays distinguishable from
// "never configured".
const effort = this._thinkingEffort;
let reasoningEffort: string | undefined =
effort === undefined || effort === 'off' || effort === 'on' ? undefined : effort;

if (reasoningEffort === undefined && kwargs['reasoning_effort'] === undefined) {
if (
reasoningEffort === undefined &&
effort !== 'off' &&
kwargs['reasoning_effort'] === undefined
) {
const hasThinkPart = history.some((message) =>
message.content.some((part) => part.type === 'think'),
);
Expand Down Expand Up @@ -560,9 +569,8 @@ export class OpenAILegacyChatProvider implements ChatProvider {
}

withThinking(effort: ThinkingEffort): OpenAILegacyChatProvider {
const reasoningEffort = effort === 'off' || effort === 'on' ? undefined : effort;
const clone = this._clone();
clone._reasoningEffort = reasoningEffort;
clone._thinkingEffort = effort;
return clone;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/**
* Scenario: OpenAI-compatible (chat completions) thinking effort encoding.
* Responsibilities: encode withThinking onto the wire, keep an explicit 'off'
* distinct from "never configured", and report the accurate effort.
* Wiring: real v2 OpenAILegacy adapter with only the remote SDK client boundary
* replaced by mocks.
* Run: pnpm exec vitest run packages/agent-core-v2/test/app/llmProtocol/providers/openai-legacy.test.ts
*/
import type { Message } from '#/app/llmProtocol/message';
import { OpenAILegacyChatProvider } from '#/app/llmProtocol/providers/openai-legacy';
import { describe, expect, it, vi } from 'vitest';

const USER_TURN: Message = {
role: 'user',
content: [{ type: 'text', text: 'Think' }],
toolCalls: [],
};

const THINK_HISTORY: Message[] = [
{ role: 'user', content: [{ type: 'text', text: 'Hello' }], toolCalls: [] },
{
role: 'assistant',
content: [
{ type: 'think', think: 'Thinking...' },
{ type: 'text', text: 'Hi!' },
],
toolCalls: [],
},
{ role: 'user', content: [{ type: 'text', text: 'How are you?' }], toolCalls: [] },
];

function createProvider(model = 'gpt-4.1'): OpenAILegacyChatProvider {
return new OpenAILegacyChatProvider({
model,
apiKey: 'test-key',
stream: false,
});
}

function makeChatCompletionResponse(model = 'test-model') {
return {
id: 'chatcmpl-test123',
object: 'chat.completion',
created: 1234567890,
model,
choices: [
{
index: 0,
message: { role: 'assistant', content: 'Hello' },
finish_reason: 'stop',
},
],
usage: { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 },
};
}

async function captureRequestBody(
provider: OpenAILegacyChatProvider,
history: Message[],
): Promise<Record<string, unknown>> {
let capturedBody: Record<string, unknown> | undefined;
(
provider as unknown as { _client: { chat: { completions: { create: unknown } } } }
)._client.chat.completions.create = vi.fn().mockImplementation((params: unknown) => {
capturedBody = params as Record<string, unknown>;
return Promise.resolve(makeChatCompletionResponse());
});

const stream = await provider.generate('', [], history);
for await (const part of stream) void part;

if (capturedBody === undefined) {
throw new Error('Expected provider.generate() to call chat.completions.create');
}
return capturedBody;
}

describe('OpenAILegacyChatProvider withThinking', () => {
it('passes a concrete effort through verbatim and reports it', async () => {
const provider = createProvider().withThinking('high');

const body = await captureRequestBody(provider, [USER_TURN]);
expect(body['reasoning_effort']).toBe('high');
expect(provider.thinkingEffort).toBe('high');
});

it('reports a null thinkingEffort until withThinking is called', () => {
expect(createProvider().thinkingEffort).toBeNull();
});

it('sends no reasoning_effort for "on" without ThinkPart history and reports "on"', async () => {
const provider = createProvider().withThinking('on');

const body = await captureRequestBody(provider, [USER_TURN]);
expect(body['reasoning_effort']).toBeUndefined();
expect(provider.thinkingEffort).toBe('on');
});

it('sends no reasoning_effort for "off" and reports "off"', async () => {
const provider = createProvider().withThinking('off');

const body = await captureRequestBody(provider, [USER_TURN]);
expect(body['reasoning_effort']).toBeUndefined();
expect(provider.thinkingEffort).toBe('off');
});

it('clears a concrete effort set earlier when turned off', async () => {
const provider = createProvider().withThinking('high').withThinking('off');
expect(provider.thinkingEffort).toBe('off');

const body = await captureRequestBody(provider, [USER_TURN]);
expect(body['reasoning_effort']).toBeUndefined();
});

it('auto-injects reasoning_effort when ThinkPart history exists and thinking is unconfigured', async () => {
// Issue #1616: strict OpenAI-compatible gateways require a paired
// reasoning_effort when the history carries reasoning_content.
const body = await captureRequestBody(createProvider(), THINK_HISTORY);
expect(body['reasoning_effort']).toBe('medium');
});

it('still auto-injects reasoning_effort for an explicit "on"', async () => {
const body = await captureRequestBody(createProvider().withThinking('on'), THINK_HISTORY);
expect(body['reasoning_effort']).toBe('medium');
});

it('does not auto-inject reasoning_effort when thinking was explicitly turned off', async () => {
// An explicit withThinking('off') is not "never configured": with thinking
// off, the auto-enable must not switch reasoning back on (or leak the field
// to models that reject it).
const provider = createProvider().withThinking('off');

const body = await captureRequestBody(provider, THINK_HISTORY);
expect(body['reasoning_effort']).toBeUndefined();
expect(provider.thinkingEffort).toBe('off');
});

it('does not overwrite reasoning_effort pinned via withGenerationKwargs', async () => {
const provider = createProvider().withGenerationKwargs({ reasoning_effort: 'high' });

const body = await captureRequestBody(provider, THINK_HISTORY);
expect(body['reasoning_effort']).toBe('high');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ describe('ProtocolAdapterRegistry', () => {
apiKey: 'sk',
});

expect(Reflect.get(provider.withThinking('max'), '_reasoningEffort')).toBe('max');
expect(provider.withThinking('max').thinkingEffort).toBe('max');
expect(Reflect.get(provider.withThinking('medium'), '_reasoningEffort')).toBe('medium');
expect(provider.withThinking('medium').thinkingEffort).toBe('medium');
});

Expand Down
32 changes: 22 additions & 10 deletions packages/kosong/src/providers/openai-legacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ export class OpenAILegacyChatProvider implements ChatProvider {
private _baseUrl: string | undefined;
private _defaultHeaders: Record<string, string> | undefined;
private _reasoningKey: string | undefined;
private _reasoningEffort: string | undefined;
private _thinkingEffort: ThinkingEffort | undefined;
private _generationKwargs: OpenAILegacyGenerationKwargs;
private _toolMessageConversion: ToolMessageConversion;
private _client: OpenAI | undefined;
Expand All @@ -505,7 +505,7 @@ export class OpenAILegacyChatProvider implements ChatProvider {
normalizedReasoningKey !== undefined && normalizedReasoningKey.length > 0
? normalizedReasoningKey
: undefined;
this._reasoningEffort = undefined;
this._thinkingEffort = undefined;
this._generationKwargs =
options.maxTokens !== undefined ? completionTokenKwargs(this._model, options.maxTokens) : {};
this._toolMessageConversion = options.toolMessageConversion ?? null;
Expand All @@ -520,8 +520,7 @@ export class OpenAILegacyChatProvider implements ChatProvider {
}

get thinkingEffort(): ThinkingEffort | null {
if (this._reasoningEffort === undefined) return null;
return this._reasoningEffort === 'none' ? 'off' : this._reasoningEffort;
return this._thinkingEffort ?? null;
}

get modelParameters(): Record<string, unknown> {
Expand Down Expand Up @@ -555,16 +554,27 @@ export class OpenAILegacyChatProvider implements ChatProvider {
this._generationKwargs,
);

// Determine reasoning_effort
let reasoningEffort: string | undefined = this._reasoningEffort;
// Determine reasoning_effort. 'off' and 'on' have no wire encoding on
// chat-completions APIs, so they send no reasoning_effort field; only a
// concrete effort (low/medium/high/...) is passed through verbatim.
const effort = this._thinkingEffort;
let reasoningEffort: string | undefined =
effort === undefined || effort === 'off' || effort === 'on' ? undefined : effort;

// Auto-enable reasoning_effort when the history contains ThinkPart but reasoning
// was not explicitly configured. This prevents server validation errors from APIs
// (e.g. One API) that require reasoning_effort when messages contain reasoning_content.
// Skip when the caller already pinned reasoning_effort via withGenerationKwargs —
// their value would otherwise be silently overwritten below.
// their value would otherwise be silently overwritten below. An explicit 'off'
// from withThinking is honored as well: with thinking turned off the
// auto-enable must not silently switch reasoning back on (or leak the field
// to models that reject it).
// See: https://github.com/MoonshotAI/kimi-code/issues/1616
if (reasoningEffort === undefined && kwargs['reasoning_effort'] === undefined) {
if (
reasoningEffort === undefined &&
effort !== 'off' &&
kwargs['reasoning_effort'] === undefined
) {
const hasThinkPart = history.some((message) =>
message.content.some((part) => part.type === 'think'),
);
Expand Down Expand Up @@ -618,9 +628,11 @@ export class OpenAILegacyChatProvider implements ChatProvider {
}

withThinking(effort: ThinkingEffort): OpenAILegacyChatProvider {
const reasoningEffort = effort === 'off' || effort === 'on' ? undefined : effort;
const clone = this._clone();
clone._reasoningEffort = reasoningEffort;
// Store the requested effort verbatim; the wire encoding is derived per
// request so an explicit 'off' stays distinguishable from "never
// configured" (which the history-based auto-enable relies on).
clone._thinkingEffort = effort;
return clone;
}

Expand Down
83 changes: 83 additions & 0 deletions packages/kosong/test/openai-legacy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,43 @@ describe('OpenAILegacyChatProvider', () => {
expect(maxBody['reasoning_effort']).toBe('max');
expect(xhighBody['reasoning_effort']).toBe('xhigh');
});

it('.withThinking("off") sends no reasoning_effort and reports "off"', async () => {
const provider = createProvider().withThinking('off');
const history: Message[] = [
{ role: 'user', content: [{ type: 'text', text: 'Think' }], toolCalls: [] },
];
const body = await captureRequestBody(provider, '', [], history);

expect(body['reasoning_effort']).toBeUndefined();
expect(provider.thinkingEffort).toBe('off');
});

it('.withThinking("on") sends no reasoning_effort without ThinkPart history and reports "on"', async () => {
const provider = createProvider().withThinking('on');
const history: Message[] = [
{ role: 'user', content: [{ type: 'text', text: 'Think' }], toolCalls: [] },
];
const body = await captureRequestBody(provider, '', [], history);

expect(body['reasoning_effort']).toBeUndefined();
expect(provider.thinkingEffort).toBe('on');
});

it('reports a null thinkingEffort until withThinking is called', () => {
expect(createProvider().thinkingEffort).toBeNull();
});

it('.withThinking("off") clears a concrete effort set earlier', async () => {
const provider = createProvider().withThinking('high').withThinking('off');
expect(provider.thinkingEffort).toBe('off');

const history: Message[] = [
{ role: 'user', content: [{ type: 'text', text: 'Think' }], toolCalls: [] },
];
const body = await captureRequestBody(provider, '', [], history);
expect(body['reasoning_effort']).toBeUndefined();
});
});

describe('auto reasoning_effort', () => {
Expand Down Expand Up @@ -1082,6 +1119,52 @@ describe('OpenAILegacyChatProvider', () => {

expect(body['reasoning_effort']).toBe('high');
});

it('does not auto-inject reasoning_effort when thinking was explicitly turned off', async () => {
// An explicit withThinking('off') is not the same as "never configured":
// with thinking off, auto-injection must not silently switch reasoning
// back on (or leak reasoning_effort to models that reject the field).
const provider = createProvider({ model: 'some-model' }).withThinking('off');
const history: Message[] = [
{ role: 'user', content: [{ type: 'text', text: 'Hello' }], toolCalls: [] },
{
role: 'assistant',
content: [
{ type: 'think', think: 'Thinking...' },
{ type: 'text', text: 'Hi!' },
],
toolCalls: [],
},
{ role: 'user', content: [{ type: 'text', text: 'How are you?' }], toolCalls: [] },
];
const body = await captureRequestBody(provider, '', [], history);

expect(body['reasoning_effort']).toBeUndefined();
expect(provider.thinkingEffort).toBe('off');
});

it('still auto-injects reasoning_effort for an explicit "on"', async () => {
// 'on' keeps the #1616 behavior: thinking enabled without a concrete
// effort still pairs reasoning_effort with ThinkPart history so strict
// OpenAI-compatible gateways don't 400.
const provider = createProvider({ model: 'some-model' }).withThinking('on');
const history: Message[] = [
{ role: 'user', content: [{ type: 'text', text: 'Hello' }], toolCalls: [] },
{
role: 'assistant',
content: [
{ type: 'think', think: 'Thinking...' },
{ type: 'text', text: 'Hi!' },
],
toolCalls: [],
},
{ role: 'user', content: [{ type: 'text', text: 'How are you?' }], toolCalls: [] },
];
const body = await captureRequestBody(provider, '', [], history);

expect(body['reasoning_effort']).toBe('medium');
expect(provider.thinkingEffort).toBe('on');
});
});

describe('default reasoning protocol (no explicit reasoningKey)', () => {
Expand Down
Loading