[Server] Add PropertyDescriberInterface extension point for SchemaGenerator#314
[Server] Add PropertyDescriberInterface extension point for SchemaGenerator#314peter-si wants to merge 2 commits into
Conversation
peter-si
force-pushed
the
feat/property-describer-extension-point
branch
from
3f148f3 to
cd2ef84
Compare
Lets callers teach SchemaGenerator how to render specific value-object
types (DateTime, Uuid, Money, ...) as more useful JSON Schema fragments
than the generic `{type: "object"}` fallback. Describers are consulted,
in order, for class-typed parameters before generic class inspection.
The first non-null result wins; description / default / nullable are
layered onto the described schema without overwriting it.
Ships two default describers in `Mcp\Capability\Discovery\PropertyDescriber\`:
- DateTimePropertyDescriber → {type: "string", format: "date-time"}
- UuidPropertyDescriber → {type: "string", format: "uuid"}
The new constructor parameter defaults to an empty iterable, so existing
callers stay unaffected.
peter-si
force-pushed
the
feat/property-describer-extension-point
branch
from
cd2ef84 to
972bb42
Compare
peter-si
requested review from
CodeWithKyrian,
Nyholm,
chr-hertel and
soyuka
as code owners
chr-hertel
left a comment
chr-hertel
left a comment
There was a problem hiding this comment.
Hi @peter-si,
thanks for this proposal - i like the general idea, but left some comments about the design.
also, i think it would be good to add docs and think about higher-level usage => how to add describers on the Mcp\Server\Builder.
Thanks already!
| * | ||
| * @return array<string, mixed>|null Schema fragment, or null to pass to the next describer | ||
| */ | ||
| public function describe(string $className): ?array; |
There was a problem hiding this comment.
since we're only relying on the $className and don't have any dynamic, we could be more explicit while designing this interface.
think of sth like public static function supportedClass(): class-string
=> no implicit nullable as encoded non-support
=> static index+lookup instead of iterating over all describers for a property
There was a problem hiding this comment.
Changed the interface to static supportedClass(): class-string plus a non-nullable describe(): array, so support is declared explicitly rather than encoded as a null return. The generator matches a parameter's class against supportedClass() via is_a() (so \DateTimeInterface still covers \DateTimeImmutable, Uuid covers UuidV4, etc.) and memoizes the resolution per concrete class, so describers aren't re-scanned per property.
I kept an ordered scan + per-class cache rather than a flat exact-class index, because the shipped describers need to match subclasses/interfaces, which an exact-key map can't express.
| { | ||
| } | ||
|
|
||
| public function uuidParam(\Symfony\Component\Uid\Uuid $bookingId): void |
There was a problem hiding this comment.
please use an import here for the type
There was a problem hiding this comment.
Done - added use Symfony\Component\Uid\Uuid;; the fixture parameter is now Uuid $bookingId.
chr-hertel
added
Server
chr-hertel
changed the title
…ration Rework the describer extension point per review feedback: - Replace `describe(string $className): ?array` with an explicit `static supportedClass(): class-string` + non-nullable `describe(): array`; support is declared, not encoded as a null return. - SchemaGenerator matches a parameter's class against `supportedClass()` via `is_a()` (subtypes included), materializes the describer iterable once so an injected Generator isn't exhausted across parameters, and memoizes resolution per concrete class. - Add `Builder::addPropertyDescriber()` (opt-in; consulted in registration order, first match wins); mutually exclusive with setSchemaGenerator(). - Use a `use` import for Uuid in the test fixture. - Document the feature under Schema Generation and Validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Hi @chr-hertel thanks for the review. I've pushed_ cfad5b4 addressing the feedback:
Kept it simple for now - no priority system; happy to add one if you'd prefer it over registration order. |
|
Thanks for the update - i wonder tho if this is usable already or rather incomplete - how would you use this feature? |
2 participants
Summary
Adds a
PropertyDescriberInterfaceextension point toSchemaGeneratorso callers can teach it how to render specific class types (DateTime,Uuid, domain value objects) as targeted JSON Schema fragments rather than the generic{type: "object"}fallback.Describers are consulted, in order, for class-typed parameters before generic class inspection. The first non-null result wins; parameter-level metadata (description, default, nullable) is layered onto the described schema without overwriting fields the describer already set.
Ships two default describers:
Mcp\Capability\Discovery\PropertyDescriber\DateTimePropertyDescriber— any\DateTimeInterfaceimplementation →{type: "string", format: "date-time"}Mcp\Capability\Discovery\PropertyDescriber\UuidPropertyDescriber—Symfony\Component\Uid\Uuidand subclasses →{type: "string", format: "uuid"}Why
If an MCP tool method takes e.g.
\DateTimeInterface \$until, the generator currently falls back to{type: "object"}, which tells the LLM nothing about the expected shape and forces ad-hoc workarounds in every tool. The describer chain gives a clean extension point for the long tail of value-object types every project has, without needing to subclass or fork `SchemaGenerator`.Downstream we extend the chain further with app-specific types (Money, PhoneNumber, ...), but the two shipped here are general enough that they feel like they belong upstream.
Backwards compatibility
Tests
🤖 Generated with Claude Code