Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix o1 #5850

Merged
merged 2 commits into from
Nov 25, 2024
+8 −6
Merged

Fix o1 #5850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23350c8
fix o1 in disableGPT4
code-october Nov 21, 2024
ef24d3e
use stream when request o1
code-october Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/api/openai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function getModels(remoteModelRes: OpenAIListModelResponse) {
if (config.disableGPT4) {
remoteModelRes.data = remoteModelRes.data.filter(
(m) =>
!(m.id.startsWith("gpt-4") || m.id.startsWith("chatgpt-4o")) ||
!(m.id.startsWith("gpt-4") || m.id.startsWith("chatgpt-4o") || m.id.startsWith("o1")) ||
m.id.startsWith("gpt-4o-mini"),
Comment on lines +17 to 18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

The exception logic for mini variants is inconsistent and needs to be updated

Based on the codebase analysis:

  • The o1-mini model exists in the constants and is treated similarly to other GPT-4 class models
  • In app/api/openai.ts, only gpt-4o-mini variants are allowed to bypass the filter
  • In app/config/server.ts, there's a similar pattern where only gpt-4o-mini is excluded from restrictions

The filter should be updated to also exclude o1-mini variants since they are part of the same model class and are defined with the same knowledge cutoff date (2023-10) in the constants.

🔗 Analysis chain

Verify the exception logic for mini variants.

The current implementation allows "gpt-4o-mini" models to bypass the filter. Please verify if this exception should also apply to any mini variants of o1 models.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for any usage or documentation of mini variants of o1 models
# Expected: Find references to confirm if mini variants of o1 should be allowed

# Search for o1 model references in the codebase
rg -i "o1.*mini" 

# Search for model configuration or documentation files
fd -e md -e json -e yaml -e yml | xargs rg -i "o1.*mini"

Length of output: 146

Script:

#!/bin/bash
# Let's get more context about the model filtering logic and related constants

# Search for the complete model filtering logic
rg -A 5 -B 5 "gpt-4o-mini|o1-mini"

# Check the constant.ts file content since we found o1-mini references there
cat app/constant.ts

# Look for any other model-related configurations or filters
ast-grep --pattern 'startsWith("o1") || startsWith("gpt")'

Length of output: 17804

);
}
Expand Down
4 changes: 2 additions & 2 deletions app/client/platforms/openai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export class ChatGPTApi implements LLMApi {
// O1 not support image, tools (plugin in ChatGPTNextWeb) and system, stream, logprobs, temperature, top_p, n, presence_penalty, frequency_penalty yet.
requestPayload = {
messages,
stream: !isO1 ? options.config.stream : false,
stream: options.config.stream,
model: modelConfig.model,
temperature: !isO1 ? modelConfig.temperature : 1,
presence_penalty: !isO1 ? modelConfig.presence_penalty : 0,
Expand All @@ -247,7 +247,7 @@ export class ChatGPTApi implements LLMApi {

console.log("[Request] openai payload: ", requestPayload);

const shouldStream = !isDalle3 && !!options.config.stream && !isO1;
const shouldStream = !isDalle3 && !!options.config.stream;
const controller = new AbortController();
options.onController?.(controller);

Expand Down
3 changes: 2 additions & 1 deletion app/components/emoji.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ export function Avatar(props: { model?: ModelType; avatar?: string }) {
return (
<div className="no-dark">
{props.model?.startsWith("gpt-4") ||
props.model?.startsWith("chatgpt-4o") ? (
props.model?.startsWith("chatgpt-4o") ||
props.model?.startsWith("o1") ? (
<BlackBotIcon className="user-avatar" />
) : (
<BotIcon className="user-avatar" />
Expand Down
5 changes: 3 additions & 2 deletions app/config/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,15 @@ export const getServerSideConfig = () => {
if (customModels) customModels += ",";
customModels += DEFAULT_MODELS.filter(
(m) =>
(m.name.startsWith("gpt-4") || m.name.startsWith("chatgpt-4o")) &&
(m.name.startsWith("gpt-4") || m.name.startsWith("chatgpt-4o") || m.name.startsWith("o1")) &&
!m.name.startsWith("gpt-4o-mini"),
)
.map((m) => "-" + m.name)
.join(",");
if (
(defaultModel.startsWith("gpt-4") ||
defaultModel.startsWith("chatgpt-4o")) &&
defaultModel.startsWith("chatgpt-4o") ||
defaultModel.startsWith("o1")) &&
!defaultModel.startsWith("gpt-4o-mini")
)
defaultModel = "";
Expand Down
Loading