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 10 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.comet.opik.api;

import com.fasterxml.jackson.annotation.JsonValue;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.RequiredArgsConstructor;

import java.time.Instant;
import java.util.Arrays;
import java.util.UUID;

public sealed interface AutomationRule<T> permits AutomationRuleEvaluator {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.


UUID id();
UUID projectId();

AutomationRuleAction action();
float samplingRate();

Instant createdAt();
String createdBy();
Instant lastUpdatedAt();
String lastUpdatedBy();

@Getter
@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
enum AutomationRuleAction {

EVALUATOR("evaluator");

@JsonValue
private final String action;

public static AutomationRule.AutomationRuleAction fromString(String action) {
return Arrays.stream(values()).filter(v -> v.action.equals(action)).findFirst()
.orElseThrow(() -> new IllegalArgumentException("Unknown feedback type: " + action));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package com.comet.opik.api;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonView;
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
import com.fasterxml.jackson.databind.annotation.JsonNaming;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Pattern;
import lombok.Builder;

import java.time.Instant;
import java.util.List;
import java.util.UUID;

import static com.comet.opik.utils.ValidationUtils.NULL_OR_NOT_BLANK;

@Builder(toBuilder = true)
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public record AutomationRuleEvaluator (
// Fields and methods
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) UUID id,
@JsonView({View.Public.class, View.Write.class}) @NotNull UUID projectId,
@JsonView({View.Public.class, View.Write.class}) @NotNull AutomationRuleEvaluatorType type,

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

Choose a reason for hiding this comment

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

This is validated at the DAO level, please add validations on this API object as well.


@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) Instant createdAt,
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) String createdBy,
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) Instant lastUpdatedAt,
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) String lastUpdatedBy) implements AutomationRule {

@Override
public AutomationRuleAction action() {
return AutomationRuleAction.EVALUATOR;
}

public static class View {
public static class Write {}
public static class Public {}
}

@Builder(toBuilder = true)
public record AutomationRuleEvaluatorPage(
@JsonView({View.Public.class}) int page,
@JsonView({View.Public.class}) int size,
@JsonView({View.Public.class}) long total,
@JsonView({View.Public.class}) List<AutomationRuleEvaluator> content)
implements Page<AutomationRuleEvaluator>{

public static AutomationRuleEvaluator.AutomationRuleEvaluatorPage empty(int page) {
return new AutomationRuleEvaluator.AutomationRuleEvaluatorPage(page, 0, 0, List.of());
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.comet.opik.api;

import com.fasterxml.jackson.annotation.JsonValue;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.RequiredArgsConstructor;

@Getter
@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
public enum AutomationRuleEvaluatorType {

LLM_AS_JUDGE("llm-as-judge"),
PYTHON("python");

@JsonValue
private final String type;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.comet.opik.api;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
import com.fasterxml.jackson.databind.annotation.JsonNaming;
import jakarta.validation.constraints.NotBlank;
import lombok.Builder;

@Builder(toBuilder = true)
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public record AutomationRuleEvaluatorUpdate(
@NotBlank String code,
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

float samplingRate) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
package com.comet.opik.api.resources.v1.priv;

import com.codahale.metrics.annotation.Timed;
import com.comet.opik.api.AutomationRuleEvaluator;
import com.comet.opik.api.AutomationRuleEvaluatorUpdate;
import com.comet.opik.api.Page;
import com.comet.opik.domain.AutomationRuleEvaluatorService;
import com.comet.opik.infrastructure.auth.RequestContext;
import com.comet.opik.infrastructure.ratelimit.RateLimited;
import com.fasterxml.jackson.annotation.JsonView;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.headers.Header;
import io.swagger.v3.oas.annotations.media.Content;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.parameters.RequestBody;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.inject.Inject;
import jakarta.inject.Provider;
import jakarta.validation.Valid;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotNull;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.DELETE;
import jakarta.ws.rs.DefaultValue;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.PUT;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.Context;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.UriInfo;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

import java.net.URI;
import java.util.UUID;

@Path("/v1/private/automation/evaluators/project/{projectId}")
ldaugusto marked this conversation as resolved.
Show resolved Hide resolved
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
@Timed
@Slf4j
@RequiredArgsConstructor(onConstructor_ = @Inject)
@Tag(name = "Automation rule evaluators", description = "Automation rule evaluators resource")
public class AutomationRuleEvaluatorsResource {

private final @NonNull AutomationRuleEvaluatorService service;
private final @NonNull Provider<RequestContext> requestContext;

@GET
@Operation(operationId = "findEvaluators", summary = "Find Evaluators", description = "Find Evaluators", responses = {
@ApiResponse(responseCode = "200", description = "Evaluators resource", content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.AutomationRuleEvaluatorPage.class)))
})
@JsonView(AutomationRuleEvaluator.View.Public.class)
public Response find(@PathParam("projectId") UUID projectId,
@QueryParam("page") @Min(1) @DefaultValue("1") int page,
@QueryParam("size") @Min(1) @DefaultValue("10") int size) {

String workspaceId = requestContext.get().getWorkspaceId();
log.info("Looking for automated evaluators for project id '{}' on workspaceId '{}' (page {})", projectId,
workspaceId, page);
Page<AutomationRuleEvaluator> definitionPage = service.find(page, size, projectId, workspaceId);
log.info("Found {} automated evaluators for project id '{}' on workspaceId '{}' (page {}, total {})",
definitionPage.size(), projectId, workspaceId, page, definitionPage.total());

return Response.ok()
.entity(definitionPage)
.build();
}

@GET
@Path("/evaluator/{evaluatorId}")
ldaugusto marked this conversation as resolved.
Show resolved Hide resolved
@Operation(operationId = "getEvaluatorById", summary = "Get automation rule evaluator by id", description = "Get automation rule by id", responses = {
@ApiResponse(responseCode = "200", description = "Automation Rule resource", content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.class)))
})
@JsonView(AutomationRuleEvaluator.View.Public.class)
public Response getEvaluator(@PathParam("projectId") UUID projectId, @PathParam("evaluatorId") UUID evaluatorId) {
String workspaceId = requestContext.get().getWorkspaceId();

log.info("Looking for automated evaluator: id '{}' on project_id '{}'", projectId, workspaceId);
AutomationRuleEvaluator evaluator = service.findById(evaluatorId, projectId, workspaceId);
log.info("Found automated evaluator: id '{}' on project_id '{}'", projectId, workspaceId);

return Response.ok().entity(evaluator).build();
}

@POST
@Operation(operationId = "createAutomationRuleEvaluator", summary = "Create automation rule evaluator", description = "Create automation rule evaluator", responses = {
@ApiResponse(responseCode = "201", description = "Created", headers = {
@Header(name = "Location", required = true, example = "${basePath}/api/v1/private/automation/evaluators/project/{projectId}/evaluator/{evaluatorId}", schema = @Schema(implementation = String.class))
ldaugusto marked this conversation as resolved.
Show resolved Hide resolved
})
})
@RateLimited
public Response createEvaluator(
@RequestBody(content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.class))) @JsonView(AutomationRuleEvaluator.View.Write.class) @NotNull @Valid AutomationRuleEvaluator evaluator,
@Context UriInfo uriInfo) {

String workspaceId = requestContext.get().getWorkspaceId();
String userName = requestContext.get().getUserName();

log.info("Creating {} evaluator for project_id '{}' on workspace_id '{}'", evaluator.type(),
evaluator.projectId(), workspaceId);
AutomationRuleEvaluator savedEvaluator = service.save(evaluator, workspaceId, userName);
log.info("Created {} evaluator '{}' for project_id '{}' on workspace_id '{}'", evaluator.type(),
savedEvaluator.id(), evaluator.projectId(), workspaceId);

URI uri = uriInfo.getAbsolutePathBuilder()
.path("/projectId/%s/evaluator/%s".formatted(savedEvaluator.projectId().toString(),
savedEvaluator.id().toString()))
.build();
return Response.created(uri).build();
}

