wip: 0.5.0 - scorer updates and project scoped judgeval #19
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 introduces a significant architectural change to the Judgeval Java SDK by making the client project-scoped. This enhancement streamlines interactions with the Judgment platform by associating all operations with a specific project, improving organization and data isolation. The underlying API communication layer has been overhauled to support a new versioned API, expanding the SDK's capabilities for project, dataset, prompt, and custom scorer management. These changes aim to provide a more robust and feature-rich experience for developers using the Judgeval SDK. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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 significantly updates version 0.5.0, making the Judgeval client project-scoped and overhauling the internal API client and data models. The projectName is now a required top-level configuration, and the API client has been enhanced with new endpoints, improved logging, and better type safety. However, a security audit identified critical vulnerabilities. API clients are susceptible to path traversal and URL parameter injection due to insecure URL construction and a lack of proper encoding for dynamic parameters. Furthermore, the code generation script is vulnerable to code injection if run against a malicious OpenAPI spec. Addressing these requires implementing robust URL encoding in generated clients and proper string escaping in the code generation script. Additionally, the review noted a widespread removal of Javadoc, which is a regression in code quality impacting maintainability. There's also a minor issue with exception handling where an exception is caught and swallowed without logging. While the architectural changes are positive, the documentation loss and exception handling need to be resolved.
| } | ||
|
|
||
| public DeleteProjectResponse deleteProjects(String projectId) throws IOException, InterruptedException { | ||
| String url = buildUrl("/v1/projects/" + projectId); |
There was a problem hiding this comment.
The SDK constructs API URLs by directly concatenating dynamic parameters such as projectId, datasetName, and traceId into the URL path. If these parameters contain path traversal sequences (e.g., ../), an attacker could potentially access or manipulate unintended API endpoints. It is recommended to URL-encode all dynamic path segments using URLEncoder.encode(value, StandardCharsets.UTF_8) before appending them to the URL path.
| public CompletableFuture<ScorerExistsResponse> scorerExists(ScorerExistsRequest payload) { | ||
| String url = buildUrl("/scorer_exists/"); | ||
| public CompletableFuture<DeleteProjectResponse> deleteProjects(String projectId) { | ||
| String url = buildUrl("/v1/projects/" + projectId); |
There was a problem hiding this comment.
The SDK constructs API URLs by directly concatenating dynamic parameters such as projectId into the URL path. If these parameters contain path traversal sequences (e.g., ../), an attacker could potentially access or manipulate unintended API endpoints. It is recommended to URL-encode all dynamic path segments using URLEncoder.encode(value, StandardCharsets.UTF_8) before appending them to the URL path.
| placeholder = f"{{{param['name']}}}" | ||
| idx = remaining.index(placeholder) | ||
| if idx > 0: | ||
| parts.append(f'"{remaining[:idx]}"') |
There was a problem hiding this comment.
The generate_url_expr function embeds strings directly from the OpenAPI spec into Java code without escaping double quotes. A malicious OpenAPI spec could exploit this to inject arbitrary Java code into the generated client files. Proper escaping should be implemented for all strings retrieved from the OpenAPI spec before they are used in the generated Java source code.
| * Main entry point for the Judgment SDK. Provides access to tracer, scorer, and | ||
| * evaluation factories. | ||
| */ | ||
| public class Judgeval { |
There was a problem hiding this comment.
The Javadoc for the Judgeval class and its public methods (tracer, scorers, evaluation, builder) has been removed. This reduces the clarity and usability of the SDK for developers. Please consider restoring the Javadoc to document the purpose and usage of these public APIs.
| } catch (Exception e) { | ||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
Catching a generic Exception and returning an empty Optional without logging can hide important errors during project ID resolution, such as network issues or authentication failures. This makes debugging difficult. It's recommended to log the exception to provide visibility into what went wrong.
} catch (Exception e) {
Logger.error("Failed to resolve project ID for project '" + name + "'.", e);
return Optional.empty();
}| /** | ||
| * Factory for creating evaluation builders. | ||
| */ | ||
| public final class EvaluationFactory { |
There was a problem hiding this comment.
The Javadoc for EvaluationFactory and its create method has been removed. Please restore it to maintain code clarity and document the factory's role in creating Evaluation instances.
| /** | ||
| * Factory for creating scorer builders and accessing scorer types. | ||
| */ | ||
| public final class ScorersFactory { |
There was a problem hiding this comment.
The Javadoc for ScorersFactory and its methods has been removed. Please restore the documentation to explain the purpose of this factory and its methods for creating different types of scorers.
| * @see <a href="https://docs.judgment.ai/judgeval/cli/upload-scorers">Judgment | ||
| * Docs: Upload Scorers</a> | ||
| */ | ||
| public final class CustomScorer extends APIScorer { |
There was a problem hiding this comment.
The class-level Javadoc explaining what a CustomScorer is and how it's used has been removed. This information is valuable for developers using the SDK. Please consider restoring it, including the link to the documentation.
| */ | ||
| import java.util.Optional; | ||
|
|
||
| public final class CustomScorerFactory { |
There was a problem hiding this comment.
The Javadoc for CustomScorerFactory and its get methods has been removed. Please restore it to document how to create custom scorers, which is important for SDK usability.
| /** | ||
| * Factory for creating tracer builders. | ||
| */ | ||
| public final class TracerFactory { |
There was a problem hiding this comment.
The Javadoc for TracerFactory and its create method has been removed. Please restore it to explain the factory's purpose and how it's used to create Tracer instances.
1 participant
No description provided.