Skip to content

Do not parameterize RootModel bases in generated types - #3413

Closed
Viicos wants to merge 1 commit into
modelcontextprotocol:mainfrom
Viicos:vp/bare-rootmodel
Closed

Do not parameterize RootModel bases in generated types#3413
Viicos wants to merge 1 commit into
modelcontextprotocol:mainfrom
Viicos:vp/bare-rootmodel

Conversation

@Viicos

@Viicos Viicos commented Aug 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

When parameterized, a RootModel is usable directly as a type:

MyInt = RootModel[int]

MyInt.model_validate('1')
#> 1

However, when used as a base, the model build logic ends up running for nothing, because everything is overridden by the actual class:

class MyRoot(RootModel[int]):  # RootModel[int] is first built, then MyRoot
    root: int

By leaving models unparameterized, we gain some performance improvements on startup time: measured locally with hyperfine --warmup 3 --runs 50 'PYDANTIC_DISABLE_PLUGINS=1 python -c "from mcp_types import _types, _v2025_11_25, _v2026_07_28"':

before: 180ms, after: 170ms

I don't know if implementing a Jinja template is the right approach (there's a bit of duplication), but looks like it will be robust enough.

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Performance

Checklist

  • I am assigned to the linked issue (or it is labeled help wanted, or I'm a maintainer)
  • I have disclosed any AI assistance and can explain the change in my own words
  • 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 the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Aug 29, 2026
@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

This PR has been closed automatically. This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and this PR doesn't link an open issue yet.

  • If you're already assigned to an issue for this, add Fixes #<n> to the description and the PR will reopen on its own.
  • If there's no issue yet, please open one instead: what you ran into, why it matters for your use case, and a minimal reproduction. That context is super important to us and is what we use to decide what to prioritise.
  • If there's an issue but you're not assigned, add Fixes #<n> anyway so they're linked, then engage on the issue itself by confirming the repro or describing the approach you'd take. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. If you are assigned, this PR reopens automatically.

You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way.

CONTRIBUTING.md has the full reasoning, but in short:

  • We're a small team with very little capacity to review community PRs right now.
  • Many recent PRs are AI-generated with little human review, and reviewing one carefully still costs a maintainer as much time as it ever did. A well-described issue is usually more useful to us than the code.

Maintainers: reopen, remove missing-issue-link, or add bypass-issue-check to override.

@github-actions github-actions Bot closed this Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Viicos