Browse Source

fix: respect enableReasoningEffort setting when determining reasoning usage (#7049)

Co-authored-by: Matt Rubens <[email protected]>
Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Daniel <[email protected]>
roomote[bot] 4 months ago
parent
commit
87c42c1f26
2 changed files with 41 additions and 2 deletions
  1. 30 1
      src/shared/__tests__/api.spec.ts
  2. 11 1
      src/shared/api.ts

+ 30 - 1
src/shared/__tests__/api.spec.ts

@@ -375,12 +375,41 @@ describe("shouldUseReasoningEffort", () => {
 			reasoningEffort: "medium",
 		}
 
-		// Should return true regardless of settings
+		// Should return true regardless of settings (unless explicitly disabled)
 		expect(shouldUseReasoningEffort({ model })).toBe(true)
 		expect(shouldUseReasoningEffort({ model, settings: {} })).toBe(true)
 		expect(shouldUseReasoningEffort({ model, settings: { reasoningEffort: undefined } })).toBe(true)
 	})
 
+	test("should return false when enableReasoningEffort is false, even if reasoningEffort is set", () => {
+		const model: ModelInfo = {
+			contextWindow: 200_000,
+			supportsPromptCache: true,
+			supportsReasoningEffort: true,
+		}
+
+		const settings: ProviderSettings = {
+			enableReasoningEffort: false,
+			reasoningEffort: "medium",
+		}
+
+		expect(shouldUseReasoningEffort({ model, settings })).toBe(false)
+	})
+
+	test("should return false when enableReasoningEffort is false, even if model has reasoningEffort property", () => {
+		const model: ModelInfo = {
+			contextWindow: 200_000,
+			supportsPromptCache: true,
+			reasoningEffort: "medium",
+		}
+
+		const settings: ProviderSettings = {
+			enableReasoningEffort: false,
+		}
+
+		expect(shouldUseReasoningEffort({ model, settings })).toBe(false)
+	})
+
 	test("should return true when model supports reasoning effort and settings provide reasoning effort", () => {
 		const model: ModelInfo = {
 			contextWindow: 200_000,

+ 11 - 1
src/shared/api.ts

@@ -63,7 +63,17 @@ export const shouldUseReasoningEffort = ({
 }: {
 	model: ModelInfo
 	settings?: ProviderSettings
-}): boolean => (!!model.supportsReasoningEffort && !!settings?.reasoningEffort) || !!model.reasoningEffort
+}): boolean => {
+	// If enableReasoningEffort is explicitly set to false, reasoning should be disabled
+	if (settings?.enableReasoningEffort === false) {
+		return false
+	}
+
+	// Otherwise, use reasoning if:
+	// 1. Model supports reasoning effort AND settings provide reasoning effort, OR
+	// 2. Model itself has a reasoningEffort property
+	return (!!model.supportsReasoningEffort && !!settings?.reasoningEffort) || !!model.reasoningEffort
+}
 
 export const DEFAULT_HYBRID_REASONING_MODEL_MAX_TOKENS = 16_384
 export const DEFAULT_HYBRID_REASONING_MODEL_THINKING_TOKENS = 8_192