feat(mcp): inline $defs in generated inputSchema (closes #208)#244
Merged
feat(mcp): inline $defs in generated inputSchema (closes #208)#244
Conversation
Pydantic emits nested models as {"\$ref": "#/\$defs/Name"} with the
shape under \$defs. Spec-valid JSON Schema, but the MCP client
ecosystem is mixed — several popular consumers (including cheaper
agent runtimes seen in validation runs) don't implement \$ref
resolution. Tool discovery that looks correct in MCP Inspector
shows up as {} to those clients, producing silent "this tool takes
no params" confusion.
Add _inline_refs: recursively resolves every local \$ref into a deep
copy of the referenced \$defs body, then drops \$defs when every
reference was resolved. Sibling keys on the \$ref node (description,
title) override the resolved body's same-named keys per JSON Schema
2020-12 §8.2. Cycles are protected — Pydantic doesn't emit them
today, but the guard keeps a future cyclic shape from turning
inlining into a RecursionError.
Applied once at schema-gen time so every tool in
ADCP_TOOL_DEFINITIONS ships flat. Verified: all 57 tools now have
zero \$ref and zero \$defs in their advertised inputSchema.
- 12 new tests in test_mcp_schema_drift.py: no-\$ref invariant,
no-\$defs invariant, direct replacement, nested resolution, sibling-
key override, cycle protection, dangling ref, external-ref
ignored, required-array preservation, arrays-of-refs, no-mutation
of input, end-to-end jsonschema validation of a real payload.
- Existing 4 schema-drift tests still pass unchanged.
- External refs (http://...) are deliberately left alone — Pydantic
doesn't emit them, but if one ever shows up, silent stripping
would corrupt the schema.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code reviewer flagged three should-fixes — all addressed: S1: §8.2 misread. The docstring claimed "JSON Schema 2020-12 §8.2 composition semantics" for sibling-key override, but §8.2 actually says siblings are evaluated as an implicit allOf (AND the constraints) — not "last-write-wins override." For description / title (annotations, not assertions) the distinction is invisible, so real behavior is unchanged. Fixed: - Dropped the spec citation from the docstring. - Clarified that the merge is annotation-level and matches what Pydantic actually emits at ref sites (not the spec's general composition rule). Flagged the future-Pydantic risk. - Renamed test to test_inline_refs_sibling_annotations_override_ resolved_body to reflect the actual semantics. S2: Composition-keyword test gap. The walker handles \$ref inside anyOf / oneOf / allOf / additionalProperties correctly (generic dict/list recursion), but no test pinned that behavior. Added: - test_inline_refs_resolves_inside_anyof (Pydantic emits anyOf + \$ref for Optional[Model] — real production case). - test_inline_refs_resolves_inside_additional_properties (dict[str, NestedModel] on request fields). S3: json.dumps substring check was fragile. "\$ref" substring could false-positive on a description containing the literal word, or an enum / const with "\$ref" as a value. Replaced with an unresolved flag tracked during the walk — no stringification, no false positives. Also added test_inline_refs_does_not_false_positive_on_ ref_as_value as a regression guard for description-with-literal. import json was the only remaining json usage in mcp_tools.py; removing the substring check removed the import too. Micro-nit: moved import copy to the module top rather than inside the function body. 19 schema-drift tests pass (4 existing + 15 inliner tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #208 — small DX win for MCP clients that don't resolve JSON Schema references.
Problem
Pydantic emits nested models as `{"$ref": "#/$defs/Name"}` with the shape under `$defs`. Spec-valid, but a surprisingly large slice of the MCP client ecosystem doesn't implement `$ref` resolution — those clients see `{"$ref": ...}` as an empty object, reporting "this tool takes no params" to users.
Fix
Add `_inline_refs`: recursively resolves every local `$ref` into a deep copy of the referenced `$defs` body, then drops `$defs` when every reference resolved. Sibling keys on the `$ref` node (description, title) override the resolved body's same-named keys per JSON Schema 2020-12 §8.2. Cycles guarded — Pydantic doesn't emit them today, but the guard keeps a future cyclic shape from turning inlining into a RecursionError.
Applied at schema-gen time. All 57 tools now have zero `$ref` and zero `$defs` in their advertised `inputSchema` — verified in the new invariant tests.
Design decisions
Test plan
🤖 Generated with Claude Code