Skip to content

Conversation

@maeryo
Copy link
Contributor

@maeryo maeryo commented Oct 11, 2025

Changes

  • Updated DecimalFormatSymbols to use period as decimal separator (Locale.US)
  • Fixed test expectations in testComplexInputSchemaTool:
    • formattedResult: "11,000""11.000"
    • tags: Added actual test values ["test", "calculator", "integration"]
  • Fixed test expectations in testComplexNestedSchema:
    • formattedResult: "0,00""0.00"

Motivation

  1. Standard Decimal Notation: Using a comma as a decimal separator could be confusing as it's commonly used for thousands separators in many locales. Standardizing on the period (.) follows international conventions and makes the output clearer.

  2. Test Data Correction: During the Validate PR workflow for Add metadata support to callTool method #289, discovered that the tags array in the expectedContent variable of testComplexInputSchemaTool() was incorrectly set to an empty array, which didn't match the actual input values being tested.

Copy link
Contributor

@kpavlov kpavlov left a comment

Choose a reason for hiding this comment

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

Thank you, @maeryo
I am curious, why ',' was used as decimal separator. LGTM

@maeryo
Copy link
Contributor Author

maeryo commented Oct 12, 2025

@kpavlov Ironically, to resolve the test errors occurring during the validate PR process, this PR must be force-merged. This PR includes fixes for incorrect test cases.

@maeryo maeryo requested a review from kpavlov October 17, 2025 07:09
@devcrocod devcrocod force-pushed the fix/test-calculator-format-and-tags branch from eacc8be to 4f5ca7c Compare October 20, 2025 11:18
@devcrocod devcrocod force-pushed the fix/test-calculator-format-and-tags branch from bca63e4 to ae3e11e Compare October 22, 2025 20:11
@devcrocod devcrocod merged commit 8f9f484 into modelcontextprotocol:main Oct 23, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants