From 36ebd63716b7304abda603d346d7fbd3092dcda8 Mon Sep 17 00:00:00 2001 From: clh02467605 Date: Mon, 27 Jul 2026 14:57:50 +0800 Subject: [PATCH] fix(text): omit enable_thinking by default and retry when API requires false Non-streaming chat no longer forces enable_thinking=false, which breaks thinking-only models. Retry once with false only when the server demands it. --- .../src/commands/auth/login-api-key.ts | 57 +++++--- packages/commands/src/commands/text/chat.ts | 36 +++-- packages/commands/tests/e2e/auth.e2e.test.ts | 2 +- .../commands/tests/e2e/text-chat.e2e.test.ts | 13 +- packages/core/src/index.ts | 1 + packages/core/src/models/index.ts | 7 + packages/core/src/models/thinking.ts | 76 ++++++++++ packages/core/tests/thinking.test.ts | 136 ++++++++++++++++++ 8 files changed, 290 insertions(+), 38 deletions(-) create mode 100644 packages/core/src/models/index.ts create mode 100644 packages/core/src/models/thinking.ts create mode 100644 packages/core/tests/thinking.test.ts diff --git a/packages/commands/src/commands/auth/login-api-key.ts b/packages/commands/src/commands/auth/login-api-key.ts index 6200f8e..9408498 100644 --- a/packages/commands/src/commands/auth/login-api-key.ts +++ b/packages/commands/src/commands/auth/login-api-key.ts @@ -4,6 +4,9 @@ import { chatPath, requestJson, normalizeModelBaseUrl, + applyChatEnableThinking, + resolveChatEnableThinking, + withEnableThinkingRetry, type AuthPersistPatch, type AuthStore, type Identity, @@ -58,32 +61,48 @@ export async function validateAndPersistApiKey( ? normalizeModelBaseUrl(profile.persistBaseUrl) : undefined; const validationModel = profile.defaultTextModel || "qwen3.7-max"; + const body: { + model: string; + messages: Array<{ role: string; content: string }>; + max_tokens: number; + stream: boolean; + enable_thinking?: boolean; + } = { + model: validationModel, + messages: [{ role: "user", content: "hi" }], + max_tokens: 1, + stream: false, + }; + const requestOpts = { url: baseUrl + chatPath(), method: "POST", headers: { Authorization: `Bearer ${key}` }, timeout: Math.min(deps.settings.timeout, 30), - body: { - model: validationModel, - messages: [{ role: "user", content: "hi" }], - max_tokens: 1, - stream: false, - enable_thinking: validationModel === "qwen3.8-max-preview", - }, + body, }; - for (let attempt = 1; attempt <= 3; attempt++) { - try { - await requestJson(httpDeps, requestOpts); - break; - } catch (error) { - if (attempt >= 3 || !canRetry(error)) { - process.stderr.write("Failed\n"); - throw error; - } - const delayMs = RETRY_DELAY_BASE_MS * 2 ** (attempt - 1); - await new Promise((resolve) => setTimeout(resolve, delayMs)); - } + try { + await withEnableThinkingRetry({ + // Validation requests are always non-streaming. + initial: resolveChatEnableThinking({ stream: false }), + apply: (value) => applyChatEnableThinking(body, value), + run: async () => { + for (let attempt = 1; attempt <= 3; attempt++) { + try { + await requestJson(httpDeps, requestOpts); + return; + } catch (error) { + if (attempt >= 3 || !canRetry(error)) throw error; + const delayMs = RETRY_DELAY_BASE_MS * 2 ** (attempt - 1); + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } + }, + }); + } catch (error) { + process.stderr.write("Failed\n"); + throw error; } process.stderr.write("Valid\n"); diff --git a/packages/commands/src/commands/text/chat.ts b/packages/commands/src/commands/text/chat.ts index 0ab05a3..6117fa4 100644 --- a/packages/commands/src/commands/text/chat.ts +++ b/packages/commands/src/commands/text/chat.ts @@ -4,6 +4,9 @@ import { parseSSE, detectOutputFormat, readTextFromPathOrStdin, + applyChatEnableThinking, + resolveChatEnableThinking, + withEnableThinkingRetry, type ChatMessage, type ChatRequest, type ChatResponse, @@ -124,7 +127,8 @@ export default defineCommand({ const { system, messages } = parseMessages(flags); const model = flags.model || settings.defaultTextModel || "qwen3.7-max"; - const shouldStream = flags.stream || process.stdout.isTTY; + // Coerce isTTY (may be undefined) so stream:false is serialized. + const shouldStream = Boolean(flags.stream || process.stdout.isTTY); const format = detectOutputFormat(settings.output); // Build messages array with system prompt @@ -144,16 +148,13 @@ export default defineCommand({ if (flags.temperature !== undefined) body.temperature = flags.temperature; if (flags.topP !== undefined) body.top_p = flags.topP; - if (flags.enableThinking) { - body.enable_thinking = true; - if (flags.thinkingBudget !== undefined) { - body.thinking_budget = flags.thinkingBudget; - } - } else if (!shouldStream) { - // DashScope qwen3 models default to enable_thinking=true server-side, but - // non-streaming calls require it to be explicitly false. Stream calls - // support thinking, so leave the field unset there (server handles it). - body.enable_thinking = false; + const enableThinking = resolveChatEnableThinking({ + enableThinking: flags.enableThinking, + stream: shouldStream, + }); + applyChatEnableThinking(body, enableThinking); + if (enableThinking === true && flags.thinkingBudget !== undefined) { + body.thinking_budget = flags.thinkingBudget; } if (flags.tool) { @@ -229,10 +230,15 @@ export default defineCommand({ resultOut.write("\n"); } } else { - const response = await ctx.client.requestJson({ - path: chatPath(), - method: "POST", - body, + const response = await withEnableThinkingRetry({ + initial: enableThinking, + apply: (value) => applyChatEnableThinking(body, value), + run: () => + ctx.client.requestJson({ + path: chatPath(), + method: "POST", + body, + }), }); const text = response.choices?.[0]?.message?.content ?? ""; diff --git a/packages/commands/tests/e2e/auth.e2e.test.ts b/packages/commands/tests/e2e/auth.e2e.test.ts index 5db70c1..3a04b47 100644 --- a/packages/commands/tests/e2e/auth.e2e.test.ts +++ b/packages/commands/tests/e2e/auth.e2e.test.ts @@ -317,7 +317,7 @@ describe("e2e: auth", () => { body: { model: "qwen3.8-max-preview", stream: false, - enable_thinking: true, + enable_thinking: false, }, }); diff --git a/packages/commands/tests/e2e/text-chat.e2e.test.ts b/packages/commands/tests/e2e/text-chat.e2e.test.ts index a5f4d5e..cbd7064 100644 --- a/packages/commands/tests/e2e/text-chat.e2e.test.ts +++ b/packages/commands/tests/e2e/text-chat.e2e.test.ts @@ -34,7 +34,7 @@ describe.skipIf(!isDashScopeE2EReady())("e2e: text chat（DashScope）", () => { "--model", "qwen3.7-max", "--message", - "干跑", + "dry-run", "--max-tokens", "8", "--output", @@ -42,10 +42,17 @@ describe.skipIf(!isDashScopeE2EReady())("e2e: text chat（DashScope）", () => { ]); expect(exitCode, stderr).toBe(0); const data = parseStdoutJson<{ - request?: { model?: string; messages?: Array<{ content?: string }> }; + request?: { + model?: string; + messages?: Array<{ content?: string }>; + enable_thinking?: boolean; + stream?: boolean; + }; }>(stdout); expect(data.request?.model).toBe("qwen3.7-max"); - expect(data.request?.messages?.some((m) => m.content === "干跑")).toBe(true); + expect(data.request?.messages?.some((message) => message.content === "dry-run")).toBe(true); + expect(data.request?.stream).toBe(false); + expect(data.request?.enable_thinking).toBe(false); }); test("【qwen3.7-max】文本对话", async () => { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e49ee6c..10d0911 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -14,5 +14,6 @@ export * from "./finetune/index.ts"; export * from "./deploy/index.ts"; export * from "./types/index.ts"; export * from "./utils/index.ts"; +export * from "./models/index.ts"; export * from "./telemetry/index.ts"; export * from "./advisor/index.ts"; diff --git a/packages/core/src/models/index.ts b/packages/core/src/models/index.ts new file mode 100644 index 0000000..4af5e3d --- /dev/null +++ b/packages/core/src/models/index.ts @@ -0,0 +1,7 @@ +export { + adjustEnableThinkingAfterError, + applyChatEnableThinking, + resolveChatEnableThinking, + withEnableThinkingRetry, + type EnableThinkingAdjustResult, +} from "./thinking.ts"; diff --git a/packages/core/src/models/thinking.ts b/packages/core/src/models/thinking.ts new file mode 100644 index 0000000..a5d25b1 --- /dev/null +++ b/packages/core/src/models/thinking.ts @@ -0,0 +1,76 @@ +/** resolve / adjust / retry helpers for chat `enable_thinking`. */ + +/** Resolve the initial `enable_thinking` value (`undefined` omits the field). */ +export function resolveChatEnableThinking(options: { + enableThinking?: boolean; + /** Whether the request is streaming. */ + stream?: boolean; +}): boolean | undefined { + if (options.enableThinking) return true; + if (options.stream === false) return false; + return undefined; +} + +export type EnableThinkingAdjustResult = + | { kind: "retry"; value: boolean | undefined } + | { kind: "none" }; + +/** Map clear `enable_thinking` constraint errors to a one-shot retry adjustment. */ +export function adjustEnableThinkingAfterError( + current: boolean | undefined, + errorMessage: string, +): EnableThinkingAdjustResult { + if (current !== true && /enable_thinking parameter is restricted to\s*true/i.test(errorMessage)) { + return { kind: "retry", value: true }; + } + + if (current === undefined && /enable_thinking must be set to false/i.test(errorMessage)) { + return { kind: "retry", value: false }; + } + + if (current !== undefined && /does not support enable_thinking/i.test(errorMessage)) { + return { kind: "retry", value: undefined }; + } + + return { kind: "none" }; +} + +/** Set or remove `enable_thinking`; clear `thinking_budget` when disabled or omitted. */ +export function applyChatEnableThinking( + body: { enable_thinking?: boolean; thinking_budget?: number }, + value: boolean | undefined, +): void { + if (value === undefined) { + delete body.enable_thinking; + delete body.thinking_budget; + return; + } + if (value === false) { + body.enable_thinking = false; + delete body.thinking_budget; + return; + } + body.enable_thinking = true; +} + +function errorMessageOf(error: unknown): string { + if (error instanceof Error) return error.message; + return String(error); +} + +/** Run once, then retry once if the error indicates an `enable_thinking` constraint. */ +export async function withEnableThinkingRetry(options: { + initial: boolean | undefined; + apply: (value: boolean | undefined) => void; + run: () => Promise; +}): Promise { + options.apply(options.initial); + try { + return await options.run(); + } catch (error) { + const adjusted = adjustEnableThinkingAfterError(options.initial, errorMessageOf(error)); + if (adjusted.kind === "none") throw error; + options.apply(adjusted.value); + return await options.run(); + } +} diff --git a/packages/core/tests/thinking.test.ts b/packages/core/tests/thinking.test.ts new file mode 100644 index 0000000..547acf4 --- /dev/null +++ b/packages/core/tests/thinking.test.ts @@ -0,0 +1,136 @@ +import { expect, test } from "vite-plus/test"; +import { + adjustEnableThinkingAfterError, + applyChatEnableThinking, + resolveChatEnableThinking, + withEnableThinkingRetry, +} from "../src/models/thinking.ts"; + +test("resolveChatEnableThinking：显式开启为 true，非流式默认 false，流式默认 omit", () => { + expect(resolveChatEnableThinking({ enableThinking: true })).toBe(true); + expect(resolveChatEnableThinking({ enableThinking: true, stream: false })).toBe(true); + expect(resolveChatEnableThinking({ stream: false })).toBe(false); + expect(resolveChatEnableThinking({ enableThinking: false, stream: false })).toBe(false); + expect(resolveChatEnableThinking({ stream: true })).toBeUndefined(); + expect(resolveChatEnableThinking({})).toBeUndefined(); +}); + +test("adjustEnableThinkingAfterError：false/omit 被要求 true 时重试为 true", () => { + expect( + adjustEnableThinkingAfterError( + false, + "The value of the enable_thinking parameter is restricted to True.", + ), + ).toEqual({ kind: "retry", value: true }); + expect( + adjustEnableThinkingAfterError( + undefined, + "The value of the enable_thinking parameter is restricted to True.", + ), + ).toEqual({ kind: "retry", value: true }); +}); + +test("adjustEnableThinkingAfterError：omit 被要求 false 时重试为 false", () => { + expect( + adjustEnableThinkingAfterError( + undefined, + "parameter.enable_thinking must be set to false for non-streaming calls", + ), + ).toEqual({ kind: "retry", value: false }); +}); + +test("adjustEnableThinkingAfterError：不支持时去掉字段", () => { + expect( + adjustEnableThinkingAfterError(false, "The model qwen-turbo does not support enable_thinking."), + ).toEqual({ kind: "retry", value: undefined }); + expect( + adjustEnableThinkingAfterError(true, "The model qwen-turbo does not support enable_thinking."), + ).toEqual({ kind: "retry", value: undefined }); +}); + +test("adjustEnableThinkingAfterError：无关错误不调整", () => { + expect(adjustEnableThinkingAfterError(undefined, "Access denied")).toEqual({ kind: "none" }); + expect(adjustEnableThinkingAfterError(false, "Model not exist")).toEqual({ kind: "none" }); + expect( + adjustEnableThinkingAfterError( + true, + "The value of the enable_thinking parameter is restricted to True.", + ), + ).toEqual({ kind: "none" }); +}); + +test("applyChatEnableThinking：设置 / 删除字段，并在关闭时清 thinking_budget", () => { + const body: { enable_thinking?: boolean; thinking_budget?: number } = { + thinking_budget: 1024, + }; + applyChatEnableThinking(body, true); + expect(body.enable_thinking).toBe(true); + expect(body.thinking_budget).toBe(1024); + + applyChatEnableThinking(body, false); + expect(body.enable_thinking).toBe(false); + expect(body).not.toHaveProperty("thinking_budget"); + + body.thinking_budget = 2048; + applyChatEnableThinking(body, undefined); + expect(body).not.toHaveProperty("enable_thinking"); + expect(body).not.toHaveProperty("thinking_budget"); +}); + +test("withEnableThinkingRetry：restricted-to-true 时从 false 重试为 true", async () => { + const values: Array = []; + let calls = 0; + + const result = await withEnableThinkingRetry({ + initial: false, + apply: (value) => { + values.push(value); + }, + run: async () => { + calls += 1; + if (calls === 1) { + throw new Error("The value of the enable_thinking parameter is restricted to True."); + } + return "ok"; + }, + }); + + expect(result).toBe("ok"); + expect(calls).toBe(2); + expect(values).toEqual([false, true]); +}); + +test("withEnableThinkingRetry：must-be-false 时从 omit 重试为 false", async () => { + const values: Array = []; + let calls = 0; + + const result = await withEnableThinkingRetry({ + initial: undefined, + apply: (value) => { + values.push(value); + }, + run: async () => { + calls += 1; + if (calls === 1) { + throw new Error("parameter.enable_thinking must be set to false for non-streaming calls"); + } + return "ok"; + }, + }); + + expect(result).toBe("ok"); + expect(calls).toBe(2); + expect(values).toEqual([undefined, false]); +}); + +test("withEnableThinkingRetry：无关错误原样抛出", async () => { + await expect( + withEnableThinkingRetry({ + initial: false, + apply: () => {}, + run: async () => { + throw new Error("Access denied"); + }, + }), + ).rejects.toThrow(/Access denied/); +});