-
Notifications
You must be signed in to change notification settings - Fork 61.2k
Fix provider comparison issue in model comparison #6467
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
base: main
Are you sure you want to change the base?
Conversation
Fixed an issue where comparing models using the provider object directly resulted in errors. Changed to compare provider.id to ensure the comparison is based on a unique identifier, accurately determining if the models are the same. If the provider comparison fails, more models will be stored in persistStore. Although this issue is not immediately visible in the frontend due to subsequent processing, it leads to increased memory usage and longer startup times with each page reload.
@Aiharara is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe update refines the logic for merging model configurations in the application's store. Specifically, when merging persisted and current state, the code now matches models by comparing their Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 0
🧹 Nitpick comments (1)
app/store/config.ts (1)
206-206
: Good fix for the object comparison issue!The change from comparing full provider objects to comparing provider IDs is correct. Objects in JavaScript/TypeScript are compared by reference, not by value, which was causing the incorrect comparison results mentioned in the PR description.
This change aligns perfectly with how models are identified in the
mergeModels
function (lines 180-187), where you're already using${model.name}@${model?.provider?.id}
as the unique identifier.For additional robustness, consider adding optional chaining:
- (v) => v.name === pModel.name && v.provider.id === pModel.provider.id, + (v) => v.name === pModel.name && v.provider?.id === pModel.provider?.id,This would prevent potential runtime errors if any provider object is null or undefined.
老问题了,之前就有提交过,没被处理 #6268 |
An old question, I have submitted it before, but it has not been processed #6268 |
Fixed an issue where comparing models using the provider object directly resulted in errors. Changed to compare provider.id to ensure the comparison is based on a unique identifier, accurately determining if the models are the same.
If the provider comparison fails, more models will be stored in persistStore. Although this issue is not immediately visible in the frontend due to subsequent processing, it leads to increased memory usage and longer startup times with each page reload.
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
修复模型对比中的 provider 对比问题
修复了在对比模型时使用 provider 对象直接比较导致的错误。之前的实现中,provider 是一个对象,直接比较对象会导致对比失败。已修改为对比 provider.id,确保对比的是唯一标识符,从而准确判断是否为同一模型。
因为如果对比 provider 失败,persistStore 中会存入越来越多的 models。虽然由于后续处理问题在前端不会被立即发现,但随着每次重载网页,会消耗更多的内存,并伴随更长的启动时间。
更改内容:
将 v.provider === pModel.provider 修改为 v.provider.id === pModel.provider.id。
影响:
此修复确保模型对比逻辑的准确性,避免因对象直接比较导致的错误,优化了内存使用和启动时间。
Fix provider comparison issue in model comparison
Fixed an issue where comparing models using the provider object directly resulted in errors. The previous implementation compared provider as an object, which failed. Changed to compare provider.id to ensure the comparison is based on a unique identifier, accurately determining if the models are the same.
If the provider comparison fails, more models will be stored in persistStore. Although this issue is not immediately visible in the frontend due to subsequent processing, it leads to increased memory usage and longer startup times with each page reload.
Changes:
Changed v.provider === pModel.provider to v.provider.id === pModel.provider.id.
Impact:
This fix ensures the accuracy of model comparison logic, preventing errors caused by direct object comparison, and optimizes memory usage and startup time.
Summary by CodeRabbit