Skip to content

refactor(py): widen wrap_model middleware signature to ModelResponse | Operation - #6022

Open
huangjeff5 wants to merge 2 commits into
mainfrom
types/wrap-model-operation
Open

refactor(py): widen wrap_model middleware signature to ModelResponse | Operation#6022
huangjeff5 wants to merge 2 commits into
mainfrom
types/wrap-model-operation

Conversation

@huangjeff5

Copy link
Copy Markdown
Contributor

Product Context & Motivation

Genkit supports both standard synchronous/streaming models and long-running background models (such as video generation with Veo or Deep Research). Model middleware (such as logging, telemetry, or safety filters) needs to operate seamlessly across both foreground model responses (ModelResponse) and long-running operations (Operation[ModelResponse, Any]).

This PR widens the wrap_model middleware type annotations to allow background operation handles to pass through middleware pipelines cleanly without static type-checker errors.

Key Changes

  • Updated wrap_model type annotations in genkit._core._middleware to accept ModelResponse | Operation[ModelResponse, Any].
  • Aligns model middleware typing with the full spectrum of Genkit model execution modes.

Verification & Safety

  • Non-breaking type-system update; existing middleware functions for standard ModelResponse remain fully valid.
  • Verified with pytest py/packages/genkit/tests/genkit/ai/ (537 passed).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 updates the wrap_model method in _middleware.py to support background operations by widening its return type and its next_fn parameter's return type to ModelResponse | Operation. The reviewer notes that wrap_generate and its next_fn should also be widened to ModelResponse | Operation to prevent static type-checker errors, as background operations returned by wrap_model will propagate up through wrap_generate.

Comment on lines +271 to +272
next_fn: Callable[[ModelHookParams, GenerateMiddlewareContext], Awaitable[ModelResponse | Operation]],
) -> ModelResponse | Operation:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

While widening wrap_model to return ModelResponse | Operation is necessary to support background operations, wrap_generate (defined at line 258) and its next_fn are still typed to return only ModelResponse. Since wrap_generate wraps the tool loop iteration which executes the model call, any background operation returned by wrap_model will propagate up through wrap_generate's next_fn. This will cause static type-checker errors because next_fn is expected to return ModelResponse but will actually return ModelResponse | Operation at runtime. To ensure type safety across the entire pipeline, wrap_generate in both BaseMiddleware and MiddlewareDef should also be widened to return ModelResponse | Operation.

@huangjeff5 huangjeff5 changed the title types(py): widen wrap_model middleware signature to ModelResponse | Operation refactor(py): widen wrap_model middleware signature to ModelResponse | Operation Aug 12, 2026
cursor Bot pushed a commit to jeffdh5/genkit that referenced this pull request Aug 14, 2026
Coerce persisted generate() dumps, raise INVALID_ARGUMENT for bad Veo
config, advertise the remaining GenerateVideosConfig fields, and default
background models to longRunning. wrap_model next() stays a bare
Operation; genkit-ai#6022 should drop its assert.

Co-authored-by: jeffdh5 <jeffdh5@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant