fix(genai): Support cached_content for Google context caching#1987
Merged
jxnl merged 2 commits into567-labs:mainfrom Jan 8, 2026
Merged
fix(genai): Support cached_content for Google context caching#1987jxnl merged 2 commits into567-labs:mainfrom
jxnl merged 2 commits into567-labs:mainfrom
Conversation
When using Google's explicit context caching, the API requires that `system_instruction`, `tools`, and `tool_config` are NOT set in the request - they should already be part of the cached content. This PR: - Extracts `cached_content` from user-provided GenerateContentConfig - Skips adding `system_instruction`, `tools`, `tool_config` when `cached_content` is present - Adds `cached_content` to the list of config fields that get merged Fixes issues similar to 567-labs#1759 (labels not preserved) See: https://ai.google.dev/gemini-api/docs/caching
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to b4de631 in 1 minute and 0 seconds. Click for details.
- Reviewed
261lines of code in2files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. instructor/providers/gemini/utils.py:350
- Draft comment:
Good addition: 'cached_content' is now included in the config fields merge. This ensures that any user-provided cached content is propagated correctly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold85%None
2. instructor/providers/gemini/utils.py:887
- Draft comment:
The extraction of 'cached_content' (along with 'thinking_config') from the user config is implemented clearly. Consider refactoring this repeated extraction logic into a helper function to adhere to DRY principles. - Reason this comment was not posted:
Confidence changes required:33%<= threshold85%None
3. instructor/providers/gemini/utils.py:1000
- Draft comment:
The conditional block that omits tools, tool_config, and system_instruction when 'cached_content' is present is well-commented and clear. Consider consolidating similar extraction logic between the structured outputs and tools handlers for maintainability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold85%None
4. tests/test_genai_config_merging.py:240
- Draft comment:
Test 'test_update_genai_kwargs_config_object_cached_content' effectively verifies that 'cached_content' is extracted from the config object. Good coverage for this new feature. - Reason this comment was not posted:
Confidence changes required:0%<= threshold85%None
5. tests/test_genai_config_merging.py:285
- Draft comment:
The tests for 'handle_genai_structured_outputs' and 'handle_genai_tools' properly check that attributes like system_instruction, tools, and tool_config are omitted when 'cached_content' is provided. This comprehensive test coverage reinforces the intended behavior. - Reason this comment was not posted:
Confidence changes required:0%<= threshold85%None
Workflow ID: wflow_22PHFL1GIzlh6bW5
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Summary
When using Google's explicit context caching, the API requires that
system_instruction,tools, andtool_configare NOT set in the request - they should already be part of the cached content.This PR fixes
cached_contentpropagation for Google GenAI by:cached_contentfrom user-providedGenerateContentConfigsystem_instruction,tools,tool_configwhencached_contentis presentcached_contentto the list of config fields that get mergedRelated Issues
Documentation
Test plan
cached_contentextractionsystem_instruction/tools/tool_configare skipped whencached_contentis presentImportant
Fix handling of
cached_contentin Google GenAI context caching by skipping certain fields and adding tests.handle_genai_structured_outputs()andhandle_genai_tools(), skip settingsystem_instruction,tools, andtool_configifcached_contentis present.cached_contentfromGenerateContentConfiginupdate_genai_kwargs()and add it toconfig_fields_to_merge.test_genai_config_merging.pyto verifycached_contentextraction and behavior when present.system_instruction,tools, andtool_configare not set whencached_contentis provided.cached_contentdoes not override existing base config values.This description was created by
for b4de631. You can customize this summary. It will automatically update as commits are pushed.