fix: stringify OpenAI tool result content

[Fishbone23333]

Summary

• Convert OpenAI ChatCompletion tool result messages to string content instead of a list of content parts.
• Preserve multiple text content items by joining them with newlines.
• Add coverage for execute_tool_call() to prevent OpenAI-compatible providers from receiving list-valued tool message content.

Related Issue

Fixes #25

Verification

• uv run ruff format src\mcp_agent\workflows\llm\augmented_llm_openai.py tests\workflows\llm\test_augmented_llm_openai.py
• uv run ruff check src\mcp_agent\workflows\llm\augmented_llm_openai.py tests\workflows\llm\test_augmented_llm_openai.py
• uv run pytest tests\workflows\llm\test_augmented_llm_openai.py -q

Notes

• The focused OpenAI LLM test file passed with 25 passed.
• Warnings observed were existing Pydantic/global context warnings and pytest-asyncio loop-scope deprecation warnings.

Summary by CodeRabbit

• Bug Fixes

  • Fixed tool-call result serialization so multi-part results (text, images, embedded resources) are combined into a single, consistently formatted message, and empty results yield a clear placeholder.

• Tests

  • Added tests covering formatting for empty, plain text, image, and embedded-resource tool results and overall tool-call behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dfcf45a8-9ffc-4a3c-b73c-ce988c5bcc6d

📥 Commits

Reviewing files that changed from the base of the PR and between c89489a and bc893d9.

📒 Files selected for processing (2)

• src/mcp_agent/workflows/llm/augmented_llm_openai.py
• tests/workflows/llm/test_augmented_llm_openai.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/mcp_agent/workflows/llm/augmented_llm_openai.py

---

📝 Walkthrough

Walkthrough

This PR serializes MCP tool result content into a single newline-joined string via a new helper and updates execute_tool_call to set OpenAI tool message content to that string; tests and fixtures are updated to validate empty, text, image, and embedded resource cases.

Changes

Tool Result Serialization Fix

Layer / File(s)	Summary
Tool result content conversion src/mcp_agent/workflows/llm/augmented_llm_openai.py	New mcp_content_to_openai_tool_result_content helper serializes TextContent as text and JSON-encodes non-text parts, joining parts with newlines. execute_tool_call now sets ChatCompletionToolMessageParam.content to the resulting string.
Tool call execution test and infrastructure tests/workflows/llm/test_augmented_llm_openai.py	Adds MockableOpenAIAugmentedLLM (no-op stream), updates the mock_llm fixture, and adds tests that assert execute_tool_call returns a role="tool" message preserving tool_call_id and using the newline-joined string for various content types (empty, text, image, embedded resource).

Sequence Diagram(s)

(none)

Possibly Related PRs

• lastmile-ai/mcp-agent#341: Both PRs modify src/mcp_agent/workflows/llm/augmented_llm_openai.py’s conversion of MCP tool-result content for OpenAI: #341 changes the structure of image (and embedded resource) content emitted by mcp_content_to_openai_content_part, which the main PR then reuses/serializes when building the tool message content.
• lastmile-ai/mcp-agent#330: Both PRs modify OpenAIAugmentedLLM.execute_tool_call in src/mcp_agent/workflows/llm/augmented_llm_openai.py to change how empty/no-content MCP tool results are handled and serialized into the returned ChatCompletionToolMessageParam.content.

Suggested Reviewers

• saqadri
• rholinshead

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hopped through code one night,
Newlines stitched the tool-result right,
Text stayed simple, images wrapped in JSON snug,
Tests nibbled bugs and gave a tiny tug,
Now tool content sleeps snug and bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: converting OpenAI tool result content from a list to a single string representation.
Linked Issues check	✅ Passed	The PR directly addresses issue #25 by serializing tool result content to a single string instead of a list, and includes test coverage for the fix as specified.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the OpenAI tool result content serialization issue; no out-of-scope modifications were introduced.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/workflows/llm/test_augmented_llm_openai.py (1)

372-397: ⚡ Quick win

Consider adding test coverage for additional edge cases.

The current test validates the main scenario (multiple text parts joined with newlines), but consider adding tests for:

• Empty content list
• Single TextContent item
• Non-text content (ImageContent, EmbeddedResource) that should be JSON-encoded
• Mixed text and non-text content

These additional cases would improve confidence in the helper's robustness.

💡 Suggested additional test cases

