feat(py): MiddlewareV2 implementation + implement listValues API in reflection server#5045
feat(py): MiddlewareV2 implementation + implement listValues API in reflection server#5045huangjeff5 wants to merge 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive middleware system for Genkit, including new implementations for retries, fallbacks, media downloading, and system prompt simulation. The core generation logic has been refactored to support these hooks across model calls and tool executions. Feedback highlights several areas for improvement: the media downloader should raise errors for oversized content rather than truncating it and should reuse HTTP clients for efficiency; the retry logic requires better jitter management and removal of unreachable code; and the system prompt simulation should handle multiple system messages. Additionally, internal recursive calls in the generation action should be updated to prevent redundant telemetry spans.
py/packages/genkit/src/genkit/_core/_middleware/_download_request_media.py
Outdated
Show resolved
Hide resolved
py/packages/genkit/src/genkit/_core/_middleware/_download_request_media.py
Outdated
Show resolved
Hide resolved
py/packages/genkit/src/genkit/_core/_middleware/_simulate_system_prompt.py
Show resolved
Hide resolved
…Error on oversized media (instead of truncating), reuse httpx client, fix _generate.py recursive call to use internal _generate_action (no redundant telemetry spans), fix retry jitter to be capped by max_delay_ms + remove unreachable code, handle multiple system messages in simulate_system_prompt, and delete unused _middleware/__init__.py.
…dpoint. Passes ty, pyrefly, pyright, ruff, and pytest 3.10-3.14 (1045 tests). Changes: raise GenkitError on oversized media, reuse httpx client, fix _generate_action recursive call (no redundant spans), fix _generate_action signature type mismatch (BaseMiddleware vs ModelMiddleware), fix retry jitter + remove unreachable code, handle multiple system messages in simulate_system_prompt, delete unused _middleware/__init__.py, fix _base.py wrong import of GenerateActionOptions, add missing GenerateActionOptions import in generate_test.py, align /api/values with JS v1 (400 for missing or unsupported type param).
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Genkit middleware system from function-based to a more structured class-based approach, introducing a BaseMiddleware class and several new middleware implementations for functionalities like context augmentation, media download, retry, fallback, system prompt simulation, and request validation. The core generation logic (_ai/_generate.py) and prompt definitions (_ai/_aio.py, _ai/_prompt.py) have been updated to integrate with this new middleware architecture. Additionally, the reflection endpoints (_core/_reflection.py) and registry (_core/_registry.py) have been enhanced to support the new middleware and improve robustness, including changes to how /api/values handles different type parameters. Test files and sample code have been refactored to align with these changes. Feedback includes addressing a potential side effect in _generate_action where the middleware list is mutated before being copied, optimizing _DownloadRequestMediaMiddleware to avoid resource leaks by using a shared httpx.AsyncClient instance, and aligning the /api/values endpoint's type parameter handling with the PR description to return an empty dictionary for unknown types instead of a 400 error.
py/packages/genkit/src/genkit/_core/_middleware/_download_request_media.py
Outdated
Show resolved
Hide resolved
|
/gemini review |
| @@ -0,0 +1,121 @@ | |||
| # Copyright 2025 Google LLC | |||
There was a problem hiding this comment.
Code Review
This pull request introduces a class-based middleware architecture for Genkit, replacing the previous functional approach with BaseMiddleware hooks for generation, model calls, and tool execution. It includes several new built-in middleware components and enhances the reflection server's plugin initialization and value reporting. Review feedback suggests refactoring the reflection server to avoid hardcoded strings for supported types and recommends restoring informative error handling in the middleware sample to prevent suppressing diagnostic information.
| if type_param != 'defaultModel': | ||
| return JSONResponse( | ||
| {'error': f"'type' {type_param} is not supported. Only 'defaultModel' is supported"}, | ||
| status_code=400, | ||
| headers={'x-genkit-version': version}, | ||
| ) |
There was a problem hiding this comment.
To improve maintainability, consider defining the supported value types in a collection rather than hardcoding 'defaultModel'. This will make it easier to add more supported types in the future and makes the error message dynamic.
supported_types = ('defaultModel',)
if type_param not in supported_types:
return JSONResponse(
{'error': f"'type' {type_param} is not supported. Only {', '.join(supported_types)} are supported"},
status_code=400,
headers={'x-genkit-version': version},
)| except Exception: # noqa: S110 | ||
| pass |
There was a problem hiding this comment.
The broad except Exception: pass can hide important errors from users running this sample. The previous implementation, which caught the exception and printed a helpful message about the GEMINI_API_KEY, was more user-friendly. Please consider restoring a similar error handling mechanism to improve the developer experience.
| except Exception: # noqa: S110 | |
| pass | |
| except Exception as e: | |
| logger.error("Sample failed to run. If you see an auth error, check your GEMINI_API_KEY.", exc_info=e) |
Description
Implements middleware v2 for Python Genkit. Middleware can now intercept both the
generatecall (high-level, before request construction) and the raw model call, enabling composable pre/post-processing hooks.What's new
Middleware architecture
genkit._core._middlewarepackage with aBaseMiddlewareabstract base class exposing two hooks:wrap_generate(full generate pipeline) andwrap_model(raw model dispatch)SimulateSystemPrompt,AugmentWithContext,ValidateSupport,DownloadRequestMedia,Retry,Fallbackgenkit.middlewaremodule exports all built-in middlewareGenerate loop
generate_actionnow accepts amiddlewarelist and dispatches throughdispatch_generate/dispatch_modelchains_generate.pyrestructured around innerrun_one_iterationfunction wrapped by middleware hooks_middleware.pyremoved; logic moved into the new packageRegistry / reflection fixes
Registry.list_values(kind)now returns adict[str, object](name → value map)Registry.initialize_all_plugins()added to ensure plugins register their values before listingGenkitregisters the default model viaregister_value('defaultModel', ...)on startup/api/valuesendpoint: 400 for missing or unsupported type params ('type' X is not supported. Only 'defaultModel' is supported)/api/actionsand/api/valueswrapped in try/except with safe JSON serialization (default=str)Testing
generate_test.pyandmiddleware_test.pyupdated for new dispatch signaturesreflection_test.pyextended withlistValuesandinitialize_all_pluginscoveragepy/samples/middleware/src/main.pyupdated to demonstrate v2 API