diff --git a/src/api/providers/__tests__/nanogpt.spec.ts b/src/api/providers/__tests__/nanogpt.spec.ts index 4de0998afb..e8e625aa2b 100644 --- a/src/api/providers/__tests__/nanogpt.spec.ts +++ b/src/api/providers/__tests__/nanogpt.spec.ts @@ -204,6 +204,252 @@ describe("NanoGptHandler", () => { expect(mockCreate.mock.calls[0][0]).not.toHaveProperty("temperature") }) + it("uses the model's advertised reasoning effort when settings are unset", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: ["disable", "low", "high"], + reasoningEffort: "high", + }, + }) + + await collectStream(new NanoGptHandler({ nanoGptModelId: "model:thinking" }).createMessage("sys", messages)) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "high" }) + }) + + it("uses the first supported effort when the model cannot disable reasoning", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: ["high", "medium", "low"], + }, + }) + + await collectStream(new NanoGptHandler({ nanoGptModelId: "model:thinking" }).createMessage("sys", messages)) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "low" }) + }) + + it.each([ + ["an unsupported configured effort", { reasoningEffort: "max" as const }, ["low", "high"] as const, undefined], + ["a none model default", {}, ["none", "low"] as const, "none" as const], + ["a minimal model default", {}, ["minimal", "low"] as const, "minimal" as const], + ])("uses a canonical fallback for %s", async (_name, settings, supportsReasoningEffort, reasoningEffort) => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: [...supportsReasoningEffort], + reasoningEffort, + }, + }) + + await collectStream( + new NanoGptHandler({ nanoGptModelId: "model:thinking", ...settings }).createMessage("sys", messages), + ) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "low" }) + }) + + it("uses a configured effort when reasoning support is boolean", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: true, + }, + }) + + await collectStream( + new NanoGptHandler({ nanoGptModelId: "model:thinking", reasoningEffort: "high" }).createMessage( + "sys", + messages, + ), + ) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "high" }) + }) + + it("honors disable when optional reasoning support is boolean", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: true, + reasoningEffort: "high", + }, + }) + + await collectStream( + new NanoGptHandler({ nanoGptModelId: "model:thinking", reasoningEffort: "disable" }).createMessage( + "sys", + messages, + ), + ) + + expect(mockCreate.mock.calls[0][0]).not.toHaveProperty("reasoning_effort") + }) + + it("omits an unset optional effort when disable is supported and no default is advertised", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: ["disable", "low", "high"], + }, + }) + + await collectStream(new NanoGptHandler({ nanoGptModelId: "model:thinking" }).createMessage("sys", messages)) + + expect(mockCreate.mock.calls[0][0]).not.toHaveProperty("reasoning_effort") + }) + + it.each([ + ["a stale disable effort", { reasoningEffort: "disable" as const }], + ["a stale disabled toggle", { enableReasoningEffort: false }], + ])("uses a supported fallback for %s when the model cannot disable reasoning", async (_name, settings) => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: ["low", "high"], + }, + }) + + await collectStream( + new NanoGptHandler({ nanoGptModelId: "model:thinking", ...settings }).createMessage("sys", messages), + ) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "low" }) + }) + + it.each([undefined, true] as const)( + "omits reasoning effort when the disable option is selected and enableReasoningEffort is %s", + async (enableReasoningEffort) => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: ["disable", "low", "high"], + reasoningEffort: "high", + }, + }) + await collectStream( + new NanoGptHandler({ + nanoGptModelId: "model:thinking", + enableReasoningEffort, + reasoningEffort: "disable", + }).createMessage("sys", messages), + ) + + expect(mockCreate.mock.calls[0][0]).not.toHaveProperty("reasoning_effort") + }, + ) + + it("resolves none to the canonical lowest supported effort when reasoning is enabled", async () => { + await collectStream( + new NanoGptHandler({ + nanoGptModelId: "model:thinking", + enableReasoningEffort: true, + reasoningEffort: "none", + }).createMessage("sys", messages), + ) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "low" }) + }) + + it("resolves none to the canonical lowest supported effort even when the model supports disable", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: ["disable", "low", "high"], + }, + }) + await collectStream( + new NanoGptHandler({ + nanoGptModelId: "model:thinking", + enableReasoningEffort: true, + reasoningEffort: "none", + }).createMessage("sys", messages), + ) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "low" }) + }) + + it("resolves none to the lowest canonical effort when reasoning support is boolean", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: true, + }, + }) + await collectStream( + new NanoGptHandler({ + nanoGptModelId: "model:thinking", + enableReasoningEffort: true, + reasoningEffort: "none", + }).createMessage("sys", messages), + ) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "low" }) + }) + + it("resolves none to the first supported effort when low is not available", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: ["high"], + }, + }) + await collectStream( + new NanoGptHandler({ + nanoGptModelId: "model:thinking", + enableReasoningEffort: true, + reasoningEffort: "none", + }).createMessage("sys", messages), + ) + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "high" }) + }) + + it("omits reasoning effort when reasoning is explicitly disabled", async () => { + vi.mocked(getModels).mockResolvedValue({ + "model:thinking": { + maxTokens: 128000, + contextWindow: 1050000, + supportsPromptCache: false, + supportsReasoningEffort: ["disable", "low", "high"], + reasoningEffort: "high", + }, + }) + await collectStream( + new NanoGptHandler({ + nanoGptModelId: "model:thinking", + enableReasoningEffort: false, + reasoningEffort: "high", + }).createMessage("sys", messages), + ) + + expect(mockCreate.mock.calls[0][0]).not.toHaveProperty("reasoning_effort") + }) + it("keeps Muse Spark tool-result history contiguous across turns", async () => { const modelId = "meta/muse-spark-1.2-contributor" vi.mocked(getModels).mockResolvedValue({ @@ -370,6 +616,14 @@ describe("NanoGptHandler", () => { }) describe("completePrompt", () => { + it("uses the same default reasoning effort as streaming requests", async () => { + mockCreate.mockResolvedValue({ choices: [{ message: { content: "response" } }] }) + + await new NanoGptHandler({ nanoGptModelId: "model:thinking" }).completePrompt("prompt") + + expect(mockCreate.mock.calls[0][0]).toMatchObject({ reasoning_effort: "low" }) + }) + it("requests cache-capable routing without changing the completion model ID", async () => { mockCreate.mockResolvedValue({ choices: [{ message: { content: "response" } }] }) const handler = new NanoGptHandler({ diff --git a/src/api/providers/nanogpt.ts b/src/api/providers/nanogpt.ts index 43d4641251..5fac46eb5e 100644 --- a/src/api/providers/nanogpt.ts +++ b/src/api/providers/nanogpt.ts @@ -32,15 +32,41 @@ type NanoGptCachingRequest = { caching?: true } const NANO_GPT_MERGED_TOOL_RESULT_MODELS = new Set(["meta/muse-spark-1.2-contributor"]) const NANO_GPT_ASTRA_MODEL_IDS = new Set(["openai/gpt-6-astra", "openai/gpt-6-astra-pro"]) +const NANO_GPT_REASONING_EFFORTS = ["low", "medium", "high", "xhigh", "max"] as const function getReasoningEffort(options: ApiHandlerOptions, info: ModelInfo): ReasoningEffortExtended | undefined { const configured = options.reasoningEffort + // "none" with enableReasoningEffort: true is an explicit level selection, not a disable. const reasoningDisabled = - configured === "disable" || configured === "none" || options.enableReasoningEffort === false + configured === "disable" || + (configured === "none" && options.enableReasoningEffort !== true) || + options.enableReasoningEffort === false const supported = info.supportsReasoningEffort - if (!reasoningDisabled && configured && configured !== "minimal") { - if (supported === true || (Array.isArray(supported) && supported.includes(configured))) return configured + if (reasoningDisabled && (supported === true || (Array.isArray(supported) && supported.includes("disable")))) { + return undefined + } + + // When "none" is explicitly enabled, resolve it to the lowest canonical supported effort. + const noneEnabled = !reasoningDisabled && configured === "none" + const candidates = [reasoningDisabled ? undefined : configured, info.reasoningEffort] + if (noneEnabled || (Array.isArray(supported) && !supported.includes("disable"))) { + candidates.push( + NANO_GPT_REASONING_EFFORTS.find( + (effort) => supported === true || (Array.isArray(supported) && supported.includes(effort)), + ), + ) + } + + for (const effort of candidates) { + if ( + effort && + effort !== "none" && + effort !== "minimal" && + (supported === true || (Array.isArray(supported) && supported.includes(effort))) + ) { + return effort + } } const fallback = info.reasoningEffort diff --git a/webview-ui/src/components/settings/ThinkingBudget.tsx b/webview-ui/src/components/settings/ThinkingBudget.tsx index d8ee0cd448..eb1cbbb86a 100644 --- a/webview-ui/src/components/settings/ThinkingBudget.tsx +++ b/webview-ui/src/components/settings/ThinkingBudget.tsx @@ -99,12 +99,10 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod ? ["disable", ...baseAvailableOptions] : baseAvailableOptions - // Default reasoning effort - use model's default if available - // GPT-5 models have "medium" as their default in the model configuration + // Use the model's declared default when present; otherwise fall back based on requiredReasoningEffort. const modelDefaultReasoningEffort = modelInfo?.reasoningEffort as ReasoningEffortExtended | undefined - const defaultReasoningEffort: ReasoningEffortOption = modelInfo?.requiredReasoningEffort - ? modelDefaultReasoningEffort || "medium" - : "disable" + const defaultReasoningEffort: ReasoningEffortOption = + modelDefaultReasoningEffort ?? (modelInfo?.requiredReasoningEffort ? "medium" : "disable") // Current reasoning effort from settings, or fall back to default. // Clamp to availableOptions so the Select trigger always renders a valid option. const storedReasoningEffort = apiConfiguration.reasoningEffort as ReasoningEffortOption | undefined @@ -116,23 +114,16 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod ? rawReasoningEffort : fallbackReasoningEffort - // Set default reasoning effort when model supports it and no value is set + // Keep normalized defaults pending so Save persists them to the provider profile. useEffect(() => { if ( isReasoningEffortSupported && - modelInfo?.requiredReasoningEffort && storedReasoningEffort !== currentReasoningEffort && currentReasoningEffort !== "disable" ) { - setApiConfigurationField("reasoningEffort", currentReasoningEffort as ReasoningEffortExtended, false) + setApiConfigurationField("reasoningEffort", currentReasoningEffort as ReasoningEffortExtended) } - }, [ - isReasoningEffortSupported, - storedReasoningEffort, - currentReasoningEffort, - modelInfo?.requiredReasoningEffort, - setApiConfigurationField, - ]) + }, [isReasoningEffortSupported, storedReasoningEffort, currentReasoningEffort, setApiConfigurationField]) // Sync enableReasoningEffort based on selection // "disable" turns off reasoning; "none" is a valid level (reasoning enabled) diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx index 034038e27e..07a4c3a018 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx @@ -277,6 +277,9 @@ const mockApiOptions = ({ apiConfiguration, setApiConfigurationField }: any) => {provider} ))} + ) @@ -491,6 +494,23 @@ describe("SettingsView - Change Detection Fix", () => { expect(onDone).toHaveBeenCalled() }, 10000) + it("persists a normalized reasoning default through Save", async () => { + ;(useExtensionState as any).mockReturnValue(createExtensionState()) + + renderWithExtensionState(, { queryClient }) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeDisabled()) + + fireEvent.click(screen.getByTestId("set-reasoning-default")) + expect(screen.getByTestId("save-button")).toBeEnabled() + + fireEvent.click(screen.getByTestId("save-button")) + expect(mockPostMessage).toHaveBeenCalledWith({ + type: "upsertApiConfiguration", + text: "default", + apiConfiguration: expect.objectContaining({ reasoningEffort: "high" }), + }) + }, 10000) + // These tests are passing for the basic case but failing due to vi.doMock limitations // The core fix has been verified - when no actual changes are made, no unsaved changes dialog appears diff --git a/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx b/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx index 8cb6a6fe99..b002758417 100644 --- a/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx @@ -5,7 +5,7 @@ import React from "react" import { render, screen, fireEvent } from "@/utils/test-utils" -import type { ModelInfo } from "@roo-code/types" +import type { ModelInfo, ProviderSettings } from "@roo-code/types" import { ThinkingBudget } from "../ThinkingBudget" @@ -79,6 +79,12 @@ describe("ThinkingBudget", () => { vi.clearAllMocks() }) + it("should render nothing when model information is unavailable", () => { + const { container } = render() + + expect(container.firstChild).toBeNull() + }) + it("should render nothing when model doesn't support thinking", () => { const { container } = render( { }) it("should fall back to first available option when stored value is not in the explicit array", () => { + const setApiConfigurationField = vi.fn() // Covers the clamp branch: defaultReasoningEffort="disable" but array omits "disable" render( { // The select value should be "low" (first item), not "disable" expect(screen.getByTestId("select")).toHaveAttribute("data-value", "low") + expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "low") + expect(setApiConfigurationField).toHaveBeenCalledWith("enableReasoningEffort", true, false) + }) + + it("should use and persist an optional model's advertised reasoning default", () => { + const setApiConfigurationField = vi.fn() + render( + , + ) + + expect(screen.getByTestId("select")).toHaveAttribute("data-value", "high") + expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "high") + expect(setApiConfigurationField).toHaveBeenCalledWith("enableReasoningEffort", true, false) + }) + + it("should preserve an explicit disable selection for optional reasoning", () => { + const setApiConfigurationField = vi.fn() + render( + , + ) + + expect(screen.getByTestId("select")).toHaveAttribute("data-value", "disable") + expect(setApiConfigurationField).not.toHaveBeenCalled() }) it("should normalize an invalid disabled value to the default for required reasoning", () => { @@ -307,7 +356,165 @@ describe("ThinkingBudget", () => { ) expect(screen.getByTestId("select")).toHaveAttribute("data-value", "max") - expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "max", false) + expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "max") + }) + + it("should use the first supported effort when required reasoning has no advertised default", () => { + const setApiConfigurationField = vi.fn() + render( + , + ) + + expect(screen.getByTestId("select")).toHaveAttribute("data-value", "low") + expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "low") + expect(setApiConfigurationField).toHaveBeenCalledWith("enableReasoningEffort", true, false) + }) + + it("should normalize stale disable to the first supported effort after switching to a required model", () => { + const setApiConfigurationField = vi.fn() + const { rerender } = render( + , + ) + + setApiConfigurationField.mockClear() + + rerender( + , + ) + + expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "low") + expect(setApiConfigurationField).toHaveBeenCalledWith("enableReasoningEffort", true, false) + // The two effects fire only what is needed: no third enableReasoningEffort write. + expect(setApiConfigurationField).not.toHaveBeenCalledWith("reasoningEffort", "disable") + }) + + it("should use medium when boolean reasoning support is required without an advertised default", () => { + const setApiConfigurationField = vi.fn() + render( + , + ) + + expect(screen.getByTestId("select")).toHaveAttribute("data-value", "medium") + expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "medium") + }) + + it("should synchronize a default when model reasoning metadata changes", () => { + const setApiConfigurationField = vi.fn() + const { rerender } = render( + , + ) + + setApiConfigurationField.mockClear() + rerender( + , + ) + + expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "high") + }) + + it.each<{ + name: string + apiConfiguration: ProviderSettings + modelInfo: ModelInfo + expected: string + expectedWrite?: string + }>([ + { + name: "keeps a supported stored effort over the model default", + apiConfiguration: { reasoningEffort: "low", enableReasoningEffort: true }, + modelInfo: { + ...reasoningEffortModelInfo, + supportsReasoningEffort: ["disable", "low", "high"], + reasoningEffort: "high", + }, + expected: "low", + expectedWrite: undefined, + }, + { + name: "normalizes an unsupported stored effort to the model default", + apiConfiguration: { reasoningEffort: "max", enableReasoningEffort: true }, + modelInfo: { + ...reasoningEffortModelInfo, + supportsReasoningEffort: ["disable", "low", "high"], + reasoningEffort: "high", + }, + expected: "high", + expectedWrite: "high", + }, + { + name: "defaults optional boolean reasoning support to disabled", + apiConfiguration: {}, + modelInfo: reasoningEffortModelInfo, + expected: "disable", + expectedWrite: undefined, + }, + ])("$name", ({ apiConfiguration, modelInfo, expected, expectedWrite }) => { + const setApiConfigurationField = vi.fn() + render( + , + ) + + expect(screen.getByTestId("select")).toHaveAttribute("data-value", expected) + if (expectedWrite) { + expect(setApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", expectedWrite) + } else { + expect(setApiConfigurationField).not.toHaveBeenCalledWith("reasoningEffort", expect.anything()) + expect(setApiConfigurationField).not.toHaveBeenCalledWith( + "enableReasoningEffort", + expect.anything(), + false, + ) + } }) it("should fall back to rawReasoningEffort when availableOptions is empty", () => { @@ -327,6 +534,21 @@ describe("ThinkingBudget", () => { expect(screen.getByTestId("select")).toHaveAttribute("data-value", "medium") }) + it("should retain the disabled fallback when availableOptions is empty and settings are unset", () => { + render( + , + ) + + expect(screen.getByTestId("select")).toHaveAttribute("data-value", "disable") + }) + it("should show 'disable' option when supportsReasoningEffort array explicitly includes disable", () => { render(