@PUT
ldaugusto marked this conversation as resolved.
Show resolved Hide resolved
@Path("/evaluator/{id}")
@Operation(operationId = "updateAutomationRuleEvaluator", summary = "update Automation Rule Evaluator by id", description = "update Automation Rule Evaluator by id", responses = {
@ApiResponse(responseCode = "204", description = "No content"),
})
@RateLimited
public Response updateEvaluator(@PathParam("id") UUID id,
@PathParam("projectId") UUID projectId,
@RequestBody(content = @Content(schema = @Schema(implementation = AutomationRuleEvaluatorUpdate.class))) @NotNull @Valid AutomationRuleEvaluatorUpdate evaluatorUpdate) {

String workspaceId = requestContext.get().getWorkspaceId();
String userName = requestContext.get().getUserName();

log.info("Updating automation rule evaluator by id '{}' and project_id '{}' on workspace_id '{}'", id,
projectId, workspaceId);
service.update(id, projectId, workspaceId, userName, evaluatorUpdate);
log.info("Updated automation rule evaluator by id '{}' and project_id '{}' on workspace_id '{}'", id, projectId,
workspaceId);

return Response.noContent().build();
}

@DELETE
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're generally creating batch endpoints, so you can delete by sending multiple ids.

@Path("/evaluator/{id}")
@Operation(operationId = "deleteAutomationRuleEvaluatorById", summary = "Delete Automation Rule Evaluator by id", description = "Delete a single Automation Rule Evaluator by id", responses = {
@ApiResponse(responseCode = "204", description = "No content"),
})
public Response deleteEvaluator(@PathParam("id") UUID id, @PathParam("projectId") UUID projectId) {

String workspaceId = requestContext.get().getWorkspaceId();
log.info("Deleting dataset by id '{}' on workspace_id '{}'", id, workspaceId);
service.delete(id, projectId, workspaceId);
log.info("Deleted dataset by id '{}' on workspace_id '{}'", id, workspaceId);
return Response.noContent().build();
}

