[OPIK-1377] add guardrails to trace #1736
Open
+896
−126
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrescrz
reviewed
Apr 4, 2025
| @Builder(toBuilder = true) | ||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) | ||
| public record Item(double score) { |
There was a problem hiding this comment.
Minor: let's align the precision with the Guardrails backend, but probably a float should be enough and more performant (including storage space).
| @Builder(toBuilder = true) | ||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) | ||
| public record GuardrailTopicDetails(Map<String, Double> scores) { |
| @Builder(toBuilder = true) | ||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) | ||
| public record Item(@NonNull String name, double score) { |
| @Schema(accessMode = Schema.AccessMode.READ_ONLY) Instant lastUpdatedAt, | ||
| @Schema(accessMode = Schema.AccessMode.READ_ONLY) String createdBy, | ||
| @Schema(accessMode = Schema.AccessMode.READ_ONLY) String lastUpdatedBy) { | ||
| @NonNull List<Item> items) { |
There was a problem hiding this comment.
These items overlap with the model above, but I'd say you need polymorphic types for config and details fields here.
| @@ -47,6 +47,8 @@ public record Trace( | |||
| Trace.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) List<FeedbackScore> feedbackScores, | |||
| @JsonView({ | |||
| Trace.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) List<Comment> comments, | |||
| @JsonView({ | |||
| Trace.View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) List<GuardrailsCheck> guardrailsChecks, | |||
There was a problem hiding this comment.
Related to the main open discussion: this response model won't be able to return top level fields, only the granular validations. As the Guardrails service is likely the source of truth for that determination, we don't have a way to track it in the opik-backend.
idoberko2
force-pushed
the
ido/opik-1386-guardrails-ingestion
branch
2 times, most recently
from
cb5a88a to
631b369
Compare
idoberko2
force-pushed
the
ido/opik-1386-guardrails-ingestion
branch
2 times, most recently
from
ccec599 to
11378e4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details
This PR adds guardrails to the
Traceobject. Served in bothgetTraceByIdlogic as well asfindTraces.Issues
OPIK-1377
Testing
Added two E2E tests covering the new logic.