-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: Auto select model only for active provider #6229
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
Conversation
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.
Important
Looks good to me! 👍
Reviewed everything up to cedf42d in 1 minute and 23 seconds. Click for details.
- Reviewed
78lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/DropdownModelProvider.tsx:81
- Draft comment:
The new helper function 'checkModelExists' centralizes the model existence check. Consider renaming it (e.g. to 'modelExists') or having it return a boolean to more clearly represent its intended use. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. web-app/src/containers/DropdownModelProvider.tsx:92
- Draft comment:
In the useEffect for initializing the model, the code first calls 'selectModelProvider' with the provided model and then checks its existence to potentially reset the selection. This leads to two consecutive calls. Consider checking the model’s validity before calling 'selectModelProvider' to avoid unnecessary updates. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. web-app/src/containers/DropdownModelProvider.tsx:98
- Draft comment:
For the 'useLastUsedModel' branch, the fallback has been changed from hardcoded values to an empty selection. Ensure that this empty selection aligns with the intended default behavior for new chats when the last-used model is invalid. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. web-app/src/containers/DropdownModelProvider.tsx:158
- Draft comment:
The formatting change in the provider filtering condition improves readability by splitting the conditions across lines. This is stylistically sound; no functional issues detected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_Tqz0QOb5Rvk9zKN4
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Barecheck - Code coverage reportTotal: 37.58%Your code coverage diff: 0.00% ▴ Uncovered files and lines
|
dinhlongviolin1
left a comment
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.
LGTM
Describe Your Changes
This pull request refactors the model selection logic in the
DropdownModelProvidercomponent to improve reliability and maintainability. The main change is the introduction of a helper function to verify model existence, which simplifies and centralizes the logic for checking if a selected or last-used model is available. Additionally, the default model selection behavior has been adjusted to avoid hardcoded fallbacks.Model selection logic improvements:
checkModelExistshelper function to centralize and simplify the verification of model existence within active providers.checkModelExistsfor both the current model and the last-used model, ensuring only valid models are selected and falling back to an empty selection if not found.llamacpp,llama3.2:3b) in favor of a more flexible fallback to an empty selection when no valid model is available.Code style and formatting:
Fixes Issues
Screen.Recording.2025-08-19.at.14.43.10.mp4
Self Checklist
Important
Refactor
DropdownModelProvider.tsxto improve model selection logic by centralizing model existence checks and removing hardcoded defaults.checkModelExistsinDropdownModelProvider.tsxto centralize model existence checks.checkModelExistsfor current and last-used models, defaulting to empty selection if invalid.llamacpp,llama3.2:3b), using empty selection as fallback.This description was created by
for cedf42d. You can customize this summary. It will automatically update as commits are pushed.