Skip to content

feat(models): add LlmRequest.to_generate_content_body() - #6935

Open
cnpierrepapi wants to merge 1 commit into
google:mainfrom
cnpierrepapi:feat/llm-request-generate-content-body
Open

feat(models): add LlmRequest.to_generate_content_body()#6935
cnpierrepapi wants to merge 1 commit into
google:mainfrom
cnpierrepapi:feat/llm-request-generate-content-body

Conversation

@cnpierrepapi

@cnpierrepapi cnpierrepapi commented Aug 28, 2026

Copy link
Copy Markdown

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Opening this at @surajksharma07's go-ahead on that thread, which also settled that the helper belongs on LlmRequest in ADK rather than in genai.

Problem:

LlmRequest.config is a single flat GenerateContentConfig with 35 fields. On the wire those fields go to three different destinations, and nothing on the type says which is which:

top level (8):         cachedContent, labels, modelArmorConfig, safetySettings,
                       serviceTier, systemInstruction, toolConfig, tools
generationConfig (22): audioTimestamp, audioTranscriptionConfig, candidateCount,
                       frequencyPenalty, imageConfig, logprobs, maxOutputTokens,
                       mediaResolution, modelConfig, presencePenalty, responseLogprobs,
                       responseMimeType, responseModalities, responseSchema, routingConfig,
                       seed, speechConfig, stopSequences, temperature, thinkingConfig,
                       topK, topP
dropped (3):           httpOptions, automaticFunctionCalling, shouldReturnHttpResponse

Getting the split wrong does not raise. A custom BaseLlm that flattens config by hand and buries tools inside generationConfig still gets a 200 with a well formed response in it. The tools are simply absent, so the model answers from memory instead of calling anything, and it reads as a model that chose not to call the tool.

Solution:

LlmRequest.to_generate_content_body(), delegating to the same converters google-genai uses for its own built-in models (_GenerateContentParameters_to_vertex / _to_mldev) rather than keeping a hand written field list.

The delegation is the point. A documented table or a hardcoded list is a second copy of the mapping, and it goes stale the moment GenerateContentConfig gains a field — every custom BaseLlm in the wild then silently drops that field. I can vouch for the rot personally: the field table I first posted on #6880 said 6 fields go top level. It is 8. I had missed modelArmorConfig and serviceTier, and that list was less than a day old when I wrote it.

Three edge cases are handled, since each would bite anyone calling the converters directly:

  1. The converters return the model under a private _url key for the genai client to build the path from. It is routing information, not body content, and sending it is a 400. Stripped.
  2. model_selection_config lands in generationConfig as modelConfig. It is the only field that changes name in transit, so grepping your own config keys against the emitted body gives a false miss on exactly that one. Covered by a test.
  3. enable_enhanced_civic_answers raises ValueError in Vertex mode (Developer API only). Loud, which is right, but it surfaces from inside the genai converter and a BaseLlm author reading that traceback has no reason to connect it to a field they set several layers up. Re-raised with the LlmRequest.config field name and the target API attached, chained via from exc so the original is intact.

The helper takes an api_client when the caller has one, and falls back to a vertexai flag otherwise, so it stays usable from a BaseLlm that talks REST directly and never builds a genai client. Vertex and the Developer API differ in more than the URL — some fields exist on one and not the other — so that choice is explicit at the call site rather than inferred.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

13 new tests in tests/unittests/models/test_generate_content_body.py, run together with the existing test_llm_request.py:

$ pytest tests/unittests/models/test_generate_content_body.py \
         tests/unittests/models/test_llm_request.py -q
46 passed, 5 warnings in 1.81s

Environment: Python 3.12, google-genai 2.18.1, against main at 2c9c00d.

Coverage: tools landing top level rather than in generationConfig; temperature staying in generationConfig; the three client-only fields never reaching the wire; the _url strip; the full 33-field set splitting 8/22/3 with nothing unaccounted for; the model_selection_configmodelConfig rename; the Developer-API-only field being named in the error message; a field Vertex rejects being accepted on the Developer API; a string system_instruction becoming a content block; a request with no config at all; the method matching the free function; and the API mode being passed through.

The one that matters most for the rot argument: test_every_config_field_is_classified walks GenerateContentConfig.model_fields and fails if a field appears that lands in none of the three buckets. If genai adds a field and this helper cannot place it, CI says so instead of a user finding out through a silently dropped parameter.

Manual End-to-End (E2E) Tests:

Verified against live Vertex AI (gemini-2.5-flash, ADC, us-central1) before this was written up — a config with 33 of the 35 fields populated (omitting response_json_schema, which is mutually exclusive with response_schema, and enable_enhanced_civic_answers, which is Developer-API only) pushed through the converter produced exactly the 8/22/3 split above, with tools at the top level and no field silently lost. The unit tests reproduce that split offline, so no credentials are needed to check it.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

Formatted with pyink==25.12 (the version pinned in pyproject.toml) using the repo's own
[tool.pyink] config, run in a python:3.12-slim container. All three files come back
unchanged.

The Google CLA is signed.

LlmRequest.config is one flat GenerateContentConfig namespace, but its
fields go to three places on the wire: 8 at the top level, 22 inside
generationConfig, and 3 that are client-side only and rejected by the
endpoint. Nothing on the type says which is which, and getting it wrong
does not raise. A custom BaseLlm that buries tools in generationConfig
gets a 200 back with no function call in it, because none was offered.

Rather than document the split, delegate to the same converter
google-genai uses for its own built-in models, so there is one copy of
the mapping and it moves when GenerateContentConfig moves. A hand
written list is a second copy that goes stale the moment the type gains
a field.

Also handles three things that bite anyone calling the converter raw:
strips the private "_url" routing key, which is a 400 if sent as body;
covers the model_selection_config -> modelConfig rename, the only field
that changes name in transit; and re-raises genai's ValueError with the
LlmRequest.config field that caused it, so the error points at the line
that set it rather than at the converter.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A custom BaseLlm has no documented mapping from LlmRequest.config to a Vertex request body, and getting it wrong drops your tools with no error

2 participants