feat: deprecate v0 package structure #18
Conversation
Summary of ChangesHello @abhishekg999, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant refactoring to deprecate the v0 package structure, which simplifies the overall API and project layout. The changes primarily involve moving files out of versioned packages, removing deprecated classes, and cleaning up the API surface. A notable change is the removal of the judgeval-instrumentation-openai module. While the refactoring is largely well-executed, I've identified two critical issues that need to be addressed. One is a package declaration mismatch that will cause compilation to fail, and the other is a bug in trace evaluation logic that will cause failures when using custom scorers. Please see the detailed comments for suggestions on how to fix these issues.
| @@ -1,16 +1,115 @@ | |||
| package com.judgmentlabs.judgeval; | |||
| package com.judgmentlabs.judgeval.v1; | |||
There was a problem hiding this comment.
The package declaration com.judgmentlabs.judgeval.v1 does not match the file's directory structure. Since this file is located at judgeval-java/src/main/java/com/judgmentlabs/judgeval/Judgeval.java, the package should be com.judgmentlabs.judgeval. This mismatch will cause compilation errors.
| package com.judgmentlabs.judgeval.v1; | |
| package com.judgmentlabs.judgeval; |
| private TraceEvaluationRun createTraceEvaluationRun(BaseScorer scorer, String traceId, | ||
| String spanId) { | ||
| String evalName = generateRunId("async_trace_evaluate_", spanId); | ||
| return new TraceEvaluationRunBuilder() | ||
| .projectName(configuration.projectName()) | ||
| .evalName(evalName) | ||
| .model(model) | ||
| .trace(traceId, spanId) | ||
| .addScorer(scorer) | ||
| .build(); | ||
| } | ||
|
|
||
| private static List<List<Object>> convertTraceAndSpanIds(List<List<String>> traceAndSpanIds) { | ||
| if (traceAndSpanIds == null || traceAndSpanIds.isEmpty()) | ||
| throw new IllegalArgumentException("Trace and span IDs are required for trace evaluations."); | ||
| List<List<Object>> converted = new java.util.ArrayList<>(); | ||
| for (List<String> pair : traceAndSpanIds) { | ||
| if (pair == null || pair.size() != 2) | ||
| throw new IllegalArgumentException("Each trace and span ID pair must contain exactly 2 elements."); | ||
| converted.add(List.of(pair.get(0), pair.get(1))); | ||
| } | ||
| return converted; | ||
| TraceEvaluationRun evaluationRun = new TraceEvaluationRun(); | ||
| evaluationRun.setId(UUID.randomUUID().toString()); | ||
| evaluationRun.setProjectName(projectName); | ||
| evaluationRun.setEvalName(evalName); | ||
| evaluationRun.setTraceAndSpanIds(List.of(List.of(traceId, spanId))); | ||
| evaluationRun.setJudgmentScorers(List.of(scorer.getScorerConfig())); | ||
| evaluationRun.setCustomScorers(List.of()); | ||
| evaluationRun.setIsOffline(false); | ||
| evaluationRun.setIsBucketRun(false); | ||
| evaluationRun.setCreatedAt(Instant.now().atOffset(ZoneOffset.UTC).format(DateTimeFormatter.ISO_INSTANT)); | ||
|
|
||
| return evaluationRun; | ||
| } |
There was a problem hiding this comment.
The createTraceEvaluationRun method does not correctly handle CustomScorer. It unconditionally calls scorer.getScorerConfig(), which throws an UnsupportedOperationException for CustomScorer instances. This will cause asyncTraceEvaluate to fail when used with a custom scorer. You should handle CustomScorer separately, similar to how it's done in createEvaluationRun.
private TraceEvaluationRun createTraceEvaluationRun(BaseScorer scorer, String traceId,
String spanId) {
String evalName = generateRunId("async_trace_evaluate_", spanId);
TraceEvaluationRun evaluationRun = new TraceEvaluationRun();
evaluationRun.setId(UUID.randomUUID().toString());
evaluationRun.setProjectName(projectName);
evaluationRun.setEvalName(evalName);
evaluationRun.setTraceAndSpanIds(List.of(List.of(traceId, spanId)));
if (scorer instanceof CustomScorer) {
evaluationRun.setJudgmentScorers(List.of());
evaluationRun.setCustomScorers(List.of((com.judgmentlabs.judgeval.internal.api.models.BaseScorer) scorer));
} else {
evaluationRun.setJudgmentScorers(List.of(scorer.getScorerConfig()));
evaluationRun.setCustomScorers(List.of());
}
evaluationRun.setIsOffline(false);
evaluationRun.setIsBucketRun(false);
evaluationRun.setCreatedAt(Instant.now().atOffset(ZoneOffset.UTC).format(DateTimeFormatter.ISO_INSTANT));
return evaluationRun;
}
No description provided.