feat: allow thinking_config in generate_content_config (#4108)

[Akshat8510]

Overview
This PR implements the feature request in #4108 to allow thinking_config to be set directly within generate_content_config, bringing the Python SDK in line with the Go implementation.

Changes

• llm_agent.py: Relaxed the validation logic in validate_generate_content_config to remove the ValueError for thinking_config.
• Precedence Warning: Added an override of model_post_init in LlmAgent to issue a UserWarning if both a planner and a manual thinking_config are provided.
• built_in_planner.py: Updated apply_thinking_config to log an INFO message when the planner overwrites an existing configuration on the LlmRequest.

Testing
Verified with a reproduction script covering:

1. Successful initialization of an agent with direct thinking_config.
2. Validation of UserWarning during initialization when conflicting configurations are present.
3. Confirmation of logger output when the planner performs an overwrite.

Closes: #4108
Tagging @invictus2010 for visibility.

[gemini-code-assist]

Summary of Changes

Hello @Akshat8510, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the LlmAgent by allowing thinking_config to be set directly within generate_content_config, mirroring the behavior of the Go SDK. It introduces a warning mechanism to inform users when a planner's configuration overrides a directly provided thinking_config, and adds logging to track when the built-in planner performs such an overwrite.

Highlights

• Direct thinking_config support: The generate_content_config now directly accepts thinking_config, aligning the Python SDK with the Go implementation.
• Validation relaxation: The ValueError that previously prevented setting thinking_config directly within generate_content_config has been removed.
• Precedence warning: A UserWarning is now issued if both a planner and a manual thinking_config are provided, clarifying that the planner's configuration will take precedence.
• Planner overwrite logging: The BuiltInPlanner now logs an INFO message when it overwrites an existing thinking_config on an LlmRequest.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly implements the feature to allow thinking_config in generate_content_config. The changes in llm_agent.py to relax validation and add a warning for conflicting configurations are well-implemented. However, I've found a couple of issues in built_in_planner.py related to logging that need to be addressed. Specifically, an incorrect logger is being imported from the venv module. I've also pointed out a minor issue with a duplicate import in llm_agent.py. Please see my detailed comments.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request correctly implements the feature to allow thinking_config to be set directly in generate_content_config. The changes are well-aligned with the description: the validation in LlmAgent is relaxed, a UserWarning is added for conflicting configurations at initialization, and an INFO log is added in BuiltInPlanner to signal when a user-provided configuration is being overwritten at runtime. The implementation is solid. I have one suggestion to improve the readability of the condition for the new warning.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a requested feature to allow thinking_config to be set directly in generate_content_config. The changes are well-implemented, removing the previous validation and adding appropriate warnings and logs to guide the user when conflicting configurations are provided.

I've added one suggestion to make the checks for existing configurations more explicit and robust by using is not None instead of relying on truthiness.

As a follow-up, you might want to update the docstring for LlmAgent.generate_content_config (line 288) which currently states that thinking_config must be configured via the planner. This is no longer strictly true with the changes in this PR.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This PR successfully enables thinking_config in generate_content_config. The logic is sound, and the UserWarning for conflicting settings is a thoughtful addition.

However, a critical issue exists: the unit tests have not been updated. Specifically, test_validate_generate_content_config_thinking_config_throw in tests/unittests/agents/test_llm_agent_fields.py will now fail as it expects a ValueError that is no longer raised. This test must be fixed.

Furthermore, new unit tests should be added to cover the new warning and logging behaviors to ensure long-term maintainability, rather than relying solely on a manual reproduction script.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request correctly implements the feature to allow thinking_config in generate_content_config. The changes are well-structured, including relaxing the validation, adding a UserWarning for conflicting configurations, and logging when a configuration is overwritten. My feedback includes a suggestion to improve the clarity of a docstring to better inform users about configuration precedence.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request effectively implements the requested feature to allow thinking_config directly in generate_content_config. The logic for handling precedence with a planner and providing a UserWarning is well-implemented. The addition of an informational log message when the planner's configuration takes precedence is also a nice touch for improving observability.

My main feedback, as detailed in the review comments, is the need to add unit tests for the new functionality. Additionally, please note that this change will likely break the existing test test_validate_generate_content_config_thinking_config_throw, which should be updated or removed. Ensuring comprehensive test coverage is crucial for maintaining code quality.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request successfully implements the requested feature to allow thinking_config in generate_content_config. The changes in llm_agent.py and built_in_planner.py are well-implemented, including the warning for conflicting configurations and the informational logging on overwrite. However, the associated test file, tests/unittests/agents/test_llm_agent_fields.py, has several critical issues that need to be addressed. There are multiple duplicated test functions, incorrectly indented tests that will cause syntax errors, and leftover code from a deleted test that will cause a runtime error. I've left specific comments on these issues. Please clean up the test file to ensure the test suite is correct and maintainable.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request successfully implements the requested feature to allow thinking_config in generate_content_config. The changes are logical, including the removal of the validation error and the addition of a warning for conflicting configurations. The new logging in the planner is also a good addition for transparency.

My review includes a suggestion to refactor a boolean check for better maintainability and points out duplicated test functions that should be removed. Overall, the implementation is solid and the accompanying tests are thorough, aside from the duplication.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request successfully implements the feature to allow thinking_config to be set directly in generate_content_config. The changes are logical and well-tested with new unit tests. I have two main points of feedback. First, an existing test that is now obsolete due to the changes needs to be removed to prevent CI failures. Second, a new test function has been placed inside a class definition, which harms readability and should be moved. Please see the detailed comments for specifics.

I am having trouble creating individual review comments. Click here to see my feedback.

src/google/adk/agents/llm_agent.py (852-853)

With the removal of this validation, the test test_validate_generate_content_config_thinking_config_throw in tests/unittests/agents/test_llm_agent_fields.py is now obsolete and will fail. Please ensure it is removed as part of this pull request.

tests/unittests/agents/test_llm_agent_fields.py (350-368)

This new test function test_builtin_planner_overwrite_logging has been inserted in the middle of the TestCanonicalTools class definition. While this is syntactically valid, it makes the code very difficult to read and maintain. Please move this function to be outside the class definition, for example, after the TestCanonicalTools class block concludes.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request successfully implements the requested feature to allow thinking_config within generate_content_config. The changes are logical, including the relaxed validation, the warning for conflicting configurations, and the informative logging when a planner overwrites a setting. The accompanying tests are thorough and cover the new behaviors well.

I have identified one critical issue in the new model_post_init method that could lead to an AttributeError during agent initialization under certain conditions. I've also noted a minor improvement for one of the new test functions. Please see the specific comments for details.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request successfully implements the ability to set thinking_config directly in generate_content_config, which is a great feature for consistency with other SDKs. The changes are well-implemented: the validation has been correctly relaxed, a helpful UserWarning is now issued in case of conflicting configurations, and the planner's behavior of overwriting the config is now clearly logged. The accompanying tests are thorough and cover all new logic paths, ensuring the changes are robust. My only suggestions are minor style improvements in the test file to maintain consistency.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request effectively implements the requested feature to allow thinking_config in generate_content_config. The changes are logical, with appropriate warnings for conflicting configurations and logging for overrides. The new functionality is also well-covered by unit tests. I have one suggestion to improve code clarity in built_in_planner.py by removing a redundant check.

[Akshat8510]

@gemini-code-assist/gemini review

[gemini-code-assist]

Code Review

This pull request successfully implements the feature to allow thinking_config to be set directly in generate_content_config. The changes are well-structured: the validation in LlmAgent is correctly relaxed, a helpful UserWarning is added for cases where both planner and generate_content_config provide a thinking_config, and the BuiltInPlanner now logs when it overwrites a configuration, improving transparency. The accompanying tests are thorough, covering the new success path, the warning mechanism, and the logging behavior. The code quality is high, and I have no further suggestions for improvement. Great work!

[Akshat8510]

I have addressed all the feedback from the gemini-code-assist bot, including refactoring the precedence checks, cleaning up imports, adding unit tests, and removing redundant logic. The bot has given a final approval with no further suggestions. Ready for final maintainer review! Thanks