-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: remove experimental toggle #6255
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 cfbc6b9 in 1 minute and 19 seconds. Click for details.
- Reviewed
218
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
5
draft 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:60
- Draft comment:
Removed the experimentalFeatures toggle: the destructuring now only retrieves spellCheckChatInput and the tool dropdown now checks only for model capabilities and MCP server status. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, describing what changes were made without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements.
2. web-app/src/containers/SettingsMenu.tsx:14
- Draft comment:
Removed experimentalFeatures import and its conditional usage: the MCP servers menu item is now rendered unconditionally. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, describing what was changed without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements.
3. web-app/src/hooks/useChat.ts:246
- Draft comment:
Removed the experimentalFeatures check from tool filtering: availableTools now depends solely on model capabilities, which simplifies the logic. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining what was done in the code without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest any improvements.
4. web-app/src/hooks/useGeneralSetting.ts:6
- Draft comment:
Removed the experimentalFeatures state and its setter from the Zustand store, cleaning up the general settings as the experimental toggle is no longer needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, describing what was done without providing any actionable feedback or suggestions. It doesn't align with the rules for useful comments.
5. web-app/src/routes/settings/general.tsx:208
- Draft comment:
Removed the Experimental Features toggle UI and its associated handler from the Advanced settings card to simplify the UI. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining what was done without providing any actionable feedback or suggestions. It doesn't align with the rules for useful comments.
Workflow ID: wflow_6S1TxL6UTY7nmpbx
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 8de5c17 in 1 minute and 18 seconds. Click for details.
- Reviewed
524
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
3
draft 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/__tests__/SettingsMenu.test.tsx:26
- Draft comment:
Removed the experimentalFeatures property from the useGeneralSetting mock. Verify that SettingsMenu no longer depends on experimentalFeatures (e.g. for rendering MCP servers) and that the tests reflect the new default behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify that a specific change does not affect the functionality of the SettingsMenu component. This falls under asking the author to ensure behavior is intended, which is against the rules. The comment does not provide a specific suggestion or point out a specific issue with the code.
2. web-app/src/hooks/__tests__/useGeneralSetting.test.ts:40
- Draft comment:
Removed experimentalFeatures and related setExperimentalFeatures tests from the hook. Ensure all remaining state updates (like currentLanguage, spellCheckChatInput, and huggingfaceToken) are unaffected. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that certain state updates are unaffected by the removal of experimental features. This falls under asking the author to ensure behavior, which is not allowed by the rules.
3. web-app/src/routes/settings/__tests__/general.test.tsx:294
- Draft comment:
Removed tests for experimental features toggle from the general settings route. Confirm that the UI now consistently renders MCP servers and that testing focuses on the remaining settings (e.g. huggingface token, spell check). - 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_SlIxslvRjsmSVLSI
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Describe Your Changes
Fix issue: #5753
This change removes the experimental toggle. All servers will be disabled by default, and users must toggle them all. This eliminates the need for the experimental toggle. The model shows a tool indicator but can't find tools to enable which is weird.
Fixes Issues
Self Checklist
Important
Remove experimental toggle, affecting
ChatInput
,SettingsMenu
, anduseGeneralSetting
, with corresponding test updates.experimentalFeatures
toggle fromChatInput.tsx
,SettingsMenu.tsx
, anduseGeneralSetting.ts
.MCP Servers
are now always shown inSettingsMenu.tsx
.useChat.ts
.SettingsMenu.test.tsx
to reflect removal of experimental toggle.experimentalFeatures
inuseGeneralSetting.test.ts
.general.test.tsx
to remove checks for experimental features toggle.experimentalFeatures
fromuseGeneralSetting.ts
state and related functions.This description was created by
for 8de5c17. You can customize this summary. It will automatically update as commits are pushed.