From 0312d4f4f479f6ffde65c6d617d380ed55d8fec5 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 3 Feb 2025 16:08:34 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7=20refactor:=20Revamp=20Model=20and?= =?UTF-8?q?=20Tool=20Filtering=20Logic=20(#5637)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔧 fix: Update regex to correctly match OpenAI model identifiers * 🔧 fix: Enhance tool filtering logic in ToolService to handle inclusion and exclusion criteria for basic tools and toolkits * feat: support o3-mini Azure streaming * chore: Update model filtering logic to exclude audio and realtime models * ci: linting error --- api/app/clients/OpenAIClient.js | 1 + api/server/services/ModelService.js | 5 +++-- api/server/services/ToolService.js | 11 +++++++++++ api/strategies/openidStrategy.spec.js | 25 +++++++++++++------------ 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/api/app/clients/OpenAIClient.js b/api/app/clients/OpenAIClient.js index 93fbe3c5b..9334f1c28 100644 --- a/api/app/clients/OpenAIClient.js +++ b/api/app/clients/OpenAIClient.js @@ -1282,6 +1282,7 @@ ${convo} if ( this.isOmni === true && (this.azure || /o1(?!-(?:mini|preview)).*$/.test(modelOptions.model)) && + !/o3-.*$/.test(this.modelOptions.model) && modelOptions.stream ) { delete modelOptions.stream; diff --git a/api/server/services/ModelService.js b/api/server/services/ModelService.js index 0547d0318..1394a5d69 100644 --- a/api/server/services/ModelService.js +++ b/api/server/services/ModelService.js @@ -159,8 +159,9 @@ const fetchOpenAIModels = async (opts, _models = []) => { } if (baseURL === openaiBaseURL) { - const regex = /(text-davinci-003|gpt-|o1-)/; - models = models.filter((model) => regex.test(model)); + const regex = /(text-davinci-003|gpt-|o\d+-)/; + const excludeRegex = /audio|realtime/; + models = models.filter((model) => regex.test(model) && !excludeRegex.test(model)); const instructModels = models.filter((model) => model.includes('instruct')); const otherModels = models.filter((model) => !model.includes('instruct')); models = otherModels.concat(instructModels); diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 08891c99b..cf88c0b19 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -101,6 +101,17 @@ function loadAndFormatTools({ directory, adminFilter = [], adminIncluded = [] }) const basicToolInstances = [new Calculator(), ...createYouTubeTools({ override: true })]; for (const toolInstance of basicToolInstances) { const formattedTool = formatToOpenAIAssistantTool(toolInstance); + let toolName = formattedTool[Tools.function].name; + toolName = toolkits.some((toolkit) => toolName.startsWith(toolkit.pluginKey)) + ? toolName.split('_')[0] + : toolName; + if (filter.has(toolName) && included.size === 0) { + continue; + } + + if (included.size > 0 && !included.has(toolName)) { + continue; + } tools.push(formattedTool); } diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index 7b8a3107a..cea7c5e4a 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -245,15 +245,18 @@ describe('setupOpenId', () => { const userinfo = { ...baseUserinfo }; // Act - const { user } = await validate(tokenset, userinfo); + await validate(tokenset, userinfo); // Assert – updateUser should be called and the user object updated - expect(updateUser).toHaveBeenCalledWith(existingUser._id, expect.objectContaining({ - provider: 'openid', - openidId: baseUserinfo.sub, - username: baseUserinfo.username, - name: `${baseUserinfo.given_name} ${baseUserinfo.family_name}`, - })); + expect(updateUser).toHaveBeenCalledWith( + existingUser._id, + expect.objectContaining({ + provider: 'openid', + openidId: baseUserinfo.sub, + username: baseUserinfo.username, + name: `${baseUserinfo.given_name} ${baseUserinfo.family_name}`, + }), + ); }); it('should enforce the required role and reject login if missing', async () => { @@ -268,9 +271,7 @@ describe('setupOpenId', () => { // Assert – verify that the strategy rejects login expect(user).toBe(false); - expect(details.message).toBe( - 'You must have the "requiredRole" role to log in.', - ); + expect(details.message).toBe('You must have the "requiredRole" role to log in.'); }); it('should attempt to download and save the avatar if picture is provided', async () => { @@ -292,10 +293,10 @@ describe('setupOpenId', () => { delete userinfo.picture; // Act - const { user } = await validate(tokenset, userinfo); + await validate(tokenset, userinfo); // Assert – fetch should not be called and avatar should remain undefined or empty expect(fetch).not.toHaveBeenCalled(); // Depending on your implementation, user.avatar may be undefined or an empty string. }); -}); \ No newline at end of file +});