-
Notifications
You must be signed in to change notification settings - Fork 623
feat(java): genkit java #3996
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
feat(java): genkit java #3996
Conversation
Summary of ChangesHello @xavidop, 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 marks a significant expansion of the Genkit framework by introducing a full-fledged Java SDK. It provides Java developers with the necessary tools and abstractions to seamlessly integrate AI capabilities into their applications, covering everything from model interaction and tool orchestration to advanced features like Retrieval Augmented Generation (RAG), evaluation workflows, and stateful conversational experiences. The new SDK is designed with observability in mind, leveraging OpenTelemetry for comprehensive tracing and metrics. 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. Ignored Files
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a massive and impressive amount of work: the initial implementation of the Genkit Java SDK. The overall architecture is well-structured, with clear separation of concerns between the core, ai, telemetry, session, and evaluation packages. The use of core abstractions like Action and Registry, and patterns like Plugin for extensibility, lays a strong foundation. The inclusion of advanced features like multi-agent sessions, human-in-the-loop interrupts, and comprehensive telemetry from the start is commendable. The code is generally clean, well-documented, and follows good Java practices. I've found a couple of issues related to JSON schema definitions that need to be addressed, but otherwise, this is an excellent contribution.
| public Map<String, Object> getInputSchema() { | ||
| // Define the input schema for embedders | ||
| // This follows the EmbedRequestSchema from genkit-tools | ||
| Map<String, Object> schema = new HashMap<>(); | ||
| schema.put("type", "object"); | ||
|
|
||
| Map<String, Object> properties = new HashMap<>(); | ||
|
|
||
| // input: array of documents | ||
| Map<String, Object> inputProp = new HashMap<>(); | ||
| inputProp.put("type", "array"); | ||
| Map<String, Object> itemSchema = new HashMap<>(); | ||
| itemSchema.put("type", "object"); | ||
| Map<String, Object> itemProps = new HashMap<>(); | ||
| Map<String, Object> textProp = new HashMap<>(); | ||
| textProp.put("type", "string"); | ||
| textProp.put("description", "Text content to embed"); | ||
| itemProps.put("text", textProp); | ||
| itemSchema.put("properties", itemProps); | ||
| inputProp.put("items", itemSchema); | ||
| inputProp.put("description", "Array of documents to embed"); | ||
| properties.put("input", inputProp); | ||
|
|
||
| // options: optional configuration | ||
| Map<String, Object> optionsProp = new HashMap<>(); | ||
| optionsProp.put("type", "object"); | ||
| optionsProp.put("description", "Optional embedding configuration"); | ||
| properties.put("options", optionsProp); | ||
|
|
||
| schema.put("properties", properties); | ||
| schema.put("required", new String[]{"input"}); | ||
|
|
||
| return schema; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON schema defined for the input of the Embedder action appears to be incorrect. The schema describes the input as {"input": [{"text": "..."}]}, but the EmbedRequest class expects a structure closer to {"input": [{"content": [{"text": "..."}]}]} based on the Document and Part class definitions. This mismatch will likely cause deserialization errors when the action is invoked with JSON, for example from the Dev UI.
The schema should be updated to accurately reflect the structure of EmbedRequest containing a list of Document objects. The schema defined in Indexer.java for its documents property could be a good reference.
| Map<String, Object> numberItem = new HashMap<>(); | ||
| numberItem.put("type", "number"); | ||
| embeddingArrayProp.put("items", numberItem); | ||
| embeddingProps.put("embedding", embeddingArrayProp); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON schema for the output of the Embedder action has an incorrect property name. The schema uses "embedding" for the vector property, but the EmbedResponse.Embedding class uses "values" and is annotated with @JsonProperty("values"). This will lead to issues with schema validation and client-side tooling that relies on the schema. The property name in the output schema should be changed from "embedding" to "values" to match the class definition.
| embeddingProps.put("embedding", embeddingArrayProp); | |
| embeddingProps.put("values", embeddingArrayProp); |
|
moved to here: https://github.com/genkit-ai/genkit-java |
Description here... Help the reviewer by:
Checklist (if applicable):