-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: handle manual toggle vision for remote model #6281
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 45ba949 in 1 minute and 44 seconds. Click for details.
- Reviewed
64lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft 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/ChatInput.tsx:106
- Draft comment:
Good update for mmproj support: the added else-if properly flags models with 'vision'. Be cautious that using selectedModel?.capabilities as a dependency may trigger extra runs if the array reference changes frequently. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. web-app/src/containers/dialogs/EditModel.tsx:178
- Draft comment:
Simplified the vision switch UI by removing the tooltip and disabled state. Confirm that manual toggling is intended and that users have enough cues for this change. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_UKPiBxBHkVJYJpYH
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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 8d06c3a in 1 minute and 8 seconds. Click for details.
- Reviewed
88lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft 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/ChatInput.tsx:543
- Draft comment:
Wrapping the file attachment element in a TooltipProvider/Tooltip block is a nice UI clarification. Increasing the container size from 'h-6' to 'h-7' also improves the clickable area. Just verify that the translation key t('visions') clearly communicates its purpose. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%The comment starts with a purely informative statement about the UI change, which is not allowed. However, it ends with a suggestion to verify the translation key, which is a specific request and could be useful. The comment should be edited to remove the purely informative part and keep the useful suggestion.
2. web-app/src/containers/ChatInput.tsx:607
- Draft comment:
The update changing container classes from 'h-6' to 'h-7' for interactive icons (including embeddings, web search, and reasoning icons) ensures a more consistent and accessible UI. This consistency across elements improves the overall touch target size. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_y6WGQ98T9FuVUIzn
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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
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 62eb422 in 1 minute and 26 seconds. Click for details.
- Reviewed
40lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft 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:417
- Draft comment:
Restricting ModelSetting rendering to 'llamacpp' models is intentional, but confirm that other providers with vision capability don’t need this UI. This condition may hide settings for non-llamacpp models if added in the future. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. web-app/src/hooks/useModelProvider.ts:241
- Draft comment:
The migration code now applies only for 'llamacpp' providers. Ensure that this restriction is deliberate so that non-llamacpp models are not affected if they need similar initialization of settings. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that the restriction to 'llamacpp' providers is deliberate. This falls under the rule of not asking the author to confirm their intention or ensure behavior is intended. Therefore, this comment should be removed.
Workflow ID: wflow_ZRzWOsYl3B4LsjXj
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Describe Your Changes
This pull request makes improvements to how the application checks and manages model capabilities, specifically the "vision" capability, and refines the UI for editing model capabilities. The main changes involve more accurate detection of model capabilities, better handling of the
mmprojflag, and a cleaner UI for editing the vision capability.Capability detection and handling:
hasMmprojflag inChatInput.tsxnow more accurately checks if the selected model has the "vision" capability, ensuring that only models with this capability are flagged appropriately.useEffecthook inChatInput.tsxis updated to includeselectedModel?.capabilities, ensuring the effect runs when model capabilities change.UI improvements:
EditModel.tsxhave been removed, resulting in a cleaner and simpler UI for editing model capabilities.Fixes Issues
step 1 edit manually model -> switch vision into true

step 2 back to conv and check chat input, there should be image icon

Self Checklist
Important
Enhances model capability detection and UI for 'vision' capability, improving accuracy and simplifying the interface.
ChatInput.tsx: Improved logic forhasMmprojflag to accurately detect 'vision' capability for non-llamacpp models.DropdownModelProvider.tsx: AddedcheckAndUpdateModelVisionCapability()to update 'vision' capability for llamacpp models.useModelProvider.ts: UpdatedselectModelProvider()to handle 'vision' capability for llamacpp models.EditModel.tsx: Removed tooltip and disabled state for 'vision' capability switch, simplifying the UI.ChatInput.tsx: Updated tooltip and icon sizes for better UI consistency.This description was created by
for 62eb422. You can customize this summary. It will automatically update as commits are pushed.