Skip to content

feat(py/plugins/google-genai): raw Interactions HTTP client, converters, and options - #5855

Open
cabljac wants to merge 1 commit into
jh-interactions-1-corefrom
jh-interactions-2-transport
Open

feat(py/plugins/google-genai): raw Interactions HTTP client, converters, and options#5855
cabljac wants to merge 1 commit into
jh-interactions-1-corefrom
jh-interactions-2-transport

Conversation

@cabljac

@cabljac cabljac commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Second slice of the #5806 split (stacked on #5854): minimal async HTTP client for the Google AI Interactions API, request/response converters between Genkit and Interactions wire types, ClientOptions resolution, shared error-mapping utilities, and the google-genai >=2.14.0,<3 bump.

Part of the stack splitting #5806; recombined it reproduces that PR byte-for-byte. Co-authored with @huangjeff5.

Breaking changes

  • google-genai dependency floor moves from >=1.63.0 to >=2.14.0,<3 (major version bump) for the Interactions SDK types. Downstream projects pinned below 2.x will fail to resolve.

@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 support for the Google AI Interactions API within the genkit-google-genai package, adding HTTP client helpers, message converters, configuration options, and utility functions, along with corresponding unit tests. It also bumps the google-genai dependency requirement to version 2.14.0. The review feedback highlights three key issues: a bug in ensure_tool_ids that can cause tool request and response ID mismatches when some requests already have a ref, a control flow anti-pattern in cancel_interaction where an exception is raised on a successful path, and a potential crash in status_from_api_error if error.code is non-numeric.

Comment on lines +145 to +173
def ensure_tool_ids(messages: list[Message]) -> list[Message]:
"""Assign stable tool call IDs so wire payloads stay pairable."""
generated_ids: list[str] = []
next_id_counter = 0

new_messages = [message.model_copy(deep=True) for message in messages]

for message in new_messages:
for part in message.content:
root = part.root
if isinstance(root, ToolRequestPart) and root.tool_request and not root.tool_request.ref:
new_id = f'genkit-auto-id-{next_id_counter}'
next_id_counter += 1
root.tool_request.ref = new_id
generated_ids.append(new_id)

# Responses without refs reuse request IDs in order; unmatched ones get orphan IDs.
for message in new_messages:
for part in message.content:
root = part.root
if isinstance(root, ToolResponsePart) and root.tool_response and not root.tool_response.ref:
matched_id = generated_ids.pop(0) if generated_ids else None
if matched_id:
root.tool_response.ref = matched_id
else:
root.tool_response.ref = f'genkit-orphan-id-{next_id_counter}'
next_id_counter += 1

return new_messages

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

There is a bug in how tool request and response IDs are matched when some requests already have a ref while others do not.

Currently, generated_ids only stores the auto-generated IDs for requests that did not have a ref. When processing responses, any response without a ref pops from generated_ids in order. If a request earlier in the list already had a ref, the first response without a ref (which corresponds to that first request) will incorrectly consume the auto-generated ID of the second request, causing a complete mismatch of tool IDs.

To fix this, we should track the refs of all requests in order, and safely remove matched refs when processing responses that already have them.

def ensure_tool_ids(messages: list[Message]) -> list[Message]:
    '''Assign stable tool call IDs so wire payloads stay pairable.'''
    generated_ids: list[str] = []
    next_id_counter = 0

    new_messages = [message.model_copy(deep=True) for message in messages]

    for message in new_messages:
        for part in message.content:
            root = part.root
            if isinstance(root, ToolRequestPart) and root.tool_request:
                if not root.tool_request.ref:
                    new_id = f'genkit-auto-id-{next_id_counter}'
                    next_id_counter += 1
                    root.tool_request.ref = new_id
                generated_ids.append(root.tool_request.ref)

    # Responses without refs reuse request IDs in order; unmatched ones get orphan IDs.
    for message in new_messages:
        for part in message.content:
            root = part.root
            if isinstance(root, ToolResponsePart) and root.tool_response:
                if not root.tool_response.ref:
                    matched_id = generated_ids.pop(0) if generated_ids else None
                    if matched_id:
                        root.tool_response.ref = matched_id
                    else:
                        root.tool_response.ref = f'genkit-orphan-id-{next_id_counter}'
                        next_id_counter += 1
                elif root.tool_response.ref in generated_ids:
                    generated_ids.remove(root.tool_response.ref)

    return new_messages

