fix(VertexAI): use :batchEmbedContents for gemini-embedding models - #14284
fix(VertexAI): use :batchEmbedContents for gemini-embedding models#14284Veer Jain (veerjain-1) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Vertex AI embedding generation for gemini-embedding-* models by switching VertexAIEmbeddingClient from the legacy :predict endpoint to :batchEmbedContents (with the correct request/response shapes) while keeping :predict for older embedding models.
Changes:
- Route
gemini-embedding-*models to:batchEmbedContentsand deserialize the correspondingembeddings[].valuesresponse format. - Add new request/response DTOs for the Vertex AI
batchEmbedContentspayload. - Add unit tests and test data validating endpoint selection, payload shape, and response parsing for gemini embedding models.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/src/Connectors/Connectors.Google/Core/VertexAI/VertexAIEmbeddingClient.cs | Selects :batchEmbedContents vs :predict based on model id and processes the corresponding response format. |
| dotnet/src/Connectors/Connectors.Google/Core/VertexAI/VertexAIEmbedContentRequest.cs | Introduces the requests[] payload shape required by batchEmbedContents. |
| dotnet/src/Connectors/Connectors.Google/Core/VertexAI/VertexAIEmbedContentResponse.cs | Introduces the embeddings[] response model for batchEmbedContents. |
| dotnet/src/Connectors/Connectors.Google.UnitTests/TestData/vertex_embed_content_response.json | Adds fixture JSON for the batchEmbedContents response format. |
| dotnet/src/Connectors/Connectors.Google.UnitTests/Core/VertexAI/VertexAIClientEmbeddingsGenerationTests.cs | Adds tests ensuring correct endpoint selection, payload structure, and response parsing for gemini embedding models. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): 95c1cd0e7abf
Model: claude-opus-4.8
Overview
The PR cleanly routes gemini-embedding-* models to the :batchEmbedContents endpoint with new internal request/response DTOs while preserving the legacy :predict path, gated by a single construction-time decision that keeps the endpoint suffix and payload shape in lockstep. Routing, dimension threading, and strict ([JsonRequired]) response deserialization are well guarded and covered by new unit tests. The one residual risk is that each requests[] item in the new Vertex batch body omits the per-request model field that the sibling GoogleAI batchEmbedContents implementation deliberately sends, which can cause the exact runtime rejection this PR aims to eliminate. The stubbed test handler cannot catch that live-API contract gap.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 high) across 1 file. Details are attached to the affected lines below.
Affected areas: dotnet/src/Connectors/Connectors.Google/Core/VertexAI/VertexAIEmbedContentRequest.cs
|
|
||
| public static VertexAIEmbedContentRequest FromData(IEnumerable<string> data, int? dimensions = null) => new() | ||
| { | ||
| Requests = data.Select(text => new EmbedContentRequestItem |
There was a problem hiding this comment.
Each item in the :batchEmbedContents request body is built with only content (and optional outputDimensionality) and never sets a per-request model. The sibling GoogleAI implementation for this same endpoint sets it explicitly (GoogleAIEmbeddingRequest.cs:39, Model = $"models/{modelId}"), because the batchEmbedContents schema treats each requests[] entry as a full embed request whose model is required. If Vertex enforces the same requirement, every gemini-embedding-* call will fail at runtime with an HTTP 400 — the same class of error this PR is meant to fix — and the stubbed test handler cannot detect it since it never validates the body against the live API.
Populate a model field on each EmbedContentRequestItem with the fully-qualified Vertex resource name (projects/{projectId}/locations/{location}/publishers/google/models/{modelId}), or confirm against the Vertex batchEmbedContents REST reference that omission is accepted and document that decision. Please verify against a live endpoint before merge.
|
@microsoft-github-policy-service agree |
Resolves #14265
This PR updates
VertexAIEmbeddingGeneratorto use the:batchEmbedContentsendpoint and corresponding payload format forgemini-embedding-*models, fixing the400 FAILED_PRECONDITIONerror while retaining the legacy:predictpath for older models.