SEP-1330: Elicitation Enum Schema Improvements and Standards Compliance #1246
+222
−16
Conversation
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
chughtapan
force-pushed
the
enum_updates
branch
3 times, most recently
from
136b2ee to
8728fe2
Compare
Contributor
felixweinberger
left a comment
felixweinberger
left a comment
There was a problem hiding this comment.
Hi @chughtapan thanks for this - I notice you've marked this as a SEP, those are generally for enhancements to the protocol itself i.e. at github.com/mode
Is this meant as an attachment to a SEP or do you plan on creating one?
Contributor
Author
Its an attachment for modelcontextprotocol/modelcontextprotocol#1330 |
Contributor
Author
|
@felixweinberger the SEP was accepted, so this is ready for review now. |
felixweinberger
added
needs maintainer action
chughtapan
changed the title
felixweinberger
force-pushed
the
enum_updates
branch
from
fe8e693 to
6ddfee7
Compare
Contributor
Picking this up as #1542 is part of the November spec release and will be pushing to the branch here - thanks @chughtapan for starting on this! |
felixweinberger
added
enhancement
The merge commit 6dc50e2 accidentally removed two test sections: - Valid list[str] multi-select test in test_elicitation_with_optional_fields - Complete test_elicitation_with_enum_titles test function This commit restores both deleted tests.
Fix bug where Union types containing string sequences (like list[str] | None) were incorrectly rejected by elicitation schema validation. Changes: - Updated _is_primitive_field() to check for string sequences in Union types - Added try-except wrapper to _is_string_sequence() to handle non-class origins - Added test case for Optional[list[str]] validation This ensures that optional multi-select enum fields work correctly with the SEP-1330 implementation.
Add test_elicitation_sep1330_enum_schemas tool that demonstrates all 5 enum schema patterns introduced in SEP-1330: - Untitled single-select (simple enum array) - Titled single-select (oneOf with const/title) - Untitled multi-select (array with items.enum) - Titled multi-select (array with items.oneOf) - Legacy format (enum + enumNames for backward compatibility) This enables conformance testing of the SEP-1330 implementation in the Python SDK, matching the TypeScript server implementation.
felixweinberger
force-pushed
the
enum_updates
branch
from
83d1fe3 to
84066d8
Compare
felixweinberger
force-pushed
the
enum_updates
branch
from
84066d8 to
815d885
Compare
Add coverage pragmas to defensive code paths that are difficult to test but necessary for completeness: - elicitation.py: None/NoneType annotation handling (line 48) - elicitation.py: TypeError exception in _is_string_sequence (lines 72-74) - test_elicitation.py: fallback return statements in test tools These paths handle edge cases that don't occur in normal execution but provide safety for unexpected inputs.
- Rename test_elicitation_sep1330_enum_schemas to test_elicitation_sep1330_enums to match conformance test expectation - Add missing type: "string" to untitledMulti items schema for conformance test validation
felixweinberger
approved these changes
Nov 23, 2025
Contributor
felixweinberger
left a comment
felixweinberger
left a comment
There was a problem hiding this comment.
Confirmed that passes conformance tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation and Context
This PR implements the changes required to implement multi-select enums in modelcontextprotocol/modelcontextprotocol#1330
How Has This Been Tested?
All new and existing tests pass
Breaking Changes
No
Types of changes
Checklist
Additional context