mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-10 01:29:17 +00:00
fix(core): disable qwen thinking via chat_template_kwargs on non-DashScope servers (#6271)
* fix(core): disable qwen thinking via chat_template_kwargs on non-DashScope servers
For hybrid-thinking qwen models, enable_thinking:false was only emitted for
DashScope providers, always as a top-level request field. Self-hosted
OpenAI-compatible servers (vLLM, SGLang) render the chat template server-side
and read the switch from chat_template_kwargs; they silently ignore a
top-level enable_thinking.
The visible failure is that auto-approval mode is unusable against a
self-hosted qwen endpoint: the permission classifier issues short
structured-output calls with a small token budget, and because thinking is
never disabled the model spends that budget emitting <think>, returns
truncated JSON, and every tool call fails closed.
When the model is a qwen/coder-model and the provider is not DashScope, set
chat_template_kwargs.enable_thinking = false (merged with any existing
chat_template_kwargs) instead of the top-level field. DashScope behavior is
unchanged. Adds a unit test for the non-DashScope path.
* fix(core): strip contradictory top-level enable_thinking on non-DashScope path
Address review: when a qwen model runs against a non-DashScope endpoint and a
provider preset injected enable_thinking:true via extra_body, emitting only
chat_template_kwargs.enable_thinking=false left the contradictory top-level
field in place. Servers that honour both signals could keep thinking enabled
despite the opt-out. Delete the top-level field on this path so the nested
switch is authoritative, matching the codebase's rule of not leaking the
qwen-specific enable_thinking field to non-DashScope servers.
Also covers the coder-model non-DashScope arm with a dedicated test, and
strengthens the vLLM test to inject a top-level enable_thinking:true and
assert it is stripped.
* test(core): cover chat_template_kwargs merge on non-DashScope path
Address review: assert the else-branch spreads pre-existing
chat_template_kwargs before appending enable_thinking:false, so a refactor
that drops the spread and loses user-configured kwargs is caught. Injects
chat_template_kwargs:{apply_chat_template:true} via buildRequest and expects
the merged {apply_chat_template:true, enable_thinking:false}.
---------
Co-authored-by: BeantownBytes <code@beantownbytes.com>
This commit is contained in:
parent
4675274ee4
commit
015ee42489
2 changed files with 168 additions and 7 deletions
|
|
@ -1108,6 +1108,141 @@ describe('ContentGenerationPipeline', () => {
|
|||
expect(apiCall.enable_thinking).toBeUndefined();
|
||||
});
|
||||
|
||||
it('disables qwen thinking via chat_template_kwargs on a non-DashScope endpoint (vLLM/SGLang)', async () => {
|
||||
// Self-hosted OpenAI-compatible servers render the chat template
|
||||
// server-side and read the thinking switch from `chat_template_kwargs`,
|
||||
// silently ignoring a top-level `enable_thinking`. A qwen model on such
|
||||
// an endpoint must therefore get the switch nested, not top-level — and
|
||||
// any top-level `enable_thinking: true` a provider preset injected via
|
||||
// extra_body must be stripped so it can't contradict the opt-out.
|
||||
mockContentGeneratorConfig = {
|
||||
...mockContentGeneratorConfig,
|
||||
baseUrl: 'https://llm.example.com/v1',
|
||||
model: 'Qwen3.6-27B',
|
||||
} as ContentGeneratorConfig;
|
||||
mockConfig = {
|
||||
...mockConfig,
|
||||
contentGeneratorConfig: mockContentGeneratorConfig,
|
||||
};
|
||||
pipeline = new ContentGenerationPipeline(mockConfig);
|
||||
|
||||
(mockProvider.buildRequest as Mock).mockImplementation((req) => ({
|
||||
...req,
|
||||
enable_thinking: true, // Simulates extra_body injection
|
||||
}));
|
||||
|
||||
const request: GenerateContentParameters = {
|
||||
model: 'Qwen3.6-27B',
|
||||
contents: [{ parts: [{ text: 'Suggest' }], role: 'user' }],
|
||||
config: { thinkingConfig: { includeThoughts: false } },
|
||||
};
|
||||
|
||||
(mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue([
|
||||
{ role: 'user', content: 'Suggest' },
|
||||
]);
|
||||
(mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue(
|
||||
new GenerateContentResponse(),
|
||||
);
|
||||
(mockClient.chat.completions.create as Mock).mockResolvedValue({
|
||||
id: 'r',
|
||||
choices: [{ message: { content: 'ok' }, finish_reason: 'stop' }],
|
||||
} as OpenAI.Chat.ChatCompletion);
|
||||
|
||||
await pipeline.execute(request, 'forked_query');
|
||||
|
||||
const apiCall = (mockClient.chat.completions.create as Mock).mock
|
||||
.calls[0][0];
|
||||
expect(apiCall.chat_template_kwargs).toEqual({ enable_thinking: false });
|
||||
expect(apiCall.enable_thinking).toBeUndefined();
|
||||
});
|
||||
|
||||
it('disables coder-model thinking via chat_template_kwargs on a non-DashScope endpoint', async () => {
|
||||
// `coder-model` is the QWEN_OAUTH default, but a user can point it at a
|
||||
// self-hosted endpoint. The `model === 'coder-model'` arm must reach the
|
||||
// non-DashScope chat_template_kwargs path just like a `qwen*` model.
|
||||
mockContentGeneratorConfig = {
|
||||
...mockContentGeneratorConfig,
|
||||
baseUrl: 'https://llm.example.com/v1',
|
||||
model: 'coder-model',
|
||||
} as ContentGeneratorConfig;
|
||||
mockConfig = {
|
||||
...mockConfig,
|
||||
contentGeneratorConfig: mockContentGeneratorConfig,
|
||||
};
|
||||
pipeline = new ContentGenerationPipeline(mockConfig);
|
||||
|
||||
const request: GenerateContentParameters = {
|
||||
model: 'coder-model',
|
||||
contents: [{ parts: [{ text: 'Suggest' }], role: 'user' }],
|
||||
config: { thinkingConfig: { includeThoughts: false } },
|
||||
};
|
||||
|
||||
(mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue([
|
||||
{ role: 'user', content: 'Suggest' },
|
||||
]);
|
||||
(mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue(
|
||||
new GenerateContentResponse(),
|
||||
);
|
||||
(mockClient.chat.completions.create as Mock).mockResolvedValue({
|
||||
id: 'r',
|
||||
choices: [{ message: { content: 'ok' }, finish_reason: 'stop' }],
|
||||
} as OpenAI.Chat.ChatCompletion);
|
||||
|
||||
await pipeline.execute(request, 'forked_query');
|
||||
|
||||
const apiCall = (mockClient.chat.completions.create as Mock).mock
|
||||
.calls[0][0];
|
||||
expect(apiCall.chat_template_kwargs).toEqual({ enable_thinking: false });
|
||||
expect(apiCall.enable_thinking).toBeUndefined();
|
||||
});
|
||||
|
||||
it('merges enable_thinking into pre-existing chat_template_kwargs on a non-DashScope endpoint', async () => {
|
||||
// The non-DashScope path spreads any existing `chat_template_kwargs`
|
||||
// before appending `enable_thinking: false`. Guard the merge so a future
|
||||
// refactor can't silently drop user-configured kwargs.
|
||||
mockContentGeneratorConfig = {
|
||||
...mockContentGeneratorConfig,
|
||||
baseUrl: 'https://llm.example.com/v1',
|
||||
model: 'Qwen3.6-27B',
|
||||
} as ContentGeneratorConfig;
|
||||
mockConfig = {
|
||||
...mockConfig,
|
||||
contentGeneratorConfig: mockContentGeneratorConfig,
|
||||
};
|
||||
pipeline = new ContentGenerationPipeline(mockConfig);
|
||||
|
||||
(mockProvider.buildRequest as Mock).mockImplementation((req) => ({
|
||||
...req,
|
||||
chat_template_kwargs: { apply_chat_template: true },
|
||||
}));
|
||||
|
||||
const request: GenerateContentParameters = {
|
||||
model: 'Qwen3.6-27B',
|
||||
contents: [{ parts: [{ text: 'Suggest' }], role: 'user' }],
|
||||
config: { thinkingConfig: { includeThoughts: false } },
|
||||
};
|
||||
|
||||
(mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue([
|
||||
{ role: 'user', content: 'Suggest' },
|
||||
]);
|
||||
(mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue(
|
||||
new GenerateContentResponse(),
|
||||
);
|
||||
(mockClient.chat.completions.create as Mock).mockResolvedValue({
|
||||
id: 'r',
|
||||
choices: [{ message: { content: 'ok' }, finish_reason: 'stop' }],
|
||||
} as OpenAI.Chat.ChatCompletion);
|
||||
|
||||
await pipeline.execute(request, 'forked_query');
|
||||
|
||||
const apiCall = (mockClient.chat.completions.create as Mock).mock
|
||||
.calls[0][0];
|
||||
expect(apiCall.chat_template_kwargs).toEqual({
|
||||
apply_chat_template: true,
|
||||
enable_thinking: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('does NOT emit enable_thinking on a non-qwen model routed through DashScope', async () => {
|
||||
// DashScope's compatible-mode endpoint routes multiple model families
|
||||
// (qwen3, GLM, DeepSeek). Hostname alone is not enough — GLM uses
|
||||
|
|
|
|||
|
|
@ -584,13 +584,39 @@ export class ContentGenerationPipeline {
|
|||
// start with `qwen` but is the most common hybrid-thinking model
|
||||
// for first-time users, so it must be covered.
|
||||
const model = (context.model ?? '').toLowerCase();
|
||||
if (
|
||||
DashScopeOpenAICompatibleProvider.isDashScopeProvider(
|
||||
this.contentGeneratorConfig,
|
||||
) &&
|
||||
(model.startsWith('qwen') || model === 'coder-model')
|
||||
) {
|
||||
typed['enable_thinking'] = false;
|
||||
if (model.startsWith('qwen') || model === 'coder-model') {
|
||||
if (
|
||||
DashScopeOpenAICompatibleProvider.isDashScopeProvider(
|
||||
this.contentGeneratorConfig,
|
||||
)
|
||||
) {
|
||||
typed['enable_thinking'] = false;
|
||||
} else {
|
||||
// Non-DashScope OpenAI-compatible servers (vLLM, SGLang, ...) render
|
||||
// the model's chat template server-side and read the thinking switch
|
||||
// from `chat_template_kwargs`, not a top-level `enable_thinking`
|
||||
// (which they silently ignore). Send it there so hybrid qwen models
|
||||
// actually stop emitting <think> when reasoning is disabled — e.g.
|
||||
// the auto-mode permission classifier's short structured-output
|
||||
// calls, which otherwise spend their small token budget on thinking
|
||||
// and fail closed. Servers that don't recognise `chat_template_kwargs`
|
||||
// ignore the unknown field, so the switch is a harmless no-op there.
|
||||
//
|
||||
// Drop any top-level `enable_thinking` a provider preset injected via
|
||||
// extra_body (provider-config.ts emits it for models configured with
|
||||
// `enableThinking: true`): leaving it would contradict the
|
||||
// `chat_template_kwargs` opt-out on servers that honour both, and
|
||||
// keeps this path from leaking the qwen-specific field top-level.
|
||||
delete typed['enable_thinking'];
|
||||
const existing = (typed['chat_template_kwargs'] ?? {}) as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
typed['chat_template_kwargs'] = {
|
||||
...existing,
|
||||
enable_thinking: false,
|
||||
};
|
||||
}
|
||||
}
|
||||
// Strip reasoning config — extra_body could inject it, overriding
|
||||
// buildReasoningConfig's decision to return {} for disabled thinking.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue