Skip to content

Commit

Permalink
🔧 refactor: Revamp Model and Tool Filtering Logic (danny-avila#5637)
Browse files Browse the repository at this point in the history
* 🔧 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
  • Loading branch information
danny-avila authored and justinmdickey committed Feb 7, 2025
1 parent e1c5881 commit 7622061
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 14 deletions.
1 change: 1 addition & 0 deletions api/app/clients/OpenAIClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions api/server/services/ModelService.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions api/server/services/ToolService.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
25 changes: 13 additions & 12 deletions api/strategies/openidStrategy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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.
});
});
});

0 comments on commit 7622061

Please sign in to comment.