Comment on lines +162 to +174
try:
await request(
method='POST',
url=url,
api_key=api_key,
client_options=client_options,
)
# Successful cancel often surfaces as CANCELLED; normalize both paths.
raise GenkitError(status='CANCELLED', message='successfully cancelled')
except GenkitError as error:
if error.status == 'CANCELLED':
return Interaction.model_validate({'id': interaction_id, 'status': 'cancelled'})
raise

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.

medium

Raising an exception (GenkitError) on the successful path just to catch it and return a normalized Interaction is a control flow anti-pattern. It also discards the actual successful response returned by the server, which may contain valuable metadata or fields.

Instead, we can return the response directly on success (optionally copying it to normalize the status to 'cancelled'), and only catch GenkitError with status 'CANCELLED' for cases where the server returned an error that represents a successful cancellation.

Suggested change
try:
await request(
method='POST',
url=url,
api_key=api_key,
client_options=client_options,
)
# Successful cancel often surfaces as CANCELLED; normalize both paths.
raise GenkitError(status='CANCELLED', message='successfully cancelled')
except GenkitError as error:
if error.status == 'CANCELLED':
return Interaction.model_validate({'id': interaction_id, 'status': 'cancelled'})
raise
try:
res = await request(
method='POST',
url=url,
api_key=api_key,
client_options=client_options,
)
return res.model_copy(update={'status': 'cancelled'})
except GenkitError as error:
if error.status == 'CANCELLED':
return Interaction.model_validate({'id': interaction_id, 'status': 'cancelled'})
raise

}
if candidate in valid:
return cast(StatusName, candidate)
return status_for_http_code(int(error.code or 0))

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.

medium

If error.code is a non-numeric string (e.g., a status name or string representation of an error), calling int(error.code) will raise a ValueError and crash the error mapping process. This is particularly problematic because it masks the original API error and makes debugging difficult.

We should wrap the conversion in a try-except block to safely fall back to 0 if parsing fails.

Suggested change
return status_for_http_code(int(error.code or 0))
try:
code = int(error.code or 0)
except (TypeError, ValueError):
code = 0
return status_for_http_code(code)

…rs, and options

Transport layer extracted from #5806: minimal async HTTP client for the
Google AI Interactions API, request/response converters between Genkit and
Interactions wire types, ClientOptions resolution, and shared error-mapping
utilities. Bumps google-genai to >=2.14.0,<3 for the Interactions SDK types.

Co-authored-by: Jeff Huang <huangjeff@google.com>
@cabljac
cabljac force-pushed the jh-interactions-2-transport branch from 5013ae9 to e079a08 Compare July 30, 2026 12:19
# Inline data URLs travel as base64; anything else is a reference the API fetches.
url = part.media.url
if url.startswith('data:'):
block['data'] = url[url.index(',') + 1 :]

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.

I think url[url.index(',') + 1:] can throw a bare ValueError: substring not found if a data URL arrives without a comma. The other failure modes in this function all raise descriptive errors, so a malformed media part could plausibly be a confusing one to debug. Something like:

if ',' not in url:
    raise ValueError('Malformed data URL for media part: missing payload separator')
block['data'] = url.split(',', 1)[1]

raise RuntimeError(
'Multimodal embedding relies on google-genai client internals that are '
'unavailable in the installed google-genai version; install google-genai>=1.63.0.'
'unavailable in the installed google-genai version; install google-genai>=2.3.0.'

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.

This message suggests google-genai>=2.3.0 but the floor this PR sets in pyproject is >=2.14.0,<3, so the suggested install can't satisfy the guard. The test in googlegenai_embedder_test.py asserts the same string. Both should move to 2.14.0, unless 2.3.0 is the real feature floor for the private transport.

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.

1 participant