fix: GoogleGenaiSamplingHandler leaks thought parts and gives unhelpful errors on empty responses (#3849)

* Fix GoogleGenaiSamplingHandler thought part leaking and unhelpful errors

- Filter thought parts (part.thought=True) from response content instead
  of leaking them as TextContent in _response_to_result_with_tools
- Include finish_reason in error messages when no content is found, so
  safety-filtered responses (SAFETY, RECITATION, etc.) are distinguishable
- Add specific error message for thinking-only responses in
  _response_to_create_message_result

Fixes #3846

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Add tests for thought part filtering and error message improvements

Tests cover:
- Thought parts filtered from tool-path responses
- Thought-only responses produce descriptive errors
- Safety-filtered responses include finish_reason in error
- Normal responses (text + function calls) unaffected

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix ruff format and ty check issues

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix ty check errors in tests

Remove unused ty: ignore comments from lines where isinstance() narrows
the type, and add correct ty: ignore[invalid-argument-type] and
ty: ignore[not-subscriptable] comments on lines in newly added test
functions where ty cannot infer the union type is a list. Also apply
ruff format fix in test_task_return_types.py.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix: use all() not any() for thinking-only detection

Addresses review feedback: any() would misclassify mixed responses
(thought + function_call) as thinking-only, hiding the real error.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: strawgate

src/fastmcp/client/sampling/handlers/google_genai.py

@@ -322,7 +322,18 @@ def _response_to_create_message_result(
     """Convert Google GenAI response to CreateMessageResult (no tools)."""
     if not (text := response.text):
         candidate = _get_candidate_from_response(response)
-        msg = f"No content in response: {candidate.finish_reason}"
+        # Check if the response only contained thinking
+        has_thoughts = (
+            candidate.content
+            and candidate.content.parts
+            and all(getattr(p, "thought", False) for p in candidate.content.parts)
+        )
+        if has_thoughts:
+            msg = (
+                "Model returned only thinking/reasoning content with no response text."
+            )
+        else:
+            msg = f"No content in response (finish_reason={candidate.finish_reason})"
         raise ValueError(msg)
 
     return CreateMessageResult(
@@ -365,7 +376,7 @@ def _response_to_result_with_tools(
     if candidate.content and candidate.content.parts:
         for part in candidate.content.parts:
             # Note: Skip thought parts from thinking_config - not relevant for MCP responses
-            if part.text:
+            if part.text and not part.thought:
                 content.append(TextContent(type="text", text=part.text))
             elif part.function_call is not None:
                 fc = part.function_call
@@ -380,7 +391,9 @@ def _response_to_result_with_tools(
                 )
 
     if not content:
-        raise ValueError("No content in response from completion")
+        finish = candidate.finish_reason if candidate else "unknown"
+        msg = f"No content in response from completion (finish_reason={finish})"
+        raise ValueError(msg)
 
     return CreateMessageResultWithTools(
         content=content,

tests/client/sampling/handlers/test_google_genai_handler.py

@@ -460,3 +460,142 @@ def test_convert_tool_strips_titles():
     assert "title" not in schema
     assert "title" not in schema["properties"]["query"]
     assert schema["properties"]["query"]["type"] == "string"
+
+
+# ────────────────────────────────────────────────────────────
+# Tests for thought-part filtering and improved error messages
+# (PR #3849)
+# ────────────────────────────────────────────────────────────
+
+
+def test_thought_parts_filtered_on_tool_path():
+    """Thought parts should be excluded; only the real text part should appear."""
+    mock_candidate = MagicMock(spec=Candidate)
+    mock_candidate.content = MagicMock()
+    mock_candidate.content.parts = [
+        Part(text="thinking about the problem...", thought=True),
+        Part(text="Here is the real answer"),
+    ]
+    mock_candidate.finish_reason = "STOP"
+
+    mock_response = MagicMock(spec=GenerateContentResponse)
+    mock_response.candidates = [mock_candidate]
+
+    result = _response_to_result_with_tools(mock_response, model="gemini-2.5-flash")
+
+    assert len(result.content) == 1  # ty: ignore[invalid-argument-type]
+    assert isinstance(result.content[0], TextContent)  # ty: ignore[not-subscriptable]
+    assert result.content[0].text == "Here is the real answer"  # ty: ignore[not-subscriptable]
+
+
+def test_thought_only_response_on_tool_path_raises():
+    """When the response contains ONLY thought parts the tool path should raise
+    a ValueError whose message includes the finish_reason."""
+    mock_candidate = MagicMock(spec=Candidate)
+    mock_candidate.content = MagicMock()
+    mock_candidate.content.parts = [
+        Part(text="deep thinking...", thought=True),
+    ]
+    mock_candidate.finish_reason = "STOP"
+
+    mock_response = MagicMock(spec=GenerateContentResponse)
+    mock_response.candidates = [mock_candidate]
+
+    with pytest.raises(ValueError, match="finish_reason=STOP"):
+        _response_to_result_with_tools(mock_response, model="gemini-2.5-flash")
+
+
+def test_thought_only_response_on_non_tool_path_raises():
+    """When the non-tool path receives only thoughts the error message should
+    mention 'thinking/reasoning content'."""
+    mock_candidate = MagicMock(spec=Candidate)
+    mock_candidate.content = MagicMock()
+    mock_candidate.content.parts = [
+        Part(text="internal reasoning...", thought=True),
+    ]
+    mock_candidate.finish_reason = "STOP"
+
+    mock_response = MagicMock(spec=GenerateContentResponse)
+    mock_response.text = None  # No real text available
+    mock_response.candidates = [mock_candidate]
+
+    with pytest.raises(ValueError, match="thinking/reasoning content"):
+        _response_to_create_message_result(mock_response, model="gemini-2.5-flash")
+
+
+def test_safety_filtered_response_on_tool_path_raises():
+    """A safety-filtered response (no parts) should raise with the finish_reason
+    included in the error message."""
+    mock_candidate = MagicMock(spec=Candidate)
+    mock_candidate.content = MagicMock()
+    mock_candidate.content.parts = []  # Empty parts after safety filtering
+    mock_candidate.finish_reason = "SAFETY"
+
+    mock_response = MagicMock(spec=GenerateContentResponse)
+    mock_response.candidates = [mock_candidate]
+
+    with pytest.raises(ValueError, match="finish_reason=SAFETY"):
+        _response_to_result_with_tools(mock_response, model="gemini-2.5-flash")
+
+
+def test_safety_filtered_response_on_non_tool_path_raises():
+    """A safety-filtered response on the non-tool path should include
+    finish_reason in the error."""
+    mock_candidate = MagicMock(spec=Candidate)
+    mock_candidate.content = MagicMock()
+    mock_candidate.content.parts = []
+    mock_candidate.finish_reason = "SAFETY"
+
+    mock_response = MagicMock(spec=GenerateContentResponse)
+    mock_response.text = None
+    mock_response.candidates = [mock_candidate]
+
+    with pytest.raises(ValueError, match="finish_reason=SAFETY"):
+        _response_to_create_message_result(mock_response, model="gemini-2.5-flash")
+
+
+def test_normal_response_text_and_function_call():
+    """A normal response with both real text and a function call should
+    produce both TextContent and ToolUseContent in the result."""
+    mock_candidate = MagicMock(spec=Candidate)
+    mock_candidate.content = MagicMock()
+    mock_candidate.content.parts = [
+        Part(text="Let me look that up."),
+        Part(function_call=FunctionCall(name="lookup", args={"q": "test"})),
+    ]
+    mock_candidate.finish_reason = "STOP"
+
+    mock_response = MagicMock(spec=GenerateContentResponse)
+    mock_response.candidates = [mock_candidate]
+
+    result = _response_to_result_with_tools(mock_response, model="gemini-2.5-flash")
+
+    assert len(result.content) == 2  # ty: ignore[invalid-argument-type]
+    assert isinstance(result.content[0], TextContent)  # ty: ignore[not-subscriptable]
+    assert result.content[0].text == "Let me look that up."  # ty: ignore[not-subscriptable]
+    assert isinstance(result.content[1], ToolUseContent)  # ty: ignore[not-subscriptable]
+    assert result.content[1].name == "lookup"  # ty: ignore[not-subscriptable]
+    assert result.content[1].input == {"q": "test"}  # ty: ignore[not-subscriptable]
+    assert result.stopReason == "toolUse"
+
+
+def test_thought_with_function_call_keeps_function_call():
+    """When thinking parts accompany a function call, only the function call
+    should appear in the content (thought parts filtered out)."""
+    mock_candidate = MagicMock(spec=Candidate)
+    mock_candidate.content = MagicMock()
+    mock_candidate.content.parts = [
+        Part(text="reasoning about what tool to use...", thought=True),
+        Part(function_call=FunctionCall(name="get_weather", args={"city": "NYC"})),
+    ]
+    mock_candidate.finish_reason = "STOP"
+
+    mock_response = MagicMock(spec=GenerateContentResponse)
+    mock_response.candidates = [mock_candidate]
+
+    result = _response_to_result_with_tools(mock_response, model="gemini-2.5-flash")
+
+    assert len(result.content) == 1  # ty: ignore[invalid-argument-type]
+    assert isinstance(result.content[0], ToolUseContent)  # ty: ignore[not-subscriptable]
+    assert result.content[0].name == "get_weather"  # ty: ignore[not-subscriptable]
+    assert result.stopReason == "toolUse"