Skip to content

Conversation

@himanshusinghs
Copy link
Collaborator

@himanshusinghs himanshusinghs commented Nov 26, 2025

Proposed changes

This PR updates the schema of embeddingsParameters to expect a union on stringified numbers instead of numbers because OpenAPI JSON Schema supports enum of string only.

Checklist

@himanshusinghs himanshusinghs changed the title fix: fix the schema for embeddings parameter fix: fix the schema for embeddings parameter MCP-281 Nov 26, 2025
@himanshusinghs himanshusinghs marked this pull request as ready for review November 26, 2025 15:10
@himanshusinghs himanshusinghs requested a review from a team as a code owner November 26, 2025 15:10
Copilot AI review requested due to automatic review settings November 26, 2025 15:10
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 the schema for embedding parameters to ensure compatibility with OpenAPI JSON Schema requirements. The change addresses the limitation that OpenAPI JSON Schema only supports enum values as strings, not numbers.

Key Changes:

  • Updated outputDimension schema to accept stringified numbers ("256", "512", etc.) instead of numeric literals
  • Added transformation logic to parse string values back to numbers for internal use
  • Updated all test cases to use stringified dimension values

Reviewed changes

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

File Description
src/tools/mongodb/mongodbSchemas.ts Modified schema definitions to accept stringified numbers for outputDimension with transformation logic, and separated API parameters schema to maintain numeric types internally
tests/integration/tools/mongodb/read/aggregate.test.ts Updated all test cases to use stringified outputDimension values ("256" instead of 256)
tests/integration/tools/mongodb/create/insertMany.test.ts Updated test cases and commented code to use stringified outputDimension values

.transform((value): number => Number.parseInt(value))
.optional()
.default(1024),
.default("1024"),
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The default value should be applied before the transformation, not after. The .default() should come before .transform() to provide the default string value that will then be transformed to a number. Current order means the default is applied after transformation, which expects a number but receives a string.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

@himanshusinghs himanshusinghs Nov 26, 2025

Choose a reason for hiding this comment

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

@gagik do you know if the order of function application is relevant here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried using the schema as is on its own and it seemed to work fine.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 19708808618

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 80.182%

Files with Coverage Reduction New Missed Lines %
src/tools/atlas/connect/connectCluster.ts 2 79.15%
Totals Coverage Status
Change from base Build 19701626410: -0.02%
Covered Lines: 6475
Relevant Lines: 7982

💛 - Coveralls

@himanshusinghs himanshusinghs merged commit 313ee7f into main Nov 26, 2025
18 checks passed
@himanshusinghs himanshusinghs deleted the fix/MCP-281-correct-schema branch November 26, 2025 15:31
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