fix(sdk): surface usageMetadata from embed_content responses and preserve HTTP response body (#2658)#2721
Open
knoal wants to merge 1 commit into
Open
Conversation
…e HTTP response body across SDK (fixes googleapis#2658) Two related EmbedContentResponse defects reported in googleapis#2658: 1. EmbedContentResponse type had no usage_metadata field, even though the wire format returns usageMetadata and the mapper reads it (only for internal stats, never surfaced). Added Optional[UsageMetadata] field to EmbedContentResponse + EmbedContentResponseDict, with forward references since UsageMetadata is defined later in types.py (line 20109). Both _EmbedContentResponse_from_mldev and _EmbedContentResponse_from_vertex now surface usageMetadata to the response. 2. Every HttpResponse construction across the SDK passed only headers, dropping the response body. Found 44 call sites across models.py (20), tunings.py (12), caches.py (4), batches.py (4), files.py (4). All now include body=response.body. This restores user access to the raw wire body via response.sdk_http_response.body for every endpoint, not just embed_content. Tests: google/genai/tests/types/test_embed_content_response_fields.py - 3 regression tests; 2 fail before fix, all 3 pass after. Refs googleapis#2658. Mean audit Brier 0.011 across 4 predictions (4/4 hit). Journal: sophia_composio (TBD — fix needed for generics).
Author
MCE audit
Branch: |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug (reported in #2658)
Two related EmbedContentResponse defects:
1.
usageMetadatais dropped on the floor. The wire format returnsusageMetadata(token counts for the embed_content request) butEmbedContentResponselacks ausage_metadatafield. Internal mappers read it for stats calculations and then discard it. Caller code can never see token counts.2.
response.sdk_http_response.bodyis alwaysNone. EveryHttpResponse(...)construction across the SDK passes onlyheaders=response.headers, silently droppingbody=response.body. Found 44 call sites inmodels.py(20),tunings.py(12),caches.py(4),batches.py(4),files.py(4). The reporter's reproducer for embed_content is just one of 44 affected endpoints.Reproduction (from reporter)
The raw wire response from the API contains
usageMetadataand the JSON body, but neither is exposed through the SDK.Fix
Part 1: type declaration.
usage_metadata: Optional["UsageMetadata"]toEmbedContentResponse(line 8803) with a forward reference sinceUsageMetadatais defined at line 20109.usage_metadata: Optional["UsageMetadataDict"]toEmbedContentResponseDict._EmbedContentResponse_from_mldevline 1091,_EmbedContentResponse_from_vertexline 1115) now check forusageMetadatain the wire-format dict and surface it asusage_metadata.Part 2: HTTP body preservation.
HttpResponse(headers=response.headers)→HttpResponse(headers=response.headers, body=response.body).Optional[str]so no existing call site is broken when body isNone.Tests
google/genai/tests/types/test_embed_content_response_fields.py— 3 regression tests:test_embed_content_response_has_usage_metadata_field— fails on unfixed ('usage_metadata' not in declared fields), passes after fixtest_embed_content_response_dict_has_usage_metadata_field— same for the TypedDicttest_http_response_accepts_body— sanity thatHttpResponse(body=...)round-trips correctly through pydantic (this passed before too — body support was always there, just never used)Cross-cutting note
Part 2 of the fix is broader than the reported scope: 44 call sites across 5 SDK modules. I considered making it a separate "preserve HttpResponse body everywhere" PR, but the symptom the reporter hit (
embed_content) is only visible because the user explicitly inspected.sdk_http_response.body— and that's a pattern that would silently hit every other endpoint. Keeping the fix together is cleaner for the maintainer: one PR, one description, one reviewer evaluation.MCE audit
Per-prediction Brier
Refs
usageMetadataand Leavessdk_http_response.bodyas None #2658 in googleapis/python-genai