fix: use tool_responses role for gemma4 models in LiteLLM integration#5655
Open
jfrometa88 wants to merge 3 commits intogoogle:mainfrom
Open
fix: use tool_responses role for gemma4 models in LiteLLM integration#5655jfrometa88 wants to merge 3 commits intogoogle:mainfrom
jfrometa88 wants to merge 3 commits intogoogle:mainfrom
Conversation
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.
Link to Issue or Description of Change
Closes: #5650
Problem
Gemma4 models served via Ollama do not recognise
role: "tool"in the message history. When tool results were appended using this role, the model entered an infinite loop re-calling the tool instead of generating a final response.Two separate code paths were affected:
_content_to_message_param— builds tool result messages fromfunction_responseparts._ensure_tool_results— heals incomplete histories by injecting placeholder messages when a tool call has no matching result. This function had both the trigger condition and the fallback block hardcoded torole: "tool", so it neither recognised incomingtool_responsesmessages as valid resolutions nor generated the correct role in the placeholder it injected.Solution
Introduced a model-aware role selector in both affected functions. When the model name contains
gemma4, the role is set to"tool_responses"; otherwise it falls back to the standard"tool".In
_content_to_message_param:tool_role = "tool_responses" if "gemma4" in model.lower() else "tool"
In
_ensure_tool_results, a singleexpected_tool_rolevariable is computed once before the loop and reused consistently in three places: the healing trigger condition, theelifbranch that clears resolved call IDs, and the post-loop fallback block. This eliminates the previous dual-condition pattern where the twoifblocks were not mutually exclusive and could both fire on the same message.Testing Plan
Unit Tests:
Added
tests/unittests/test_lite_llm_gemma_tool_role.py.Covers:
gemma4, the generated message usesrole: "tool_responses"instead ofrole: "tool".function_responseparts are present in a single content object.role: "tool"is preserved for all other model names.Full test suite result: 5709 passed, 2300 warnings.
Manual End-to-End (E2E) Tests:
Setup:
Steps:
Before the fix for gemma4 models: history contained both a phantom role: "tool" error message and the real role: "tool_responses" result, causing the model to process the error last and loop.
After the fix: history contains only the real role: "tool_responses" result.
Checklist