-
Notifications
You must be signed in to change notification settings - Fork 32
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
Featuring SQ mech tool #240
base: main
Are you sure you want to change the base?
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 @bgodlin . Can you please add at least 1 test?
Example here: https://github.com/valory-xyz/mech/blob/main/tests/test_tools.py#L106
@bgodlin will you be able to address the comment from Ardian? |
@bgodlin can you address merge conflicts? |
@0xArdi Minor adjustments have been made to the project. Rather than merely accepting the query and explaining the output from the indexer that receives it, the tool now assists in constructing a query from natural language. However, there is still room for improvement in terms of quality:
Furthermore, as indicated by the failing CI, the unit tests are not passing. This issue is specifically related to the |
@@ -0,0 +1,53 @@ | |||
query_examples = """ |
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.
copyright header is missing here, which the CI complains about
openai: | ||
version: ==1.11.0 | ||
tiktoken: | ||
version: ==0.5.1 |
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.
openai: | |
version: ==1.11.0 | |
tiktoken: | |
version: ==0.5.1 | |
openai: | |
version: ==1.11.0 | |
tiktoken: | |
version: ==0.5.1 | |
requests: {} |
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.
CI complains about this
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.
A couple of required changes to address CI failures
@@ -0,0 +1,15 @@ | |||
name: graphql_response_analyser |
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.
there are some missing required fields here:
aea.exceptions.AEAValidationError: The following errors occurred during validation:
- : 'fingerprint' is a required property
- : 'fingerprint_ignore_patterns' is a required property
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.
some more check failures, i left a comment what needs to be changed
Proposed changes
Introducing a new mech tool
Fixes
N/A
Types of changes
What types of changes does your code introduce? (A breaking change is a fix or feature that would cause existing functionality and APIs to not work as expected.)
Put an
x
in the box that appliesChecklist
Put an
x
in the boxes that apply.main
branch (left side). Also you should start your branch off ourmain
.Further comments
N/A