-
Notifications
You must be signed in to change notification settings - Fork 3
[Integration] Add Cleanlab + Strands Agent integration #107
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
@@ -0,0 +1,9 @@ | |||
# Requirements for cleanlab_codex strands integration | |||
# Core dependencies | |||
cleanlab-tlm>=1.1.14 |
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.
we need it for form_response_string_chat_completions_api
like functions from cleanlab_tlm.utils.chat
@@ -0,0 +1,9 @@ | |||
# Requirements for cleanlab_codex strands integration |
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.
can you explicitly confirm that you've tested using the cleanlab-codex
in an environment where strands
and these other optional dependencies are NOT installed, and the rest of codex package still works as usual? I.e. that these dependencies truly are optional
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.
I tested this in an env without strands + with strands. We are not adding any dependencies to the cleanlab package here, just in a requirements.txt for users to manually install
# Requirements for cleanlab_codex strands integration | ||
# Core dependencies | ||
cleanlab-tlm>=1.1.14 | ||
openai |
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.
cleanlab.validate() expects messages in OpenAI format, to pass typechecker I had to typecast which means we now require types during runtime here.
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.
just had small suggestions and things to confirm.
I'm inclined not to invest other engineers' time on this experimental module. But if there's anything you are concerned about in terms of affecting the overall Codex experience for developers who are not using this strands module, then please do ask engineer to review that aspect.
I fixed the typecasting issue. This should be good to go. |
Key Info
Note: typeignore is added to the types because this is experimental we do not want to require importing any strands code into the real package
Implementation plan: link
Priority:
What changed?
This adds an experimental folder with a strands integration
What do you want the reviewer(s) to focus on?
Checklist