-
Notifications
You must be signed in to change notification settings - Fork 904
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: litellm for multiple model support #304
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.
👍 Looks good to me!
- Reviewed the entire pull request up to 8e71121
- Looked at
182
lines of code in5
files - Took 39 seconds to review
More info
- Skipped
1
files when reviewing. - Skipped posting
1
additional comments because they didn't meet confidence threshold of50%
.
1. agents-api/pyproject.toml:28
:
- Assessed confidence :
66%
- Grade:
0%
- Comment:
The PR introduces a new dependencylitellm
and uses itsacompletion
function throughout the code. It would be beneficial to include a description or context in the PR about why this change is necessary and what benefits it brings. - Reasoning:
The PR introduces a new dependencylitellm
for handling model completions. Theacompletion
function fromlitellm
is used to replace the previous model client completions. However, the PR does not provide a description or context for why this change is necessary or what benefits it brings. This makes it difficult to assess the impact and necessity of the change.
Workflow ID: wflow_fKO1RXnzvjOjDbaT
You can customize Ellipsis with review rules, user-specific overrides, quiet
mode, and more. See docs.
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.
👍 Looks good to me!
- Performed an incremental review on 39b118b
- Looked at
12
lines of code in1
files - Took 54 seconds to review
More info
- Skipped
0
files when reviewing. - Skipped posting
1
additional comments because they didn't meet confidence threshold of50%
.
1. agents-api/agents_api/model_registry.py:106
:
- Assessed confidence :
100%
- Grade:
0%
- Comment:
The PR description indicates significant changes involving the integration oflitellm
and updates in model management, but the provided diff only shows a minor unrelated change. Please ensure that all relevant changes are included in the PR. - Reasoning:
The PR description mentions that thelitellm
library is integrated for handling multiple models and thatlitellm.acompletion
replaces model client calls insession.py
andsummarization.py
. However, the provided diff only shows a minor change inmodel_registry.py
which is unrelated to the key points described. This discrepancy suggests that the PR might be incomplete or the wrong files/diffs might have been included in the description.
Workflow ID: wflow_pdhegBrYg5QWyLJj
You can customize Ellipsis with review rules, user-specific overrides, quiet
mode, and more. See docs.
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.
👍 Looks good to me!
- Performed an incremental review on e545583
- Looked at
37
lines of code in2
files - Took 1 minute and 4 seconds to review
More info
- Skipped
0
files when reviewing. - Skipped posting
1
additional comments because they didn't meet confidence threshold of50%
.
1. agents-api/agents_api/routers/sessions/session.py:352
:
- Assessed confidence :
100%
- Grade:
0%
- Comment:
The PR description states that model client calls are replaced withlitellm.acompletion
, but the diff does not show these changes. Please ensure that the intended updates are included in the PR. - Reasoning:
The PR description mentions that thelitellm
library is integrated and replaces existing model client calls withlitellm.acompletion
. However, the diff provided does not show any changes related to replacing model client calls insession.py
orsummarization.py
as described. This could be an oversight or an incomplete PR.
Workflow ID: wflow_F58ghtxZMhdXStp0
You can customize Ellipsis with review rules, user-specific overrides, quiet
mode, and more. See docs.
Summary:
Integrates the
litellm
library to support multiple models, replacing existing model client calls and updating model management.Key points:
litellm
library for handling multiple models.litellm.acompletion
insession.py
andsummarization.py
.model_registry.py
to manage new model lists and validation.litellm
to project dependencies inpyproject.toml
.Generated with ❤️ by ellipsis.dev