Skip to content

fix(py/plugins/google-genai): camelCase embedder options and request chunking - #6302

Open
hilariie wants to merge 6 commits into
mainfrom
fix/py-google-genai-embedder-options-batching
Open

hilariie wants to merge 6 commits into
mainfrom
fix/py-google-genai-embedder-options-batching

Conversation

@hilariie

@hilariie hilariie commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #6187

Summary

Embedder options are validated through a typed schema that accepts both snake_case and camelCase keys (taskType/task_type, outputDimensionality/output_dimensionality, title, version), tolerates unknown keys, and is advertised as customOptions per backend. Vertex embedders additionally accept mimeType and autoTruncate. Invalid options raise GenkitError with INVALID_ARGUMENT naming the field.

Requests are chunked and reassembled in input order: 100 contents per call on the Gemini API, 250 on Vertex text-embedding models, and 1 on Vertex gemini-* models, which accept a single input text per request. The multimodal path sends one :predict call per document instead of rejecting multi-document requests.

EmbeddingTaskType gains CODE_RETRIEVAL_QUERY.

@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 introduces structured configuration schemas (EmbeddingConfigSchema and VertexEmbeddingConfigSchema) using Pydantic for Google GenAI and Vertex AI embedders. It also implements batching for text embedding requests and updates multimodal embedding to process multiple documents sequentially rather than rejecting them. The feedback recommends executing both text embedding batches and multimodal embedding requests concurrently using asyncio.gather with a semaphore to reduce latency, which also requires importing asyncio.

Comment thread py/packages/genkit-google-genai/src/genkit_google_genai/models/embedder.py Outdated
Comment thread py/packages/genkit-google-genai/src/genkit_google_genai/models/embedder.py Outdated

@cabljac cabljac 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.

The :predict request_dict assertions and the reverse-completion ordering test are the right shape. One fix, rest inline.

_run_bounded doesn't cancel its children when the outer generate() is cancelled. asyncio.wait doesn't propagate, so on abort or timeout the in-flight calls keep running and billing. The FIRST_EXCEPTION path is fine, it's just the outer-cancel case. BaseException handler, cancel pending, gather with return_exceptions, re-raise. Floor is 3.10 so no TaskGroup. Or drop the concurrency, which #6187 doesn't ask for anyway, and it goes away.

One that can't anchor inline: google.py:533 still types config as dict[str, object] while this PR exports the two schemas, so passing a typed instance is a pyright error even though _parse_options handles it. The dict typing predates this (#6104), the mismatch doesn't.

Also, not this PR: Go batches gemini-embedding-001 at 250 (embedder.go:35) against the Vertex doc's "each request can only include a single input text". Your 1 is right, Go's wrong.

#6187

Comment thread py/packages/genkit-google-genai/src/genkit_google_genai/models/embedder.py Outdated
@hilariie

Copy link
Copy Markdown
Contributor Author

Resolved all comments in a256a5a

@hilariie
hilariie requested a review from cabljac September 14, 2026 16:39

@cabljac cabljac 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.

The cancellation fix and its test are right, and the dropped-option warning is what I was after. One item from the last round is still open, it wasn't inline-anchored so easy to miss. Two nits inline.

google.py:533 still types config as dict[str, object] | None. Passing an EmbeddingConfigSchema instance is a pyright error even though _parse_options accepts it. EmbedderRef.config is Any, so widening to EmbeddingConfigSchema | dict[str, object] | None is enough. Same type on family_embedder_ref in _model_refs.py:110.

#6187

Comment thread py/packages/genkit-google-genai/test/models/googlegenai_embedder_test.py Outdated
extra = options.model_extra or {}
dropped = [name for name in VERTEX_ONLY_OPTION_NAMES if name in extra]
if dropped:
logger.warning(

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.

nit: this fires on every generate(). An indexing loop with mimeType in the ref config emits one warning per call. Warn-once per embedder instance would be quieter. Fine to leave if you prefer.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Leaving this one. _create_embedder_action builds a fresh Embedder inside _run on every request, so a per-instance flag would still warn every call. Happy to add a process-wide once if you would rather have it.

@hilariie

Copy link
Copy Markdown
Contributor Author

Sorry, missed that one last round. Done in 429d8781b: embedding() and family_embedder_ref take EmbeddingConfigSchema | dict[str, object] | None, with a test passing an instance through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(py/plugins/google-genai): embedder options are snake_case only and requests are unbatched

2 participants