-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: add claude-4 #5829
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
feat: add claude-4 #5829
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 79d9955 in 59 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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/package.json:72
- Draft comment:
PR title suggests adding Claude-4 to the Anthropic models list, but the diff only shows a token.js dependency bump. Ensure that code changes updating the default model list are also included. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_4y4NiUIzFpD8BpI6
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 4d2fcac in 1 minute and 55 seconds. Click for details.
- Reviewed
20lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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/hooks/useModelProvider.ts:63
- Draft comment:
The ordering in mergedModels was reversed (provider models filtered come before existing stored models). Please add an inline comment explaining this change to ensure updated default models (e.g. Claude-4) are prioritized as intended. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The change does appear intentional and meaningful - it affects which models take precedence when there are conflicts. However, the comment is primarily asking for documentation rather than pointing out a bug or suggesting a concrete improvement. The reasoning about Claude-4 seems speculative since there's no clear evidence in the code about specific model priorities. The comment identifies a real change in behavior, and documentation could be helpful for future maintainers. Not having this documentation could lead to confusion about model precedence. While documentation can be valuable, our rules state we should not make purely informative comments. The code change appears intentional and the behavior is clear from the code itself. We should delete this comment as it's primarily requesting documentation rather than identifying a concrete issue that needs fixing.
Workflow ID: wflow_DpvWgK1IPKdqpkeH
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Describe Your Changes
This is to add Claude-4 to default Anthropic models list.
Fixes Issues
Self Checklist
Important
Add Claude-4 to default Anthropic models and reorder model merging in
useModelProvider.ts.useModelProvider.ts, reorder merging of models insetProviders()to prioritize provider models over existing models.token.jsversion from0.7.15to0.7.16inpackage.json.This description was created by
for 4d2fcac. You can customize this summary. It will automatically update as commits are pushed.