-
Notifications
You must be signed in to change notification settings - Fork 636
Elisa/fix(py)/update genai sample flows #4123
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
Conversation
Co-authored-by: Mengqin Shen <mengqin@google.com>
feat(py/genkit): add define_partial for Handlebars partials Expose define_partial in GenkitRegistry class for parity with JS SDK. Allows programmatic registration of reusable template fragments.
… veneer_test.py (#4091)
# Conflicts: # py/packages/genkit/src/genkit/blocks/generate.py # py/samples/google-genai-hello/src/main.py
Summary of ChangesHello @MengqinShen, 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 significant advancements to the Genkit framework, enhancing both its Go and Python ecosystems. The Go implementation gains a robust, experimental session management API and comprehensive documentation across its core packages, improving clarity and developer experience. Concurrently, the Python side expands its model integration with a new DeepSeek plugin and refines its development workflow by adopting 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 is a substantial pull request that introduces several new features and improvements across the Go, Python, and JavaScript SDKs. Key additions include expanded sample flows for Google GenAI, a new DeepSeek plugin for Python, and an experimental session management package for Go. The Go packages have also been significantly improved with extensive godoc documentation. The changes are well-structured and include corresponding tests and samples for the new functionalities.
I've identified a couple of areas for improvement. One is related to the logic for handling tool_choice in the OpenAI-compatible plugin, which might lead to unexpected behavior. The other is a minor limitation in a Go utility function for extracting JSON from markdown. Overall, this is a high-quality contribution.
| var jsonMarkdownRegex = regexp.MustCompile("(?si)```json\\s*(.*?)```") | ||
|
|
||
| // plainMarkdownRegex matches fenced code blocks without any language identifier. | ||
| var plainMarkdownRegex = regexp.MustCompile("(?s)```\\s*\\n(.*?)```") |
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 regex for plain markdown blocks, plainMarkdownRegex, requires a newline after the opening fence (```). This means it will not match a fenced code block where the content starts on the same line, such as ```{"foo": "bar"}```, which is valid markdown. This could lead to failures in extracting JSON from such formatted strings.
| if any(msg.role == Role.TOOL for msg in request.messages): | ||
| # After a tool response, stop forcing additional tool calls. | ||
| openai_config['tool_choice'] = 'none' | ||
| elif request.tool_choice: | ||
| openai_config['tool_choice'] = request.tool_choice |
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 logic to force tool_choice to 'none' when a tool response is present in the message history is a good heuristic to prevent tool-calling loops. However, it overrides any user-specified tool_choice which might be unexpected. For example, if a user explicitly sets tool_choice: 'required', this logic will change it to 'none', preventing the model from calling a required tool.
Consider making this behavior conditional. For instance, only force tool_choice to 'none' if the original request.tool_choice is not set or is 'auto'. If it's 'required' or a specific tool definition, it might be better to respect the user's explicit instruction.
This PR is to add extra flows in google-genai sample to match that in JS SDK.