@DELETE
@Operation(operationId = "deleteAutomationRuleEvaluatorByProject", summary = "Delete all project Automation Rule Evaluators", description = "Delete all Automation Rule Evaluator in a project", responses = {
@ApiResponse(responseCode = "204", description = "No content"),
})
public Response deleteProjectEvaluators(@PathParam("projectId") UUID projectId) {

String workspaceId = requestContext.get().getWorkspaceId();
log.info("Deleting evaluators from project_id '{}' on workspace_id '{}'", projectId, workspaceId);
service.deleteByProject(projectId, workspaceId);
log.info("Deleted evaluators from project_id '{}' on workspace_id '{}'", projectId, workspaceId);
return Response.noContent().build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package com.comet.opik.domain;

import com.comet.opik.api.AutomationRule;
import com.comet.opik.api.AutomationRuleEvaluator;
import com.comet.opik.infrastructure.db.UUIDArgumentFactory;
import org.jdbi.v3.sqlobject.config.RegisterArgumentFactory;
import org.jdbi.v3.sqlobject.config.RegisterConstructorMapper;
import org.jdbi.v3.sqlobject.config.RegisterRowMapper;
import org.jdbi.v3.sqlobject.customizer.Bind;
import org.jdbi.v3.sqlobject.customizer.BindList;
import org.jdbi.v3.sqlobject.customizer.BindMethods;
import org.jdbi.v3.sqlobject.statement.SqlQuery;
import org.jdbi.v3.sqlobject.statement.SqlUpdate;

import java.util.Set;
import java.util.UUID;

@RegisterArgumentFactory(UUIDArgumentFactory.class)
@RegisterRowMapper(AutomationRuleRowMapper.class)
@RegisterConstructorMapper(AutomationRuleEvaluator.class)
interface AutomationRuleDAO {

@SqlUpdate("INSERT INTO automation_rules(id, project_id, workspace_id, `action`, sampling_rate, created_by, last_updated_by) "+
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: Is action reserved value? Otherwise, no need to surround it with grave accent character.

"VALUES (:rule.id, :rule.projectId, :workspaceId, :rule.action, :rule.samplingRate, :rule.createdBy, :rule.lastUpdatedBy)")
void saveBaseRule(@BindMethods("rule") AutomationRule rule, @Bind("workspaceId") String workspaceId);

@SqlUpdate("""
UPDATE automation_rules
SET sampling_rate = :samplingRate,
last_updated_by = :lastUpdatedBy
WHERE id = :id AND project_id = :projectId AND workspace_id = :workspaceId
""")
int updateBaseRule(@Bind("id") UUID id,
@Bind("projectId") UUID projectId,
@Bind("workspaceId") String workspaceId,
@Bind("samplingRate") float samplingRate,
@Bind("lastUpdatedBy") String lastUpdatedBy);

@SqlUpdate("DELETE FROM automation_rules WHERE id = :id AND project_id = :projectId AND workspace_id = :workspaceId")
void delete(@Bind("id") UUID id, @Bind("projectId") UUID projectId, @Bind("workspaceId") String workspaceId);
Comment on lines +39 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unnecessary, as you have the version with multiple ids.


@SqlUpdate("DELETE FROM automation_rules WHERE `action` = :action AND project_id = :projectId AND workspace_id = :workspaceId")
void deleteByProject(@Bind("projectId") UUID projectId,
@Bind("workspaceId") String workspaceId,
@Bind("action") AutomationRule.AutomationRuleAction action);

@SqlUpdate("DELETE FROM automation_rules WHERE id IN (<ids>) AND project_id = :projectId AND workspace_id = :workspaceId")
void delete(@BindList("ids") Set<UUID> ids, @Bind("projectId") UUID projectId, @Bind("workspaceId") String workspaceId);
Comment on lines +42 to +48
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just use a template to have a single method, by making action and ids optional.


@SqlQuery("SELECT COUNT(*) FROM automation_rules WHERE project_id = :projectId AND workspace_id = :workspaceId")
long findCount(@Bind("projectId") UUID projectId, @Bind("workspaceId") String workspaceId);

@SqlQuery("SELECT COUNT(*) FROM automation_rules WHERE project_id = :projectId AND workspace_id = :workspaceId AND `action` = :action")
long findCountByActionType(@Bind("projectId") UUID projectId, @Bind("workspaceId") String workspaceId, @Bind("action") AutomationRule.AutomationRuleAction action);
Comment on lines +50 to +54
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same, use a template to make action optional. We don't need two methods for the same thing.

}
Loading
Loading