-
Notifications
You must be signed in to change notification settings - Fork 191
feat: Add Gemini "Thinking" support to GoogleGenAIChatGenerator #2212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
My impression is that handling thinking + tool calls have some peculiarities to take into consideration |
Ok let me check |
...le_genai/src/haystack_integrations/components/generators/google_genai/chat/chat_generator.py
Outdated
Show resolved
Hide resolved
...le_genai/src/haystack_integrations/components/generators/google_genai/chat/chat_generator.py
Show resolved
Hide resolved
...le_genai/src/haystack_integrations/components/generators/google_genai/chat/chat_generator.py
Show resolved
Hide resolved
...le_genai/src/haystack_integrations/components/generators/google_genai/chat/chat_generator.py
Show resolved
Hide resolved
...le_genai/src/haystack_integrations/components/generators/google_genai/chat/chat_generator.py
Show resolved
Hide resolved
@@ -797,6 +808,115 @@ def test_live_run_with_parallel_tools(self, tools): | |||
# Check that the response mentions both temperature readings | |||
assert "22" in message.text or "15" in message.text | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add unit tests to cover the new methods and code paths introduced in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the CI, I see this coverage
Name Stmts Miss Branch BrPart Cover Missing
---------------------------------------------------------------------------------------------------------------------------------
src/haystack_integrations/components/generators/google_genai/chat/chat_generator.py 338 53 156 32 80% 77-78, 99-106, 108->118, 110->118, 123->128, 136-137, 141-142, 145-150, 161-163, 184-188, 195-205, 228-229, 275->309, 278->309, 305->279, 542, 564->568, 611-616, 631, 670->676, 672->671, 677-681, 684->666, 692, 697, 737-739, 772-774, 850-851, 862, 866, 904-905, 959-960, 971, 975, 1013-1014
Could you please add some additional unit tests to cover what changed in this PR?
For example, _convert_message_to_google_genai_format
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice nice, on it! One sec 🙏
@julian-risch @anakin87 is this one good to go? LMK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok for me.
My impression is that @julian-risch wants to review as well.
@julian-risch would you please take a look - I'd like to integrate this one and release a new version soon |
@julian-risch I think @anakin87 is a very good reviewer so I took the liberty to move this PR forward. |
Included in https://pypi.org/project/google-genai-haystack/2.1.2/ release |
Why:
Introduces support for Gemini "thinking/reasoning" .Models can now show their reasoning process, maintain thought context across multi-turn conversations, and use configurable thinking budgets.
What:
thinking_budget
configuration viageneration_kwargs
with model-specific validationReasoningContent
objectsHow can it be used:
How did you test it:
Notes for the reviewer:
_aggregate_streaming_chunks_with_reasoning
extends the generic aggregator (40 lines vs 78 if duplicated)