Skip to content

fix(core): mirror Qwen reasoning history field #4289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
YuanHanzhong wants to merge 1 commit into
base: main
Choose a base branch
from
+83 −1
Open

fix(core): mirror Qwen reasoning history field #4289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 38 additions & 0 deletions packages/core/src/core/openaiContentGenerator/provider/default.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,44 @@ describe('DefaultOpenAICompatibleProvider', () => {
});
});

it('should mirror reasoning_content to reasoning for Qwen model history', () => {
Copy link
Copy Markdown
Collaborator

@wenshao wenshao May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The single test covers only the happy path (one assistant message with reasoning_content). Four critical branches are untested:

  1. Non-Qwen model no-op — model: 'gpt-4' with reasoning_content should NOT inject reasoning
  2. Empty reasoning_content — reasoning_content: '' should skip mirroring (explicit length === 0 guard)
  3. Existing reasoning field — assistant message with both reasoning and reasoning_content should NOT overwrite reasoning
  4. Non-assistant messages — user/system messages with reasoning_content should be ignored (role !== 'assistant' guard)

These guards are the most frequently hit code paths and regressions here would silently break API behavior.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

const qwenProvider = new DefaultOpenAICompatibleProvider(
{
...mockContentGeneratorConfig,
model: 'Qwen/Qwen3.6-35B-A3B',
} as ContentGeneratorConfig,
mockCliConfig,
);
const originalRequest: OpenAI.Chat.ChatCompletionCreateParams = {
model: 'Qwen/Qwen3.6-35B-A3B',
messages: [
{
role: 'assistant',
content: 'The answer.',
reasoning_content: 'Prior reasoning.',
} as OpenAI.Chat.ChatCompletionAssistantMessageParam & {
reasoning_content: string;
},
],
};

const result = qwenProvider.buildRequest(originalRequest, 'prompt-id');
const assistant = result.messages[0] as
| OpenAI.Chat.ChatCompletionAssistantMessageParam
| (OpenAI.Chat.ChatCompletionAssistantMessageParam & {
reasoning_content?: string;
reasoning?: string;
});

expect(assistant).toEqual(
expect.objectContaining({
reasoning_content: 'Prior reasoning.',
reasoning: 'Prior reasoning.',
}),
);
expect(originalRequest.messages[0]).not.toHaveProperty('reasoning');
});

it('should not include extra_body when not configured', () => {
const originalRequest: OpenAI.Chat.ChatCompletionCreateParams = {
model: 'gpt-4',
Expand Down
46 changes: 45 additions & 1 deletion packages/core/src/core/openaiContentGenerator/provider/default.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
hasExplicitOutputLimit,
} from '../../tokenLimits.js';

const QWEN_MODEL_MARKER = 'qwen';

/**
* Default provider for standard OpenAI-compatible APIs
*/
Expand Down Expand Up @@ -76,10 +78,11 @@ export class DefaultOpenAICompatibleProvider
// This prevents occupying too much context window with output reservation
const requestWithTokenLimits = this.applyOutputTokenLimit(request);

return {
const builtRequest = {
...requestWithTokenLimits,
...(extraBody ? extraBody : {}),
};
return this.mirrorQwenReasoningContent(builtRequest);
}

getDefaultGenerationConfig(): GenerateContentConfig {
Expand Down Expand Up @@ -167,4 +170,45 @@ export class DefaultOpenAICompatibleProvider
max_tokens: effectiveMaxTokens,
};
}

Copy link
Copy Markdown
Collaborator

@wenshao wenshao May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Missing JSDoc on mirrorQwenReasoningContent. The method silently transforms messages by copying reasoning_content to reasoning, but there is no comment explaining why this is needed. Other methods like applyOutputTokenLimit and DeepSeek's equivalent have JSDoc blocks explaining the motivation and linked issues.

Suggested change
/**
* Mirrors `reasoning_content` to `reasoning` for Qwen model history.
* Qwen's self-hosted API requires the `reasoning` field (not just
* `reasoning_content`) when replaying assistant messages in multi-turn
* conversations. This copies the content without removing the original
* field to preserve backward compatibility.
*
* Fixes #4285
*/
private mirrorQwenReasoningContent(

— DeepSeek/deepseek-v4-pro via Qwen Code /review

private mirrorQwenReasoningContent(
request: OpenAI.Chat.ChatCompletionCreateParams,
): OpenAI.Chat.ChatCompletionCreateParams {
const model = request.model.toLowerCase();
if (!model.includes(QWEN_MODEL_MARKER)) {
return request;
}

let changed = false;
const messages = request.messages.map((message) => {
if (message.role !== 'assistant') {
return message;
}

const extended = message as unknown as Record<string, unknown>;
Copy link
Copy Markdown
Collaborator

@wenshao wenshao May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The double-cast as unknown as Record<string, unknown> to access reasoning_content can be replaced with the project's existing typed interface. ExtendedChatCompletionAssistantMessageParam (exported from converter.ts) already declares reasoning_content?: string | null. DeepSeek's equivalent function uses it directly:

Suggested change
const extended = message as unknown as Record<string, unknown>;
const extended = message as ExtendedChatCompletionAssistantMessageParam;
const reasoningContent = extended.reasoning_content;

This catches typos, provides IDE support, and is consistent with how ensureReasoningContentOnToolCalls in deepseek.ts accesses the same field. The write side (reasoning) still needs a cast since that field isn't in the type.

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

const reasoningContent = extended['reasoning_content'];
if (
typeof reasoningContent !== 'string' ||
reasoningContent.length === 0 ||
typeof extended['reasoning'] === 'string'
) {
return message;
}

changed = true;
return {
...extended,
reasoning: reasoningContent,
} as unknown as OpenAI.Chat.ChatCompletionMessageParam;
});

if (!changed) {
return request;
}

return {
...request,
messages,
};
}
}
Loading