Skip to content

Conversation

@joshrubin-p360
Copy link
Contributor

@joshrubin-p360 joshrubin-p360 commented Sep 30, 2025

Description

Contributors Checklist

Review Checklist

  • I have self-reviewed my changes
  • My Pull Request is ready for review

@strawgate
Copy link
Collaborator

strawgate commented Sep 30, 2025

this requires a more complex fix,

my guess is we might just need to change this:

    if any(isinstance(item, ContentBlock) for item in result):
        return [
            _convert_to_single_content_block(item, serializer)
            if not isinstance(item, ContentBlock)
            else item
            for item in result
        ]

to check for ContentBlock | Audio | Image | File in the any() clause

@joshrubin-p360
Copy link
Contributor Author

joshrubin-p360 commented Sep 30, 2025

this requires a more complex fix,

my guess is we might just need to change this:

    if any(isinstance(item, ContentBlock) for item in result):
        return [
            _convert_to_single_content_block(item, serializer)
            if not isinstance(item, ContentBlock)
            else item
            for item in result
        ]

to check for ContentBlock | Audio | Image | File in the any() clause

Yeah I saw the issue with my first fix, so I changed to only convert fastmcp types (Audio, Image, File).

May need to update some tests, I'm not sure if the failing tests is intended behavior or not.

@strawgate
Copy link
Collaborator

strawgate commented Sep 30, 2025

you will need to review the developer guide to ensure your change passes linting, formatting, and type checking and then I would recommend running the tests locally

I dont think you need to modify the isinstance check in the list comprehension

@strawgate
Copy link
Collaborator

I am happy to take over this PR if you'd like otherwise i'll leave you to it, please tag me when you're ready for a review or if you need any help

@joshrubin-p360
Copy link
Contributor Author

I am happy to take over this PR if you'd like otherwise i'll leave you to it, please tag me when you're ready for a review or if you need any help

That’d be really helpful if you took over. I don’t currently have the bandwidth to finish up the PR so that’d be really helpful, thanks!

@strawgate
Copy link
Collaborator

@joshrubin-p360 should be ready for you to test

@strawgate strawgate requested a review from Copilot October 3, 2025 20:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug in the fastmcp type conversion logic to correctly handle lists containing Image, Audio, and File types. The issue was that the condition checking for ContentBlock types was not comprehensive enough, causing mixed content lists to be incorrectly aggregated into a single TextContent block instead of being processed as individual content blocks.

  • Updated the type checking condition to include Image, Audio, and File types alongside ContentBlock
  • Added test coverage to verify the fix works correctly for lists of convertible types

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/fastmcp/tools/tool.py Fixed the type checking condition in _convert_to_content to properly handle Image, Audio, and File types
tests/tools/test_tool.py Added test case to verify conversion behavior with lists of content types

@strawgate strawgate changed the title Fix fastmcp type list to text content Fix improper conversion of list[Audio | File | Image] helpers Oct 3, 2025
@strawgate strawgate merged commit 5edbe57 into jlowin:main Oct 3, 2025
4 checks passed
@joshrubin-p360
Copy link
Contributor Author

@strawgate Thanks for getting this wrapped up!

@strawgate
Copy link
Collaborator

Let me know if you have any other issues!

@jlowin jlowin added the bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. label Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert FastMCP Types to String if no ContentBlock in List Return Type

3 participants