Repository navigation
Replace RootModel wrappers with type aliases and TypeAdapter validation - #3470
Conversation
📚 Documentation preview
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline finding, I also checked the auth path in src/mcp/server/auth/handlers/authorize.py — the TypeAdapter(AnyUrl) swap raises the same ValidationError the existing handler catches, so redirect_uri validation behavior is unchanged — and the _wire_fields RootModel-branch removal in src/mcp/client/session.py, which is safe now that the TID251 ban (with the generated-files exemption removed) keeps RootModel rows out of the surface types.
Extended reasoning...
A finding on the FuncMetadata.output_model compatibility break is already posted inline, so this note only records what else was concretely examined and ruled out: the authorization handler's redirect_uri validation (exception type and catch site unchanged), the elicitation property-schema gate (same PrimitiveSchemaDefinition validation via adapter), and the client session's wire-field computation (the dropped RootModel-unwrapping branch is dead once no generated type is a RootModel, which the widened lint ban enforces). The remaining bulk of the diff is regenerated code plus generator changes, which the inline finding and a maintainer's judgment on the API-contract question should govern.
2 verified lower-impact observations (convention, logging or cleanup points) were not posted.
There was a problem hiding this comment.
🔴 For dict[str, T]-returning tools, the documented public field FuncMetadata.output_model now holds an Annotated[...] alias instead of the BaseModel subclass it held on every released 2.x version, so existing user code calling meta.output_model.model_validate(...), model_json_schema() or issubclass(meta.output_model, BaseModel) crashes with AttributeError/TypeError after a routine minor upgrade — AGENTS.md says 2.x observable behaviour must not change even with shims. Fix: keep dict outputs class-shaped without RootModel (e.g. a thin wrapper exposing model_validate) or defer this to 3.0 behind a deprecation path; also update the now-stale docstring at line 317 ("synthesized model for ... dict[str, T]").
Extended reasoning...
The PR description flags this as a breaking change ("validate it with TypeAdapter") — sized against the repo contract, that note does not hold: AGENTS.md states the 2.x public API is a compatibility contract and observable-behaviour changes "should generally be avoided" even when softened by @ deprecated shims; there is no shim here. FuncMetadata is documented user-facing API (docs/servers/structured-output.md, updated in this PR, tells users to inspect output_model). On base, _create_dict_model (removed at old lines 625-638) returned a RootModel subclass named {func}DictOutput; func_metadata()'s docstring still promises "a synthesized model for ... dict[str, T] ... returns" (line 317-318). After merge, _create_output_model line 511 stores Annotated[dict[str, T], Field(title=...)], and the field annotation widened from type[Any] | None to Annotated[Any, ...] (line 123), so pyright no longer flags misuse either. Any 2.x consumer that introspects tool metadata (test harnesses, custom validation layers, tool registries re-deriving schemas) and calls a classmethod on…
Verification: normal — acknowledged in diff: the PR description's "Breaking Changes" section states "FuncMetadata.output_model for dictionary returns now holds a type annotation instead of a model class; validate it with TypeAdapter", so this is a declared trade-off put up for sign-off, not an oversight; conflicts with stated purpose: restoring a BaseModel class for root-level dict serialization would…
2 participants
Replace the reintroduced
RootModelwrappers with plain types and unions validated byTypeAdapter, following #1910. Update code generation and enforce Ruff'sTID251ban for both import paths, including generated files.How Has This Been Tested?
The full suite passes with 5,968 tests and 100% line and branch coverage;
strict-no-coverpasses. The focused suite passes all 605 tests on Python 3.10, and Ruff, Pyright, the lockfile check, and regeneration checks pass.Breaking Changes
Wire formats are preserved.
FuncMetadata.output_modelfor dictionary returns now holds a type annotation instead of a model class; validate it withTypeAdapter. TheAnyUrlModelauthorization helper is removed.AI Disclaimer
This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.