Skip to content

docs(design): FastMCP native registration investigation (#209)#250

Merged
bokelley merged 1 commit into
mainfrom
bokelley/209-mcp-native-design
Apr 21, 2026
Merged

docs(design): FastMCP native registration investigation (#209)#250
bokelley merged 1 commit into
mainfrom
bokelley/209-mcp-native-design

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

  • Investigation for mcp: investigate FastMCP native Pydantic-typed tool registration to delete generation layer #209. Recommendation: stay with the current generation layer.
  • FastMCP native @tool() with a pydantic-typed param nests args under the param name; AdCP needs request fields flat at the tool-argument level. No unwrap option in FastMCP.
  • Full writeup at docs/design/mcp-native-registration.md — covers wire-shape mismatch, behavior that would need reimplementation (_inline_refs, additionalProperties: true, MRO-walk filtering, per-instance handler binding), startup-cost comparison, and re-open triggers.
  • Notes two smaller follow-up wins that are not native migrations: consolidating ADCP_TOOL_DEFINITIONS + _tool_to_request into one declarative table, and dropping unused hand-crafted inputSchema stubs.

Closes #209.

Test plan

  • Doc-only change; no code modified. Review the writeup for accuracy of the FastMCP source-level claims and the recommendation.

🤖 Generated with Claude Code

Recommendation: stay with generation layer. FastMCP native path cannot
produce the flat tool-arg surface AdCP requires without rebuilding the
generation logic at registration time. Writeup documents the wire-shape
mismatch, behavior that would need reimplementation, and re-open triggers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit d32694f into main Apr 21, 2026
10 checks passed
KonstantinMirin pushed a commit to KonstantinMirin/adcp-client-python that referenced this pull request May 13, 2026
Upstream spec added a cluster of breaking changes — most Asset classes
went from 'discriminator is implicit' to 'asset_type is required'.
Pydantic honors 'required' literally, breaking ergonomic construction:

  # Before upstream change:    TextContent(content='hello')

  # After upstream change, pre-fix:
    TextContent(asset_type='text', content='hello')  # must repeat tag

Fix: inject_literal_discriminator_defaults() in post_generate_fixes.py.
Pattern-matches AnnAssign nodes whose annotation is Literal['x'] (bare
or Annotated-wrapped) with no default, appends ` = 'x'` on the same
line. Touched 687 fields across 615 classes in 105 files — Asset
types, pricing options, webhook types, catalog types, all spec
discriminator fields.

Wire consumption unchanged. Literal type still rejects other values,
so validation strength preserved. Wire dicts with the tag present
validate as before; dicts without the tag now also work (fall through
to default) — useful for minimal clients.

Pattern-based, not value-specific: robust to spec-value churn as long
as the single-value-Literal discriminator shape holds.

Also pulls upstream PRs adcontextprotocol#250 (design doc) and adcontextprotocol#251 (A2A contextId,
+22 integration tests). All green.

+16 new tests in tests/test_literal_discriminator_defaults.py. Covers
Asset defaults, VAST/DAAST dual-discriminator defaults, validation-
strength preservation, wire-format compatibility, injector meta-tests
(skips multi-value literals, extracts single-value, skips non-Literal).

2102 passing (was 2086), mypy clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

mcp: investigate FastMCP native Pydantic-typed tool registration to delete generation layer

1 participant