[Online Scoring] Automation Rules endpoints #945

ldaugusto · 2024-12-20T13:57:38Z

Details

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

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

andrescrz

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!

apps/opik-backend/src/main/java/com/comet/opik/api/AutomationRuleEvaluator.java

andrescrz · 2024-12-20T15:50:15Z

apps/opik-backend/src/main/java/com/comet/opik/api/AutomationRuleEvaluator.java

+        @JsonView({View.Public.class,
+                View.Write.class}) @Pattern(regexp = NULL_OR_NOT_BLANK, message = "must not be blank") String code,


This field should probably be a polymorphic model, so we can define LLM as a judge evaluators and python evaluators, with different payloads.

Let's update code to be a JsonNode.

andrescrz · 2024-12-20T15:50:50Z

apps/opik-backend/src/main/java/com/comet/opik/api/AutomationRuleEvaluator.java

+            return new AutomationRuleEvaluator.AutomationRuleEvaluatorPage(page, 0, 0, List.of());
+        }
+    }
+}


Minor: run spotless on your PRs, so code is automatically formatted.

This is a previous miss from our side, we need to automate this.

...end/src/main/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResource.java

apps/opik-backend/src/main/java/com/comet/opik/domain/AutomationRuleDAO.java

apps/opik-backend/src/main/java/com/comet/opik/domain/AutomationRuleService.java

andrescrz · 2024-12-20T16:12:14Z

apps/opik-backend/src/main/java/com/comet/opik/domain/AutomationRuleService.java

+    }
+
+    @Override
+    public Optional<AutomationRuleEvaluator> getById(@NonNull UUID id, @NonNull UUID projectId,


andrescrz · 2024-12-20T16:12:41Z

apps/opik-backend/src/main/java/com/comet/opik/domain/AutomationRuleService.java

+    }
+
+    @Override
+    public AutomationRuleEvaluator findById(@NonNull UUID id, @NonNull UUID projectId, @NonNull String workspaceId) {


Just overload the method by wrapping the id in a set.

...d/src/main/resources/liquibase/db-app-state/migrations/000008_add_automation_rule_tables.sql

…o be based in a sample

…rogramming in case the string changes.

andrescrz

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

andrescrz · 2024-12-23T12:06:43Z

apps/opik-backend/src/main/java/com/comet/opik/api/AutomationRule.java

+import java.util.Arrays;
+import java.util.UUID;
+
+public sealed interface AutomationRule<T> permits AutomationRuleEvaluator {


Let's make this interface and its subclasses self de/serialisable without requiring any external help. You just need to configure it with JsonTypeInfo and JsonSubTypes annotations etc., based on the action property.

andrescrz · 2024-12-23T12:12:49Z

apps/opik-backend/src/main/java/com/comet/opik/api/AutomationRuleEvaluatorUpdate.java

+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
+public record AutomationRuleEvaluatorUpdate(
+        @NotBlank String code,


Same, let's change code to a JsonNode field.

...end/src/main/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResource.java

andrescrz · 2024-12-23T12:37:45Z

...d/src/main/resources/liquibase/db-app-state/migrations/000009_add_automation_rule_tables.sql

+CREATE TABLE IF NOT EXISTS automation_rule_evaluators (
+    id CHAR(36),
+
+    `type` ENUM('llm_as_judge', 'python') NOT NULL,


...d/src/main/resources/liquibase/db-app-state/migrations/000009_add_automation_rule_tables.sql

andrescrz · 2024-12-23T12:39:44Z

...d/src/main/resources/liquibase/db-app-state/migrations/000009_add_automation_rule_tables.sql

+    INDEX `automation_rules_idx` (workspace_id, project_id, id)
+);
+
+CREATE TABLE IF NOT EXISTS automation_rule_evaluators (


As general convention, we denormalise created_at etc. fields in all tables.

andrescrz · 2024-12-23T12:40:39Z

apps/opik-backend/src/main/java/com/comet/opik/api/AutomationRuleEvaluator.java

+        @JsonView({View.Public.class,
+                View.Write.class}) @Pattern(regexp = NULL_OR_NOT_BLANK, message = "must not be blank") String code,


Let's update code to be a JsonNode.

ldaugusto added the work in progress label Dec 20, 2024

ldaugusto requested a review from andrescrz December 20, 2024 13:57

ldaugusto requested a review from a team as a code owner December 20, 2024 13:57

cleaning up

59cb8c2

andrescrz reviewed Dec 20, 2024

View reviewed changes

ldaugusto added 8 commits December 20, 2024 23:53

Splitting Rule and Evaluator types

bdf11fe

cleaning up

6d731f6

moving 'sampling rate' to the base rule; other rule actions would als…

eccaa4b

…o be based in a sample

removing RequestContext references from Service

a79b400

Read-only rule evaluator ids

a20a452

cleaning up

a544940

moving project to endpoint root and cleaning up descriptions

84f5197

Removing hard coded 'evaluator' from sqls to Action bind. Defensive p…

f4310cb

…rogramming in case the string changes.

andrescrz reviewed Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Online Scoring] Automation Rules endpoints #945

[Online Scoring] Automation Rules endpoints #945

ldaugusto commented Dec 20, 2024

andrescrz left a comment •

edited

Loading

andrescrz Dec 20, 2024

andrescrz Dec 23, 2024

andrescrz Dec 20, 2024

andrescrz Dec 20, 2024

andrescrz Dec 20, 2024

andrescrz left a comment

andrescrz Dec 23, 2024

andrescrz Dec 23, 2024

andrescrz Dec 23, 2024

andrescrz Dec 23, 2024

andrescrz Dec 23, 2024

		@JsonView({View.Public.class,
		View.Write.class}) @Pattern(regexp = NULL_OR_NOT_BLANK, message = "must not be blank") String code,

[Online Scoring] Automation Rules endpoints #945

Are you sure you want to change the base?

[Online Scoring] Automation Rules endpoints #945

Conversation

ldaugusto commented Dec 20, 2024

Details

Issues

Testing

Documentation

andrescrz left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrescrz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrescrz left a comment •

edited

Loading