From 6c0aad423f7239d70e8d4594fd5608b7cc7a2be1 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 1 Dec 2025 12:00:54 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=90=20refactor:=20Exclude=20Params=20f?= =?UTF-8?q?rom=20OAI=20Reasoning=20Models=20(#10745)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 📐 refactor: Exclude Params from OAI Reasoning Models - Introduced a new test suite for `getOpenAILLMConfig` covering various model configurations, including basic settings, reasoning models, and web search functionality. - Validated parameter handling for different models, ensuring correct exclusions and conversions, particularly for temperature and max_tokens. - Enhanced tests for default and additional parameters, drop parameters, and verbosity handling, ensuring robust coverage of the configuration logic. * ci: Update OpenAI model version in configuration tests - Changed model references from 'gpt-5' to 'gpt-4' across multiple test cases in the `getOpenAIConfig` function. - Adjusted related parameter handling to ensure compatibility with the updated model version, including maxTokens and temperature settings. - Enhanced test coverage for model options and their expected configurations. --- .../api/src/endpoints/openai/config.spec.ts | 31 +- packages/api/src/endpoints/openai/llm.spec.ts | 602 ++++++++++++++++++ packages/api/src/endpoints/openai/llm.ts | 30 +- 3 files changed, 643 insertions(+), 20 deletions(-) create mode 100644 packages/api/src/endpoints/openai/llm.spec.ts diff --git a/packages/api/src/endpoints/openai/config.spec.ts b/packages/api/src/endpoints/openai/config.spec.ts index 8d85014e79..405873490f 100644 --- a/packages/api/src/endpoints/openai/config.spec.ts +++ b/packages/api/src/endpoints/openai/config.spec.ts @@ -26,7 +26,7 @@ describe('getOpenAIConfig', () => { it('should apply model options', () => { const modelOptions = { - model: 'gpt-5', + model: 'gpt-4', temperature: 0.7, max_tokens: 1000, }; @@ -34,14 +34,11 @@ describe('getOpenAIConfig', () => { const result = getOpenAIConfig(mockApiKey, { modelOptions }); expect(result.llmConfig).toMatchObject({ - model: 'gpt-5', + model: 'gpt-4', temperature: 0.7, - modelKwargs: { - max_completion_tokens: 1000, - }, + maxTokens: 1000, }); expect((result.llmConfig as Record).max_tokens).toBeUndefined(); - expect((result.llmConfig as Record).maxTokens).toBeUndefined(); }); it('should separate known and unknown params from addParams', () => { @@ -286,7 +283,7 @@ describe('getOpenAIConfig', () => { it('should ignore non-boolean web_search values in addParams', () => { const modelOptions = { - model: 'gpt-5', + model: 'gpt-4', web_search: true, }; @@ -399,7 +396,7 @@ describe('getOpenAIConfig', () => { it('should handle verbosity parameter in modelKwargs', () => { const modelOptions = { - model: 'gpt-5', + model: 'gpt-4', temperature: 0.7, verbosity: Verbosity.high, }; @@ -407,7 +404,7 @@ describe('getOpenAIConfig', () => { const result = getOpenAIConfig(mockApiKey, { modelOptions }); expect(result.llmConfig).toMatchObject({ - model: 'gpt-5', + model: 'gpt-4', temperature: 0.7, }); expect(result.llmConfig.modelKwargs).toEqual({ @@ -417,7 +414,7 @@ describe('getOpenAIConfig', () => { it('should allow addParams to override verbosity in modelKwargs', () => { const modelOptions = { - model: 'gpt-5', + model: 'gpt-4', verbosity: Verbosity.low, }; @@ -451,7 +448,7 @@ describe('getOpenAIConfig', () => { it('should nest verbosity under text when useResponsesApi is enabled', () => { const modelOptions = { - model: 'gpt-5', + model: 'gpt-4', temperature: 0.7, verbosity: Verbosity.low, useResponsesApi: true, @@ -460,7 +457,7 @@ describe('getOpenAIConfig', () => { const result = getOpenAIConfig(mockApiKey, { modelOptions }); expect(result.llmConfig).toMatchObject({ - model: 'gpt-5', + model: 'gpt-4', temperature: 0.7, useResponsesApi: true, }); @@ -496,7 +493,6 @@ describe('getOpenAIConfig', () => { it('should move maxTokens to modelKwargs.max_completion_tokens for GPT-5+ models', () => { const modelOptions = { model: 'gpt-5', - temperature: 0.7, max_tokens: 2048, }; @@ -504,7 +500,6 @@ describe('getOpenAIConfig', () => { expect(result.llmConfig).toMatchObject({ model: 'gpt-5', - temperature: 0.7, }); expect(result.llmConfig.maxTokens).toBeUndefined(); expect(result.llmConfig.modelKwargs).toEqual({ @@ -1684,7 +1679,7 @@ describe('getOpenAIConfig', () => { it('should not override existing modelOptions with defaultParams', () => { const result = getOpenAIConfig(mockApiKey, { modelOptions: { - model: 'gpt-5', + model: 'gpt-4', temperature: 0.9, }, customParams: { @@ -1697,7 +1692,7 @@ describe('getOpenAIConfig', () => { }); expect(result.llmConfig.temperature).toBe(0.9); - expect(result.llmConfig.modelKwargs?.max_completion_tokens).toBe(1000); + expect(result.llmConfig.maxTokens).toBe(1000); }); it('should allow addParams to override defaultParams', () => { @@ -1845,7 +1840,7 @@ describe('getOpenAIConfig', () => { it('should preserve order: defaultParams < addParams < modelOptions', () => { const result = getOpenAIConfig(mockApiKey, { modelOptions: { - model: 'gpt-5', + model: 'gpt-4', temperature: 0.9, }, customParams: { @@ -1863,7 +1858,7 @@ describe('getOpenAIConfig', () => { expect(result.llmConfig.temperature).toBe(0.9); expect(result.llmConfig.topP).toBe(0.8); - expect(result.llmConfig.modelKwargs?.max_completion_tokens).toBe(500); + expect(result.llmConfig.maxTokens).toBe(500); }); }); }); diff --git a/packages/api/src/endpoints/openai/llm.spec.ts b/packages/api/src/endpoints/openai/llm.spec.ts new file mode 100644 index 0000000000..a4eb7c78e3 --- /dev/null +++ b/packages/api/src/endpoints/openai/llm.spec.ts @@ -0,0 +1,602 @@ +import { + Verbosity, + EModelEndpoint, + ReasoningEffort, + ReasoningSummary, +} from 'librechat-data-provider'; +import { getOpenAILLMConfig, extractDefaultParams, applyDefaultParams } from './llm'; +import type * as t from '~/types'; + +describe('getOpenAILLMConfig', () => { + describe('Basic Configuration', () => { + it('should create a basic configuration with required fields', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + }, + }); + + expect(result.llmConfig).toHaveProperty('apiKey', 'test-api-key'); + expect(result.llmConfig).toHaveProperty('model', 'gpt-4'); + expect(result.llmConfig).toHaveProperty('streaming', true); + expect(result.tools).toEqual([]); + }); + + it('should handle model options including temperature and penalties', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + temperature: 0.7, + frequency_penalty: 0.5, + presence_penalty: 0.3, + }, + }); + + expect(result.llmConfig).toHaveProperty('temperature', 0.7); + expect(result.llmConfig).toHaveProperty('frequencyPenalty', 0.5); + expect(result.llmConfig).toHaveProperty('presencePenalty', 0.3); + }); + + it('should handle max_tokens conversion to maxTokens', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + max_tokens: 4096, + }, + }); + + expect(result.llmConfig).toHaveProperty('maxTokens', 4096); + expect(result.llmConfig).not.toHaveProperty('max_tokens'); + }); + }); + + describe('OpenAI Reasoning Models (o1/o3/gpt-5)', () => { + const reasoningModels = [ + 'o1', + 'o1-mini', + 'o1-preview', + 'o1-pro', + 'o3', + 'o3-mini', + 'gpt-5', + 'gpt-5-pro', + 'gpt-5-turbo', + ]; + + const excludedParams = [ + 'frequencyPenalty', + 'presencePenalty', + 'temperature', + 'topP', + 'logitBias', + 'n', + 'logprobs', + ]; + + it.each(reasoningModels)( + 'should exclude unsupported parameters for reasoning model: %s', + (model) => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model, + temperature: 0.7, + frequency_penalty: 0.5, + presence_penalty: 0.3, + topP: 0.9, + logitBias: { '50256': -100 }, + n: 2, + logprobs: true, + } as Partial, + }); + + excludedParams.forEach((param) => { + expect(result.llmConfig).not.toHaveProperty(param); + }); + + expect(result.llmConfig).toHaveProperty('model', model); + expect(result.llmConfig).toHaveProperty('streaming', true); + }, + ); + + it('should preserve maxTokens for reasoning models', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'o1', + max_tokens: 4096, + temperature: 0.7, + }, + }); + + expect(result.llmConfig).toHaveProperty('maxTokens', 4096); + expect(result.llmConfig).not.toHaveProperty('temperature'); + }); + + it('should preserve other valid parameters for reasoning models', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'o1', + max_tokens: 8192, + stop: ['END'], + }, + }); + + expect(result.llmConfig).toHaveProperty('maxTokens', 8192); + expect(result.llmConfig).toHaveProperty('stop', ['END']); + }); + + it('should handle GPT-5 max_tokens conversion to max_completion_tokens', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-5', + max_tokens: 8192, + stop: ['END'], + }, + }); + + expect(result.llmConfig.modelKwargs).toHaveProperty('max_completion_tokens', 8192); + expect(result.llmConfig).not.toHaveProperty('maxTokens'); + expect(result.llmConfig).toHaveProperty('stop', ['END']); + }); + + it('should combine user dropParams with reasoning exclusion params', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'o3-mini', + temperature: 0.7, + stop: ['END'], + }, + dropParams: ['stop'], + }); + + expect(result.llmConfig).not.toHaveProperty('temperature'); + expect(result.llmConfig).not.toHaveProperty('stop'); + }); + + it('should NOT exclude parameters for non-reasoning models', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4-turbo', + temperature: 0.7, + frequency_penalty: 0.5, + presence_penalty: 0.3, + topP: 0.9, + }, + }); + + expect(result.llmConfig).toHaveProperty('temperature', 0.7); + expect(result.llmConfig).toHaveProperty('frequencyPenalty', 0.5); + expect(result.llmConfig).toHaveProperty('presencePenalty', 0.3); + expect(result.llmConfig).toHaveProperty('topP', 0.9); + }); + + it('should NOT exclude parameters for gpt-5.x versioned models (they support sampling params)', () => { + const versionedModels = ['gpt-5.1', 'gpt-5.1-turbo', 'gpt-5.2', 'gpt-5.5-preview']; + + versionedModels.forEach((model) => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model, + temperature: 0.7, + frequency_penalty: 0.5, + presence_penalty: 0.3, + topP: 0.9, + }, + }); + + expect(result.llmConfig).toHaveProperty('temperature', 0.7); + expect(result.llmConfig).toHaveProperty('frequencyPenalty', 0.5); + expect(result.llmConfig).toHaveProperty('presencePenalty', 0.3); + expect(result.llmConfig).toHaveProperty('topP', 0.9); + }); + }); + + it('should NOT exclude parameters for gpt-5-chat (it supports sampling params)', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-5-chat', + temperature: 0.7, + frequency_penalty: 0.5, + presence_penalty: 0.3, + topP: 0.9, + }, + }); + + expect(result.llmConfig).toHaveProperty('temperature', 0.7); + expect(result.llmConfig).toHaveProperty('frequencyPenalty', 0.5); + expect(result.llmConfig).toHaveProperty('presencePenalty', 0.3); + expect(result.llmConfig).toHaveProperty('topP', 0.9); + }); + + it('should handle reasoning models with reasoning_effort parameter', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + endpoint: EModelEndpoint.openAI, + modelOptions: { + model: 'o1', + reasoning_effort: ReasoningEffort.high, + temperature: 0.7, + }, + }); + + expect(result.llmConfig).toHaveProperty('reasoning_effort', ReasoningEffort.high); + expect(result.llmConfig).not.toHaveProperty('temperature'); + }); + }); + + describe('OpenAI Web Search Models', () => { + it('should exclude parameters for gpt-4o search models', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4o-search-preview', + temperature: 0.7, + top_p: 0.9, + seed: 42, + } as Partial, + }); + + expect(result.llmConfig).not.toHaveProperty('temperature'); + expect(result.llmConfig).not.toHaveProperty('top_p'); + expect(result.llmConfig).not.toHaveProperty('seed'); + }); + + it('should preserve max_tokens for search models', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4o-search', + max_tokens: 4096, + temperature: 0.7, + }, + }); + + expect(result.llmConfig).toHaveProperty('maxTokens', 4096); + expect(result.llmConfig).not.toHaveProperty('temperature'); + }); + }); + + describe('Web Search Functionality', () => { + it('should enable web search with Responses API', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + web_search: true, + }, + }); + + expect(result.llmConfig).toHaveProperty('useResponsesApi', true); + expect(result.tools).toContainEqual({ type: 'web_search' }); + }); + + it('should handle web search with OpenRouter', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + useOpenRouter: true, + modelOptions: { + model: 'gpt-4', + web_search: true, + }, + }); + + expect(result.llmConfig.modelKwargs).toHaveProperty('plugins', [{ id: 'web' }]); + expect(result.llmConfig).toHaveProperty('include_reasoning', true); + }); + + it('should disable web search via dropParams', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + web_search: true, + }, + dropParams: ['web_search'], + }); + + expect(result.tools).not.toContainEqual({ type: 'web_search' }); + }); + }); + + describe('GPT-5 max_tokens Handling', () => { + it('should convert maxTokens to max_completion_tokens for GPT-5 models', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-5', + max_tokens: 8192, + }, + }); + + expect(result.llmConfig.modelKwargs).toHaveProperty('max_completion_tokens', 8192); + expect(result.llmConfig).not.toHaveProperty('maxTokens'); + }); + + it('should convert maxTokens to max_output_tokens for GPT-5 with Responses API', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-5', + max_tokens: 8192, + }, + addParams: { + useResponsesApi: true, + }, + }); + + expect(result.llmConfig.modelKwargs).toHaveProperty('max_output_tokens', 8192); + expect(result.llmConfig).not.toHaveProperty('maxTokens'); + }); + }); + + describe('Reasoning Parameters', () => { + it('should handle reasoning_effort for OpenAI endpoint', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + endpoint: EModelEndpoint.openAI, + modelOptions: { + model: 'o1', + reasoning_effort: ReasoningEffort.high, + }, + }); + + expect(result.llmConfig).toHaveProperty('reasoning_effort', ReasoningEffort.high); + }); + + it('should use reasoning object for non-OpenAI endpoints', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + endpoint: 'custom', + modelOptions: { + model: 'o1', + reasoning_effort: ReasoningEffort.high, + reasoning_summary: ReasoningSummary.concise, + }, + }); + + expect(result.llmConfig).toHaveProperty('reasoning'); + expect(result.llmConfig.reasoning).toEqual({ + effort: ReasoningEffort.high, + summary: ReasoningSummary.concise, + }); + }); + + it('should use reasoning object when useResponsesApi is true', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + endpoint: EModelEndpoint.openAI, + modelOptions: { + model: 'o1', + reasoning_effort: ReasoningEffort.medium, + reasoning_summary: ReasoningSummary.detailed, + }, + addParams: { + useResponsesApi: true, + }, + }); + + expect(result.llmConfig).toHaveProperty('reasoning'); + expect(result.llmConfig.reasoning).toEqual({ + effort: ReasoningEffort.medium, + summary: ReasoningSummary.detailed, + }); + }); + }); + + describe('Default and Add Parameters', () => { + it('should apply default parameters when fields are undefined', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + }, + defaultParams: { + temperature: 0.5, + topP: 0.9, + }, + }); + + expect(result.llmConfig).toHaveProperty('temperature', 0.5); + expect(result.llmConfig).toHaveProperty('topP', 0.9); + }); + + it('should NOT override existing values with default parameters', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + temperature: 0.8, + }, + defaultParams: { + temperature: 0.5, + }, + }); + + expect(result.llmConfig).toHaveProperty('temperature', 0.8); + }); + + it('should apply addParams and override defaults', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + }, + defaultParams: { + temperature: 0.5, + }, + addParams: { + temperature: 0.9, + seed: 42, + }, + }); + + expect(result.llmConfig).toHaveProperty('temperature', 0.9); + expect(result.llmConfig).toHaveProperty('seed', 42); + }); + + it('should handle unknown params via modelKwargs', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + }, + addParams: { + custom_param: 'custom_value', + }, + }); + + expect(result.llmConfig.modelKwargs).toHaveProperty('custom_param', 'custom_value'); + }); + }); + + describe('Drop Parameters', () => { + it('should drop specified parameters', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + temperature: 0.7, + topP: 0.9, + }, + dropParams: ['temperature'], + }); + + expect(result.llmConfig).not.toHaveProperty('temperature'); + expect(result.llmConfig).toHaveProperty('topP', 0.9); + }); + }); + + describe('OpenRouter Configuration', () => { + it('should include include_reasoning for OpenRouter', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + useOpenRouter: true, + modelOptions: { + model: 'gpt-4', + }, + }); + + expect(result.llmConfig).toHaveProperty('include_reasoning', true); + }); + }); + + describe('Verbosity Handling', () => { + it('should add verbosity to modelKwargs', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + verbosity: Verbosity.high, + }, + }); + + expect(result.llmConfig.modelKwargs).toHaveProperty('verbosity', Verbosity.high); + }); + + it('should convert verbosity to text object with Responses API', () => { + const result = getOpenAILLMConfig({ + apiKey: 'test-api-key', + streaming: true, + modelOptions: { + model: 'gpt-4', + verbosity: Verbosity.low, + }, + addParams: { + useResponsesApi: true, + }, + }); + + expect(result.llmConfig.modelKwargs).toHaveProperty('text', { verbosity: Verbosity.low }); + expect(result.llmConfig.modelKwargs).not.toHaveProperty('verbosity'); + }); + }); +}); + +describe('extractDefaultParams', () => { + it('should extract default values from param definitions', () => { + const paramDefinitions = [ + { key: 'temperature', default: 0.7 }, + { key: 'maxTokens', default: 4096 }, + { key: 'noDefault' }, + ]; + + const result = extractDefaultParams(paramDefinitions); + + expect(result).toEqual({ + temperature: 0.7, + maxTokens: 4096, + }); + }); + + it('should return undefined for undefined or non-array input', () => { + expect(extractDefaultParams(undefined)).toBeUndefined(); + expect(extractDefaultParams(null as unknown as undefined)).toBeUndefined(); + }); + + it('should handle empty array', () => { + const result = extractDefaultParams([]); + expect(result).toEqual({}); + }); +}); + +describe('applyDefaultParams', () => { + it('should apply defaults only when field is undefined', () => { + const target: Record = { + temperature: 0.8, + maxTokens: undefined, + }; + + const defaults = { + temperature: 0.5, + maxTokens: 4096, + topP: 0.9, + }; + + applyDefaultParams(target, defaults); + + expect(target).toEqual({ + temperature: 0.8, + maxTokens: 4096, + topP: 0.9, + }); + }); +}); diff --git a/packages/api/src/endpoints/openai/llm.ts b/packages/api/src/endpoints/openai/llm.ts index e10bf1d556..c88122310a 100644 --- a/packages/api/src/endpoints/openai/llm.ts +++ b/packages/api/src/endpoints/openai/llm.ts @@ -259,9 +259,35 @@ export function getOpenAILLMConfig({ } /** - * Note: OpenAI Web Search models do not support any known parameters besides `max_tokens` + * Note: OpenAI reasoning models (o1/o3/gpt-5) do not support temperature and other sampling parameters + * Exception: gpt-5-chat and versioned models like gpt-5.1 DO support these parameters */ - if (modelOptions.model && /gpt-4o.*search/.test(modelOptions.model as string)) { + if ( + modelOptions.model && + /\b(o[13]|gpt-5)(?!\.|-chat)(?:-|$)/.test(modelOptions.model as string) + ) { + const reasoningExcludeParams = [ + 'frequencyPenalty', + 'presencePenalty', + 'temperature', + 'topP', + 'logitBias', + 'n', + 'logprobs', + ]; + + const updatedDropParams = dropParams || []; + const combinedDropParams = [...new Set([...updatedDropParams, ...reasoningExcludeParams])]; + + combinedDropParams.forEach((param) => { + if (param in llmConfig) { + delete llmConfig[param as keyof t.OAIClientOptions]; + } + }); + } 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` + */ const searchExcludeParams = [ 'frequency_penalty', 'presence_penalty',