-
Notifications
You must be signed in to change notification settings - Fork 60.7k
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.
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
Add DEFAULT_COMPRESS_MODEL configuration #6139
base: main
Are you sure you want to change the base?
Conversation
@Noctiro is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce a new optional configuration parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant E as Environment (.env)
participant S as Server (getServerSideConfig)
participant A as Access Store
participant C as Chat Module
E->>S: Provide DEFAULT_COMPRESS_MODEL value
S->>S: Read and process DEFAULT_COMPRESS_MODEL (reset if GPT-4 disabled)
S->>A: Return configuration with defaultCompressModel
A->>A: Update DEFAULT_ACCESS_STATE with defaultCompressModel
C->>A: Request model config (via getSummarizeModel)
A-->>C: Return compressModel and provider details
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.env.template
(1 hunks)README.md
(1 hunks)README_CN.md
(1 hunks)README_JA.md
(1 hunks)app/api/config/route.ts
(1 hunks)app/components/model-config.tsx
(1 hunks)app/config/server.ts
(4 hunks)app/store/access.ts
(2 hunks)app/store/chat.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .env.template
🔇 Additional comments (10)
app/api/config/route.ts (1)
17-17
: LGTM!The addition of
defaultCompressModel
to theDANGER_CONFIG
object follows the established pattern and is correctly populated from the server configuration.app/store/access.ts (1)
134-134
: LGTM!The addition of
defaultCompressModel
toDEFAULT_ACCESS_STATE
with an empty string default value is appropriate.app/config/server.ts (3)
26-26
: LGTM!The addition of
DEFAULT_COMPRESS_MODEL
to theProcessEnv
interface with proper JSDoc comment is well-documented.
135-135
: LGTM!The initialization of
defaultCompressModel
and the GPT4 validation logic are correctly implemented, ensuring consistency with the application's GPT4 access controls.Also applies to: 146-148
261-261
: LGTM!The addition of
defaultCompressModel
to the return object maintains consistency with the configuration pattern.app/components/model-config.tsx (1)
262-270
: LGTM! Great UI enhancement.The implementation of provider-based model grouping improves the user experience by organizing models in a more structured way. The use of
lodash-es
'sgroupBy
function is an efficient approach.app/store/chat.ts (1)
126-132
: LGTM! The new configuration check is correctly implemented.The changes prioritize the user-defined compression model over the default model selection logic, providing more flexibility in model configuration.
README_CN.md (1)
247-250
: LGTM! Documentation is clear and consistent.The new environment variable
DEFAULT_COMPRESS_MODEL
is well-documented with a clear description of its purpose.README_JA.md (1)
220-223
: LGTM! Japanese documentation is accurate and consistent.The new environment variable
DEFAULT_COMPRESS_MODEL
is well-documented with a clear Japanese translation of its purpose.README.md (1)
324-327
: LGTM! English documentation is clear and consistent.The new environment variable
DEFAULT_COMPRESS_MODEL
is well-documented with a clear description of its purpose.
const defaultCompressModel = res.defaultCompressModel ?? ""; | ||
if (defaultCompressModel) { | ||
const [model, providerName] = | ||
getModelProvider(defaultCompressModel); | ||
DEFAULT_CONFIG.modelConfig.compressModel = model; | ||
DEFAULT_CONFIG.modelConfig.compressProviderName = | ||
providerName as any; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for getModelProvider call.
The code should handle potential errors from getModelProvider
to prevent runtime exceptions.
const defaultCompressModel = res.defaultCompressModel ?? "";
if (defaultCompressModel) {
+ try {
const [model, providerName] =
getModelProvider(defaultCompressModel);
DEFAULT_CONFIG.modelConfig.compressModel = model;
DEFAULT_CONFIG.modelConfig.compressProviderName =
providerName as any;
+ } catch (error) {
+ console.error("[Config] failed to parse compress model:", error);
+ }
}
📝 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.
const defaultCompressModel = res.defaultCompressModel ?? ""; | |
if (defaultCompressModel) { | |
const [model, providerName] = | |
getModelProvider(defaultCompressModel); | |
DEFAULT_CONFIG.modelConfig.compressModel = model; | |
DEFAULT_CONFIG.modelConfig.compressProviderName = | |
providerName as any; | |
} | |
const defaultCompressModel = res.defaultCompressModel ?? ""; | |
if (defaultCompressModel) { | |
try { | |
const [model, providerName] = | |
getModelProvider(defaultCompressModel); | |
DEFAULT_CONFIG.modelConfig.compressModel = model; | |
DEFAULT_CONFIG.modelConfig.compressProviderName = | |
providerName as any; | |
} catch (error) { | |
console.error("[Config] failed to parse compress model:", error); | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The defaultModel was also set in the same way earlier, and it doesn’t seem to throw an exception.
前几行defaultModel也是用相同的方式设置的,貌似是不会抛出异常
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Noctiro After checking the getModelProvider
implementation, I can confirm that your approach is correct. The function is designed to handle all cases safely through string operations, which explains why defaultModel
works reliably without error handling. My previous suggestion was overly cautious and unnecessary.
🔗 Analysis chain
@Noctiro You make a valid point. Since defaultModel
uses the same pattern successfully without error handling, I'll withdraw my suggestion. Thank you for the clarification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find and display the implementation of getModelProvider function
ast-grep --pattern 'export function getModelProvider' -A 30
Length of output: 61
Script:
#!/bin/bash
# Find getModelProvider implementation using ripgrep
rg -U "export function getModelProvider" -A 30
Length of output: 1422
✏️ Learnings added
Learnt from: Noctiro
PR: ChatGPTNextWeb/NextChat#6139
File: app/store/access.ts:251-258
Timestamp: 2025-02-04T03:46:50.415Z
Learning: The getModelProvider function in app/utils/model.ts uses safe string operations to parse model names with optional provider (format: modelName@providerId), making explicit error handling unnecessary as it always returns a valid [model, provider] tuple where provider can be undefined.
Learnt from: Noctiro
PR: ChatGPTNextWeb/NextChat#6139
File: app/store/access.ts:251-258
Timestamp: 2025-02-04T03:46:50.415Z
Learning: The getModelProvider function used in access store for parsing model configurations (defaultModel, defaultCompressModel) has built-in safety mechanisms and doesn't require explicit try-catch error handling.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
添加自定义压缩(总结)模型默认项
Add DEFAULT_COMPRESS_MODEL configuration
将
compress model
设置项的样式与model
设置项的样式统一Align the style of the
compress model
setting with that of themodel
setting.📝 补充信息 | Additional Information
I also noticed that both 'compress' and 'summary' appear in the code. Should I merge them into 'summary' in this PR?
还有我注意到代码中存在compress和summary这两个词汇同时存在,需要顺便在我这个PR将其合为一个summary?
Summary by CodeRabbit