`@pytest.mark.asyncio`
async def test_execute_tool_call_with_empty_content(self, mock_llm):
    """Test that empty tool result content is handled gracefully."""
    tool_call = ChatCompletionMessageToolCall(
        id="tool_123",
        type="function",
        function={"name": "test_tool", "arguments": json.dumps({"query": "test"})},
    )
    mock_llm.call_tool = AsyncMock(
        return_value=MagicMock(
            content=[],
            isError=False,
        )
    )
    
    result = await mock_llm.execute_tool_call(tool_call)
    
    assert result["content"] == ""  # or "[No content in tool result]" if you adopt that pattern

`@pytest.mark.asyncio`
async def test_execute_tool_call_with_non_text_content(self, mock_llm):
    """Test that non-text content is JSON-encoded in tool result."""
    from mcp.types import ImageContent
    
    tool_call = ChatCompletionMessageToolCall(
        id="tool_123",
        type="function",
        function={"name": "test_tool", "arguments": json.dumps({"query": "test"})},
    )
    mock_llm.call_tool = AsyncMock(
        return_value=MagicMock(
            content=[
                ImageContent(type="image", data="base64data", mimeType="image/png"),
            ],
            isError=False,
        )
    )
    
    result = await mock_llm.execute_tool_call(tool_call)
    
    # Should be JSON-encoded
    assert result["role"] == "tool"
    assert isinstance(result["content"], str)
    # Verify it's valid JSON and contains expected structure
    import json
    content_obj = json.loads(result["content"])
    assert content_obj["type"] == "image_url"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/workflows/llm/test_augmented_llm_openai.py` around lines 372 - 397, Add
extra pytest.asyncio test cases to
tests/workflows/llm/test_augmented_llm_openai.py covering edge cases for
mock_llm.execute_tool_call: create tests named e.g.
test_execute_tool_call_with_empty_content,
test_execute_tool_call_with_single_text_content,
test_execute_tool_call_with_non_text_content, and
test_execute_tool_call_with_mixed_content that mirror the existing
test_execute_tool_call_returns_string_content pattern; for each, set
mock_llm.call_tool = AsyncMock(return_value=MagicMock(content=...,
isError=False)), use ChatCompletionMessageToolCall as in the original, then
assert result["role"] == "tool", result["tool_call_id"] == "tool_123", and
validate result["content"] is "" for empty, equals the single text string for
single-text, is a JSON-encoded string for non-text types (e.g.
ImageContent/EmbeddedResource) and that mixed text+non-text joins text with
newlines while JSON-encoding non-text parts or producing a predictable combined
string per the helper’s spec.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/mcp_agent/workflows/llm/augmented_llm_openai.py`:
- Around line 1192-1204: mcp_content_to_openai_tool_result_content currently
returns an empty string for an empty iterable; update it to return the same
fallback used elsewhere ("[No content in tool result]") for consistency. In the
function mcp_content_to_openai_tool_result_content, after collecting text_parts
from TextContent, ImageContent, and EmbeddedResource (use the existing
json.dumps(mcp_content_to_openai_content_part(...)) logic), detect when
text_parts is empty and return "[No content in tool result]" instead of "". Keep
all existing handling for non-empty content unchanged so only the empty-case
behavior is altered to match multipart_converter_openai.py's convention.

---

Nitpick comments:
In `@tests/workflows/llm/test_augmented_llm_openai.py`:
- Around line 372-397: Add extra pytest.asyncio test cases to
tests/workflows/llm/test_augmented_llm_openai.py covering edge cases for
mock_llm.execute_tool_call: create tests named e.g.
test_execute_tool_call_with_empty_content,
test_execute_tool_call_with_single_text_content,
test_execute_tool_call_with_non_text_content, and
test_execute_tool_call_with_mixed_content that mirror the existing
test_execute_tool_call_returns_string_content pattern; for each, set
mock_llm.call_tool = AsyncMock(return_value=MagicMock(content=...,
isError=False)), use ChatCompletionMessageToolCall as in the original, then
assert result["role"] == "tool", result["tool_call_id"] == "tool_123", and
validate result["content"] is "" for empty, equals the single text string for
single-text, is a JSON-encoded string for non-text types (e.g.
ImageContent/EmbeddedResource) and that mixed text+non-text joins text with
newlines while JSON-encoding non-text parts or producing a predictable combined
string per the helper’s spec.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 01347a23-0f36-43e0-93b9-46bf3f7d27e6

📥 Commits

Reviewing files that changed from the base of the PR and between f62d849 and c89489a.

📒 Files selected for processing (2)

• src/mcp_agent/workflows/llm/augmented_llm_openai.py
• tests/workflows/llm/test_augmented_llm_openai.py