Skip to content

Conversation

@DaleSeo
Copy link
Contributor

@DaleSeo DaleSeo commented Dec 10, 2025

Motivation and Context

Following @alexhancock's suggestion in #566 (comment) to consolidate caching logic into the primary public method, this PR applies the same refactoring pattern to schema_for_type and cached_schema_for_type.

Previously, we had two public functions serving the same purpose:

  • schema_for_type<T>() -> JsonObject - no caching
  • cached_schema_for_type<T>() -> Arc<JsonObject> - with caching

This duplication can create confusion about which to use and led to inefficient Arc-to-JsonObject-to-Arc conversions. Following the pattern established in #566, caching is now an internal optimization detail, not part of the public API.

How Has This Been Tested?

Added a few new unit tests

Breaking Changes

Functionality remains the same with improved performance.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@github-actions github-actions bot added T-test Testing related changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels Dec 10, 2025
@DaleSeo DaleSeo marked this pull request as ready for review December 10, 2025 00:59
@alexhancock alexhancock self-requested a review December 10, 2025 01:03
Copy link
Collaborator

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

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

Yes, this is the way!

@alexhancock alexhancock merged commit 8d33b15 into modelcontextprotocol:main Dec 10, 2025
11 checks passed
@github-actions github-actions bot mentioned this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants