Add latest models and remove legacy ones#6421
Add latest models and remove legacy ones#6421Davidlasky wants to merge 26 commits intoChatGPTNextWeb:mainfrom
Conversation
|
@Davidlasky is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces several modifications across the project. A new ignore entry for a local Dockerfile is added to the VCS configuration. In the client platform files, unused constants and conditions have been removed, and the OpenAI integration now includes an optional property with refined model type checks and token handling. The constants file has been updated with new model identifiers, cutoff dates, and regex patterns. Additionally, the utility function for timeout calculation now evaluates extra conditions, and test cases have been adjusted to reflect updated model names and vision model arrays. Default model configuration parameters have also been tuned. The Dockerfile and CI workflow were updated to use a newer Node.js version and switch from Yarn to npm. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OpenAI
Client->>OpenAI: Send request with model config
alt Model type is O1, O3, or O4
OpenAI->>OpenAI: Set reasoning_effort (if O4) and adjust token limits
OpenAI->>OpenAI: Apply fixed temperature, penalties, and max tokens
else Other models
OpenAI->>OpenAI: Use configured parameters and max tokens
end
OpenAI->>Client: Return RequestPayload
sequenceDiagram
participant Caller as Request Initiator
participant TimeoutFunc as getTimeoutMSByModel
Caller->>TimeoutFunc: Call getTimeoutMSByModel(model)
alt Model includes "-thinking", "o4", or "pro"
TimeoutFunc->>Caller: Log "thinking model" and return extended timeout
else
TimeoutFunc->>Caller: Log "normal model" and return standard timeout
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
test/vision-model-checker.test.ts (1)
16-34: 💡 Verification agent🧩 Analysis chain
Verify complete vision model coverage
This test covers the new models mentioned in the PR (gemini-2.5-pro), but I don't see "o1" which was mentioned in the PR objectives.
🏁 Script executed:
#!/bin/bash # Check for the presence of "o1" in VISION_MODEL_REGEXES rg -A 1 "VISION_MODEL_REGEXES" app/constant.tsLength of output: 175
Action: Update the regex or test suite to include the 'o1' model
The current VISION_MODEL_REGEXES (in
app/constant.ts) only includes the/vision/pattern and does not cover the"o1"model specified in the PR objectives. Please adjust the regex definitions or update the test suite accordingly to ensure complete vision model coverage.
- Location:
app/constant.ts(whereVISION_MODEL_REGEXESis defined)- Required Update: Include a regex that matches
"o1"if that model is expected to be recognized.
🧹 Nitpick comments (1)
app/utils.ts (1)
307-318: Consider more specific model matching for timeoutsWhile adding
model.includes("pro")to the timeout logic will work for the new models mentioned in the PR, this pattern might be too broad and could inadvertently apply longer timeouts to models that don't need them.Consider using a more specific condition or maintaining a list of models that specifically need the longer timeout:
- model.includes("-thinking") || - model.includes("pro") + model.includes("-thinking") || + (model.includes("pro") && ( + model.includes("gemini-2.5-pro") || + model.includes("gemini-1.5-pro") + ))The enhanced logging is helpful for debugging timeout issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
.gitignore(1 hunks)app/client/platforms/anthropic.ts(0 hunks)app/client/platforms/google.ts(0 hunks)app/client/platforms/openai.ts(4 hunks)app/constant.ts(3 hunks)app/utils.ts(1 hunks)test/model-available.test.ts(1 hunks)test/vision-model-checker.test.ts(1 hunks)
💤 Files with no reviewable changes (2)
- app/client/platforms/anthropic.ts
- app/client/platforms/google.ts
🧰 Additional context used
🧬 Code Definitions (1)
app/client/platforms/openai.ts (1)
app/utils.ts (1)
getTimeoutMSByModel(299-320)
🔇 Additional comments (13)
.gitignore (1)
52-52: LGTM: Adding local Dockerfile to gitignoreAdding
Dockerfile.localto .gitignore is a good practice to prevent local Docker configurations from being committed to the repository.test/model-available.test.ts (1)
6-6: Model name update looks goodUpdating the test to use "gpt-4o" instead of "gpt-4" aligns with the PR objective of removing legacy models and adding new ones.
test/vision-model-checker.test.ts (1)
18-18: Vision model list update aligns with PR objectivesUpdating the vision models test list to include "gpt-4o" instead of "gpt-4-vision" and adding "gemini-2.5-pro" aligns with the PR objective of updating to newer models.
Also applies to: 22-22
app/client/platforms/openai.ts (5)
70-71: Added new property for O3 models' reasoning capabilityThe addition of
reasoning_effortoptional parameter allows controlling how much reasoning effort the O3 models will apply, which is a key feature of these newer models.
201-203: Clear model type identification with separated boolean variablesGood refactoring that improves readability by explicitly separating O1 and O3 model checks, then combining them into a single variable for shared behaviors.
247-250: Setting high reasoning effort for O3 models by defaultThis change implements the PR objective to adjust the default setting for o3-mini to use high reasoning effort, which should improve the quality of responses.
254-258: Model-specific token handling logicThe improved conditional logic handles token parameters differently based on model type:
- For O1: Uses
max_completion_tokensas per O1 API requirements- For other vision models: Sets
max_tokenswith a minimum threshold of 4000This change ensures each model type receives appropriate token limits.
301-304: Enhanced timeout calculation for different model typesThe timeout logic now uses
getTimeoutMSByModelfunction which applies different timeout values based on model type. This likely addresses the timeout issue with thinking models mentioned in the PR objectives.app/constant.ts (5)
412-412: Updated GEMINI_SUMMARIZE_MODEL to newer versionChanged from "gemini-pro" to "gemini-2.0-flash", which aligns with the PR objective to remove legacy models and add latest versions.
416-422: Updated knowledge cutoff dates for latest modelsThe changes include:
- Updated default cutoff from "2023-10" (more recent than previous)
- Added new entries for Gemini and Claude models with appropriate cutoff dates
- These updates ensure the models reference the correct knowledge timeframes
This aligns with adding new models from Google and Anthropic as mentioned in the PR objectives.
448-448: Added vision capability detection for new modelsAdded regex patterns to identify "gemini-2.5-pro" and "o1" as vision models, which supports the PR objective of adding new vision models (o1 and gemini-2.5-pro-exp).
Also applies to: 456-456
470-470: Added new OpenAI modelsAdded "o1" and "gpt-4.5-preview" to the OpenAI models array, which aligns with the PR objective to add new models and specifically mentions o1 as one of the new vision models.
Also applies to: 472-472
480-481: Added new Google modelsAdded "gemini-2.0-flash-lite" and "gemini-2.5-pro-exp-03-25" to the Google models array. The latter is specifically mentioned in the PR objectives as one of the new vision models being added.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
Dockerfile (1)
9-13: Lock‑file mismatch and registry pinning
COPY package.json package-lock.json ./– the project previously used Yarn; ensurepackage-lock.jsonis committed and up‑to‑date, or the build will ignore your intended dependency tree.- Using a mirror with
npm config set registryhard‑codes every install to npmmirror.com. Consider shipping an.npmrcinstead so local devs inherit the same config, or gate this behindARG NPM_REGISTRY=.- Add
--prefer-offline --no-audit --fund=falseflags to speed up CI and silence audit noise.Example improvement:
-RUN npm config set registry 'https://registry.npmmirror.com/' -RUN npm install +ARG NPM_REGISTRY=https://registry.npmmirror.com/ +RUN npm config set registry "$NPM_REGISTRY" \ + && npm ci --prefer-offline --no-audit --fund=falseapp/utils/hooks.ts (1)
6-11: Prefer aSetfor O(1) look‑ups and easier extensionSince
allowedProvidersis used only for membership tests, converting it to aSetavoids the linear cost ofArray.prototype.includesinside every filter operation.-const allowedProviders = [ - ServiceProvider.OpenAI, - ServiceProvider.Google, - ServiceProvider.Anthropic, -]; +const allowedProviders = new Set<ServiceProvider>([ + ServiceProvider.OpenAI, + ServiceProvider.Google, + ServiceProvider.Anthropic, +]);Remember to change
includes→hasbelow.app/constant.ts (1)
457-483: Minor naming inconsistencies (alibabaModes,moonshotModes, …)The variable suffix alternates between
ModelsandModes, which can confuse future maintainers. Consider renaming for consistency:-const alibabaModes = ["qwen-vl-max"]; +const alibabaModels = ["qwen-vl-max"];Repeat for
moonshotModes,xAIModes, etc.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
Dockerfile(2 hunks)app/client/platforms/openai.ts(5 hunks)app/constant.ts(13 hunks)app/utils.ts(1 hunks)app/utils/hooks.ts(1 hunks)test/vision-model-checker.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- test/vision-model-checker.test.ts
- app/utils.ts
- app/client/platforms/openai.ts
🔇 Additional comments (5)
Dockerfile (2)
22-22:npm run buildassumes lifecycle scripts are already migratedDouble‑check that
package.jsonhas a"build"script using npm syntax and that no Yarn‑specific commands remain (workspaces,yarn build --filteretc.).
Otherwise this stage will fail at runtime.
32-33: ENV variables added but not documented / referenced in code
ANTHROPIC_API_KEYandVISION_MODELSare declared, yet no default values or documentation are provided.
• If they are required at runtime, the container will silently start with empty strings, leading to confusing 401s.
• Add them toREADME.mdand considerARG+ENVcombo with sensible defaults (<unset>).[ suggest_nitpick]
app/utils/hooks.ts (1)
24-31: Filtering excludes Azure and other user‑added providers – is that intentional?Users who configure their own Azure‑OpenAI or Moonshot endpoints via the UI will now see their custom models disappear from the picker because
provider.providerNamewon’t match the hard‑coded list.
If the goal is merely to hide built‑in unsupported models, consider:if (model.provider?.providerType === "azure" && model.available) return trueOr expose a toggle in settings.
app/constant.ts (2)
411-413: Summarise model constants updated – check downstream defaultsChanging
SUMMARIZE_MODELto"gpt-4.1-mini"andGEMINI_SUMMARIZE_MODELto"gemini-2.0-flash"affects every place where the summariser runs automatically (message export, auto‑condense etc.).
Verify token pricing and availability limits for these newer models; otherwise costs could spike or requests may 404 if the tenant lacks access.
502-505: All Azure‑flavoured OpenAI models set toavailable: falseIf users rely on Azure deployments, the UI may hide these models entirely.
Confirm that the “unavailable” flag is deliberate; otherwise revert totrueor make availability user‑configurable.
| @@ -1,29 +1,25 @@ | |||
| FROM node:18-alpine AS base | |||
| FROM node:23-alpine AS base | |||
There was a problem hiding this comment.
Node 23‑alpine may not yet be an LTS image – verify runtime compatibility and availability
Jumping from node:18-alpine (current LTS) to node:23-alpine brings bleeding‑edge V8 semantics and potential ABI changes.
• Some native add‑ons compiled during npm install (e.g. sharp, bcrypt) may break.
• Docker Hub does not currently publish an official 23-alpine tag; the build will 404.
Unless you explicitly need Node 23 features, stick to the latest LTS (node:20‑alpine) or test the new tag’s existence in CI before merging.
| export const VISION_MODEL_REGEXES = [ | ||
| /vision/, | ||
| /gpt-4o/, | ||
| /gpt-4/, | ||
| /claude-3/, | ||
| /gemini-1\.5/, | ||
| /gemini-exp/, | ||
| /gemini-2\.0/, | ||
| /learnlm/, | ||
| /qwen-vl/, | ||
| /qwen2-vl/, | ||
| /gpt-4-turbo(?!.*preview)/, // Matches "gpt-4-turbo" but not "gpt-4-turbo-preview" | ||
| /gemini/, | ||
| /^dall-e-3$/, // Matches exactly "dall-e-3" | ||
| /glm-4v/, | ||
| /vl/i, | ||
| /o1/, | ||
| /o3/, | ||
| /o4/, | ||
| ]; |
There was a problem hiding this comment.
/gpt-4/ regex is overly broad – non‑vision GPT‑4 models will be mis‑classified
Replacing the old /gpt-4o/ with /gpt-4/ means text‑only models like "gpt-4.1-mini" now satisfy VISION_MODEL_REGEXES, triggering vision‑specific UI flows and higher timeouts.
Recommend refining the pattern:
-/gpt-4/,
+/gpt-4(?:o|v?ision)/, // gpt-4o, gpt-4-vision
+/\bgpt-4\.1\b/, // explicit 4.1 vision variant if neededor enumerate explicit model names similar to the new o1|o3|o4 additions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const VISION_MODEL_REGEXES = [ | |
| /vision/, | |
| /gpt-4o/, | |
| /gpt-4/, | |
| /claude-3/, | |
| /gemini-1\.5/, | |
| /gemini-exp/, | |
| /gemini-2\.0/, | |
| /learnlm/, | |
| /qwen-vl/, | |
| /qwen2-vl/, | |
| /gpt-4-turbo(?!.*preview)/, // Matches "gpt-4-turbo" but not "gpt-4-turbo-preview" | |
| /gemini/, | |
| /^dall-e-3$/, // Matches exactly "dall-e-3" | |
| /glm-4v/, | |
| /vl/i, | |
| /o1/, | |
| /o3/, | |
| /o4/, | |
| ]; | |
| export const VISION_MODEL_REGEXES = [ | |
| /vision/, | |
| - /gpt-4/, | |
| + /gpt-4(?:o|v?ision)/, // gpt-4o, gpt-4-vision | |
| + /\bgpt-4\.1\b/, // explicit 4.1 vision variant if needed | |
| /claude-3/, | |
| /gemini/, | |
| /^dall-e-3$/, // Matches exactly "dall-e-3" | |
| /o1/, | |
| /o3/, | |
| /o4/, | |
| ]; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
31-31: Validate cache key for npm dependencies
Switching the cache key to targetpackage-lock.jsonis appropriate for npm. Ensure that the pattern'**/package-lock.json'correctly matches your lockfile’s path.You could simplify by leveraging only the built‑in cache from
setup-node(removing this manual cache) to reduce complexity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🔇 Additional comments (3)
.github/workflows/test.yml (3)
24-25: Review Node.js version and caching strategy
You've updatednode-versionto23and setcache: 'npm'.
- Confirm that Node.js 23 is officially supported and stable for your CI environment; you may prefer the latest LTS (e.g., 20.x) for production use.
- Verify that the built-in npm caching in
setup-nodedoesn’t conflict with the separateactions/cachestep below.
36-36: Switch install command to npm
Replacingyarn installwithnpm installaligns with the package manager change. Please verify that no residual Yarn commands or lockfiles remain in the repository.
39-39: Update test command for npm
Changingyarn test:citonpm run test:ciis correct; ensure that atest:ciscript is defined in yourpackage.jsonand that it runs as expected under the new Node.js version.
|
similar feature already merged and deployed. |
2 participants
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
📝 补充信息 | Additional Information
Summary by CodeRabbit
.gitignoreand Docker environment variables.