Skip to content

Conversation

kpavlov
Copy link
Contributor

@kpavlov kpavlov commented Sep 28, 2025

  • Add OpenAPI Generator to build system to automatically generate MCP protocol data classes from the official schema.
  • Configure code quality tools to exclude generated files
  • Add deserialization test

Motivation and Context

Writing classes by hand is error-prone, when json schema exists. See #185,
Also, autogenerating model classes ensures the SDK is in sync with the protocol and can quickly catch up with its changes (see #218)

How Has This Been Tested?

Regression tests

Breaking Changes

No

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

@kpavlov kpavlov force-pushed the kpavlov/185-codegen branch 4 times, most recently from 79831a6 to 9ab62c6 Compare September 28, 2025 11:27
@kpavlov kpavlov added the enhancement New feature or request label Sep 28, 2025
@kpavlov kpavlov requested a review from devcrocod September 28, 2025 11:31
@kpavlov kpavlov marked this pull request as ready for review September 28, 2025 11:41
@kpavlov kpavlov requested a review from e5l September 29, 2025 08:03
Copy link
Contributor

@e5l e5l left a comment

Choose a reason for hiding this comment

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

lgtm

- Add OpenAPI Generator to build system to automatically generate MCP protocol data classes from the official schema.
- Configure code quality tools to exclude generated files
- Add deserialization test
@kpavlov kpavlov force-pushed the kpavlov/185-codegen branch from 4fe34c7 to c75a8cd Compare September 29, 2025 09:10
@kpavlov kpavlov merged commit d962e28 into main Sep 29, 2025
4 checks passed
@kpavlov kpavlov deleted the kpavlov/185-codegen branch September 29, 2025 09:34
Copy link
Contributor

@devcrocod devcrocod left a comment

Choose a reason for hiding this comment

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

thanks for your contribution
I appreciate the effort, but I don’t plan to merge this pull request into the sdk for several reasons:

  • quality of the sdk public api. Our schema specification implementation is part of the public API. Current kotlin openapi generators tend to produce classes that aren't ideal for public sdk. We would end up shipping a noticeably worse api. You can see the openai-java and anthropic-java-sdk, even though they use the higher-quality stainless generator. And that choice is justified by their much larger schemas. The resulting apis are still hard to use, and maintainers have had to manually fix them and improvements.
  • build-time codegen. This pr adds generated classes quietly at build time. As example, openai-java and anthropic-java-sdk check generated sources into sdk itself, together with tests and examples
  • This pr introduces significant backward-incompatible changes without clear, tangible benefits
  • Corner cases are not covered. openapi covers a few edge cases (e.g., types-util, toJson, union). We would still have to handle these manually, which largely nullifies the benefit of the generator and can be a net negative, because such issues may slip through tests and reach a release
  • we already have schema implementation. . Each schema update we make is backward-compatible and easy to track. Small updates can be applied manually while carefully handling corner cases and API design. A generator is more likely to introduce errors that our users would discover

Generators can be useful tools and do reduce some effort. In this case, however, I believe the risks and costs outweigh the benefits.

For these reasons, I would prefer to decline this pr for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants