-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix description duplication #773
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.
❌ Changes requested. Reviewed everything up to 2b83f1e in 22 seconds
More details
- Looked at
42
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_qUticwvFqApH4FO9
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
This comment was generated by github-actions[bot]! JS SDK Coverage Report📊 Coverage report for JS SDK can be found at the following URL: 📁 Test report folder can be found at the following URL: |
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.
❌ Changes requested. Incremental review on 055fd8c in 51 seconds
More details
- Looked at
79
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. python/composio/tools/toolset.py:836
- Draft comment:
The use ofmodel_copy(deep=True)
is unnecessary here sinceActionModel
is being instantiated fromschema
. Consider removing it for simplicity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is questioning the necessity ofmodel_copy(deep=True)
. This method is used to create a deep copy of the model, which might be necessary if the original object is mutable and could be modified elsewhere. Without more context on howActionModel
is used, it's hard to definitively say if the deep copy is unnecessary.
I might be missing the specific behavior ofActionModel
and whether it requires a deep copy to prevent unintended side effects. The comment assumes that a deep copy is not needed, but this might not be the case if the object is modified elsewhere.
Given the lack of context, it's safer to assume that the author addedmodel_copy(deep=True)
for a reason, possibly to prevent side effects. Without strong evidence that it's unnecessary, the comment should be removed.
Remove the comment as there is no strong evidence thatmodel_copy(deep=True)
is unnecessary. The author likely added it to prevent side effects.
Workflow ID: wflow_w92sbi4OkfLgKgkN
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
== "Name of the user. Please provide a value of type string. This parameter is required." | ||
) | ||
|
||
(schema_1,) = ComposioToolSet().get_action_schemas(actions=[some_action]) |
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.
The second assertion in this test is redundant as it checks the same condition without any change in state or input. Consider removing it.
(schema_1,) = ComposioToolSet().get_action_schemas(actions=[some_action]) |
Important
Fixes description duplication in
get_action_schemas()
and adds a test to ensure consistent action schema descriptions.get_action_schemas()
intoolset.py
by modifying how descriptions are appended.test_get_action_schemas_description_for_runtime_tool()
intest_toolset.py
to verify action schema descriptions are not duplicated.name
parameter insome_action
has a consistent description.This description was created by for 055fd8c. It will automatically update as commits are pushed.