Skip to content
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

[Online Scoring] Automation Rules endpoints #945

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

ldaugusto
Copy link

@ldaugusto ldaugusto commented Dec 20, 2024

Details

Data model, DAO, service layer and endpoints for Automation Rule Evaluators.

We're using a polymorphic type for the evaluator code, which at least for now for LLM-as-Judge, we will use a JsonNode type

Issues

OPIK-590
OPIK-591

Testing

Documentation

Data model, DAO, service layer and endpoints for Automation Rule Evaluators.

I've started with a more complicated approach, but in the end decided for a much simpler one which hopefully is easy enough to extend later into non-evaluator automation rules. Also as we've decided for a non-validation approach in the code, we're just going with a text field.

## Issues
OPIK-590
OPIK-591
@ldaugusto ldaugusto requested a review from andrescrz December 20, 2024 13:57
@ldaugusto ldaugusto requested a review from a team as a code owner December 20, 2024 13:57
Copy link
Collaborator

@andrescrz andrescrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in the design, the rule, action and evaluator can be up to 3 different entities. Fine to go with an approach where they're all collapsed into 1 in order to deliver faster. However, this can make extendability for future requirements harder.

For this PR, focus on making the evaluator payload a polymorphic value, so we can define any type of evaluator (python, llm as a judge). The current code field of type string is a concerning implementation.

Then review a bit the paths of the new resource and adjust them.

Everything else is minor polishings. I didn't go in the tests deeply, as still commented out and because this is in-progress.

Thank you very much for putting this together and congrats for your first PR in Opik!

Copy link
Collaborator

@andrescrz andrescrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in better state, but let's keep polishing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants