From c1b3125b20ccc2ede7f2d3a9761fecd8e4f04c93 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 1 Jun 2026 09:14:06 -0400 Subject: [PATCH] fix: Address custom reasoning review cases --- .../services/Config/loadCustomConfig.js | 3 +- .../services/Config/loadCustomConfig.spec.js | 18 +++-- .../openai/config.backward-compat.spec.ts | 4 +- .../api/src/endpoints/openai/config.spec.ts | 21 ++++-- packages/api/src/endpoints/openai/llm.spec.ts | 38 ++++++++++ packages/api/src/endpoints/openai/llm.ts | 71 +++++++++++-------- 6 files changed, 109 insertions(+), 46 deletions(-) diff --git a/api/server/services/Config/loadCustomConfig.js b/api/server/services/Config/loadCustomConfig.js index c914754974..c719a84665 100644 --- a/api/server/services/Config/loadCustomConfig.js +++ b/api/server/services/Config/loadCustomConfig.js @@ -177,7 +177,8 @@ https://www.librechat.ai/docs/configuration/stt_tts`); // Validate and fill out missing values for custom parameters function parseCustomParams(endpointName, customParams) { - const paramEndpoint = customParams.defaultParamsEndpoint; + const paramEndpoint = customParams.defaultParamsEndpoint ?? 'custom'; + customParams.defaultParamsEndpoint = paramEndpoint; customParams.paramDefinitions = customParams.paramDefinitions || []; // Checks if `defaultParamsEndpoint` is a key in `paramSettings`. diff --git a/api/server/services/Config/loadCustomConfig.spec.js b/api/server/services/Config/loadCustomConfig.spec.js index 3fce8777e3..aeffa52476 100644 --- a/api/server/services/Config/loadCustomConfig.spec.js +++ b/api/server/services/Config/loadCustomConfig.spec.js @@ -11,7 +11,7 @@ jest.mock('librechat-data-provider', () => { paramSettings: { foo: {}, bar: {}, - custom: {}, + custom: [], openrouter: [ { key: 'promptCache', @@ -59,6 +59,7 @@ jest.mock('@librechat/data-schemas', () => { const axios = require('axios'); const { loadYaml } = require('@librechat/api'); const { logger } = require('@librechat/data-schemas'); +const { ReasoningParameterFormat } = require('librechat-data-provider'); const loadCustomConfig = require('./loadCustomConfig'); describe('loadCustomConfig', () => { @@ -307,11 +308,16 @@ describe('loadCustomConfig', () => { ); }); - it('throws an error when defaultParamsEndpoint is not provided', async () => { - const malformedCustomParams = { defaultParamsEndpoint: undefined }; - await expect(loadCustomParams(malformedCustomParams)).rejects.toThrow( - 'defaultParamsEndpoint of "Google" endpoint is invalid. Valid options are foo, bar, custom, openrouter, google', - ); + it('defaults defaultParamsEndpoint when only reasoningFormat is provided', async () => { + const parsedConfig = await loadCustomParams({ + reasoningFormat: ReasoningParameterFormat.reasoningObject, + }); + + expect(parsedConfig.endpoints.custom[0].customParams).toEqual({ + defaultParamsEndpoint: 'custom', + reasoningFormat: ReasoningParameterFormat.reasoningObject, + paramDefinitions: [], + }); }); it('fills the paramDefinitions with missing values', async () => { diff --git a/packages/api/src/endpoints/openai/config.backward-compat.spec.ts b/packages/api/src/endpoints/openai/config.backward-compat.spec.ts index bd9ace78a6..acc3496b2b 100644 --- a/packages/api/src/endpoints/openai/config.backward-compat.spec.ts +++ b/packages/api/src/endpoints/openai/config.backward-compat.spec.ts @@ -10,7 +10,7 @@ describe('getOpenAIConfig - Backward Compatibility', () => { describe('OpenAI endpoint', () => { it('should handle GPT-5 model with reasoning and web search', () => { const apiKey = 'sk-proj-somekey'; - const endpoint = undefined; + const endpoint = EModelEndpoint.openAI; const options = { modelOptions: { model: 'gpt-5-nano', @@ -138,7 +138,7 @@ describe('getOpenAIConfig - Backward Compatibility', () => { it('should handle Azure OpenAI with Responses API and reasoning', () => { const apiKey = 'some_azure_key'; - const endpoint = undefined; + const endpoint = EModelEndpoint.azureOpenAI; const options = { modelOptions: { model: 'gpt-5', diff --git a/packages/api/src/endpoints/openai/config.spec.ts b/packages/api/src/endpoints/openai/config.spec.ts index e97feedebf..9817b2d82a 100644 --- a/packages/api/src/endpoints/openai/config.spec.ts +++ b/packages/api/src/endpoints/openai/config.spec.ts @@ -80,7 +80,7 @@ describe('getOpenAIConfig', () => { expect(result.llmConfig.modelKwargs).toBeUndefined(); }); - it('should handle reasoning params for `useResponsesApi`', () => { + it('should pass custom endpoint reasoning object through modelKwargs for `useResponsesApi`', () => { const modelOptions = { reasoning_effort: ReasoningEffort.high, reasoning_summary: ReasoningSummary.detailed, @@ -90,9 +90,12 @@ describe('getOpenAIConfig', () => { modelOptions: { ...modelOptions, useResponsesApi: true }, }); - expect(result.llmConfig.reasoning).toEqual({ - effort: ReasoningEffort.high, - summary: ReasoningSummary.detailed, + expect(result.llmConfig.reasoning).toBeUndefined(); + expect(result.llmConfig.modelKwargs).toEqual({ + reasoning: { + effort: ReasoningEffort.high, + summary: ReasoningSummary.detailed, + }, }); expect((result.llmConfig as Record).reasoning_effort).toBeUndefined(); expect((result.llmConfig as Record).reasoning_summary).toBeUndefined(); @@ -1072,11 +1075,12 @@ describe('getOpenAIConfig', () => { const result = getOpenAIConfig(mockApiKey, { modelOptions: { ...modelOptions, useResponsesApi: true } as Partial, }); + const reasoning = result.llmConfig?.reasoning ?? result.llmConfig?.modelKwargs?.reasoning; if (shouldHaveReasoning) { - expect(result.llmConfig?.reasoning).toBeDefined(); + expect(reasoning).toBeDefined(); } else { - expect(result.llmConfig?.reasoning).toBeUndefined(); + expect(reasoning).toBeUndefined(); } }); }); @@ -1154,6 +1158,7 @@ describe('getOpenAIConfig', () => { frequency_penalty: 0.5, presence_penalty: 0.6, max_tokens: 1000, + reasoning_effort: ReasoningEffort.high, custom_param: 'should-remain', }; @@ -1168,6 +1173,7 @@ describe('getOpenAIConfig', () => { /** `presence_penalty` is converted to `presencePenalty` */ expect(result.llmConfig.maxTokens).toBe(1000); // max_tokens is allowed expect((result.llmConfig as Record).custom_param).toBe('should-remain'); + expect(result.llmConfig.modelKwargs).toBeUndefined(); }); }); @@ -1275,10 +1281,11 @@ describe('getOpenAIConfig', () => { streaming: false, useResponsesApi: true, // From web_search }); + expect(result.llmConfig.reasoning).toBeUndefined(); expect(result.llmConfig.maxTokens).toBe(2000); expect(result.llmConfig.modelKwargs).toEqual({ text: { verbosity: Verbosity.medium }, - reasoning_effort: ReasoningEffort.high, + reasoning: { effort: ReasoningEffort.high }, customParam: 'custom-value', }); expect(result.tools).toEqual([{ type: 'web_search' }]); diff --git a/packages/api/src/endpoints/openai/llm.spec.ts b/packages/api/src/endpoints/openai/llm.spec.ts index df63759aec..2d52454e87 100644 --- a/packages/api/src/endpoints/openai/llm.spec.ts +++ b/packages/api/src/endpoints/openai/llm.spec.ts @@ -518,6 +518,44 @@ describe('getOpenAILLMConfig', () => { expect(result.llmConfig.modelKwargs).toBeUndefined(); }); + it('should use Responses API reasoning when web_search enables Responses API', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + endpoint: 'custom', + modelOptions: { + model: 'provider/reasoning-model', + reasoning_effort: ReasoningEffort.high, + reasoning_summary: ReasoningSummary.concise, + web_search: true, + }, + }); + + expect(result.llmConfig).toHaveProperty('useResponsesApi', true); + expect(result.llmConfig).not.toHaveProperty('reasoning'); + expect(result.llmConfig.modelKwargs).toHaveProperty('reasoning', { + effort: ReasoningEffort.high, + summary: ReasoningSummary.concise, + }); + expect(result.tools).toContainEqual({ type: 'web_search' }); + }); + + it('should remove reasoning kwargs for GPT-4o search models', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + endpoint: 'custom', + modelOptions: { + model: 'gpt-4o-search', + reasoning_effort: ReasoningEffort.high, + }, + }); + + expect(result.llmConfig).not.toHaveProperty('reasoning'); + expect(result.llmConfig).not.toHaveProperty('reasoning_effort'); + expect(result.llmConfig.modelKwargs).toBeUndefined(); + }); + it('should use reasoning object when useResponsesApi is true', () => { const result = getOpenAILLMConfig({ apiKey: 'test-api-key', diff --git a/packages/api/src/endpoints/openai/llm.ts b/packages/api/src/endpoints/openai/llm.ts index d366e84588..ea74274c3b 100644 --- a/packages/api/src/endpoints/openai/llm.ts +++ b/packages/api/src/endpoints/openai/llm.ts @@ -107,6 +107,23 @@ function isOpenAIEndpoint(endpoint?: EModelEndpoint | string | null): boolean { return endpoint === EModelEndpoint.openAI || endpoint === EModelEndpoint.azureOpenAI; } +function deleteConfigParam({ + param, + llmConfig, + modelKwargs, +}: { + param: string; + llmConfig: OpenAILLMConfig; + modelKwargs: Record; +}) { + if (param in llmConfig) { + delete llmConfig[param as keyof t.OAIClientOptions]; + } + if (param in modelKwargs) { + delete modelKwargs[param]; + } +} + const openRouterAnthropicVerbosityByEffort: Record< string, NonNullable @@ -250,20 +267,24 @@ function applyReasoningConfig({ } const reasoning = getReasoningObject({ reasoningEffort, reasoningSummary }); - if (llmConfig.useResponsesApi === true) { - llmConfig.reasoning = reasoning; + if (reasoningFormat === ReasoningParameterFormat.disabled) { return false; } if (isOpenAIEndpoint(endpoint)) { + if (llmConfig.useResponsesApi === true) { + llmConfig.reasoning = reasoning; + return false; + } if (reasoningEffort) { llmConfig.reasoning_effort = reasoningEffort; } return false; } - if (reasoningFormat === ReasoningParameterFormat.disabled) { - return false; + if (llmConfig.useResponsesApi === true) { + modelKwargs.reasoning = reasoning; + return true; } if (reasoningFormat === ReasoningParameterFormat.reasoningObject) { @@ -519,16 +540,6 @@ export function getOpenAILLMConfig({ modelKwargs, llmConfig, }) || hasModelKwargs; - } else { - hasModelKwargs = - applyReasoningConfig({ - endpoint, - llmConfig, - modelKwargs, - reasoningFormat, - reasoningEffort: reasoning_effort, - reasoningSummary: reasoning_summary, - }) || hasModelKwargs; } if (llmConfig.max_tokens != null) { @@ -559,6 +570,18 @@ export function getOpenAILLMConfig({ llmConfig.promptCache = true; } + if (!useOpenRouter) { + hasModelKwargs = + applyReasoningConfig({ + endpoint, + llmConfig, + modelKwargs, + reasoningFormat, + reasoningEffort: reasoning_effort, + reasoningSummary: reasoning_summary, + }) || hasModelKwargs; + } + /** DeepSeek thinking-mode requires `reasoning_content` replay on tool turns (#13366). */ if ( typeof modelOptions.model === 'string' && @@ -588,11 +611,7 @@ export function getOpenAILLMConfig({ const updatedDropParams = dropParams || []; const combinedDropParams = [...new Set([...updatedDropParams, ...reasoningExcludeParams])]; - combinedDropParams.forEach((param) => { - if (param in llmConfig) { - delete llmConfig[param as keyof t.OAIClientOptions]; - } - }); + combinedDropParams.forEach((param) => deleteConfigParam({ param, llmConfig, modelKwargs })); } else if (modelOptions.model && /gpt-4o.*search/.test(modelOptions.model as string)) { /** * Note: OpenAI Web Search models do not support any known parameters besides `max_tokens` @@ -617,17 +636,9 @@ export function getOpenAILLMConfig({ const updatedDropParams = dropParams || []; const combinedDropParams = [...new Set([...updatedDropParams, ...searchExcludeParams])]; - combinedDropParams.forEach((param) => { - if (param in llmConfig) { - delete llmConfig[param as keyof t.OAIClientOptions]; - } - }); + combinedDropParams.forEach((param) => deleteConfigParam({ param, llmConfig, modelKwargs })); } else if (dropParams && Array.isArray(dropParams)) { - dropParams.forEach((param) => { - if (param in llmConfig) { - delete llmConfig[param as keyof t.OAIClientOptions]; - } - }); + dropParams.forEach((param) => deleteConfigParam({ param, llmConfig, modelKwargs })); } hasModelKwargs = @@ -649,7 +660,7 @@ export function getOpenAILLMConfig({ hasModelKwargs = true; } - if (hasModelKwargs) { + if (hasModelKwargs && Object.keys(modelKwargs).length > 0) { llmConfig.modelKwargs = modelKwargs; }