Skip to content

Conversation

@jlowin
Copy link
Owner

@jlowin jlowin commented Apr 30, 2025

Copilot AI review requested due to automatic review settings April 30, 2025 21:30
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 updates JSON serialization throughout the codebase to use pydantic_core.to_json with pretty‐printed (indented) output instead of the previous json.dumps approach. Key changes include updating test expectations for formatted output in multiple tests and refactoring conversion logic in the server and resource modules to use the new serialization method.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/tools/test_tool_manager.py Updates expected JSON formatting in tool manager tests.
tests/server/test_server_interactions.py Adjusts expected JSON formatting for various tool return types (including bytes, UUIDs, etc.).
tests/server/test_mount.py Updates expected JSON formatting for mounted resource responses.
tests/resources/test_resource_template.py Updates expected JSON formatting for resource template tests.
tests/resources/test_function_resources.py Modifies expected output in function resource tests to reflect pretty-printed JSON.
src/fastmcp/tools/tool.py Replaces the manual conversion and try/except block with a direct pydantic_core.to_json call.
src/fastmcp/server/server.py Refactors prompt handling to construct PromptMessages explicitly, removing earlier JSON conversion.
src/fastmcp/resources/types.py Updates resource conversion logic to use pydantic_core.to_json for non-str results.
src/fastmcp/prompts/prompt.py Replaces json.dumps with pydantic_core.to_json in prompt rendering for consistent formatting.
Comments suppressed due to low confidence (1)

tests/tools/test_tool_manager.py:390

  • Consider comparing the deserialized JSON (using json.loads) rather than asserting exact string equality on pretty printed output, to make the tests less fragile against formatting changes.
assert result[0].text == '[\n  "rex",\n  "gertrude"\n]'

return [TextContent(type="text", text=json.dumps(jsonable_result))]
except Exception:
result = str(result)
result = pydantic_core.to_json(result, fallback=str, indent=2).decode()
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

While the fallback is supplied, the removal of the try/except block means any unexpected serialization error will now propagate. Consider reintroducing error handling or logging around this conversion to help diagnose issues in production scenarios.

Copilot uses AI. Check for mistakes.
@jlowin jlowin added the enhancement Improvement to existing functionality. For issues and smaller PR improvements. label Apr 30, 2025
@jlowin jlowin merged commit 517b535 into main Apr 30, 2025
4 checks passed
@jlowin jlowin deleted the json branch April 30, 2025 22:12
jordicore pushed a commit to jordicore/fastmcp that referenced this pull request Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to existing functionality. For issues and smaller PR improvements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants