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

[OPIK-736] Fix deserialization issue #1022

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.comet.opik.api;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.annotation.JsonView;
import io.swagger.v3.oas.annotations.media.DiscriminatorMapping;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
Expand All @@ -19,54 +21,50 @@
@SuperBuilder(toBuilder = true)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "type", visible = true)
@JsonSubTypes({
@JsonSubTypes.Type(value = AutomationRuleEvaluatorLlmAsJudge.class, name = "llm_as_judge")
@JsonSubTypes.Type(value = AutomationRuleEvaluatorLlmAsJudge.class, name = AutomationRuleEvaluatorType.Constants.LLM_AS_JUDGE)
})
@Schema(name = "AutomationRuleEvaluator", discriminatorProperty = "type", discriminatorMapping = {
@DiscriminatorMapping(value = "llm_as_judge", schema = AutomationRuleEvaluatorLlmAsJudge.class)
@DiscriminatorMapping(value = AutomationRuleEvaluatorType.Constants.LLM_AS_JUDGE, schema = AutomationRuleEvaluatorLlmAsJudge.class)
})
@AllArgsConstructor
public abstract sealed class AutomationRuleEvaluator<T>
implements
AutomationRule<T>
public abstract sealed class AutomationRuleEvaluator<T> implements AutomationRule<T>
permits AutomationRuleEvaluatorLlmAsJudge {

@JsonView({View.Public.class})
@Schema(accessMode = Schema.AccessMode.READ_ONLY)
UUID id;
private UUID id;

@JsonView({View.Public.class})
@Schema(accessMode = Schema.AccessMode.READ_ONLY)
UUID projectId;
private UUID projectId;

@JsonView({View.Public.class, View.Write.class})
@Schema(accessMode = Schema.AccessMode.READ_WRITE)
@NotBlank
String name;
private String name;

@JsonView({View.Public.class, View.Write.class})
@Schema(accessMode = Schema.AccessMode.READ_WRITE)
Float samplingRate;
private Float samplingRate;

@JsonView({View.Public.class})
@Schema(accessMode = Schema.AccessMode.READ_ONLY)
Instant createdAt;
private Instant createdAt;

@JsonView({View.Public.class})
@Schema(accessMode = Schema.AccessMode.READ_ONLY)
String createdBy;
private String createdBy;

@JsonView({View.Public.class})
@Schema(accessMode = Schema.AccessMode.READ_ONLY)
Instant lastUpdatedAt;
private Instant lastUpdatedAt;

@JsonView({View.Public.class})
@Schema(accessMode = Schema.AccessMode.READ_ONLY)
String lastUpdatedBy;
private String lastUpdatedBy;

@JsonView({View.Public.class})
public abstract AutomationRuleEvaluatorType type();
@NotNull @JsonView({View.Public.class, View.Write.class})
public abstract AutomationRuleEvaluatorType getType();

@JsonView({View.Public.class, View.Write.class})
@JsonIgnore
public abstract T getCode();

@Override
Expand All @@ -87,12 +85,12 @@ public record AutomationRuleEvaluatorPage(
View.Public.class}) int page,
@JsonView({View.Public.class}) int size,
@JsonView({View.Public.class}) long total,
@JsonView({View.Public.class}) List<AutomationRuleEvaluatorLlmAsJudge> content)
@JsonView({View.Public.class}) List<AutomationRuleEvaluator<?>> content)
implements
Page<AutomationRuleEvaluatorLlmAsJudge>{
Page<AutomationRuleEvaluator<?>>{

public static AutomationRuleEvaluator.AutomationRuleEvaluatorPage empty(int page) {
return new AutomationRuleEvaluator.AutomationRuleEvaluatorPage(page, 0, 0, List.of());
public static AutomationRuleEvaluatorPage empty(int page) {
return new AutomationRuleEvaluatorPage(page, 0, 0, List.of());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonView;
import dev.langchain4j.data.message.ChatMessageType;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import lombok.Builder;
Expand All @@ -18,16 +17,15 @@
import java.util.Map;
import java.util.UUID;

import static com.comet.opik.api.AutomationRuleEvaluatorLlmAsJudge.LlmAsJudgeCode;

@EqualsAndHashCode(callSuper = true)
@Data
@SuperBuilder(toBuilder = true)
@ToString(callSuper = true)
public final class AutomationRuleEvaluatorLlmAsJudge
extends
AutomationRuleEvaluator<AutomationRuleEvaluatorLlmAsJudge.LlmAsJudgeCode> {
public final class AutomationRuleEvaluatorLlmAsJudge extends AutomationRuleEvaluator<LlmAsJudgeCode> {

@NotNull @JsonView({View.Public.class, View.Write.class})
@Schema(accessMode = Schema.AccessMode.READ_WRITE)
private LlmAsJudgeCode code;

@Builder(toBuilder = true)
Expand Down Expand Up @@ -68,14 +66,15 @@ public record LlmAsJudgeModelParameters(
@ConstructorProperties({"id", "projectId", "name", "samplingRate", "code", "createdAt", "createdBy",
"lastUpdatedAt", "lastUpdatedBy"})
public AutomationRuleEvaluatorLlmAsJudge(UUID id, UUID projectId, @NotBlank String name, Float samplingRate,
@NotNull LlmAsJudgeCode code,
Instant createdAt, String createdBy, Instant lastUpdatedAt, String lastUpdatedBy) {
@NotNull LlmAsJudgeCode code, Instant createdAt, String createdBy, Instant lastUpdatedAt,
String lastUpdatedBy) {
super(id, projectId, name, samplingRate, createdAt, createdBy, lastUpdatedAt, lastUpdatedBy);
this.code = code;
}

@Override
public AutomationRuleEvaluatorType type() {
public AutomationRuleEvaluatorType getType() {
return AutomationRuleEvaluatorType.LLM_AS_JUDGE;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
import lombok.AccessLevel;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import lombok.experimental.UtilityClass;

import java.util.Arrays;

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

LLM_AS_JUDGE("llm_as_judge");
LLM_AS_JUDGE(Constants.LLM_AS_JUDGE);

@JsonValue
private final String type;
Expand All @@ -21,4 +22,9 @@ public static AutomationRuleEvaluatorType fromString(String type) {
.filter(v -> v.type.equals(type)).findFirst()
.orElseThrow(() -> new IllegalArgumentException("Unknown evaluator type: " + type));
}

@UtilityClass
public static class Constants {
public static final String LLM_AS_JUDGE = "llm_as_judge";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.codahale.metrics.annotation.Timed;
import com.comet.opik.api.AutomationRuleEvaluator;
import com.comet.opik.api.AutomationRuleEvaluatorLlmAsJudge;
import com.comet.opik.api.AutomationRuleEvaluatorUpdate;
import com.comet.opik.api.BatchDelete;
import com.comet.opik.api.Page;
Expand Down Expand Up @@ -42,6 +41,9 @@
import java.net.URI;
import java.util.UUID;

import static com.comet.opik.api.AutomationRuleEvaluator.AutomationRuleEvaluatorPage;
import static com.comet.opik.api.AutomationRuleEvaluator.View;

@Path("/v1/private/automations/projects/{projectId}/evaluators/")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
Expand All @@ -56,9 +58,9 @@ public class AutomationRuleEvaluatorsResource {

@GET
@Operation(operationId = "findEvaluators", summary = "Find project Evaluators", description = "Find project Evaluators", responses = {
@ApiResponse(responseCode = "200", description = "Evaluators resource", content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.AutomationRuleEvaluatorPage.class)))
@ApiResponse(responseCode = "200", description = "Evaluators resource", content = @Content(schema = @Schema(implementation = AutomationRuleEvaluatorPage.class)))
})
@JsonView(AutomationRuleEvaluator.View.Public.class)
@JsonView(View.Public.class)
public Response find(@PathParam("projectId") UUID projectId,
@QueryParam("name") String name,
@QueryParam("page") @Min(1) @DefaultValue("1") int page,
Expand All @@ -67,12 +69,12 @@ public Response find(@PathParam("projectId") UUID projectId,
String workspaceId = requestContext.get().getWorkspaceId();
log.info("Looking for automated evaluators for project id '{}' on workspaceId '{}' (page {})", projectId,
workspaceId, page);
Page<AutomationRuleEvaluatorLlmAsJudge> definitionPage = service.find(projectId, workspaceId, name, page, size);
Page<AutomationRuleEvaluator<?>> evaluatorPage = service.find(projectId, workspaceId, name, page, size);
log.info("Found {} automated evaluators for project id '{}' on workspaceId '{}' (page {}, total {})",
definitionPage.size(), projectId, workspaceId, page, definitionPage.total());
evaluatorPage.size(), projectId, workspaceId, page, evaluatorPage.total());

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

Expand All @@ -81,12 +83,12 @@ public Response find(@PathParam("projectId") UUID projectId,
@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)
@JsonView(View.Public.class)
public Response getEvaluator(@PathParam("projectId") UUID projectId, @PathParam("id") 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);
AutomationRuleEvaluator<?> evaluator = service.findById(evaluatorId, projectId, workspaceId);
log.info("Found automated evaluator: id '{}' on project_id '{}'", projectId, workspaceId);

return Response.ok().entity(evaluator).build();
Expand All @@ -101,16 +103,16 @@ public Response getEvaluator(@PathParam("projectId") UUID projectId, @PathParam(
@RateLimited
public Response createEvaluator(
@PathParam("projectId") UUID projectId,
@RequestBody(content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.class))) @JsonView(AutomationRuleEvaluator.View.Write.class) @NotNull @Valid AutomationRuleEvaluator<?> evaluator,
@RequestBody(content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.class))) @JsonView(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(),
log.info("Creating {} evaluator for project_id '{}' on workspace_id '{}'", evaluator.getType(),
evaluator.getProjectId(), workspaceId);
AutomationRuleEvaluator<?> savedEvaluator = service.save(evaluator, projectId, workspaceId, userName);
log.info("Created {} evaluator '{}' for project_id '{}' on workspace_id '{}'", evaluator.type(),
log.info("Created {} evaluator '{}' for project_id '{}' on workspace_id '{}'", evaluator.getType(),
savedEvaluator.getId(), evaluator.getProjectId(), workspaceId);

URI uri = uriInfo.getBaseUriBuilder()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
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;
Expand All @@ -20,7 +19,8 @@

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

@SqlUpdate("INSERT INTO automation_rules(id, project_id, workspace_id, `action`, name, sampling_rate) " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ public class AutomationRuleEvaluatorRowMapper implements RowMapper<AutomationRul

@Override
public AutomationRuleEvaluatorModel<?> map(ResultSet rs, StatementContext ctx) throws SQLException {

var type = AutomationRuleEvaluatorType.fromString(rs.getString("type"));

return switch (type) {
case LLM_AS_JUDGE -> ctx.findMapperFor(LlmAsJudgeAutomationRuleEvaluatorModel.class)
.orElseThrow(() -> new IllegalStateException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.Set;
import java.util.UUID;

import static com.comet.opik.api.AutomationRuleEvaluator.AutomationRuleEvaluatorPage;
import static com.comet.opik.infrastructure.db.TransactionTemplateAsync.READ_ONLY;
import static com.comet.opik.infrastructure.db.TransactionTemplateAsync.WRITE;

Expand All @@ -42,7 +43,7 @@ <E, T extends AutomationRuleEvaluator<E>> T findById(@NonNull UUID id, @NonNull

void delete(@NonNull Set<UUID> ids, @NonNull UUID projectId, @NonNull String workspaceId);

AutomationRuleEvaluator.AutomationRuleEvaluatorPage find(@NonNull UUID projectId, @NonNull String workspaceId,
AutomationRuleEvaluatorPage find(@NonNull UUID projectId, @NonNull String workspaceId,
String name, int page, int size);

List<AutomationRuleEvaluatorLlmAsJudge> findAll(@NonNull UUID projectId, @NonNull String workspaceId,
Expand All @@ -68,7 +69,7 @@ public <E, T extends AutomationRuleEvaluator<E>> T save(T inputRuleEvaluator,
UUID id = idGenerator.generateId();
IdGenerator.validateVersion(id, "AutomationRuleEvaluator");

return template.inTransaction(WRITE, handle -> {
var savedEvaluator = template.inTransaction(WRITE, handle -> {
var evaluatorsDAO = handle.attach(AutomationRuleEvaluatorDAO.class);

AutomationRuleEvaluatorModel<?> evaluator = switch (inputRuleEvaluator) {
Expand All @@ -92,8 +93,7 @@ public <E, T extends AutomationRuleEvaluator<E>> T save(T inputRuleEvaluator,
evaluatorsDAO.saveBaseRule(evaluator, workspaceId);
evaluatorsDAO.saveEvaluator(evaluator);

return findById(evaluator.id(), evaluator.projectId(), workspaceId);

return evaluator;
} catch (UnableToExecuteStatementException e) {
if (e.getCause() instanceof SQLIntegrityConstraintViolationException) {
log.info(EVALUATOR_ALREADY_EXISTS, e);
Expand All @@ -103,6 +103,8 @@ public <E, T extends AutomationRuleEvaluator<E>> T save(T inputRuleEvaluator,
}
}
});

return findById(savedEvaluator.id(), savedEvaluator.projectId(), workspaceId);
}

@Override
Expand Down Expand Up @@ -147,7 +149,7 @@ public <E, T extends AutomationRuleEvaluator<E>> T findById(@NonNull UUID id, @N
log.debug("Finding AutomationRuleEvaluator with id '{}' in projectId '{}' and workspaceId '{}'", id, projectId,
workspaceId);

return (T) template.inTransaction(READ_ONLY, handle -> {
return template.inTransaction(READ_ONLY, handle -> {
var dao = handle.attach(AutomationRuleEvaluatorDAO.class);
var singleIdSet = Collections.singleton(id);
var criteria = AutomationRuleEvaluatorCriteria.builder().ids(singleIdSet).build();
Expand All @@ -158,6 +160,7 @@ public <E, T extends AutomationRuleEvaluator<E>> T findById(@NonNull UUID id, @N
case LlmAsJudgeAutomationRuleEvaluatorModel llmAsJudge ->
AutomationModelEvaluatorMapper.INSTANCE.map(llmAsJudge);
})
.map(evaluator -> (T) evaluator)
.orElseThrow(this::newNotFoundException);
});
}
Expand Down Expand Up @@ -188,7 +191,7 @@ private NotFoundException newNotFoundException() {
}

@Override
public AutomationRuleEvaluator.AutomationRuleEvaluatorPage find(@NonNull UUID projectId,
public AutomationRuleEvaluatorPage find(@NonNull UUID projectId,
@NonNull String workspaceId,
String name,
int pageNum, int size) {
Expand All @@ -202,19 +205,20 @@ public AutomationRuleEvaluator.AutomationRuleEvaluatorPage find(@NonNull UUID pr
var offset = (pageNum - 1) * size;

var criteria = AutomationRuleEvaluatorCriteria.builder().name(name).build();
var automationRuleEvaluators = dao.find(workspaceId, projectId, criteria, offset, size)
.stream()
.map(evaluator -> switch (evaluator) {
case LlmAsJudgeAutomationRuleEvaluatorModel llmAsJudge ->
AutomationModelEvaluatorMapper.INSTANCE.map(llmAsJudge);
})
.toList();
List<AutomationRuleEvaluator<?>> automationRuleEvaluators = List.copyOf(
dao.find(workspaceId, projectId, criteria, offset, size)
.stream()
.map(evaluator -> switch (evaluator) {
case LlmAsJudgeAutomationRuleEvaluatorModel llmAsJudge ->
AutomationModelEvaluatorMapper.INSTANCE.map(llmAsJudge);
})
.toList());

log.info("Found {} AutomationRuleEvaluators for projectId '{}'", automationRuleEvaluators.size(),
projectId);
return new AutomationRuleEvaluator.AutomationRuleEvaluatorPage(pageNum, automationRuleEvaluators.size(),
return new AutomationRuleEvaluatorPage(pageNum, automationRuleEvaluators.size(),
total,
automationRuleEvaluators);

});
}

Expand Down
Loading
Loading