feat: add new middleware package including fallback, filesystem, and retry, and update core AI modules to support it.#4542
Conversation
…retry, and update core AI modules to support it.
Summary of ChangesHello @pavelgj, 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 significantly enhances the robustness and flexibility of Genkit's AI interactions by introducing a dedicated middleware package. It provides essential capabilities such as intelligent model fallback for handling transient errors, automatic request retries with exponential backoff, and a secure mechanism for LLMs to interact with the local filesystem. These additions empower developers to build more resilient and capable AI applications within the Genkit framework. 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
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.
Code Review
This pull request introduces a new @genkit-ai/middleware package, which includes three useful middleware: fallback, filesystem, and retry. The implementations are well-structured and come with good test coverage, including security considerations for the filesystem middleware. I have a few suggestions for the retry middleware to make its behavior safer and more conventional, specifically regarding its default error handling and jitter calculation. I've also suggested an improvement for the package's README to make it more informative for users. Overall, this is a great addition to Genkit's capabilities.
| } else { | ||
| shouldRetry = true; | ||
| } |
There was a problem hiding this comment.
The current implementation retries on any error that is not a GenkitError. This is a very broad policy and can lead to retrying non-transient errors like TypeError or other programming bugs, potentially hiding issues and causing unnecessary retries. It's generally safer to only retry on known transient errors.
I suggest removing this else block to only retry on the specified GenkitError statuses. If retrying on other errors is desired, it could be an explicit opt-in feature. Note that this change will require updating the should retry on a non-GenkitError test case in tests/retry_test.ts.
| # Genkit Middleware | ||
|
|
||
| This package provides middleware for Genkit. |
There was a problem hiding this comment.
This README is very minimal. To improve usability for developers, please expand it to include:
- A list of the middleware provided in this package (
fallback,filesystem,retry). - A brief description of what each middleware does.
- Basic usage examples for each middleware, perhaps linking to the files in the
examples/directory.
| onError?.(error, i + 1); | ||
| let delay = currentDelay; | ||
| if (!noJitter) { | ||
| delay = delay + 1000 * Math.pow(2, i) * Math.random(); |
There was a problem hiding this comment.
The jitter calculation is unconventional. The jitter amount is based on a hardcoded 1000ms and grows exponentially, which can be disproportionate to the configured delay. A more standard approach is to make jitter proportional to the current backoff delay.
I suggest using a method like 'decorrelated jitter' to prevent a 'thundering herd' of clients. The suggested code will apply jitter between 50% and 100% of the calculated delay. This change would also require updating the should apply jitter test in tests/retry_test.ts.
| delay = delay + 1000 * Math.pow(2, i) * Math.random(); | |
| delay = delay / 2 + Math.random() * (delay / 2); |
…irectly invoke fallback models.
Intercept errors thrown by filesystem tools (e.g., `readFile`, `listFiles`) and convert them into user messages injected into the conversation context. This prevents the execution from crashing on errors like ENOENT and allows the model to react to the failure details. - Add middleware logic to catch tool errors and queue them as user messages. - Inject queued error messages into the prompt during the generate phase. - Add robustness tests to verify error handling and message flow.
- Update `readFile` tool to detect mime types using the newly added `mime` dependency. - Handle image files by reading them as base64 and injecting them as media parts into the conversation. - Wrap text file content in `<read_file>` XML tags for clearer context delineation. - Refactor message queue logic to append tool outputs and errors to the current user message turn instead of creating new turns. - Update filesystem example configuration with `maxTurns`.
Checklist (if applicable):